| Message ID | 20200616082613.892-1-fe@dev.tdt.de |
|---|---|
| State | Superseded |
| Delegated to: | John Crispin |
| Headers | show |
| Series | [OpenWrt-Devel] lantiq: add dsl line_state mapping | expand |
Hi, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Florian Eckert > Sent: Dienstag, 16. Juni 2020 10:26 > To: john@phrozen.org; dev@kresin.me; Eckert.Florian@googlemail.com > Cc: openwrt-devel@lists.openwrt.org; Florian Eckert <fe@dev.tdt.de> > Subject: [OpenWrt-Devel] [PATCH] lantiq: add dsl line_state mapping > > The line_state of the DSL connection is described in the system via a > hexadecimal variable. With this change the hexadecimal is mapped to a > decimal value. With this change it is now possible to store this value in a > database, so that it can be easily evaluated. > > This is especially relevant for the collectd and gravana backend. > > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > --- > .../base-files/lib/functions/lantiq_dsl.sh | 82 +++++++++++++------ > 1 file changed, 55 insertions(+), 27 deletions(-) > > diff --git a/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > b/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > index 11b02fc4aa..4827d10bc5 100755 > --- a/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > +++ b/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > @@ -650,40 +650,68 @@ line_data() { > line_state() { > local lsg=$(dsl_cmd lsg) > local ls=$(dsl_val "$lsg" nLineState); > - local s; > + local s n; > > case "$ls" in > - "0x0") s="not initialized" ;; > - "0x1") s="exception" ;; > - "0x10") s="not updated" ;; > - "0xff") s="idle request" ;; > - "0x100") s="idle" ;; > - "0x1ff") s="silent request" ;; > - "0x200") s="silent" ;; > - "0x300") s="handshake" ;; > - "0x380") s="full_init" ;; > - "0x400") s="discovery" ;; > - "0x500") s="training" ;; > - "0x600") s="analysis" ;; > - "0x700") s="exchange" ;; > - "0x800") s="showtime_no_sync" ;; > - "0x801") s="showtime_tc_sync" ;; > - "0x900") s="fastretrain" ;; > - "0xa00") s="lowpower_l2" ;; > - "0xb00") s="loopdiagnostic active" ;; > - "0xb10") s="loopdiagnostic data exchange" ;; > - "0xb20") s="loopdiagnostic data request" ;; > - "0xc00") s="loopdiagnostic complete" ;; > - "0x1000000") s="test" ;; > - "0xd00") s="resync" ;; > - "0x3c0") s="short init entry" ;; > - "") s="not running daemon"; ls="0xfff" ;; > - *) s="unknown" ;; > + "0x0") s="not initialized" > + n=1 ;; Wouldn't it be more user-friendly to just use the decimal number equivalent of the hex code? Empty and error could be modelled with negative numbers then. Just meant as a suggestion, I won't block this if you keep the current scheme ... Best Adrian > + "0x1") s="exception" > + n=2 ;; > + "0x10") s="not updated" > + n=3 ;; > + "0xff") s="idle request" > + n=4 ;; > + "0x100") s="idle" > + n=5 ;; > + "0x1ff") s="silent request" > + n=6 ;; > + "0x200") s="silent" > + n=7 ;; > + "0x300") s="handshake" > + n=8 ;; > + "0x380") s="full_init" > + n=9 ;; > + "0x400") s="discovery" > + n=10 ;; > + "0x500") s="training" > + n=11 ;; > + "0x600") s="analysis" > + n=12 ;; > + "0x700") s="exchange" > + n=13 ;; > + "0x800") s="showtime_no_sync" > + n=14 ;; > + "0x801") s="showtime_tc_sync" > + n=15 ;; > + "0x900") s="fastretrain" > + n=16 ;; > + "0xa00") s="lowpower_l2" > + n=17 ;; > + "0xb00") s="loopdiagnostic active" > + n=18 ;; > + "0xb10") s="loopdiagnostic data exchange" > + n=19 ;; > + "0xb20") s="loopdiagnostic data request" > + n=20 ;; > + "0xc00") s="loopdiagnostic complete" > + n=21 ;; > + "0x1000000") s="test" > + n=22 ;; > + "0xd00") s="resync" > + n=23 ;; > + "0x3c0") s="short init entry" > + n=24 ;; > + "") s="not running daemon" > + ls="0xfff" > + n=25 ;; > + *) s="unknown" > + n=26 ;; > esac > > if [ "$action" = "lucistat" ]; then > echo "dsl.line_state_num=$ls" > echo "dsl.line_state_detail=\"$s\"" > + echo "dsl.line_state_mapping=$n" > if [ "$ls" = "0x801" ]; then > echo "dsl.line_state=\"UP\"" > else > -- > 2.20.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Hi Adrian, thanks for your comment >> - "0xd00") s="resync" ;; >> - "0x3c0") s="short init entry" ;; >> - "") s="not running daemon"; ls="0xfff" ;; >> - *) s="unknown" ;; >> + "0x0") s="not initialized" >> + n=1 ;; > > Wouldn't it be more user-friendly to just use the decimal number > equivalent of the hex code? I've been thinking the same thing. But then I have a problem when I want to display the values over time (collectd/gravana). From my point of view the distances are then too small or too large it does not give equal gradations. >> - "0x1000000") s="test" ;; Especially the test value jumps out of line there. I think this is unlikely and could be neglected, but I would like to have a linear gradation. >> - "") s="not running daemon"; ls="0xfff" ;; >> - *) s="unknown" ;; > Empty and error could be modelled with negative numbers then. We can still do that. So we have for "" and * the value -1 and -2. Best regards Florian
> -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Florian Eckert > Sent: Dienstag, 16. Juni 2020 10:26 > To: john@phrozen.org; dev@kresin.me; Eckert.Florian@googlemail.com > Cc: openwrt-devel@lists.openwrt.org; Florian Eckert <fe@dev.tdt.de> > Subject: [OpenWrt-Devel] [PATCH] lantiq: add dsl line_state mapping > > The line_state of the DSL connection is described in the system via a > hexadecimal variable. With this change the hexadecimal is mapped to a > decimal value. With this change it is now possible to store this value in a > database, so that it can be easily evaluated. Interesting file this lantiq_dsl.sh ... I'm wondering whether all of this really need to be in this file, or whether stuff can be moved to the package actually dealing with it? This might also make it easier to change it when necessary. Adrian > > This is especially relevant for the collectd and gravana backend. > > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > --- > .../base-files/lib/functions/lantiq_dsl.sh | 82 +++++++++++++------ > 1 file changed, 55 insertions(+), 27 deletions(-) > > diff --git a/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > b/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > index 11b02fc4aa..4827d10bc5 100755 > --- a/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > +++ b/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh > @@ -650,40 +650,68 @@ line_data() { > line_state() { > local lsg=$(dsl_cmd lsg) > local ls=$(dsl_val "$lsg" nLineState); > - local s; > + local s n; > > case "$ls" in > - "0x0") s="not initialized" ;; > - "0x1") s="exception" ;; > - "0x10") s="not updated" ;; > - "0xff") s="idle request" ;; > - "0x100") s="idle" ;; > - "0x1ff") s="silent request" ;; > - "0x200") s="silent" ;; > - "0x300") s="handshake" ;; > - "0x380") s="full_init" ;; > - "0x400") s="discovery" ;; > - "0x500") s="training" ;; > - "0x600") s="analysis" ;; > - "0x700") s="exchange" ;; > - "0x800") s="showtime_no_sync" ;; > - "0x801") s="showtime_tc_sync" ;; > - "0x900") s="fastretrain" ;; > - "0xa00") s="lowpower_l2" ;; > - "0xb00") s="loopdiagnostic active" ;; > - "0xb10") s="loopdiagnostic data exchange" ;; > - "0xb20") s="loopdiagnostic data request" ;; > - "0xc00") s="loopdiagnostic complete" ;; > - "0x1000000") s="test" ;; > - "0xd00") s="resync" ;; > - "0x3c0") s="short init entry" ;; > - "") s="not running daemon"; ls="0xfff" ;; > - *) s="unknown" ;; > + "0x0") s="not initialized" > + n=1 ;; > + "0x1") s="exception" > + n=2 ;; > + "0x10") s="not updated" > + n=3 ;; > + "0xff") s="idle request" > + n=4 ;; > + "0x100") s="idle" > + n=5 ;; > + "0x1ff") s="silent request" > + n=6 ;; > + "0x200") s="silent" > + n=7 ;; > + "0x300") s="handshake" > + n=8 ;; > + "0x380") s="full_init" > + n=9 ;; > + "0x400") s="discovery" > + n=10 ;; > + "0x500") s="training" > + n=11 ;; > + "0x600") s="analysis" > + n=12 ;; > + "0x700") s="exchange" > + n=13 ;; > + "0x800") s="showtime_no_sync" > + n=14 ;; > + "0x801") s="showtime_tc_sync" > + n=15 ;; > + "0x900") s="fastretrain" > + n=16 ;; > + "0xa00") s="lowpower_l2" > + n=17 ;; > + "0xb00") s="loopdiagnostic active" > + n=18 ;; > + "0xb10") s="loopdiagnostic data exchange" > + n=19 ;; > + "0xb20") s="loopdiagnostic data request" > + n=20 ;; > + "0xc00") s="loopdiagnostic complete" > + n=21 ;; > + "0x1000000") s="test" > + n=22 ;; > + "0xd00") s="resync" > + n=23 ;; > + "0x3c0") s="short init entry" > + n=24 ;; > + "") s="not running daemon" > + ls="0xfff" > + n=25 ;; > + *) s="unknown" > + n=26 ;; > esac > > if [ "$action" = "lucistat" ]; then > echo "dsl.line_state_num=$ls" > echo "dsl.line_state_detail=\"$s\"" > + echo "dsl.line_state_mapping=$n" > if [ "$ls" = "0x801" ]; then > echo "dsl.line_state=\"UP\"" > else > -- > 2.20.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Hi Adrian, >> The line_state of the DSL connection is described in the system via a >> hexadecimal variable. With this change the hexadecimal is mapped to a >> decimal value. With this change it is now possible to store this value >> in a >> database, so that it can be easily evaluated. > > Interesting file this lantiq_dsl.sh ... > That´s probably right! > I'm wondering whether all of this really need to be in this file, or > whether stuff can be moved to the package actually dealing with it? > This might also make it easier to change it when necessary. > This file is sourced twice: - dsl_control of package ltq-adsl-app [1] - dsl_control of package ltq-vdsl-app [2] If we take this from the target folder then we have to make our own packet ltq-dsl-common for example. And the packages ltq-adsl-app and ltq-vdsl-app could depend on this. When we create a new package, we may also want to move other files from the target directory to the new package? - lantiq.sh [3] This is sourced in 02_network files on the lantiq targets. - led_dsl.sh [4] - pppoa.sh [5] - uci-defaults [6] - dsl_notify.sh [7] These are candidates that could also moved to the new package Best regards Florian [1] https://github.com/openwrt/openwrt/blob/master/package/network/config/ltq-adsl-app/files/dsl_control#L11 [2] https://github.com/openwrt/openwrt/blob/master/package/network/config/ltq-vdsl-app/files/dsl_control#L11 [3] https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/lib/functions/lantiq.sh [4] https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/etc/hotplug.d/dsl/led_dsl.sh [5] https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/etc/hotplug.d/dsl/pppoa.sh [6] https://github.com/openwrt/openwrt/tree/master/target/linux/lantiq/base-files/etc/uci-defaults [7] https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/sbin/dsl_notify.sh
Hi Florian, > On Jun 16, 2020, at 14:18, Florian Eckert <fe@dev.tdt.de> wrote: > > Hi Adrian, > > thanks for your comment > >>> - "0xd00") s="resync" ;; >>> - "0x3c0") s="short init entry" ;; >>> - "") s="not running daemon"; ls="0xfff" ;; >>> - *) s="unknown" ;; >>> + "0x0") s="not initialized" >>> + n=1 ;; >> Wouldn't it be more user-friendly to just use the decimal number >> equivalent of the hex code? > > I've been thinking the same thing. > But then I have a problem when I want to display the values over time (collectd/gravana). > From my point of view the distances are then too small or too large it does not give equal gradations. I guess lantiq (or rather maxlinear nowadays) might introduce new hex codes into existing gaps which will cause a distruption of the nice monotonic order oc the enumerating decimals you went for. Is there no other data type in graphana that is ordered yet not scaled? Best Regards Sebastian > >>> - "0x1000000") s="test" ;; > > Especially the test value jumps out of line there. I think this is unlikely and could be neglected, > but I would like to have a linear gradation. > > >>> - "") s="not running daemon"; ls="0xfff" ;; >>> - *) s="unknown" ;; >> Empty and error could be modelled with negative numbers then. > > We can still do that. So we have for "" and * the value -1 and -2. > > Best regards > > Florian > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 2020-06-17 12:21, Florian Eckert wrote: > Hi Adrian, > >>> The line_state of the DSL connection is described in the system via a >>> hexadecimal variable. With this change the hexadecimal is mapped to a >>> decimal value. With this change it is now possible to store this >>> value in a >>> database, so that it can be easily evaluated. >> >> Interesting file this lantiq_dsl.sh ... >> > > That´s probably right! > >> I'm wondering whether all of this really need to be in this file, or >> whether stuff can be moved to the package actually dealing with it? >> This might also make it easier to change it when necessary. >> > > This file is sourced twice: > - dsl_control of package ltq-adsl-app [1] > - dsl_control of package ltq-vdsl-app [2] > > If we take this from the target folder then we have to make our own > packet ltq-dsl-common for example. > And the packages ltq-adsl-app and ltq-vdsl-app could depend on this. > > When we create a new package, we may also want to move other files > from the target directory to the new package? > > - lantiq.sh [3] This is sourced in 02_network files on the lantiq > targets. > - led_dsl.sh [4] > - pppoa.sh [5] > - uci-defaults [6] > - dsl_notify.sh [7] > > These are candidates that could also moved to the new package That's what I've already done in my working tree for VRX518 support on FB7530, to get this scripts also available on other targets than lantiq: https://github.com/3headeddevs/openwrt/commit/9f45c750f91eea230dc638c0936fb6e761214abb > > Best regards > > Florian > > [1] > https://github.com/openwrt/openwrt/blob/master/package/network/config/ltq-adsl-app/files/dsl_control#L11 > [2] > https://github.com/openwrt/openwrt/blob/master/package/network/config/ltq-vdsl-app/files/dsl_control#L11 > [3] > https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/lib/functions/lantiq.sh > [4] > https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/etc/hotplug.d/dsl/led_dsl.sh > [5] > https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/etc/hotplug.d/dsl/pppoa.sh > [6] > https://github.com/openwrt/openwrt/tree/master/target/linux/lantiq/base-files/etc/uci-defaults > [7] > https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/base-files/sbin/dsl_notify.sh > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff --git a/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh b/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh index 11b02fc4aa..4827d10bc5 100755 --- a/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh +++ b/target/linux/lantiq/base-files/lib/functions/lantiq_dsl.sh @@ -650,40 +650,68 @@ line_data() { line_state() { local lsg=$(dsl_cmd lsg) local ls=$(dsl_val "$lsg" nLineState); - local s; + local s n; case "$ls" in - "0x0") s="not initialized" ;; - "0x1") s="exception" ;; - "0x10") s="not updated" ;; - "0xff") s="idle request" ;; - "0x100") s="idle" ;; - "0x1ff") s="silent request" ;; - "0x200") s="silent" ;; - "0x300") s="handshake" ;; - "0x380") s="full_init" ;; - "0x400") s="discovery" ;; - "0x500") s="training" ;; - "0x600") s="analysis" ;; - "0x700") s="exchange" ;; - "0x800") s="showtime_no_sync" ;; - "0x801") s="showtime_tc_sync" ;; - "0x900") s="fastretrain" ;; - "0xa00") s="lowpower_l2" ;; - "0xb00") s="loopdiagnostic active" ;; - "0xb10") s="loopdiagnostic data exchange" ;; - "0xb20") s="loopdiagnostic data request" ;; - "0xc00") s="loopdiagnostic complete" ;; - "0x1000000") s="test" ;; - "0xd00") s="resync" ;; - "0x3c0") s="short init entry" ;; - "") s="not running daemon"; ls="0xfff" ;; - *) s="unknown" ;; + "0x0") s="not initialized" + n=1 ;; + "0x1") s="exception" + n=2 ;; + "0x10") s="not updated" + n=3 ;; + "0xff") s="idle request" + n=4 ;; + "0x100") s="idle" + n=5 ;; + "0x1ff") s="silent request" + n=6 ;; + "0x200") s="silent" + n=7 ;; + "0x300") s="handshake" + n=8 ;; + "0x380") s="full_init" + n=9 ;; + "0x400") s="discovery" + n=10 ;; + "0x500") s="training" + n=11 ;; + "0x600") s="analysis" + n=12 ;; + "0x700") s="exchange" + n=13 ;; + "0x800") s="showtime_no_sync" + n=14 ;; + "0x801") s="showtime_tc_sync" + n=15 ;; + "0x900") s="fastretrain" + n=16 ;; + "0xa00") s="lowpower_l2" + n=17 ;; + "0xb00") s="loopdiagnostic active" + n=18 ;; + "0xb10") s="loopdiagnostic data exchange" + n=19 ;; + "0xb20") s="loopdiagnostic data request" + n=20 ;; + "0xc00") s="loopdiagnostic complete" + n=21 ;; + "0x1000000") s="test" + n=22 ;; + "0xd00") s="resync" + n=23 ;; + "0x3c0") s="short init entry" + n=24 ;; + "") s="not running daemon" + ls="0xfff" + n=25 ;; + *) s="unknown" + n=26 ;; esac if [ "$action" = "lucistat" ]; then echo "dsl.line_state_num=$ls" echo "dsl.line_state_detail=\"$s\"" + echo "dsl.line_state_mapping=$n" if [ "$ls" = "0x801" ]; then echo "dsl.line_state=\"UP\"" else
The line_state of the DSL connection is described in the system via a hexadecimal variable. With this change the hexadecimal is mapped to a decimal value. With this change it is now possible to store this value in a database, so that it can be easily evaluated. This is especially relevant for the collectd and gravana backend. Signed-off-by: Florian Eckert <fe@dev.tdt.de> --- .../base-files/lib/functions/lantiq_dsl.sh | 82 +++++++++++++------ 1 file changed, 55 insertions(+), 27 deletions(-)