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

2020-04-02 Thread Victorien Molle
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
---
 dbus/DBus-interface |   9 ++
 src/dbus.c  | 341 +++-
 src/dnsmasq.h   |   4 +-
 src/lease.c |   6 +-
 src/rfc2131.c   |  12 +-
 5 files changed, 359 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 4123391..e3c709a 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -55,6 +55,7 @@ const char* introspection_xml_template =
 "\n"
 "  \n"
 "\n"
+#ifdef HAVE_DHCP
 "\n"
 "  \n"
 "  \n"
@@ -70,7 +71,13 @@ const char* introspection_xml_template =
 "  \n"
 "  \n"
 "\n"
-#ifdef HAVE_DHCP
+"\n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"\n"
 "\n"
 "   \n"
 "   \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,285 @@ void check_dbus_listeners()
 }
 
 #ifdef HAVE_DHCP
-void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
+static unsigned char *grab_extradata_into_var(unsigned char *buf, unsigned 
char *end, DBusBasicValue *var)
+{
+  unsigned char *next = NULL;
+  char *val = NULL;
+
+  if (buf && (buf != end))
+{
+  for (next = buf; ; next++)
+  if (next == end)
+{
+  next = NULL;
+  break;
+}
+  else if (*next == 0)
+break;
+
+  if (next && (next != buf))
+  {
+char *p;
+/* No "=" in value */
+if ((p = strchr((char *)buf, '=')))
+  *p = 0;
+val = (char *)buf;
+  }
+}
+  
+  /* Assign var value */
+  if (!val)
+var->str = "";
+  else
+var->str = val;
+   
+  return next ? next + 1 : NULL;
+}
+
+/*
+  As this 

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

2020-04-02 Thread Victorien Molle
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 +, 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 
> >> ---
> >>  dbus/DBus-interface |   9 ++
> >>

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

2020-02-13 Thread Victorien Molle
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 
---
 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 =
 "\n"
 "  \n"
 "\n"
+#ifdef HAVE_DHCP
 "\n"
 "  \n"
 "  \n"
@@ -70,7 +71,13 @@ const char* introspection_xml_template =
 "  \n"
 "  \n"
 "\n"
-#ifdef HAVE_DHCP
+"\n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"\n"
 "\n"
 "   \n"
 "   \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, dic

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

2020-02-13 Thread Victorien Molle
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
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
3) if there is an IPv6 entry in the leases file, DNSMasq will emit a D-Bus 
signal
   when starting/restarting/reloading DNSMasq

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.

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 
> ---
>  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 =
>  "\n"
>  "  \n"
>  "\n"
> +#ifdef HAVE_DHCP
>  "\n"
>  "  \n"
>  "  \n"
> @@ -70,7 +71,13 @@ const char* introspection_xml_template =
>  "  \n"
>  "  \n"
>  "\n"
> -#ifdef HAVE_DHCP
> +"\n"
> +"  \n"
> +"  \n"
> +"  \n"
> +"  \n"
> +"  \n"
> +"\n"
>  "\n"
>  "   \n"
>  "   \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;
> +

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

2020-02-11 Thread Victorien Molle
Hi Simon, sorry for the late reply.

Here are my answers:

1) This is because I prefer to work with integers and not strings.
I personnaly prefer to declare enums for each action and then perform
a boolean comparison instead of doing strings comparison.
But I can use action strings if you prefer.

2) 3) and 3a) I will look at it.

4) Agree but I would like to be sure of one thing. At the start of
DNSMasq, the start_time will be stored in the 'now' variable (line 240 of
dnsmasq.c) and then in 'daemon->soa_sn' variable (line 253 of dnsmasq.c).
Is it the right way to use daemon->soa_sn when calling the 'difftime' function
outside of the 'main' function, or is there a better way to access to the 'now'
variable outside of the 'main' function?

5) For now, I included data I need for my own usage but I can embed more
data if necessary.

Regards,
Victorien

On Sun, Jan 26, 2020 at 11:52:35PM +, Simon Kelley wrote:
> Sorry to keep pushing this back at you, but I looked in more detail and
> there are things that are not yet right.
> 
> 1) Why is the action the internal arbitrary integer? Wouldn't it be
> better for this to be a string, deleted/added/updated or even the name
> of the earlier signals; DhcpLeaseAdded etc which are in the action_str
> variable already?
> 
> 2) There is a few instances of code in dbus.c which have got wrapped with
> 
> #if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
> 
> which is pointless, since HAVE_DBUS must be defined otherwise the whole
> file is omitted.
> 
> 3) You're allocating a copy of the vendorclass but that's not necessary,
> just past the pointer to the start of the extradata buffer.
> 
> 3a) For an IPv6 lease, the vendorclass is different, see the code around
> line 586 in src/helper.c and line 1783 in src/rfc3315.c, the extradata
> buffer contains a (string) vendor-id first, and then some number of
> vendor-class strings, the number is stored in lease->vendorclass_count.
> Again, you don't need to copy these, just generate pointers into the
> zero-delimited extradata buffer.
> 
> 4) The lease expiration time is in unix epoch time, except sometimes it
> isn't, if dnsmasq is compiled with   HAVE_BROKEN_RTC then it's in
> seconds since last boot. The field name in the dictionary should at
> least reflect this: but maybe it would be better to just provide the
> time until lease expiration in seconds, that's always calculated the
> same way: see helper.c line 788.
> 
> 5) Question: there's lots of other data which is exported to the script,
> as we now have an extensible way to export if via DBUS, should that data
> be included in the DBus Dict?
> 
> 
> 
> Cheers,
> 
> Simon.
> 
> 
> 
> On 23/01/2020 11:00, 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 
> > Signed-off-by: Florent Fourcot 
> > ---
> >  dbus/DBus-interface |   9 
> >  src/dbus.c  | 118 +++-
> >  src/dnsmasq.h   |   2 +-
> >  src/lease.c |   2 +-
> >  src/rfc2131.c   |  12 ++---
> >  5 files changed, 134 insertions(+), 9 deletions(-)
> > 
> > diff --git a/dbus

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

2020-01-23 Thread Victorien Molle
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 
Signed-off-by: Florent Fourcot 
---
 dbus/DBus-interface |   9 
 src/dbus.c  | 118 +++-
 src/dnsmasq.h   |   2 +-
 src/lease.c |   2 +-
 src/rfc2131.c   |  12 ++---
 5 files changed, 134 insertions(+), 9 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..c906e11 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -55,6 +55,7 @@ const char* introspection_xml_template =
 "\n"
 "  \n"
 "\n"
+#ifdef HAVE_DHCP
 "\n"
 "  \n"
 "  \n"
@@ -70,7 +71,13 @@ const char* introspection_xml_template =
 "  \n"
 "  \n"
 "\n"
-#ifdef HAVE_DHCP
+"\n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"\n"
 "\n"
 "   \n"
 "   \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,106 @@ 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[IF_NAMESIZE];
+  DBusMessage* message = NULL;
+  struct lease_info *info;
+  int fd = daemon->dhcpfd;
+#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
+  char *vendor_class;
+  int allocated = 0;
+#endif
+
+#ifdef HAVE_DHCP6
+  if (!daemon->dhcp)
+fd = daemon->dhcp6fd;
+#endif
+
+  /* Get interface name */
+  if (!indextoname(fd, lease->last_interface, interface))
+interface[0] = 0;
+
+#if defined(HAVE_SCRIPT) || defined(HAVE_DBUS)
+  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 = "";
+}
+}
+#endif
+
+  struct lease_info infos[] = {{.key = "interface", .fmt = "s", .dbus_

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

2020-01-16 Thread Victorien Molle
Hi Simon,

it's OK for 1 and 2 but I need your feedback about point 3:

Do you prefer to wrap lines that require scripting compilation option (e.g.: to 
obtain vendor_class information)?
Or do I leave the DBus code as is (including fixes of steps 1 and 2) and add 
"|| HAVE_DBUS" to all necessary parts (e.g.: add_extradata_opt function)?

Thank you.

On Fri, Jan 10, 2020 at 09:51:14PM +, Simon Kelley wrote:
> 
> 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 
> > Signed-off-by: Florent Fourcot 
> > ---
> >  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 =
> >  "  \n"
> >  "  \n"
> >  "\n"
> > +"\n"
> > +"  \n"
> > +"  \n"
> > +"  \n"
> > +"  \n"
> > +"  \n"
> > +"\n"
> >  #ifdef HAVE_DHCP
> >  "\n"
> >  "   \n"
> > @@ -98,6 +105,12 

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

2019-12-06 Thread Victorien Molle
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 
Signed-off-by: Florent Fourcot 
---
 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 =
 "  \n"
 "  \n"
 "\n"
+"\n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"  \n"
+"\n"
 #ifdef HAVE_DHCP
 "\n"
 "   \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}

Re: [Dnsmasq-discuss] [PATCH] Add interface name, vendor class if present and lease expiration time to dbus signals

2019-12-04 Thread Victorien Molle
Thanks Nicolas, I will look into the solution you are proposing.

On Wed, Dec 04, 2019 at 08:18:56PM +0100, Nicolas Cavallari wrote:
> On 04/12/2019 13:24, 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.
> > 
> > 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).
> 
> D-Bus has this disadvantage that the change you are proposing completely
> breaks the existing D-Bus API that one may be relying on, and many D-Bus
> handling libraries would be incapable of understanding both API at the time.
> 
> One solution would be to keep the old signal and also send a new signal
> with the original info plus the extra, possibly in a way that allows to
> add more over time (a dict ?).
> 
> (Also, this API should be documented in the file dbus/DBus-interface...)

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss


[Dnsmasq-discuss] [PATCH] Add interface name, vendor class if present and lease expiration time to dbus signals

2019-12-04 Thread Victorien Molle
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.

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).

Signed-off-by: Victorien Molle 
---
 src/dbus.c| 57 +--
 src/rfc2131.c |  2 +-
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/src/dbus.c b/src/dbus.c
index c0ce903..88746b2 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -59,16 +59,25 @@ const char* introspection_xml_template =
 "  \n"
 "  \n"
 "  \n"
+"  \n"
+"  \n"
+"  \n"
 "\n"
 "\n"
 "  \n"
 "  \n"
 "  \n"
+"  \n"
+"  \n"
+"  \n"
 "\n"
 "\n"
 "  \n"
 "  \n"
 "  \n"
+"  \n"
+"  \n"
+"  \n"
 "\n"
 #ifdef HAVE_DHCP
 "\n"
@@ -830,11 +839,15 @@ void check_dbus_listeners()
 #ifdef HAVE_DHCP
 void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
 {
+  char *action_str, *mac = daemon->namebuff, *interface, *vendor_class;
   DBusConnection *connection = (DBusConnection *)daemon->dbus;
+  dbus_uint64_t expires = lease->expires;
+  unsigned char *buf = lease->extradata;
   DBusMessage* message = NULL;
   DBusMessageIter args;
-  char *action_str, *mac = daemon->namebuff;
+  int allocated = 0;
   unsigned char *p;
+  struct irec *it;
   int i;
 
   if (!connection)
@@ -842,7 +855,31 @@ void emit_dbus_signal(int action, struct dhcp_lease 
*lease, char *hostname)
   
   if (!hostname)
 hostname = "";
-  
+
+  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 = "";
+}
+
 #ifdef HAVE_DHCP6
if (lease->flags & (LEASE_TA | LEASE_NA))
  {
@@ -865,19 +902,27 @@ void emit_dbus_signal(int action, struct dhcp_lease 
*lease, char *hostname)
   else if (action == ACTION_OLD)
 action_str = "DhcpLeaseUpdated";
   else
-return;
+goto out;
 
   if (!(message = dbus_message_new_signal(DNSMASQ_PATH, daemon->dbus_name, 
action_str)))
-return;
-  
+  goto out;
+
   dbus_message_iter_init_append(message, );
   
   if (dbus_message_iter_append_basic(, DBUS_TYPE_STRING, 
>addrbuff) &&
   dbus_message_iter_append_basic(, DBUS_TYPE_STRING, ) &&
-  dbus_message_iter_append_basic(, DBUS_TYPE_STRING, ))
+  dbus_message_iter_append_basic(, DBUS_TYPE_STRING, ) &&
+  dbus_message_iter_append_basic(, DBUS_TYPE_STRING, ) &&
+  dbus_message_iter_append_basic(, DBUS_TYPE_STRING, _class) &&
+  dbus_message_iter_append_basic(, DBUS_TYPE_UINT64, )
+ )
 dbus_connection_send(connection, message, NULL);
   
   dbus_message_unref(message);
+
+out:
+  if (vendor_class && allocated)
+free(vendor_class);
 }
 #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;
  
-- 
2.24.0


___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss