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

Simon Kelley simon at thekelleys.org.uk
Fri Jan 10 21:51:14 GMT 2020


I'm happy with this in principle, a few things need to be looked at

1) lease->last_interface to interface name conversion should be done
using indextoname(), see helper.c line 779 as an example.

2) the addition of the new signal to the introspection data should be
inside the #ifdef HAVE_DHCP, some existing parts of the interface are
also wrong in this respect, extra points for fixing those too :)

3) You're assuming that code within #ifdef HAVE_SCRIPT is always
included: it's possible to compile with HAVE_DBUS but not HAVE_SCRIPT
and the patch as it exists will beak that.


Cheers,

Simon.


 On 06/12/2019 09:53, 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: 3
> IP: 192.168.1.100
> Hostname: Nucleus.nucle.us
> interface: br-mgmt
> vendor_class: LaGrosseBiche
> expiration: 1575667431
> 
> Signed-off-by: Victorien Molle <victorien.molle at wifirst.fr>
> Signed-off-by: Florent Fourcot <florent.fourcot at wifirst.fr>
> ---
>  dbus/DBus-interface |   9 ++++
>  src/dbus.c          | 108 ++++++++++++++++++++++++++++++++++++++++++++
>  src/rfc2131.c       |   2 +-
>  3 files changed, 118 insertions(+), 1 deletion(-)
> 
> 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..01cf3ea 100644
> --- a/src/dbus.c
> +++ b/src/dbus.c
> @@ -70,6 +70,13 @@ const char* introspection_xml_template =
>  "      <arg name=\"hwaddr\" type=\"s\"/>\n"
>  "      <arg name=\"hostname\" type=\"s\"/>\n"
>  "    </signal>\n"
> +"    <signal name=\"DhcpLeaseNotification\">\n"
> +"      <arg name=\"notification_type\" type=\"i\"/>\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"
>  #ifdef HAVE_DHCP
>  "    <method name=\"AddDhcpLease\">\n"
>  "       <arg name=\"ipaddr\" type=\"s\"/>\n"
> @@ -98,6 +105,12 @@ struct watch {
>    struct watch *next;
>  };
>  
> +struct lease_info {
> +  char *key;
> +  char *fmt;
> +  char dbus_type;
> +  DBusBasicValue value;
> +};
>  
>  static dbus_bool_t add_watch(DBusWatch *watch, void *data)
>  {
> @@ -828,6 +841,98 @@ void check_dbus_listeners()
>  }
>  
>  #ifdef HAVE_DHCP
> +/*
> +  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, int action, struct dhcp_lease *lease, char *ipaddr, char *hwaddr, char *hostname)
> +{
> +  DBusMessageIter array, dict, iter, variant;
> +  dbus_uint64_t expires = lease->expires;
> +  unsigned char *buf = lease->extradata;
> +  dbus_int32_t act_type = action;
> +  char *interface, *vendor_class;
> +  DBusMessage* message = NULL;
> +  struct lease_info *info;
> +  int allocated = 0;
> +  struct irec *it;
> +
> +  interface = "";
> +  for (it = daemon->interfaces; it != NULL; it = it->next)
> +    {
> +      if (it->index == lease->last_interface)
> +        {
> +          interface = it->name;
> +          break;
> +        }
> +    }
> +
> +  vendor_class = "";
> +  /* As defined in rfc2131.c, the first data is the vendor class even if it is empty */
> +  if (buf && *buf != 0)
> +    {
> +      vendor_class = (char*)whine_malloc(lease->extradata_len + 1);
> +      if (vendor_class)
> +        {
> +          allocated = 1;
> +          strncpy(vendor_class, (const char*)buf, (lease->extradata_len));
> +        }
> +      else
> +        {
> +          vendor_class = "";
> +        }
> +    }
> +
> +  struct lease_info infos[] = {{.key = "interface", .fmt = "s", .dbus_type = DBUS_TYPE_STRING, .value.str = interface},
> +                               {.key = "vendor_class", .fmt = "s", .dbus_type = DBUS_TYPE_STRING, .value.str = vendor_class},
> +                               {.key = "expiration", .fmt = "t", .dbus_type = DBUS_TYPE_UINT64, .value.u64 = expires},
> +                               {NULL, NULL, DBUS_TYPE_INVALID, 0}
> +                               };
> +
> +  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_INT32, &act_type);
> +  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 (info = infos; info->key; info++) {
> +    dbus_message_iter_open_container(&array, DBUS_TYPE_DICT_ENTRY, NULL, &dict);
> +    dbus_message_iter_append_basic(&dict, DBUS_TYPE_STRING, &info->key);
> +    dbus_message_iter_open_container(&dict, DBUS_TYPE_VARIANT, info->fmt, &variant);
> +    switch (info->dbus_type)
> +      {
> +      case DBUS_TYPE_STRING:
> +        dbus_message_iter_append_basic(&variant, info->dbus_type, &info->value.str);
> +        break;
> +
> +      case DBUS_TYPE_UINT64:
> +        dbus_message_iter_append_basic(&variant, info->dbus_type, &info->value.u64);
> +        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"),
> +                                 info->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:
> +  if (vendor_class && allocated)
> +    free(vendor_class);
> +}
> +
>  void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
>  {
>    DBusConnection *connection = (DBusConnection *)daemon->dbus;
> @@ -878,6 +983,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, lease, daemon->addrbuff, mac, hostname);
>  }
>  #endif
>  
> diff --git a/src/rfc2131.c b/src/rfc2131.c
> index 0058747..0cea3c4 100644
> --- a/src/rfc2131.c
> +++ b/src/rfc2131.c
> @@ -1366,7 +1366,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
>  	      lease->flags |= LEASE_CHANGED;
>  
>  #ifdef HAVE_SCRIPT
> -	      if (daemon->lease_change_command)
> +	      if (daemon->lease_change_command || daemon->dbus)
>  		{
>  		  struct dhcp_netid *n;
>  		  
> 




More information about the Dnsmasq-discuss mailing list