| Message ID | 20210309120543.11533-1-fe@dev.tdt.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/1] ltq-vdsl-app: add line_state number and power_state number | expand |
Hi Florian, On 09/03/2021 13:05, Florian Eckert wrote: > With the old ubus dsl API, the numbers for the individual line_states and > power_states were also returned. These were not ported to the new DSL > C-API. This commit changes the following JSON output. > > * current JSON output for state: > "state": "Showtime with TC-Layer sync" > > * new JSON output for state: > "line_state": { > "number": 2049, > "string": "Showtime with TC-Layer sync" > }, this one is used by the prometheus node collector, so needs to be adapted to this change if this goes in. > > * current JSON output for power_state: > "power_state": "L0 - Synchronized" > > new JSON outpug for power_state: > "power_state": { > "number": 0, > "string": "L0 - Synchronized" > } We already talked about it, but once again for the list: I skipped both of these numeric state values when porting this to ubus/C, because those are internal and lantiq implementation specific. I'd like to avoid exposing those if possible. Looking at the various values [0], which ones are of interest here? I've only seen very few of all of those actually happening. Can we not expose new metrics instead of these internal numbers? Like there's already "up": m_bool("up", out.data.nLineState == DSL_LINESTATE_SHOWTIME_TC_SYNC); If you care e.g. about showtime you could just add: m_bool("showtime", out.data.nLineState == DSL_LINESTATE_SHOWTIME_NO_SYNC || out.data.nLineState == DSL_LINESTATE_SHOWTIME_TC_SYNC); And so on. Thanks, Andre [0] https://github.com/openwrt/openwrt/blob/master/package/network/config/ltq-vdsl-app/src/src/dsl_cpe_ubus.c#L311-L348
Hello Andre > We already talked about it, but once again for the list: Here are my thoughts on why I would like to have this for the mailinglist. > I skipped both of these numeric state values when porting this to > ubus/C, because those are internal and lantiq implementation specific. > I'd like to avoid exposing those if possible. So far we only have the lantiq DSL implementation. The only thing I can think of is that Broadcom also has DSL. But they have a strange licensing policy and are therefore negligible. I have also not yet seen a target that has this DSL frontend from broadcom. With the old implementation, we also exported the numbers via the UBUS. I already used this data for the collectd to also record the line_state and the power_state over time. There is already a pullrequest pending to integrate this into collectd [0] with some other datas > I've only seen very few of all of those actually happening. That's right, I've only seen a handful so far too But I can't tell which ones are important or negligible. > Can we not expose new metrics instead of these internal numbers? We can do that. The only question is which data we want to export. I definitely can not save strings in an RRD database. > Like there's already "up": > m_bool("up", out.data.nLineState == DSL_LINESTATE_SHOWTIME_TC_SYNC); > > If you care e.g. about showtime you could just add: > m_bool("showtime", out.data.nLineState == > DSL_LINESTATE_SHOWTIME_NO_SYNC || out.data.nLineState == > DSL_LINESTATE_SHOWTIME_TC_SYNC); That would be a possibility. The collectd db type would be a bool. From my point of view, the following values would be important. I am currently evaluating this. All other values are saved as unknown and so -1 in ma rrd database 0 -> Not initialized 1 -> Exception 256 -> Idle 512 -> Silent 768 -> Handshake 896 -> Full init 1280 -> Training 2048 -> Showtime without TC-Layer sync 2049 -> Showtime without TC-Layer sync 3328 -> Resync What is also added is the power_state. However, these are not many values. Kind regards Florian [0] https://github.com/openwrt/packages/pull/12175
diff --git a/package/network/config/ltq-vdsl-app/src/src/dsl_cpe_ubus.c b/package/network/config/ltq-vdsl-app/src/src/dsl_cpe_ubus.c index 52b2be20e1..4629620a90 100644 --- a/package/network/config/ltq-vdsl-app/src/src/dsl_cpe_ubus.c +++ b/package/network/config/ltq-vdsl-app/src/src/dsl_cpe_ubus.c @@ -306,7 +306,9 @@ static void version_information(int fd) { static void line_state(int fd) { IOCTL(DSL_LineState_t, DSL_FIO_LINE_STATE_GET) + void *c; const char *str; + switch (out.data.nLineState) { STR_CASE(DSL_LINESTATE_NOT_INITIALIZED, "Not initialized") STR_CASE(DSL_LINESTATE_EXCEPTION, "Exception") @@ -351,8 +353,13 @@ static void line_state(int fd) { str = NULL; break; }; - if (str) - m_str("state", str); + + if (str) { + c = blobmsg_open_table(&b, "line_state"); + m_u32("number", out.data.nLineState); + m_str("string", str); + blobmsg_close_table(&b, c); + } m_bool("up", out.data.nLineState == DSL_LINESTATE_SHOWTIME_TC_SYNC); } @@ -377,7 +384,9 @@ static void g997_line_inventory(int fd) { static void g997_power_management_status(int fd) { IOCTL(DSL_G997_PowerManagementStatus_t, DSL_FIO_G997_POWER_MANAGEMENT_STATUS_GET) + void *c; const char *str; + switch (out.data.nPowerManagementStatus) { STR_CASE(DSL_G997_PMS_NA, "Power management state is not available") STR_CASE(DSL_G997_PMS_L0, "L0 - Synchronized") @@ -388,8 +397,13 @@ static void g997_power_management_status(int fd) { str = NULL; break; }; - if (str) - m_str("power_state", str); + + if (str) { + c = blobmsg_open_table(&b, "power_state"); + m_u32("number", out.data.nPowerManagementStatus); + m_str("string", str); + blobmsg_close_table(&b, c); + } } static void g997_xtu_system_enabling(int fd, standard_t *standard) {
With the old ubus dsl API, the numbers for the individual line_states and power_states were also returned. These were not ported to the new DSL C-API. This commit changes the following JSON output. * current JSON output for state: "state": "Showtime with TC-Layer sync" * new JSON output for state: "line_state": { "number": 2049, "string": "Showtime with TC-Layer sync" }, * current JSON output for power_state: "power_state": "L0 - Synchronized" new JSON outpug for power_state: "power_state": { "number": 0, "string": "L0 - Synchronized" } Signed-off-by: Florian Eckert <fe@dev.tdt.de> --- v2: * Fixed typo .../ltq-vdsl-app/src/src/dsl_cpe_ubus.c | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)