[PATCH iproute2-next 00/15] Add a tool for configuration of DCB

[PATCH iproute2-next 00/15] Add a tool for configuration of DCB

[Petr Machata]

The Linux DCB interface allows configuration of a broad range of
hardware-specific attributes, such as TC scheduling, flow control, per-port
buffer configuration, TC rate, etc.

Currently a common libre tool for configuration of DCB is OpenLLDP. This
suite contains a daemon that uses Linux DCB interface to configure HW
according to the DCB TLVs exchanged over an interface. The daemon can also
be controlled by a client, through which the user can adjust and view the
configuration. The downside of using OpenLLDP is that it is somewhat
heavyweight and difficult to use in scripts, and does not support
extensions such as buffer and rate commands.

For access to many HW features, one would be perfectly fine with a
fire-and-forget tool along the lines of "ip" or "tc". For scripting in
particular, this would be ideal. This author is aware of one such tool,
mlnx_qos from Mellanox OFED scripts collection[1].

The downside here is that the tool is very verbose, the command line
language is awkward to use, it is not packaged in Linux distros, and
generally has the appearance of a very vendor-specific tool, despite not
being one.

This patchset addresses the above issues by providing a seed of a clean,
well-documented, easily usable, extensible fire-and-forget tool for DCB
configuration:

    # dcb ets set dev eni1np1 \
                  tc-tsa all:strict 0:ets 1:ets 2:ets \
		  tc-bw all:0 0:33 1:33 2:34

    # dcb ets show dev eni1np1 tc-tsa tc-bw
    tc-tsa 0:ets 1:ets 2:ets 3:strict 4:strict 5:strict 6:strict 7:strict
    tc-bw 0:33 1:33 2:34 3:0 4:0 5:0 6:0 7:0

    # dcb ets set dev eni1np1 tc-bw 1:30 2:37

    # dcb -j ets show dev eni1np1 | jq '.["tc-bw"]["2"]'
    37

The patchset proceeds as follows:

- Many tools in iproute2 have an option to work in batch mode, where the
  commands to run are given in a file. The code to handle batching is
  largely the same independent of the tool in question. In patch #1, add a
  helper to handle the batching, and migrate individual tools to use it.

- A number of configuration options come in a form of an on-off switch.
  This in turn can be considered a special case of parsing one of a given
  set of strings. Currently each tool open-codes the logic to parse the
  on-off toggle. And on top of the on-off parsing, tools have logic to set
  or unset a flag according to the keyword parsed.

  In patches #2-#7, extract helpers to parse one of a number of strings, on
  top of which build an on-off parser, on top of which build a flag set /
  unset handler. Then migrate all known instances of this code over to the
  new helpers.

- The DCB tool is built on top of libmnl. Several routines will be
  basically the same in DCB as they are currently in devlink. In patches
  #8-#10, extract them to a new module, mnl_utils, for easy reuse.

- Much of DCB is built around arrays. A syntax similar to the iplink_vlan's
  ingress-qos-map / egress-qos-map is very handy for describing changes
  done to such arrays. Therefore in patch #11, extract a helper,
  parse_mapping(), which manages parsing of key-value arrays. In patch #12,
  fix a buglet in the helper, and in patch #13, extend it to allow setting
  of all array elements in one go.

- In patch #14, add a skeleton of "dcb", which contains common helpers and
  dispatches to subtools for handling of individual objects. The skeleton
  is empty as of this patch.

  In patch #15, add "dcb_ets", a module for handling of specifically DCB
  ETS objects.

The intention is to gradually add handlers for at least PFC, APP, peer
configuration, buffers and rates.

[1] https://github.com/Mellanox/mlnx-tools/tree/master/ofed_scripts

Petr Machata (15):
  Unify batch processing across tools
  lib: Add parse_one_of(), parse_on_off()
  bridge: link: Port over to parse_on_off()
  lib: Add parse_flag_on_off(), set_flag()
  ip: iplink: Convert to use parse_on_off(), parse_flag_on_off()
  ip: iplink_vlan: Port over to parse_flag_on_off()
  ip: iplink_bridge_slave: Port over to parse_on_off()
  lib: Extract from devlink/mnlg a helper, mnlu_socket_open()
  lib: Extract from devlink/mnlg a helper, mnlu_msg_prepare()
  lib: Extract from devlink/mnlg a helper, mnlu_socket_recv_run()
  lib: Extract from iplink_vlan a helper to parse key:value arrays
  lib: parse_mapping: Update argc, argv on error
  lib: parse_mapping: Recognize a keyword "all"
  Add skeleton of a new tool, dcb
  dcb: Add a subtool for the DCB ETS object

 Makefile                 |   2 +-
 bridge/bridge.c          |  38 +---
 bridge/link.c            |  79 ++++---
 dcb/Makefile             |  24 +++
 dcb/dcb.c                | 379 +++++++++++++++++++++++++++++++++
 dcb/dcb.h                |  36 ++++
 dcb/dcb_ets.c            | 450 +++++++++++++++++++++++++++++++++++++++
 devlink/Makefile         |   2 +-
 devlink/devlink.c        |  41 +---
 devlink/mnlg.c           |  93 ++------
 include/mnl_utils.h      |  11 +
 include/utils.h          |  21 ++
 ip/ip.c                  |  46 +---
 ip/iplink.c              | 182 ++++++----------
 ip/iplink_bridge_slave.c |  12 +-
 ip/iplink_vlan.c         |  86 +++-----
 ip/ipmacsec.c            |  52 +----
 lib/Makefile             |   2 +-
 lib/mnl_utils.c          | 115 ++++++++++
 lib/utils.c              | 114 ++++++++++
 man/man8/dcb-ets.8       | 185 ++++++++++++++++
 man/man8/dcb.8           | 114 ++++++++++
 rdma/rdma.c              |  38 +---
 tc/tc.c                  |  38 +---
 24 files changed, 1652 insertions(+), 508 deletions(-)
 create mode 100644 dcb/Makefile
 create mode 100644 dcb/dcb.c
 create mode 100644 dcb/dcb.h
 create mode 100644 dcb/dcb_ets.c
 create mode 100644 include/mnl_utils.h
 create mode 100644 lib/mnl_utils.c
 create mode 100644 man/man8/dcb-ets.8
 create mode 100644 man/man8/dcb.8

-- 
2.25.1

[PATCH iproute2-next 01/15] Unify batch processing across tools

[Petr Machata]

The code for handling batches is largely the same across iproute2 tools.
Extract a helper to handle the batch, and adjust the tools to dispatch to
this helper. Sandwitch the invocation between prologue / epilogue code
specific for each tool.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 bridge/bridge.c   | 38 +++++++-------------------------------
 devlink/devlink.c | 41 +++++++----------------------------------
 include/utils.h   |  3 +++
 ip/ip.c           | 46 ++++++++++------------------------------------
 lib/utils.c       | 40 ++++++++++++++++++++++++++++++++++++++++
 rdma/rdma.c       | 38 +++++++-------------------------------
 tc/tc.c           | 38 +++++++-------------------------------
 7 files changed, 81 insertions(+), 163 deletions(-)

diff --git a/bridge/bridge.c b/bridge/bridge.c
index 453d689732bd..8f691cfdd466 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -77,20 +77,14 @@ static int do_cmd(const char *argv0, int argc, char **argv)
 	return -1;
 }
 
-static int batch(const char *name)
+static int br_batch_cmd(int argc, char *argv[], void *data)
 {
-	char *line = NULL;
-	size_t len = 0;
-	int ret = EXIT_SUCCESS;
+	return do_cmd(argv[0], argc, argv);
+}
 
-	if (name && strcmp(name, "-") != 0) {
-		if (freopen(name, "r", stdin) == NULL) {
-			fprintf(stderr,
-				"Cannot open file \"%s\" for reading: %s\n",
-				name, strerror(errno));
-			return EXIT_FAILURE;
-		}
-	}
+static int batch(const char *name)
+{
+	int ret;
 
 	if (rtnl_open(&rth, 0) < 0) {
 		fprintf(stderr, "Cannot open rtnetlink\n");
@@ -99,25 +93,7 @@ static int batch(const char *name)
 
 	rtnl_set_strict_dump(&rth);
 
-	cmdlineno = 0;
-	while (getcmdline(&line, &len, stdin) != -1) {
-		char *largv[100];
-		int largc;
-
-		largc = makeargs(line, largv, 100);
-		if (largc == 0)
-			continue;       /* blank line */
-
-		if (do_cmd(largv[0], largc, largv)) {
-			fprintf(stderr, "Command failed %s:%d\n",
-				name, cmdlineno);
-			ret = EXIT_FAILURE;
-			if (!force)
-				break;
-		}
-	}
-	if (line)
-		free(line);
+	ret = do_batch(name, force, br_batch_cmd, NULL);
 
 	rtnl_close(&rth);
 	return ret;
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 007677a5c564..7dba42c602b3 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -7745,43 +7745,16 @@ static void dl_free(struct dl *dl)
 	free(dl);
 }
 
-static int dl_batch(struct dl *dl, const char *name, bool force)
+static int dl_batch_cmd(int argc, char *argv[], void *data)
 {
-	char *line = NULL;
-	size_t len = 0;
-	int ret = EXIT_SUCCESS;
-
-	if (name && strcmp(name, "-") != 0) {
-		if (freopen(name, "r", stdin) == NULL) {
-			fprintf(stderr,
-				"Cannot open file \"%s\" for reading: %s\n",
-				name, strerror(errno));
-			return EXIT_FAILURE;
-		}
-	}
-
-	cmdlineno = 0;
-	while (getcmdline(&line, &len, stdin) != -1) {
-		char *largv[100];
-		int largc;
-
-		largc = makeargs(line, largv, 100);
-		if (!largc)
-			continue;	/* blank line */
-
-		if (dl_cmd(dl, largc, largv)) {
-			fprintf(stderr, "Command failed %s:%d\n",
-				name, cmdlineno);
-			ret = EXIT_FAILURE;
-			if (!force)
-				break;
-		}
-	}
+	struct dl *dl = data;
 
-	if (line)
-		free(line);
+	return dl_cmd(dl, argc, argv);
+}
 
-	return ret;
+static int dl_batch(struct dl *dl, const char *name, bool force)
+{
+	return do_batch(name, force, dl_batch_cmd, dl);
 }
 
 int main(int argc, char **argv)
diff --git a/include/utils.h b/include/utils.h
index 7041c4612e46..085b17b1f6e3 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -322,4 +322,7 @@ int get_time64(__s64 *time, const char *str);
 char *sprint_time(__u32 time, char *buf);
 char *sprint_time64(__s64 time, char *buf);
 
+int do_batch(const char *name, bool force,
+	     int (*cmd)(int argc, char *argv[], void *user), void *user);
+
 #endif /* __UTILS_H__ */
diff --git a/ip/ip.c b/ip/ip.c
index ac4450235370..5e31957f2420 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -121,56 +121,30 @@ static int do_cmd(const char *argv0, int argc, char **argv)
 	return EXIT_FAILURE;
 }
 
-static int batch(const char *name)
+static int ip_batch_cmd(int argc, char *argv[], void *data)
 {
-	char *line = NULL;
-	size_t len = 0;
-	int ret = EXIT_SUCCESS;
-	int orig_family = preferred_family;
+	const int *orig_family = data;
 
-	batch_mode = 1;
+	preferred_family = *orig_family;
+	return do_cmd(argv[0], argc, argv);
+}
 
-	if (name && strcmp(name, "-") != 0) {
-		if (freopen(name, "r", stdin) == NULL) {
-			fprintf(stderr,
-				"Cannot open file \"%s\" for reading: %s\n",
-				name, strerror(errno));
-			return EXIT_FAILURE;
-		}
-	}
+static int batch(const char *name)
+{
+	int orig_family = preferred_family;
+	int ret;
 
 	if (rtnl_open(&rth, 0) < 0) {
 		fprintf(stderr, "Cannot open rtnetlink\n");
 		return EXIT_FAILURE;
 	}
 
-	cmdlineno = 0;
-	while (getcmdline(&line, &len, stdin) != -1) {
-		char *largv[100];
-		int largc;
-
-		preferred_family = orig_family;
-
-		largc = makeargs(line, largv, 100);
-		if (largc == 0)
-			continue;	/* blank line */
-
-		if (do_cmd(largv[0], largc, largv)) {
-			fprintf(stderr, "Command failed %s:%d\n",
-				name, cmdlineno);
-			ret = EXIT_FAILURE;
-			if (!force)
-				break;
-		}
-	}
-	if (line)
-		free(line);
+	ret = do_batch(name, force, ip_batch_cmd, &orig_family);
 
 	rtnl_close(&rth);
 	return ret;
 }
 
-
 int main(int argc, char **argv)
 {
 	char *basename;
diff --git a/lib/utils.c b/lib/utils.c
index c98021d6ecad..9815e328c9e0 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1695,3 +1695,43 @@ char *sprint_time64(__s64 time, char *buf)
 	print_time64(buf, SPRINT_BSIZE-1, time);
 	return buf;
 }
+
+int do_batch(const char *name, bool force,
+	     int (*cmd)(int argc, char *argv[], void *data), void *data)
+{
+	char *line = NULL;
+	size_t len = 0;
+	int ret = EXIT_SUCCESS;
+
+	if (name && strcmp(name, "-") != 0) {
+		if (freopen(name, "r", stdin) == NULL) {
+			fprintf(stderr,
+				"Cannot open file \"%s\" for reading: %s\n",
+				name, strerror(errno));
+			return EXIT_FAILURE;
+		}
+	}
+
+	cmdlineno = 0;
+	while (getcmdline(&line, &len, stdin) != -1) {
+		char *largv[100];
+		int largc;
+
+		largc = makeargs(line, largv, 100);
+		if (!largc)
+			continue;	/* blank line */
+
+		if (cmd(largc, largv, data)) {
+			fprintf(stderr, "Command failed %s:%d\n",
+				name, cmdlineno);
+			ret = EXIT_FAILURE;
+			if (!force)
+				break;
+		}
+	}
+
+	if (line)
+		free(line);
+
+	return ret;
+}
diff --git a/rdma/rdma.c b/rdma/rdma.c
index 9ea2d17ffe9e..8dc2d3e344be 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -41,40 +41,16 @@ static int rd_cmd(struct rd *rd, int argc, char **argv)
 	return rd_exec_cmd(rd, cmds, "object");
 }
 
-static int rd_batch(struct rd *rd, const char *name, bool force)
+static int rd_batch_cmd(int argc, char *argv[], void *data)
 {
-	char *line = NULL;
-	size_t len = 0;
-	int ret = 0;
-
-	if (name && strcmp(name, "-") != 0) {
-		if (!freopen(name, "r", stdin)) {
-			pr_err("Cannot open file \"%s\" for reading: %s\n",
-			       name, strerror(errno));
-			return errno;
-		}
-	}
+	struct rd *rd = data;
 
-	cmdlineno = 0;
-	while (getcmdline(&line, &len, stdin) != -1) {
-		char *largv[512];
-		int largc;
-
-		largc = makeargs(line, largv, ARRAY_SIZE(largv));
-		if (!largc)
-			continue;	/* blank line */
-
-		ret = rd_cmd(rd, largc, largv);
-		if (ret) {
-			pr_err("Command failed %s:%d\n", name, cmdlineno);
-			if (!force)
-				break;
-		}
-	}
-
-	free(line);
+	return rd_cmd(rd, argc, argv);
+}
 
-	return ret;
+static int rd_batch(struct rd *rd, const char *name, bool force)
+{
+	return do_batch(name, force, rd_batch_cmd, rd);
 }
 
 static int rd_init(struct rd *rd, char *filename)
diff --git a/tc/tc.c b/tc/tc.c
index 5d57054b45fb..01fe58d06202 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -231,22 +231,16 @@ static int do_cmd(int argc, char **argv)
 	return -1;
 }
 
+static int tc_batch_cmd(int argc, char *argv[], void *data)
+{
+	return do_cmd(argc, argv);
+}
+
 static int batch(const char *name)
 {
-	char *line = NULL;
-	size_t len = 0;
-	int ret = 0;
+	int ret;
 
 	batch_mode = 1;
-	if (name && strcmp(name, "-") != 0) {
-		if (freopen(name, "r", stdin) == NULL) {
-			fprintf(stderr,
-				"Cannot open file \"%s\" for reading: %s\n",
-				name, strerror(errno));
-			return -1;
-		}
-	}
-
 	tc_core_init();
 
 	if (rtnl_open(&rth, 0) < 0) {
@@ -254,26 +248,8 @@ static int batch(const char *name)
 		return -1;
 	}
 
-	cmdlineno = 0;
-	while (getcmdline(&line, &len, stdin) != -1) {
-		char *largv[100];
-		int largc;
-
-		largc = makeargs(line, largv, 100);
-		if (largc == 0)
-			continue;	/* blank line */
-
-		if (do_cmd(largc, largv)) {
-			fprintf(stderr, "Command failed %s:%d\n",
-				name, cmdlineno);
-			ret = 1;
-			if (!force)
-				break;
-		}
-		fflush(stdout);
-	}
+	ret = do_batch(name, force, tc_batch_cmd, NULL);
 
-	free(line);
 	rtnl_close(&rth);
 	return ret;
 }
-- 
2.25.1

[PATCH iproute2-next 02/15] lib: Add parse_one_of(), parse_on_off()

[Petr Machata]

Take from the macsec code parse_one_of() and adapt so that it passes the
primary result as the main return value, and error result through a
pointer. That is the simplest way to make the code reusable across data
types without introducing extra magic.

Also from macsec take the specialization of parse_one_of() for parsing
specifically the strings "off" and "on".

Convert the macsec code to the new helpers.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 include/utils.h |  4 ++++
 ip/ipmacsec.c   | 52 +++++++++++--------------------------------------
 lib/utils.c     | 28 ++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 085b17b1f6e3..bd62cdcd7122 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -325,4 +325,8 @@ char *sprint_time64(__s64 time, char *buf);
 int do_batch(const char *name, bool force,
 	     int (*cmd)(int argc, char *argv[], void *user), void *user);
 
+int parse_one_of(const char *msg, const char *realval, const char * const *list,
+		 size_t len, int *p_err);
+int parse_on_off(const char *msg, const char *realval, int *p_err);
+
 #endif /* __UTILS_H__ */
diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index 18289ecd6d9e..bf48e8b5d0b2 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -23,8 +23,6 @@
 #include "ll_map.h"
 #include "libgenl.h"
 
-static const char * const values_on_off[] = { "off", "on" };
-
 static const char * const validate_str[] = {
 	[MACSEC_VALIDATE_DISABLED] = "disabled",
 	[MACSEC_VALIDATE_CHECK] = "check",
@@ -108,25 +106,6 @@ static void ipmacsec_usage(void)
 	exit(-1);
 }
 
-static int one_of(const char *msg, const char *realval, const char * const *list,
-		  size_t len, int *index)
-{
-	int i;
-
-	for (i = 0; i < len; i++) {
-		if (matches(realval, list[i]) == 0) {
-			*index = i;
-			return 0;
-		}
-	}
-
-	fprintf(stderr, "Error: argument of \"%s\" must be one of ", msg);
-	for (i = 0; i < len; i++)
-		fprintf(stderr, "\"%s\", ", list[i]);
-	fprintf(stderr, "not \"%s\"\n", realval);
-	return -1;
-}
-
 static int get_an(__u8 *val, const char *arg)
 {
 	int ret = get_u8(val, arg, 0);
@@ -559,8 +538,7 @@ static int do_offload(enum cmd c, int argc, char **argv)
 	if (argc == 0)
 		ipmacsec_usage();
 
-	ret = one_of("offload", *argv, offload_str, ARRAY_SIZE(offload_str),
-		     (int *)&offload);
+	offload = parse_one_of("offload", *argv, offload_str, ARRAY_SIZE(offload_str), &ret);
 	if (ret)
 		ipmacsec_usage();
 
@@ -1334,8 +1312,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			int i;
 
-			ret = one_of("encrypt", *argv, values_on_off,
-				     ARRAY_SIZE(values_on_off), &i);
+			i = parse_on_off("encrypt", *argv, &ret);
 			if (ret != 0)
 				return ret;
 			addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_ENCRYPT, i);
@@ -1343,8 +1320,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			int i;
 
-			ret = one_of("send_sci", *argv, values_on_off,
-				     ARRAY_SIZE(values_on_off), &i);
+			i = parse_on_off("send_sci", *argv, &ret);
 			if (ret != 0)
 				return ret;
 			send_sci = i;
@@ -1354,8 +1330,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			int i;
 
-			ret = one_of("end_station", *argv, values_on_off,
-				     ARRAY_SIZE(values_on_off), &i);
+			i = parse_on_off("end_station", *argv, &ret);
 			if (ret != 0)
 				return ret;
 			es = i;
@@ -1364,8 +1339,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			int i;
 
-			ret = one_of("scb", *argv, values_on_off,
-				     ARRAY_SIZE(values_on_off), &i);
+			i = parse_on_off("scb", *argv, &ret);
 			if (ret != 0)
 				return ret;
 			scb = i;
@@ -1374,8 +1348,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			int i;
 
-			ret = one_of("protect", *argv, values_on_off,
-				     ARRAY_SIZE(values_on_off), &i);
+			i = parse_on_off("protect", *argv, &ret);
 			if (ret != 0)
 				return ret;
 			addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_PROTECT, i);
@@ -1383,8 +1356,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			int i;
 
-			ret = one_of("replay", *argv, values_on_off,
-				     ARRAY_SIZE(values_on_off), &i);
+			i = parse_on_off("replay", *argv, &ret);
 			if (ret != 0)
 				return ret;
 			replay_protect = !!i;
@@ -1395,9 +1367,8 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 				invarg("expected replay window size", *argv);
 		} else if (strcmp(*argv, "validate") == 0) {
 			NEXT_ARG();
-			ret = one_of("validate", *argv,
-				     validate_str, ARRAY_SIZE(validate_str),
-				     (int *)&validate);
+			validate = parse_one_of("validate", *argv, validate_str,
+						ARRAY_SIZE(validate_str), &ret);
 			if (ret != 0)
 				return ret;
 			addattr8(n, MACSEC_BUFLEN,
@@ -1411,9 +1382,8 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 				invarg("expected an { 0..3 }", *argv);
 		} else if (strcmp(*argv, "offload") == 0) {
 			NEXT_ARG();
-			ret = one_of("offload", *argv,
-				     offload_str, ARRAY_SIZE(offload_str),
-				     (int *)&offload);
+			offload = parse_one_of("offload", *argv, offload_str,
+					       ARRAY_SIZE(offload_str), &ret);
 			if (ret != 0)
 				return ret;
 			addattr8(n, MACSEC_BUFLEN,
diff --git a/lib/utils.c b/lib/utils.c
index 9815e328c9e0..930877ae0f0d 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1735,3 +1735,31 @@ int do_batch(const char *name, bool force,
 
 	return ret;
 }
+
+int parse_one_of(const char *msg, const char *realval, const char * const *list,
+		 size_t len, int *p_err)
+{
+	int i;
+
+	for (i = 0; i < len; i++) {
+		if (list[i] && matches(realval, list[i]) == 0) {
+			*p_err = 0;
+			return i;
+		}
+	}
+
+	fprintf(stderr, "Error: argument of \"%s\" must be one of ", msg);
+	for (i = 0; i < len; i++)
+		if (list[i])
+			fprintf(stderr, "\"%s\", ", list[i]);
+	fprintf(stderr, "not \"%s\"\n", realval);
+	*p_err = -EINVAL;
+	return 0;
+}
+
+int parse_on_off(const char *msg, const char *realval, int *p_err)
+{
+	static const char * const values_on_off[] = { "off", "on" };
+
+	return parse_one_of(msg, realval, values_on_off, ARRAY_SIZE(values_on_off), p_err);
+}
-- 
2.25.1

[PATCH iproute2-next 03/15] bridge: link: Port over to parse_on_off()

[Petr Machata]

Convert bridge/link.c from a custom on_off parser to the new global one.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 bridge/link.c | 79 ++++++++++++++++++++++++---------------------------
 1 file changed, 37 insertions(+), 42 deletions(-)

diff --git a/bridge/link.c b/bridge/link.c
index 3bc7af209b8b..fa6eda849b32 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -275,22 +275,6 @@ static void usage(void)
 	exit(-1);
 }
 
-static bool on_off(char *arg, __s8 *attr, char *val)
-{
-	if (strcmp(val, "on") == 0)
-		*attr = 1;
-	else if (strcmp(val, "off") == 0)
-		*attr = 0;
-	else {
-		fprintf(stderr,
-			"Error: argument of \"%s\" must be \"on\" or \"off\"\n",
-			arg);
-		return false;
-	}
-
-	return true;
-}
-
 static int brlink_modify(int argc, char **argv)
 {
 	struct {
@@ -323,6 +307,7 @@ static int brlink_modify(int argc, char **argv)
 	__s16 mode = -1;
 	__u16 flags = 0;
 	struct rtattr *nest;
+	int ret;
 
 	while (argc > 0) {
 		if (strcmp(*argv, "dev") == 0) {
@@ -330,40 +315,49 @@ static int brlink_modify(int argc, char **argv)
 			d = *argv;
 		} else if (strcmp(*argv, "guard") == 0) {
 			NEXT_ARG();
-			if (!on_off("guard", &bpdu_guard, *argv))
-				return -1;
+			bpdu_guard = parse_on_off("guard", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "hairpin") == 0) {
 			NEXT_ARG();
-			if (!on_off("hairpin", &hairpin, *argv))
-				return -1;
+			hairpin = parse_on_off("hairpin", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "fastleave") == 0) {
 			NEXT_ARG();
-			if (!on_off("fastleave", &fast_leave, *argv))
-				return -1;
+			fast_leave = parse_on_off("fastleave", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "root_block") == 0) {
 			NEXT_ARG();
-			if (!on_off("root_block", &root_block, *argv))
-				return -1;
+			root_block = parse_on_off("root_block", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "learning") == 0) {
 			NEXT_ARG();
-			if (!on_off("learning", &learning, *argv))
-				return -1;
+			learning = parse_on_off("learning", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "learning_sync") == 0) {
 			NEXT_ARG();
-			if (!on_off("learning_sync", &learning_sync, *argv))
-				return -1;
+			learning_sync = parse_on_off("learning_sync", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "flood") == 0) {
 			NEXT_ARG();
-			if (!on_off("flood", &flood, *argv))
-				return -1;
+			flood = parse_on_off("flood", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "mcast_flood") == 0) {
 			NEXT_ARG();
-			if (!on_off("mcast_flood", &mcast_flood, *argv))
-				return -1;
+			mcast_flood = parse_on_off("mcast_flood", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "mcast_to_unicast") == 0) {
 			NEXT_ARG();
-			if (!on_off("mcast_to_unicast", &mcast_to_unicast, *argv))
-				return -1;
+			mcast_to_unicast = parse_on_off("mcast_to_unicast", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "cost") == 0) {
 			NEXT_ARG();
 			cost = atoi(*argv);
@@ -404,18 +398,19 @@ static int brlink_modify(int argc, char **argv)
 			flags |= BRIDGE_FLAGS_MASTER;
 		} else if (strcmp(*argv, "neigh_suppress") == 0) {
 			NEXT_ARG();
-			if (!on_off("neigh_suppress", &neigh_suppress,
-				    *argv))
-				return -1;
+			neigh_suppress = parse_on_off("neigh_suppress", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "vlan_tunnel") == 0) {
 			NEXT_ARG();
-			if (!on_off("vlan_tunnel", &vlan_tunnel,
-				    *argv))
-				return -1;
+			vlan_tunnel = parse_on_off("vlan_tunnel", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "isolated") == 0) {
 			NEXT_ARG();
-			if (!on_off("isolated", &isolated, *argv))
-				return -1;
+			isolated = parse_on_off("isolated", *argv, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "backup_port") == 0) {
 			NEXT_ARG();
 			backup_port_idx = ll_name_to_index(*argv);
-- 
2.25.1

[PATCH iproute2-next 04/15] lib: Add parse_flag_on_off(), set_flag()

[Petr Machata]

Some iplink code makes a heavy use of code that sets or unsets a certain
flag depending on whether "on" or "off" of specified. Extract this logic
into a new function, parse_flag_on_off(). The bit that sets or clears a
flag will be useful separately, so add it to a named function, set_flag().

Signed-off-by: Petr Machata <me@pmachata.org>
---
 include/utils.h | 10 ++++++++++
 lib/utils.c     | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/utils.h b/include/utils.h
index bd62cdcd7122..681110fcf8af 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -325,8 +325,18 @@ char *sprint_time64(__s64 time, char *buf);
 int do_batch(const char *name, bool force,
 	     int (*cmd)(int argc, char *argv[], void *user), void *user);
 
+static inline void set_flag(unsigned int *p_flags, unsigned int flag, bool on)
+{
+	if (on)
+		*p_flags |= flag;
+	else
+		*p_flags &= ~flag;
+}
+
 int parse_one_of(const char *msg, const char *realval, const char * const *list,
 		 size_t len, int *p_err);
 int parse_on_off(const char *msg, const char *realval, int *p_err);
+void parse_flag_on_off(const char *msg, const char *realval,
+		       unsigned int *p_flags, unsigned int flag, int *p_ret);
 
 #endif /* __UTILS_H__ */
diff --git a/lib/utils.c b/lib/utils.c
index 930877ae0f0d..fb25c64d36ff 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1763,3 +1763,14 @@ int parse_on_off(const char *msg, const char *realval, int *p_err)
 
 	return parse_one_of(msg, realval, values_on_off, ARRAY_SIZE(values_on_off), p_err);
 }
+
+void parse_flag_on_off(const char *msg, const char *realval,
+		       unsigned int *p_flags, unsigned int flag, int *p_ret)
+{
+	int on_off = parse_on_off(msg, realval, p_ret);
+
+	if (*p_ret)
+		return;
+
+	set_flag(p_flags, flag, on_off);
+}
-- 
2.25.1

[PATCH iproute2-next 05/15] ip: iplink: Convert to use parse_on_off(), parse_flag_on_off()

[Petr Machata]

Invoke parse_flag_on_off() instead of rolling a custom function. Several
places have the on/off logic reversed vs. how the flag is specified (e.g.
IFF_NOARP vs. "arp" on command line). For those, invoke parse_on_off() and
then set_flag() with a negated value.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 ip/iplink.c | 182 +++++++++++++++++++---------------------------------
 1 file changed, 66 insertions(+), 116 deletions(-)

diff --git a/ip/iplink.c b/ip/iplink.c
index 5ec33a98b96e..422e2fdccde5 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -135,14 +135,6 @@ static void usage(void)
 	iplink_usage();
 }
 
-static int on_off(const char *msg, const char *realval)
-{
-	fprintf(stderr,
-		"Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n",
-		msg, realval);
-	return -1;
-}
-
 static void *BODY;		/* cached dlopen(NULL) handle */
 static struct link_util *linkutil_list;
 
@@ -351,6 +343,7 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 	int len, argc = *argcp;
 	char **argv = *argvp;
 	struct rtattr *vfinfo;
+	int ret;
 
 	tivt.min_tx_rate = -1;
 	tivt.max_tx_rate = -1;
@@ -463,12 +456,9 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 			struct ifla_vf_spoofchk ivs;
 
 			NEXT_ARG();
-			if (matches(*argv, "on") == 0)
-				ivs.setting = 1;
-			else if (matches(*argv, "off") == 0)
-				ivs.setting = 0;
-			else
-				return on_off("spoofchk", *argv);
+			ivs.setting = parse_on_off("spoofchk", *argv, &ret);
+			if (ret)
+				return ret;
 			ivs.vf = vf;
 			addattr_l(&req->n, sizeof(*req), IFLA_VF_SPOOFCHK,
 				  &ivs, sizeof(ivs));
@@ -477,12 +467,9 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 			struct ifla_vf_rss_query_en ivs;
 
 			NEXT_ARG();
-			if (matches(*argv, "on") == 0)
-				ivs.setting = 1;
-			else if (matches(*argv, "off") == 0)
-				ivs.setting = 0;
-			else
-				return on_off("query_rss", *argv);
+			ivs.setting = parse_on_off("query_rss", *argv, &ret);
+			if (ret)
+				return ret;
 			ivs.vf = vf;
 			addattr_l(&req->n, sizeof(*req), IFLA_VF_RSS_QUERY_EN,
 				  &ivs, sizeof(ivs));
@@ -491,12 +478,9 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 			struct ifla_vf_trust ivt;
 
 			NEXT_ARG();
-			if (matches(*argv, "on") == 0)
-				ivt.setting = 1;
-			else if (matches(*argv, "off") == 0)
-				ivt.setting = 0;
-			else
-				invarg("Invalid \"trust\" value\n", *argv);
+			ivt.setting = parse_on_off("trust", *argv, &ret);
+			if (ret)
+				return ret;
 			ivt.vf = vf;
 			addattr_l(&req->n, sizeof(*req), IFLA_VF_TRUST,
 				  &ivt, sizeof(ivt));
@@ -594,6 +578,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 	int index = 0;
 	int group = -1;
 	int addr_len = 0;
+	int err;
 
 	ret = argc;
 
@@ -687,62 +672,53 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_MULTICAST;
 
-			if (strcmp(*argv, "on") == 0)
-				req->i.ifi_flags |= IFF_MULTICAST;
-			else if (strcmp(*argv, "off") == 0)
-				req->i.ifi_flags &= ~IFF_MULTICAST;
-			else
-				return on_off("multicast", *argv);
+			parse_flag_on_off("multicast", *argv, &req->i.ifi_flags,
+					  IFF_MULTICAST, &err);
+			if (err)
+				return err;
 		} else if (strcmp(*argv, "allmulticast") == 0) {
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_ALLMULTI;
 
-			if (strcmp(*argv, "on") == 0)
-				req->i.ifi_flags |= IFF_ALLMULTI;
-			else if (strcmp(*argv, "off") == 0)
-				req->i.ifi_flags &= ~IFF_ALLMULTI;
-			else
-				return on_off("allmulticast", *argv);
+			parse_flag_on_off("allmulticast", *argv, &req->i.ifi_flags,
+					  IFF_ALLMULTI, &err);
+			if (err)
+				return err;
 		} else if (strcmp(*argv, "promisc") == 0) {
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_PROMISC;
 
-			if (strcmp(*argv, "on") == 0)
-				req->i.ifi_flags |= IFF_PROMISC;
-			else if (strcmp(*argv, "off") == 0)
-				req->i.ifi_flags &= ~IFF_PROMISC;
-			else
-				return on_off("promisc", *argv);
+			parse_flag_on_off("promisc", *argv, &req->i.ifi_flags,
+					  IFF_PROMISC, &err);
+			if (err)
+				return err;
 		} else if (strcmp(*argv, "trailers") == 0) {
+			int on_off;
+
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_NOTRAILERS;
 
-			if (strcmp(*argv, "off") == 0)
-				req->i.ifi_flags |= IFF_NOTRAILERS;
-			else if (strcmp(*argv, "on") == 0)
-				req->i.ifi_flags &= ~IFF_NOTRAILERS;
-			else
-				return on_off("trailers", *argv);
+			on_off = parse_on_off("trailers", *argv, &err);
+			if (err)
+				return err;
+			set_flag(&req->i.ifi_flags, IFF_NOTRAILERS, !on_off);
 		} else if (strcmp(*argv, "arp") == 0) {
+			int on_off;
+
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_NOARP;
 
-			if (strcmp(*argv, "on") == 0)
-				req->i.ifi_flags &= ~IFF_NOARP;
-			else if (strcmp(*argv, "off") == 0)
-				req->i.ifi_flags |= IFF_NOARP;
-			else
-				return on_off("arp", *argv);
+			on_off = parse_on_off("arp", *argv, &err);
+			if (err)
+				return err;
+			set_flag(&req->i.ifi_flags, IFF_NOARP, !on_off);
 		} else if (strcmp(*argv, "carrier") == 0) {
 			int carrier;
 
 			NEXT_ARG();
-			if (strcmp(*argv, "on") == 0)
-				carrier = 1;
-			else if (strcmp(*argv, "off") == 0)
-				carrier = 0;
-			else
-				return on_off("carrier", *argv);
+			carrier = parse_on_off("carrier", *argv, &err);
+			if (err)
+				return err;
 
 			addattr8(&req->n, sizeof(*req), IFLA_CARRIER, carrier);
 		} else if (strcmp(*argv, "vf") == 0) {
@@ -793,12 +769,10 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 			NEXT_ARG();
 			req->i.ifi_change |= IFF_DYNAMIC;
 
-			if (strcmp(*argv, "on") == 0)
-				req->i.ifi_flags |= IFF_DYNAMIC;
-			else if (strcmp(*argv, "off") == 0)
-				req->i.ifi_flags &= ~IFF_DYNAMIC;
-			else
-				return on_off("dynamic", *argv);
+			parse_flag_on_off("dynamic", *argv, &req->i.ifi_flags,
+					  IFF_DYNAMIC, &err);
+			if (err)
+				return err;
 		} else if (matches(*argv, "type") == 0) {
 			NEXT_ARG();
 			*type = *argv;
@@ -895,12 +869,9 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 			unsigned int proto_down;
 
 			NEXT_ARG();
-			if (strcmp(*argv, "on") == 0)
-				proto_down = 1;
-			else if (strcmp(*argv, "off") == 0)
-				proto_down = 0;
-			else
-				return on_off("protodown", *argv);
+			proto_down = parse_on_off("protodown", *argv, &err);
+			if (err)
+				return err;
 			addattr8(&req->n, sizeof(*req), IFLA_PROTO_DOWN,
 				 proto_down);
 		} else if (strcmp(*argv, "gso_max_size") == 0) {
@@ -1320,6 +1291,7 @@ static int do_set(int argc, char **argv)
 	struct ifreq ifr0, ifr1;
 	char *newname = NULL;
 	int htype, halen;
+	int ret;
 
 	while (argc > 0) {
 		if (strcmp(*argv, "up") == 0) {
@@ -1357,63 +1329,41 @@ static int do_set(int argc, char **argv)
 		} else if (strcmp(*argv, "multicast") == 0) {
 			NEXT_ARG();
 			mask |= IFF_MULTICAST;
-
-			if (strcmp(*argv, "on") == 0)
-				flags |= IFF_MULTICAST;
-			else if (strcmp(*argv, "off") == 0)
-				flags &= ~IFF_MULTICAST;
-			else
-				return on_off("multicast", *argv);
+			parse_flag_on_off("multicast", *argv, &flags, IFF_MULTICAST, &ret);
+			if (ret)
+				return ret;
 		} else if (strcmp(*argv, "allmulticast") == 0) {
 			NEXT_ARG();
 			mask |= IFF_ALLMULTI;
-
-			if (strcmp(*argv, "on") == 0)
-				flags |= IFF_ALLMULTI;
-			else if (strcmp(*argv, "off") == 0)
-				flags &= ~IFF_ALLMULTI;
-			else
-				return on_off("allmulticast", *argv);
+			parse_flag_on_off("allmulticast", *argv, &flags, IFF_ALLMULTI, &ret);
 		} else if (strcmp(*argv, "promisc") == 0) {
 			NEXT_ARG();
 			mask |= IFF_PROMISC;
-
-			if (strcmp(*argv, "on") == 0)
-				flags |= IFF_PROMISC;
-			else if (strcmp(*argv, "off") == 0)
-				flags &= ~IFF_PROMISC;
-			else
-				return on_off("promisc", *argv);
+			parse_flag_on_off("promisc", *argv, &flags, IFF_PROMISC, &ret);
 		} else if (strcmp(*argv, "trailers") == 0) {
+			int on_off;
+
 			NEXT_ARG();
 			mask |= IFF_NOTRAILERS;
-
-			if (strcmp(*argv, "off") == 0)
-				flags |= IFF_NOTRAILERS;
-			else if (strcmp(*argv, "on") == 0)
-				flags &= ~IFF_NOTRAILERS;
-			else
-				return on_off("trailers", *argv);
+			on_off = parse_on_off("trailers", *argv, &ret);
+			if (ret)
+				return ret;
+			set_flag(&flags, IFF_NOTRAILERS, !on_off);
 		} else if (strcmp(*argv, "arp") == 0) {
+			int on_off;
+
 			NEXT_ARG();
 			mask |= IFF_NOARP;
-
-			if (strcmp(*argv, "on") == 0)
-				flags &= ~IFF_NOARP;
-			else if (strcmp(*argv, "off") == 0)
-				flags |= IFF_NOARP;
-			else
-				return on_off("arp", *argv);
+			on_off = parse_on_off("arp", *argv, &ret);
+			if (ret)
+				return ret;
+			set_flag(&flags, IFF_NOARP, !on_off);
 		} else if (matches(*argv, "dynamic") == 0) {
 			NEXT_ARG();
 			mask |= IFF_DYNAMIC;
-
-			if (strcmp(*argv, "on") == 0)
-				flags |= IFF_DYNAMIC;
-			else if (strcmp(*argv, "off") == 0)
-				flags &= ~IFF_DYNAMIC;
-			else
-				return on_off("dynamic", *argv);
+			parse_flag_on_off("dynamic", *argv, &flags, IFF_DYNAMIC, &ret);
+			if (ret)
+				return ret;
 		} else {
 			if (strcmp(*argv, "dev") == 0)
 				NEXT_ARG();
-- 
2.25.1

[PATCH iproute2-next 06/15] ip: iplink_vlan: Port over to parse_flag_on_off()

[Petr Machata]

Convert bridge/link.c from a hand-rolled on_off parsing to the new global
one.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 ip/iplink_vlan.c | 55 +++++++++++++++++-------------------------------
 1 file changed, 19 insertions(+), 36 deletions(-)

diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 1e6817f5de3d..66c4c0fb57f1 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -43,12 +43,6 @@ static void explain(void)
 	print_explain(stderr);
 }
 
-static int on_off(const char *msg, const char *arg)
-{
-	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n", msg, arg);
-	return -1;
-}
-
 static int vlan_parse_qos_map(int *argcp, char ***argvp, struct nlmsghdr *n,
 			      int attrtype)
 {
@@ -87,6 +81,7 @@ static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
 {
 	struct ifla_vlan_flags flags = { 0 };
 	__u16 id, proto;
+	int ret;
 
 	while (argc > 0) {
 		if (matches(*argv, "protocol") == 0) {
@@ -102,48 +97,36 @@ static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
 		} else if (matches(*argv, "reorder_hdr") == 0) {
 			NEXT_ARG();
 			flags.mask |= VLAN_FLAG_REORDER_HDR;
-			if (strcmp(*argv, "on") == 0)
-				flags.flags |= VLAN_FLAG_REORDER_HDR;
-			else if (strcmp(*argv, "off") == 0)
-				flags.flags &= ~VLAN_FLAG_REORDER_HDR;
-			else
-				return on_off("reorder_hdr", *argv);
+			parse_flag_on_off("reorder_hdr", *argv, &flags.flags, VLAN_FLAG_REORDER_HDR,
+					  &ret);
+			if (ret)
+				return ret;
 		} else if (matches(*argv, "gvrp") == 0) {
 			NEXT_ARG();
 			flags.mask |= VLAN_FLAG_GVRP;
-			if (strcmp(*argv, "on") == 0)
-				flags.flags |= VLAN_FLAG_GVRP;
-			else if (strcmp(*argv, "off") == 0)
-				flags.flags &= ~VLAN_FLAG_GVRP;
-			else
-				return on_off("gvrp", *argv);
+			parse_flag_on_off("gvrp", *argv, &flags.flags, VLAN_FLAG_GVRP, &ret);
+			if (ret)
+				return ret;
 		} else if (matches(*argv, "mvrp") == 0) {
 			NEXT_ARG();
 			flags.mask |= VLAN_FLAG_MVRP;
-			if (strcmp(*argv, "on") == 0)
-				flags.flags |= VLAN_FLAG_MVRP;
-			else if (strcmp(*argv, "off") == 0)
-				flags.flags &= ~VLAN_FLAG_MVRP;
-			else
-				return on_off("mvrp", *argv);
+			parse_flag_on_off("mvrp", *argv, &flags.flags, VLAN_FLAG_MVRP, &ret);
+			if (ret)
+				return ret;
 		} else if (matches(*argv, "loose_binding") == 0) {
 			NEXT_ARG();
 			flags.mask |= VLAN_FLAG_LOOSE_BINDING;
-			if (strcmp(*argv, "on") == 0)
-				flags.flags |= VLAN_FLAG_LOOSE_BINDING;
-			else if (strcmp(*argv, "off") == 0)
-				flags.flags &= ~VLAN_FLAG_LOOSE_BINDING;
-			else
-				return on_off("loose_binding", *argv);
+			parse_flag_on_off("loose_binding", *argv, &flags.flags,
+					  VLAN_FLAG_LOOSE_BINDING, &ret);
+			if (ret)
+				return ret;
 		} else if (matches(*argv, "bridge_binding") == 0) {
 			NEXT_ARG();
 			flags.mask |= VLAN_FLAG_BRIDGE_BINDING;
-			if (strcmp(*argv, "on") == 0)
-				flags.flags |= VLAN_FLAG_BRIDGE_BINDING;
-			else if (strcmp(*argv, "off") == 0)
-				flags.flags &= ~VLAN_FLAG_BRIDGE_BINDING;
-			else
-				return on_off("bridge_binding", *argv);
+			parse_flag_on_off("bridge_binding", *argv, &flags.flags,
+					  VLAN_FLAG_BRIDGE_BINDING, &ret);
+			if (ret)
+				return ret;
 		} else if (matches(*argv, "ingress-qos-map") == 0) {
 			NEXT_ARG();
 			if (vlan_parse_qos_map(&argc, &argv, n,
-- 
2.25.1

[PATCH iproute2-next 07/15] ip: iplink_bridge_slave: Port over to parse_on_off()

[Petr Machata]

Invoke parse_on_off() from bridge_slave_parse_on_off() instead of
hand-rolling one. Exit on failure, because the invarg that was ivoked here
before would.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 ip/iplink_bridge_slave.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/ip/iplink_bridge_slave.c b/ip/iplink_bridge_slave.c
index 79a1d2f5f5b8..f7f6da0c79b7 100644
--- a/ip/iplink_bridge_slave.c
+++ b/ip/iplink_bridge_slave.c
@@ -297,15 +297,11 @@ static void bridge_slave_print_opt(struct link_util *lu, FILE *f,
 static void bridge_slave_parse_on_off(char *arg_name, char *arg_val,
 				      struct nlmsghdr *n, int type)
 {
-	__u8 val;
-
-	if (strcmp(arg_val, "on") == 0)
-		val = 1;
-	else if (strcmp(arg_val, "off") == 0)
-		val = 0;
-	else
-		invarg("should be \"on\" or \"off\"", arg_name);
+	int ret;
+	__u8 val = parse_on_off(arg_name, arg_val, &ret);
 
+	if (ret)
+		exit(1);
 	addattr8(n, 1024, type, val);
 }
 
-- 
2.25.1

[PATCH iproute2-next 08/15] lib: Extract from devlink/mnlg a helper, mnlu_socket_open()

[Petr Machata]

This little dance of mnl_socket_open(), option setting, and bind, is the
same regardless of tool. Extract into a new module that should hold helpers
for working with libmnl, mnl_util.c.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 devlink/Makefile    |  2 +-
 devlink/mnlg.c      | 19 ++++---------------
 include/mnl_utils.h |  7 +++++++
 lib/Makefile        |  2 +-
 lib/mnl_utils.c     | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 48 insertions(+), 17 deletions(-)
 create mode 100644 include/mnl_utils.h
 create mode 100644 lib/mnl_utils.c

diff --git a/devlink/Makefile b/devlink/Makefile
index 7da7d1fa18d5..d540feb3c012 100644
--- a/devlink/Makefile
+++ b/devlink/Makefile
@@ -12,7 +12,7 @@ endif
 
 all: $(TARGETS) $(LIBS)
 
-devlink: $(DEVLINKOBJ)
+devlink: $(DEVLINKOBJ) $(LIBNETLINK)
 	$(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@
 
 install: all
diff --git a/devlink/mnlg.c b/devlink/mnlg.c
index c7d25e8713a1..9817bbad5e7d 100644
--- a/devlink/mnlg.c
+++ b/devlink/mnlg.c
@@ -19,6 +19,7 @@
 #include <linux/genetlink.h>
 
 #include "libnetlink.h"
+#include "mnl_utils.h"
 #include "utils.h"
 #include "mnlg.h"
 
@@ -263,7 +264,6 @@ struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version)
 {
 	struct mnlg_socket *nlg;
 	struct nlmsghdr *nlh;
-	int one = 1;
 	int err;
 
 	nlg = malloc(sizeof(*nlg));
@@ -274,19 +274,9 @@ struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version)
 	if (!nlg->buf)
 		goto err_buf_alloc;
 
-	nlg->nl = mnl_socket_open(NETLINK_GENERIC);
+	nlg->nl = mnlu_socket_open(NETLINK_GENERIC);
 	if (!nlg->nl)
-		goto err_mnl_socket_open;
-
-	/* Older kernels may no support capped/extended ACK reporting */
-	mnl_socket_setsockopt(nlg->nl, NETLINK_CAP_ACK, &one, sizeof(one));
-	mnl_socket_setsockopt(nlg->nl, NETLINK_EXT_ACK, &one, sizeof(one));
-
-	err = mnl_socket_bind(nlg->nl, 0, MNL_SOCKET_AUTOPID);
-	if (err < 0)
-		goto err_mnl_socket_bind;
-
-	nlg->portid = mnl_socket_get_portid(nlg->nl);
+		goto err_socket_open;
 
 	nlh = __mnlg_msg_prepare(nlg, CTRL_CMD_GETFAMILY,
 				 NLM_F_REQUEST | NLM_F_ACK, GENL_ID_CTRL, 1);
@@ -305,9 +295,8 @@ struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version)
 
 err_mnlg_socket_recv_run:
 err_mnlg_socket_send:
-err_mnl_socket_bind:
 	mnl_socket_close(nlg->nl);
-err_mnl_socket_open:
+err_socket_open:
 	free(nlg->buf);
 err_buf_alloc:
 	free(nlg);
diff --git a/include/mnl_utils.h b/include/mnl_utils.h
new file mode 100644
index 000000000000..10a064afdfe8
--- /dev/null
+++ b/include/mnl_utils.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __MNL_UTILS_H__
+#define __MNL_UTILS_H__ 1
+
+struct mnl_socket *mnlu_socket_open(int bus);
+
+#endif /* __MNL_UTILS_H__ */
diff --git a/lib/Makefile b/lib/Makefile
index 7cba1857b7fa..13f4ee15373b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -7,7 +7,7 @@ UTILOBJ = utils.o rt_names.o ll_map.o ll_types.o ll_proto.o ll_addr.o \
 	inet_proto.o namespace.o json_writer.o json_print.o \
 	names.o color.o bpf.o exec.o fs.o cg_map.o
 
-NLOBJ=libgenl.o libnetlink.o
+NLOBJ=libgenl.o libnetlink.o mnl_utils.o
 
 all: libnetlink.a libutil.a
 
diff --git a/lib/mnl_utils.c b/lib/mnl_utils.c
new file mode 100644
index 000000000000..eecb11341651
--- /dev/null
+++ b/lib/mnl_utils.c
@@ -0,0 +1,35 @@
+/*
+ * mnl_utils.c	Helpers for working with libmnl.
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include <libmnl/libmnl.h>
+
+#include "mnl_utils.h"
+
+struct mnl_socket *mnlu_socket_open(int bus)
+{
+	struct mnl_socket *nl;
+	int one = 1;
+
+	nl = mnl_socket_open(bus);
+	if (nl == NULL)
+		return NULL;
+
+	mnl_socket_setsockopt(nl, NETLINK_CAP_ACK, &one, sizeof(one));
+	mnl_socket_setsockopt(nl, NETLINK_EXT_ACK, &one, sizeof(one));
+
+	if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0)
+		goto err_bind;
+
+	return nl;
+
+err_bind:
+	mnl_socket_close(nl);
+	return NULL;
+}
-- 
2.25.1

[PATCH iproute2-next 09/15] lib: Extract from devlink/mnlg a helper, mnlu_msg_prepare()

[Petr Machata]

Allocation of a new netlink message with the two usual headers is reusable
with other netlink netlink message types. Extract it into a helper,
mnlu_msg_prepare(). Take the second header as an argument, instead of
passing in parameters to initialize it, and copy it in.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 devlink/mnlg.c      | 18 ++++++------------
 include/mnl_utils.h |  2 ++
 lib/mnl_utils.c     | 19 +++++++++++++++++++
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/devlink/mnlg.c b/devlink/mnlg.c
index 9817bbad5e7d..4995b7af06a3 100644
--- a/devlink/mnlg.c
+++ b/devlink/mnlg.c
@@ -14,7 +14,6 @@
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
-#include <time.h>
 #include <libmnl/libmnl.h>
 #include <linux/genetlink.h>
 
@@ -36,19 +35,14 @@ static struct nlmsghdr *__mnlg_msg_prepare(struct mnlg_socket *nlg, uint8_t cmd,
 					   uint16_t flags, uint32_t id,
 					   uint8_t version)
 {
+	struct genlmsghdr genl = {
+		.cmd = cmd,
+		.version = version,
+	};
 	struct nlmsghdr *nlh;
-	struct genlmsghdr *genl;
-
-	nlh = mnl_nlmsg_put_header(nlg->buf);
-	nlh->nlmsg_type	= id;
-	nlh->nlmsg_flags = flags;
-	nlg->seq = time(NULL);
-	nlh->nlmsg_seq = nlg->seq;
-
-	genl = mnl_nlmsg_put_extra_header(nlh, sizeof(struct genlmsghdr));
-	genl->cmd = cmd;
-	genl->version = version;
 
+	nlh = mnlu_msg_prepare(nlg->buf, id, flags, &genl, sizeof(genl));
+	nlg->seq = nlh->nlmsg_seq;
 	return nlh;
 }
 
diff --git a/include/mnl_utils.h b/include/mnl_utils.h
index 10a064afdfe8..86ce30f49a94 100644
--- a/include/mnl_utils.h
+++ b/include/mnl_utils.h
@@ -3,5 +3,7 @@
 #define __MNL_UTILS_H__ 1
 
 struct mnl_socket *mnlu_socket_open(int bus);
+struct nlmsghdr *mnlu_msg_prepare(void *buf, uint32_t nlmsg_type, uint16_t flags,
+				  void *extra_header, size_t extra_header_size);
 
 #endif /* __MNL_UTILS_H__ */
diff --git a/lib/mnl_utils.c b/lib/mnl_utils.c
index eecb11341651..87df1e81faf5 100644
--- a/lib/mnl_utils.c
+++ b/lib/mnl_utils.c
@@ -8,6 +8,8 @@
  *
  */
 
+#include <string.h>
+#include <time.h>
 #include <libmnl/libmnl.h>
 
 #include "mnl_utils.h"
@@ -33,3 +35,20 @@ err_bind:
 	mnl_socket_close(nl);
 	return NULL;
 }
+
+struct nlmsghdr *mnlu_msg_prepare(void *buf, uint32_t nlmsg_type, uint16_t flags,
+				  void *extra_header, size_t extra_header_size)
+{
+	struct nlmsghdr *nlh;
+	void *eh;
+
+	nlh = mnl_nlmsg_put_header(buf);
+	nlh->nlmsg_type = nlmsg_type;
+	nlh->nlmsg_flags = flags;
+	nlh->nlmsg_seq = time(NULL);
+
+	eh = mnl_nlmsg_put_extra_header(nlh, extra_header_size);
+	memcpy(eh, extra_header, extra_header_size);
+
+	return nlh;
+}
-- 
2.25.1

[PATCH iproute2-next 10/15] lib: Extract from devlink/mnlg a helper, mnlu_socket_recv_run()

[Petr Machata]

Receiving a message in libmnl is a somewhat involved operation. Devlink's
mnlg library has an implementation that is going to be handy for other
tools as well. Extract it into a new helper.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 devlink/mnlg.c      | 56 ++---------------------------------------
 include/mnl_utils.h |  2 ++
 lib/mnl_utils.c     | 61 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/devlink/mnlg.c b/devlink/mnlg.c
index 4995b7af06a3..21b10c5a5669 100644
--- a/devlink/mnlg.c
+++ b/devlink/mnlg.c
@@ -28,7 +28,6 @@ struct mnlg_socket {
 	uint32_t id;
 	uint8_t version;
 	unsigned int seq;
-	unsigned int portid;
 };
 
 static struct nlmsghdr *__mnlg_msg_prepare(struct mnlg_socket *nlg, uint8_t cmd,
@@ -57,61 +56,10 @@ int mnlg_socket_send(struct mnlg_socket *nlg, const struct nlmsghdr *nlh)
 	return mnl_socket_sendto(nlg->nl, nlh, nlh->nlmsg_len);
 }
 
-static int mnlg_cb_noop(const struct nlmsghdr *nlh, void *data)
-{
-	return MNL_CB_OK;
-}
-
-static int mnlg_cb_error(const struct nlmsghdr *nlh, void *data)
-{
-	const struct nlmsgerr *err = mnl_nlmsg_get_payload(nlh);
-
-	/* Netlink subsystems returns the errno value with different signess */
-	if (err->error < 0)
-		errno = -err->error;
-	else
-		errno = err->error;
-
-	if (nl_dump_ext_ack(nlh, NULL))
-		return MNL_CB_ERROR;
-
-	return err->error == 0 ? MNL_CB_STOP : MNL_CB_ERROR;
-}
-
-static int mnlg_cb_stop(const struct nlmsghdr *nlh, void *data)
-{
-	int len = *(int *)NLMSG_DATA(nlh);
-
-	if (len < 0) {
-		errno = -len;
-		nl_dump_ext_ack_done(nlh, len);
-		return MNL_CB_ERROR;
-	}
-	return MNL_CB_STOP;
-}
-
-static mnl_cb_t mnlg_cb_array[NLMSG_MIN_TYPE] = {
-	[NLMSG_NOOP]	= mnlg_cb_noop,
-	[NLMSG_ERROR]	= mnlg_cb_error,
-	[NLMSG_DONE]	= mnlg_cb_stop,
-	[NLMSG_OVERRUN]	= mnlg_cb_noop,
-};
-
 int mnlg_socket_recv_run(struct mnlg_socket *nlg, mnl_cb_t data_cb, void *data)
 {
-	int err;
-
-	do {
-		err = mnl_socket_recvfrom(nlg->nl, nlg->buf,
-					  MNL_SOCKET_BUFFER_SIZE);
-		if (err <= 0)
-			break;
-		err = mnl_cb_run2(nlg->buf, err, nlg->seq, nlg->portid,
-				  data_cb, data, mnlg_cb_array,
-				  ARRAY_SIZE(mnlg_cb_array));
-	} while (err > 0);
-
-	return err;
+	return mnlu_socket_recv_run(nlg->nl, nlg->seq, nlg->buf, MNL_SOCKET_BUFFER_SIZE,
+				    data_cb, data);
 }
 
 struct group_info {
diff --git a/include/mnl_utils.h b/include/mnl_utils.h
index 86ce30f49a94..fa826ef1f8fe 100644
--- a/include/mnl_utils.h
+++ b/include/mnl_utils.h
@@ -5,5 +5,7 @@
 struct mnl_socket *mnlu_socket_open(int bus);
 struct nlmsghdr *mnlu_msg_prepare(void *buf, uint32_t nlmsg_type, uint16_t flags,
 				  void *extra_header, size_t extra_header_size);
+int mnlu_socket_recv_run(struct mnl_socket *nl, unsigned int seq, void *buf, size_t buf_size,
+			 mnl_cb_t cb, void *data);
 
 #endif /* __MNL_UTILS_H__ */
diff --git a/lib/mnl_utils.c b/lib/mnl_utils.c
index 87df1e81faf5..e9461d6d6b6b 100644
--- a/lib/mnl_utils.c
+++ b/lib/mnl_utils.c
@@ -8,11 +8,14 @@
  *
  */
 
+#include <errno.h>
 #include <string.h>
 #include <time.h>
 #include <libmnl/libmnl.h>
 
+#include "libnetlink.h"
 #include "mnl_utils.h"
+#include "utils.h"
 
 struct mnl_socket *mnlu_socket_open(int bus)
 {
@@ -52,3 +55,61 @@ struct nlmsghdr *mnlu_msg_prepare(void *buf, uint32_t nlmsg_type, uint16_t flags
 
 	return nlh;
 }
+
+static int mnlu_cb_noop(const struct nlmsghdr *nlh, void *data)
+{
+	return MNL_CB_OK;
+}
+
+static int mnlu_cb_error(const struct nlmsghdr *nlh, void *data)
+{
+	const struct nlmsgerr *err = mnl_nlmsg_get_payload(nlh);
+
+	/* Netlink subsystems returns the errno value with different signess */
+	if (err->error < 0)
+		errno = -err->error;
+	else
+		errno = err->error;
+
+	if (nl_dump_ext_ack(nlh, NULL))
+		return MNL_CB_ERROR;
+
+	return err->error == 0 ? MNL_CB_STOP : MNL_CB_ERROR;
+}
+
+static int mnlu_cb_stop(const struct nlmsghdr *nlh, void *data)
+{
+	int len = *(int *)NLMSG_DATA(nlh);
+
+	if (len < 0) {
+		errno = -len;
+		nl_dump_ext_ack_done(nlh, len);
+		return MNL_CB_ERROR;
+	}
+	return MNL_CB_STOP;
+}
+
+static mnl_cb_t mnlu_cb_array[NLMSG_MIN_TYPE] = {
+	[NLMSG_NOOP]	= mnlu_cb_noop,
+	[NLMSG_ERROR]	= mnlu_cb_error,
+	[NLMSG_DONE]	= mnlu_cb_stop,
+	[NLMSG_OVERRUN]	= mnlu_cb_noop,
+};
+
+int mnlu_socket_recv_run(struct mnl_socket *nl, unsigned int seq, void *buf, size_t buf_size,
+			 mnl_cb_t cb, void *data)
+{
+	unsigned int portid = mnl_socket_get_portid(nl);
+	int err;
+
+	do {
+		err = mnl_socket_recvfrom(nl, buf, buf_size);
+		if (err <= 0)
+			break;
+		err = mnl_cb_run2(buf, err, seq, portid,
+				  cb, data, mnlu_cb_array,
+				  ARRAY_SIZE(mnlu_cb_array));
+	} while (err > 0);
+
+	return err;
+}
-- 
2.25.1

[PATCH iproute2-next 11/15] lib: Extract from iplink_vlan a helper to parse key:value arrays

[Petr Machata]

VLAN netdevices have two similar attributes: ingress-qos-map and
egress-qos-map. These attributes can be configured with a series of
802.1-priority-to-skb-priority (and vice versa) mappings. A reusable helper
along those lines will be handy for configuration of various
priority-to-tc, tc-to-algorithm, and other arrays in DCB. Therefore extract
the logic to a function parse_mapping(), move to utils.c, and dispatch to
utils.c from iplink_vlan.c.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 include/utils.h  |  4 ++++
 ip/iplink_vlan.c | 37 ++++++++++++++++---------------------
 lib/utils.c      | 28 ++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 681110fcf8af..8323e3cf1103 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -339,4 +339,8 @@ int parse_on_off(const char *msg, const char *realval, int *p_err);
 void parse_flag_on_off(const char *msg, const char *realval,
 		       unsigned int *p_flags, unsigned int flag, int *p_ret);
 
+int parse_mapping(int *argcp, char ***argvp,
+		  int (*mapping_cb)(__u32 key, char *value, void *data),
+		  void *mapping_cb_data);
+
 #endif /* __UTILS_H__ */
diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 66c4c0fb57f1..73aa94acde3c 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -43,36 +43,31 @@ static void explain(void)
 	print_explain(stderr);
 }
 
+static int parse_qos_mapping(__u32 key, char *value, void *data)
+{
+	struct nlmsghdr *n = data;
+	struct ifla_vlan_qos_mapping m = {
+		.from = key,
+	};
+
+	if (get_u32(&m.to, value, 0))
+		return 1;
+
+	addattr_l(n, 1024, IFLA_VLAN_QOS_MAPPING, &m, sizeof(m));
+	return 0;
+}
+
 static int vlan_parse_qos_map(int *argcp, char ***argvp, struct nlmsghdr *n,
 			      int attrtype)
 {
-	int argc = *argcp;
-	char **argv = *argvp;
-	struct ifla_vlan_qos_mapping m;
 	struct rtattr *tail;
 
 	tail = addattr_nest(n, 1024, attrtype);
 
-	while (argc > 0) {
-		char *colon = strchr(*argv, ':');
-
-		if (!colon)
-			break;
-		*colon = '\0';
-
-		if (get_u32(&m.from, *argv, 0))
-			return 1;
-		if (get_u32(&m.to, colon + 1, 0))
-			return 1;
-		argc--, argv++;
-
-		addattr_l(n, 1024, IFLA_VLAN_QOS_MAPPING, &m, sizeof(m));
-	}
+	if (parse_mapping(argcp, argvp, &parse_qos_mapping, n))
+		return 1;
 
 	addattr_nest_end(n, tail);
-
-	*argcp = argc;
-	*argvp = argv;
 	return 0;
 }
 
diff --git a/lib/utils.c b/lib/utils.c
index fb25c64d36ff..93521a49eaec 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1774,3 +1774,31 @@ void parse_flag_on_off(const char *msg, const char *realval,
 
 	set_flag(p_flags, flag, on_off);
 }
+
+int parse_mapping(int *argcp, char ***argvp,
+		  int (*mapping_cb)(__u32 key, char *value, void *data),
+		  void *mapping_cb_data)
+{
+	int argc = *argcp;
+	char **argv = *argvp;
+
+	while (argc > 0) {
+		char *colon = strchr(*argv, ':');
+		__u32 key;
+
+		if (!colon)
+			break;
+		*colon = '\0';
+
+		if (get_u32(&key, *argv, 0))
+			return 1;
+		if (mapping_cb(key, colon + 1, mapping_cb_data))
+			return 1;
+
+		argc--, argv++;
+	}
+
+	*argcp = argc;
+	*argvp = argv;
+	return 0;
+}
-- 
2.25.1

[PATCH iproute2-next 12/15] lib: parse_mapping: Update argc, argv on error

[Petr Machata]

Currently argc and argv are not updated unless parsing of all of the
mapping was successful. However in that case, "ip link" will point at the
wrong argument when complaining:

    # ip link add name eth0.100 link eth0 type vlan id 100 egress 1:1 2:foo
    Error: argument "1" is wrong: invalid egress-qos-map

Update argc and argv even in the case of parsing error, so that the right
element is indicated.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 lib/utils.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index 93521a49eaec..87cc6ae0cfba 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1781,6 +1781,7 @@ int parse_mapping(int *argcp, char ***argvp,
 {
 	int argc = *argcp;
 	char **argv = *argvp;
+	int ret = 0;
 
 	while (argc > 0) {
 		char *colon = strchr(*argv, ':');
@@ -1790,15 +1791,19 @@ int parse_mapping(int *argcp, char ***argvp,
 			break;
 		*colon = '\0';
 
-		if (get_u32(&key, *argv, 0))
-			return 1;
-		if (mapping_cb(key, colon + 1, mapping_cb_data))
-			return 1;
+		if (get_u32(&key, *argv, 0)) {
+			ret = 1;
+			break;
+		}
+		if (mapping_cb(key, colon + 1, mapping_cb_data)) {
+			ret = 1;
+			break;
+		}
 
 		argc--, argv++;
 	}
 
 	*argcp = argc;
 	*argvp = argv;
-	return 0;
+	return ret;
 }
-- 
2.25.1

[PATCH iproute2-next 13/15] lib: parse_mapping: Recognize a keyword "all"

[Petr Machata]

The DCB tool will have to provide an interface to a number of fixed-size
arrays. Unlike the egress- and ingress-qos-map, it makes good sense to have
an interface to set all members to the same value. For example to set
strict priority on all TCs besides select few, or to reset allocated
bandwidth to all zeroes, again besides several explicitly-given ones.

To support this usage, extend the parse_mapping() with a boolean that
determines whether this special use is supported. If "all" is given and
recognized, mapping_cb is called with the key of -1.

Have iplink_vlan pass false for allow_all.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 include/utils.h  | 2 +-
 ip/iplink_vlan.c | 2 +-
 lib/utils.c      | 6 ++++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 8323e3cf1103..cad87d39695a 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -339,7 +339,7 @@ int parse_on_off(const char *msg, const char *realval, int *p_err);
 void parse_flag_on_off(const char *msg, const char *realval,
 		       unsigned int *p_flags, unsigned int flag, int *p_ret);
 
-int parse_mapping(int *argcp, char ***argvp,
+int parse_mapping(int *argcp, char ***argvp, bool allow_all,
 		  int (*mapping_cb)(__u32 key, char *value, void *data),
 		  void *mapping_cb_data);
 
diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 73aa94acde3c..8ab2250cf110 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -64,7 +64,7 @@ static int vlan_parse_qos_map(int *argcp, char ***argvp, struct nlmsghdr *n,
 
 	tail = addattr_nest(n, 1024, attrtype);
 
-	if (parse_mapping(argcp, argvp, &parse_qos_mapping, n))
+	if (parse_mapping(argcp, argvp, false, &parse_qos_mapping, n))
 		return 1;
 
 	addattr_nest_end(n, tail);
diff --git a/lib/utils.c b/lib/utils.c
index 87cc6ae0cfba..51471ba73b8d 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1775,7 +1775,7 @@ void parse_flag_on_off(const char *msg, const char *realval,
 	set_flag(p_flags, flag, on_off);
 }
 
-int parse_mapping(int *argcp, char ***argvp,
+int parse_mapping(int *argcp, char ***argvp, bool allow_all,
 		  int (*mapping_cb)(__u32 key, char *value, void *data),
 		  void *mapping_cb_data)
 {
@@ -1791,7 +1791,9 @@ int parse_mapping(int *argcp, char ***argvp,
 			break;
 		*colon = '\0';
 
-		if (get_u32(&key, *argv, 0)) {
+		if (allow_all && matches(*argv, "all") == 0) {
+			key = (__u32) -1;
+		} else if (get_u32(&key, *argv, 0)) {
 			ret = 1;
 			break;
 		}
-- 
2.25.1

[PATCH iproute2-next 14/15] Add skeleton of a new tool, dcb

[Petr Machata]

The Linux DCB interface allows configuration of a broad range of
hardware-specific attributes, such as TC scheduling, flow control, per-port
buffer configuration, TC rate, etc. Add a new tool to show that
configuration and tweak it.

DCB allows configuration of several objects, and possibly could expand to
pre-standard CEE interfaces. Therefore the tool itself is a lean shell that
dispatches to subtools each dedicated to one of the objects.

Signed-off-by: Petr Machata <me@pmachata.org>
---
 Makefile       |   2 +-
 dcb/Makefile   |  24 ++++
 dcb/dcb.c      | 377 +++++++++++++++++++++++++++++++++++++++++++++++++
 dcb/dcb.h      |  32 +++++
 man/man8/dcb.8 | 103 ++++++++++++++
 5 files changed, 537 insertions(+), 1 deletion(-)
 create mode 100644 dcb/Makefile
 create mode 100644 dcb/dcb.c
 create mode 100644 dcb/dcb.h
 create mode 100644 man/man8/dcb.8

diff --git a/Makefile b/Makefile
index 5b040415a12b..e64c65992585 100644
--- a/Makefile
+++ b/Makefile
@@ -55,7 +55,7 @@ WFLAGS += -Wmissing-declarations -Wold-style-definition -Wformat=2
 CFLAGS := $(WFLAGS) $(CCOPTS) -I../include -I../include/uapi $(DEFINES) $(CFLAGS)
 YACCFLAGS = -d -t -v
 
-SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma man
+SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma dcb man
 
 LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
 LDLIBS += $(LIBNETLINK)
diff --git a/dcb/Makefile b/dcb/Makefile
new file mode 100644
index 000000000000..9966c8f0bfa4
--- /dev/null
+++ b/dcb/Makefile
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: GPL-2.0
+include ../config.mk
+
+TARGETS :=
+
+ifeq ($(HAVE_MNL),y)
+
+DCBOBJ = dcb.o
+TARGETS += dcb
+
+endif
+
+all: $(TARGETS) $(LIBS)
+
+dcb: $(DCBOBJ) $(LIBNETLINK)
+	$(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@
+
+install: all
+	for i in $(TARGETS); \
+	do install -m 0755 $$i $(DESTDIR)$(SBINDIR); \
+	done
+
+clean:
+	rm -f $(DCBOBJ) $(TARGETS)
diff --git a/dcb/dcb.c b/dcb/dcb.c
new file mode 100644
index 000000000000..c85008bbe1e9
--- /dev/null
+++ b/dcb/dcb.c
@@ -0,0 +1,377 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <stdio.h>
+#include <linux/dcbnl.h>
+#include <libmnl/libmnl.h>
+#include <getopt.h>
+
+#include "dcb.h"
+#include "mnl_utils.h"
+#include "namespace.h"
+#include "utils.h"
+#include "version.h"
+
+static int dcb_init(struct dcb *dcb)
+{
+	dcb->buf = malloc(MNL_SOCKET_BUFFER_SIZE);
+	if (dcb->buf == NULL) {
+		perror("Netlink buffer allocation");
+		return -1;
+	}
+
+	dcb->nl = mnlu_socket_open(NETLINK_ROUTE);
+	if (dcb->nl == NULL) {
+		perror("Open netlink socket");
+		goto err_socket_open;
+	}
+
+	new_json_obj_plain(dcb->json_output);
+	return 0;
+
+err_socket_open:
+	free(dcb->buf);
+	return -1;
+}
+
+static void dcb_fini(struct dcb *dcb)
+{
+	delete_json_obj_plain();
+	mnl_socket_close(dcb->nl);
+}
+
+static struct dcb *dcb_alloc(void)
+{
+	struct dcb *dcb;
+
+	dcb = calloc(1, sizeof(*dcb));
+	if (!dcb)
+		return NULL;
+	return dcb;
+}
+
+static void dcb_free(struct dcb *dcb)
+{
+	free(dcb);
+}
+
+struct dcb_get_attribute {
+	struct dcb *dcb;
+	int attr;
+	void *data;
+	size_t data_len;
+};
+
+static int dcb_get_attribute_attr_ieee_cb(const struct nlattr *attr, void *data)
+{
+	struct dcb_get_attribute *ga = data;
+	uint16_t len;
+
+	if (mnl_attr_get_type(attr) != ga->attr)
+		return MNL_CB_OK;
+
+	len = mnl_attr_get_payload_len(attr);
+	if (len != ga->data_len) {
+		fprintf(stderr, "Wrong len %d, expected %zd\n", len, ga->data_len);
+		return MNL_CB_ERROR;
+	}
+
+	memcpy(ga->data, mnl_attr_get_payload(attr), ga->data_len);
+	return MNL_CB_STOP;
+}
+
+static int dcb_get_attribute_attr_cb(const struct nlattr *attr, void *data)
+{
+	if (mnl_attr_get_type(attr) != DCB_ATTR_IEEE)
+		return MNL_CB_OK;
+
+	return mnl_attr_parse_nested(attr, dcb_get_attribute_attr_ieee_cb, data);
+}
+
+static int dcb_get_attribute_cb(const struct nlmsghdr *nlh, void *data)
+{
+	return mnl_attr_parse(nlh, sizeof(struct dcbmsg), dcb_get_attribute_attr_cb, data);
+}
+
+static int dcb_set_attribute_attr_cb(const struct nlattr *attr, void *data)
+{
+	uint16_t len;
+	uint8_t err;
+
+	if (mnl_attr_get_type(attr) != DCB_ATTR_IEEE)
+		return MNL_CB_OK;
+
+	len = mnl_attr_get_payload_len(attr);
+	if (len != 1) {
+		fprintf(stderr, "Response attribute expected to have size 1, not %d\n", len);
+		return MNL_CB_ERROR;
+	}
+
+	err = mnl_attr_get_u8(attr);
+	if (err) {
+		fprintf(stderr, "Error when attempting to set attribute: %s\n",
+			strerror(err));
+		return MNL_CB_ERROR;
+	}
+
+	return MNL_CB_STOP;
+}
+
+static int dcb_set_attribute_cb(const struct nlmsghdr *nlh, void *data)
+{
+	return mnl_attr_parse(nlh, sizeof(struct dcbmsg), dcb_set_attribute_attr_cb, data);
+}
+
+static int dcb_talk(struct dcb *dcb, struct nlmsghdr *nlh, mnl_cb_t cb, void *data)
+{
+	int ret;
+
+	ret = mnl_socket_sendto(dcb->nl, nlh, nlh->nlmsg_len);
+	if (ret < 0) {
+		perror("mnl_socket_sendto");
+		return -1;
+	}
+
+	return mnlu_socket_recv_run(dcb->nl, nlh->nlmsg_seq, dcb->buf, MNL_SOCKET_BUFFER_SIZE,
+				    cb, data);
+}
+
+static struct nlmsghdr *dcb_prepare(struct dcb *dcb, const char *dev,
+				    uint32_t nlmsg_type, uint8_t dcb_cmd)
+{
+	struct dcbmsg dcbm = {
+		.cmd = dcb_cmd,
+	};
+	struct nlmsghdr *nlh;
+
+	nlh = mnlu_msg_prepare(dcb->buf, nlmsg_type, NLM_F_REQUEST, &dcbm, sizeof(dcbm));
+	mnl_attr_put_strz(nlh, DCB_ATTR_IFNAME, dev);
+	return nlh;
+}
+
+int dcb_get_attribute(struct dcb *dcb, const char *dev, int attr, void *data, size_t data_len)
+{
+	struct dcb_get_attribute ga;
+	struct nlmsghdr *nlh;
+	int ret;
+
+	nlh = dcb_prepare(dcb, dev, RTM_GETDCB, DCB_CMD_IEEE_GET);
+
+	ga = (struct dcb_get_attribute) {
+		.dcb = dcb,
+		.attr = attr,
+		.data = data,
+		.data_len = data_len,
+	};
+	ret = dcb_talk(dcb, nlh, dcb_get_attribute_cb, &ga);
+	if (ret) {
+		perror("Attribute read");
+		return ret;
+	}
+	return 0;
+}
+
+int dcb_set_attribute(struct dcb *dcb, const char *dev, int attr, const void *data, size_t data_len)
+{
+	struct nlmsghdr *nlh;
+	struct nlattr *nest;
+	int ret;
+
+	nlh = dcb_prepare(dcb, dev, RTM_GETDCB, DCB_CMD_IEEE_SET);
+
+	nest = mnl_attr_nest_start(nlh, DCB_ATTR_IEEE);
+	mnl_attr_put(nlh, attr, data_len, data);
+	mnl_attr_nest_end(nlh, nest);
+
+	ret = dcb_talk(dcb, nlh, dcb_set_attribute_cb, NULL);
+	if (ret) {
+		perror("Attribute write");
+		return ret;
+	}
+	return 0;
+}
+
+void dcb_print_array_num(FILE *fp, const __u8 *array, size_t size)
+{
+	SPRINT_BUF(b1);
+	SPRINT_BUF(b2);
+	size_t i;
+
+	for (i = 0; i < size; i++) {
+		snprintf(b1, sizeof(b1), "%zd", i);
+		snprintf(b2, sizeof(b2), "%zd:%%d ", i);
+		print_uint(PRINT_ANY, b1, b2, array[i]);
+	}
+}
+
+void dcb_print_array_kw(FILE *fp, const __u8 *array, size_t array_size,
+			const char *const kw[], size_t kw_size)
+{
+	SPRINT_BUF(b1);
+	SPRINT_BUF(b2);
+	size_t i;
+
+	for (i = 0; i < array_size; i++) {
+		__u8 emt = array[i];
+
+		snprintf(b1, sizeof(b1), "%zd", i);
+		snprintf(b2, sizeof(b2), "%zd:%%s ", i);
+		if (emt < kw_size && kw[emt])
+			print_string(PRINT_ANY, b1, b2, kw[emt]);
+		else
+			print_string(PRINT_ANY, b1, b2, "???");
+	}
+}
+
+void dcb_print_named_array(FILE *fp, const char *name, const __u8 *array, size_t size,
+			   void (*print_array)(FILE *, const __u8 *, size_t))
+{
+	open_json_object(name);
+	print_string(PRINT_FP, NULL, "%s ", name);
+	print_array(fp, array, size);
+	close_json_object();
+}
+
+int dcb_cmd_parse_dev(struct dcb *dcb, int argc, char **argv,
+		      int (*and_then)(struct dcb *dcb, const char *dev,
+				      int argc, char **argv),
+		      void (*help)(void))
+{
+	const char *dev;
+
+	if (!argc || matches(*argv, "help") == 0) {
+		help();
+		return 0;
+	} else if (matches(*argv, "dev") == 0) {
+		NEXT_ARG();
+		dev = *argv;
+		if (check_ifname(dev)) {
+			invarg("not a valid ifname", *argv);
+			return -EINVAL;
+		}
+		NEXT_ARG_FWD();
+		return and_then(dcb, dev, argc, argv);
+	} else {
+		fprintf(stderr, "Expected `dev DEV', not `%s'", *argv);
+		help();
+		return -EINVAL;
+	}
+}
+
+static void dcb_help(void)
+{
+	fprintf(stderr,
+		"Usage: dcb [ OPTIONS ] OBJECT { COMMAND | help }\n"
+		"       dcb [ -f[orce] ] -b[atch] filename -N[etns] netnsname\n"
+		"where  OBJECT :=\n"
+		"       OPTIONS := { -V[ersion] | -j[son] | -p[retty] | -v[erbose] }\n");
+}
+
+static int dcb_cmd(struct dcb *dcb, int argc, char **argv)
+{
+	if (!argc || matches(*argv, "help") == 0) {
+		dcb_help();
+		return 0;
+	}
+
+	fprintf(stderr, "Object \"%s\" is unknown\n", *argv);
+	return -ENOENT;
+}
+
+static int dcb_batch_cmd(int argc, char *argv[], void *data)
+{
+	struct dcb *dcb = data;
+
+	return dcb_cmd(dcb, argc, argv);
+}
+
+static int dcb_batch(struct dcb *dcb, const char *name, bool force)
+{
+	return do_batch(name, force, dcb_batch_cmd, dcb);
+}
+
+int main(int argc, char **argv)
+{
+	static const struct option long_options[] = {
+		{ "Version",		no_argument,		NULL, 'V' },
+		{ "force",		no_argument,		NULL, 'f' },
+		{ "batch",		required_argument,	NULL, 'b' },
+		{ "json",		no_argument,		NULL, 'j' },
+		{ "pretty",		no_argument,		NULL, 'p' },
+		{ "Netns",		required_argument,	NULL, 'N' },
+		{ NULL, 0, NULL, 0 }
+	};
+	const char *batch_file = NULL;
+	bool force = false;
+	struct dcb *dcb;
+	int opt;
+	int err;
+	int ret;
+
+	dcb = dcb_alloc();
+	if (!dcb) {
+		fprintf(stderr, "Failed to allocate memory for dcb\n");
+		return EXIT_FAILURE;
+	}
+
+	while ((opt = getopt_long(argc, argv, "Vfb:njpvsN:",
+				  long_options, NULL)) >= 0) {
+
+		switch (opt) {
+		case 'V':
+			printf("dcb utility, iproute2-%s\n", version);
+			ret = EXIT_SUCCESS;
+			goto dcb_free;
+		case 'f':
+			force = true;
+			break;
+		case 'b':
+			batch_file = optarg;
+			break;
+		case 'j':
+			dcb->json_output = true;
+			break;
+		case 'p':
+			pretty = true;
+			break;
+		case 'N':
+			if (netns_switch(optarg)) {
+				ret = EXIT_FAILURE;
+				goto dcb_free;
+			}
+			break;
+		default:
+			fprintf(stderr, "Unknown option.\n");
+			dcb_help();
+			ret = EXIT_FAILURE;
+			goto dcb_free;
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	err = dcb_init(dcb);
+	if (err) {
+		ret = EXIT_FAILURE;
+		goto dcb_free;
+	}
+
+	if (batch_file)
+		err = dcb_batch(dcb, batch_file, force);
+	else
+		err = dcb_cmd(dcb, argc, argv);
+
+	if (err) {
+		ret = EXIT_FAILURE;
+		goto dcb_fini;
+	}
+
+	ret = EXIT_SUCCESS;
+
+dcb_fini:
+	dcb_fini(dcb);
+dcb_free:
+	dcb_free(dcb);
+
+	return ret;
+}
diff --git a/dcb/dcb.h b/dcb/dcb.h
new file mode 100644
index 000000000000..7334ef7a94d8
--- /dev/null
+++ b/dcb/dcb.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DCB_H__
+#define __DCB_H__ 1
+
+#include <stdbool.h>
+#include <stddef.h>
+
+/* dcb.c */
+
+struct dcb {
+	char *buf;
+	struct mnl_socket *nl;
+	bool json_output;
+};
+
+int dcb_cmd_parse_dev(struct dcb *dcb, int argc, char **argv,
+		      int (*and_then)(struct dcb *dcb, const char *dev,
+				      int argc, char **argv),
+		      void (*help)(void));
+
+int dcb_get_attribute(struct dcb *dcb, const char *dev, int attr,
+		      void *data, size_t data_len);
+int dcb_set_attribute(struct dcb *dcb, const char *dev, int attr,
+		      const void *data, size_t data_len);
+
+void dcb_print_named_array(FILE *fp, const char *name, const __u8 *array, size_t size,
+			   void (*print_array)(FILE *, const __u8 *, size_t));
+void dcb_print_array_num(FILE *fp, const __u8 *array, size_t size);
+void dcb_print_array_kw(FILE *fp, const __u8 *array, size_t array_size,
+			const char *const kw[], size_t kw_size);
+
+#endif /* __DCB_H__ */
diff --git a/man/man8/dcb.8 b/man/man8/dcb.8
new file mode 100644
index 000000000000..25ddf204d60e
--- /dev/null
+++ b/man/man8/dcb.8
@@ -0,0 +1,103 @@
+.TH DCB 8 "19 October 2020" "iproute2" "Linux"
+.SH NAME
+dcb \- show / manipulate DCB (Data Center Bridging) settings
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+
+.ti -8
+.B dcb
+.RB "[ " -force " ] "
+.BI "-batch " filename
+.sp
+
+.ti -8
+.B dcb
+.RI "[ " OPTIONS " ] "
+.B help
+.sp
+
+.SH OPTIONS
+
+.TP
+.BR "\-V" , " --Version"
+Print the version of the
+.B dcb
+utility and exit.
+
+.TP
+.BR "\-b", " \-batch " <FILENAME>
+Read commands from provided file or standard input and invoke them. First
+failure will cause termination of dcb.
+
+.TP
+.B \-force
+Don't terminate dcb on errors in batch mode. If there were any errors during
+execution of the commands, the application return code will be non zero.
+
+.TP
+.BR "\-j" , " --json"
+Generate JSON output.
+
+.TP
+.BR "\-p" , " --pretty"
+When combined with -j generate a pretty JSON output.
+
+.SH OBJECTS
+
+.SH COMMANDS
+
+A \fICOMMAND\fR specifies the action to perform on the object. The set of
+possible actions depends on the object type. As a rule, it is possible to
+.B show
+objects and to invoke topical
+.B help,
+which prints a list of available commands and argument syntax conventions.
+
+.SH ARRAY PARAMETERS
+
+Like commands, specification of parameters is in the domain of individual
+objects (and their commands) as well. However, much of the DCB interface
+revolves around arrays of fixed size that specify one value per some key, such
+as per traffic class or per priority. There is therefore a single syntax for
+adjusting elements of these arrays. It consists of a series of
+\fIKEY\fB:\fIVALUE\fR pairs, where the meaning of the individual keys and values
+depends on the parameter.
+
+The elements are evaluated in order from left to right, and the latter ones
+override the earlier ones. The elements that are not specified on the command
+line are queried from the kernel and their current value is retained.
+
+As an example, take a made-up parameter tc-juju, which can be set to charm
+traffic in a given TC with either good luck or bad luck. \fIKEY\fR can therefore
+be 0..7 (as is usual for TC numbers in DCB), and \fIVALUE\fR either of
+\fBnone\fR, \fBgood\fR, and \fBbad\fR. An example of changing a juju value of
+TCs 0 and 7, while leaving all other intact, would then be:
+
+.P
+# dcb foo set dev eth0 tc-juju 0:good 7:bad
+
+A special key, \fBall\fR, is recognized which sets the same value to all array
+elements. This can be combined with the usual single-element syntax. E.g. in the
+following, the juju or all keys is set to \fBnone\fR, except 0 and 7, which have
+other values:
+
+.P
+# dcb foo set dev eth0 tc-juju all:none 0:good 7:bad
+
+.SH EXIT STATUS
+Exit status is 0 if command was successful or a positive integer upon failure.
+
+.SH SEE ALSO
+.BR dcb-ets (8)
+.br
+
+.SH REPORTING BUGS
+Report any bugs to the Network Developers mailing list
+.B <netdev@vger.kernel.org>
+where the development and maintenance is primarily done.
+You do not have to be subscribed to the list to send a message there.
+
+.SH AUTHOR
+Petr Machata <me@pmachata.org>
-- 
2.25.1

[PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

[Petr Machata]

ETS, for "Enhanced Transmission Selection", is a set of configurations that
permit configuration of mapping of priorities to traffic classes, traffic
selection algorithm to use per traffic class, bandwidth allocation, etc.

Add a dcb subtool to allow showing and tweaking of individual ETS
configuration options. For example:

    # dcb ets show dev eni1np1
    willing on ets_cap 8 cbs off
    tc-bw 0:0 1:0 2:0 3:0 4:100 5:0 6:0 7:0
    pg-bw 0:0 1:0 2:0 3:0 4:0 5:0 6:0 7:0
    tc-tsa 0:strict 1:strict 2:strict 3:strict 4:ets 5:strict 6:strict 7:strict
    prio-tc 0:1 1:3 2:5 3:0 4:0 5:0 6:0 7:0
    reco-tc-bw 0:0 1:0 2:0 3:0 4:0 5:0 6:0 7:0
    reco-tc-tsa 0:strict 1:strict 2:strict 3:strict 4:strict 5:strict 6:strict 7:strict
    reco-prio-tc 0:0 1:0 2:0 3:0 4:0 5:0 6:0 7:0

Signed-off-by: Petr Machata <me@pmachata.org>
---
 dcb/Makefile       |   2 +-
 dcb/dcb.c          |   4 +-
 dcb/dcb.h          |   4 +
 dcb/dcb_ets.c      | 450 +++++++++++++++++++++++++++++++++++++++++++++
 man/man8/dcb-ets.8 | 185 +++++++++++++++++++
 man/man8/dcb.8     |  11 ++
 6 files changed, 654 insertions(+), 2 deletions(-)
 create mode 100644 dcb/dcb_ets.c
 create mode 100644 man/man8/dcb-ets.8

diff --git a/dcb/Makefile b/dcb/Makefile
index 9966c8f0bfa4..895817163562 100644
--- a/dcb/Makefile
+++ b/dcb/Makefile
@@ -5,7 +5,7 @@ TARGETS :=
 
 ifeq ($(HAVE_MNL),y)
 
-DCBOBJ = dcb.o
+DCBOBJ = dcb.o dcb_ets.o
 TARGETS += dcb
 
 endif
diff --git a/dcb/dcb.c b/dcb/dcb.c
index c85008bbe1e9..7df69b065522 100644
--- a/dcb/dcb.c
+++ b/dcb/dcb.c
@@ -262,7 +262,7 @@ static void dcb_help(void)
 	fprintf(stderr,
 		"Usage: dcb [ OPTIONS ] OBJECT { COMMAND | help }\n"
 		"       dcb [ -f[orce] ] -b[atch] filename -N[etns] netnsname\n"
-		"where  OBJECT :=\n"
+		"where  OBJECT := ets\n"
 		"       OPTIONS := { -V[ersion] | -j[son] | -p[retty] | -v[erbose] }\n");
 }
 
@@ -271,6 +271,8 @@ static int dcb_cmd(struct dcb *dcb, int argc, char **argv)
 	if (!argc || matches(*argv, "help") == 0) {
 		dcb_help();
 		return 0;
+	} else if (matches(*argv, "ets") == 0) {
+		return dcb_cmd_ets(dcb, argc - 1, argv + 1);
 	}
 
 	fprintf(stderr, "Object \"%s\" is unknown\n", *argv);
diff --git a/dcb/dcb.h b/dcb/dcb.h
index 7334ef7a94d8..12aa9c9d2427 100644
--- a/dcb/dcb.h
+++ b/dcb/dcb.h
@@ -29,4 +29,8 @@ void dcb_print_array_num(FILE *fp, const __u8 *array, size_t size);
 void dcb_print_array_kw(FILE *fp, const __u8 *array, size_t array_size,
 			const char *const kw[], size_t kw_size);
 
+/* dcb_ets.c */
+
+int dcb_cmd_ets(struct dcb *dcb, int argc, char **argv);
+
 #endif /* __DCB_H__ */
diff --git a/dcb/dcb_ets.c b/dcb/dcb_ets.c
new file mode 100644
index 000000000000..c0d6cede5411
--- /dev/null
+++ b/dcb/dcb_ets.c
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <errno.h>
+#include <stdio.h>
+#include <linux/dcbnl.h>
+
+#include "dcb.h"
+#include "utils.h"
+
+static void dcb_ets_help_set(void)
+{
+	fprintf(stderr,
+		"Usage: dcb ets set dev STRING\n"
+		"           [ willing { on | off } ]\n"
+		"           [ { tc-tsa | reco-tc-tsa } TSA-MAP ]\n"
+		"           [ { pg-bw | tc-bw | reco-tc-bw } BW-MAP ]\n"
+		"           [ { prio-tc | reco-prio-tc } PRIO-MAP ]\n"
+		"\n"
+		" where TSA-MAP := [ TSA-MAP ] TSA-MAPPING\n"
+		"       TSA-MAPPING := { all | TC }:{ strict | cbs | ets | vendor }\n"
+		"       BW-MAP := [ BW-MAP ] BW-MAPPING\n"
+		"       BW-MAPPING := { all | TC }:INTEGER\n"
+		"       PRIO-MAP := [ PRIO-MAP ] PRIO-MAPPING\n"
+		"       PRIO-MAPPING := { all | PRIO }:TC\n"
+		"       TC := { 0 .. 7 }\n"
+		"       PRIO := { 0 .. 7 }\n"
+		"\n"
+	);
+}
+
+static void dcb_ets_help_show(void)
+{
+	fprintf(stderr,
+		"Usage: dcb ets show dev STRING\n"
+		"           [ willing | ets-cap | cbs | tc-tsa | reco-tc-tsa |\n"
+		"             pg-bw | tc-bw | reco-tc-bw | prio-tc |\n"
+		"             reco-prio-tc ]\n"
+		"\n"
+	);
+}
+
+static void dcb_ets_help(void)
+{
+	fprintf(stderr,
+		"Usage: dcb ets help\n"
+		"\n"
+	);
+	dcb_ets_help_show();
+	dcb_ets_help_set();
+}
+
+static const char *const tsa_names[] = {
+	[IEEE_8021QAZ_TSA_STRICT] = "strict",
+	[IEEE_8021QAZ_TSA_CB_SHAPER] = "cbs",
+	[IEEE_8021QAZ_TSA_ETS] = "ets",
+	[IEEE_8021QAZ_TSA_VENDOR] = "vendor",
+};
+
+static int dcb_ets_parse_mapping(__u32 key, __u8 value, __u8 max_value,
+				 __u8 *array, const char *what)
+{
+	bool is_all = key == (__u32) -1;
+
+	if (!is_all && key >= IEEE_8021QAZ_MAX_TCS) {
+		fprintf(stderr, "In %s mapping, TC is expected to be 0..%d\n", what,
+			IEEE_8021QAZ_MAX_TCS - 1);
+		return -EINVAL;
+	}
+
+	if (value > max_value) {
+		fprintf(stderr, "In %s mapping, the value is expected to be 0..%d\n",
+			what, max_value);
+		return -EINVAL;
+	}
+
+	if (is_all) {
+		for (key = 0; key < IEEE_8021QAZ_MAX_TCS; key++)
+			array[key] = value;
+	} else {
+		array[key] = value;
+	}
+
+	return 0;
+}
+
+static int dcb_ets_parse_mapping_tc_tsa(__u32 key, char *value, void *data)
+{
+	__u8 tsa;
+	int ret;
+
+	tsa = parse_one_of("TSA", value, tsa_names, ARRAY_SIZE(tsa_names), &ret);
+	if (ret)
+		return ret;
+
+	return dcb_ets_parse_mapping(key, tsa, -1, data, "TC:TSA");
+}
+
+static int dcb_ets_parse_mapping_tc_int(__u32 key, char *value, __u8 max_value,
+					__u8 *array, const char *what)
+{
+	__u8 int_value;
+
+	if (get_u8(&int_value, value, 0))
+		return -EINVAL;
+
+	return dcb_ets_parse_mapping(key, int_value, max_value, array, what);
+}
+
+static int dcb_ets_parse_mapping_tc_bw(__u32 key, char *value, void *data)
+{
+	return dcb_ets_parse_mapping_tc_int(key, value, 100, data, "TC:BW");
+}
+
+static int dcb_ets_parse_mapping_prio_tc(unsigned int key, char *value, void *data)
+{
+	return dcb_ets_parse_mapping_tc_int(key, value, IEEE_8021QAZ_MAX_TCS, data, "PRIO:TC");
+}
+
+static void dcb_print_array_tsa(FILE *fp, const __u8 *array, size_t size)
+{
+	dcb_print_array_kw(fp, array, size, tsa_names, ARRAY_SIZE(tsa_names));
+}
+
+static void dcb_ets_print_willing(FILE *fp, const struct ieee_ets *ets)
+{
+	print_string(PRINT_ANY, "willing", "willing %s ", ets->willing ? "on" : "off");
+}
+
+static void dcb_ets_print_ets_cap(FILE *fp, const struct ieee_ets *ets)
+{
+	print_uint(PRINT_ANY, "ets_cap", "ets_cap %d ", ets->ets_cap);
+}
+
+static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets *ets)
+{
+	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" : "off");
+}
+
+static void dcb_ets_print_tc_bw(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_print_named_array(fp, "tc-bw", ets->tc_tx_bw, ARRAY_SIZE(ets->tc_tx_bw),
+			      dcb_print_array_num);
+}
+
+static void dcb_ets_print_pg_bw(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_print_named_array(fp, "pg-bw", ets->tc_rx_bw, ARRAY_SIZE(ets->tc_rx_bw),
+			      dcb_print_array_num);
+}
+
+static void dcb_ets_print_tc_tsa(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_print_named_array(fp, "tc-tsa", ets->tc_tsa, ARRAY_SIZE(ets->tc_tsa),
+			      dcb_print_array_tsa);
+}
+
+static void dcb_ets_print_prio_tc(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_print_named_array(fp, "prio-tc", ets->prio_tc, ARRAY_SIZE(ets->prio_tc),
+			      dcb_print_array_num);
+}
+
+static void dcb_ets_print_reco_tc_bw(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_print_named_array(fp, "reco-tc-bw", ets->tc_reco_bw, ARRAY_SIZE(ets->tc_reco_bw),
+			      dcb_print_array_num);
+}
+
+static void dcb_ets_print_reco_tc_tsa(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_print_named_array(fp, "reco-tc-tsa", ets->tc_reco_tsa, ARRAY_SIZE(ets->tc_reco_tsa),
+			      dcb_print_array_tsa);
+}
+
+static void dcb_ets_print_reco_prio_tc(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_print_named_array(fp, "reco-prio-tc", ets->reco_prio_tc, ARRAY_SIZE(ets->reco_prio_tc),
+			      dcb_print_array_num);
+}
+
+static void dcb_ets_print(FILE *fp, const struct ieee_ets *ets)
+{
+	dcb_ets_print_willing(fp, ets);
+	dcb_ets_print_ets_cap(fp, ets);
+	dcb_ets_print_cbs(fp, ets);
+	print_nl();
+
+	dcb_ets_print_tc_bw(fp, ets);
+	print_nl();
+
+	dcb_ets_print_pg_bw(fp, ets);
+	print_nl();
+
+	dcb_ets_print_tc_tsa(fp, ets);
+	print_nl();
+
+	dcb_ets_print_prio_tc(fp, ets);
+	print_nl();
+
+	dcb_ets_print_reco_tc_bw(fp, ets);
+	print_nl();
+
+	dcb_ets_print_reco_tc_tsa(fp, ets);
+	print_nl();
+
+	dcb_ets_print_reco_prio_tc(fp, ets);
+	print_nl();
+}
+
+static int dcb_ets_get(struct dcb *dcb, const char *dev, struct ieee_ets *ets)
+{
+	return dcb_get_attribute(dcb, dev, DCB_ATTR_IEEE_ETS, ets, sizeof(*ets));
+}
+
+static int dcb_ets_validate_bw(const __u8 bw[], const __u8 tsa[], const char *what)
+{
+	bool has_ets = false;
+	unsigned int total = 0;
+	unsigned int tc;
+
+	for (tc = 0; tc < IEEE_8021QAZ_MAX_TCS; tc++) {
+		if (tsa[tc] == IEEE_8021QAZ_TSA_ETS) {
+			has_ets = true;
+			break;
+		}
+	}
+
+	/* TC bandwidth is only intended for ETS, but 802.1Q-2018 only requires
+	 * that the sum be 100, and individual entries 0..100. It explicitly
+	 * notes that non-ETS TCs can have non-0 TC bandwidth during
+	 * reconfiguration.
+	 */
+	for (tc = 0; tc < IEEE_8021QAZ_MAX_TCS; tc++) {
+		if (bw[tc] > 100) {
+			fprintf(stderr, "%d%% for TC %d of %s is not a valid bandwidth percentage, expected 0..100%%\n",
+				bw[tc], tc, what);
+			return -EINVAL;
+		}
+		total += bw[tc];
+	}
+
+	/* This is what 802.1Q-2018 requires. */
+	if (total == 100)
+		return 0;
+
+	/* But this requirement does not make sense for all-strict
+	 * configurations. Anything else than 0 does not make sense: either BW
+	 * has not been reconfigured for the all-strict allocation yet, at which
+	 * point we expect sum of 100. Or it has already been reconfigured, at
+	 * which point accept 0.
+	 */
+	if (!has_ets && total == 0)
+		return 0;
+
+	fprintf(stderr, "Bandwidth percentages in %s sum to %d%%, expected %d%%\n",
+		what, total, has_ets ? 100 : 0);
+	return -EINVAL;
+}
+
+static int dcb_ets_set(struct dcb *dcb, const char *dev, const struct ieee_ets *ets)
+{
+	/* Do not validate pg-bw, which is not standard and has unclear
+	 * meaning.
+	 */
+	if (dcb_ets_validate_bw(ets->tc_tx_bw, ets->tc_tsa, "tc-bw") ||
+	    dcb_ets_validate_bw(ets->tc_reco_bw, ets->tc_reco_tsa, "reco-tc-bw"))
+		return -EINVAL;
+
+	return dcb_set_attribute(dcb, dev, DCB_ATTR_IEEE_ETS, ets, sizeof(*ets));
+}
+
+static int dcb_cmd_ets_set(struct dcb *dcb, const char *dev, int argc, char **argv)
+{
+	struct ieee_ets ets;
+	int ret;
+
+	if (!argc) {
+		dcb_ets_help_set();
+		return 0;
+	}
+
+	ret = dcb_ets_get(dcb, dev, &ets);
+	if (ret)
+		return ret;
+
+	do {
+		if (matches(*argv, "help") == 0) {
+			dcb_ets_help_set();
+			return 0;
+		} else if (matches(*argv, "willing") == 0) {
+			NEXT_ARG();
+			ets.willing = parse_on_off("willing", *argv, &ret);
+			if (ret)
+				return ret;
+		} else if (matches(*argv, "tc-tsa") == 0) {
+			NEXT_ARG();
+			ret = parse_mapping(&argc, &argv, true, &dcb_ets_parse_mapping_tc_tsa,
+					    ets.tc_tsa);
+			if (ret) {
+				fprintf(stderr, "Invalid tc-tsa mapping %s\n", *argv);
+				return ret;
+			}
+			continue;
+		} else if (matches(*argv, "reco-tc-tsa") == 0) {
+			NEXT_ARG();
+			ret = parse_mapping(&argc, &argv, true, &dcb_ets_parse_mapping_tc_tsa,
+					    ets.tc_reco_tsa);
+			if (ret) {
+				fprintf(stderr, "Invalid reco-tc-tsa mapping %s\n", *argv);
+				return ret;
+			}
+			continue;
+		} else if (matches(*argv, "tc-bw") == 0) {
+			NEXT_ARG();
+			ret = parse_mapping(&argc, &argv, true, &dcb_ets_parse_mapping_tc_bw,
+					    ets.tc_tx_bw);
+			if (ret) {
+				fprintf(stderr, "Invalid tc-bw mapping %s\n", *argv);
+				return ret;
+			}
+			continue;
+		} else if (matches(*argv, "pg-bw") == 0) {
+			NEXT_ARG();
+			ret = parse_mapping(&argc, &argv, true, &dcb_ets_parse_mapping_tc_bw,
+					    ets.tc_rx_bw);
+			if (ret) {
+				fprintf(stderr, "Invalid pg-bw mapping %s\n", *argv);
+				return ret;
+			}
+			continue;
+		} else if (matches(*argv, "reco-tc-bw") == 0) {
+			NEXT_ARG();
+			ret = parse_mapping(&argc, &argv, true, &dcb_ets_parse_mapping_tc_bw,
+					    ets.tc_reco_bw);
+			if (ret) {
+				fprintf(stderr, "Invalid reco-tc-bw mapping %s\n", *argv);
+				return ret;
+			}
+			continue;
+		} else if (matches(*argv, "prio-tc") == 0) {
+			NEXT_ARG();
+			ret = parse_mapping(&argc, &argv, true, &dcb_ets_parse_mapping_prio_tc,
+					    ets.prio_tc);
+			if (ret) {
+				fprintf(stderr, "Invalid prio-tc mapping %s\n", *argv);
+				return ret;
+			}
+			continue;
+		} else if (matches(*argv, "reco-prio-tc") == 0) {
+			NEXT_ARG();
+			ret = parse_mapping(&argc, &argv, true, &dcb_ets_parse_mapping_prio_tc,
+					    ets.reco_prio_tc);
+			if (ret) {
+				fprintf(stderr, "Invalid reco-prio-tc mapping %s\n", *argv);
+				return ret;
+			}
+			continue;
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			dcb_ets_help_set();
+			return -EINVAL;
+		}
+
+		NEXT_ARG_FWD();
+	} while (argc > 0);
+
+	return dcb_ets_set(dcb, dev, &ets);
+}
+
+static int dcb_cmd_ets_show(struct dcb *dcb, const char *dev, int argc, char **argv)
+{
+	struct ieee_ets ets;
+	int ret;
+
+	ret = dcb_ets_get(dcb, dev, &ets);
+	if (ret)
+		return ret;
+
+	open_json_object(NULL);
+
+	if (!argc) {
+		dcb_ets_print(stdout, &ets);
+		goto out;
+	}
+
+	do {
+		if (matches(*argv, "help") == 0) {
+			dcb_ets_help();
+			return 0;
+		} else if (matches(*argv, "willing") == 0) {
+			dcb_ets_print_willing(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "ets-cap") == 0) {
+			dcb_ets_print_ets_cap(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "cbs") == 0) {
+			dcb_ets_print_cbs(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "tc-tsa") == 0) {
+			dcb_ets_print_tc_tsa(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "reco-tc-tsa") == 0) {
+			dcb_ets_print_reco_tc_tsa(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "tc-bw") == 0) {
+			dcb_ets_print_tc_bw(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "pg-bw") == 0) {
+			dcb_ets_print_pg_bw(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "reco-tc-bw") == 0) {
+			dcb_ets_print_reco_tc_bw(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "prio-tc") == 0) {
+			dcb_ets_print_prio_tc(stdout, &ets);
+			print_nl();
+		} else if (matches(*argv, "reco-prio-tc") == 0) {
+			dcb_ets_print_reco_prio_tc(stdout, &ets);
+			print_nl();
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			dcb_ets_help();
+			return -EINVAL;
+		}
+
+		NEXT_ARG_FWD();
+	} while (argc > 0);
+
+out:
+	close_json_object();
+	return 0;
+}
+
+int dcb_cmd_ets(struct dcb *dcb, int argc, char **argv)
+{
+	if (!argc || matches(*argv, "help") == 0) {
+		dcb_ets_help();
+		return 0;
+	} else if (matches(*argv, "show") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_ets_show, dcb_ets_help_show);
+	} else if (matches(*argv, "set") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_ets_set, dcb_ets_help_set);
+	} else {
+		fprintf(stderr, "What is \"%s\"?\n", *argv);
+		dcb_ets_help();
+		return -EINVAL;
+	}
+}
diff --git a/man/man8/dcb-ets.8 b/man/man8/dcb-ets.8
new file mode 100644
index 000000000000..5286199f180f
--- /dev/null
+++ b/man/man8/dcb-ets.8
@@ -0,0 +1,185 @@
+.TH DCB-ETS 8 "19 October 2020" "iproute2" "Linux"
+.SH NAME
+dcb-ets \- show / manipulate ETS (Enhanced Transmission Selection) settings of
+the DCB (Data Center Bridging) subsystem
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+
+.ti -8
+.B dcb
+.RI "[ " OPTIONS " ] "
+.B ets
+.RI "{ " COMMAND " | " help " }"
+.sp
+
+.ti -8
+.B dcb ets show dev
+.RI DEV
+.B "[ {" willing "|" ets-cap "|" cbs "|" tc-tsa "|" reco-tc-tsa "|"
+.B pg-bw "|" tc-bw "|" reco-tc-bw "|" prio-tc "|"
+.B reco-prio-tc "} ]"
+
+.ti -8
+.B dcb ets set dev
+.RI DEV
+.B "[" willing "{" on "|" off "} ]"
+.B "[ {" tc-tsa "|" reco-tc-tsa "}" \fITSA-MAP\fB "]"
+.B "[ {" pg-bw "|" tc-bw "|" reco-tc-bw "}" \fIBW-MAP\fB "]"
+.B "[ {" prio-tc "|" reco-prio-tc "}" \fIPRIO-MAP\fB "]"
+
+.ti -8
+.IR TSA-MAP " := [ " TSA-MAP " ] " TSA-MAPPING
+
+.ti -8
+.IR TSA-MAPPING " := { " TC " | " \fBall " }" \fB: "{ " \fBstrict\fR " | "
+.IR \fBcbs\fR " | " \fBets\fR " | " \fBvendor\fR " }"
+
+.ti -8
+.IR BW-MAP " := [ " BW-MAP " ] " BW-MAPPING
+
+.ti -8
+.IR BW-MAPPING " := { " TC " | " \fBall " }" \fB:\fIINTEGER\fR
+
+.ti -8
+.IR PRIO-MAP " := [ " PRIO-MAP " ] " PRIO-MAPPING
+
+.ti -8
+.IR PRIO-MAPPING " := { " PRIO " | " \fBall " }" \fB:\fITC\fR
+
+.ti -8
+.IR TC " := { " \fB0\fR " .. " \fB7\fR " }"
+
+.ti -8
+.IR PRIO " := { " \fB0\fR " .. " \fB7\fR " }"
+
+
+.SH DESCRIPTION
+
+.B dcb ets
+is used to configure Enhanced Transmission Selection attributes through Linux
+DCB (Data Center Bridging) interface. ETS permits configuration of mapping of
+priorities to traffic classes, traffic selection algorithm to use per traffic
+class, bandwidth allocation, etc.
+
+Two DCB TLVs are related to the ETS feature: a configuration and recommendation
+values. Recommendation values are named with a prefix
+.B reco-,
+while the configuration ones have plain names.
+
+.SH PARAMETERS
+
+For read-write parameters, the following describes only the write direction,
+i.e. as used with the \fBset\fR command. For the \fBshow\fR command, the
+parameter name is to be used as a simple keyword without further arguments. This
+instructs the tool to show the value of a given parameter. When no parameters
+are given, the tool shows the complete ETS configuration.
+
+.TP
+.B ets-cap
+A read-only property that shows the number of supported ETS traffic classes.
+
+.TP
+.B cbs
+A read-only property that is enabled if the driver and the hardware support the
+CBS Transmission Selection Algorithm.
+
+.TP
+.B willing \fR{ \fBon\fR | \fBoff\fR }
+Whether local host should accept configuration from peer TLVs.
+
+.TP
+.B prio-tc \fITC-MAP
+.TQ
+.B reco-prio-tc \fITC-MAP
+\fITC-MAP\fR uses the array parameter syntax, see dcb(8) for details. Keys are
+priorities, values are traffic classes. For each priority sets a TC where
+traffic with that priority is directed to.
+
+.TP
+.B tc-tsa \fITSA-MAP
+.TQ
+.B reco-tc-tsa \fITSA-MAP
+\fITC-MAP\fR uses the array parameter syntax, see dcb(8) for details. Keys are
+TCs, values are Transmission Selection Algorithm (TSA) keywords described below.
+For each TC sets an algorithm used for deciding how traffic queued up at this TC
+is scheduled for transmission. Supported TSAs are:
+
+.B strict
+- for strict priority, where traffic in higher-numbered TCs always takes
+precedence over traffic in lower-numbered TCs.
+.br
+.B ets
+- for Enhanced Traffic Selection, where available bandwidth is distributed among
+the ETS-enabled TCs according to the weights set by
+.B tc-bw
+and
+.B reco-tc-bw\fR,
+respectively.
+.br
+.B cbs
+- for Credit Based Shaper, where traffic is scheduled in a strict manner up to
+the limit set by a shaper.
+.br
+.B vendor
+- for vendor-specific traffic selection algorithm.
+
+.TP
+.B tc-bw \fIBW-MAP
+.TQ
+.B reco-tc-bw \fIBW-MAP
+\fIBW-MAP\fR uses the array parameter syntax, see dcb(8) for details. Keys are
+TCs, values are integers representing percent of available bandwidth given to
+the traffic class in question. The value should be 0 for TCs whose TSA is not
+\fBets\fR, and the sum of all values shall be 100. As an exception to the
+standard wording, a configuration with no ETS TCs is permitted to sum up to 0
+instead.
+.br
+
+.TP
+.B pg-bw \fIBW-MAP
+The precise meaning of \fBpg-bw\fR is not standardized, but the assumption seems
+to be that the same scheduling process as on the transmit side is applicable on
+receive side as well, and configures receive bandwidth allocation for \fBets\fR
+ingress traffic classes (priority groups).
+
+.SH EXAMPLE & USAGE
+
+Configure ETS priomap in a one-to-one fashion:
+
+.P
+# dcb ets set dev eth0 prio-tc 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
+
+Set TSA and transmit bandwidth configuration:
+
+.P
+# dcb ets set dev eth0 tc-tsa all:strict 0:ets 1:ets 2:ets \\
+.br
+                       tc-bw all:0 0:33 1:33 2:34
+
+Show what was set:
+
+.P
+# dcb ets show dev eth0 prio-tc tc-tsa tc-bw
+.br
+prio-tc 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
+.br
+tc-tsa 0:ets 1:ets 2:ets 3:strict 4:strict 5:strict 6:strict 7:strict
+.br
+tc-bw 0:33 1:33 2:34 3:0 4:0 5:0 6:0 7:0
+
+.SH EXIT STATUS
+Exit status is 0 if command was successful or a positive integer upon failure.
+
+.SH SEE ALSO
+.BR dcb (8)
+
+.SH REPORTING BUGS
+Report any bugs to the Network Developers mailing list
+.B <netdev@vger.kernel.org>
+where the development and maintenance is primarily done.
+You do not have to be subscribed to the list to send a message there.
+
+.SH AUTHOR
+Petr Machata <me@pmachata.org>
diff --git a/man/man8/dcb.8 b/man/man8/dcb.8
index 25ddf204d60e..ec116f6ce265 100644
--- a/man/man8/dcb.8
+++ b/man/man8/dcb.8
@@ -6,6 +6,13 @@ dcb \- show / manipulate DCB (Data Center Bridging) settings
 .ad l
 .in +8
 
+.ti -8
+.B dcb
+.RI "[ " OPTIONS " ] "
+.B ets
+.RI "{ " COMMAND " | " help " }"
+.sp
+
 .ti -8
 .B dcb
 .RB "[ " -force " ] "
@@ -46,6 +53,10 @@ When combined with -j generate a pretty JSON output.
 
 .SH OBJECTS
 
+.TP
+.B ets
+- Configuration of ETS (Enhanced Transmission Selection)
+
 .SH COMMANDS
 
 A \fICOMMAND\fR specifies the action to perform on the object. The set of
-- 
2.25.1

Re: [PATCH iproute2-next 11/15] lib: Extract from iplink_vlan a helper to parse key:value arrays

[Roman Mashak]

Petr Machata <me@pmachata.org> writes:


[...]

> +static int parse_qos_mapping(__u32 key, char *value, void *data)
> +{
> +	struct nlmsghdr *n = data;
> +	struct ifla_vlan_qos_mapping m = {
> +		.from = key,
> +	};
> +
> +	if (get_u32(&m.to, value, 0))
> +		return 1;
> +
> +	addattr_l(n, 1024, IFLA_VLAN_QOS_MAPPING, &m, sizeof(m));
> +	return 0;
> +}

addatr_l() may fail if netlink buffer size is not sufficient, may be:

return addattr_l(...);

would be better.

Re: [PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

[Jakub Kicinski]

On Tue, 20 Oct 2020 02:58:23 +0200 Petr Machata wrote:
> +static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets *ets)
> +{
> +	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" : "off");
> +}

I'd personally lean in the direction ethtool is taking and try to limit
string values in json output as much as possible. This would be a good
fit for bool.

Re: [PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Tue, 20 Oct 2020 02:58:23 +0200 Petr Machata wrote:
>> +static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets *ets)
>> +{
>> +	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" : "off");
>> +}
>
> I'd personally lean in the direction ethtool is taking and try to limit
> string values in json output as much as possible. This would be a good
> fit for bool.

Yep, makes sense. The value is not user-toggleable, so the on / off
there is just arbitrary.

I'll consider it for "willing" as well. That one is user-toggleable, and
the "on" / "off" makes sense for consistency with the command line. But
that doesn't mean it can't be a boolean in JSON.

Re: [PATCH iproute2-next 11/15] lib: Extract from iplink_vlan a helper to parse key:value arrays

[Petr Machata]

Roman Mashak <mrv@mojatatu.com> writes:

> Petr Machata <me@pmachata.org> writes:
>
>
> [...]
>
>> +static int parse_qos_mapping(__u32 key, char *value, void *data)
>> +{
>> +	struct nlmsghdr *n = data;
>> +	struct ifla_vlan_qos_mapping m = {
>> +		.from = key,
>> +	};
>> +
>> +	if (get_u32(&m.to, value, 0))
>> +		return 1;
>> +
>> +	addattr_l(n, 1024, IFLA_VLAN_QOS_MAPPING, &m, sizeof(m));
>> +	return 0;
>> +}
>
> addatr_l() may fail if netlink buffer size is not sufficient, may be:
>
> return addattr_l(...);
>
> would be better.

Ack, makes sense to fix this since I'm moving the code around anyway.

Re: [PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

[Stephen Hemminger]

On Tue, 20 Oct 2020 22:43:37 +0200
Petr Machata <me@pmachata.org> wrote:

> Jakub Kicinski <kuba@kernel.org> writes:
> 
> > On Tue, 20 Oct 2020 02:58:23 +0200 Petr Machata wrote:  
> >> +static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets *ets)
> >> +{
> >> +	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" : "off");
> >> +}  
> >
> > I'd personally lean in the direction ethtool is taking and try to limit
> > string values in json output as much as possible. This would be a good
> > fit for bool.  
> 
> Yep, makes sense. The value is not user-toggleable, so the on / off
> there is just arbitrary.
> 
> I'll consider it for "willing" as well. That one is user-toggleable, and
> the "on" / "off" makes sense for consistency with the command line. But
> that doesn't mean it can't be a boolean in JSON.

There are three ways of representing a boolean. You chose the worst.
Option 1: is to use a json null value to indicate presence.
      this works well for a flag.
Option 2: is to use json bool.
	this looks awkward in non-json output
Option 3: is to use a string
     	but this makes the string output something harder to consume
	in json.

Re: [PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

[Petr Machata]

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Tue, 20 Oct 2020 22:43:37 +0200
> Petr Machata <me@pmachata.org> wrote:
>
>> Jakub Kicinski <kuba@kernel.org> writes:
>>
>> > On Tue, 20 Oct 2020 02:58:23 +0200 Petr Machata wrote:
>> >> +static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets *ets)
>> >> +{
>> >> +	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" : "off");
>> >> +}
>> >
>> > I'd personally lean in the direction ethtool is taking and try to limit
>> > string values in json output as much as possible. This would be a good
>> > fit for bool.
>>
>> Yep, makes sense. The value is not user-toggleable, so the on / off
>> there is just arbitrary.
>>
>> I'll consider it for "willing" as well. That one is user-toggleable, and
>> the "on" / "off" makes sense for consistency with the command line. But
>> that doesn't mean it can't be a boolean in JSON.
>
> There are three ways of representing a boolean. You chose the worst.
> Option 1: is to use a json null value to indicate presence.
>       this works well for a flag.
> Option 2: is to use json bool.
> 	this looks awkward in non-json output
> Option 3: is to use a string
>      	but this makes the string output something harder to consume
> 	in json.

What seems to be used commonly for these on/off toggles is the following
pattern:

	print_string(PRINT_FP, NULL, "willing %s ", ets->willing ? "on" : "off");
	print_bool(PRINT_JSON, "willing", NULL, true);

That way the JSON output is easy to query and the FP output is obvious
and compatible with the command line. Does that work for you?

Re: [PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

[Stephen Hemminger]

On Thu, 22 Oct 2020 01:48:58 +0200
Petr Machata <me@pmachata.org> wrote:

> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > On Tue, 20 Oct 2020 22:43:37 +0200
> > Petr Machata <me@pmachata.org> wrote:
> >  
> >> Jakub Kicinski <kuba@kernel.org> writes:
> >>  
> >> > On Tue, 20 Oct 2020 02:58:23 +0200 Petr Machata wrote:  
> >> >> +static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets *ets)
> >> >> +{
> >> >> +	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" : "off");
> >> >> +}  
> >> >
> >> > I'd personally lean in the direction ethtool is taking and try to limit
> >> > string values in json output as much as possible. This would be a good
> >> > fit for bool.  
> >>
> >> Yep, makes sense. The value is not user-toggleable, so the on / off
> >> there is just arbitrary.
> >>
> >> I'll consider it for "willing" as well. That one is user-toggleable, and
> >> the "on" / "off" makes sense for consistency with the command line. But
> >> that doesn't mean it can't be a boolean in JSON.  
> >
> > There are three ways of representing a boolean. You chose the worst.
> > Option 1: is to use a json null value to indicate presence.
> >       this works well for a flag.
> > Option 2: is to use json bool.
> > 	this looks awkward in non-json output
> > Option 3: is to use a string
> >      	but this makes the string output something harder to consume
> > 	in json.  
> 
> What seems to be used commonly for these on/off toggles is the following
> pattern:
> 
> 	print_string(PRINT_FP, NULL, "willing %s ", ets->willing ? "on" : "off");
> 	print_bool(PRINT_JSON, "willing", NULL, true);
> 
> That way the JSON output is easy to query and the FP output is obvious
> and compatible with the command line. Does that work for you?

Yes, that is hybrid, maybe it should be a helper function?

Re: [PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

[Petr Machata]

On October 22, 2020 2:11:01 AM GMT+02:00, Stephen Hemminger <stephen@networkplumber.org> wrote:
>On Thu, 22 Oct 2020 01:48:58 +0200
>Petr Machata <me@pmachata.org> wrote:
>
>> Stephen Hemminger <stephen@networkplumber.org> writes:
>> 
>> > On Tue, 20 Oct 2020 22:43:37 +0200
>> > Petr Machata <me@pmachata.org> wrote:
>> >  
>> >> Jakub Kicinski <kuba@kernel.org> writes:
>> >>  
>> >> > On Tue, 20 Oct 2020 02:58:23 +0200 Petr Machata wrote:  
>> >> >> +static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets
>*ets)
>> >> >> +{
>> >> >> +	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" :
>"off");
>> >> >> +}  
>> >> >
>> >> > I'd personally lean in the direction ethtool is taking and try
>to limit
>> >> > string values in json output as much as possible. This would be
>a good
>> >> > fit for bool.  
>> >>
>> >> Yep, makes sense. The value is not user-toggleable, so the on /
>off
>> >> there is just arbitrary.
>> >>
>> >> I'll consider it for "willing" as well. That one is
>user-toggleable, and
>> >> the "on" / "off" makes sense for consistency with the command
>line. But
>> >> that doesn't mean it can't be a boolean in JSON.  
>> >
>> > There are three ways of representing a boolean. You chose the
>worst.
>> > Option 1: is to use a json null value to indicate presence.
>> >       this works well for a flag.
>> > Option 2: is to use json bool.
>> > 	this looks awkward in non-json output
>> > Option 3: is to use a string
>> >      	but this makes the string output something harder to consume
>> > 	in json.  
>> 
>> What seems to be used commonly for these on/off toggles is the
>following
>> pattern:
>> 
>> 	print_string(PRINT_FP, NULL, "willing %s ", ets->willing ? "on" :
>"off");
>> 	print_bool(PRINT_JSON, "willing", NULL, true);
>> 
>> That way the JSON output is easy to query and the FP output is
>obvious
>> and compatible with the command line. Does that work for you?
>
>Yes, that is hybrid, maybe it should be a helper function?

Yep, I'll do the same dance as for the other helpers in the patch set. Currently this is cut and pasted all over the place.