netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2-next] tc: pedit: Support JSON dumping
@ 2020-04-22 17:06 Petr Machata
  2020-04-22 20:02 ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Petr Machata @ 2020-04-22 17:06 UTC (permalink / raw)
  To: netdev; +Cc: Petr Machata, dsahern, stephen

The action pedit does not currently support dumping to JSON. Convert
print_pedit() to the print_* family of functions so that dumping is correct
both in plain and JSON mode. In plain mode, the output is character for
character the same as it was before. In JSON mode, this is an example dump:

$ tc filter add dev dummy0 ingress prio 125 flower \
         action pedit ex munge udp dport set 12345 \
	                 munge ip ttl add 1        \
			 munge offset 10 u8 clear
$ tc -j filter show dev dummy0 ingress | jq
[
  {
    "protocol": "all",
    "pref": 125,
    "kind": "flower",
    "chain": 0
  },
  {
    "protocol": "all",
    "pref": 125,
    "kind": "flower",
    "chain": 0,
    "options": {
      "handle": 1,
      "keys": {},
      "not_in_hw": true,
      "actions": [
        {
          "order": 1,
          "kind": "pedit",
          "control_action": {
            "type": "pass"
          },
          "nkeys": 3,
          "index": 1,
          "ref": 1,
          "bind": 1,
          "keys": [
            {
              "htype": "udp",
              "offset": 0,
              "cmd": "set",
              "val": "3039",
              "mask": "ffff0000"
            },
            {
              "htype": "ipv4",
              "offset": 8,
              "cmd": "add",
              "val": "1000000",
              "mask": "ffffff"
            },
            {
              "htype": "network",
              "offset": 8,
              "cmd": "set",
              "val": "0",
              "mask": "ffff00ff"
            }
          ]
        }
      ]
    }
  }
]

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tc/m_pedit.c | 64 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index fccfd17c..d3aa08de 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -714,20 +714,28 @@ static const char * const pedit_htype_str[] = {
 	[TCA_PEDIT_KEY_EX_HDR_TYPE_UDP] = "udp",
 };
 
-static void print_pedit_location(FILE *f,
-				 enum pedit_header_type htype, __u32 off)
+static int print_pedit_location(FILE *f,
+				enum pedit_header_type htype, __u32 off)
 {
-	if (htype == TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK) {
-		fprintf(f, "%d", (unsigned int)off);
-		return;
+	char *buf = NULL;
+	int rc;
+
+	if (htype != TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK) {
+		if (htype < ARRAY_SIZE(pedit_htype_str))
+			rc = asprintf(&buf, "%s", pedit_htype_str[htype]);
+		else
+			rc = asprintf(&buf, "unknown(%d)", htype);
+		if (rc < 0)
+			return rc;
+		print_string(PRINT_ANY, "htype", "%s", buf);
+		print_int(PRINT_ANY, "offset", "%+d", off);
+	} else {
+		print_string(PRINT_JSON, "htype", NULL, "network");
+		print_int(PRINT_ANY, "offset", "%d", off);
 	}
 
-	if (htype < ARRAY_SIZE(pedit_htype_str))
-		fprintf(f, "%s", pedit_htype_str[htype]);
-	else
-		fprintf(f, "unknown(%d)", htype);
-
-	fprintf(f, "%c%d", (int)off  >= 0 ? '+' : '-', abs((int)off));
+	free(buf);
+	return 0;
 }
 
 static int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg)
@@ -735,6 +743,7 @@ static int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct tc_pedit_sel *sel;
 	struct rtattr *tb[TCA_PEDIT_MAX + 1];
 	struct m_pedit_key_ex *keys_ex = NULL;
+	int err;
 
 	if (arg == NULL)
 		return -1;
@@ -774,11 +783,12 @@ static int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg)
 		}
 	}
 
-	fprintf(f, " pedit ");
+	print_string(PRINT_ANY, "kind", " %s ", "pedit");
 	print_action_control(f, "action ", sel->action, " ");
-	fprintf(f,"keys %d\n ", sel->nkeys);
-	fprintf(f, "\t index %u ref %d bind %d", sel->index, sel->refcnt,
-		sel->bindcnt);
+	print_uint(PRINT_ANY, "nkeys", "keys %d\n", sel->nkeys);
+	print_uint(PRINT_ANY, "index", " \t index %u", sel->index);
+	print_int(PRINT_ANY, "ref", " ref %d", sel->refcnt);
+	print_int(PRINT_ANY, "bind", " bind %d", sel->bindcnt);
 
 	if (show_stats) {
 		if (tb[TCA_PEDIT_TM]) {
@@ -787,6 +797,7 @@ static int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg)
 			print_tm(f, tm);
 		}
 	}
+	open_json_array(PRINT_JSON, "keys");
 	if (sel->nkeys) {
 		int i;
 		struct tc_pedit_key *key = sel->keys;
@@ -804,21 +815,28 @@ static int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg)
 				key_ex++;
 			}
 
-			fprintf(f, "\n\t key #%d", i);
+			open_json_object(NULL);
+			print_uint(PRINT_FP, NULL, "\n\t key #%d  at ", i);
 
-			fprintf(f, "  at ");
+			err = print_pedit_location(f, htype, key->off);
+			if (err)
+				return err;
 
-			print_pedit_location(f, htype, key->off);
-
-			fprintf(f, ": %s %08x mask %08x",
-				cmd ? "add" : "val",
-				(unsigned int)ntohl(key->val),
-				(unsigned int)ntohl(key->mask));
+			print_string(PRINT_FP, NULL, ": %s",
+				     cmd ? "add" : "val");
+			print_string(PRINT_JSON, "cmd", NULL,
+				     cmd ? "add" : "set");
+			print_hex(PRINT_ANY, "val", " %08x",
+				  (unsigned int)ntohl(key->val));
+			print_hex(PRINT_ANY, "mask", " mask %08x",
+				  (unsigned int)ntohl(key->mask));
+			close_json_object();
 		}
 	} else {
 		fprintf(f, "\npedit %x keys %d is not LEGIT", sel->index,
 			sel->nkeys);
 	}
+	close_json_array(PRINT_JSON, " ");
 
 	print_nl();
 
-- 
2.20.1


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

* Re: [PATCH iproute2-next] tc: pedit: Support JSON dumping
  2020-04-22 17:06 [PATCH iproute2-next] tc: pedit: Support JSON dumping Petr Machata
@ 2020-04-22 20:02 ` Stephen Hemminger
  2020-04-23  9:59   ` Petr Machata
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2020-04-22 20:02 UTC (permalink / raw)
  To: Petr Machata; +Cc: netdev, dsahern

On Wed, 22 Apr 2020 20:06:15 +0300
Petr Machata <petrm@mellanox.com> wrote:

> +			print_string(PRINT_FP, NULL, ": %s",
> +				     cmd ? "add" : "val");
> +			print_string(PRINT_JSON, "cmd", NULL,
> +				     cmd ? "add" : "set");

Having different outputs for JSON and file here. Is that necessary?
JSON output is new, and could just mirror existing usage.

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

* Re: [PATCH iproute2-next] tc: pedit: Support JSON dumping
  2020-04-22 20:02 ` Stephen Hemminger
@ 2020-04-23  9:59   ` Petr Machata
  2020-04-26 18:23     ` David Ahern
  0 siblings, 1 reply; 7+ messages in thread
From: Petr Machata @ 2020-04-23  9:59 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, dsahern


Stephen Hemminger <stephen@networkplumber.org> writes:

> On Wed, 22 Apr 2020 20:06:15 +0300
> Petr Machata <petrm@mellanox.com> wrote:
>
>> +			print_string(PRINT_FP, NULL, ": %s",
>> +				     cmd ? "add" : "val");
>> +			print_string(PRINT_JSON, "cmd", NULL,
>> +				     cmd ? "add" : "set");
>
> Having different outputs for JSON and file here. Is that necessary?
> JSON output is new, and could just mirror existing usage.

This code outputs this bit:

            {
              "htype": "udp",
              "offset": 0,
              "cmd": "set",   <----
              "val": "3039",
              "mask": "ffff0000"
            },

There are currently two commands, set and add. The words used to
configure these actions are set and add as well. The way these commands
are dumped should be the same, too. The only reason why "set" is
reported as "val" in file is that set used to be the implied action.

JSON doesn't have to be backward compatible, so it should present the
expected words.

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

* Re: [PATCH iproute2-next] tc: pedit: Support JSON dumping
  2020-04-23  9:59   ` Petr Machata
@ 2020-04-26 18:23     ` David Ahern
  2020-04-27 23:09       ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2020-04-26 18:23 UTC (permalink / raw)
  To: Petr Machata, Stephen Hemminger; +Cc: netdev, dsahern

On 4/23/20 3:59 AM, Petr Machata wrote:
> 
> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
>> On Wed, 22 Apr 2020 20:06:15 +0300
>> Petr Machata <petrm@mellanox.com> wrote:
>>
>>> +			print_string(PRINT_FP, NULL, ": %s",
>>> +				     cmd ? "add" : "val");
>>> +			print_string(PRINT_JSON, "cmd", NULL,
>>> +				     cmd ? "add" : "set");
>>
>> Having different outputs for JSON and file here. Is that necessary?
>> JSON output is new, and could just mirror existing usage.
> 
> This code outputs this bit:
> 
>             {
>               "htype": "udp",
>               "offset": 0,
>               "cmd": "set",   <----
>               "val": "3039",
>               "mask": "ffff0000"
>             },
> 
> There are currently two commands, set and add. The words used to
> configure these actions are set and add as well. The way these commands
> are dumped should be the same, too. The only reason why "set" is
> reported as "val" in file is that set used to be the implied action.
> 
> JSON doesn't have to be backward compatible, so it should present the
> expected words.
> 

Stephen: do you agree?

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

* Re: [PATCH iproute2-next] tc: pedit: Support JSON dumping
  2020-04-26 18:23     ` David Ahern
@ 2020-04-27 23:09       ` Stephen Hemminger
  2020-04-28  9:32         ` Petr Machata
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2020-04-27 23:09 UTC (permalink / raw)
  To: David Ahern; +Cc: Petr Machata, netdev

On Sun, 26 Apr 2020 12:23:04 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 4/23/20 3:59 AM, Petr Machata wrote:
> > 
> > Stephen Hemminger <stephen@networkplumber.org> writes:
> >   
> >> On Wed, 22 Apr 2020 20:06:15 +0300
> >> Petr Machata <petrm@mellanox.com> wrote:
> >>  
> >>> +			print_string(PRINT_FP, NULL, ": %s",
> >>> +				     cmd ? "add" : "val");
> >>> +			print_string(PRINT_JSON, "cmd", NULL,
> >>> +				     cmd ? "add" : "set");  
> >>
> >> Having different outputs for JSON and file here. Is that necessary?
> >> JSON output is new, and could just mirror existing usage.  
> > 
> > This code outputs this bit:
> > 
> >             {
> >               "htype": "udp",
> >               "offset": 0,
> >               "cmd": "set",   <----
> >               "val": "3039",
> >               "mask": "ffff0000"
> >             },
> > 
> > There are currently two commands, set and add. The words used to
> > configure these actions are set and add as well. The way these commands
> > are dumped should be the same, too. The only reason why "set" is
> > reported as "val" in file is that set used to be the implied action.
> > 
> > JSON doesn't have to be backward compatible, so it should present the
> > expected words.
> >   
> 
> Stephen: do you agree?

Sure that is fine, maybe a comment would help?

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

* Re: [PATCH iproute2-next] tc: pedit: Support JSON dumping
  2020-04-27 23:09       ` Stephen Hemminger
@ 2020-04-28  9:32         ` Petr Machata
  2020-04-28 11:47           ` Petr Machata
  0 siblings, 1 reply; 7+ messages in thread
From: Petr Machata @ 2020-04-28  9:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Ahern, netdev


Stephen Hemminger <stephen@networkplumber.org> writes:

> On Sun, 26 Apr 2020 12:23:04 -0600
> David Ahern <dsahern@gmail.com> wrote:
>
>> On 4/23/20 3:59 AM, Petr Machata wrote:
>> >
>> > Stephen Hemminger <stephen@networkplumber.org> writes:
>> >
>> >> On Wed, 22 Apr 2020 20:06:15 +0300
>> >> Petr Machata <petrm@mellanox.com> wrote:
>> >>
>> >>> +			print_string(PRINT_FP, NULL, ": %s",
>> >>> +				     cmd ? "add" : "val");
>> >>> +			print_string(PRINT_JSON, "cmd", NULL,
>> >>> +				     cmd ? "add" : "set");
>> >>
>> >> Having different outputs for JSON and file here. Is that necessary?
>> >> JSON output is new, and could just mirror existing usage.
>> >
>> > This code outputs this bit:
>> >
>> >             {
>> >               "htype": "udp",
>> >               "offset": 0,
>> >               "cmd": "set",   <----
>> >               "val": "3039",
>> >               "mask": "ffff0000"
>> >             },
>> >
>> > There are currently two commands, set and add. The words used to
>> > configure these actions are set and add as well. The way these commands
>> > are dumped should be the same, too. The only reason why "set" is
>> > reported as "val" in file is that set used to be the implied action.
>> >
>> > JSON doesn't have to be backward compatible, so it should present the
>> > expected words.
>> >
>>
>> Stephen: do you agree?
>
> Sure that is fine, maybe a comment would help?

Something like this?

                        /* In FP, report the "set" command as "val" to keep
                         * backward compatibility.
                         */
			print_string(PRINT_FP, NULL, ": %s",
				     cmd ? "add" : "val");
			print_string(PRINT_JSON, "cmd", NULL,
				     cmd ? "add" : "set");

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

* Re: [PATCH iproute2-next] tc: pedit: Support JSON dumping
  2020-04-28  9:32         ` Petr Machata
@ 2020-04-28 11:47           ` Petr Machata
  0 siblings, 0 replies; 7+ messages in thread
From: Petr Machata @ 2020-04-28 11:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Ahern, netdev


Petr Machata <petrm@mellanox.com> writes:

> Stephen Hemminger <stephen@networkplumber.org> writes:
>
>> On Sun, 26 Apr 2020 12:23:04 -0600
>> David Ahern <dsahern@gmail.com> wrote:
>>
>>> On 4/23/20 3:59 AM, Petr Machata wrote:
>>> >
>>> > Stephen Hemminger <stephen@networkplumber.org> writes:
>>> >
>>> >> On Wed, 22 Apr 2020 20:06:15 +0300
>>> >> Petr Machata <petrm@mellanox.com> wrote:
>>> >>
>>> >>> +			print_string(PRINT_FP, NULL, ": %s",
>>> >>> +				     cmd ? "add" : "val");
>>> >>> +			print_string(PRINT_JSON, "cmd", NULL,
>>> >>> +				     cmd ? "add" : "set");
>>> >>
>>> >> Having different outputs for JSON and file here. Is that necessary?
>>> >> JSON output is new, and could just mirror existing usage.
>>> >
>>> > This code outputs this bit:
>>> >
>>> >             {
>>> >               "htype": "udp",
>>> >               "offset": 0,
>>> >               "cmd": "set",   <----
>>> >               "val": "3039",
>>> >               "mask": "ffff0000"
>>> >             },
>>> >
>>> > There are currently two commands, set and add. The words used to
>>> > configure these actions are set and add as well. The way these commands
>>> > are dumped should be the same, too. The only reason why "set" is
>>> > reported as "val" in file is that set used to be the implied action.
>>> >
>>> > JSON doesn't have to be backward compatible, so it should present the
>>> > expected words.
>>> >
>>>
>>> Stephen: do you agree?
>>
>> Sure that is fine, maybe a comment would help?
>
> Something like this?
>
>                         /* In FP, report the "set" command as "val" to keep
>                          * backward compatibility.
>                          */
> 			print_string(PRINT_FP, NULL, ": %s",
> 				     cmd ? "add" : "val");
> 			print_string(PRINT_JSON, "cmd", NULL,
> 				     cmd ? "add" : "set");

I just sent it as a v2 of the patch, we can discuss there.

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

end of thread, other threads:[~2020-04-28 11:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-22 17:06 [PATCH iproute2-next] tc: pedit: Support JSON dumping Petr Machata
2020-04-22 20:02 ` Stephen Hemminger
2020-04-23  9:59   ` Petr Machata
2020-04-26 18:23     ` David Ahern
2020-04-27 23:09       ` Stephen Hemminger
2020-04-28  9:32         ` Petr Machata
2020-04-28 11:47           ` Petr Machata

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