[Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal

Simon Kelley simon at thekelleys.org.uk
Thu Feb 20 23:44:58 GMT 2020


On 13/02/2020 13:58, Victorien Molle wrote:
> Hello Simon,
> 
> this is not the final patch. I rewrote the way to build extensible dictionary to be able to
> add a vendorclass n times when in IPv6 mode.
> 
> Moreover, as I don't really work with IPv6, I don't know if the IPv6 part of the code is correct.
> I faced multiple issues:
> 1) I don't really know how to configure dhclient to send n vendorclass identifiers

Neither do I, but it seems to be possible using  dhcpcd, search for
"vendclass" in the dhcpcd.conf manpage.

> 2) when requesting an IPv6 against DNSMasq, it does not always emit a D-Bus signal, 
>    even if I erase the content of /var/lib/misc/dnsmasq.leases

Wiping the lease database is not obvious, you need to stop dnsmasq,
erase /var/lib/misc/dnsmasq.leases then restart dnsmasq.


> 3) if there is an IPv6 entry in the leases file, DNSMasq will emit a D-Bus signal
>    when starting/restarting/reloading DNSMasq

I think that's true for IPv4 leases too. Dnsmasq calls the dhcp-script
with an "old" event for all existing leases when it restarts, and the
D-Bus signal is called under the same circumstances. It would be
possible to change this is it was not considered sensible.
> 
> About entries in the dictionary, I currently added the 'IP mode' (IPv6 or not) to
> be able to correctly parse the dictionary outside of DNSMasq (by a program which would
> receive the D-Bus signal). I also want to know if the method I used to create/populate
> the dictionary is OK for you.
> To be honest, I don't really know what I can add to this dictionary, so tell me what you
> want to include in it.

Look at the --dhcp-script entry in the man page. That has a
comprehensive list of all the data that's made available to the script
that gets run when a lease changes. The same data should go to the D-Bus
signal that happens for the same reason.

the queue_script() function in src/helper.c and the calls to
grab_extradata should give you information about how to access the
various fields.


Cheers,

Simon.

> 
> Regards,
> Victorien
> 
> On Thu, Feb 13, 2020 at 02:33:15PM +0100, Victorien Molle wrote:
>> For our usage, we need to have more informations sent over D-Bus such as the interface
>> name, the vendor class identifier and the lease expiration time.
>>
>> To achieve this, we add a new D-Bus signal "DhcpLeaseNotification" which exports the
>> requested informations as a dictionnary.
>> It also has the advantage to be flexible if someone wants to add a new entry in the
>> future.
>>
>> Note: in order to get leases extradata be populated, we enabled this feature if D-Bus
>> is enabled in configuration file (this was enabled in the past only if a script was
>> ran on leases updates).
>>
>> Here is an example of the obtained result with a Python3 program:
>>
>> ''' Define our D-Bus callback '''
>> def cb(action, ipaddr, hwaddr, hostname, info):
>>     print(f'Action: {action}')
>>     print(f'IP: {ipaddr}')
>>     print(f'Hostname: {hostname}')
>>     for k in info:
>>         print(f'{k}: {info.get(k)}')
>>
>> ''' Connect to signal DhcpLeaseNotification on interface uk.org.thekelleys.dnsmasq '''
>> DNSMasq.listen('DhcpLeaseNotification', callback=cb)
>>
>> ''' Run GLib loop '''
>> GLib.MainLoop().run()
>>
>> ''' When DNSMasq receives a DHCP request, here is the result: '''
>>
>> Action: DhcpLeaseAdded
>> IP: 192.168.1.100
>> Hostname: Nucleus.nucle.us
>> interface: br-mgmt
>> expiration: 43200
>> ipv6: 0
>> vendor_class: LaGrosseBiche
>>
>> Signed-off-by: Victorien Molle <victorien.molle at wifirst.fr>
>> ---
>>  dbus/DBus-interface |   9 ++
>>  src/dbus.c          | 215 +++++++++++++++++++++++++++++++++++++++++++-
>>  src/dnsmasq.h       |   4 +-
>>  src/lease.c         |   6 +-
>>  src/rfc2131.c       |  12 +--
>>  5 files changed, 233 insertions(+), 13 deletions(-)
>>
>> diff --git a/dbus/DBus-interface b/dbus/DBus-interface
>> index 954c5b9..ed42551 100644
>> --- a/dbus/DBus-interface
>> +++ b/dbus/DBus-interface
>> @@ -273,14 +273,23 @@ DhcpLeaseAdded
>>  ---------------
>>  
>>  This signal is emitted when a DHCP lease for a given IP address is created.
>> +This will also trigger the DhcpLeaseNotification signal.
>>  
>>  DhcpLeaseDeleted
>>  ----------------
>>  
>>  This signal is emitted when a DHCP lease for a given IP address is deleted.
>> +This will also trigger the DhcpLeaseNotification signal.
>>  
>>  DhcpLeaseUpdated
>>  ----------------
>>  
>>  This signal is emitted when a DHCP lease for a given IP address is updated.
>> +This will also trigger the DhcpLeaseNotification signal.
>> +
>> +DhcpLeaseNotification
>> +---------------------
>> +
>> +This signal is emitted when a DHCP lease action is triggered. It exports,
>> +as a dictionnary, more informations than the other signals.
>>   
>> diff --git a/src/dbus.c b/src/dbus.c
>> index c0ce903..468f393 100644
>> --- a/src/dbus.c
>> +++ b/src/dbus.c
>> @@ -55,6 +55,7 @@ const char* introspection_xml_template =
>>  "    <method name=\"SetBogusPrivOption\">\n"
>>  "      <arg name=\"boguspriv\" direction=\"in\" type=\"b\"/>\n"
>>  "    </method>\n"
>> +#ifdef HAVE_DHCP
>>  "    <signal name=\"DhcpLeaseAdded\">\n"
>>  "      <arg name=\"ipaddr\" type=\"s\"/>\n"
>>  "      <arg name=\"hwaddr\" type=\"s\"/>\n"
>> @@ -70,7 +71,13 @@ const char* introspection_xml_template =
>>  "      <arg name=\"hwaddr\" type=\"s\"/>\n"
>>  "      <arg name=\"hostname\" type=\"s\"/>\n"
>>  "    </signal>\n"
>> -#ifdef HAVE_DHCP
>> +"    <signal name=\"DhcpLeaseNotification\">\n"
>> +"      <arg name=\"notification_type\" type=\"s\"/>\n"
>> +"      <arg name=\"ipaddr\" type=\"s\"/>\n"
>> +"      <arg name=\"hwaddr\" type=\"s\"/>\n"
>> +"      <arg name=\"hostname\" type=\"s\"/>\n"
>> +"      <arg name=\"lease_info\" type=\"a{sv}\"/>\n"
>> +"    </signal>\n"
>>  "    <method name=\"AddDhcpLease\">\n"
>>  "       <arg name=\"ipaddr\" type=\"s\"/>\n"
>>  "       <arg name=\"hwaddr\" type=\"s\"/>\n"
>> @@ -98,6 +105,13 @@ struct watch {
>>    struct watch *next;
>>  };
>>  
>> +struct lease_info {
>> +  char *key;
>> +  char *fmt;
>> +  char dbus_type;
>> +  DBusBasicValue value;
>> +  struct lease_info *next;
>> +};
>>  
>>  static dbus_bool_t add_watch(DBusWatch *watch, void *data)
>>  {
>> @@ -137,6 +151,48 @@ static void remove_watch(DBusWatch *watch, void *data)
>>    w = data; /* no warning */
>>  }
>>  
>> +static struct lease_info* add_lease_info(struct lease_info *parent, char *key, char *fmt, char dbus_type, DBusBasicValue value)
>> +{
>> +  struct lease_info *info, *it;
>> +
>> +  /* Allocate new struct */
>> +  if (!(info = whine_malloc(sizeof(struct lease_info))))
>> +    return parent;
>> +
>> +  /* Populate struct */
>> +  info->key = key;
>> +  info->fmt = fmt;
>> +  info->dbus_type = dbus_type;
>> +  info->value = value;
>> +  info->next = NULL;
>> +
>> +  /*
>> +    Assign newly allocated struct to parent.
>> +    It implies this is the first lease info we want to add
>> +  */
>> +  if (!parent)
>> +    parent = info;
>> +  else
>> +  {
>> +    for (it = parent; it->next != NULL; it = it->next);
>> +    it->next = info;
>> +  }
>> +
>> +  return parent;
>> +}
>> +
>> +static void destroy_lease_infos(struct lease_info *infos)
>> +{
>> +  struct lease_info *it;
>> +
>> +  while (infos)
>> +  {
>> +    it = infos->next;
>> +    free(infos);
>> +    infos = it;
>> +  }
>> +}
>> +
>>  static void dbus_read_servers(DBusMessage *message)
>>  {
>>    DBusMessageIter iter;
>> @@ -828,7 +884,159 @@ void check_dbus_listeners()
>>  }
>>  
>>  #ifdef HAVE_DHCP
>> -void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
>> +/*
>> +  As this function is called by emit_dbus_signal, we already have access to ipaddr, hwaddr and hostname attributes
>> +  NOTE: connection attribute is currently not NULL as it is verified by emit_dbus_signal
>> +*/
>> +void emit_dhcplease_notification(DBusConnection *connection, char *action, struct dhcp_lease *lease, char *ipaddr, char *hwaddr, char *hostname, time_t now)
>> +{
>> +  DBusMessageIter array, dict, iter, variant;
>> +  unsigned char *buf = lease->extradata;
>> +  int fd = daemon->dhcpfd, cpt, is6;
>> +  struct lease_info *infos, *it;
>> +  char *vendor_class, sfmt[24];
>> +  char interface[IF_NAMESIZE];
>> +  DBusMessage* message = NULL;
>> +  DBusBasicValue value;
>> +
>> +#ifdef HAVE_DHCP6
>> +  if (!daemon->dhcp)
>> +    fd = daemon->dhcp6fd;
>> +#endif
>> +
>> +  /* Get interface name */
>> +  if (!indextoname(fd, lease->last_interface, interface))
>> +    interface[0] = 0;
>> +
>> +  /* Check if working with IPv6 */
>> +  is6 = !!(lease->flags & (LEASE_TA | LEASE_NA));
>> +
>> +  /* Start building dictionary with interface */
>> +  value.str = interface;
>> +  infos = add_lease_info(NULL, "interface", "s", DBUS_TYPE_STRING, value);
>> +
>> +  /* Add expiration time */
>> +  if (lease->expires != 0)
>> +    value.u64 = (dbus_uint64_t)difftime(lease->expires, now);
>> +  else
>> +    value.u64 = 0;
>> +  infos = add_lease_info(infos, "expiration", "t", DBUS_TYPE_UINT64, value);
>> +
>> +  /* Add flag to know if we use IPv6 or not */
>> +  value.bool_val = is6 == 1 ? TRUE : FALSE;
>> +  infos = add_lease_info(infos, "ipv6", "b", DBUS_TYPE_BOOLEAN, value);
>> +
>> +  vendor_class = "";
>> +  if (buf && *buf != 0)
>> +    {
>> +      if (!is6)
>> +      {
>> +        /* As defined in rfc2131.c, the first data is the vendor class even if it is empty */
>> +        vendor_class = (char*)lease->extradata;
>> +        value.str = vendor_class;
>> +        infos = add_lease_info(infos, "vendor_class", "s", DBUS_TYPE_STRING, value);
>> +      }
>> +#ifdef HAVE_DHCP6
>> +      else
>> +      {
>> +        value.i32 = (dbus_int32_t)lease->vendorclass_count;
>> +        infos = add_lease_info(infos, "vendorclass_count", "i", DBUS_TYPE_INT32, value);
>> +        if (lease->vendorclass_count != 0)
>> +        {
>> +          /* Add VENDOR-ID string */
>> +          value.str = (char*)lease->extradata;
>> +          infos = add_lease_info(infos, "vendor_id", "s", DBUS_TYPE_STRING, value);
>> +
>> +          /* Set pointer to the right offset */
>> +          vendor_class = (char*)(lease->extradata + strlen(value.str) + 1);
>> +
>> +          /* Add vendor_class strings */
>> +          for (cpt = 0; cpt < lease->vendorclass_count; ++cpt)
>> +          {
>> +            snprintf(sfmt, sizeof(sfmt), "vendor_class%d", cpt);
>> +            value.str = vendor_class;
>> +            infos = add_lease_info(infos, sfmt, "s", DBUS_TYPE_STRING, value);
>> +
>> +            /* Update vendor_class pointer */
>> +            vendor_class += strlen(value.str) + 1;
>> +          }
>> +        }
>> +#endif
>> +      }
>> +    }
>> +
>> +  if (!(message = dbus_message_new_signal(DNSMASQ_PATH, daemon->dbus_name, "DhcpLeaseNotification")))
>> +    goto out;
>> +
>> +  dbus_message_iter_init_append(message, &iter);
>> +  dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &action);
>> +  dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &ipaddr);
>> +  dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &hwaddr);
>> +  dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &hostname);
>> +  dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &array);
>> +  for (it = infos; it; it = it->next) {
>> +    dbus_message_iter_open_container(&array, DBUS_TYPE_DICT_ENTRY, NULL, &dict);
>> +    dbus_message_iter_append_basic(&dict, DBUS_TYPE_STRING, &it->key);
>> +    dbus_message_iter_open_container(&dict, DBUS_TYPE_VARIANT, it->fmt, &variant);
>> +    switch (it->dbus_type)
>> +      {
>> +      case DBUS_TYPE_BOOLEAN:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.bool_val);
>> +        break;
>> +
>> +      case DBUS_TYPE_INT16:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.i16);
>> +        break;
>> +
>> +      case DBUS_TYPE_UINT16:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.u16);
>> +        break;
>> +
>> +      case DBUS_TYPE_INT32:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.i32);
>> +        break;
>> +
>> +      case DBUS_TYPE_UINT32:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.u32);
>> +        break;
>> +
>> +      case DBUS_TYPE_INT64:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.i64);
>> +        break;
>> +
>> +      case DBUS_TYPE_UINT64:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.u64);
>> +        break;
>> +
>> +      case DBUS_TYPE_DOUBLE:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.dbl);
>> +        break;
>> +
>> +      case DBUS_TYPE_STRING:
>> +        dbus_message_iter_append_basic(&variant, it->dbus_type, &it->value.str);
>> +        break;
>> +
>> +      default:
>> +        dbus_message_iter_abandon_container(&dict, &variant);
>> +        dbus_message_iter_abandon_container_if_open(&array, &dict);
>> +        dbus_message_iter_abandon_container_if_open(&iter, &array);
>> +        dbus_message_unref(message);
>> +        my_syslog(LOG_WARNING, _("Unknown D-Bus type specified for key '%s'. This will completely disable 'DhcpLeaseNotification' signal"),
>> +                                 it->key);
>> +        goto out;
>> +      }
>> +    dbus_message_iter_close_container(&dict, &variant);
>> +    dbus_message_iter_close_container(&array, &dict);
>> +  }
>> +  dbus_message_iter_close_container(&iter, &array);
>> +  dbus_connection_send(connection, message, NULL);
>> +  dbus_message_unref(message);
>> +
>> +out:
>> +  destroy_lease_infos(infos);
>> +}
>> +
>> +void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname, time_t now)
>>  {
>>    DBusConnection *connection = (DBusConnection *)daemon->dbus;
>>    DBusMessage* message = NULL;
>> @@ -878,6 +1086,9 @@ void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
>>      dbus_connection_send(connection, message, NULL);
>>    
>>    dbus_message_unref(message);
>> +
>> +  /* Now emit a notification with more informations */
>> +  emit_dhcplease_notification(connection, action_str, lease, daemon->addrbuff, mac, hostname, now);
>>  }
>>  #endif
>>  
>> diff --git a/src/dnsmasq.h b/src/dnsmasq.h
>> index 8e047fc..d53fe38 100644
>> --- a/src/dnsmasq.h
>> +++ b/src/dnsmasq.h
>> @@ -1415,7 +1415,7 @@ void lease_update_from_configs(void);
>>  int do_script_run(time_t now);
>>  void rerun_scripts(void);
>>  void lease_find_interfaces(time_t now);
>> -#ifdef HAVE_SCRIPT
>> +#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
>>  void lease_add_extradata(struct dhcp_lease *lease, unsigned char *data, 
>>  			 unsigned int len, int delim);
>>  #endif
>> @@ -1465,7 +1465,7 @@ char *dbus_init(void);
>>  void check_dbus_listeners(void);
>>  void set_dbus_listeners(void);
>>  #  ifdef HAVE_DHCP
>> -void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname);
>> +void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname, time_t now);
>>  #  endif
>>  #endif
>>  
>> diff --git a/src/lease.c b/src/lease.c
>> index 081d90e..97e7b3e 100644
>> --- a/src/lease.c
>> +++ b/src/lease.c
>> @@ -1109,7 +1109,7 @@ int do_script_run(time_t now)
>>  	  queue_script(ACTION_DEL, lease, lease->old_hostname, now);
>>  #endif
>>  #ifdef HAVE_DBUS
>> -	  emit_dbus_signal(ACTION_DEL, lease, lease->old_hostname);
>> +	  emit_dbus_signal(ACTION_DEL, lease, lease->old_hostname, now);
>>  #endif
>>  	  old_leases = lease->next;
>>  	  
>> @@ -1144,7 +1144,7 @@ int do_script_run(time_t now)
>>  #endif
>>  #ifdef HAVE_DBUS
>>  	emit_dbus_signal((lease->flags & LEASE_NEW) ? ACTION_ADD : ACTION_OLD, lease,
>> -			 lease->fqdn ? lease->fqdn : lease->hostname);
>> +			 lease->fqdn ? lease->fqdn : lease->hostname, now);
>>  #endif
>>  	lease->flags &= ~(LEASE_NEW | LEASE_CHANGED | LEASE_AUX_CHANGED);
>>  	
>> @@ -1158,7 +1158,7 @@ int do_script_run(time_t now)
>>    return 0; /* nothing to do */
>>  }
>>  
>> -#ifdef HAVE_SCRIPT
>> +#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
>>  /* delim == -1 -> delim = 0, but embedded 0s, creating extra records, are OK. */
>>  void lease_add_extradata(struct dhcp_lease *lease, unsigned char *data, unsigned int len, int delim)
>>  {
>> diff --git a/src/rfc2131.c b/src/rfc2131.c
>> index 0058747..d4e9802 100644
>> --- a/src/rfc2131.c
>> +++ b/src/rfc2131.c
>> @@ -21,7 +21,7 @@
>>  #define option_len(opt) ((int)(((unsigned char *)(opt))[1]))
>>  #define option_ptr(opt, i) ((void *)&(((unsigned char *)(opt))[2u+(unsigned int)(i)]))
>>  
>> -#ifdef HAVE_SCRIPT
>> +#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
>>  static void add_extradata_opt(struct dhcp_lease *lease, unsigned char *opt);
>>  #endif
>>  
>> @@ -97,7 +97,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
>>    struct dhcp_opt *o;
>>    unsigned char pxe_uuid[17];
>>    unsigned char *oui = NULL, *serial = NULL;
>> -#ifdef HAVE_SCRIPT
>> +#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
>>    unsigned char *class = NULL;
>>  #endif
>>  
>> @@ -163,7 +163,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
>>  		  unsigned char *y = option_ptr(opt, offset + elen + 5);
>>  		  oui = option_find1(x, y, 1, 1);
>>  		  serial = option_find1(x, y, 2, 1);
>> -#ifdef HAVE_SCRIPT
>> +#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
>>  		  class = option_find1(x, y, 3, 1);		  
>>  #endif
>>  		  /* If TR069-id is present set the tag "cpewan-id" to facilitate echoing 
>> @@ -1365,8 +1365,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
>>  	      /* pick up INIT-REBOOT events. */
>>  	      lease->flags |= LEASE_CHANGED;
>>  
>> -#ifdef HAVE_SCRIPT
>> -	      if (daemon->lease_change_command)
>> +#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
>> +	      if (daemon->lease_change_command || daemon->dbus)
>>  		{
>>  		  struct dhcp_netid *n;
>>  		  
>> @@ -1661,7 +1661,7 @@ static int sanitise(unsigned char *opt, char *buf)
>>    return 1;
>>  }
>>  
>> -#ifdef HAVE_SCRIPT
>> +#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
>>  static void add_extradata_opt(struct dhcp_lease *lease, unsigned char *opt)
>>  {
>>    if (!opt)
>> -- 
>> 2.24.0
>>
> 




More information about the Dnsmasq-discuss mailing list