[Dnsmasq-discuss] [PATCH] Add new extensible D-Bus signal
Simon Kelley
simon at thekelleys.org.uk
Fri Jan 17 18:51:49 GMT 2020
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 +0000, 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 <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;
>>>
>>>
>>
>>
>> _______________________________________________
>> Dnsmasq-discuss mailing list
>> Dnsmasq-discuss at lists.thekelleys.org.uk
>> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
>
More information about the Dnsmasq-discuss
mailing list