[PATCH iproute2 v2 0/5] fix json output for some keys

[PATCH iproute2 v2 0/5] fix json output for some keys

[Roi Dayan]

Hi,

This series is fixing output for tos, ttl, ct_zone, ct_mark
in json format.

Thanks,
Roi

Eli Britstein (5):
  tc_util: introduce a function to print JSON/non-JSON masked numbers
  tc_util: add an option to print masked numbers with/without a newline
  tc: flower: fix newline prints for ct-mark and ct-zone
  tc_util: fix JSON prints for ct-mark and ct-zone
  tc: flower: fix output for ip tos and ttl

 tc/f_flower.c | 19 +++-----------
 tc/m_ct.c     |  4 +--
 tc/tc_util.c  | 79 +++++++++++++++++++++++++++++++++++++++--------------------
 tc/tc_util.h  |  6 +++--
 4 files changed, 62 insertions(+), 46 deletions(-)

-- 
2.8.4

[PATCH iproute2 v2 1/5] tc_util: introduce a function to print JSON/non-JSON masked numbers

[Roi Dayan]

From: Eli Britstein <elibr@mellanox.com>

Introduce a function to print masked number with a different output for
JSON or non-JSON methods, as a pre-step towards printing numbers using
this common function.

Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---

Notes:
    v2:
    - use _SL_ instead of \n for newline.
    - return earlier if attr is not defined.

 tc/tc_util.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tc/tc_util.h |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 0eb530408d05..6bd098feff14 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -915,6 +915,45 @@ compat_xstats:
 		*xstats = tb[TCA_XSTATS];
 }
 
+static void print_masked_type(__u32 type_max,
+			      __u32 (*rta_getattr_type)(const struct rtattr *),
+			      const char *name, struct rtattr *attr,
+			      struct rtattr *mask_attr)
+{
+	SPRINT_BUF(namefrm);
+	__u32 value, mask;
+	SPRINT_BUF(out);
+	size_t done;
+
+	if (!attr)
+		return;
+
+	value = rta_getattr_type(attr);
+	mask = mask_attr ? rta_getattr_type(mask_attr) : type_max;
+
+	if (is_json_context()) {
+		sprintf(namefrm, "\n  %s %%u", name);
+		print_hu(PRINT_ANY, name, namefrm,
+			 rta_getattr_type(attr));
+		if (mask != type_max) {
+			char mask_name[SPRINT_BSIZE-6];
+
+			sprintf(mask_name, "%s_mask", name);
+			print_string(PRINT_FP, NULL, "%s  ", _SL_);
+			sprintf(namefrm, "%s %%u", mask_name);
+			print_hu(PRINT_ANY, mask_name, namefrm, mask);
+		}
+	} else {
+		done = sprintf(out, "%u", value);
+		if (mask != type_max)
+			sprintf(out + done, "/0x%x", mask);
+
+		print_string(PRINT_FP, NULL, "%s  ", _SL_);
+		sprintf(namefrm, "%s %%s", name);
+		print_string(PRINT_ANY, name, namefrm, out);
+	}
+}
+
 void print_masked_u32(const char *name, struct rtattr *attr,
 		      struct rtattr *mask_attr)
 {
@@ -958,3 +997,15 @@ void print_masked_u16(const char *name, struct rtattr *attr,
 	sprintf(namefrm, " %s %%s", name);
 	print_string(PRINT_ANY, name, namefrm, out);
 }
+
+static __u32 __rta_getattr_u8_u32(const struct rtattr *attr)
+{
+	return rta_getattr_u8(attr);
+}
+
+void print_masked_u8(const char *name, struct rtattr *attr,
+		     struct rtattr *mask_attr)
+{
+	print_masked_type(UINT8_MAX,  __rta_getattr_u8_u32, name, attr,
+			  mask_attr);
+}
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 0c3425abc62f..7e5d93cbac66 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -131,4 +131,6 @@ void print_masked_u32(const char *name, struct rtattr *attr,
 		      struct rtattr *mask_attr);
 void print_masked_u16(const char *name, struct rtattr *attr,
 		      struct rtattr *mask_attr);
+void print_masked_u8(const char *name, struct rtattr *attr,
+		     struct rtattr *mask_attr);
 #endif
-- 
2.8.4

[PATCH iproute2 v2 2/5] tc_util: add an option to print masked numbers with/without a newline

[Roi Dayan]

From: Eli Britstein <elibr@mellanox.com>

Add an option to print masked numbers with or without a newline, as a
pre-step towards using a common function.

Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 tc/f_flower.c |  4 ++--
 tc/m_ct.c     |  4 ++--
 tc/tc_util.c  | 25 +++++++++++++------------
 tc/tc_util.h  |  6 +++---
 4 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index a2a230162f78..41b81217e47e 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1847,13 +1847,13 @@ static void flower_print_ct_label(struct rtattr *attr,
 static void flower_print_ct_zone(struct rtattr *attr,
 				 struct rtattr *mask_attr)
 {
-	print_masked_u16("ct_zone", attr, mask_attr);
+	print_masked_u16("ct_zone", attr, mask_attr, false);
 }
 
 static void flower_print_ct_mark(struct rtattr *attr,
 				 struct rtattr *mask_attr)
 {
-	print_masked_u32("ct_mark", attr, mask_attr);
+	print_masked_u32("ct_mark", attr, mask_attr, false);
 }
 
 static void flower_print_key_id(const char *name, struct rtattr *attr)
diff --git a/tc/m_ct.c b/tc/m_ct.c
index d79eb5e361ac..8df2f6103601 100644
--- a/tc/m_ct.c
+++ b/tc/m_ct.c
@@ -466,8 +466,8 @@ static int print_ct(struct action_util *au, FILE *f, struct rtattr *arg)
 		print_string(PRINT_ANY, "action", " %s", "clear");
 	}
 
-	print_masked_u32("mark", tb[TCA_CT_MARK], tb[TCA_CT_MARK_MASK]);
-	print_masked_u16("zone", tb[TCA_CT_ZONE], NULL);
+	print_masked_u32("mark", tb[TCA_CT_MARK], tb[TCA_CT_MARK_MASK], false);
+	print_masked_u16("zone", tb[TCA_CT_ZONE], NULL, false);
 	ct_print_labels(tb[TCA_CT_LABELS], tb[TCA_CT_LABELS_MASK]);
 	ct_print_nat(ct_action, tb);
 
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 6bd098feff14..e9f3e5a227f9 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -918,7 +918,7 @@ compat_xstats:
 static void print_masked_type(__u32 type_max,
 			      __u32 (*rta_getattr_type)(const struct rtattr *),
 			      const char *name, struct rtattr *attr,
-			      struct rtattr *mask_attr)
+			      struct rtattr *mask_attr, bool newline)
 {
 	SPRINT_BUF(namefrm);
 	__u32 value, mask;
@@ -939,23 +939,24 @@ static void print_masked_type(__u32 type_max,
 			char mask_name[SPRINT_BSIZE-6];
 
 			sprintf(mask_name, "%s_mask", name);
-			print_string(PRINT_FP, NULL, "%s  ", _SL_);
-			sprintf(namefrm, "%s %%u", mask_name);
+			if (newline)
+				print_string(PRINT_FP, NULL, "%s ", _SL_);
+			sprintf(namefrm, " %s %%u", mask_name);
 			print_hu(PRINT_ANY, mask_name, namefrm, mask);
 		}
 	} else {
 		done = sprintf(out, "%u", value);
 		if (mask != type_max)
 			sprintf(out + done, "/0x%x", mask);
-
-		print_string(PRINT_FP, NULL, "%s  ", _SL_);
-		sprintf(namefrm, "%s %%s", name);
+		if (newline)
+			print_string(PRINT_FP, NULL, "%s ", _SL_);
+		sprintf(namefrm, " %s %%s", name);
 		print_string(PRINT_ANY, name, namefrm, out);
 	}
 }
 
 void print_masked_u32(const char *name, struct rtattr *attr,
-		      struct rtattr *mask_attr)
+		      struct rtattr *mask_attr, bool newline)
 {
 	__u32 value, mask;
 	SPRINT_BUF(namefrm);
@@ -972,12 +973,12 @@ void print_masked_u32(const char *name, struct rtattr *attr,
 	if (mask != UINT32_MAX)
 		sprintf(out + done, "/0x%x", mask);
 
-	sprintf(namefrm, " %s %%s", name);
+	sprintf(namefrm, "%s %s %%s", newline ? "\n " : "", name);
 	print_string(PRINT_ANY, name, namefrm, out);
 }
 
 void print_masked_u16(const char *name, struct rtattr *attr,
-		      struct rtattr *mask_attr)
+		      struct rtattr *mask_attr, bool newline)
 {
 	__u16 value, mask;
 	SPRINT_BUF(namefrm);
@@ -994,7 +995,7 @@ void print_masked_u16(const char *name, struct rtattr *attr,
 	if (mask != UINT16_MAX)
 		sprintf(out + done, "/0x%x", mask);
 
-	sprintf(namefrm, " %s %%s", name);
+	sprintf(namefrm, "%s %s %%s", newline ? "\n " : "", name);
 	print_string(PRINT_ANY, name, namefrm, out);
 }
 
@@ -1004,8 +1005,8 @@ static __u32 __rta_getattr_u8_u32(const struct rtattr *attr)
 }
 
 void print_masked_u8(const char *name, struct rtattr *attr,
-		     struct rtattr *mask_attr)
+		     struct rtattr *mask_attr, bool newline)
 {
 	print_masked_type(UINT8_MAX,  __rta_getattr_u8_u32, name, attr,
-			  mask_attr);
+			  mask_attr, newline);
 }
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 7e5d93cbac66..9adf2ab42138 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -128,9 +128,9 @@ int action_a2n(char *arg, int *result, bool allow_num);
 bool tc_qdisc_block_exists(__u32 block_index);
 
 void print_masked_u32(const char *name, struct rtattr *attr,
-		      struct rtattr *mask_attr);
+		      struct rtattr *mask_attr, bool newline);
 void print_masked_u16(const char *name, struct rtattr *attr,
-		      struct rtattr *mask_attr);
+		      struct rtattr *mask_attr, bool newline);
 void print_masked_u8(const char *name, struct rtattr *attr,
-		     struct rtattr *mask_attr);
+		     struct rtattr *mask_attr, bool newline);
 #endif
-- 
2.8.4

[PATCH iproute2 v2 3/5] tc: flower: fix newline prints for ct-mark and ct-zone

[Roi Dayan]

From: Eli Britstein <elibr@mellanox.com>

Matches of ct-mark and ct-zone were printed all in the same line. Fix
that so each ct match is printed in a separate line.

Example:
$ tc qdisc add dev eth0 ingress
$ tc filter add dev eth0 protocol ip parent ffff: prio 1 flower skip_hw \
      ct_zone 5 ct_mark 6/0xf action ct commit zone 7 mark 8/0xf drop

Before:
$ tc -s filter show dev eth0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4 ct_zone 5 ct_mark 6/0xf
  skip_hw
  not_in_hw
        action order 1: ct commit mark 8/0xf zone 7 drop
         index 1 ref 1 bind 1 installed 31 sec used 31 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

After:
$ tc -s filter show dev eth0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  ct_zone 5
  ct_mark 6/0xf
  skip_hw
  not_in_hw
        action order 1: ct commit mark 8/0xf zone 7 drop
         index 1 ref 1 bind 1 installed 108 sec used 108 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

Fixes: c8a494314c40 ("tc: Introduce tc ct action")
Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 tc/f_flower.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 41b81217e47e..724577563c27 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1847,13 +1847,13 @@ static void flower_print_ct_label(struct rtattr *attr,
 static void flower_print_ct_zone(struct rtattr *attr,
 				 struct rtattr *mask_attr)
 {
-	print_masked_u16("ct_zone", attr, mask_attr, false);
+	print_masked_u16("ct_zone", attr, mask_attr, true);
 }
 
 static void flower_print_ct_mark(struct rtattr *attr,
 				 struct rtattr *mask_attr)
 {
-	print_masked_u32("ct_mark", attr, mask_attr, false);
+	print_masked_u32("ct_mark", attr, mask_attr, true);
 }
 
 static void flower_print_key_id(const char *name, struct rtattr *attr)
-- 
2.8.4

[PATCH iproute2 v2 4/5] tc_util: fix JSON prints for ct-mark and ct-zone

[Roi Dayan]

From: Eli Britstein <elibr@mellanox.com>

Fix the output of ct-mark and ct-zone (both for matches and actions) to
be different in JSON/non-JSON mode.

Example:
$ tc qdisc add dev eth0 ingress
$ tc filter add dev eth0 protocol ip parent ffff: prio 1 flower skip_hw \
      ct_zone 5 ct_mark 6/0xf action ct commit zone 7 mark 8/0xf drop

Non JSON format remains the same:
$ tc filter show dev eth0 parent ffff:
$ tc -s filter show dev ens1f0_0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  ct_zone 5
  ct_mark 6/0xf
  skip_hw
  not_in_hw
        action order 1: ct commit mark 8/0xf zone 7 drop
         index 1 ref 1 bind 1 installed 108 sec used 108 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

JSON format is changed (partial output):
$ tc -p -j filter show dev eth0 parent ffff:
Before:
        "options": {
            "keys": {
                "ct_zone": "5",
                "ct_mark": "6/0xf"
                ...
        "actions": [ {
                "order": 1,
                "kind": "ct",
                "action": "commit",
                "mark": "8/0xf",
                "zone": "7",
                ...
After:
        "options": {
            "keys": {
                "ct_zone": 5,
                "ct_mark": 6,
                "ct_mark_mask": 15
                ...
        "actions": [ {
                "order": 1,
                "kind": "ct",
                "action": "commit",
                "mark": 8,
                "mark_mask": 15,
                "zone": 7,
                ...

Fixes: c8a494314c40 ("tc: Introduce tc ct action")
Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 tc/tc_util.c | 41 ++++++++---------------------------------
 1 file changed, 8 insertions(+), 33 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index e9f3e5a227f9..393721e33bf0 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -958,45 +958,20 @@ static void print_masked_type(__u32 type_max,
 void print_masked_u32(const char *name, struct rtattr *attr,
 		      struct rtattr *mask_attr, bool newline)
 {
-	__u32 value, mask;
-	SPRINT_BUF(namefrm);
-	SPRINT_BUF(out);
-	size_t done;
-
-	if (!attr)
-		return;
-
-	value = rta_getattr_u32(attr);
-	mask = mask_attr ? rta_getattr_u32(mask_attr) : UINT32_MAX;
-
-	done = sprintf(out, "%u", value);
-	if (mask != UINT32_MAX)
-		sprintf(out + done, "/0x%x", mask);
+	print_masked_type(UINT32_MAX, rta_getattr_u32, name, attr, mask_attr,
+			  newline);
+}
 
-	sprintf(namefrm, "%s %s %%s", newline ? "\n " : "", name);
-	print_string(PRINT_ANY, name, namefrm, out);
+static __u32 __rta_getattr_u16_u32(const struct rtattr *attr)
+{
+	return rta_getattr_u16(attr);
 }
 
 void print_masked_u16(const char *name, struct rtattr *attr,
 		      struct rtattr *mask_attr, bool newline)
 {
-	__u16 value, mask;
-	SPRINT_BUF(namefrm);
-	SPRINT_BUF(out);
-	size_t done;
-
-	if (!attr)
-		return;
-
-	value = rta_getattr_u16(attr);
-	mask = mask_attr ? rta_getattr_u16(mask_attr) : UINT16_MAX;
-
-	done = sprintf(out, "%u", value);
-	if (mask != UINT16_MAX)
-		sprintf(out + done, "/0x%x", mask);
-
-	sprintf(namefrm, "%s %s %%s", newline ? "\n " : "", name);
-	print_string(PRINT_ANY, name, namefrm, out);
+	print_masked_type(UINT16_MAX, __rta_getattr_u16_u32, name, attr,
+			  mask_attr, newline);
 }
 
 static __u32 __rta_getattr_u8_u32(const struct rtattr *attr)
-- 
2.8.4

[PATCH iproute2 v2 5/5] tc: flower: fix output for ip tos and ttl

[Roi Dayan]

From: Eli Britstein <elibr@mellanox.com>

Fix the output for ip tos and ttl to be numbers in JSON format.

Example:
$ tc qdisc add dev eth0 ingress
$ tc filter add dev eth0 protocol ip parent ffff: prio 1 flower skip_hw \
      ip_tos 5/0xf action drop

Non JSON format remains the same:
$ tc filter show dev eth0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  ip_tos 5/0xf
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 1 ref 1 bind 1

JSON format is changed (partial output):
$ tc -p -j filter show dev eth0 parent ffff:
Before:
        "options": {
            "keys": {
                "ip_tos": "0x5/f",
                ...
After:
        "options": {
            "keys": {
                "ip_tos": 5,
                "ip_tos_mask": 15,
                ...

Fixes: 6ea2c2b1cff6 ("tc: flower: add support for matching on ip tos and ttl")
Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 tc/f_flower.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 724577563c27..1b518ef30583 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1617,20 +1617,7 @@ static void flower_print_ip_proto(__u8 *p_ip_proto,
 static void flower_print_ip_attr(const char *name, struct rtattr *key_attr,
 				 struct rtattr *mask_attr)
 {
-	SPRINT_BUF(namefrm);
-	SPRINT_BUF(out);
-	size_t done;
-
-	if (!key_attr)
-		return;
-
-	done = sprintf(out, "0x%x", rta_getattr_u8(key_attr));
-	if (mask_attr)
-		sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
-
-	print_string(PRINT_FP, NULL, "%s  ", _SL_);
-	sprintf(namefrm, "%s %%s", name);
-	print_string(PRINT_ANY, name, namefrm, out);
+	print_masked_u8(name, key_attr, mask_attr, true);
 }
 
 static void flower_print_matching_flags(char *name,
-- 
2.8.4

Re: [PATCH iproute2 v2 1/5] tc_util: introduce a function to print JSON/non-JSON masked numbers

[Stephen Hemminger]

On Thu, 14 Nov 2019 14:44:37 +0200
Roi Dayan <roid@mellanox.com> wrote:

> +			print_string(PRINT_FP, NULL, "%s  ", _SL_);

Maybe use print_nl() but you don't have to.

Re: [PATCH iproute2 v2 1/5] tc_util: introduce a function to print JSON/non-JSON masked numbers

[Roi Dayan]

On 2019-11-15 1:13 AM, Stephen Hemminger wrote:
> On Thu, 14 Nov 2019 14:44:37 +0200
> Roi Dayan <roid@mellanox.com> wrote:
> 
>> +			print_string(PRINT_FP, NULL, "%s  ", _SL_);
> 
> Maybe use print_nl() but you don't have to.
> 

I also need the 2 spaces for indentation here.

Re: [PATCH iproute2 v2 0/5] fix json output for some keys

[Stephen Hemminger]

On Thu, 14 Nov 2019 14:44:36 +0200
Roi Dayan <roid@mellanox.com> wrote:

> Hi,
> 
> This series is fixing output for tos, ttl, ct_zone, ct_mark
> in json format.
> 
> Thanks,
> Roi
> 
> Eli Britstein (5):
>   tc_util: introduce a function to print JSON/non-JSON masked numbers
>   tc_util: add an option to print masked numbers with/without a newline
>   tc: flower: fix newline prints for ct-mark and ct-zone
>   tc_util: fix JSON prints for ct-mark and ct-zone
>   tc: flower: fix output for ip tos and ttl
> 
>  tc/f_flower.c | 19 +++-----------
>  tc/m_ct.c     |  4 +--
>  tc/tc_util.c  | 79 +++++++++++++++++++++++++++++++++++++++--------------------
>  tc/tc_util.h  |  6 +++--
>  4 files changed, 62 insertions(+), 46 deletions(-)
> 

Applied, thanks