Re: [Dnsmasq-discuss] ubus problem

2019-04-10 Thread Jan Willem Janssen
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

2019-04-08 Thread Jan Willem Janssen
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

2019-04-02 Thread Jan Willem Janssen
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_object_type =
+  

[Dnsmasq-discuss] [PATCH] Improve UBus support

2019-03-25 Thread Jan Willem Janssen
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 = _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" :