[Dnsmasq-discuss] [PATCH] Improve UBus support
Simon Kelley
simon at thekelleys.org.uk
Fri Mar 29 21:52:53 GMT 2019
This all looks sensible, with one exception: the logging in
set_ubus_listeners() and check_ubus_listeners() and associated with the
calls to check_ubus_listeners can potentially massively span the logs -
a long lived error will log multiple lines every time dnsmasq spins its
event loop. It would be good to have rate limiting there. Set a flag
after logging the error which inhibits further errors, clear it once a
normal situation is restored.
Cheers,
Simon.
On 25/03/2019 12:12, Jan Willem Janssen wrote:
> Improve the UBus support in DNSMASQ:
>
> - Aligned the handling of UBus connections with the DBus code as it makes it a bit easier
> to comprehend and allow for automatic reconnects when the connection to UBus drops;
> - make sure that DNSMASQ can connect to UBus when running as another user than root;
> - added status checks and logging to the various UBus calls to aid debugging from an
> enduser point of view;
> - show the (lack of) support for UBus in the configuration string.
>
> ---
> src/config.h | 4 ++
> src/dnsmasq.c | 38 ++++++++++++++++-
> src/dnsmasq.h | 6 +++
> src/ubus.c | 114 ++++++++++++++++++++++++++++++++++++++++----------
> 4 files changed, 139 insertions(+), 23 deletions(-)
>
> diff --git a/src/config.h b/src/config.h
> index 203d69e..99b22eb 100644
> --- a/src/config.h
> +++ b/src/config.h
> @@ -362,6 +362,10 @@ static char *compile_opts =
> "no-"
> #endif
> "DBus "
> +#ifndef HAVE_UBUS
> +"no-"
> +#endif
> +"UBus "
> #ifndef LOCALEDIR
> "no-"
> #endif
> diff --git a/src/dnsmasq.c b/src/dnsmasq.c
> index 3f3edbd..5d89aa1 100644
> --- a/src/dnsmasq.c
> +++ b/src/dnsmasq.c
> @@ -420,6 +420,18 @@ int main (int argc, char **argv)
> die(_("DBus not available: set HAVE_DBUS in src/config.h"), NULL, EC_BADCONF);
> #endif
>
> + if (option_bool(OPT_UBUS))
> +#ifdef HAVE_UBUS
> + {
> + const char *err;
> + daemon->ubus = NULL;
> + if ((err = ubus_init()))
> + my_syslog(LOG_WARNING, _("UBus init failed: %s"), (char *)err, EC_MISC);
> + }
> +#else
> + die(_("UBus not available: set HAVE_UBUS in src/config.h"), NULL, EC_BADCONF);
> +#endif
> +
> if (daemon->port != 0)
> pre_allocate_sfds();
>
> @@ -812,6 +824,16 @@ int main (int argc, char **argv)
> }
> #endif
>
> +#ifdef HAVE_UBUS
> + if (option_bool(OPT_UBUS))
> + {
> + if (daemon->ubus)
> + my_syslog(LOG_INFO, _("UBus support enabled: connected to system bus"));
> + else
> + my_syslog(LOG_INFO, _("UBus support enabled: bus connection pending"));
> + }
> +#endif
> +
> #ifdef HAVE_DNSSEC
> if (option_bool(OPT_DNSSEC_VALID))
> {
> @@ -1000,7 +1022,7 @@ int main (int argc, char **argv)
>
> #ifdef HAVE_UBUS
> if (option_bool(OPT_UBUS))
> - set_ubus_listeners();
> + set_ubus_listeners();
> #endif
>
> #ifdef HAVE_DHCP
> @@ -1135,7 +1157,19 @@ int main (int argc, char **argv)
>
> #ifdef HAVE_UBUS
> if (option_bool(OPT_UBUS))
> - check_ubus_listeners();
> + {
> + /* if we didn't create a UBus connection, retry now. */
> + if (!daemon->ubus)
> + {
> + const char *err;
> + if ((err = ubus_init()))
> + my_syslog(LOG_WARNING, _("UBus problem: %s"), err);
> + if (daemon->ubus)
> + my_syslog(LOG_INFO, _("Connected to system UBus"));
> + }
> +
> + check_ubus_listeners();
> + }
> #endif
>
> check_dns_listeners(now);
> diff --git a/src/dnsmasq.h b/src/dnsmasq.h
> index 0e409b4..f5e154e 100644
> --- a/src/dnsmasq.h
> +++ b/src/dnsmasq.h
> @@ -1119,6 +1119,11 @@ extern struct daemon {
> #ifdef HAVE_DBUS
> struct watch *watches;
> #endif
> + /* UBus stuff */
> +#ifdef HAVE_UBUS
> + /* void * here to avoid depending on ubus headers outside ubus.c */
> + void *ubus;
> +#endif
>
> /* TFTP stuff */
> struct tftp_transfer *tftp_trans, *tftp_done_trans;
> @@ -1456,6 +1461,7 @@ void emit_dbus_signal(int action, struct dhcp_lease *lease, char
> *hostname);
>
> /* ubus.c */
> #ifdef HAVE_UBUS
> +const char *ubus_init(void);
> void set_ubus_listeners(void);
> void check_ubus_listeners(void);
> void ubus_event_bcast(const char *type, const char *mac, const char *ip, const char
> *name, const char *interface);
> diff --git a/src/ubus.c b/src/ubus.c
> index 703a60d..c58c231 100644
> --- a/src/ubus.c
> +++ b/src/ubus.c
> @@ -20,29 +20,94 @@
>
> #include <libubus.h>
>
> -static struct ubus_context *ubus = NULL;
> static struct blob_buf b;
> +static int notify;
>
> static int ubus_handle_metrics(struct ubus_context *ctx, struct ubus_object *obj,
> struct ubus_request_data *req, const char *method,
> struct blob_attr *msg);
> -static struct ubus_method ubus_object_methods[] = {
> - {.name = "metrics", .handler = ubus_handle_metrics},
> +
> +static void ubus_subscribe_cb(struct ubus_context *ctx, struct ubus_object *obj);
> +
> +static const struct ubus_method ubus_object_methods[] = {
> + UBUS_METHOD_NOARG("metrics", ubus_handle_metrics),
> };
>
> -static struct ubus_object_type ubus_object_type = UBUS_OBJECT_TYPE("dnsmasq",
> ubus_object_methods);
> +static struct ubus_object_type ubus_object_type =
> + UBUS_OBJECT_TYPE("dnsmasq", ubus_object_methods);
>
> static struct ubus_object ubus_object = {
> .name = "dnsmasq",
> .type = &ubus_object_type,
> .methods = ubus_object_methods,
> .n_methods = ARRAY_SIZE(ubus_object_methods),
> + .subscribe_cb = ubus_subscribe_cb,
> };
>
> +static void ubus_subscribe_cb(struct ubus_context *ctx, struct ubus_object *obj)
> +{
> + (void)ctx;
> +
> + my_syslog(LOG_DEBUG, _("UBus subscription callback: %s subscriber(s)"), obj-
>> has_subscribers ? "1" : "0");
> + notify = obj->has_subscribers;
> +}
> +
> +void ubus_destroy(struct ubus_context *ubus)
> +{
> + // Forces re-initialization when we're reusing the same definitions later on.
> + ubus_object.id = 0;
> + ubus_object_type.id = 0;
> +
> + ubus_free(ubus);
> + daemon->ubus = NULL;
> +}
> +
> +static void ubus_disconnect_cb(struct ubus_context *ubus)
> +{
> + int ret;
> +
> + ret = ubus_reconnect(ubus, NULL);
> + if (ret)
> + {
> + my_syslog(LOG_ERR, _("Failed to reconnect to UBus: %s"), ubus_strerror(ret));
> +
> + ubus_destroy(ubus);
> + }
> +}
> +
> +const char *ubus_init()
> +{
> + struct ubus_context *ubus = NULL;
> + int ret;
> +
> + ubus = ubus_connect(NULL);
> + if (!ubus)
> + {
> + ret = UBUS_STATUS_CONNECTION_FAILED;
> + return ubus_strerror(ret);
> + }
> +
> + ubus->connection_lost = ubus_disconnect_cb;
> +
> + ret = ubus_add_object(ubus, &ubus_object);
> + if (ret)
> + {
> + return ubus_strerror(ret);
> + }
> +
> + daemon->ubus = ubus;
> +
> + return NULL;
> +}
> +
> void set_ubus_listeners()
> {
> + struct ubus_context *ubus = (struct ubus_context *)daemon->ubus;
> if (!ubus)
> - return;
> + {
> + my_syslog(LOG_ERR, _("Cannot set UBus listeners: no connection"));
> + return;
> + }
>
> poll_listen(ubus->sock.fd, POLLIN);
> poll_listen(ubus->sock.fd, POLLERR);
> @@ -51,46 +116,51 @@ void set_ubus_listeners()
>
> void check_ubus_listeners()
> {
> + struct ubus_context *ubus = (struct ubus_context *)daemon->ubus;
> if (!ubus)
> {
> - ubus = ubus_connect(NULL);
> - if (!ubus)
> - return;
> - ubus_add_object(ubus, &ubus_object);
> + my_syslog(LOG_ERR, _("Cannot poll UBus listeners: no connection"));
> + return;
> }
>
> if (poll_check(ubus->sock.fd, POLLIN))
> ubus_handle_event(ubus);
>
> - if (poll_check(ubus->sock.fd, POLLHUP))
> + if (poll_check(ubus->sock.fd, POLLHUP | POLLERR))
> {
> - ubus_free(ubus);
> - ubus = NULL;
> + my_syslog(LOG_INFO, _("Disconnecting from UBus"));
> +
> + ubus_destroy(ubus);
> }
> }
>
> -
> static int ubus_handle_metrics(struct ubus_context *ctx, struct ubus_object *obj,
> struct ubus_request_data *req, const char *method,
> struct blob_attr *msg)
> {
> int i;
> - blob_buf_init(&b, 0);
>
> - for(i=0; i < __METRIC_MAX; i++)
> + (void)obj;
> + (void)method;
> + (void)msg;
> +
> + blob_buf_init(&b, BLOBMSG_TYPE_TABLE);
> +
> + for (i=0; i < __METRIC_MAX; i++)
> blobmsg_add_u32(&b, get_metric_name(i), daemon->metrics[i]);
>
> - ubus_send_reply(ctx, req, b.head);
> -
> - return 0;
> + return ubus_send_reply(ctx, req, b.head);
> }
>
> void ubus_event_bcast(const char *type, const char *mac, const char *ip, const char
> *name, const char *interface)
> {
> - if (!ubus || !ubus_object.has_subscribers)
> + struct ubus_context *ubus = (struct ubus_context *)daemon->ubus;
> + int ret;
> +
> + if (!ubus || !notify)
> return;
>
> - blob_buf_init(&b, 0);
> + blob_buf_init(&b, BLOBMSG_TYPE_TABLE);
> if (mac)
> blobmsg_add_string(&b, "mac", mac);
> if (ip)
> @@ -100,7 +170,9 @@ void ubus_event_bcast(const char *type, const char *mac, const char
> *ip, const c
> if (interface)
> blobmsg_add_string(&b, "interface", interface);
>
> - ubus_notify(ubus, &ubus_object, type, b.head, -1);
> + ret = ubus_notify(ubus, &ubus_object, type, b.head, -1);
> + if (!ret)
> + my_syslog(LOG_ERR, _("Failed to send UBus event: %s"), ubus_strerror(ret));
> }
>
More information about the Dnsmasq-discuss
mailing list