[PATCH v2 iproute2-next 00/11] clang warning fixes

[PATCH v2 iproute2-next 00/11] clang warning fixes

[Stephen Hemminger]

This patch set makes iproute2-next main branch compile without warnings
on Clang 11 (and probably later versions).

Still needs more testing before merge. There are likely to be some
unnecessary output format changes from this.

v2
  - use print name value


Stephen Hemminger (11):
  tc: add format attribute to tc_print_rate
  utils: add format attribute
  netem: fix clang warnings
  flower: fix clang warnings
  tc_util: fix clang warning in print_masked_type
  ipl2tp: fix clang warning
  can: fix clang warning
  tipc: fix clang warning about empty format string
  tunnel: fix clang warning
  libbpf: fix clang warning about format non-literal
  json_print: suppress clang format warning

 include/utils.h  |  4 +++-
 ip/ipl2tp.c      |  5 +++--
 ip/iplink_can.c  |  5 +++--
 ip/tunnel.c      |  9 +--------
 lib/bpf_libbpf.c |  6 ++++--
 lib/json_print.c |  8 ++++++++
 tc/f_flower.c    | 38 ++++++++++++--------------------------
 tc/q_netem.c     | 33 ++++++++++++++++++++-------------
 tc/tc_util.c     | 20 +++++++-------------
 tipc/link.c      |  2 +-
 10 files changed, 62 insertions(+), 68 deletions(-)

-- 
2.30.2

[PATCH v2 iproute2-next 01/11] tc: add format attribute to tc_print_rate

[Stephen Hemminger]

From: Stephen Hemminger <stephen@networkplumber.org>

This catches future errors and silences warning from Clang.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/tc_util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 48065897cee7..6d5eb754831a 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -247,7 +247,8 @@ int get_percent_rate64(__u64 *rate, const char *str, const char *dev)
 	return get_rate64(rate, r_str);
 }
 
-void tc_print_rate(enum output_type t, const char *key, const char *fmt,
+void __attribute__((format(printf, 3, 0)))
+tc_print_rate(enum output_type t, const char *key, const char *fmt,
 		   unsigned long long rate)
 {
 	print_rate(use_iec, t, key, fmt, rate);
-- 
2.30.2

[PATCH v2 iproute2-next 02/11] utils: add format attribute

[Stephen Hemminger]

From: Stephen Hemminger <stephen@networkplumber.org>

One more format attribute needed to resolve clang warnings.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 include/utils.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/utils.h b/include/utils.h
index b6c468e9cc86..d644202cc529 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -261,7 +261,9 @@ int print_timestamp(FILE *fp);
 void print_nlmsg_timestamp(FILE *fp, const struct nlmsghdr *n);
 
 unsigned int print_name_and_link(const char *fmt,
-				 const char *name, struct rtattr *tb[]);
+				 const char *name, struct rtattr *tb[])
+	__attribute__((format(printf, 1, 0)));
+
 
 #define BIT(nr)                 (UINT64_C(1) << (nr))
 
-- 
2.30.2

[PATCH v2 iproute2-next 03/11] netem: fix clang warnings

[Stephen Hemminger]

From: Stephen Hemminger <stephen@networkplumber.org>

Netem is using empty format string to not print values.
Clang complains about this hack so don't do it.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/q_netem.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/tc/q_netem.c b/tc/q_netem.c
index 2e5a46ab7b25..c483386894ea 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -59,10 +59,12 @@ static void explain1(const char *arg)
 #define MAX_DIST	(16*1024)
 
 /* Print values only if they are non-zero */
-static void __print_int_opt(const char *label_json, const char *label_fp,
-			    int val)
+static void __attribute__((format(printf, 2, 0)))
+__print_int_opt(const char *label_json, const char *label_fp, int val)
 {
-	print_int(PRINT_ANY, label_json, val ? label_fp : "", val);
+	print_int(PRINT_JSON, label_json, NULL, val);
+	if (val != 0)
+		print_int(PRINT_FP, NULL, label_fp, val);
 }
 #define PRINT_INT_OPT(label, val)			\
 	__print_int_opt(label, " " label " %d", (val))
@@ -70,8 +72,8 @@ static void __print_int_opt(const char *label_json, const char *label_fp,
 /* Time print prints normally with varying units, but for JSON prints
  * in seconds (1ms vs 0.001).
  */
-static void __print_time64(const char *label_json, const char *label_fp,
-			   __u64 val)
+static void __attribute__((format(printf, 2, 0)))
+__print_time64(const char *label_json, const char *label_fp, __u64 val)
 {
 	SPRINT_BUF(b1);
 
@@ -85,8 +87,8 @@ static void __print_time64(const char *label_json, const char *label_fp,
 /* Percent print prints normally in percentage points, but for JSON prints
  * an absolute value (1% vs 0.01).
  */
-static void __print_percent(const char *label_json, const char *label_fp,
-			    __u32 per)
+static void __attribute__((format(printf, 2, 0)))
+__print_percent(const char *label_json, const char *label_fp, __u32 per)
 {
 	print_float(PRINT_FP, NULL, label_fp, (100. * per) / UINT32_MAX);
 	print_float(PRINT_JSON, label_json, NULL, (1. * per) / UINT32_MAX);
@@ -802,9 +804,10 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 		rate64 = rate64 ? : rate->rate;
 		tc_print_rate(PRINT_ANY, "rate", " rate %s", rate64);
 		PRINT_INT_OPT("packetoverhead", rate->packet_overhead);
-		print_uint(PRINT_ANY, "cellsize",
-			   rate->cell_size ? " cellsize %u" : "",
-			   rate->cell_size);
+
+		print_uint(PRINT_JSON, "cellsize", NULL, rate->cell_size);
+		if (rate->cell_size)
+			print_uint(PRINT_FP, NULL, " cellsize %u", rate->cell_size);
 		PRINT_INT_OPT("celloverhead", rate->cell_overhead);
 		close_json_object();
 	}
@@ -824,9 +827,13 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 		close_json_object();
 	}
 
-	print_bool(PRINT_ANY, "ecn", ecn ? " ecn " : "", ecn);
-	print_luint(PRINT_ANY, "gap", qopt.gap ? " gap %lu" : "",
-		    (unsigned long)qopt.gap);
+	print_bool(PRINT_JSON, "ecn", NULL, ecn);
+	if (ecn)
+		print_bool(PRINT_FP, NULL, " ecn ", ecn);
+
+	print_luint(PRINT_JSON, "gap", NULL, qopt.gap);
+	if (qopt.gap)
+		print_luint(PRINT_FP, NULL, " gap %lu", qopt.gap);
 
 	return 0;
 }
-- 
2.30.2

[PATCH v2 iproute2-next 04/11] flower: fix clang warnings

[Stephen Hemminger]

Clang complains about passing non-format string to print_string.
Handle this by splitting json and non-json parts.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 tc/f_flower.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 6d70b92a2894..1dcce8c4aa95 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1925,10 +1925,9 @@ static int __mask_bits(char *addr, size_t len)
 	return bits;
 }
 
-static void flower_print_eth_addr(char *name, struct rtattr *addr_attr,
+static void flower_print_eth_addr(const char *name, struct rtattr *addr_attr,
 				  struct rtattr *mask_attr)
 {
-	SPRINT_BUF(namefrm);
 	SPRINT_BUF(out);
 	SPRINT_BUF(b1);
 	size_t done;
@@ -1950,8 +1949,7 @@ static void flower_print_eth_addr(char *name, struct rtattr *addr_attr,
 	}
 
 	print_nl();
-	sprintf(namefrm, "  %s %%s", name);
-	print_string(PRINT_ANY, name, namefrm, out);
+	print_string_name_value(name, out);
 }
 
 static void flower_print_eth_type(__be16 *p_eth_type,
@@ -2064,7 +2062,6 @@ static void flower_print_ip_addr(char *name, __be16 eth_type,
 {
 	struct rtattr *addr_attr;
 	struct rtattr *mask_attr;
-	SPRINT_BUF(namefrm);
 	SPRINT_BUF(out);
 	size_t done;
 	int family;
@@ -2096,9 +2093,9 @@ static void flower_print_ip_addr(char *name, __be16 eth_type,
 		sprintf(out + done, "/%d", bits);
 
 	print_nl();
-	sprintf(namefrm, "  %s %%s", name);
-	print_string(PRINT_ANY, name, namefrm, out);
+	print_string_name_value(name, out);
 }
+
 static void flower_print_ip4_addr(char *name, struct rtattr *addr_attr,
 				  struct rtattr *mask_attr)
 {
@@ -2124,22 +2121,19 @@ static void flower_print_port_range(char *name, struct rtattr *min_attr,
 		print_hu(PRINT_JSON, "end", NULL, rta_getattr_be16(max_attr));
 		close_json_object();
 	} else {
-		SPRINT_BUF(namefrm);
 		SPRINT_BUF(out);
 		size_t done;
 
 		done = sprintf(out, "%u", rta_getattr_be16(min_attr));
 		sprintf(out + done, "-%u", rta_getattr_be16(max_attr));
 		print_nl();
-		sprintf(namefrm, "  %s %%s", name);
-		print_string(PRINT_ANY, name, namefrm, out);
+		print_string_name_value(name, out);
 	}
 }
 
 static void flower_print_tcp_flags(const char *name, struct rtattr *flags_attr,
 				   struct rtattr *mask_attr)
 {
-	SPRINT_BUF(namefrm);
 	SPRINT_BUF(out);
 	size_t done;
 
@@ -2151,8 +2145,7 @@ static void flower_print_tcp_flags(const char *name, struct rtattr *flags_attr,
 		sprintf(out + done, "/%x", rta_getattr_be16(mask_attr));
 
 	print_nl();
-	sprintf(namefrm, "  %s %%s", name);
-	print_string(PRINT_ANY, name, namefrm, out);
+	print_string_name_value(name, out);
 }
 
 static void flower_print_ct_state(struct rtattr *flags_attr,
@@ -2239,14 +2232,11 @@ static void flower_print_ct_mark(struct rtattr *attr,
 
 static void flower_print_key_id(const char *name, struct rtattr *attr)
 {
-	SPRINT_BUF(namefrm);
-
 	if (!attr)
 		return;
 
 	print_nl();
-	sprintf(namefrm, "  %s %%u", name);
-	print_uint(PRINT_ANY, name, namefrm, rta_getattr_be32(attr));
+	print_uint_name_value(name, rta_getattr_be32(attr));
 }
 
 static void flower_print_geneve_opts(const char *name, struct rtattr *attr,
@@ -2342,8 +2332,9 @@ static void flower_print_erspan_opts(const char *name, struct rtattr *attr,
 	sprintf(strbuf, "%u:%u:%u:%u", ver, idx, dir, hwid);
 }
 
-static void flower_print_enc_parts(const char *name, const char *namefrm,
-				   struct rtattr *attr, char *key, char *mask)
+static void __attribute__((format(printf, 2, 0)))
+flower_print_enc_parts(const char *name, const char *namefrm,
+		       struct rtattr *attr, char *key, char *mask)
 {
 	char *key_token, *mask_token, *out;
 	int len;
@@ -2431,7 +2422,6 @@ static void flower_print_masked_u8(const char *name, struct rtattr *attr,
 {
 	const char *value_str = NULL;
 	__u8 value, mask;
-	SPRINT_BUF(namefrm);
 	SPRINT_BUF(out);
 	size_t done;
 
@@ -2452,8 +2442,7 @@ static void flower_print_masked_u8(const char *name, struct rtattr *attr,
 		sprintf(out + done, "/%d", mask);
 
 	print_nl();
-	sprintf(namefrm, "  %s %%s", name);
-	print_string(PRINT_ANY, name, namefrm, out);
+	print_string_name_value(name, out);
 }
 
 static void flower_print_u8(const char *name, struct rtattr *attr)
@@ -2463,14 +2452,11 @@ static void flower_print_u8(const char *name, struct rtattr *attr)
 
 static void flower_print_u32(const char *name, struct rtattr *attr)
 {
-	SPRINT_BUF(namefrm);
-
 	if (!attr)
 		return;
 
 	print_nl();
-	sprintf(namefrm, "  %s %%u", name);
-	print_uint(PRINT_ANY, name, namefrm, rta_getattr_u32(attr));
+	print_uint_name_value(name, rta_getattr_u32(attr));
 }
 
 static void flower_print_mpls_opt_lse(struct rtattr *lse)
-- 
2.30.2

[PATCH v2 iproute2-next 05/11] tc_util: fix clang warning in print_masked_type

[Stephen Hemminger]

Clang complains about passing a non-format string but the code here.
The old code was doing extra work the JSON case. JSON ignores one line mode.
This also fixes output format in oneline mode.

Fixes: 04b215015ba8 ("tc_util: introduce a function to print JSON/non-JSON masked numbers")
Cc: elibr@mellanox.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/tc_util.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 6d5eb754831a..67960d2c03d7 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -783,7 +783,6 @@ static void print_masked_type(__u32 type_max,
 			      const char *name, struct rtattr *attr,
 			      struct rtattr *mask_attr, bool newline)
 {
-	SPRINT_BUF(namefrm);
 	__u32 value, mask;
 	SPRINT_BUF(out);
 	size_t done;
@@ -795,26 +794,20 @@ static void print_masked_type(__u32 type_max,
 	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));
+		print_hu(PRINT_JSON, name, NULL, value);
 		if (mask != type_max) {
 			char mask_name[SPRINT_BSIZE-6];
 
 			sprintf(mask_name, "%s_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);
+			print_hu(PRINT_JSON, mask_name, NULL, mask);
 		}
 	} else {
 		done = sprintf(out, "%u", value);
 		if (mask != type_max)
 			sprintf(out + done, "/0x%x", mask);
-		if (newline)
-			print_string(PRINT_FP, NULL, "%s ", _SL_);
-		sprintf(namefrm, " %s %%s", name);
-		print_string(PRINT_ANY, name, namefrm, out);
+
+		print_nl();
+		print_string_name_value(name, out);
 	}
 }
 
-- 
2.30.2

[PATCH v2 iproute2-next 06/11] ipl2tp: fix clang warning

[Stephen Hemminger]

Clang complains about passing non-format string.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/ipl2tp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index 77bc3249c7ec..569723581ec2 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -191,8 +191,9 @@ static int delete_session(struct l2tp_parm *p)
 	return 0;
 }
 
-static void print_cookie(const char *name, const char *fmt,
-			 const uint8_t *cookie, int len)
+static void __attribute__((format(printf, 2, 0)))
+print_cookie(const char *name, const char *fmt,
+	     const uint8_t *cookie, int len)
 {
 	char abuf[32];
 	size_t n;
-- 
2.30.2

[PATCH v2 iproute2-next 07/11] can: fix clang warning

[Stephen Hemminger]

Fix warning about passing non-format string.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/iplink_can.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ip/iplink_can.c b/ip/iplink_can.c
index f4b375280cd7..67c913808083 100644
--- a/ip/iplink_can.c
+++ b/ip/iplink_can.c
@@ -330,8 +330,9 @@ static void can_print_nl_indent(void)
 	print_string(PRINT_FP, NULL, "%s", "\t ");
 }
 
-static void can_print_timing_min_max(const char *json_attr, const char *fp_attr,
-				     int min, int max)
+static void __attribute__((format(printf, 2, 0)))
+can_print_timing_min_max(const char *json_attr, const char *fp_attr,
+			 int min, int max)
 {
 	print_null(PRINT_FP, NULL, fp_attr, NULL);
 	open_json_object(json_attr);
-- 
2.30.2

[PATCH v2 iproute2-next 08/11] tipc: fix clang warning about empty format string

[Stephen Hemminger]

When calling json_print with json only use a NULL instead of
empty string.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tipc/link.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tipc/link.c b/tipc/link.c
index 9994ada2a367..53f49c8937db 100644
--- a/tipc/link.c
+++ b/tipc/link.c
@@ -332,7 +332,7 @@ static int _show_link_stat(const char *name, struct nlattr *attrs[],
 	open_json_object(NULL);
 
 	print_string(PRINT_ANY, "link", "Link <%s>\n", name);
-	print_string(PRINT_JSON, "state", "", NULL);
+	print_string(PRINT_JSON, "state", NULL, NULL);
 	open_json_array(PRINT_JSON, NULL);
 	if (attrs[TIPC_NLA_LINK_ACTIVE])
 		print_string(PRINT_ANY, NULL, "  %s", "ACTIVE");
-- 
2.30.2

[PATCH v2 iproute2-next 09/11] tunnel: fix clang warning

[Stephen Hemminger]

To fix clang warning about passing non-format string.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/tunnel.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/ip/tunnel.c b/ip/tunnel.c
index 88585cf3177b..f2632f43babf 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -298,14 +298,7 @@ void tnl_print_endpoint(const char *name, const struct rtattr *rta, int family)
 			value = "unknown";
 	}
 
-	if (is_json_context()) {
-		print_string(PRINT_JSON, name, NULL, value);
-	} else {
-		SPRINT_BUF(b1);
-
-		snprintf(b1, sizeof(b1), "%s %%s ", name);
-		print_string(PRINT_FP, NULL, b1, value);
-	}
+	print_string_name_value(name, value);
 }
 
 void tnl_print_gre_flags(__u8 proto,
-- 
2.30.2

[PATCH v2 iproute2-next 10/11] libbpf: fix clang warning about format non-literal

[Stephen Hemminger]

Add format attribute to the format string in print routines.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/bpf_libbpf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/bpf_libbpf.c b/lib/bpf_libbpf.c
index 50ef16bd4612..921716aec8c6 100644
--- a/lib/bpf_libbpf.c
+++ b/lib/bpf_libbpf.c
@@ -23,12 +23,14 @@
 
 #include "bpf_util.h"
 
-static int verbose_print(enum libbpf_print_level level, const char *format, va_list args)
+static int __attribute__((format(printf, 2, 0)))
+verbose_print(enum libbpf_print_level level, const char *format, va_list args)
 {
 	return vfprintf(stderr, format, args);
 }
 
-static int silent_print(enum libbpf_print_level level, const char *format, va_list args)
+static int __attribute__((format(printf, 2, 0)))
+silent_print(enum libbpf_print_level level, const char *format, va_list args)
 {
 	if (level > LIBBPF_WARN)
 		return 0;
-- 
2.30.2

[PATCH v2 iproute2-next 11/11] json_print: suppress clang format warning

[Stephen Hemminger]

Clang complains about using non-format string in print_color_tv.

The ideal fix would be to put format attribute on all the print_XXX functions
in json_print.h. But that leads to furthur complications because the existing
code may pass a NULL as format if the format is unused since the print
is being done only for JSON output.

The comprimise is to just disable the warning for the one place
it shows up.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/json_print.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/json_print.c b/lib/json_print.c
index e3a88375fe7c..741acdcff990 100644
--- a/lib/json_print.c
+++ b/lib/json_print.c
@@ -299,6 +299,13 @@ int print_color_null(enum output_type type,
 	return ret;
 }
 
+/*
+ * This function does take printf style argument but applying
+ * format attribute to causes more warnings since the print_XXX
+ * functions are used with NULL for format if unused.
+ */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
 int print_color_tv(enum output_type type,
 		   enum color_attr color,
 		   const char *key,
@@ -311,6 +318,7 @@ int print_color_tv(enum output_type type,
 
 	return print_color_float(type, color, key, fmt, time);
 }
+#pragma GCC diagnostic pop
 
 /* Print line separator (if not in JSON mode) */
 void print_nl(void)
-- 
2.30.2

Re: [PATCH v2 iproute2-next 00/11] clang warning fixes

[David Ahern]

On 1/11/22 10:54 AM, Stephen Hemminger wrote:
> This patch set makes iproute2-next main branch compile without warnings
> on Clang 11 (and probably later versions).
> 
> Still needs more testing before merge. There are likely to be some
> unnecessary output format changes from this.
> 

I think the tc patches are the only likely candidates. The
print_string_name_value conversion should be clean.

Jamal: As I recall you have a test suite for tc. Can you test this set?

Re: [PATCH v2 iproute2-next 00/11] clang warning fixes

[Stephen Hemminger]

On Sun, 16 Jan 2022 16:18:53 -0700
David Ahern <dsahern@gmail.com> wrote:

> On 1/11/22 10:54 AM, Stephen Hemminger wrote:
> > This patch set makes iproute2-next main branch compile without warnings
> > on Clang 11 (and probably later versions).
> > 
> > Still needs more testing before merge. There are likely to be some
> > unnecessary output format changes from this.
> >   
> 
> I think the tc patches are the only likely candidates. The
> print_string_name_value conversion should be clean.
> 
> Jamal: As I recall you have a test suite for tc. Can you test this set?

There was a blank after newline and with print_string_name_value that goes away.
Lets introduce something like print_nl_indent() to handle that

Re: [PATCH v2 iproute2-next 00/11] clang warning fixes

[Jamal Hadi Salim]

On 2022-01-16 18:18, David Ahern wrote:
> On 1/11/22 10:54 AM, Stephen Hemminger wrote:
>> This patch set makes iproute2-next main branch compile without warnings
>> on Clang 11 (and probably later versions).
>>
>> Still needs more testing before merge. There are likely to be some
>> unnecessary output format changes from this.
>>
> 
> I think the tc patches are the only likely candidates. The
> print_string_name_value conversion should be clean.
> 
> Jamal: As I recall you have a test suite for tc. Can you test this set?

We try to push, whenever we can, to kernel tdc tests. The Intel robot
should catch issues based on what we have there. If we make part of the
acceptance process (incumbent on people who create the patches) to
run those tests it would help getting cleaner submissions. Not sure
if we can have a bot doing this..

Punting to Victor(on Cc) to run the tests and double check if we
have test  cases that cover for these changes.

cheers,
jamal

Re: [PATCH v2 iproute2-next 00/11] clang warning fixes

[Victor Nogueira]

Hi,
Sorry for not responding sooner. I patched iproute2 and several
existing tests failed.
Example:
Test 696a: Add simple ct action

All test results:

1..1
not ok 1 696a - Add simple ct action
Could not match regex pattern. Verify command output:
total acts 1

action order 0: ct
zone 0 pipe
index 42 ref 1 bind 0

The problem is the additional new line added.

WIthout this patch:
https://patchwork.kernel.org/project/netdevbpf/patch/20220117175019.13993-6-stephen@networkplumber.org/
it the output of tc actions list action ct is:

total acts 1

action order 0: ct zone 0 pipe
index 42 ref 1 bind 0

With it it is:

total acts 1

action order 0: ct
zone 0 pipe
index 42 ref 1 bind 0

So I believe the problem is just formatting, however it still breaks some tests

cheers,
Victor

On 17 Jan 2022, at 12:29, Jamal Hadi Salim <jhs@mojatatu.com> wrote:

On 2022-01-16 18:18, David Ahern wrote:

On 1/11/22 10:54 AM, Stephen Hemminger wrote:

This patch set makes iproute2-next main branch compile without warnings
on Clang 11 (and probably later versions).

Still needs more testing before merge. There are likely to be some
unnecessary output format changes from this.

I think the tc patches are the only likely candidates. The
print_string_name_value conversion should be clean.
Jamal: As I recall you have a test suite for tc. Can you test this set?


We try to push, whenever we can, to kernel tdc tests. The Intel robot
should catch issues based on what we have there. If we make part of the
acceptance process (incumbent on people who create the patches) to
run those tests it would help getting cleaner submissions. Not sure
if we can have a bot doing this..

Punting to Victor(on Cc) to run the tests and double check if we
have test  cases that cover for these changes.

cheers,
jamal

Re: [PATCH v2 iproute2-next 00/11] clang warning fixes

[David Ahern]

On 1/20/22 5:48 AM, Victor Nogueira wrote:
> Hi,
> Sorry for not responding sooner. I patched iproute2 and several
> existing tests failed.
> Example:
> Test 696a: Add simple ct action
> 
> All test results:
> 
> 1..1
> not ok 1 696a - Add simple ct action
> Could not match regex pattern. Verify command output:
> total acts 1
> 
> action order 0: ct
> zone 0 pipe
> index 42 ref 1 bind 0
> 
> The problem is the additional new line added.
> 
> WIthout this patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/20220117175019.13993-6-stephen@networkplumber.org/
> it the output of tc actions list action ct is:
> 
> total acts 1
> 
> action order 0: ct zone 0 pipe
> index 42 ref 1 bind 0
> 
> With it it is:
> 
> total acts 1
> 
> action order 0: ct
> zone 0 pipe
> index 42 ref 1 bind 0
> 
> So I believe the problem is just formatting, however it still breaks some tests
> 

Thanks, Victor. Hopefully that is addressed in v3 of the set.

Re: [PATCH v2 iproute2-next 00/11] clang warning fixes

[Victor Nogueira]

On Thu, Jan 20, 2022 at 4:20 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 1/20/22 5:48 AM, Victor Nogueira wrote:
> > Hi,
> > Sorry for not responding sooner. I patched iproute2 and several
> > existing tests failed.
> > Example:
> > Test 696a: Add simple ct action
> >
> > All test results:
> >
> > 1..1
> > not ok 1 696a - Add simple ct action
> > Could not match regex pattern. Verify command output:
> > total acts 1
> >
> > action order 0: ct
> > zone 0 pipe
> > index 42 ref 1 bind 0
> >
> > The problem is the additional new line added.
> >
> > WIthout this patch:
> > https://patchwork.kernel.org/project/netdevbpf/patch/20220117175019.13993-6-stephen@networkplumber.org/
> > it the output of tc actions list action ct is:
> >
> > total acts 1
> >
> > action order 0: ct zone 0 pipe
> > index 42 ref 1 bind 0
> >
> > With it it is:
> >
> > total acts 1
> >
> > action order 0: ct
> > zone 0 pipe
> > index 42 ref 1 bind 0
> >
> > So I believe the problem is just formatting, however it still breaks some tests
> >
>
> Thanks, Victor. Hopefully that is addressed in v3 of the set.
>

Actually, I tested using v3 patches.
Sorry, I should've said that in the previous email.

cheers,
Victor