| Message ID | 1550756567-18227-5-git-send-email-ayal@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Ahern |
| Headers | show |
| Series | Add support for devlink health | expand |
Thu, Feb 21, 2019 at 02:42:40PM CET, ayal@mellanox.com wrote: >Add a new helper functions which outputs only values (without name >label) for different types: boolean, uint, uint64, string and binary. >In addition add a helper function which prints only the name label. > >Signed-off-by: Aya Levin <ayal@mellanox.com> >Reviewed-by: Moshe Shemesh <moshe@mellanox.com> >--- > devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 65 insertions(+), 1 deletion(-) > >diff --git a/devlink/devlink.c b/devlink/devlink.c >index b073ae020d52..5d69c4f24f29 100644 >--- a/devlink/devlink.c >+++ b/devlink/devlink.c >@@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl) > pr_out("\n"); > } > >- Please avoid changes like this. [...]
On Thu, 21 Feb 2019 15:42:40 +0200 Aya Levin <ayal@mellanox.com> wrote: > Add a new helper functions which outputs only values (without name > label) for different types: boolean, uint, uint64, string and binary. > In addition add a helper function which prints only the name label. > > Signed-off-by: Aya Levin <ayal@mellanox.com> > Reviewed-by: Moshe Shemesh <moshe@mellanox.com> > --- > devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 65 insertions(+), 1 deletion(-) > > diff --git a/devlink/devlink.c b/devlink/devlink.c > index b073ae020d52..5d69c4f24f29 100644 > --- a/devlink/devlink.c > +++ b/devlink/devlink.c > @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl) > pr_out("\n"); > } > > - > static void pr_out_str(struct dl *dl, const char *name, const char *val) > { > if (dl->json_output) { > @@ -1636,6 +1635,71 @@ 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) > +{ > + if (dl->json_output) > + jsonw_bool(dl->jw, value); > + else > + pr_out(" %s", value ? "true" : "false"); > +} > + > +static void pr_out_uint_value(struct dl *dl, unsigned int value) > +{ > + if (dl->json_output) > + jsonw_uint(dl->jw, value); > + else > + pr_out(" %u", value); > +} > + > +static void pr_out_uint64_value(struct dl *dl, uint64_t value) > +{ > + if (dl->json_output) > + jsonw_u64(dl->jw, value); > + else > + pr_out(" %lu", value); > +} > + > +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len) > +{ > + int i = 1; > + > + if (dl->json_output) > + jsonw_start_array(dl->jw); > + else > + pr_out("\n"); > + > + while (i < len) { > + if (dl->json_output) { > + jsonw_printf(dl->jw, "%d", data[i]); > + } else { > + pr_out(" %02x", data[i]); > + if (!(i % 16)) > + pr_out("\n"); > + } > + i++; > + } > + if (dl->json_output) > + jsonw_end_array(dl->jw); > + else if ((i - 1) % 16) > + pr_out("\n"); > +} > + > +static void pr_out_str_value(struct dl *dl, const char *value) > +{ > + if (dl->json_output) > + jsonw_string(dl->jw, value); > + else > + pr_out(" %s", value); > +} > + > +static void pr_out_name(struct dl *dl, const char *name) > +{ > + if (dl->json_output) > + jsonw_name(dl->jw, name); > + else > + pr_out(" %s:", name); > +} > + > static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr) > { > if (dl->json_output) { NAK Please convert devlink to use existing iproute2 json_print infrasructure rather than reinventing it.
Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote: >On Thu, 21 Feb 2019 15:42:40 +0200 >Aya Levin <ayal@mellanox.com> wrote: > >> Add a new helper functions which outputs only values (without name >> label) for different types: boolean, uint, uint64, string and binary. >> In addition add a helper function which prints only the name label. >> >> Signed-off-by: Aya Levin <ayal@mellanox.com> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com> >> --- >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 65 insertions(+), 1 deletion(-) >> >> diff --git a/devlink/devlink.c b/devlink/devlink.c >> index b073ae020d52..5d69c4f24f29 100644 >> --- a/devlink/devlink.c >> +++ b/devlink/devlink.c >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl) >> pr_out("\n"); >> } >> >> - >> static void pr_out_str(struct dl *dl, const char *name, const char *val) >> { >> if (dl->json_output) { >> @@ -1636,6 +1635,71 @@ 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) >> +{ >> + if (dl->json_output) >> + jsonw_bool(dl->jw, value); >> + else >> + pr_out(" %s", value ? "true" : "false"); >> +} >> + >> +static void pr_out_uint_value(struct dl *dl, unsigned int value) >> +{ >> + if (dl->json_output) >> + jsonw_uint(dl->jw, value); >> + else >> + pr_out(" %u", value); >> +} >> + >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value) >> +{ >> + if (dl->json_output) >> + jsonw_u64(dl->jw, value); >> + else >> + pr_out(" %lu", value); >> +} >> + >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len) >> +{ >> + int i = 1; >> + >> + if (dl->json_output) >> + jsonw_start_array(dl->jw); >> + else >> + pr_out("\n"); >> + >> + while (i < len) { >> + if (dl->json_output) { >> + jsonw_printf(dl->jw, "%d", data[i]); >> + } else { >> + pr_out(" %02x", data[i]); >> + if (!(i % 16)) >> + pr_out("\n"); >> + } >> + i++; >> + } >> + if (dl->json_output) >> + jsonw_end_array(dl->jw); >> + else if ((i - 1) % 16) >> + pr_out("\n"); >> +} >> + >> +static void pr_out_str_value(struct dl *dl, const char *value) >> +{ >> + if (dl->json_output) >> + jsonw_string(dl->jw, value); >> + else >> + pr_out(" %s", value); >> +} >> + >> +static void pr_out_name(struct dl *dl, const char *name) >> +{ >> + if (dl->json_output) >> + jsonw_name(dl->jw, name); >> + else >> + pr_out(" %s:", name); >> +} >> + >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr) >> { >> if (dl->json_output) { > >NAK > >Please convert devlink to use existing iproute2 json_print infrasructure >rather than reinventing it. That requires whole devlink/devlink.c code change. Could this be done as a follow-up?
Fri, Feb 22, 2019 at 07:01:25PM CET, stephen@networkplumber.org wrote: >On Fri, 22 Feb 2019 09:58:32 +0100 >Jiri Pirko <jiri@resnulli.us> wrote: > >> Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote: >> >On Thu, 21 Feb 2019 15:42:40 +0200 >> >Aya Levin <ayal@mellanox.com> wrote: >> > >> >> Add a new helper functions which outputs only values (without name >> >> label) for different types: boolean, uint, uint64, string and binary. >> >> In addition add a helper function which prints only the name label. >> >> >> >> Signed-off-by: Aya Levin <ayal@mellanox.com> >> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com> >> >> --- >> >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> >> 1 file changed, 65 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/devlink/devlink.c b/devlink/devlink.c >> >> index b073ae020d52..5d69c4f24f29 100644 >> >> --- a/devlink/devlink.c >> >> +++ b/devlink/devlink.c >> >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl) >> >> pr_out("\n"); >> >> } >> >> >> >> - >> >> static void pr_out_str(struct dl *dl, const char *name, const char *val) >> >> { >> >> if (dl->json_output) { >> >> @@ -1636,6 +1635,71 @@ 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) >> >> +{ >> >> + if (dl->json_output) >> >> + jsonw_bool(dl->jw, value); >> >> + else >> >> + pr_out(" %s", value ? "true" : "false"); >> >> +} >> >> + >> >> +static void pr_out_uint_value(struct dl *dl, unsigned int value) >> >> +{ >> >> + if (dl->json_output) >> >> + jsonw_uint(dl->jw, value); >> >> + else >> >> + pr_out(" %u", value); >> >> +} >> >> + >> >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value) >> >> +{ >> >> + if (dl->json_output) >> >> + jsonw_u64(dl->jw, value); >> >> + else >> >> + pr_out(" %lu", value); >> >> +} >> >> + >> >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len) >> >> +{ >> >> + int i = 1; >> >> + >> >> + if (dl->json_output) >> >> + jsonw_start_array(dl->jw); >> >> + else >> >> + pr_out("\n"); >> >> + >> >> + while (i < len) { >> >> + if (dl->json_output) { >> >> + jsonw_printf(dl->jw, "%d", data[i]); >> >> + } else { >> >> + pr_out(" %02x", data[i]); >> >> + if (!(i % 16)) >> >> + pr_out("\n"); >> >> + } >> >> + i++; >> >> + } >> >> + if (dl->json_output) >> >> + jsonw_end_array(dl->jw); >> >> + else if ((i - 1) % 16) >> >> + pr_out("\n"); >> >> +} >> >> + >> >> +static void pr_out_str_value(struct dl *dl, const char *value) >> >> +{ >> >> + if (dl->json_output) >> >> + jsonw_string(dl->jw, value); >> >> + else >> >> + pr_out(" %s", value); >> >> +} >> >> + >> >> +static void pr_out_name(struct dl *dl, const char *name) >> >> +{ >> >> + if (dl->json_output) >> >> + jsonw_name(dl->jw, name); >> >> + else >> >> + pr_out(" %s:", name); >> >> +} >> >> + >> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr) >> >> { >> >> if (dl->json_output) { >> > >> >NAK >> > >> >Please convert devlink to use existing iproute2 json_print infrasructure >> >rather than reinventing it. >> >> That requires whole devlink/devlink.c code change. Could this be done as >> a follow-up? > >There is a tradeoff, if more code creeps in it is harder to do the code change. >Can you commit to a timeframe to convert the JSON support code? Okay, I'll take care of it in March. Promise.
On Fri, 22 Feb 2019 09:58:32 +0100 Jiri Pirko <jiri@resnulli.us> wrote: > Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote: > >On Thu, 21 Feb 2019 15:42:40 +0200 > >Aya Levin <ayal@mellanox.com> wrote: > > > >> Add a new helper functions which outputs only values (without name > >> label) for different types: boolean, uint, uint64, string and binary. > >> In addition add a helper function which prints only the name label. > >> > >> Signed-off-by: Aya Levin <ayal@mellanox.com> > >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com> > >> --- > >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > >> 1 file changed, 65 insertions(+), 1 deletion(-) > >> > >> diff --git a/devlink/devlink.c b/devlink/devlink.c > >> index b073ae020d52..5d69c4f24f29 100644 > >> --- a/devlink/devlink.c > >> +++ b/devlink/devlink.c > >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl) > >> pr_out("\n"); > >> } > >> > >> - > >> static void pr_out_str(struct dl *dl, const char *name, const char *val) > >> { > >> if (dl->json_output) { > >> @@ -1636,6 +1635,71 @@ 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) > >> +{ > >> + if (dl->json_output) > >> + jsonw_bool(dl->jw, value); > >> + else > >> + pr_out(" %s", value ? "true" : "false"); > >> +} > >> + > >> +static void pr_out_uint_value(struct dl *dl, unsigned int value) > >> +{ > >> + if (dl->json_output) > >> + jsonw_uint(dl->jw, value); > >> + else > >> + pr_out(" %u", value); > >> +} > >> + > >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value) > >> +{ > >> + if (dl->json_output) > >> + jsonw_u64(dl->jw, value); > >> + else > >> + pr_out(" %lu", value); > >> +} > >> + > >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len) > >> +{ > >> + int i = 1; > >> + > >> + if (dl->json_output) > >> + jsonw_start_array(dl->jw); > >> + else > >> + pr_out("\n"); > >> + > >> + while (i < len) { > >> + if (dl->json_output) { > >> + jsonw_printf(dl->jw, "%d", data[i]); > >> + } else { > >> + pr_out(" %02x", data[i]); > >> + if (!(i % 16)) > >> + pr_out("\n"); > >> + } > >> + i++; > >> + } > >> + if (dl->json_output) > >> + jsonw_end_array(dl->jw); > >> + else if ((i - 1) % 16) > >> + pr_out("\n"); > >> +} > >> + > >> +static void pr_out_str_value(struct dl *dl, const char *value) > >> +{ > >> + if (dl->json_output) > >> + jsonw_string(dl->jw, value); > >> + else > >> + pr_out(" %s", value); > >> +} > >> + > >> +static void pr_out_name(struct dl *dl, const char *name) > >> +{ > >> + if (dl->json_output) > >> + jsonw_name(dl->jw, name); > >> + else > >> + pr_out(" %s:", name); > >> +} > >> + > >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr) > >> { > >> if (dl->json_output) { > > > >NAK > > > >Please convert devlink to use existing iproute2 json_print infrasructure > >rather than reinventing it. > > That requires whole devlink/devlink.c code change. Could this be done as > a follow-up? There is a tradeoff, if more code creeps in it is harder to do the code change. Can you commit to a timeframe to convert the JSON support code?
On 2/21/2019 4:08 PM, Jiri Pirko wrote: > Thu, Feb 21, 2019 at 02:42:40PM CET, ayal@mellanox.com wrote: >> Add a new helper functions which outputs only values (without name >> label) for different types: boolean, uint, uint64, string and binary. >> In addition add a helper function which prints only the name label. >> >> Signed-off-by: Aya Levin <ayal@mellanox.com> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com> >> --- >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 65 insertions(+), 1 deletion(-) >> >> diff --git a/devlink/devlink.c b/devlink/devlink.c >> index b073ae020d52..5d69c4f24f29 100644 >> --- a/devlink/devlink.c >> +++ b/devlink/devlink.c >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl) >> pr_out("\n"); >> } >> >> - > > Please avoid changes like this. Thanks, Will fix in next version > > > [...] >
diff --git a/devlink/devlink.c b/devlink/devlink.c index b073ae020d52..5d69c4f24f29 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl) pr_out("\n"); } - static void pr_out_str(struct dl *dl, const char *name, const char *val) { if (dl->json_output) { @@ -1636,6 +1635,71 @@ 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) +{ + if (dl->json_output) + jsonw_bool(dl->jw, value); + else + pr_out(" %s", value ? "true" : "false"); +} + +static void pr_out_uint_value(struct dl *dl, unsigned int value) +{ + if (dl->json_output) + jsonw_uint(dl->jw, value); + else + pr_out(" %u", value); +} + +static void pr_out_uint64_value(struct dl *dl, uint64_t value) +{ + if (dl->json_output) + jsonw_u64(dl->jw, value); + else + pr_out(" %lu", value); +} + +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len) +{ + int i = 1; + + if (dl->json_output) + jsonw_start_array(dl->jw); + else + pr_out("\n"); + + while (i < len) { + if (dl->json_output) { + jsonw_printf(dl->jw, "%d", data[i]); + } else { + pr_out(" %02x", data[i]); + if (!(i % 16)) + pr_out("\n"); + } + i++; + } + if (dl->json_output) + jsonw_end_array(dl->jw); + else if ((i - 1) % 16) + pr_out("\n"); +} + +static void pr_out_str_value(struct dl *dl, const char *value) +{ + if (dl->json_output) + jsonw_string(dl->jw, value); + else + pr_out(" %s", value); +} + +static void pr_out_name(struct dl *dl, const char *name) +{ + if (dl->json_output) + jsonw_name(dl->jw, name); + else + pr_out(" %s:", name); +} + static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr) { if (dl->json_output) {