Re: [PATCH 4/5] initd: Don't search the environment list if the watchdog, fd is initialized

2020-09-29 Thread Michael Jones
On Tue, Sep 29, 2020 at 1:59 PM John Crispin  wrote:
>
>
> On 29.09.20 20:55, Michael Jones wrote:
> > On Tue, Sep 29, 2020 at 1:47 PM John Crispin  wrote:
> >>
> >> On 29.09.20 18:22, Michael Jones wrote:
> >>> Signed-off-by: Michael Jones 
> >>> ---
> >>>watchdog.c | 4 ++--
> >>>1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/watchdog.c b/watchdog.c
> >>> index 20830c3..ac5b656 100644
> >>> --- a/watchdog.c
> >>> +++ b/watchdog.c
> >>> @@ -49,11 +49,11 @@ static void watchdog_timeout_cb(struct uloop_timeout 
> >>> *t)
> >>>
> >>>static int watchdog_open(bool cloexec)
> >>>{
> >>> -char *env = getenv("WDTFD");
> >>> -
> >>>if (wdt_fd >= 0)
> >>>return wdt_fd;
> >>>
> >>> +char *env = getenv("WDTFD");
> >>> +
> >>>if (env) {
> >>>DEBUG(2, "Watchdog handover: fd=%s\n", env);
> >>>wdt_fd = atoi(env);
> >> this breaks c99 compliance
> >>
> >>   John
> >>
> > Do you mean C89 compliance? This should compile just fine in C99.
> >
> > C99 was released 20 years ago, and C89 30 years ago. I'm personally
> > not interested in supporting either.
> >
> > The patch can be modified, or used as inspiration, by someone who is
> > concerned about C89/C99 compliance and would like to see the
> > watchdog_open() function improved in this way.
>
>
> variable declarations should always be at the start of the function.
>
>  John
>

Sorry, I disagree, and there was no documentation that I could see
anywhere related to this on either the OpenWRT wiki, or the procd
repository.

Take the patches or leave them. I'm not going to make updates.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH 4/5] initd: Don't search the environment list if the watchdog, fd is initialized

2020-09-29 Thread Michael Jones
On Tue, Sep 29, 2020 at 1:47 PM John Crispin  wrote:
>
>
> On 29.09.20 18:22, Michael Jones wrote:
> > Signed-off-by: Michael Jones 
> > ---
> >   watchdog.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/watchdog.c b/watchdog.c
> > index 20830c3..ac5b656 100644
> > --- a/watchdog.c
> > +++ b/watchdog.c
> > @@ -49,11 +49,11 @@ static void watchdog_timeout_cb(struct uloop_timeout *t)
> >
> >   static int watchdog_open(bool cloexec)
> >   {
> > -char *env = getenv("WDTFD");
> > -
> >   if (wdt_fd >= 0)
> >   return wdt_fd;
> >
> > +char *env = getenv("WDTFD");
> > +
> >   if (env) {
> >   DEBUG(2, "Watchdog handover: fd=%s\n", env);
> >   wdt_fd = atoi(env);
>
> this breaks c99 compliance
>
>  John
>

Do you mean C89 compliance? This should compile just fine in C99.

C99 was released 20 years ago, and C89 30 years ago. I'm personally
not interested in supporting either.

The patch can be modified, or used as inspiration, by someone who is
concerned about C89/C99 compliance and would like to see the
watchdog_open() function improved in this way.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH 1/5] initd: Add ubus argument to trigger watchdog tickle.

2020-09-29 Thread Michael Jones
Signed-off-by: Michael Jones 
---
 system.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/system.c b/system.c
index 0fb98f1..ef7943a 100644
--- a/system.c
+++ b/system.c
@@ -325,6 +325,7 @@ enum {
 WDT_TIMEOUT,
 WDT_MAGICCLOSE,
 WDT_STOP,
+    WDT_TICKLE,
 __WDT_MAX
 };
 
@@ -333,6 +334,7 @@ static const struct blobmsg_policy
watchdog_policy[__WDT_MAX] = {
 [WDT_TIMEOUT] = { .name = "timeout", .type = BLOBMSG_TYPE_INT32 },
 [WDT_MAGICCLOSE] = { .name = "magicclose", .type = BLOBMSG_TYPE_BOOL },
 [WDT_STOP] = { .name = "stop", .type = BLOBMSG_TYPE_BOOL },
+    [WDT_TICKLE] = { .name = "tickle", .type = BLOBMSG_TYPE_BOOL },
 };
 
 static int watchdog_set(struct ubus_context *ctx, struct ubus_object *obj,
@@ -367,6 +369,9 @@ static int watchdog_set(struct ubus_context *ctx,
struct ubus_object *obj,
     watchdog_timeout(timeout);
 }
 
+    if (tb[WDT_TICKLE] && blobmsg_get_bool(tb[WDT_TICKLE]))
+        watchdog_ping();
+
 if (tb[WDT_MAGICCLOSE])
     watchdog_set_magicclose(blobmsg_get_bool(tb[WDT_MAGICCLOSE]));
 
-- 
2.26.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH 5/5] initd: Convert the watchdog_fd return value from char* to int

2020-09-29 Thread Michael Jones
This change improves the frequently called path of determining
if the watchdog is alive when responding to ubus transactions
at the expense of complicating the less frequently called code
of transitioning to the upgraded binary, and transitioning from
pre-init to procd.

Signed-off-by: Michael Jones 
---
 initd/preinit.c | 11 ---
 system.c    |  2 +-
 sysupgrade.c    |  8 +---
 watchdog.c  | 11 ++-
 watchdog.h  |  6 +++---
 5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/initd/preinit.c b/initd/preinit.c
index 9dfe5c1..255ceeb 100644
--- a/initd/preinit.c
+++ b/initd/preinit.c
@@ -90,7 +90,7 @@ fail:
 static void
 spawn_procd(struct uloop_process *proc, int ret)
 {
-    char *wdt_fd = watchdog_fd();
+    int wdt_fd = watchdog_fd();
 char *argv[] = { "/sbin/procd", NULL};
 char dbg[2];
 
@@ -104,8 +104,13 @@ spawn_procd(struct uloop_process *proc, int ret)
 check_sysupgrade();
 
 DEBUG(2, "Exec to real procd now\n");
-    if (wdt_fd)
-        setenv("WDTFD", wdt_fd, 1);
+
+    if (wdt_fd >= 0) {
+        char fd_buf[12];
+        snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd);
+        setenv("WDTFD", fd_buf, 1);
+    }
+
 check_dbglvl();
 if (debug > 0) {
     snprintf(dbg, 2, "%d", debug);
diff --git a/system.c b/system.c
index ef7943a..caae0ec 100644
--- a/system.c
+++ b/system.c
@@ -378,7 +378,7 @@ static int watchdog_set(struct ubus_context *ctx,
struct ubus_object *obj,
 if (tb[WDT_STOP])
     watchdog_set_stopped(blobmsg_get_bool(tb[WDT_STOP]));
 
-    if (watchdog_fd() == NULL)
+    if (watchdog_fd() < 0)
     status = "offline";
 else if (watchdog_get_stopped())
     status = "stopped";
diff --git a/sysupgrade.c b/sysupgrade.c
index fc588b0..20e1ef0 100644
--- a/sysupgrade.c
+++ b/sysupgrade.c
@@ -29,7 +29,7 @@ void sysupgrade_exec_upgraded(const char *prefix, char
*path,
           const char *backup, char *command,
           struct blob_attr *options)
 {
-    char *wdt_fd = watchdog_fd();
+    int wdt_fd = watchdog_fd();
 char *argv[] = { "/sbin/upgraded", NULL, NULL, NULL};
 struct blob_attr *option;
 int rem;
@@ -44,9 +44,11 @@ void sysupgrade_exec_upgraded(const char *prefix,
char *path,
 argv[1] = path;
 argv[2] = command;
 
-    if (wdt_fd) {
+    if (wdt_fd >= 0) {
+        char fd_buf[12];
+        snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd);
     watchdog_set_cloexec(false);
-        setenv("WDTFD", wdt_fd, 1);
+        setenv("WDTFD", fd_buf, 1);
 }
 
 if (backup)
diff --git a/watchdog.c b/watchdog.c
index ac5b656..3c565d2 100644
--- a/watchdog.c
+++ b/watchdog.c
@@ -146,16 +146,9 @@ int watchdog_frequency(int frequency)
 return wdt_frequency;
 }
 
-char* watchdog_fd(void)
+int watchdog_fd(void)
 {
-    static char fd_buf[12];
-
-    if (wdt_fd < 0)
-        return NULL;
-
-    snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd);
-
-    return fd_buf;
+    return wdt_fd;
 }
 
 void watchdog_init(int preinit)
diff --git a/watchdog.h b/watchdog.h
index 73c75d5..3c35d9a 100644
--- a/watchdog.h
+++ b/watchdog.h
@@ -19,7 +19,7 @@
 
 #ifndef DISABLE_INIT
 void watchdog_init(int preinit);
-char* watchdog_fd(void);
+int watchdog_fd(void);
 int watchdog_timeout(int timeout);
 int watchdog_frequency(int frequency);
 void watchdog_set_magicclose(bool val);
@@ -33,9 +33,9 @@ static inline void watchdog_init(int preinit)
 {
 }
 
-static inline char* watchdog_fd(void)
+static inline int watchdog_fd(void)
 {
-    return "";
+    return -1;
 }
 
 static inline int watchdog_timeout(int timeout)
-- 
2.26.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH 2/5] initd: Re-use the watchdog_set_cloexec() function from, watchdog_open()

2020-09-29 Thread Michael Jones
Signed-off-by: Michael Jones 
---
 watchdog.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/watchdog.c b/watchdog.c
index 9d770b4..20b6e20 100644
--- a/watchdog.c
+++ b/watchdog.c
@@ -65,8 +65,7 @@ static int watchdog_open(bool cloexec)
 if (wdt_fd < 0)
     return wdt_fd;
 
-    if (cloexec)
-        fcntl(wdt_fd, F_SETFD, fcntl(wdt_fd, F_GETFD) | FD_CLOEXEC);
+    watchdog_set_cloexec(cloexec);
 
 return wdt_fd;
 }
-- 
2.26.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH 3/5] initd: Ensure that watchdog frequency changes apply right away

2020-09-29 Thread Michael Jones
If the watchdog frequency is changed from high to low,
the watchdog won't be tickled again until the previous
period has expired, which may result in a watchdog timeout.
This change ensures that the new frequency is applied immediately.

Signed-off-by: Michael Jones 
---
 watchdog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/watchdog.c b/watchdog.c
index 20b6e20..20830c3 100644
--- a/watchdog.c
+++ b/watchdog.c
@@ -140,6 +140,7 @@ int watchdog_frequency(int frequency)
 if (frequency) {
     DEBUG(4, "Set watchdog frequency: %ds\n", frequency);
     wdt_frequency = frequency;
+        watchdog_timeout_cb(_timeout);
 }
 
 return wdt_frequency;
-- 
2.26.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH 4/5] initd: Don't search the environment list if the watchdog, fd is initialized

2020-09-29 Thread Michael Jones
Signed-off-by: Michael Jones 
---
 watchdog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/watchdog.c b/watchdog.c
index 20830c3..ac5b656 100644
--- a/watchdog.c
+++ b/watchdog.c
@@ -49,11 +49,11 @@ static void watchdog_timeout_cb(struct uloop_timeout *t)
 
 static int watchdog_open(bool cloexec)
 {
-    char *env = getenv("WDTFD");
-
 if (wdt_fd >= 0)
     return wdt_fd;
 
+    char *env = getenv("WDTFD");
+
 if (env) {
     DEBUG(2, "Watchdog handover: fd=%s\n", env);
     wdt_fd = atoi(env);
-- 
2.26.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH 0/5] Improvements to the hardware watchdog code of procd

2020-09-29 Thread Michael Jones
This set of patches makes various minor improvements to the hardware
watchdog code of procd.

The first patch adds a new feature to allow the watchdog to be manually
tickled via ubus.

The other patches are minor optimizations and improvements.

Michael Jones (5):
  initd: Add ubus argument to trigger watchdog tickle.
  initd: Re-use the watchdog_set_cloexec() function from watchdog_open()
  initd: Ensure that watchdog frequency changes apply right away
  initd: Don't search the environment list if the watchdog fd is initialized
  initd: Convert the watchdog_fd return value from char* to int

 initd/preinit.c | 11 ---
 system.c    |  7 ++-
 sysupgrade.c    |  8 +---
 watchdog.c  | 19 ++-
 watchdog.h  |  6 +++---
 5 files changed, 28 insertions(+), 23 deletions(-)

-- 
2.26.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


UCI: Making a config read-only?

2020-08-26 Thread Michael Jones
I wish to make a configuration file read-only in UCI.

I tried to do this by bind-mounting the default config for it in /rom
onto the file in /etc/config.

However, while this does prevent changes to that config from
persisting across reboots, it doesn't prevent changes from showing up
in, e.g. "uci get" or "uci show", because uci is storing the changes
in /tmp/.uci/

Is there a way to truly make a configuration read only in UCI, so that
changes are rejected, and not stored in /tmp/.uci/ ?

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Controlling host routes via ubus / netifd

2020-08-12 Thread Michael Jones
Hi List.

I am trying to control the routes on my OpenWRT system
programmatically, and would like to know if this is possible to do via
ubus and netifd.

I've located the ubus target "ubus call network add_host_route
'{"interface":"wan", "target":"1.0.0.0"}'" However, it does not seem
to actually do anything when called other than return "interface":
"wan".

I see that the ubus function for this is in netifd's ubus.c,
netifd_add_host_route(), which then calls
interface_ip_add_target_route() in interface-ip.c. This function
checks the indicated target for all zeros, and if all zeros are given
sets the route as a default route.

My main objective with this is that I want to be able to tell netifd
to make a particular interface (e.g. wan, wwan, so on) the default
route with a ubus command.

Is there an existing ubus endpoint that can do this?

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: jsonfilter and queries with '-' in them

2020-08-12 Thread Michael Jones
Thanks. That works.

Could you write a little bit about the underlying cause of this just
so I can understand better?

On Wed, Aug 12, 2020 at 3:59 PM Jo-Philipp Wich  wrote:
>
> Hi,
>
> as already implied by Dirk, you need to use bracket notation for labels that
> are not valid variable identifiers.
>
> So instead of `@.foo-bar` use `@["foo-bar"]`.
>
> ~ Jo
>
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


jsonfilter and queries with '-' in them

2020-08-12 Thread Michael Jones
Hi List,

I am trying to query netifd via ubus for the list of ipv4 ipaddresses
associated with an interface.

Currently my query looks like

ubus call network.interface.wan status | jsonfilter -e "@.ipv4-address"
Syntax error: Invalid escape sequence
In expression @.ipv4-address
Near here --^

However, underscores work just fine.

ubus call network.interface.wan status | jsonfilter -e "@.dns_metric"
0

Is there a specific trick to informing jsonfilter of identifiers with
dashes in them?

I can work around this by returning anything that has an "address"
key. This isn't optimal since it'll give me ipv6 addresses as well,
but I can filter those out with grep.

ubus call network.interface.wan status | jsonfilter -e "@.*[*].address"
10.1.0.1

Or further constrain it by only returning the "address" key of objects
that have a "mask" key
ubus call network.interface.wan status | jsonfilter -e "@.*[@.mask].address"
10.1.0.1

But I'd really like to understand why it's unhappy with the '-' character.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] /usr/share/libubox/jshn.sh and piping json text to json_load

2020-06-13 Thread Michael Jones
Hi List,

I'm looking to load some json text that's being piped through grep / sed /
awk into the json handling capabilities provided by
/usr/share/libubox/jshn.sh

Looking here:
https://git.openwrt.org/?p=project/libubox.git;a=blob;f=jshn.c;h=1b685e5fb0d853c4053389242b92c34967beb94f;hb=HEAD

I don't see any kind of handling of stdin. So I'm guessing that piping data
into the shell json handling code isn't an expected use case.

Does anyone have any suggestions beyond the obvious solution of loading the
data into a shell variable first?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v2] build: improve ccache support

2020-06-12 Thread Michael Jones
On Fri, Jun 12, 2020 at 1:44 PM Roman Yeryomin  wrote:

> Set CCACHE_DIR to $(TOPDIR)/.ccache and CCACHE_BASEDIR to $(TOPDIR).
> This allows to do clean and dirclean. Cache hit rate for test build
> after dirclean is ~65%.
> If CCACHE is enabled stats are printed out at the end of building process.
> CCACHE_DIR config variable allows to override default, which could be
> useful
> when sharing cache with many builds.
> cacheclean make target allows to clean the cache.
>
> Changes from v1:
> - remove ccache directory using CCACHE_DIR variable
> - remove ccache leftovers from sdk and toolchain make files
> - introduce CONFIG_CCACHE_DIR variable
> - introduce cacheclean make target
>
>

Wonderful, this solves my use case.

A minor nitpick, but you may want to also add a
CONFIG_CCACHE_COMPILER_CHECK as well.

I personally will not change it, as I use the value that you are already
setting, but it's something to consider.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] build: improve ccache support

2020-06-04 Thread Michael Jones
> I agree with Felix. Having one ccache directory for multiple repos
> doesn't make much sense to me as most probably they are for different
> platforms. And even if they are for same platform there are more chances
> for ccache corruption and deleting it will affect all those repos. So it
> still can be done with symlink but IMO should be done manually to make
> sure you know what you are doing.
> Also BASEDIR should be changed too, probably, if we want to go that way.
>
>
I am already using a shared ccache directory for multiple builds in my
continuous integration system. I'm accomplishing this by patching the build
system to change the directory for the ccache directory.

If it were a .config option, your use-case would be directly supported, as
well as my use-case, without as much custom patching.

I imagine that there are plenty of other organizations which would use the
.config based functionality to customize things to their liking.

If ccache is corrupted there are much larger problems than slowing down
other builds.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] netifd: per-interface MTU settings vs per-route MTU settings?

2020-06-04 Thread Michael Jones
On Mon, Jun 1, 2020 at 9:39 PM Reuben Dowle  wrote:

> When changing an interface MTU direct through ip command, netifd will get
> be out of sync. The netifd internally tracked MTU (discoverable through
> ubus call network.device status '{"name":"xxx"}' call will always show 1500
> (or whatever the value was when netifd discovered/created the interface).
>
>
>
> I am not aware of any actual issues caused by this, but it is worth
> considering.
>
>
>
> Finding some method to update through ubus is preferable in my opinion.
>
>
>
> For the project I have been working on, we added a ubus network.device
> set_attr method to netifd to solve this problem – see attached patch. Then
> you can use this in the protocol handler:
>
>
>
> [ -n "$mtu" ] && {
> echo "Setting MTU to $mtu"
> /sbin/ubus call network.device set_attr
> "{\"device\":\"$ifname\", \"mtu\":\"$mtu\"}"
> }
>

For what it's worth, I would very much like to see a patch like this merged
into netifd. Modifying the MTU after the fact makes me uncomfortable.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] base-files: add list-enabled/disabled to service function in /etc/profile

2020-06-03 Thread Michael Jones
On Wed, Jun 3, 2020 at 6:21 PM Stan Grishin  wrote:

> To obtain the list of enabled (for autostart) services, you'd type
> service list-enabled. For disabled services service list-disabled. It
> is useful when you need to quickly check which services are
> enabled/disabled or when helping other users troubleshoot.
>
> An alternative to list-enabled/list-disabled that I have considered
> was to output the enabled status of available services below the usage
> output, ie replace:
> if [ -n "$1" ]; then
> echo "service "'"'"$1"'"'" not found, the
> following services are available:"
> ls "/etc/init.d"
> fi
>
> with
>
> if [ -n "$1" ]; then
> echo "service "'"'"$1"'"'" not found, the
> following services are available:"
> for F in /etc/init.d/* ; do
> $F enabled && echo "$F (autostart enabled)" ||
> echo "$F (autostart **disabled**)"
> done;
> fi
>
>
> Please elaborate on the list-start and list-stop question, I'm not
> sure I understand the purpose of those.
>

Originally I asked that question because I misunderstood what the goal of
this change was.

I thought that you were proposing to add the ability to enable / disable
multiple services at the same time, so I was asking about the ability to
start / stop multiple services at the same time.

It's clear not that's not what you were trying to propose.

So instead, what about listing the services that are running, and also
listing the services that are configured, but not running?

I don't know that that provides a lot of value, so it may not be worth
doing.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] base-files: add list-enabled/disabled to service function in /etc/profile

2020-06-03 Thread Michael Jones
On Wed, Jun 3, 2020 at 6:42 AM Stan Grishin  wrote:

> Implement service list-enabled and service list-disabled to provide an
> easy way
> for users to list enabled/disabled services from CLI.
>
> Signed-off-by: Stan Grishin 
> ---
>  package/base-files/files/etc/profile | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/package/base-files/files/etc/profile
> b/package/base-files/files/etc/profile
> index 0beff1608f..e8350cfd6a 100644
> --- a/package/base-files/files/etc/profile
> +++ b/package/base-files/files/etc/profile
> @@ -38,3 +38,24 @@ in order to prevent unauthorized SSH logins.
>  --
>  EOF
>  fi
> +
> +service() {
> +if [ "$1" = "list-enabled" ]; then
> +for F in /etc/init.d/* ; do
> +$F enabled && echo "$F enabled"
> +done;
> +elif [ "$1" = "list-disabled" ]; then
> +for F in /etc/init.d/* ; do
> +$F enabled || echo "$F disabled"
> +done;
> +elif [ -f "/etc/init.d/$1" ]; then
> +/etc/init.d/$@
> +else
> +echo "Usage: service
> list-disabled|list-enabled| [command]"
> +if [ -n "$1" ]; then
> +echo "service "'"'"$1"'"'" not found, the
> following services are available:"
> +ls "/etc/init.d"
> +fi
> +return 1
> +fi
> +}
> --
> 2.25.1
>
>
Could you provide examples of how this would be used?

What about "list-start" and "list-stop" as well?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] uci API wrong

2020-06-03 Thread Michael Jones
On Wed, Jun 3, 2020 at 3:22 AM 魏艳艳  wrote:

> Dear!
> I've tried to write configurations using UCI API in my projects, but
> they often cause my processes to crash, I don't know why?I hope you can
> help me。
> 1、platform:X86
> This is the information that my Program crashed
> (gdb) where
> #0  0x7fef72046277 in raise () from /lib64/libc.so.6
> #1  0x7fef72047968 in abort () from /lib64/libc.so.6
> #2  0x7fef72088d37 in __libc_message () from /lib64/libc.so.6
> #3  0x7fef72091499 in _int_free () from /lib64/libc.so.6
> #4  0x7fef723df3ef in uci_free_element (e=0xa63d50) at
> /home/uci/list.c:72
> #5  0x7fef723df573 in uci_free_package (package=package@entry=0xa63380)
> at /home/uci/list.c:283
> #6  0x7fef723dfb0d in uci_cleanup (ctx=0xa64300) at
> /home/uci/libuci.c:126
> #7  0x7fef723dfb6b in uci_free_context (ctx=0xa64300) at
> /home/uci/libuci.c:80
> #8  0x00402876 in uci_set_value (path=0x40cf59
> "/tmp/DataCollection/MODMAN", config=0x40cf51 "rundata", section=0x40cf43
> "modmanruninfo",
> option=0x40cfbf "akMODMANVTxMuteOpenAMIP", value=0x61b780
>  "0") at ft_uci.c:446
> #9  0x00403dec in Write_Mcast_Data (def=1) at ft_daq.c:166
> #10 0x0040426e in main (argc=1, argv=0x7ffe84fa78c8) at
> ft_daq.c:264
>
> 2、This is the function that writes the configuration file,My program calls
> uci_set_value function 20 times for 1s.
> /*/
> void uci_set_value(char *path, char *config, char *section, char *option,
> char *value)
> {
> struct uci_context *ctx = NULL;
> struct uci_ptr ptr;
> int ret = UCI_OK;
> char str[128] = {0};
> char filepath[128] = {0};
>
> ctx = uci_alloc_context();
> if (!ctx)
> {
> ULOG_ERR("uci_alloc_context error,
> config=%s,section=%s,option=%s,value=%s\n", config, section, option, value);
> return;
> }
> if (NULL != path)
> {
> uci_set_confdir(ctx, path);
> }
>
> memset(, 0, sizeof(ptr));
> memset(str, 0 , sizeof(str));
> sprintf(str, "%s.%s.%s=%s", config, section, option, value);
>
> if (uci_lookup_ptr(ctx, , str, true) != 0)
> {
> ULOG_ERR("uci_lookup_ptr error, str=%s\n", str);
> uci_free_context(ctx);
> return;
> }
> memset(filepath, 0, sizeof(filepath));
> sprintf(filepath, "%s/%s", path, config);
>
> if ((0 != strcmp(ctx->confdir, path)) || (0 != strcmp(filepath,
> ptr.p->path)))
> {
> if (ptr.p)
> {
> uci_unload(ctx, ptr.p);
> }
> uci_free_context(ctx);
> return;
> }
>
> ret = uci_set(ctx, );
> if (0 == ret)
> {
> ret = uci_commit(ctx, , false);
> }
> else
> {
> ULOG_ERR("uci_set error, str=%s\n", str);
> }
>
> if (ptr.p)
> {
> uci_unload(ctx, ptr.p);
> }
> // Program terminated with signal 6, Aborted.
> uci_free_context(ctx);
>
> return ;
> }
> /*/
>
> 3、My config file
> config modman 'modmanruninfo'
> option akTestDID 'unknown'
> option akTestUptime 'unknown'
> option akTestSatelliteNetworkID 'unknown'
> akMODMANVTxMuteOpenAMIP 'unknown'
>



I don't know what's wrong with your code above, and I haven't tried to
analyze it for mistakes.

I'm replying to you because I wrote a C++ wrapper around UCI a while ago,
that involved documenting some of the conventions of the libuci API.

I'm providing it here as use-at-your-own-risk, and will not provide
updates, bug fixes, security fixes, or really anything, pertaining to this
code. There are almost certainly bugs in this code, because I do not use
it. I abandoned this approach to interacting with the UCI subsystem in
favor of the rpcd UCI module.

Released under the same license as libUCI itself (LGPL 2.1).

It may help you understand the libuci c API. It may not.

/** \file
 *
 */

#include 
#include 
#include 
#include 

#include "uci.h"

#ifndef LIBUCIXX_PARSE_UCI_CONFIG_FILE_H_A2CD8106_1003_4A6D_9A46_4A128EA83262
#define LIBUCIXX_PARSE_UCI_CONFIG_FILE_H_A2CD8106_1003_4A6D_9A46_4A128EA83262
#pragma once

namespace libucixx
{

namespace impl
{

/** \class ScopeGuard
 * The class \c ScopeGuard provides a mechanism to call \c operator()
on the object
 * provided as the constructor parameter for the \c ScopeGuard object
when \c ScopeGuard
 * has it's destructor called. This provides a guaranteed, exception
safe, way of ensuring
 * that cleanup operations are called at end of scope.
 */
template
struct ScopeGuard final
{
/**
 * Does nothing more than store the parameter in a member variable.
 */
ScopeGuard(T t)
 : m_t(std::move(t))
{ }

/**
 * Calls the stored object's \c operator() function.
 */
~ScopeGuard()
{
m_t();
}

private:
T m_t; ///< The stored object to invoke on destruction.
}; // struct 

Re: [OpenWrt-Devel] [PATCH] build: improve ccache support

2020-06-01 Thread Michael Jones
On Mon, Jun 1, 2020 at 1:22 PM Felix Fietkau  wrote:

> On 2020-06-01 19:11, Michael Jones wrote:
> >
> >
> > On Mon, Jun 1, 2020 at 10:33 AM Roman Yeryomin  > <mailto:ro...@advem.lv>> wrote:
> >
> > Set CCACHE_DIR to $(TOPDIR)/.ccache and CCACHE_BASEDIR to $(TOPDIR).
> > This allows to do clean and dirclean. Cache hit rate for test build
> > after dirclean is ~65%.
> > If CCACHE is enabled stats are printed out at the end of building
> > process.
> >
> > Signed-off-by: Roman Yeryomin mailto:ro...@advem.lv
> >>
> >
> >
> > This certainly looks like an improvement.
> >
> > However, there is an important usage case that this change doesn't
> address.
> >
> > Frequently when I am working on OpenWRT related things, I have many
> > different workspaces all tied to the same git repository hosted
> > externally. The reason for this is to allow multiple builds to live and
> > run independently.
> >
> > Having the CCACHE_DIR be located *inside* the repository doesn't share
> > the cache between multiple workspaces.
> >
> > So can the CCACHE_DIR be made configurable at build time based on the
> > .config file? Perhaps it can default to this, and only be set to the
> > value in .config if provided? For my purposes, I would always set the
> > CCACHE_DIR to a path that all of my workspaces use.
> I don't think there's a need for that config option. You could simply
> add a .ccache symlink in your source dir and point it to your shared
> cache. I do the same with dl on my trees.
>
> - Felix
>

I disagree.

Having build behavior change based on a symlink is undesirable.

If it were a config option, it becomes a documented feature that is easily
discoverable in the menu config.

Additionally, having the ccache directory be a configuration option allows
it to persist across clones of the git repository, if the .config file is
stored in git. A symlink would need to be manually re-configured on each
clone.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] build: improve ccache support

2020-06-01 Thread Michael Jones
On Mon, Jun 1, 2020 at 10:33 AM Roman Yeryomin  wrote:

> Set CCACHE_DIR to $(TOPDIR)/.ccache and CCACHE_BASEDIR to $(TOPDIR).
> This allows to do clean and dirclean. Cache hit rate for test build
> after dirclean is ~65%.
> If CCACHE is enabled stats are printed out at the end of building process.
>
> Signed-off-by: Roman Yeryomin 
>

This certainly looks like an improvement.

However, there is an important usage case that this change doesn't address.

Frequently when I am working on OpenWRT related things, I have many
different workspaces all tied to the same git repository hosted externally.
The reason for this is to allow multiple builds to live and run
independently.

Having the CCACHE_DIR be located *inside* the repository doesn't share the
cache between multiple workspaces.

So can the CCACHE_DIR be made configurable at build time based on the
.config file? Perhaps it can default to this, and only be set to the value
in .config if provided? For my purposes, I would always set the CCACHE_DIR
to a path that all of my workspaces use.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] netifd: per-interface MTU settings vs per-route MTU settings?

2020-05-25 Thread Michael Jones
On Mon, May 25, 2020 at 8:00 AM Aleksander Morgado 
wrote:

> Other protocol handlers, like uqmi, are also able to setup the MTU,
> and instead of doing it via proto_send_update, they just do it like
> this:
>
> [ -n "$mtu" ] && {
> echo "Setting MTU to $mtu"
> /sbin/ip link set dev $ifname mtu $mtu
> }
>
> I guess we can do the same in the ModemManager protocol handler.
>
>
That does seem like it would work, but I'm wary of race conditions with the
message sent to netifd via ubus, and further feel like it would be better
to have netifd learn how to set the mtu directly, if it doesn't already
know how.

Are any of the netifd project's contributors able to comment on this?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] netifd: per-interface MTU settings vs per-route MTU settings?

2020-05-23 Thread Michael Jones
On Thu, May 21, 2020, 07:44 Aleksander Morgado 
wrote:

> Hey!
>
> There's an ongoing discussion in the ModemManager package in github
> related to how the MTU may be configured in the network interface
> based on what the MM bearer object reports:
> https://github.com/openwrt/packages/issues/11383
>
> E.g.:
>
> $ ip route show
> default via 25.225.105.222 dev wwan0 proto static src 25.225.105.221 mtu
> 1430
>
> Looks like the protocol handler implementation for netifd may allow
> setting the MTU to the routes associated with the network interface,
> instead of assigning the MTU value to the interface itself. Are both
> methods completely equivalent? Is there any reason to avoid setting
> the MTU in the interface when using netifd, or just not implemented
> yet?
>
> Cheers!
>
> --
> Aleksander
> https://aleksander.es




I'm actually not aware of any method to set the interface MTU via netifd
that isn't a UCI setting.

There's very little documentation about the supported ubus methods and
parameter formats.

Does anyone have information on this?

>
>
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Experimental "release goals" for 19.07.4

2020-05-23 Thread Michael Jones
On Sat, May 23, 2020, 13:01 Baptiste Jonglez 
wrote:

> If the initial concept looks good, we can think about automating some of
> it:
> tag bug reports on Flyspray/Github/Gitlab as blocking for a specific
> release, use "milestones", automatically find commits that satisfy a goal,
> etc.
>


I find it very confusing that there are so many different places for bugs
to live and be tracked.

What's going on with gitlab? Is that going to be the source of truth for
OpenWRT? Or will we always have 3 different bug trackers?

>
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Ubus based service watchdog?

2020-05-15 Thread Michael Jones
On Thu, May 14, 2020 at 5:56 PM Wes Turner  wrote:

> FWIW, k8s has Liveness, Readiness and Startup Probes
>
> https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/
> ::
>
> > The kubelet uses startup probes to know when a container application has
> started. If such a probe is configured, it disables liveness and readiness
> checks until it succeeds, making sure those probes don’t interfere with the
> application startup. This can be used to adopt liveness checks on slow
> starting containers, avoiding them getting killed by the kubelet before
> they are up and running.
>
>
Good suggestion.

It's starting to look like people like the idea of having procd poke the
service and require a reply, instead of the other way around. That's fine
with me.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Ubus based service watchdog?

2020-05-15 Thread Michael Jones
On Fri, May 15, 2020 at 9:57 AM Henrique de Moraes Holschuh 
wrote:

> On 15/05/2020 03:58, Petr Štetiar wrote:
> > In other words I think, that one can solve this use case with current
> > solutions, no need to bloat procd.
>
> And if you're going to bloat procd, it pays to look at what the
> equivalent systemd functionality provides (it has a per-service
> application-aware watchdog and a
> please-wait-a-bit-more-I-am-doing-a-clean-shutdown API), to better know
> the problem space.
>
>
I have a mild objection to the quick jump to the word "bloat", which I've
observed used on this list frequently

That being said, yes, I'm a heavy user of the systemd watchdog
functionality, and is part of the reason why I want to add this to procd.

Thank you for making sure this aspect of service management was brought to
the list's attention though.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Ubus based service watchdog?

2020-05-15 Thread Michael Jones
On Fri, May 15, 2020 at 4:35 AM Petr Štetiar  wrote:

> Michael Jones  [2020-05-15 02:39:52]:
>
> > What's wrong with monit is that it's documentation is gigantic
>
> Good documentation with a lot of examples is hardly a problem, its a bonus
> point for me.
>
>
I think you misunderstood.

Monit has a massive feature surface. I'm not even going to consider it, it
goes far far beyond the level of capability that's appropriate for my use
case.


> > for a relatively trivial need.
>
> Your need, your current trivial use case. Overall project
> goals/design/universe should be taken into account.
>

Sure, accept the patch or don't. I'm happy to take guidance from the
OpenWRT community, but ultimately will implement the things I need the way
I need them. Designing patches so that they are palatable to OpenWRT is a
secondary concern. Once I have it working to the level that I'm happy with,
the patch will be provided under the appropriate license, and then it's not
my problem beyond helping serious efforts at integrating it.

> If ubus is failing, there's a much larger problem than my service failing.
>
> And you don't want to recover from this even more critical situation?
>

If UBus fails, there's not a realistic recovery option beyond restarting
the system, I'd rather that be PID 1's responsibility as well.

> This requires that my program be able to communicate with ubus natively
> and
> > offer a ubus endpoint that can be queried.
>
> Then use file, socket or whatever suits your use case.
>

Of course, I will do what suits my use case. In this situation, the use
case is that I want service monitoring built into the daemon that manages
services.

I was responding to your suggestion of advertising a status string on ubus,
and explaining why that's not viable.

> More complicated than a simple timer in procd.
>
> Which is not flexible enough, tailored exactly to your very simple use
> case.
>

Perfect is the enemy of the good. My proposal is very simple, and very
flexible. It's not literally as flexible as it possibly can be while
simultaneously having no negatives, but no solution is.

> It's hardly bloat.
>
> Just take a look one step further.  Once OpenWrt accepts this feature its
> official.  What is going to happen afterwards in the OpenWrt universe?
>

> Folks will of course start using this feature, adding support for this
> feature
> into their critical services (take your answer to ISC dhcp support as
> example), which would in most cases mean local patch(es) as this feature
> could
> be hardly upstreamed.
>

Lots of projects accept patches for specific init system features. If
systemd specific patches can be upstreamed, so can procd patches.

Also consider that OpenWRT already has patches for lots of programs to
communicate over ubus. Are you insinuating that those patches are
undesirable?

This is the cost that comes with maintaining your own set of system
services that no other project family uses. You don't see procd used in
desktop distributions, or cellphones, or "infotainment" systems in cars,
etc. (That I'm aware of, I'm happy to be corrected). Custom tools require
customization to things.

Procd does not have the feature set that I require for my purposes. I am
happy with other parts of the OpenWRT ecosystem, so instead of switching to
a different distribution, I'm open to making contributions so everyone can
benefit.

So in the end, we're going to have not so trivial amount of local patches
> for
> ubus service watchdog support, which would break DRY principle,


Hardly. Each package is different, so the decision making logic to
communicate with a service watchdog will be different. At worst, you have
10-20 lines of code duplicated per package, and that'll be whatever
quantity of boilerplate communicating with UBus requires.


> and probably result in additional maintenance/QA work with every package
> version bump.
>

Trade off between additional reliability and code complexity. It's a tale
as old as software.


> In order to avoid this bloat, unnecessary patch hell, one would perhaps
> need
> to implement kind of monit/cron solution in procd/umonitd, so it would be
> possible to have custom/generic service liveliness checks defined in the
> service init scripts or UCI configuration.
>
> Maybe all is needed is some kind of monitrc generator from UCI configs/init
> scripts?
>

Your counter proposal to a (estimated) 200-300 lines of code patch in
procd, which could even be a compile-time-option, is to have services
depend on a package that has more features than I know what to do with?

How is monit going to work with procd's jails? The whole point of the jails
is to isolate services into their own mini-container type thing. So now we
have to poke a hole in the jail to allow monit to monitor the service?

And monit

Re: [OpenWrt-Devel] Ubus based service watchdog?

2020-05-15 Thread Michael Jones
On Fri, May 15, 2020 at 1:58 AM Petr Štetiar  wrote:

> Michael Jones  [2020-05-13 12:48:49]:
>
> Hi,
>
> > I have a critical service on my OpenWRT system that needs monitoring and
> > re-starting if it's failed.
>
> whats wrong with monit[1]? It was designed exactly for this purpose and is
> much more flexible.
>
>
What's wrong with monit is that it's documentation is gigantic for a
relatively trivial need. This disqualifies it as being designed exactly for
the purpose.


> > I've been looking for a mechanism in procd that would allow me to request
> > that my service be terminated if it did not periodically notify some
> > watchdog endpoint via ubus.
>
> So instead of proper error handling and crashing your service ASAP, you're
> now
> going to add another ubus layer which might possibly fail as well.


If ubus is failing, there's a much larger problem than my service failing.


> You know, your service could happily ping the watchdog endpoint, yet still
> fail in other
> parts. You want something more robust.
>

Ubus would only be pinged when the service does the thing it's designed to
do. In this case, there'll be some communication with the internet that
involves bi-directional communication. No risk of false positives.


>
> I would simply add ubus status method to that critical service,


This requires that my program be able to communicate with ubus natively and
offer a ubus endpoint that can be queried.

UBus is fundamentally incompatible with programs that have their own event
loop. Or was, last I investigated. I have not had time to dig into ubox to
make the necessary improvements to allow external loop drivers.

Having the program being managed call "ubus call
service.$servicename.watchdog " in whatever way it wants to is more
flexible. All programs can launch sub processes, even if they have to
resort to fork+exec.



> then check the
> output in the cron shell/Lua script and kill the service if the output of
> the
> ubus status method wouldnt match liveliness for that service.
>

More complicated than a simple timer in procd.


> In other words I think, that one can solve this use case with current
> solutions, no need to bloat procd.
>
>
It's hardly bloat. It's a very simple feature that serves a core need in
service management as a generic concern.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Ubus based service watchdog?

2020-05-14 Thread Michael Jones
On Thu, May 14, 2020, 23:43 Philip Prindeville <
philipp_s...@redfish-solutions.com> wrote:

>
> > On May 13, 2020, at 11:48 AM, Michael Jones 
> wrote:
> >
> > I have a critical service on my OpenWRT system that needs monitoring and
> re-starting if it's failed.
> >
> > I've been looking for a mechanism in procd that would allow me to
> request that my service be terminated if it did not periodically notify
> some watchdog endpoint via ubus.
> >
> > It seems to me like this is not something that is currently supported by
> procd, and I've written my own clumsy "watchdog" wrapper program that tries
> to do the job.
> >
> > Are there any plans to support a feature like this in procd directly in
> the future?
> >
> > If there are no plans, and I were to write code for this, would OpenWRT
> be interested in reviewing, and then merging, patches?
> >
> > Are there other people on this list who would take advantage of this
> feature if it were available? If so, what functionality would you like to
> see?
> >
>
>
> So existing services that are launched by procd (like ISC dhcpd) would
> need to be patched to support this?
>
> -Philip
>

Yes. That would be the case.

It would need to be an opt-in feature. Its not possible for a service
watchdog to be opt out because service watchdogs need to be tailored to the
service being watched.

>
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Sysupgrade and Failed to kill all processes

2020-05-13 Thread Michael Jones
On Wed, May 13, 2020 at 6:29 PM Philip Prindeville <
philipp_s...@redfish-solutions.com> wrote:

>
> My lights-out machine rooms all have a dialup line, a terminal server, and
> a power strip where I can cycle outlets… because I don’t like driving 7
> hours each way for 1 hour of productive work.
>
>
My OpenWRT boxes are deployed in deep rural areas on other continents

Sadly, I haven't been able to convince customers to upgrade to remotely
controllable power strips.  :-)
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Sysupgrade and Failed to kill all processes

2020-05-13 Thread Michael Jones
>
> How the entire upgrade process works would be a good subject for
> documenting on the Wiki if it’s not already.
>

Feel free :-)


> How long are you thinking this I/O will take to complete?
>

 Longer than the blazing speed of /bin/sh looping over /proc/*

(1) It shouldn’t be happening very often.  Hopefully.
>

It happens all the time for me. I have customers complaining my ear off
about their router not upgrading properly.


> (2) If the box is in an indeterminate state then it’s not always clear
> that there’s a safe path forward, and sometimes this is something that a
> human needs to ascertain.
>

There's no human that can ascertain anything. The board that is being
upgraded is being upgraded from inside an enclosed shell where no human can
access it aside from the ethernet port, or wifi card. Given that the
upgrade process shuts off all the possible methods of interacting with the
system, having the board hang forever complaining "Can't kill all
processes" is not useful. Now a human has to go physically interact with
it, to cut power and restore power.


> (3) You might also want to collect data about the failure so you can fix
> it and stop it from happening again.  Proceeding would efface all of that.
>

There's no data to be gathered. There is no shell. Even if you had UART /
Serial / VGA + Keyboard, the upgrade process has already terminated any CLI
interface. So there's no data that can be gathered.

What if the failure left the box in a partially compromised state?  Would
> you want your firewall to “fail open”?  I wouldn’t.
>

I want my router to not render itself useless every 10th time I update it.

As for firewalls failing open, that depends heavily on context. Do I want a
device that's in the middle of no where, where sending a human to access it
costs a lot of money, to fail in a state where there's no possibility of
communication? Or do I want it to fail in a way that I can still log into
it?

Further, we're still talking about failing to sigkill processes prior to an
upgrade. Nothing has changed on disk at this point in the upgrade, so a
failure to kill all processes should not render the box into powered up
brick.

The man page for signal(2) says:
>
>The signals SIGKILL and SIGSTOP cannot be caught or ignored.
>
> but yeah, if you’re in the kernel when the signal arrives, and you get
> stuck in there, then your process won’t go away and it becomes a moot point.
>

Right, that's what I suspect is happening.


> > I was under the impression that the only reliable way to ensure all
> processes terminate is to use cgroups, and put the processes to terminate
> in the freezer group and then kill them off after they've been frozen.
> Otherwise you have basically a race condition between the termination of
> processes and the creation of children. E.g. a fork-bomb could prevent all
> processes from being terminated.
>
>
> That assumes you have a kernel with CGROUPS compiled in.
>
RIght, so if you don't have CGROUPS compiled in, there's no reliable way to
terminate all processes. It can't be done.

For my purposes, I would have Cgroups compiled in. I have no concern for
increase in firmware size. Others may care, about firmware size, but they
can decide on the tradeoff between having cgroups or not having them.


> Also, if you have fork-bombs, why haven’t they brought down the system
> earlier?  And why would you have untrusted services/programs on your system
> in the first place?  This isn’t a general computing base with naive users
> picking up malware inadvertently, etc.  It’s a closed software ecosystem
> (in theory… how it gets mangled downstream is a different question).
>

It was merely an example.

I’m speculating but it could be for any number of reasons…  Keeping procd
> simple…  There might be ordering or dependency that requires doing the
> shutdown in a particular order… There might be services (like squid if
> socks or proxy web access is required) that might be needed by the upgrade
> process in some scenarios…
>

The upgrade process does not interact with any network services. All
necessary files are completely verified as on disk and correct prior to the
stage of the upgrade process that I'm talking about.

If there is an ordering dependency, then the services in question must
describe that dependency in their /etc/init.d/ files.

Procd is the manager of services. There's no meaningful complexity or
binary size cost to having it do a "for(auto const& service : services) {
service.shutdown(); }" loop (forgive the C++-ism, It's simply an example),
and the cost is well worth the additional reliability that it would provide.


> When *not* using cgroups?  I thought you just argued for using cgroups to
> avoid the fork-race condition above…
>
Yes, cgroups are the ideal.

But even if cgroups are not available, procd should still attempt to use
the service management system to shut things down *prior* to looping with
sigterm and sigkill.
If cgroups are available, then 

Re: [OpenWrt-Devel] Ubus based service watchdog?

2020-05-13 Thread Michael Jones
On Wed, May 13, 2020 at 1:53 PM Eric Romano  wrote:

> It does seem like process supervision would be a nice capability to
> have within procd.
>
> Previously I've done this by a combination of:
> 1. trying to crash / exit the process when an inconsistent state is hit.
> 2. monitoring for delayed writes to a timestamp pidfile in temp.
>
> procd's respawn directive can handle the first case (i forget but
> something like "respawn 10 5 0" did fine), but the second one...
> depended on a clumsy nanny process to reap / kill any that failed to
> update their timestamp.
>
> Feature wise, it would be nice to have more than one way to perform
> the process health check.
>
> Some options that come to mind are:
> - a ping / pong -like method via ubus
> - expecting the process to "touch" a specified file, which procd could
> read stat times for
> - either of the above, with the ability to trigger a refresh via
> signal like SIGUSR1 or SIGUSR2, etc.
>
> Would be good to hear what other methods have been tried by others,
> and what a generic interface to this could look like in procd's
> config.
>
>
>
The way I'm currently doing it is to use abstract unix domain sockets. The
wrapper program opens the abstract domain socket for read, and the
subordinate program open for write.

If the wrapper program does not get at least one byte to read from whatever
is writing to the socket, every $timeout then it kills the child.

The idea that I had in mind originally was for the service in question to
need to send a message to some endpoint in ubus. This allows:
1) External programs can do the watchdog poking for the service in
question. E.g. your program need not understand ubus, it only needs to know
how to launch the ubus CLI program to send the message.
2) This allows for the watchdog behavior to grow additional features over
time
3) This allows for the watchdog management to be seperate from procd,
conceptually.

But I'd be happy to see other approaches as well.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Sysupgrade and Failed to kill all processes

2020-05-13 Thread Michael Jones
On Wed, May 13, 2020 at 12:29 PM Michael Jones 
wrote:

>
>
> On Wed, May 13, 2020 at 3:58 AM Jo-Philipp Wich  wrote:
>
>> Hi,
>>
>>
> Stuff like umounting external disks, fsync / swapoff etc. come to mind as
>> well
>> which should be doable at this point.
>>
>>
>>
> Right, that's also feasible.
>
> In fact I don't see any code at all for unmounting existing filesystem
> mounts. Thanks for pointing that out.
>


I think that it's also reasonable to have /sbin/upgraded, in its role as
PID1, have some kind of safety valve on when the upgrade has failed.

E.g. if /lib/upgrade/script2 has returned, at all, the system needs to
reboot, because at this point /sbin/upgraded should be the only process
running.

if /lib/upgrade/script2 has not returned after 1 hour, there's no chance
that the upgrade will succeed, so reboot.

In both situations, the board may be in a bad state. But there's nothing
that can be done.

/sbin/upgraded offers the user no CLI interactions at all, so there's no
recovery actions that could be taken even if there was a UART / Serial /
VGA + Keyboard connection to the board to allow user interaction.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] Ubus based service watchdog?

2020-05-13 Thread Michael Jones
I have a critical service on my OpenWRT system that needs monitoring and
re-starting if it's failed.

I've been looking for a mechanism in procd that would allow me to request
that my service be terminated if it did not periodically notify some
watchdog endpoint via ubus.

It seems to me like this is not something that is currently supported by
procd, and I've written my own clumsy "watchdog" wrapper program that tries
to do the job.

Are there any plans to support a feature like this in procd directly in the
future?

If there are no plans, and I were to write code for this, would OpenWRT be
interested in reviewing, and then merging, patches?

Are there other people on this list who would take advantage of this
feature if it were available? If so, what functionality would you like to
see?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Sysupgrade and Failed to kill all processes

2020-05-13 Thread Michael Jones
On Wed, May 13, 2020 at 3:58 AM Jo-Philipp Wich  wrote:

> Hi,
>
> >
> > That loop-kill-all thing should be a kind of last resort really,
> what's
> > actually needed is some kind of "init 1" procd equivalent which
> shuts down all
> > services in a more or less clean manner.
> >
> >
> > Oddly enough, the /lib/upgrade/stage2 script has some aspect of this. It
> > explicitly shuts down (kill -9) telnet, dropbear, and ash before looping
> with
> > sigTERM, and then again with sigKILL.
> >
> > I find it very odd that it's explicitly singling out telnet, dropbear,
> and
> > ash. My OpenWRT build doesn't have any of these installed in the first
> place.
> > E.g. I have OpenSSH, and it's jumping straight to kill -9 instead of
> sending
> > sigTERM first like it should.
>
> These are (in the case of telnet, were) the default services offering shell
> access in standard images the sysupgrade script was tailored for.
>
> The intention is to kill all user shell sessions to prevent interference
> with
> the subsequent upgrade process. An openssh case simply hasn't been added
> since
> it is uncommon, especially on lower end devices.
>
> The subsequent TERM / KILL loops are a poor mans attempt to cleanly shut
> down
> services. It obviously won't work for things having expensive teardown
> procedures (databases, squid proxy, etc.) - those really should be handled
> manually by the user before invoking sysupgrade. I mean obviously one can
> extend the grace period, but I guess there will always be unhandled cases.
>
>
I merely meant that i thought it odd that instead of using sigTERM on the
user-interactable processes, we jump straight to sigKILL.

I don't really see why singling out the user interactable processes does
any good, if they'd be sigTERM and then sigKILL-ed like everything else.




> Uhm, yeah sure, we could try writing the image again I guess. But
> eventually
> you have to give up if the storage device simply cannot be written cleanly.
>
>
Of course. Eventually we know it won't succeed, but a flaky storage doesn't
necessarily mean a second attempt won't succeed. Or an attempt to write the
data in smaller pieces.

My concern is that one error and giving up will lead to more soft-bricks
than two errors and giving up.

We could bikeshed on this forever though. I merely meant that one retry
isn't unreasonable. 50 probably is.



> Stuff like umounting external disks, fsync / swapoff etc. come to mind as
> well
> which should be doable at this point.
>
>
>
Right, that's also feasible.

In fact I don't see any code at all for unmounting existing filesystem
mounts. Thanks for pointing that out.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Sysupgrade and Failed to kill all processes

2020-05-13 Thread Michael Jones
On Wed, May 13, 2020 at 4:42 AM Kevin 'ldir' Darbyshire-Bryant <
l...@darbyshire-bryant.me.uk> wrote:

>
>
> > On 13 May 2020, at 09:57, Jo-Philipp Wich  wrote:
> >
> > Hi,
> >
> >>
> >>That loop-kill-all thing should be a kind of last resort really,
> what's
> >>actually needed is some kind of "init 1" procd equivalent which
> shuts down all
> >>services in a more or less clean manner.
> >>
>
> Beware the watchdog Luke.
>
>
Could you elaborate on this?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Sysupgrade and Failed to kill all processes

2020-05-13 Thread Michael Jones
> That loop-kill-all thing should be a kind of last resort really, what's
> actually needed is some kind of "init 1" procd equivalent which shuts down
> all
> services in a more or less clean manner.
>
>
Oddly enough, the /lib/upgrade/stage2 script has some aspect of this. It
explicitly shuts down (kill -9) telnet, dropbear, and ash before looping
with sigTERM, and then again with sigKILL.

I find it very odd that it's explicitly singling out telnet, dropbear, and
ash. My OpenWRT build doesn't have any of these installed in the first
place. E.g. I have OpenSSH, and it's jumping straight to kill -9 instead of
sending sigTERM first like it should.

I imagine this is the reason why I've had my SSH sessions hang
indefinitely when sysupgrading a board with dropbear.

I'm just not sure offhand how much possible error conditions there are
> besides
> the actual image writing itself, which you cannot recover from if it dies
> midway.
>

I would expect that if the image writing fails, at least one more attempt
should be made before giving up. Rendering the device soft-bricked is very
much not desirable...


No it is not. When the logic was implemented there wasn't any cgroup support
> in OpenWrt. Sysupgrade was introduced in 2007 when we still supported Linux
> 2.4 on some targets. Using the freezer cgroup probably makes sense
> nowadays,
> it will however further bloat the kernel which might hurt various lower end
> targets, flash space wise.
>
> Ok, noted.

I suppose I should point out that I'm not personally interested in the
lower end devices, but I understand where you're coming from there.

Perhaps a way to address this in a reliable way:

1) If cgroups support is detected at runtime (or conditional compilation to
save even more space in the binary), procd, acting as it's role of PID 1
places all services that it creates into their own cgroup. I don't know how
this interacts with procd jails, but perhaps some code from that can be
adapted and reused.
1.a) I would even add that there should be a top-level cgroup that should
contain all service-cgroups as nested cgroups, so that *everything* can be
terminated in one fell swoop.

2) on sysupgrade, just prior to execvp /sbin/upgraded, procd gracefully
shuts down all services that are running.
2.a) If cgroups are available, then after shutting down all services, use
the cgroup freezer to terminate any services cgroups that still have active
processes.
2.b) Use the global cgroup to nuke everything from orbit.

3) /sbin/upgraded handles terminating any remaining processes. This isn't
something that should be practically handled in a shell script. Moving the
logic for this into /sbin/upgraded means that the only safety check is that
it not try to terminate pid1.

4) Now /lib/upgrade/stage2 doesn't need to worry about terminating
processes, and can focus entirely on handling the ramdisk chroot logic.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] Sysupgrade and Failed to kill all processes

2020-05-12 Thread Michael Jones
I've been investigating a problem with sysupgrade failing with the error
message "Failed to kill all processes", and then hanging indefinitely.

This happens maybe once every 10-20 sysupgrades, and it's kind of a pain.

So far I've determined this workflow that the sysupgrade command follows.
Note, I'm not aiming for 100% accuracy, but just broad strokes.


1) /sbin/sysupgrade locates the file to upgrade from on the filesystem, or
if the second option to sysupgrade starts with http://, it downloads the
firmware file using wget.
2) /sbn/sysupgrade does some minor validation of various things, and grabs
whatever config files it thinks the end user wants to be restored and packs
them up into some kind of tarball.
3) sysupgrade sends a message, via ubus, to procd, to initiate the upgrade.
4) Procd does some stuff which I haven't finished completely understanding
just yet, but it looks like firmware verification to make sure we don't
upgrade to a bad firmware file.
5) It *does not* appear that procd will proactively terminate services
until everything (or almost everything) is shut down. Seems like something
that should be added to increase reliability.
6) procd replaces itself (execvp systemcall) with the program
/sbin/upgraded. This means that procd is *no longer running*, PID 1 is now
/sbin/upgraded. So service management is not possible at this point.
7) /sbin/upgraded now acts as PID1. It executes the shell script
/lib/upgrade/stage2 with parameters.
8) The shell script loops on all processes, and sends them the TERM signal,
and then the KILL signal. See email subjec for problems with this.
9) the shell script creates a new ram filesystem, mounts it, then copies
over a very small set of binaries into it.
10) The shell script changes root into the new ram filesystem
11) Inside the ramfilesystem, the shell script writes the upgraded firmware
and saved configuration to disk
12) Reboot.


Now that the very rough summary is out of the way, I have 4 questions.

1) I notice that the shell script /lib/upgrade/stage2 is doing a tight loop
with kill -9 to terminate processes. However, it's only looping a maximum
of 10 times, and its going as fast as the shell can loop.

What's to stop this loop from quickly going through every process almost
immediately 10 times, before a process that would be about to terminate
terminates? The process in question may be handling some kind of IO, so the
kernel wouldn't immediately terminate it.

Shouldn't there be some very brief sleep at the end of each loop iteration
to ensure that the processes that are going to practically terminate have
done so?

2) Why is the behavior on failure to terminate processes to just give up?
That leaves devices hanging without any network connectivity.
A reboot with some logging on disk would allow for remote sysupgrades to
have some kind of recoverability.

3) Is looping over sigkill a reliable way to terminate all processes?
I was under the impression that the only reliable way to ensure all
processes terminate is to use cgroups, and put the processes to terminate
in the freezer group and then kill them off after they've been frozen.
Otherwise you have basically a race condition between the termination of
processes and the creation of children. E.g. a fork-bomb could prevent all
processes from being terminated.

4) Why doesn't procd, prior to execvp the /sbin/upgraded program, shutdown
all the services that are running?

Maybe I'm just not seeing where it does this, so if that's the case, then
I'm happy to be corrected.

But I'm under the impression that when not using cgroups, stopping all
services would allow for anything that isn't double forked to be gracefully
shutdown and cleaned up after itself.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Advice needed - Proper approach to port 5G/LTE modem into OpenWRT

2020-04-30 Thread Michael Jones
On Mon, Apr 27, 2020 at 9:45 PM Jeonghum Joh 
wrote:

> Hello Michael,
>
> Thank you for sharing the helpful url. I guess mwan3[1] would deal such
> daemons - netifd and hotplug.d. Isn't it? If mwan3 deals with those, I
> guess I only need to utilize mwan3. Am I right?
>
> Thank you for answering my questions.
> Jeonghum
>
> [1] https://openwrt.org/docs/guide-user/network/wan/multiwan/mwan3
>

I've never heard of mwan3 before.

That being said, looking at the documentation page you provided a link to,
it does not appear that it will help you with getting your cellphone modem
to cooperate with the openwrt network stack.

The mwan3 program appears to only care about routing decisions that are
made on top of existing interfaces that are already working, and would not
be useful for getting those interfaces to work in the first place.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [ModemManager] How can I make ModemManager work right?

2020-04-29 Thread Michael Jones
On Wed, Apr 29, 2020 at 1:30 AM Jeonghum Joh 
wrote:

> Hello,
> Thanks to many people in this list, I was able to find ModemManager and
> I've tried applying it.
>
>  Testing environment 
>
> Target Model: MediaTek MT7622 AC4300rfb1 board
> Firmware Version : LEDE Reboot 17.01-SNAPSHOT unknown / LuCI
> Kernel Version  : 4.4.124
> Modem : HUCOM HM-900
> ModemManager
> Version : 1.12.8
> menuconfig  : (QMI on, MBIM off)
> git clone URL:
> https://github.com/openwrt/packages/tree/master/net/modemmanager
>
> Noted Device Files : Those files and paths are noted/watched
> qmichannel: /dev/cdc-wdm0
> usbnet_adapter  : /sys/class/net/wwan0
> /sys/bus/usb/devices/2-1/manufacturer : QCOM
> /sys/bus/usb/devices/2-1/idVendor:05C6
> /sys/bus/usb/devices/2-1/idProduct   :90db
> /sys/bus/usb/devices/2-1/speed :5000
> /sys/bus/usb/devices/2-1/product  :SDXPRAIRIE-MTP _SN:B02CE51B
> /sys/bus/usb/devices/2-1/version   :3.20
> /sys/bus/usb/devices/2-1:1.2/net/wwan0
> /sys/bus/usb/devices/2-1:1.2/net/wwan0/device/driver
> /sys/bus/usb/devices/2-1:1.2/net/wwan0/device
> /sys/bus/usb/devices/2-1:1.2/usbmisc/cdc-wdm0
> /sys/devices/platform/1a0c.usb/usb2/2-1/2-1:1.2
>
> Kernel modules watched via lsmod
> cdc_wdm 8821  1 qmi_wwan
> qmi_wwan6252  0
> usbcore   153512 20
> option,usb_wwan,qmi_wwan,cdc_ncm,cdc_ether,usbserial,usbnet,usblp,cdc_wdm,cdc_acm,usb_storage,xhci_mtk,xhci_plat_hcd,xhci_pci,xhci_hcd,uhci_hcd,ohci_platform,ohci_hcd,ehci_platform,ehci_hcd
> usbnet 19027  3 qmi_wwan,cdc_ncm,cdc_ether
>
> Above is original environment.
> On this circumstances, I disabled CM provided from modem provider :
> hucom-cm
> And I added one configuration section into the bottom of
> /etc/config/network :
> config interface 'broadband'
> option device '/sys/devices/platform/1a0c.usb/usb2/2-1/2-1:1.2'
> option proto 'modemmanager'
> option apn '5g-internet.sktelecom.com'
> option username ''
> option password ''
> option pincode ''
> option lowpower '1'
>
> Under these settings, I confirmed that
> - hucom-cm is disabled
> - ModemManager is alive
>  4430 root  229m D/usr/sbin/ModemManager
>
> The wwan0 is not shown from ifconfig and ping 8.8.8.8 fails saying Network
> is unreachable.
> So, My first trying to utilize MM seemed to be failed.
>
> Originally hucom-cm sets bridge mode like shown below:
> BRIDGE_MODE_FILE : /sys/module/qmi_wwan/parameters/bridge_mode
> BRIDGE_IPV4_FILE: /sys/module/qmi_wwan/parameters/bridge_ipv4
> But under ModemManager these files are not shown.
>
> And I've found many plugin libraries under /usr/lib/ModemManager:
> libmm-plugin-altair-lte.so libmm-plugin-mtk.so
>  libmm-plugin-thuraya.so
>
> Could someone help me?
>
> Do I need to look deeper into ModemManager?
> Do I need to implement new plugin library for our product?
> Or do I need to forget about ModemManager?
>
> My already given hucom-cm anyway works. It brings data via wwan0 and
> establish network interface wwan0.
> Problem is that hucom-cm sets up firewall rules and routing settings
> redundantly and unnecessarily. So I can make it not to do these unneeded
> settings. This way would be good approach I believe.
> But we'd like to make our product indepecntant from specific modem. I
> guessed that opensource ModemManager infrastructure would give us some sort
> of independence from specific modem and in other words it would give some
> abstraction..
>
> I am not sure of anything. Could someone give me some light so that I can
> find the best way to go?
>
> Additional question : If I write our own logic as a plugin library of
> ModemManager, wouldn't it necessarily have to stick to GPL? Can we apply
> commercial license for the plugin?
>
> Thank you very much in advance.
> Jeonghum
>
>
>
Is it practical for you to upgrade to a newer version of OpenWRT? 17.01 is
pretty old

But  since I've been dealing with modem manager related problems in my own
work, I can try to give you some advice.

First, I recommend *not* trying to start with the OpenWRT Netifd
integration. It's a massive pain to work with. Lots of undocumented parts
of the protocol that netifd scripts use to communicate with the netifd c
language binary that I gave up on trying to understand, and just built
work-arounds for.

Instead, before using netifd, try talking directly to modemmanager using
"mmcli".

mmcli can be used to determine if modemmanager understands how to talk to
your modem, and if it does it can be used to issue the connect command
(e.g. mmcli --simple-connect) and then display the connection parameters
that the modem reports are to be used for internet connectivity.

Once you've verified that your modem and modemmanager are able to talk to
each other, 

Re: [OpenWrt-Devel] Advice needed - Proper approach to port 5G/LTE modem into OpenWRT

2020-04-27 Thread Michael Jones
On Mon, Apr 27, 2020 at 7:42 AM Bjørn Mork  wrote:

> Jeonghum Joh  writes:
>
> > I am porting a 5G/LTE modem into OpenWRT.
>
> Follow the instructions for LTE modems.  A 5G modem is pretty much the
> same wrt drivers and basic management.  At least for Qualcomm based
> modems on a USB bus.  Have no experience with anything else.  The Intel
> and Huawei modems are competely unknown to me, and most likely
> unsupported for the forseeable future.  And I'm also blank on the magic
> of Qualcomms PCIe interface. Qualcomm did work on a driver, but it's
> been a long time since I saw any update on that.  I guess no one cares
> enough.  SuperSpeed USB is fine for most users for now.
>
> Anyway, several X55 based modems are already supported out of the box in
> OpenWrt master.  There is no need to reinvent the wheel if you are using
> one of those.
>
> You may obviously decide to implement your own alternative solutions,
> like using some vendor software. But that will limit the user community
> severely. At least until the solution attracts more users.  And
> community support depends on users, which I believe is something you
> should consider since you have ended up in this forum.  You are unlikely
> to find anyone here who have any experience with your particular vendor
> software version.
>
> Personally, I am happy to give advice about anything regardless of
> experience.  But the quality of that advice is probably a tiny bit
> better when it is based on something I've tried myself.  Or maybe not?
> Is probably bad in any case.
>
>
> Bjørn
>
>
@Jeonghum Joh

If you decide to use the connection management software that the vendor
supplied, you'll want to integrate it into Netifd and Hotplug.d to ensure
appropriate cross communication with things in the OpenWRT system.

Please look at this file to see an example of how that is done:
https://github.com/openwrt/packages/blob/master/net/modemmanager/files/modemmanager.proto
There's
a whole rabbit hole that you can follow on this topic to get every detail
right, but it's probably sufficient for your purposes to get the high level
details, and then let the OpenWRT stack take care of the rest.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Jsonfilter, return partial subobject?

2020-04-24 Thread Michael Jones
On Thu, Apr 23, 2020 at 6:38 PM Michael Jones  wrote:

>
> On Thu, Apr 23, 2020 at 3:50 PM Jo-Philipp Wich  wrote:
>
>> Hi,
>>
>> > [...]
>> > What I want to do is return a JSON string with this representation:
>> > [
>> > { "interface" : "lan", "uptime" : 1 },
>> > ...
>> > ]
>> >
>> > E.g., I want to filter the json not down to a single value, but to a
>> > collection of key-value pairs by excluding items that don't match.
>>
>> that is not directly possible. You can use the shell export mode together
>> with
>> the field separator to build a list of tuples safe for processing, printf
>> the
>> intermediate fields and finally use the array mode to build a proper list:
>>
>> -- 8< --
>> eval $(ubus call network.interface dump | \
>>  jsonfilter -F ': ' -e
>> 'tuples=@.interface[@.up=true]["interface","uptime"]')
>>
>> for tuple in $tuples; do
>>  printf '{ "%s": %d }\n' "${tuple%:*}" "${tuple#*:}"
>> done | jsonfilter -a -e @
>> -- >8 --
>>
>> Will result in something like the output below:
>>
>> [ { "lan": 4409874 }, { "loopback": 4409873 }, { "modem": 803939 }, {
>> "wan":
>> 4040845 }, { "wan6": 2681477 } ]
>>
>>
>> Returning subsets of objects is not directly supported unfortunately but
>> I'll
>> think about how to add something like this if I find the time.
>>
>> ~ Jo
>
>

As a follow on to this, in order to extract two values, one would need to
do something like this

eval $(ubus call network.interface dump | jsonfilter -F ':: ' -e
'tuples=@.interface[@.up=true]["interface","uptime","proto"]')

for tuple in $tuples
do
middle=$(expr $tuple : '.*:\(.*\):.*')
printf '{ "interface" : "%s", "uptime" : %d, "proto" : "%s"}\n'
"${tuple%%:*}" "$middle" "${tuple##*:}"
done | jsonfilter -a -e @

Notably this also breaks if the extracted text of each value contains
strings
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Jsonfilter, return partial subobject?

2020-04-23 Thread Michael Jones
On Thu, Apr 23, 2020 at 3:50 PM Jo-Philipp Wich  wrote:

> Hi,
>
> > [...]
> > What I want to do is return a JSON string with this representation:
> > [
> > { "interface" : "lan", "uptime" : 1 },
> > ...
> > ]
> >
> > E.g., I want to filter the json not down to a single value, but to a
> > collection of key-value pairs by excluding items that don't match.
>
> that is not directly possible. You can use the shell export mode together
> with
> the field separator to build a list of tuples safe for processing, printf
> the
> intermediate fields and finally use the array mode to build a proper list:
>
> -- 8< --
> eval $(ubus call network.interface dump | \
>  jsonfilter -F ': ' -e
> 'tuples=@.interface[@.up=true]["interface","uptime"]')
>
> for tuple in $tuples; do
>  printf '{ "%s": %d }\n' "${tuple%:*}" "${tuple#*:}"
> done | jsonfilter -a -e @
> -- >8 --
>
> Will result in something like the output below:
>
> [ { "lan": 4409874 }, { "loopback": 4409873 }, { "modem": 803939 }, {
> "wan":
> 4040845 }, { "wan6": 2681477 } ]
>
>
> Returning subsets of objects is not directly supported unfortunately but
> I'll
> think about how to add something like this if I find the time.
>
> ~ Jo
>
>
Thank you very much for the suggestion on using the -F flag, and the printf
command.

This is less verbose than the jshn.sh script that I wrote in my followup
email, so it will probably be what I use for now.

Please let me know if I can help with adding more capabilities to the
jsonfilter program.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] Jsonfilter, return partial subobject?

2020-04-23 Thread Michael Jones
On Thu, Apr 23, 2020 at 12:55 PM Michael Jones 
wrote:

> I'm quite reluctant to do multiple processing sweeps. E.g. the following
> (probably syntax-error-full, and erroneous output) shell script is
> extremely unattractive
>
> json=`ubus call network.interface dump | jsonfilter -e '@.interface'`
> echo '['
> for itf in $(echo $json | jsonfilter -e '@[@.up=true].interface')
> do
> echo "{ \"interface\" : \"$itf\" , \"uptime\" : $(echo $json |
> jsonfilter -e "@[@.interface=$itf].uptime") }," # Oh look, an unconditional
> trailing comma... that won't parse until JSON5...
> done
> echo ']'
>
>
I suppose I should follow my original email up with an acknowledgement of
the jshn.sh library (and jshn program).

So I know that the script could be improved to work (and work correctly)
like this:

. /usr/share/libubox/jshn.sh
json_init
json_add_array arr

jsonoutput=`ubus call network.interface dump | jsonfilter -e '@.interface'`

for itf in $(echo $jsonoutput | jsonfilter -e '@[@.up=true].interface')
do
json_add_object
json_add_string interface $itf
json_add_intuptime$(echo $jsonoutput | jsonfilter -e
"@[@.interface='$itf'].uptime")
json_close_object
done

json_close_array
json_dump | jsonfilter -e '@.arr'


But I didn't want to muddle the question of "can I do what I want with
jsonfilter by itself", because I don't want to involve the use of a shell
script at all if I can avoid it!
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] Jsonfilter, return partial subobject?

2020-04-23 Thread Michael Jones
I'm trying to work with the jsonfilter program included in the jsonpath
project from Jo-Philipp (https://git.openwrt.org/?p=project/jsonpath.gi
t)

Perhaps this is a failure on my part to understand the documentation
provided by this referenced webpage: http://goessner.net/articles/JsonPath/

I'm trying to filter out parts of a json string while still retaining some
structured identifying information.

For example, lets say, hypothetically, I had some JSON string containing an
array of objects, and I want only two key / value pairs to be returned.

Let's use the result of "ubus call network.interface dump" as the example:

This shell script
> ubus call network.interface dump | jsonfilter -e '@.interface' |
jsonfilter -e '@[@.up=true].interface'

Will return the following, on my OpenWRT system:
lan
loopback
wan
wan6
wwan

What I want to do is return a JSON string with this representation:
[
{ "interface" : "lan", "uptime" : 1 },
...
]

E.g., I want to filter the json not down to a single value, but to a
collection of key-value pairs by excluding items that don't match.


So, based on the documentation that I've read, the expression might look
something like this:

> ubus call network.interface dump | jsonfilter -e '@.interface' |
jsonfilter -e '@[@.up=true].[interface, uptime]'

Of course, that syntax does not work, and so far the only use of the union
operator in jsonfilter is like this expression, which returns all
interfaces (e.g. either up OR not up)

> ubus call network.interface dump | jsonfilter -e '@.interface' |
jsonfilter -e '@[@.up=true, @.up=false].interface'

I can't find any examples (beyond one or two very simplistic examples) of
how to use the jsonfilter program on stackoverflow, nor the OpenWRT mailing
list, nor the OpenWRT Discourse

I'm quite reluctant to do multiple processing sweeps. E.g. the following
(probably syntax-error-full, and erroneous output) shell script is
extremely unattractive

json=`ubus call network.interface dump | jsonfilter -e '@.interface'`
echo '['
for itf in $(echo $json | jsonfilter -e '@[@.up=true].interface')
do
echo "{ \"interface\" : \"$itf\" , \"uptime\" : $(echo $json |
jsonfilter -e "@[@.interface=$itf].uptime") }," # Oh look, an unconditional
trailing comma... that won't parse until JSON5...
done
echo ']'

Of course, the json string that I'm trying to manipulate is not coming from
"ubus call network.interface dump", it's from a third party program that
can't be modified, I'm only using the ubus JSON as an example to illustrate
the general concept of what I'm asking for.


Is this something that jsonfilter is capable of doing at all? Or am I
having a fundamental misunderstanding of the program's capabilities?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] rpcd: fix respawn settings

2020-03-11 Thread Michael Jones
On Fri, Mar 6, 2020 at 1:19 AM Jo-Philipp Wich  wrote:

> Hi,
>
> > rpcd crashes for me daily, to the point where i have a script that
> restarts it
> > every 5 minutes.
> >
> > It also gets hung a lot without crashing, and stops serving responses to
> ubus
> > traffic.
>
> I've never heard about anything like that until now, not even in the forum
> or
> IRC chatter. Getting some details here would be interesting.
>
> ~ Jo
>

I've reviewed the mailing list for the previous year, and found this post:

http://lists.infradead.org/pipermail/openwrt-devel/2019-October/019592.html

Which appears to have been merged into rpcd with this commit :
https://git.openwrt.org/?p=project/rpcd.git;a=commit;h=bd0ed2521476c3e5b6c1a0e0bd2c386ea809d74b

This post / commit appears to identify the same crash that my scripts cause.

However, the commit for the OpenWRT 18.06 branch (Still receiving security
fixes, as far as I can tell), has this commit for RPCD

https://github.com/openwrt/openwrt/blob/openwrt-18.06/package/system/rpcd/Makefile

commit 3aa81d0dfae167eccc26203bd0c96f3e3450f253
Author: Jo-Philipp Wich 
Date:   Wed Nov 28 12:12:04 2018 +0100

file: access exec timeout via daemon ops structure

Since the plugin is not linked, but dlopen()'d with RTLD_LOCAL, we
cannot
access global rpcd variables but need to access them via the common ops
structure symbol.

Signed-off-by: Jo-Philipp Wich 



To see if that really fixed the issue, I will update my build of rpcd
from 3aa81d0dfae167eccc26203bd0c96f3e3450f253
to bd0ed2521476c3e5b6c1a0e0bd2c386ea809d74b (or git head, perhaps) to see
if the crash gets resolved.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] rpcd: fix respawn settings

2020-03-05 Thread Michael Jones
On Thu, Mar 5, 2020 at 1:41 PM Petr Štetiar  wrote:

>
>
> Mar 5, 2020 19:54:49 Michael Jones :
>
> > The flip side here is that rpcd likes to crash a lot.
>
> 0 (zero) bugs found https://bugs.openwrt.org/index.php?string=rpcd


Saying there are zero bugs on a bug tracker where issues go to be ignored
is not a convincing argument.

rpcd crashes for me daily, to the point where i have a script that restarts
it every 5 minutes.

It also gets hung a lot without crashing, and stops serving responses to
ubus traffic.

This is *only* with the UCI plugin, mind you. I don't use any of the other
ones.

If I create a bug report on flyspray, will it actually be looked at? Or
will I be talking to myself?

OpenWRT has a well-deserved reputation for user originated bug reports and
requests for help going ignored. I've asked dozens of questions over the
years on the forums that received no answer, and I've filed bugs that were
still open with no feedback from anyone, last I bothered to check (Note:
Not many of them have this email associated. I've worked many jobs that
involved openwrt in some way)

Note: I don't have any animosity about this. Volunteers are volunteers, I'm
not expecting anyone to do anything. I'm just saying that that's not a
valid argument unless or until the OpenWRT community engagement improves to
the point where the bug tracker and forum stop being echo chambers. Will
that happen? I don't know. Should it happen? I don't know.


> By preventing automatic restarts, you're all but ensuring that users will
> experience denial-of-service, even in the absence of malicious traffic.
>
> Default respawn retry value was 5, now is infinite and this patch restores
> it back to 5 respawns.
>

Right, which means that you're re-introducing the
denial-of-service-in-the-absence-of-traffic problem. I'm not saying that's
the wrong thing to do.

>
> > Is rpcd subject to fuzz testing, to discover potential security issues
>
> Not yet, it's planed. It's just one of the methods, you'll never be 100%
> sure anyway.
>

How can I help?

I don't accept that you can't be 100% certain. Tools like
https://klee.github.io/ can get you so close to 100% certainty that it's
effectively 100%.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] rpcd: fix respawn settings

2020-03-05 Thread Michael Jones
On Thu, Mar 5, 2020 at 5:35 AM Petr Štetiar  wrote:

> Karl Palsson  [2020-03-05 11:18:02]:
>
> > > Commit 432ec292ccc8 ("rpcd: add respawn param") has introduced
> > > infinite restarting of the service which could be reached over
> > > network.
> >
> > Didn't we already decide that this wasn't the case?
>
> < jow> ubus itself has no network transport
> < jow> it is reachable via http://.../ubus in case uhttpd-mod-ubus is
> installed (not the default) or via http://.../cgi-bin/luci/admin/ubus
> (default)
> < jow> the latter emulates uhttpd-mob-ubus in Lua code
> < jow> it takes incoming http requests, parses the body json and invokes
> ubus via libubus
>
> I understand this as Yes, it is available over network.
>
> > Sure, now it's a DoS instead :) It's always a tradeoff, but I
> > think you're glossing over the tradeoff here.
>
> Secure by default.
>
> -- ynezz
>
>
The flip side here is that rpcd likes to crash a lot.

By preventing automatic restarts, you're all but ensuring that users will
experience denial-of-service, even in the absence of malicious traffic.

Is rpcd subject to fuzz testing, to discover potential security issues that
makes limiting the restarts attractive?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] ccache: update to 3.7.7

2020-01-16 Thread Michael Jones
On Thu, Jan 16, 2020 at 12:42 PM Hans Dedecker  wrote:

> On Thu, Jan 16, 2020 at 4:36 PM DENG Qingfang 
> wrote:
> >
> > Update ccache to 3.7.7
> >
> > Release notes:
> > https://ccache.dev/releasenotes.html#_ccache_3_7_7
> >
> > Signed-off-by: DENG Qingfang 
> Patch applied, thx
>
> Hans
> > ---
> >  tools/ccache/Makefile   | 4 ++--
> >  tools/ccache/patches/100-honour-copts.patch | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/ccache/Makefile b/tools/ccache/Makefile
> > index 50a1a412a8..4e99207872 100644
> > --- a/tools/ccache/Makefile
> > +++ b/tools/ccache/Makefile
> > @@ -8,11 +8,11 @@ include $(TOPDIR)/rules.mk
> >  include $(INCLUDE_DIR)/target.mk
> >
> >  PKG_NAME:=ccache
> > -PKG_VERSION:=3.7.6
> > +PKG_VERSION:=3.7.7
> >
> >  PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.xz
> >  PKG_SOURCE_URL:=
> https://github.com/ccache/ccache/releases/download/v$(PKG_VERSION)
> >
> -PKG_HASH:=73e2633ac9bca387b5a39c72a8f85634670c4091dab639228c433898163c86c0
> >
> +PKG_HASH:=b7c1d6d6fe42f18e424de92746af863e0bc85794da3d69e44300840c478c98cd
> >
> >  include $(INCLUDE_DIR)/host-build.mk
> >
> > diff --git a/tools/ccache/patches/100-honour-copts.patch
> b/tools/ccache/patches/100-honour-copts.patch
> > index 97bacae2d2..a3cef56213 100644
> > --- a/tools/ccache/patches/100-honour-copts.patch
> > +++ b/tools/ccache/patches/100-honour-copts.patch
> > @@ -1,6 +1,6 @@
> >  --- a/src/ccache.c
> >  +++ b/src/ccache.c
> > -@@ -2224,6 +2224,7 @@ calculate_object_hash(struct args *args,
> > +@@ -2220,6 +2220,7 @@ calculate_object_hash(struct args *args,
> > "CPLUS_INCLUDE_PATH",
> > "OBJC_INCLUDE_PATH",
> > "OBJCPLUS_INCLUDE_PATH", // clang
> > --
> > 2.24.1
>
>

In my testing, I've found that ccache measurably does not change the build
times for OpenWRT.

Is your experience different?

Relevant flyspray conversations:

https://bugs.openwrt.org/index.php?do=details_id=2596
https://bugs.openwrt.org/index.php?do=details_id=2597

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] UBus wire protocol documentation

2019-12-07 Thread Michael Jones
I'm planning to implement a Ubus client using Boost.Asio / C++'s Networking
TS, in C++, complete with a new implementation of the ubus wire protocol
(in C++).

Is there any documentation available about the Ubus wire protocol?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v2 libubox 05/10] add assert.h component

2019-11-21 Thread Michael Jones
On Thu, Nov 21, 2019 at 12:17 AM Petr Štetiar  wrote:

> Michael Jones  [2019-11-20 18:12:14]:
>
> > You may need #include_next, to avoid recursion.
>
> Is this some theoretical experience? Or you can provide me with some build
> breakage? Just wondering, because #include should work this out with "" and
> <>.
>
> BTW I would rather rename that include file to assert_internal.h, then
> using
> include_next. Anyway, as this builds fine on gcc 4.8,6,7,8,9 and with
> clang-7,8,9,10 I would say, that it should be fine as it is.
>
> -- ynezz
>

My experience with this was with MSVC in 2015 or so, and a heavily patched
version of STLPort.

It's not obvious to me that you would encounter the same problem with GCC /
Clang.

I'm personally instantly averse to possible recursion issues because of how
tricky they can be to debug, but if no one is concerned about it, it's not
really an issue.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v2 libubox 05/10] add assert.h component

2019-11-20 Thread Michael Jones
You may need #include_next, to avoid recursion.

On Wed, Nov 20, 2019, 15:45 Petr Štetiar  wrote:

> In order to allow seamless assert() usage in release builds without the
> need for fiddling with CMake C flags as CMake adds -DNDEBUG switch in
> release builds which disable assert().
>
> Signed-off-by: Petr Štetiar 
> ---
>  assert.h | 9 +
>  1 file changed, 9 insertions(+)
>  create mode 100644 assert.h
>
> diff --git a/assert.h b/assert.h
> new file mode 100644
> index ..84f54718366a
> --- /dev/null
> +++ b/assert.h
> @@ -0,0 +1,9 @@
> +#pragma once
> +
> +#ifdef NDEBUG
> +#undef NDEBUG
> +#include 
> +#define NDEBUG
> +#else
> +#include 
> +#endif
>
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v2 1/1] rpcd: add respawn param

2019-11-11 Thread Michael Jones
On Sat, Nov 9, 2019 at 6:53 AM Hauke Mehrtens  wrote:

> On 11/7/19 2:31 PM, Florian Eckert wrote:
> > The rpcd service is an important service, but if the service stops
> > working for any reason, no one will ever respawn that service. With this
> > commit, the procd service will monitor if the rpcd service
> > is running. If the rpcd service has crashed, then
> > procd respawns the rpcd service.
> >
> > Signed-off-by: Florian Eckert 
> > ---
> >  package/system/rpcd/Makefile| 2 +-
> >  package/system/rpcd/files/rpcd.init | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/system/rpcd/Makefile b/package/system/rpcd/Makefile
> > index 6f23bbe96e..fcbcc613a6 100644
> > --- a/package/system/rpcd/Makefile
> > +++ b/package/system/rpcd/Makefile
> > @@ -8,7 +8,7 @@
> >  include $(TOPDIR)/rules.mk
> >
> >  PKG_NAME:=rpcd
> > -PKG_RELEASE:=1
> > +PKG_RELEASE:=2
> >
> >  PKG_SOURCE_PROTO:=git
> >  PKG_SOURCE_URL=$(PROJECT_GIT)/project/rpcd.git
> > diff --git a/package/system/rpcd/files/rpcd.init
> b/package/system/rpcd/files/rpcd.init
> > index 447133c67a..3e9ea5bbf3 100755
> > --- a/package/system/rpcd/files/rpcd.init
> > +++ b/package/system/rpcd/files/rpcd.init
> > @@ -12,6 +12,7 @@ start_service() {
> >
> >   procd_open_instance
> >   procd_set_param command "$PROG" ${socket:+-s "$socket"}
> ${timeout:+-t "$timeout"}
> > + procd_set_param respawn ${respawn_retry:-0}
>
> Why do you set the respawn_retry to 0 by default?
>
> >   procd_close_instance
> >  }
> >
>

(Resending to include the list itself. Apologies if anyone receives a
duplicate.)

How would procd determine that rpcd is stuck, instead of crashed?

Is there support for some kind of heartbeating behavior, similar to what
systemd offers?

One would imagine that procd's close integration with ubus would allow for
this functionality to exist without a lot of additional code.

http://0pointer.de/blog/projects/watchdog.html
> This service will automatically be restarted if it hasn't pinged the
system manager for longer than 30s or if it fails otherwise. If it is
restarted this way more often than 4 times in 5min action is taken and the
system quickly rebooted, with all file systems being clean when it comes up
again.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel