* [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements
@ 2019-10-02 14:35 Tariq Toukan
2019-10-02 14:35 ` [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print Tariq Toukan
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Tariq Toukan @ 2019-10-02 14:35 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern
Cc: netdev, Moshe Shemesh, Aya Levin, Jiri Pirko, Tariq Toukan
Hi,
This patchset by Aya enhances FMSG output and fixes bugs in devlink health.
Patch 1 adds a helper function wrapping repeating code which determines
whether left-hand-side space separator in needed or not. It's not
needed after a newline.
Patch 2 fixes left justification of FMSG output. Prior to this patch
the FMSG output had an extra space on the left-hand-side. This patch
avoids this by looking at a flag turned on by pr_out_new_line.
Patch 3 fixes inconsistency between input and output in devlink health
show command.
Series generated against master commit:
8c2093e5d20c ip vrf: Add json support for show command
Thanks,
Tariq
V2:
- Dropped 4th patch, similar one is already accepted:
4fb98f08956f devlink: fix segfault on health command
Aya Levin (3):
devlink: Add helper for left justification print
devlink: Left justification on FMSG output
devlink: Fix inconsistency between command input and output
devlink/devlink.c | 62 +++++++++++++++++++++++------------------------
1 file changed, 31 insertions(+), 31 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print
2019-10-02 14:35 [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Tariq Toukan
@ 2019-10-02 14:35 ` Tariq Toukan
2019-10-02 14:48 ` Stephen Hemminger
2019-10-02 14:35 ` [PATCH V2 iproute2 2/3] devlink: Left justification on FMSG output Tariq Toukan
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Tariq Toukan @ 2019-10-02 14:35 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern
Cc: netdev, Moshe Shemesh, Aya Levin, Jiri Pirko, Tariq Toukan
From: Aya Levin <ayal@mellanox.com>
Introduce a helper function which wraps code that adds a left hand side
space separator unless it follows a newline.
Fixes: e3d0f0c0e3d8 ("devlink: add option to generate JSON output")
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 | 45 ++++++++++++++++++++-------------------------
1 file changed, 20 insertions(+), 25 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 5e762e37540c..d654dc7bc637 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -379,6 +379,12 @@ static bool dl_no_arg(struct dl *dl)
return dl_argc(dl) == 0;
}
+static void __pr_out_indent_newline(struct dl *dl)
+{
+ if (!g_indent_newline && !dl->json_output)
+ pr_out(" ");
+}
+
static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_BUS_NAME] = MNL_TYPE_NUL_STRING,
[DEVLINK_ATTR_DEV_NAME] = MNL_TYPE_NUL_STRING,
@@ -1828,14 +1834,11 @@ static void pr_out_port_handle_end(struct dl *dl)
static void pr_out_str(struct dl *dl, const char *name, const char *val)
{
- if (dl->json_output) {
+ __pr_out_indent_newline(dl);
+ if (dl->json_output)
jsonw_string_field(dl->jw, name, val);
- } else {
- if (g_indent_newline)
- pr_out("%s %s", name, val);
- else
- pr_out(" %s %s", name, val);
- }
+ else
+ pr_out("%s %s", name, val);
}
static void pr_out_bool(struct dl *dl, const char *name, bool val)
@@ -1848,29 +1851,23 @@ static void pr_out_bool(struct dl *dl, const char *name, bool val)
static void pr_out_uint(struct dl *dl, const char *name, unsigned int val)
{
- if (dl->json_output) {
+ __pr_out_indent_newline(dl);
+ if (dl->json_output)
jsonw_uint_field(dl->jw, name, val);
- } else {
- if (g_indent_newline)
- pr_out("%s %u", name, val);
- else
- pr_out(" %s %u", name, val);
- }
+ else
+ pr_out("%s %u", name, val);
}
static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
{
+ __pr_out_indent_newline(dl);
if (val == (uint64_t) -1)
return pr_out_str(dl, name, "unlimited");
- if (dl->json_output) {
+ if (dl->json_output)
jsonw_u64_field(dl->jw, name, val);
- } else {
- if (g_indent_newline)
- pr_out("%s %"PRIu64, name, val);
- else
- pr_out(" %s %"PRIu64, name, val);
- }
+ else
+ pr_out("%s %"PRIu64, name, val);
}
static void pr_out_bool_value(struct dl *dl, bool value)
@@ -6118,14 +6115,12 @@ static void pr_out_region_handle_end(struct dl *dl)
static void pr_out_region_snapshots_start(struct dl *dl, bool array)
{
+ __pr_out_indent_newline(dl);
if (dl->json_output) {
jsonw_name(dl->jw, "snapshot");
jsonw_start_array(dl->jw);
} else {
- if (g_indent_newline)
- pr_out("snapshot %s", array ? "[" : "");
- else
- pr_out(" snapshot %s", array ? "[" : "");
+ pr_out("snapshot %s", array ? "[" : "");
}
}
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V2 iproute2 2/3] devlink: Left justification on FMSG output
2019-10-02 14:35 [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Tariq Toukan
2019-10-02 14:35 ` [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print Tariq Toukan
@ 2019-10-02 14:35 ` Tariq Toukan
2019-10-02 14:35 ` [PATCH V2 iproute2 3/3] devlink: Fix inconsistency between command input and output Tariq Toukan
2019-10-09 3:23 ` [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Stephen Hemminger
3 siblings, 0 replies; 7+ messages in thread
From: Tariq Toukan @ 2019-10-02 14:35 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern
Cc: netdev, Moshe Shemesh, Aya Levin, Jiri Pirko, Tariq Toukan
From: Aya Levin <ayal@mellanox.com>
FMSG output is dynamic, space separator must be on the left hand side of
the value. Otherwise output has redundant left indentation regardless
the hierarchy.
Before the 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
channel ix: 2 tc: 0 txq ix: 2 sqn: 18 HW state: 1 stopped: false cc: 5 pc: 5 CQ: cqn: 14 HW status: 0
channel ix: 3 tc: 0 txq ix: 3 sqn: 22 HW state: 1 stopped: false cc: 0 pc: 0 CQ: cqn: 18 HW status: 0
With the 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
channel ix: 2 tc: 0 txq ix: 2 sqn: 18 HW state: 1 stopped: false cc: 5 pc: 5 CQ: cqn: 14 HW status: 0
channel ix: 3 tc: 0 txq ix: 3 sqn: 22 HW state: 1 stopped: false cc: 0 pc: 0 CQ: cqn: 18 HW status: 0
Fixes: 844a61764c6f ("devlink: Add helper functions for name and value separately")
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 | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index d654dc7bc637..50baf82af937 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1872,26 +1872,29 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
static void pr_out_bool_value(struct dl *dl, bool value)
{
+ __pr_out_indent_newline(dl);
if (dl->json_output)
jsonw_bool(dl->jw, value);
else
- pr_out(" %s", value ? "true" : "false");
+ pr_out("%s", value ? "true" : "false");
}
static void pr_out_uint_value(struct dl *dl, unsigned int value)
{
+ __pr_out_indent_newline(dl);
if (dl->json_output)
jsonw_uint(dl->jw, value);
else
- pr_out(" %u", value);
+ pr_out("%u", value);
}
static void pr_out_uint64_value(struct dl *dl, uint64_t value)
{
+ __pr_out_indent_newline(dl);
if (dl->json_output)
jsonw_u64(dl->jw, value);
else
- pr_out(" %"PRIu64, value);
+ pr_out("%"PRIu64, value);
}
static bool is_binary_eol(int i)
@@ -1918,18 +1921,20 @@ static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
static void pr_out_str_value(struct dl *dl, const char *value)
{
+ __pr_out_indent_newline(dl);
if (dl->json_output)
jsonw_string(dl->jw, value);
else
- pr_out(" %s", value);
+ pr_out("%s", value);
}
static void pr_out_name(struct dl *dl, const char *name)
{
+ __pr_out_indent_newline(dl);
if (dl->json_output)
jsonw_name(dl->jw, name);
else
- pr_out(" %s:", name);
+ pr_out("%s:", name);
}
static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V2 iproute2 3/3] devlink: Fix inconsistency between command input and output
2019-10-02 14:35 [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Tariq Toukan
2019-10-02 14:35 ` [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print Tariq Toukan
2019-10-02 14:35 ` [PATCH V2 iproute2 2/3] devlink: Left justification on FMSG output Tariq Toukan
@ 2019-10-02 14:35 ` Tariq Toukan
2019-10-09 3:23 ` [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Stephen Hemminger
3 siblings, 0 replies; 7+ messages in thread
From: Tariq Toukan @ 2019-10-02 14:35 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern
Cc: netdev, Moshe Shemesh, Aya Levin, Jiri Pirko, Tariq Toukan
From: Aya Levin <ayal@mellanox.com>
In devlink health show command the reporter's name parameter is called
reporter, but in the output the reporter's name is referred to as name
Before this patch:
$ devlink health show pci/0000:04:00.0 reporter tx
pci/0000:04:00.0:
name tx
state healthy error 0 recover 0 grace_period 500 auto_recover true
After this patch:
$ devlink health show pci/0000:04:00.0 reporter tx
pci/0000:04:00.0:
reporter tx
state healthy error 0 recover 0 grace_period 500 auto_recover true
Reported-by: Jiri Pirko <jiri@mellanox.com>
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 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 50baf82af937..5bbe0bddd910 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -6660,7 +6660,7 @@ static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
pr_out_handle_start_arr(dl, tb_health);
- pr_out_str(dl, "name",
+ pr_out_str(dl, "reporter",
mnl_attr_get_str(tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME]));
if (!dl->json_output) {
__pr_out_newline();
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print
2019-10-02 14:35 ` [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print Tariq Toukan
@ 2019-10-02 14:48 ` Stephen Hemminger
2019-10-02 17:14 ` Jiri Pirko
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2019-10-02 14:48 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Ahern, netdev, Moshe Shemesh, Aya Levin, Jiri Pirko
On Wed, 2 Oct 2019 17:35:14 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> static void pr_out_str(struct dl *dl, const char *name, const char *val)
> {
> - if (dl->json_output) {
> + __pr_out_indent_newline(dl);
> + if (dl->json_output)
> jsonw_string_field(dl->jw, name, val);
> - } else {
> - if (g_indent_newline)
> - pr_out("%s %s", name, val);
> - else
> - pr_out(" %s %s", name, val);
> - }
> + else
> + pr_out("%s %s", name, val)
Overall this looks like an improvement.
Why doesn't devlink already use existing json_print infrastructure?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print
2019-10-02 14:48 ` Stephen Hemminger
@ 2019-10-02 17:14 ` Jiri Pirko
0 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2019-10-02 17:14 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Tariq Toukan, David Ahern, netdev, Moshe Shemesh, Aya Levin, Jiri Pirko
Wed, Oct 02, 2019 at 04:48:04PM CEST, stephen@networkplumber.org wrote:
>On Wed, 2 Oct 2019 17:35:14 +0300
>Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> static void pr_out_str(struct dl *dl, const char *name, const char *val)
>> {
>> - if (dl->json_output) {
>> + __pr_out_indent_newline(dl);
>> + if (dl->json_output)
>> jsonw_string_field(dl->jw, name, val);
>> - } else {
>> - if (g_indent_newline)
>> - pr_out("%s %s", name, val);
>> - else
>> - pr_out(" %s %s", name, val);
>> - }
>> + else
>> + pr_out("%s %s", name, val)
>
>Overall this looks like an improvement.
>
>Why doesn't devlink already use existing json_print infrastructure?
It will happen soon hopefully. I have it on the todo list and hopefully
also a person to do it :)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements
2019-10-02 14:35 [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Tariq Toukan
` (2 preceding siblings ...)
2019-10-02 14:35 ` [PATCH V2 iproute2 3/3] devlink: Fix inconsistency between command input and output Tariq Toukan
@ 2019-10-09 3:23 ` Stephen Hemminger
3 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2019-10-09 3:23 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Ahern, netdev, Moshe Shemesh, Aya Levin, Jiri Pirko
On Wed, 2 Oct 2019 17:35:13 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> Hi,
>
> This patchset by Aya enhances FMSG output and fixes bugs in devlink health.
>
> Patch 1 adds a helper function wrapping repeating code which determines
> whether left-hand-side space separator in needed or not. It's not
> needed after a newline.
> Patch 2 fixes left justification of FMSG output. Prior to this patch
> the FMSG output had an extra space on the left-hand-side. This patch
> avoids this by looking at a flag turned on by pr_out_new_line.
> Patch 3 fixes inconsistency between input and output in devlink health
> show command.
>
> Series generated against master commit:
> 8c2093e5d20c ip vrf: Add json support for show command
>
> Thanks,
> Tariq
>
> V2:
> - Dropped 4th patch, similar one is already accepted:
> 4fb98f08956f devlink: fix segfault on health command
>
> Aya Levin (3):
> devlink: Add helper for left justification print
> devlink: Left justification on FMSG output
> devlink: Fix inconsistency between command input and output
>
> devlink/devlink.c | 62 +++++++++++++++++++++++------------------------
> 1 file changed, 31 insertions(+), 31 deletions(-)
>
Series applied.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-09 3:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 14:35 [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Tariq Toukan
2019-10-02 14:35 ` [PATCH V2 iproute2 1/3] devlink: Add helper for left justification print Tariq Toukan
2019-10-02 14:48 ` Stephen Hemminger
2019-10-02 17:14 ` Jiri Pirko
2019-10-02 14:35 ` [PATCH V2 iproute2 2/3] devlink: Left justification on FMSG output Tariq Toukan
2019-10-02 14:35 ` [PATCH V2 iproute2 3/3] devlink: Fix inconsistency between command input and output Tariq Toukan
2019-10-09 3:23 ` [PATCH V2 iproute2 0/3] Devlink health FMSG fixes and enhancements Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).