* [PATCH iproute2 1/3] devlink: Print health reporter's dump time-stamp in a helper function
2019-12-11 15:45 [PATCH iproute2 0/3] Devlink health reporter's issues Tariq Toukan
@ 2019-12-11 15:45 ` Tariq Toukan
2019-12-11 15:45 ` [PATCH iproute2 2/3] devlink: Add a new time-stamp format for health reporter's dump Tariq Toukan
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Tariq Toukan @ 2019-12-11 15:45 UTC (permalink / raw)
To: David Ahern, Stephen Hemminger; +Cc: netdev, ayal, moshe, jiri, Tariq Toukan
From: Aya Levin <ayal@mellanox.com>
Add pr_out_dump_reporter prefix to the helper function's name and
encapsulate the print in it.
Fixes: 2f1242efe9d0 ("devlink: Add devlink health show command")
Signed-off-by: Aya Levin <ayal@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
devlink/devlink.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 0b8985f32636..aee6c87cbce7 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -6654,8 +6654,11 @@ static const char *health_state_name(uint8_t state)
}
}
-static void format_logtime(uint64_t time_ms, char *ts_date, char *ts_time)
+static void pr_out_dump_reporter_format_logtime(struct dl *dl, const struct nlattr *attr)
{
+ char dump_date[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+ char dump_time[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+ uint64_t time_ms = mnl_attr_get_u64(attr);
struct sysinfo s_info;
struct tm *info;
time_t now, sec;
@@ -6673,16 +6676,16 @@ static void format_logtime(uint64_t time_ms, char *ts_date, char *ts_time)
sec = now - s_info.uptime + time_ms / 1000;
info = localtime(&sec);
out:
- strftime(ts_date, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%Y-%m-%d", info);
- strftime(ts_time, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%H:%M:%S", info);
+ strftime(dump_date, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%Y-%m-%d", info);
+ strftime(dump_time, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%H:%M:%S", info);
+ pr_out_str(dl, "last_dump_date", dump_date);
+ pr_out_str(dl, "last_dump_time", dump_time);
}
static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
{
struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
enum devlink_health_reporter_state state;
- const struct nlattr *attr;
- uint64_t time_ms;
int err;
err = mnl_attr_parse_nested(tb_health[DEVLINK_ATTR_HEALTH_REPORTER],
@@ -6710,17 +6713,8 @@ static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT]));
pr_out_u64(dl, "recover",
mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT]));
- if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]) {
- char dump_date[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
- char dump_time[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
-
- attr = tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS];
- time_ms = mnl_attr_get_u64(attr);
- format_logtime(time_ms, dump_date, dump_time);
-
- pr_out_str(dl, "last_dump_date", dump_date);
- pr_out_str(dl, "last_dump_time", dump_time);
- }
+ if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS])
+ pr_out_dump_reporter_format_logtime(dl, tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]);
if (tb[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD])
pr_out_u64(dl, "grace_period",
mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD]));
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH iproute2 2/3] devlink: Add a new time-stamp format for health reporter's dump
2019-12-11 15:45 [PATCH iproute2 0/3] Devlink health reporter's issues Tariq Toukan
2019-12-11 15:45 ` [PATCH iproute2 1/3] devlink: Print health reporter's dump time-stamp in a helper function Tariq Toukan
@ 2019-12-11 15:45 ` Tariq Toukan
2019-12-11 15:45 ` [PATCH iproute2 3/3] devlink: Fix fmsg nesting in non JSON output Tariq Toukan
2019-12-17 4:53 ` [PATCH iproute2 0/3] Devlink health reporter's issues Stephen Hemminger
3 siblings, 0 replies; 8+ messages in thread
From: Tariq Toukan @ 2019-12-11 15:45 UTC (permalink / raw)
To: David Ahern, Stephen Hemminger; +Cc: netdev, ayal, moshe, jiri, Tariq Toukan
From: Aya Levin <ayal@mellanox.com>
Introduce a new attribute representing a new time-stamp format: current
time in ns (to comply with y2038) instead of jiffies. If the new
attribute was received, translate the time-stamp accordingly (ns).
Fixes: 2f1242efe9d0 ("devlink: Add devlink health show command")
Signed-off-by: Aya Levin <ayal@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
devlink/devlink.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index aee6c87cbce7..f0181e41faa4 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -6682,6 +6682,25 @@ out:
pr_out_str(dl, "last_dump_time", dump_time);
}
+static void pr_out_dump_report_timestamp(struct dl *dl, const struct nlattr *attr)
+{
+ char dump_date[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+ char dump_time[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+ time_t tv_sec;
+ struct tm *tm;
+ uint64_t ts;
+
+ ts = mnl_attr_get_u64(attr);
+ tv_sec = ts / 1000000000;
+ tm = localtime(&tv_sec);
+
+ strftime(dump_date, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%Y-%m-%d", tm);
+ strftime(dump_time, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%H:%M:%S", tm);
+
+ pr_out_str(dl, "last_dump_date", dump_date);
+ pr_out_str(dl, "last_dump_time", dump_time);
+}
+
static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
{
struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
@@ -6713,7 +6732,9 @@ static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT]));
pr_out_u64(dl, "recover",
mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT]));
- if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS])
+ if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS_NS])
+ pr_out_dump_report_timestamp(dl, tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS_NS]);
+ else if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS])
pr_out_dump_reporter_format_logtime(dl, tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]);
if (tb[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD])
pr_out_u64(dl, "grace_period",
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH iproute2 3/3] devlink: Fix fmsg nesting in non JSON output
2019-12-11 15:45 [PATCH iproute2 0/3] Devlink health reporter's issues Tariq Toukan
2019-12-11 15:45 ` [PATCH iproute2 1/3] devlink: Print health reporter's dump time-stamp in a helper function Tariq Toukan
2019-12-11 15:45 ` [PATCH iproute2 2/3] devlink: Add a new time-stamp format for health reporter's dump Tariq Toukan
@ 2019-12-11 15:45 ` Tariq Toukan
2019-12-17 4:53 ` [PATCH iproute2 0/3] Devlink health reporter's issues Stephen Hemminger
3 siblings, 0 replies; 8+ messages in thread
From: Tariq Toukan @ 2019-12-11 15:45 UTC (permalink / raw)
To: David Ahern, Stephen Hemminger; +Cc: netdev, ayal, moshe, jiri, Tariq Toukan
From: Aya Levin <ayal@mellanox.com>
When an object or an array opening follows a name (label), add a new
line and indentation before printing the label. When name (label) is
followed by a value, print both at the same line.
Prior to this patch nesting was not visible in a non JSON output:
JSON:
{
"Common config": {
"SQ": {
"stride size": 64,
"size": 1024
},
"CQ": {
"stride size": 64,
"size": 1024
} },
"SQs": [ {
"channel ix": 0,
"sqn": 10,
"HW state": 1,
"stopped": false,
"cc": 0,
"pc": 0,
"CQ": {
"cqn": 6,
"HW status": 0
}
},{
"channel ix": 0,
"sqn": 14,
"HW state": 1,
"stopped": false,
"cc": 0,
"pc": 0,
"CQ": {
"cqn": 10,
"HW status": 0
}
} ]
}
Before this patch:
Common Config: SQ: stride size: 64 size: 1024
CQ: stride size: 64 size: 1024
SQs:
channel ix: 0 tc: 0 txq ix: 0 sqn: 10 HW state: 1 stopped: false cc: 0 pc: 0 CQ: cqn: 6 HW status: 0
channel ix: 1 tc: 0 txq ix: 1 sqn: 14 HW state: 1 stopped: false cc: 0 pc: 0 CQ: cqn: 10 HW status: 0
With this patch:
Common config:
SQ:
stride size: 64 size: 1024
CQ:
stride size: 64 size: 1024
SQs:
channel ix: 0 sqn: 10 HW state: 1 stopped: false cc: 0 pc: 0
CQ:
cqn: 6 HW status: 0
channel ix: 1 sqn: 14 HW state: 1 stopped: false cc: 0 pc: 0
CQ:
cqn: 10 HW status: 0
Fixes: 7b8baf834d5e ("devlink: Add devlink health diagnose command")
Signed-off-by: Aya Levin <ayal@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
devlink/devlink.c | 106 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 88 insertions(+), 18 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index f0181e41faa4..95f05a0b5223 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -6463,12 +6463,23 @@ static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
return MNL_CB_OK;
}
+static void pr_out_fmsg_name(struct dl *dl, char **name)
+{
+ if (!*name)
+ return;
+
+ pr_out_name(dl, *name);
+ free(*name);
+ *name = NULL;
+}
+
struct nest_entry {
int attr_type;
struct list_head list;
};
struct fmsg_cb_data {
+ char *name;
struct dl *dl;
uint8_t value_type;
struct list_head entry_list;
@@ -6498,6 +6509,56 @@ static int cmd_fmsg_nest_queue(struct fmsg_cb_data *fmsg_data,
return MNL_CB_OK;
}
+static void pr_out_fmsg_group_start(struct dl *dl, char **name)
+{
+ __pr_out_newline();
+ pr_out_fmsg_name(dl, name);
+ __pr_out_newline();
+ __pr_out_indent_inc();
+}
+
+static void pr_out_fmsg_group_end(struct dl *dl)
+{
+ __pr_out_newline();
+ __pr_out_indent_dec();
+}
+
+static void pr_out_fmsg_start_object(struct dl *dl, char **name)
+{
+ if (dl->json_output) {
+ pr_out_fmsg_name(dl, name);
+ jsonw_start_object(dl->jw);
+ } else {
+ pr_out_fmsg_group_start(dl, name);
+ }
+}
+
+static void pr_out_fmsg_end_object(struct dl *dl)
+{
+ if (dl->json_output)
+ jsonw_end_object(dl->jw);
+ else
+ pr_out_fmsg_group_end(dl);
+}
+
+static void pr_out_fmsg_start_array(struct dl *dl, char **name)
+{
+ if (dl->json_output) {
+ pr_out_fmsg_name(dl, name);
+ jsonw_start_array(dl->jw);
+ } else {
+ pr_out_fmsg_group_start(dl, name);
+ }
+}
+
+static void pr_out_fmsg_end_array(struct dl *dl)
+{
+ if (dl->json_output)
+ jsonw_end_array(dl->jw);
+ else
+ pr_out_fmsg_group_end(dl);
+}
+
static int cmd_fmsg_nest(struct fmsg_cb_data *fmsg_data, uint8_t nest_value,
bool start)
{
@@ -6512,26 +6573,17 @@ static int cmd_fmsg_nest(struct fmsg_cb_data *fmsg_data, uint8_t nest_value,
switch (value) {
case DEVLINK_ATTR_FMSG_OBJ_NEST_START:
if (start)
- pr_out_entry_start(dl);
+ pr_out_fmsg_start_object(dl, &fmsg_data->name);
else
- pr_out_entry_end(dl);
+ pr_out_fmsg_end_object(dl);
break;
case DEVLINK_ATTR_FMSG_PAIR_NEST_START:
break;
case DEVLINK_ATTR_FMSG_ARR_NEST_START:
- if (dl->json_output) {
- if (start)
- jsonw_start_array(dl->jw);
- else
- jsonw_end_array(dl->jw);
- } else {
- if (start) {
- __pr_out_newline();
- __pr_out_indent_inc();
- } else {
- __pr_out_indent_dec();
- }
- }
+ if (start)
+ pr_out_fmsg_start_array(dl, &fmsg_data->name);
+ else
+ pr_out_fmsg_end_array(dl);
break;
default:
return -EINVAL;
@@ -6569,12 +6621,16 @@ static int cmd_fmsg_object_cb(const struct nlmsghdr *nlh, void *data)
return err;
break;
case DEVLINK_ATTR_FMSG_OBJ_NAME:
- pr_out_name(dl, mnl_attr_get_str(nla_object));
+ free(fmsg_data->name);
+ fmsg_data->name = strdup(mnl_attr_get_str(nla_object));
+ if (!fmsg_data->name)
+ return -ENOMEM;
break;
case DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE:
fmsg_data->value_type = mnl_attr_get_u8(nla_object);
break;
case DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA:
+ pr_out_fmsg_name(dl, &fmsg_data->name);
err = fmsg_value_show(dl, fmsg_data->value_type,
nla_object);
if (err != MNL_CB_OK)
@@ -6587,6 +6643,20 @@ static int cmd_fmsg_object_cb(const struct nlmsghdr *nlh, void *data)
return MNL_CB_OK;
}
+static void cmd_fmsg_init(struct dl *dl, struct fmsg_cb_data *data)
+{
+ /* FMSG is dynamic: opening of an object or array causes a
+ * newline. JSON starts with an { or [, but plain text should
+ * not start with a new line. Ensure this by setting
+ * g_new_line_count to 1: avoiding newline before the first
+ * print.
+ */
+ g_new_line_count = 1;
+ data->name = NULL;
+ data->dl = dl;
+ INIT_LIST_HEAD(&data->entry_list);
+}
+
static int cmd_health_object_common(struct dl *dl, uint8_t cmd, uint16_t flags)
{
struct fmsg_cb_data data;
@@ -6600,9 +6670,9 @@ static int cmd_health_object_common(struct dl *dl, uint8_t cmd, uint16_t flags)
if (err)
return err;
- data.dl = dl;
- INIT_LIST_HEAD(&data.entry_list);
+ cmd_fmsg_init(dl, &data);
err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_fmsg_object_cb, &data);
+ free(data.name);
return err;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH iproute2 0/3] Devlink health reporter's issues
2019-12-11 15:45 [PATCH iproute2 0/3] Devlink health reporter's issues Tariq Toukan
` (2 preceding siblings ...)
2019-12-11 15:45 ` [PATCH iproute2 3/3] devlink: Fix fmsg nesting in non JSON output Tariq Toukan
@ 2019-12-17 4:53 ` Stephen Hemminger
2019-12-17 15:08 ` David Ahern
3 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2019-12-17 4:53 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Ahern, netdev, ayal, moshe, jiri
On Wed, 11 Dec 2019 17:45:33 +0200
Tariq Toukan <tariqt@mellanox.com> wrote:
> Hi,
>
> This patchset by Aya fixes two issues: wrong time-stamp on a dump in
> devlink health reporter and messy display of non JSON output in devlink
> health diagnostics and dump.
>
> 1) Wrong time-stamp on a dump in devlink health reporter:
> This bug fix consist of 2 patches. First patch refactors the current
> implementation of helper function which displays the reporter's dump
> time-stamp and add the actual print to the function's body.
> The second patch introduces a new attribute which is the time-stamp in
> current time in nsec instead of jiffies. When the new attribute is
> present try and translate the time-stamp according to 'current time'.
>
> 2) Messy display of non-JSON output in devlink health diagnostics and dump:
> This patch mainly deals with dynamic object and array opening. The
> label is stored and only when the proceeding value arrives the name is
> printed with regards to the context.
>
> Series generated against the shared master/net-next head:
> 24bee3bf9752 tipc: add new commands to set TIPC AEAD key
>
> Regards,
> Tariq
>
> Aya Levin (3):
> devlink: Print health reporter's dump time-stamp in a helper function
> devlink: Add a new time-stamp format for health reporter's dump
> devlink: Fix fmsg nesting in non JSON output
>
> devlink/devlink.c | 153 +++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 119 insertions(+), 34 deletions(-)
>
Applied, devlink really needs to get converted to same json
library as rest of iproute2.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH iproute2 0/3] Devlink health reporter's issues
2019-12-17 4:53 ` [PATCH iproute2 0/3] Devlink health reporter's issues Stephen Hemminger
@ 2019-12-17 15:08 ` David Ahern
2019-12-17 16:33 ` Stephen Hemminger
2019-12-18 14:53 ` Jiri Pirko
0 siblings, 2 replies; 8+ messages in thread
From: David Ahern @ 2019-12-17 15:08 UTC (permalink / raw)
To: Stephen Hemminger, Tariq Toukan; +Cc: netdev, ayal, moshe, jiri
On 12/16/19 9:53 PM, Stephen Hemminger wrote:
> Applied, devlink really needs to get converted to same json
> library as rest of iproute2.
We have been saying that for too many releases. rdma has fixed its json
support; devlink needs to the same.
Maybe it is time to stop taking patches.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2 0/3] Devlink health reporter's issues
2019-12-17 15:08 ` David Ahern
@ 2019-12-17 16:33 ` Stephen Hemminger
2019-12-18 14:53 ` Jiri Pirko
1 sibling, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2019-12-17 16:33 UTC (permalink / raw)
To: David Ahern; +Cc: Tariq Toukan, netdev, ayal, moshe, jiri
On Tue, 17 Dec 2019 08:08:31 -0700
David Ahern <dsahern@gmail.com> wrote:
> On 12/16/19 9:53 PM, Stephen Hemminger wrote:
> > Applied, devlink really needs to get converted to same json
> > library as rest of iproute2.
>
> We have been saying that for too many releases. rdma has fixed its json
> support; devlink needs to the same.
>
> Maybe it is time to stop taking patches.
Or add nagware patch :-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2 0/3] Devlink health reporter's issues
2019-12-17 15:08 ` David Ahern
2019-12-17 16:33 ` Stephen Hemminger
@ 2019-12-18 14:53 ` Jiri Pirko
1 sibling, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2019-12-18 14:53 UTC (permalink / raw)
To: David Ahern; +Cc: Stephen Hemminger, Tariq Toukan, netdev, ayal, moshe, jiri
Tue, Dec 17, 2019 at 04:08:31PM CET, dsahern@gmail.com wrote:
>On 12/16/19 9:53 PM, Stephen Hemminger wrote:
>> Applied, devlink really needs to get converted to same json
>> library as rest of iproute2.
>
>We have been saying that for too many releases. rdma has fixed its json
>support; devlink needs to the same.
It's under work now, patches are going to be sent soon, hopefully.
>
>Maybe it is time to stop taking patches.
^ permalink raw reply [flat|nested] 8+ messages in thread