diff mbox series

[iproute2-next,master,v2] bridge: fdb show: fix fdb entry state output (+ add json support)

Message ID 20200727162009.7618-1-julien@cumulusnetworks.com
State Changes Requested
Delegated to: David Ahern
Headers show
Series [iproute2-next,master,v2] bridge: fdb show: fix fdb entry state output (+ add json support) | expand

Commit Message

Julien Fortin July 27, 2020, 4:20 p.m. UTC 
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"
    }
]

This patch also fixes the non-json output, from:
    state=0x42
to:
    state 0x42

This will only be displayed if the FDB entry has an unknown value.

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(-)

Comments

Stephen Hemminger July 27, 2020, 4:30 p.m. UTC | #1 
On Mon, 27 Jul 2020 18:20:09 +0200
Julien Fortin <julien@cumulusnetworks.com> wrote:

> diff --git a/bridge/fdb.c b/bridge/fdb.c
> index d1f8afbe..765f4e51 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)

Please keep the "state=%#x" for the non JSON case.
No need to change output format.
Julien Fortin July 27, 2020, 11:25 p.m. UTC | #2 
On Mon, Jul 27, 2020 at 6:30 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 27 Jul 2020 18:20:09 +0200
> Julien Fortin <julien@cumulusnetworks.com> wrote:
>
> > diff --git a/bridge/fdb.c b/bridge/fdb.c
> > index d1f8afbe..765f4e51 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)
>
> Please keep the "state=%#x" for the non JSON case.
> No need to change output format.

My v1 patch (see below) kept the "state=" but you asked me to remove
it and re-submit.

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;
 }
David Ahern July 28, 2020, 9:08 p.m. UTC | #3 
On 7/27/20 5:25 PM, Julien Fortin wrote:
> On Mon, Jul 27, 2020 at 6:30 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>>
>> On Mon, 27 Jul 2020 18:20:09 +0200
>> Julien Fortin <julien@cumulusnetworks.com> wrote:
>>
>>> diff --git a/bridge/fdb.c b/bridge/fdb.c
>>> index d1f8afbe..765f4e51 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)
>>
>> Please keep the "state=%#x" for the non JSON case.
>> No need to change output format.
> 
> My v1 patch (see below) kept the "state=" but you asked me to remove
> it and re-submit.
> 
> 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;
>  }
> 

the v1 patch looks correct to me. Resubmit, but this should go to main
branch since it is a bug fix.
diff mbox series

Patch

diff --git a/bridge/fdb.c b/bridge/fdb.c
index d1f8afbe..765f4e51 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;
 }