Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
Von meinem iPhone gesendet ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
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 ++
> >> 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
> >> del
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
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 ++
>> 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"
>> "
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
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;
> +};
>
> 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;
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
On 16/01/2020 15:00, Victorien Molle wrote:
> 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)?
The second of those, please. Basically, you should be able run
make COPTS='-DHAVE_DBUS -DNO_SCRIPT'
without error.
Cheers,
Simon.
>
> 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 @@ 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
>>> +*/
>>> +vo
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
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 @@ 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;
> >
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
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 @@ 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_mal
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
Hello, On 09/01/2020 12:10, Pali Rohár wrote: Hello! I looked at this API, but is has same problems as old one. It is not possible to extend it in future (if it would be needed). Could you please design API in way that can be extended in future? (See my previous email with details how to achieve it) Thanks for our time. Additional data are now in lease_info, a dictionary (as you suggested in patch V1). Previous values already in old signals are hardcoded, to simplify migration of users from previous to this new one. So, I'm not sure to understand your point. Could you be more specific? -- Florent Fourcot. ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
On Thursday 09 January 2020 12:17:52 Florent Fourcot wrote: > Hello, > > On 09/01/2020 12:10, Pali Rohár wrote: > > Hello! I looked at this API, but is has same problems as old one. It is > > not possible to extend it in future (if it would be needed). Could you > > please design API in way that can be extended in future? (See my > > previous email with details how to achieve it) > > Thanks for our time. Additional data are now in lease_info, a dictionary (as > you suggested in patch V1). Previous values already in old signals are > hardcoded, to simplify migration of users from previous to this new one. Ah, right. So you put there also all previous parameters from old signal (which are fixed and cannot be changed). And then new parameters are put into dictionary which is part of those old parameters. Well, this can work. > So, I'm not sure to understand your point. Could you be more specific? -- Pali Rohár pali.ro...@gmail.com ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
On Thursday 09 January 2020 11:15:09 Florent Fourcot wrote: > Hello Simon, > > On 06/12/2019 10: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. > > > > Could you have a look on this patch? It's a new notification feature, with > more DHCP details inside. We are available to improve it if needed. Hello! I looked at this API, but is has same problems as old one. It is not possible to extend it in future (if it would be needed). Could you please design API in way that can be extended in future? (See my previous email with details how to achieve it) -- Pali Rohár pali.ro...@gmail.com ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
Hello Simon, On 06/12/2019 10: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. Could you have a look on this patch? It's a new notification feature, with more DHCP details inside. We are available to improve it if needed. Best regards, -- Florent Fourcot. ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
