Re: [Dnsmasq-discuss] ubus problem
On Mon, 2019-04-08 at 20:41 +0100, Simon Kelley wrote: > > I've to give it some thought about how we could support multiple Dnsmasq > > instances in > > combination with UBus. Not sure how the DBus implementation would handle > > this... > > It doesn't: the path is a compile-time parameter. > > It's not clear that the entities on the other end of the UBus under > openwrt could cope with multiple instances. The pragmatic solution might > be to turn Ubus off for one of them. There's one solution I can think of: making the name under which we register the UBus object configurable (with "dnsmasq" as default for backwards compatibility). It would allow multiple instances to be configured each with their own unique name. We could extend the existing `enable-ubus` flag to allow this name to be supplied from the command line/configuration file. @Simon Kelley: WDYT? ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] ubus problem
Hi, On Mon, 2019-04-08 at 16:24 +0200, e9hack wrote: > From the first I see: > daemon.info dnsmasq[1808]: Connected to system UBus > daemon.info dnsmasq[1808]: UBus support enabled: connected to system bus > > From the second I see: > daemon.err dnsmasq[1809]: Cannot add object to UBus: Invalid argument > daemon.info dnsmasq[1809]: UBus support enabled: bus connection pending > > Sometimes, the starting sequence is inverted. The error occurs all the time > on the > seconded started instance only. > > This occurs since this commit: > > commit a2b8220f4e82e454bbc0013ee83ea3220111d92e > Improved UBus supported I proposed that change. The reason you see this now is that I've added extra logging to the results of the various UBus operations. Previously, UBus failures were simply ignored giving you no clue as to why it did not work. >From your description it is quite easy to explain what is happening here: UBus >prevents you from registering objects with duplicate names: hence the "invalid argument" message. I've to give it some thought about how we could support multiple Dnsmasq instances in combination with UBus. Not sure how the DBus implementation would handle this... Regards, Jan Willem ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Improve UBus support
On Fri, 2019-03-29 at 21:52 +, Simon Kelley wrote: > 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. Good point. I've reworked my patch to only log errors once and attached it to this mail. Thanks for reviewing it! Regards, Jan Willem From ee38d5bf46a5947a978d9209aee5142a2ce452c6 Mon Sep 17 00:00:00 2001 From: Jan Willem Janssen Date: Mon, 25 Mar 2019 12:42:23 +0100 Subject: [PATCH] Improved UBus supported To: dnsmasq-discuss@lists.thekelleys.org.uk - aligned the handling of UBus connections with the DBus code as it makes it a bit easier to comprehend; - added logging to the various UBus calls to aid debugging from an enduser point of view, but be careful to not flood the logs; - show the (lack of) support for UBus in the configuration string. --- src/config.h | 4 ++ src/dnsmasq.c | 32 +++- src/dnsmasq.h | 6 +++ src/ubus.c| 138 ++ 4 files changed, 157 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..db5ef68 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -420,6 +420,16 @@ 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 +{ + daemon->ubus = NULL; + ubus_init(); +} +#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 +822,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 +1020,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 +1155,15 @@ 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) +{ + ubus_init(); +} + + check_ubus_listeners(); +} #endif check_dns_listeners(now); diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 0e409b4..11a443c 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 +void 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..c2cca22 100644 --- a/src/ubus.c +++ b/src/ubus.c @@ -20,29 +20,112 @@ #include -static struct ubus_context *ubus = NULL; static struct blob_buf b; +static int notify; +static int error_logged = 0; 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
[Dnsmasq-discuss] [PATCH] Improve UBus support
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 -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" :