All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2] tc: flower: fix json output with mpls lse
@ 2020-12-18 22:25 Guillaume Nault
  2021-01-07 16:48 ` Guillaume Nault
  0 siblings, 1 reply; 9+ messages in thread
From: Guillaume Nault @ 2020-12-18 22:25 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

The json output of the TCA_FLOWER_KEY_MPLS_OPTS attribute was invalid.

Example:

  $ tc filter add dev eth0 ingress protocol mpls_uc flower mpls \
      lse depth 1 label 100                                     \
      lse depth 2 label 200

  $ tc -json filter show dev eth0 ingress
    ...{"eth_type":"8847",
        "  mpls":["    lse":["depth":1,"label":100],
                  "    lse":["depth":2,"label":200]]}...

This is invalid as the arrays, introduced by "[", can't contain raw
string:value pairs. Those must be enclosed into "{}" to form valid json
ojects. Also, there are spurious whitespaces before the mpls and lse
strings because of the indentation used for normal output.

Fix this by putting all LSE parameters (depth, label, tc, bos and ttl)
into the same json object. The "mpls" key now directly contains a list
of such objects.

Also, handle strings differently for normal and json output, so that
json strings don't get spurious indentation whitespaces.

Normal output isn't modified.
The json output now looks like:

  $ tc -json filter show dev eth0 ingress
    ...{"eth_type":"8847",
        "mpls":[{"depth":1,"label":100},
                {"depth":2,"label":200}]}...

Fixes: eb09a15c12fb ("tc: flower: support multiple MPLS LSE match")
Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 tc/f_flower.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 00c919fd..27731078 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -2476,7 +2476,7 @@ static void flower_print_u32(const char *name, struct rtattr *attr)
 	print_uint(PRINT_ANY, name, namefrm, rta_getattr_u32(attr));
 }
 
-static void flower_print_mpls_opt_lse(const char *name, struct rtattr *lse)
+static void flower_print_mpls_opt_lse(struct rtattr *lse)
 {
 	struct rtattr *tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1];
 	struct rtattr *attr;
@@ -2493,7 +2493,8 @@ static void flower_print_mpls_opt_lse(const char *name, struct rtattr *lse)
 		     RTA_PAYLOAD(lse));
 
 	print_nl();
-	open_json_array(PRINT_ANY, name);
+	print_string(PRINT_FP, NULL, "    lse", NULL);
+	open_json_object(NULL);
 	attr = tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH];
 	if (attr)
 		print_hhu(PRINT_ANY, "depth", " depth %u",
@@ -2511,10 +2512,10 @@ static void flower_print_mpls_opt_lse(const char *name, struct rtattr *lse)
 	attr = tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL];
 	if (attr)
 		print_hhu(PRINT_ANY, "ttl", " ttl %u", rta_getattr_u8(attr));
-	close_json_array(PRINT_JSON, NULL);
+	close_json_object();
 }
 
-static void flower_print_mpls_opts(const char *name, struct rtattr *attr)
+static void flower_print_mpls_opts(struct rtattr *attr)
 {
 	struct rtattr *lse;
 	int rem;
@@ -2523,11 +2524,12 @@ static void flower_print_mpls_opts(const char *name, struct rtattr *attr)
 		return;
 
 	print_nl();
-	open_json_array(PRINT_ANY, name);
+	print_string(PRINT_FP, NULL, "  mpls", NULL);
+	open_json_array(PRINT_JSON, "mpls");
 	rem = RTA_PAYLOAD(attr);
 	lse = RTA_DATA(attr);
 	while (RTA_OK(lse, rem)) {
-		flower_print_mpls_opt_lse("    lse", lse);
+		flower_print_mpls_opt_lse(lse);
 		lse = RTA_NEXT(lse, rem);
 	};
 	if (rem)
@@ -2650,7 +2652,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
 	flower_print_ip_attr("ip_ttl", tb[TCA_FLOWER_KEY_IP_TTL],
 			    tb[TCA_FLOWER_KEY_IP_TTL_MASK]);
 
-	flower_print_mpls_opts("  mpls", tb[TCA_FLOWER_KEY_MPLS_OPTS]);
+	flower_print_mpls_opts(tb[TCA_FLOWER_KEY_MPLS_OPTS]);
 	flower_print_u32("mpls_label", tb[TCA_FLOWER_KEY_MPLS_LABEL]);
 	flower_print_u8("mpls_tc", tb[TCA_FLOWER_KEY_MPLS_TC]);
 	flower_print_u8("mpls_bos", tb[TCA_FLOWER_KEY_MPLS_BOS]);
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2020-12-18 22:25 [PATCH iproute2] tc: flower: fix json output with mpls lse Guillaume Nault
@ 2021-01-07 16:48 ` Guillaume Nault
  2021-01-07 17:13   ` Jakub Kicinski
  0 siblings, 1 reply; 9+ messages in thread
From: Guillaume Nault @ 2021-01-07 16:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

On Fri, Dec 18, 2020 at 11:25:32PM +0100, Guillaume Nault wrote:
> The json output of the TCA_FLOWER_KEY_MPLS_OPTS attribute was invalid.
> 
> Example:
> 
>   $ tc filter add dev eth0 ingress protocol mpls_uc flower mpls \
>       lse depth 1 label 100                                     \
>       lse depth 2 label 200
> 
>   $ tc -json filter show dev eth0 ingress
>     ...{"eth_type":"8847",
>         "  mpls":["    lse":["depth":1,"label":100],
>                   "    lse":["depth":2,"label":200]]}...

Is there any problem with this patch?
It's archived in patchwork, but still in state "new". Therefore I guess
it was dropped before being considered for review.

This problem precludes the implementation of a kernel selftest for
TCA_FLOWER_KEY_MPLS_OPTS.

Just let me know if I should respin.

Thanks,

William


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2021-01-07 16:48 ` Guillaume Nault
@ 2021-01-07 17:13   ` Jakub Kicinski
  2021-01-07 17:39     ` David Ahern
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2021-01-07 17:13 UTC (permalink / raw)
  To: Guillaume Nault; +Cc: Stephen Hemminger, netdev, David Ahern

On Thu, 7 Jan 2021 17:48:56 +0100 Guillaume Nault wrote:
> On Fri, Dec 18, 2020 at 11:25:32PM +0100, Guillaume Nault wrote:
> > The json output of the TCA_FLOWER_KEY_MPLS_OPTS attribute was invalid.
> > 
> > Example:
> > 
> >   $ tc filter add dev eth0 ingress protocol mpls_uc flower mpls \
> >       lse depth 1 label 100                                     \
> >       lse depth 2 label 200
> > 
> >   $ tc -json filter show dev eth0 ingress
> >     ...{"eth_type":"8847",
> >         "  mpls":["    lse":["depth":1,"label":100],
> >                   "    lse":["depth":2,"label":200]]}...  
> 
> Is there any problem with this patch?
> It's archived in patchwork, but still in state "new". Therefore I guess
> it was dropped before being considered for review.

Erm, that's weird. I think Alexei mentioned that auto-archiving is
turned on in the new netdevbpf patchwork instance. My guess is it got
auto archived :S

Here is the list of all patches that are Archived as New:

https://patchwork.kernel.org/project/netdevbpf/list/?state=1&archive=true

Should any of these have been reviewed?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2021-01-07 17:13   ` Jakub Kicinski
@ 2021-01-07 17:39     ` David Ahern
  2021-01-11 10:57       ` Guillaume Nault
  0 siblings, 1 reply; 9+ messages in thread
From: David Ahern @ 2021-01-07 17:39 UTC (permalink / raw)
  To: Jakub Kicinski, Guillaume Nault; +Cc: Stephen Hemminger, netdev

On 1/7/21 10:13 AM, Jakub Kicinski wrote:
> On Thu, 7 Jan 2021 17:48:56 +0100 Guillaume Nault wrote:
>> On Fri, Dec 18, 2020 at 11:25:32PM +0100, Guillaume Nault wrote:
>>> The json output of the TCA_FLOWER_KEY_MPLS_OPTS attribute was invalid.
>>>
>>> Example:
>>>
>>>   $ tc filter add dev eth0 ingress protocol mpls_uc flower mpls \
>>>       lse depth 1 label 100                                     \
>>>       lse depth 2 label 200
>>>
>>>   $ tc -json filter show dev eth0 ingress
>>>     ...{"eth_type":"8847",
>>>         "  mpls":["    lse":["depth":1,"label":100],
>>>                   "    lse":["depth":2,"label":200]]}...  
>>
>> Is there any problem with this patch?
>> It's archived in patchwork, but still in state "new". Therefore I guess
>> it was dropped before being considered for review.
> 
> Erm, that's weird. I think Alexei mentioned that auto-archiving is
> turned on in the new netdevbpf patchwork instance. My guess is it got
> auto archived :S
> 
> Here is the list of all patches that are Archived as New:
> 
> https://patchwork.kernel.org/project/netdevbpf/list/?state=1&archive=true
> 
> Should any of these have been reviewed?
> 


Interesting. I thought some patches had magically disappeared - and some
of those are in that list.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2021-01-07 17:39     ` David Ahern
@ 2021-01-11 10:57       ` Guillaume Nault
  2021-01-11 15:30         ` David Ahern
  0 siblings, 1 reply; 9+ messages in thread
From: Guillaume Nault @ 2021-01-11 10:57 UTC (permalink / raw)
  To: David Ahern; +Cc: Jakub Kicinski, Stephen Hemminger, netdev

On Thu, Jan 07, 2021 at 10:39:03AM -0700, David Ahern wrote:
> On 1/7/21 10:13 AM, Jakub Kicinski wrote:
> > On Thu, 7 Jan 2021 17:48:56 +0100 Guillaume Nault wrote:
> >> On Fri, Dec 18, 2020 at 11:25:32PM +0100, Guillaume Nault wrote:
> >>> The json output of the TCA_FLOWER_KEY_MPLS_OPTS attribute was invalid.
> >>>
> >>> Example:
> >>>
> >>>   $ tc filter add dev eth0 ingress protocol mpls_uc flower mpls \
> >>>       lse depth 1 label 100                                     \
> >>>       lse depth 2 label 200
> >>>
> >>>   $ tc -json filter show dev eth0 ingress
> >>>     ...{"eth_type":"8847",
> >>>         "  mpls":["    lse":["depth":1,"label":100],
> >>>                   "    lse":["depth":2,"label":200]]}...  
> >>
> >> Is there any problem with this patch?
> >> It's archived in patchwork, but still in state "new". Therefore I guess
> >> it was dropped before being considered for review.
> > 
> > Erm, that's weird. I think Alexei mentioned that auto-archiving is
> > turned on in the new netdevbpf patchwork instance. My guess is it got
> > auto archived :S
> > 
> > Here is the list of all patches that are Archived as New:
> > 
> > https://patchwork.kernel.org/project/netdevbpf/list/?state=1&archive=true
> > 
> > Should any of these have been reviewed?
> > 
> 
> 
> Interesting. I thought some patches had magically disappeared - and some
> of those are in that list.

Okay, but, in the end, should I repost this patch?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2021-01-11 10:57       ` Guillaume Nault
@ 2021-01-11 15:30         ` David Ahern
  2021-01-11 15:44           ` Guillaume Nault
  0 siblings, 1 reply; 9+ messages in thread
From: David Ahern @ 2021-01-11 15:30 UTC (permalink / raw)
  To: Guillaume Nault; +Cc: Jakub Kicinski, Stephen Hemminger, netdev

On 1/11/21 3:57 AM, Guillaume Nault wrote:
> Okay, but, in the end, should I repost this patch?

I think your patches are covered, but you should check the repo to make
sure.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2021-01-11 15:30         ` David Ahern
@ 2021-01-11 15:44           ` Guillaume Nault
  2021-01-11 16:02             ` David Ahern
  0 siblings, 1 reply; 9+ messages in thread
From: Guillaume Nault @ 2021-01-11 15:44 UTC (permalink / raw)
  To: David Ahern; +Cc: Jakub Kicinski, Stephen Hemminger, netdev

On Mon, Jan 11, 2021 at 08:30:32AM -0700, David Ahern wrote:
> On 1/11/21 3:57 AM, Guillaume Nault wrote:
> > Okay, but, in the end, should I repost this patch?
> 
> I think your patches are covered, but you should check the repo to make
> sure.

This patch ("tc: flower: fix json output with mpls lse") doesn't appear
in the upstream tree.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2021-01-11 15:44           ` Guillaume Nault
@ 2021-01-11 16:02             ` David Ahern
  2021-01-11 16:06               ` Guillaume Nault
  0 siblings, 1 reply; 9+ messages in thread
From: David Ahern @ 2021-01-11 16:02 UTC (permalink / raw)
  To: Guillaume Nault; +Cc: Jakub Kicinski, Stephen Hemminger, netdev

On 1/11/21 8:44 AM, Guillaume Nault wrote:
> On Mon, Jan 11, 2021 at 08:30:32AM -0700, David Ahern wrote:
>> On 1/11/21 3:57 AM, Guillaume Nault wrote:
>>> Okay, but, in the end, should I repost this patch?
>>
>> I think your patches are covered, but you should check the repo to make
>> sure.
> 
> This patch ("tc: flower: fix json output with mpls lse") doesn't appear
> in the upstream tree.
> 
ok, you'll need to re-send.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH iproute2] tc: flower: fix json output with mpls lse
  2021-01-11 16:02             ` David Ahern
@ 2021-01-11 16:06               ` Guillaume Nault
  0 siblings, 0 replies; 9+ messages in thread
From: Guillaume Nault @ 2021-01-11 16:06 UTC (permalink / raw)
  To: David Ahern; +Cc: Jakub Kicinski, Stephen Hemminger, netdev

On Mon, Jan 11, 2021 at 09:02:04AM -0700, David Ahern wrote:
> On 1/11/21 8:44 AM, Guillaume Nault wrote:
> > On Mon, Jan 11, 2021 at 08:30:32AM -0700, David Ahern wrote:
> >> On 1/11/21 3:57 AM, Guillaume Nault wrote:
> >>> Okay, but, in the end, should I repost this patch?
> >>
> >> I think your patches are covered, but you should check the repo to make
> >> sure.
> > 
> > This patch ("tc: flower: fix json output with mpls lse") doesn't appear
> > in the upstream tree.
> > 
> ok, you'll need to re-send.

Will do, thanks.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-01-11 16:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 22:25 [PATCH iproute2] tc: flower: fix json output with mpls lse Guillaume Nault
2021-01-07 16:48 ` Guillaume Nault
2021-01-07 17:13   ` Jakub Kicinski
2021-01-07 17:39     ` David Ahern
2021-01-11 10:57       ` Guillaume Nault
2021-01-11 15:30         ` David Ahern
2021-01-11 15:44           ` Guillaume Nault
2021-01-11 16:02             ` David Ahern
2021-01-11 16:06               ` Guillaume Nault

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.