Message ID | 20200710005055.8439-1-julien@cumulusnetworks.com |
---|---|
State | Changes Requested |
Delegated to: | stephen hemminger |
Headers | show |
Series | [iproute2-next,master] bridge: fdb show: fix fdb entry state output for json context | expand |
On Fri, 10 Jul 2020 02:50:55 +0200 Julien Fortin <julien@cumulusnetworks.com> wrote: > From: Julien Fortin <julien@cumulusnetworks.com> > > bridge json fdb show is printing an incorrect / non-machine readable > value, when using -j (json output) we are expecting machine readable > data that shouldn't require special handling/parsing. > > $ bridge -j fdb show | \ > python -c \ > 'import sys,json;print(json.dumps(json.loads(sys.stdin.read()),indent=4))' > [ > { > "master": "br0", > "mac": "56:23:28:4f:4f:e5", > "flags": [], > "ifname": "vx0", > "state": "state=0x80" <<<<<<<<< with the patch: "state": "0x80" > } > ] > > Fixes: c7c1a1ef51aea7c ("bridge: colorize output and use JSON print library") > Signed-off-by: Julien Fortin <julien@cumulusnetworks.com> > --- > bridge/fdb.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/bridge/fdb.c b/bridge/fdb.c > index d2247e80..198c51d1 100644 > --- a/bridge/fdb.c > +++ b/bridge/fdb.c > @@ -62,7 +62,10 @@ static const char *state_n2a(unsigned int s) > if (s & NUD_REACHABLE) > return ""; > > - sprintf(buf, "state=%#x", s); > + if (is_json_context()) > + sprintf(buf, "%#x", s); > + else > + sprintf(buf, "state=%#x", s); > return buf; > } > Printing in non JSON case was also wrong. i.e. ... state state=0x80 should be: ... state 0x80 Let's do that. The state=xxx value only shows up if the FDB entry has a value bridge command doesn't understand. The bridge command needs to be able to display the new flag values. Please fixup the two patches and resubmit to iproute2
On Mon, Jul 13, 2020 at 4:54 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Fri, 10 Jul 2020 02:50:55 +0200 > Julien Fortin <julien@cumulusnetworks.com> wrote: > > > From: Julien Fortin <julien@cumulusnetworks.com> > > > > bridge json fdb show is printing an incorrect / non-machine readable > > value, when using -j (json output) we are expecting machine readable > > data that shouldn't require special handling/parsing. > > > > $ bridge -j fdb show | \ > > python -c \ > > 'import sys,json;print(json.dumps(json.loads(sys.stdin.read()),indent=4))' > > [ > > { > > "master": "br0", > > "mac": "56:23:28:4f:4f:e5", > > "flags": [], > > "ifname": "vx0", > > "state": "state=0x80" <<<<<<<<< with the patch: "state": "0x80" > > } > > ] > > > > Fixes: c7c1a1ef51aea7c ("bridge: colorize output and use JSON print library") > > Signed-off-by: Julien Fortin <julien@cumulusnetworks.com> > > --- > > bridge/fdb.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/bridge/fdb.c b/bridge/fdb.c > > index d2247e80..198c51d1 100644 > > --- a/bridge/fdb.c > > +++ b/bridge/fdb.c > > @@ -62,7 +62,10 @@ static const char *state_n2a(unsigned int s) > > if (s & NUD_REACHABLE) > > return ""; > > > > - sprintf(buf, "state=%#x", s); > > + if (is_json_context()) > > + sprintf(buf, "%#x", s); > > + else > > + sprintf(buf, "state=%#x", s); > > return buf; > > } > > > > Printing in non JSON case was also wrong. > i.e. > ... state state=0x80 > should be: > ... state 0x80 > > Let's do that. > > > The state=xxx value only shows up if the FDB entry has a value bridge command > doesn't understand. The bridge command needs to be able to display the new flag values. > > Please fixup the two patches and resubmit to iproute2 I'll resubmit a patch for this specific issue, to handle both json and non-json case correctly but i don't think it's necessary to resubmit "bridge: fdb get: add missing json init (new_json_obj)" as it's a different issue. (re-sending without html)
On Wed, 15 Jul 2020 01:35:33 +0200 Julien Fortin <julien@cumulusnetworks.com> wrote: > On Mon, Jul 13, 2020 at 4:54 PM Stephen Hemminger < > stephen@networkplumber.org> wrote: > > > On Fri, 10 Jul 2020 02:50:55 +0200 > > Julien Fortin <julien@cumulusnetworks.com> wrote: > > > > > From: Julien Fortin <julien@cumulusnetworks.com> > > > > > > bridge json fdb show is printing an incorrect / non-machine readable > > > value, when using -j (json output) we are expecting machine readable > > > data that shouldn't require special handling/parsing. > > > > > > $ bridge -j fdb show | \ > > > python -c \ > > > 'import > > sys,json;print(json.dumps(json.loads(sys.stdin.read()),indent=4))' > > > [ > > > { > > > "master": "br0", > > > "mac": "56:23:28:4f:4f:e5", > > > "flags": [], > > > "ifname": "vx0", > > > "state": "state=0x80" <<<<<<<<< with the patch: "state": "0x80" > > > } > > > ] > > > > > > Fixes: c7c1a1ef51aea7c ("bridge: colorize output and use JSON print > > library") > > > Signed-off-by: Julien Fortin <julien@cumulusnetworks.com> > > > --- > > > bridge/fdb.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/bridge/fdb.c b/bridge/fdb.c > > > index d2247e80..198c51d1 100644 > > > --- a/bridge/fdb.c > > > +++ b/bridge/fdb.c > > > @@ -62,7 +62,10 @@ static const char *state_n2a(unsigned int s) > > > if (s & NUD_REACHABLE) > > > return ""; > > > > > > - sprintf(buf, "state=%#x", s); > > > + if (is_json_context()) > > > + sprintf(buf, "%#x", s); > > > + else > > > + sprintf(buf, "state=%#x", s); > > > return buf; > > > } > > > > > > > Printing in non JSON case was also wrong. > > i.e. > > ... state state=0x80 > > should be: > > ... state 0x80 > > > > Let's do that. > > > > > > The state=xxx value only shows up if the FDB entry has a value bridge > > command > > doesn't understand. The bridge command needs to be able to display the new > > flag values. > > > > Please fixup the two patches and resubmit to iproute2 > > > > I'll resubmit a patch for this specific issue, to handle both json and > non-json case correctly > but i don't think it's necessary to resubmit "bridge: fdb get: add missing > json init (new_json_obj)" > as it's a different issue. Normally, if you submit a patch series, then you need to resubmit the whole series. If you submit two separate patches then each one can be reviewed and accepted alone.
diff --git a/bridge/fdb.c b/bridge/fdb.c index d2247e80..198c51d1 100644 --- a/bridge/fdb.c +++ b/bridge/fdb.c @@ -62,7 +62,10 @@ static const char *state_n2a(unsigned int s) if (s & NUD_REACHABLE) return ""; - sprintf(buf, "state=%#x", s); + if (is_json_context()) + sprintf(buf, "%#x", s); + else + sprintf(buf, "state=%#x", s); return buf; }