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

Victorien Molle victorien.molle at wifirst.fr
Thu Apr 2 08:53:31 BST 2020


Hello Simon,

sorry for the late reply, this subject has completely popped out of my mind since I faced medical issues a few days after your reply.
Fortunately, I found a new patch on my computer that almost does what you want.
I'll look at it today, test it and try to send it if everything is fine on my side.

Have a nice day.

On Thu, Feb 20, 2020 at 11:44:58PM +0000, Simon Kelley wrote:
> 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