nodewatcher: Change mechanism for client device detection

This is simpler than the previous approach and does not rely
on parsing.

This fixes:
- Interfaces being accounted for multiple times for certain
  devices
- Errors when output of bridge function changes (as with the
  current OpenWrt master)

Behavior change: Only br-mesh is evaluated, in contrast to all
bridge devices as before.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

Tested-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
---
 src/packages/fff/fff-nodewatcher/Makefile                   | 2 +-
 src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Hallo Adrian,

Es gibt eine Variable “MESH_INTERFACE”, die sollte da verwendet werden..
Außerdem: Das Verhalten wird nicht wirklich geändert, denn die Clientzahl weiterer (nicht-Client) Interfaces wird durch das “br $MESH_INTERFACE” im bridge fdb gar nicht gezählt.

Ansonsten gefällt mir die Lösung sehr gut, aber ich sehe nicht, warum man den Output von stderr im ls nach /dev/null haben möchte.

Gruß
Fabian

> On 23. Jul 2018, at 15:39, Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
> 
> This is simpler than the previous approach and does not rely
> on parsing.
> 
> This fixes:
> - Interfaces being accounted for multiple times for certain
>  devices
> - Errors when output of bridge function changes (as with the
>  current OpenWrt master)
> 
> Behavior change: Only br-mesh is evaluated, in contrast to all
> bridge devices as before.
> 
> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> 
> Tested-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> ---
> src/packages/fff/fff-nodewatcher/Makefile                   | 2 +-
> src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/packages/fff/fff-nodewatcher/Makefile b/src/packages/fff/fff-nodewatcher/Makefile
> index 633ec71..12ccb0f 100644
> --- a/src/packages/fff/fff-nodewatcher/Makefile
> +++ b/src/packages/fff/fff-nodewatcher/Makefile
> @@ -1,7 +1,7 @@
> include $(TOPDIR)/rules.mk
> 
> PKG_NAME:=fff-nodewatcher
> -PKG_VERSION:=47
> +PKG_VERSION:=48
> PKG_RELEASE:=1
> 
> PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> diff --git a/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher b/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> index 3b60500..f655e5e 100755
> --- a/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> +++ b/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> @@ -2,7 +2,7 @@
> # Netmon Nodewatcher (C) 2010-2012 Freifunk Oldenburg
> # License; GPL v3
> 
> -SCRIPT_VERSION="47"
> +SCRIPT_VERSION="48"
> 
> test -f /tmp/started || exit
> 
> @@ -260,7 +260,7 @@ crawl() {
>     #CLIENTS
>     client_count=0
>     dataclient=""
> -    CLIENT_INTERFACES=$(bridge link | awk '$2 !~/^bat/{ printf $2" " }')
> +    CLIENT_INTERFACES=$(ls /sys/class/net/br-mesh/brif 2>/dev/null | grep -v '^bat')
>     for clientif in ${CLIENT_INTERFACES}; do
>         local cc=$(bridge fdb show br "$MESH_INTERFACE" brport "$clientif" | grep -v self | grep -v permanent -c)
>         client_count=$((client_count + cc))
> --
> 2.11.0
>

Hallo Fabian,

danke für das Feedback.

Rest unten.

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf Of
> Fabian Bläse
> Sent: Dienstag, 24. Juli 2018 08:40
> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> Cc: franken-dev <franken-dev@freifunk.net>
> Subject: Re: [PATCH] nodewatcher: Change mechanism for client device
> detection
> 
> Hallo Adrian,
> 
> Es gibt eine Variable “MESH_INTERFACE”, die sollte da verwendet werden..

Ja.

> Außerdem: Das Verhalten wird nicht wirklich geändert, denn die Clientzahl
> weiterer (nicht-Client) Interfaces wird durch das “br $MESH_INTERFACE” im
> bridge fdb gar nicht gezählt.

Ja.

> 
> Ansonsten gefällt mir die Lösung sehr gut, aber ich sehe nicht, warum man den
> Output von stderr im ls nach /dev/null haben möchte.

ls schreibt ins stderr, wenn kein br-mesh existiert. Ich wollte das nicht als "Fehler" ansehen, theoretisch könnte ja ein Setup ohne br-mesh existieren und der nodewatcher würde trotzdem funktionieren. Man kann den redirect aber auch weglassen, im Prinzip isses mir egal.

Vielleicht hier nochmal kurz Feedback, dann mach ich nen V2.

Grüße

Adrian

> 
> Gruß
> Fabian
> 
> > On 23. Jul 2018, at 15:39, Adrian Schmutzler <freifunk@adrianschmutzler.de>
> wrote:
> >
> > This is simpler than the previous approach and does not rely
> > on parsing.
> >
> > This fixes:
> > - Interfaces being accounted for multiple times for certain
> >  devices
> > - Errors when output of bridge function changes (as with the
> >  current OpenWrt master)
> >
> > Behavior change: Only br-mesh is evaluated, in contrast to all
> > bridge devices as before.
> >
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> >
> > Tested-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> > ---
> > src/packages/fff/fff-nodewatcher/Makefile                   | 2 +-
> > src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher | 4 ++--
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/packages/fff/fff-nodewatcher/Makefile b/src/packages/fff/fff-
> nodewatcher/Makefile
> > index 633ec71..12ccb0f 100644
> > --- a/src/packages/fff/fff-nodewatcher/Makefile
> > +++ b/src/packages/fff/fff-nodewatcher/Makefile
> > @@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> >
> > PKG_NAME:=fff-nodewatcher
> > -PKG_VERSION:=47
> > +PKG_VERSION:=48
> > PKG_RELEASE:=1
> >
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> b/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> > index 3b60500..f655e5e 100755
> > --- a/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> > +++ b/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> > @@ -2,7 +2,7 @@
> > # Netmon Nodewatcher (C) 2010-2012 Freifunk Oldenburg
> > # License; GPL v3
> >
> > -SCRIPT_VERSION="47"
> > +SCRIPT_VERSION="48"
> >
> > test -f /tmp/started || exit
> >
> > @@ -260,7 +260,7 @@ crawl() {
> >     #CLIENTS
> >     client_count=0
> >     dataclient=""
> > -    CLIENT_INTERFACES=$(bridge link | awk '$2 !~/^bat/{ printf $2" " }')
> > +    CLIENT_INTERFACES=$(ls /sys/class/net/br-mesh/brif 2>/dev/null | grep -v
> '^bat')
> >     for clientif in ${CLIENT_INTERFACES}; do
> >         local cc=$(bridge fdb show br "$MESH_INTERFACE" brport "$clientif" |
> grep -v self | grep -v permanent -c)
> >         client_count=$((client_count + cc))
> > --
> > 2.11.0
> >

Hmm, ich bin mir nicht sicher, ob es eine gute Idee ist die Fehlermeldung zu unterdrücken.
Aber da es nur der Nodewatcher ist..

Warte mit einer v2 am besten mal noch auf weiteres Feedback, ist ja nichts akutes.

Gruß
Fabian

> On 24. Jul 2018, at 11:53, Adrian Schmutzler <mail@adrianschmutzler.de> wrote:
> 
>> Ansonsten gefällt mir die Lösung sehr gut, aber ich sehe nicht, warum man den
>> Output von stderr im ls nach /dev/null haben möchte.
> 
> ls schreibt ins stderr, wenn kein br-mesh existiert. Ich wollte das nicht als "Fehler" ansehen, theoretisch könnte ja ein Setup ohne br-mesh existieren und der nodewatcher würde trotzdem funktionieren. Man kann den redirect aber auch weglassen, im Prinzip isses mir egal.

Am Dienstag, den 24.07.2018, 08:40 +0200 schrieb Fabian Bläse:
> Hallo Adrian,
> 
> Es gibt eine Variable “MESH_INTERFACE”, die sollte da verwendet
> werden..
Ja, das wäre gut.

> Außerdem: Das Verhalten wird nicht wirklich geändert, denn die
> Clientzahl weiterer (nicht-Client) Interfaces wird durch das “br
> $MESH_INTERFACE” im bridge fdb gar nicht gezählt.
> 
> Ansonsten gefällt mir die Lösung sehr gut, aber ich sehe nicht, warum
> man den Output von stderr im ls nach /dev/null haben möchte.
Wäre auch eher dafür den Fehler zu sehen. Immerhin ist der Nodewatcher
so gedacht, das auszulesen, wenn das Interface nicht da ist, soll es
eine Meldung geben. Sonst sucht sich am Ende der User dumm und dämlich
weil die Anzahl der Clients nicht gemeldet wird. Aber im Grunde ist mir
das nicht so wichtig.

Tim

> Gruß
> Fabian
> 
> > On 23. Jul 2018, at 15:39, Adrian Schmutzler <freifunk@adrianschmut
> > zler.de> wrote:
> > 
> > This is simpler than the previous approach and does not rely
> > on parsing.
> > 
> > This fixes:
> > - Interfaces being accounted for multiple times for certain
> >  devices
> > - Errors when output of bridge function changes (as with the
> >  current OpenWrt master)
> > 
> > Behavior change: Only br-mesh is evaluated, in contrast to all
> > bridge devices as before.
> > 
> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> > 
> > Tested-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> > ---
> > src/packages/fff/fff-nodewatcher/Makefile                   | 2 +-
> > src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher | 4 ++-
> > -
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/packages/fff/fff-nodewatcher/Makefile
> > b/src/packages/fff/fff-nodewatcher/Makefile
> > index 633ec71..12ccb0f 100644
> > --- a/src/packages/fff/fff-nodewatcher/Makefile
> > +++ b/src/packages/fff/fff-nodewatcher/Makefile
> > @@ -1,7 +1,7 @@
> > include $(TOPDIR)/rules.mk
> > 
> > PKG_NAME:=fff-nodewatcher
> > -PKG_VERSION:=47
> > +PKG_VERSION:=48
> > PKG_RELEASE:=1
> > 
> > PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)
> > diff --git a/src/packages/fff/fff-
> > nodewatcher/files/usr/sbin/nodewatcher b/src/packages/fff/fff-
> > nodewatcher/files/usr/sbin/nodewatcher
> > index 3b60500..f655e5e 100755
> > --- a/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> > +++ b/src/packages/fff/fff-nodewatcher/files/usr/sbin/nodewatcher
> > @@ -2,7 +2,7 @@
> > # Netmon Nodewatcher (C) 2010-2012 Freifunk Oldenburg
> > # License; GPL v3
> > 
> > -SCRIPT_VERSION="47"
> > +SCRIPT_VERSION="48"
> > 
> > test -f /tmp/started || exit
> > 
> > @@ -260,7 +260,7 @@ crawl() {
> >     #CLIENTS
> >     client_count=0
> >     dataclient=""
> > -    CLIENT_INTERFACES=$(bridge link | awk '$2 !~/^bat/{ printf $2"
> > " }')
> > +    CLIENT_INTERFACES=$(ls /sys/class/net/br-mesh/brif 2>/dev/null
> > | grep -v '^bat')
> >     for clientif in ${CLIENT_INTERFACES}; do
> >         local cc=$(bridge fdb show br "$MESH_INTERFACE" brport
> > "$clientif" | grep -v self | grep -v permanent -c)
> >         client_count=$((client_count + cc))
> > --
> > 2.11.0
> > 
> 
>