[PATCH iproute2-next 0/9] Introduce new dcb-rewr subcommand

[PATCH iproute2-next 0/9] Introduce new dcb-rewr subcommand

[Daniel Machon]

========================================================================               
Introduction:
========================================================================

This series introduces a new DCB subcommand: rewr, which is used to
configure the in-kernel DCB rewrite table [1].

Rewrite support is added as a separate DCB subcommand, rather than an
APP opt-in flag or similar. This goes in line with what we did to dcbnl,
where rewrite is a separate object.  Obviously this requires a bit more
code to implement the new command, but much of the existing dcb-app code
(especially the bookkeeping code) can be reused. In some cases a little
adaptation is needed.

========================================================================
dcb-rewr parameters:
========================================================================

Initially, I have only made support for the prio-pcp and prio-dscp
parameters, as DSCP and PCP  are the only selectors that currently have
a user [2] and to be honest, I am not even sure it makes sense to add
dgram, stream, ethtype rewrite support - At least the rewriter of Sparx5
does not support this. Any input here is much appreciated!

Examples:

Rewrite DSCP to 63 for packets with priority 1
$ dcb rewr add dev eth0 prio-dscp 1:63

Rewrite PCP 7 and DEI to 1 for packets with priority 1
$ dcb rewr add dev eth0 prio-pcp 1:7de

A new manpage has been added, to cover the new dcb-rewr subcommand, and
its parameters. Also I took the liberty to clean up a few things in the
dcb-app manpage.

========================================================================               
Patch overview:
========================================================================

Patch #1 Exposes dcb-app and dcb-rewr shared functions in a new header
         file dcb_app.h.

Patch #2 Adds a new field 'attr' to the dcb_app_table struct, which is
         used to distinguish app and rewrite tables.

Patch #3 Modifies existing dcb-app print functions for reuse by
         dcb-rewr.

Patch #4 Modifies existing dcb-app function dcb_app_table_remove_replaced 
         for reuse by dcb-rewr

Patch #5 Modifies existing dcb-app function dcb_app_parse_mapping_cb for
         reuse by dcb-rewr.

Patch #6 Adds the new dcb-rewr subcommand with initial support for
         prio-pcp and prio-dscp rewrite.

Patch #7 Adds the dcb-rewr.8 manpage
Patch #8 Adds references to dcb-apptrust and dcb-rewr in the dcb.8
         manpage.

Patch #9 Cleans up the dcb-app.8 manpage.

[1] https://elixir.bootlin.com/linux/v6.4-rc1/source/net/dcb/dcbnl.c#L181
[2] https://elixir.bootlin.com/linux/v6.4-rc1/source/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c#L380

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
Daniel Machon (9):
      dcb: app: expose dcb-app functions in new header
      dcb: app: add new dcbnl attribute field
      dcb: app: modify dcb-app print functions for dcb-rewr reuse
      dcb: app: modify dcb_app_table_remove_replaced() for dcb-rewr reuse
      dcb: app: modify dcb_app_parse_mapping_cb for dcb-rewr reuse
      dcb: rewr: add new dcb-rewr subcommand
      man: dcb-rewr: add new manpage for dcb-rewr
      man: dcb: add additional references under 'SEE ALSO'
      man: dcb-app: clean up a few mistakes

 dcb/Makefile        |   3 +-
 dcb/dcb.c           |   4 +-
 dcb/dcb.h           |  10 +-
 dcb/dcb_app.c       | 165 +++++++++++++-------------
 dcb/dcb_app.h       |  61 ++++++++++
 dcb/dcb_rewr.c      | 332 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/dcb-app.8  |  10 +-
 man/man8/dcb-rewr.8 | 206 ++++++++++++++++++++++++++++++++
 man/man8/dcb.8      |   4 +-
 9 files changed, 702 insertions(+), 93 deletions(-)
---
base-commit: 9c7bdc9f3328fb3fd5e7b77eb7b86f6c62538143
change-id: 20230510-dcb-rewr-534b7ab637eb

Best regards,
-- 
Daniel Machon <daniel.machon@microchip.com>

[PATCH iproute2-next 1/9] dcb: app: expose dcb-app functions in new header

[Daniel Machon]

Add new headerfile dcb-app.h that exposes the functions required later
by dcb-rewr. The new dcb-rewr implementation will reuse much of the
existing dcb-app code.

I thought this called for a separate header file, instead of polluting
the existing dcb.h file.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/dcb.h     |  9 ++-------
 dcb/dcb_app.c | 54 ++++++++++++++++++------------------------------------
 dcb/dcb_app.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 43 deletions(-)

diff --git a/dcb/dcb.h b/dcb/dcb.h
index d40664f29dad..4c8a4aa25e0c 100644
--- a/dcb/dcb.h
+++ b/dcb/dcb.h
@@ -6,6 +6,8 @@
 #include <stdbool.h>
 #include <stddef.h>
 
+#include "dcb_app.h"
+
 /* dcb.c */
 
 struct dcb {
@@ -54,13 +56,6 @@ void dcb_print_array_on_off(const __u8 *array, size_t size);
 void dcb_print_array_kw(const __u8 *array, size_t array_size,
 			const char *const kw[], size_t kw_size);
 
-/* dcb_app.c */
-
-int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
-enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector);
-bool dcb_app_attr_type_validate(enum ieee_attrs_app type);
-bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
-
 /* dcb_apptrust.c */
 
 int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv);
diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
index eeb78e70f63f..df339babd8e6 100644
--- a/dcb/dcb_app.c
+++ b/dcb/dcb_app.c
@@ -10,8 +10,6 @@
 #include "utils.h"
 #include "rt_names.h"
 
-#define DCB_APP_PCP_MAX 15
-
 static const char *const pcp_names[DCB_APP_PCP_MAX + 1] = {
 	"0nd", "1nd", "2nd", "3nd", "4nd", "5nd", "6nd", "7nd",
 	"0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de"
@@ -22,6 +20,7 @@ static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = {
 	[DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP"
 };
 
+
 static void dcb_app_help_add(void)
 {
 	fprintf(stderr,
@@ -68,11 +67,6 @@ static void dcb_app_help(void)
 	dcb_app_help_add();
 }
 
-struct dcb_app_table {
-	struct dcb_app *apps;
-	size_t n_apps;
-};
-
 enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector)
 {
 	switch (selector) {
@@ -105,7 +99,7 @@ bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector)
 	return dcb_app_attr_type_get(selector) == type;
 }
 
-static void dcb_app_table_fini(struct dcb_app_table *tab)
+void dcb_app_table_fini(struct dcb_app_table *tab)
 {
 	free(tab->apps);
 }
@@ -124,8 +118,8 @@ static int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app)
 	return 0;
 }
 
-static void dcb_app_table_remove_existing(struct dcb_app_table *a,
-					  const struct dcb_app_table *b)
+void dcb_app_table_remove_existing(struct dcb_app_table *a,
+				   const struct dcb_app_table *b)
 {
 	size_t ia, ja;
 	size_t ib;
@@ -152,8 +146,8 @@ static void dcb_app_table_remove_existing(struct dcb_app_table *a,
 	a->n_apps = ja;
 }
 
-static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
-					  const struct dcb_app_table *b)
+void dcb_app_table_remove_replaced(struct dcb_app_table *a,
+				   const struct dcb_app_table *b)
 {
 	size_t ia, ja;
 	size_t ib;
@@ -189,8 +183,7 @@ static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
 	a->n_apps = ja;
 }
 
-static int dcb_app_table_copy(struct dcb_app_table *a,
-			      const struct dcb_app_table *b)
+int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b)
 {
 	size_t i;
 	int ret;
@@ -217,18 +210,12 @@ static int dcb_app_cmp_cb(const void *a, const void *b)
 	return dcb_app_cmp(a, b);
 }
 
-static void dcb_app_table_sort(struct dcb_app_table *tab)
+void dcb_app_table_sort(struct dcb_app_table *tab)
 {
 	qsort(tab->apps, tab->n_apps, sizeof(*tab->apps), dcb_app_cmp_cb);
 }
 
-struct dcb_app_parse_mapping {
-	__u8 selector;
-	struct dcb_app_table *tab;
-	int err;
-};
-
-static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
+void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
 {
 	struct dcb_app_parse_mapping *pm = data;
 	struct dcb_app app = {
@@ -260,7 +247,7 @@ static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data
 				 dcb_app_parse_mapping_cb, data);
 }
 
-static int dcb_app_parse_pcp(__u32 *key, const char *arg)
+int dcb_app_parse_pcp(__u32 *key, const char *arg)
 {
 	int i;
 
@@ -286,7 +273,7 @@ static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data)
 				 dcb_app_parse_mapping_cb, data);
 }
 
-static int dcb_app_parse_dscp(__u32 *key, const char *arg)
+int dcb_app_parse_dscp(__u32 *key, const char *arg)
 {
 	if (parse_mapping_num_all(key, arg) == 0)
 		return 0;
@@ -377,12 +364,12 @@ static bool dcb_app_is_default(const struct dcb_app *app)
 	       app->protocol == 0;
 }
 
-static bool dcb_app_is_dscp(const struct dcb_app *app)
+bool dcb_app_is_dscp(const struct dcb_app *app)
 {
 	return app->selector == IEEE_8021QAZ_APP_SEL_DSCP;
 }
 
-static bool dcb_app_is_pcp(const struct dcb_app *app)
+bool dcb_app_is_pcp(const struct dcb_app *app)
 {
 	return app->selector == DCB_APP_SEL_PCP;
 }
@@ -402,7 +389,7 @@ static bool dcb_app_is_port(const struct dcb_app *app)
 	return app->selector == IEEE_8021QAZ_APP_SEL_ANY;
 }
 
-static int dcb_app_print_key_dec(__u16 protocol)
+int dcb_app_print_key_dec(__u16 protocol)
 {
 	return print_uint(PRINT_ANY, NULL, "%d:", protocol);
 }
@@ -412,7 +399,7 @@ static int dcb_app_print_key_hex(__u16 protocol)
 	return print_uint(PRINT_ANY, NULL, "%x:", protocol);
 }
 
-static int dcb_app_print_key_dscp(__u16 protocol)
+int dcb_app_print_key_dscp(__u16 protocol)
 {
 	const char *name = rtnl_dsfield_get_name(protocol << 2);
 
@@ -422,7 +409,7 @@ static int dcb_app_print_key_dscp(__u16 protocol)
 	return print_uint(PRINT_ANY, NULL, "%d:", protocol);
 }
 
-static int dcb_app_print_key_pcp(__u16 protocol)
+int dcb_app_print_key_pcp(__u16 protocol)
 {
 	/* Print in numerical form, if protocol value is out-of-range */
 	if (protocol > DCB_APP_PCP_MAX)
@@ -577,7 +564,7 @@ static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
-static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab)
+int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab)
 {
 	uint16_t payload_len;
 	void *payload;
@@ -594,11 +581,6 @@ static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *t
 	return 0;
 }
 
-struct dcb_app_add_del {
-	const struct dcb_app_table *tab;
-	bool (*filter)(const struct dcb_app *app);
-};
-
 static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
 {
 	struct dcb_app_add_del *add_del = data;
@@ -620,7 +602,7 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
 	return 0;
 }
 
-static int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
+int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
 			   const struct dcb_app_table *tab,
 			   bool (*filter)(const struct dcb_app *))
 {
diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
new file mode 100644
index 000000000000..8e7b010dcf75
--- /dev/null
+++ b/dcb/dcb_app.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DCB_APP_H_
+#define __DCB_APP_H_
+
+struct dcb;
+
+struct dcb_app_table {
+	struct dcb_app *apps;
+	size_t n_apps;
+};
+
+struct dcb_app_add_del {
+	const struct dcb_app_table *tab;
+	bool (*filter)(const struct dcb_app *app);
+};
+
+struct dcb_app_parse_mapping {
+	__u8 selector;
+	struct dcb_app_table *tab;
+	int err;
+};
+
+#define DCB_APP_PCP_MAX 15
+
+int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
+
+int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab);
+int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
+		    const struct dcb_app_table *tab,
+		    bool (*filter)(const struct dcb_app *));
+
+void dcb_app_table_sort(struct dcb_app_table *tab);
+void dcb_app_table_fini(struct dcb_app_table *tab);
+int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b);
+void dcb_app_table_remove_replaced(struct dcb_app_table *a,
+				   const struct dcb_app_table *b);
+void dcb_app_table_remove_existing(struct dcb_app_table *a,
+				   const struct dcb_app_table *b);
+
+bool dcb_app_is_pcp(const struct dcb_app *app);
+bool dcb_app_is_dscp(const struct dcb_app *app);
+
+int dcb_app_print_key_dec(__u16 protocol);
+int dcb_app_print_key_dscp(__u16 protocol);
+int dcb_app_print_key_pcp(__u16 protocol);
+
+int dcb_app_parse_pcp(__u32 *key, const char *arg);
+int dcb_app_parse_dscp(__u32 *key, const char *arg);
+void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data);
+
+bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
+bool dcb_app_attr_type_validate(enum ieee_attrs_app type);
+enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector);
+
+#endif

-- 
2.34.1

[PATCH iproute2-next 2/9] dcb: app: add new dcbnl attribute field

[Daniel Machon]

Add a new field 'attr' to the dcb_app_table struct, in order to inject
different dcbnl get/set attributes for APP and rewrite. This is required
later, when a number of the existing dcb-app functions are refactored
for reuse by dcb-rewr.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/dcb_app.c | 18 +++++++++---------
 dcb/dcb_app.h |  1 +
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
index df339babd8e6..1d0da35f987d 100644
--- a/dcb/dcb_app.c
+++ b/dcb/dcb_app.c
@@ -570,7 +570,7 @@ int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab)
 	void *payload;
 	int ret;
 
-	ret = dcb_get_attribute_va(dcb, dev, DCB_ATTR_IEEE_APP_TABLE, &payload, &payload_len);
+	ret = dcb_get_attribute_va(dcb, dev, tab->attr, &payload, &payload_len);
 	if (ret != 0)
 		return ret;
 
@@ -588,7 +588,7 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
 	struct nlattr *nest;
 	size_t i;
 
-	nest = mnl_attr_nest_start(nlh, DCB_ATTR_IEEE_APP_TABLE);
+	nest = mnl_attr_nest_start(nlh, add_del->tab->attr);
 
 	for (i = 0; i < add_del->tab->n_apps; i++) {
 		const struct dcb_app *app = &add_del->tab->apps[i];
@@ -697,7 +697,7 @@ static int dcb_cmd_app_parse_add_del(struct dcb *dcb, const char *dev,
 
 static int dcb_cmd_app_add(struct dcb *dcb, const char *dev, int argc, char **argv)
 {
-	struct dcb_app_table tab = {};
+	struct dcb_app_table tab = { .attr = DCB_ATTR_IEEE_APP_TABLE };
 	int ret;
 
 	ret = dcb_cmd_app_parse_add_del(dcb, dev, argc, argv, &tab);
@@ -711,7 +711,7 @@ static int dcb_cmd_app_add(struct dcb *dcb, const char *dev, int argc, char **ar
 
 static int dcb_cmd_app_del(struct dcb *dcb, const char *dev, int argc, char **argv)
 {
-	struct dcb_app_table tab = {};
+	struct dcb_app_table tab = { .attr = DCB_ATTR_IEEE_APP_TABLE };
 	int ret;
 
 	ret = dcb_cmd_app_parse_add_del(dcb, dev, argc, argv, &tab);
@@ -725,7 +725,7 @@ static int dcb_cmd_app_del(struct dcb *dcb, const char *dev, int argc, char **ar
 
 static int dcb_cmd_app_show(struct dcb *dcb, const char *dev, int argc, char **argv)
 {
-	struct dcb_app_table tab = {};
+	struct dcb_app_table tab = { .attr = DCB_ATTR_IEEE_APP_TABLE };
 	int ret;
 
 	ret = dcb_app_get(dcb, dev, &tab);
@@ -777,7 +777,7 @@ out:
 
 static int dcb_cmd_app_flush(struct dcb *dcb, const char *dev, int argc, char **argv)
 {
-	struct dcb_app_table tab = {};
+	struct dcb_app_table tab = { .attr = DCB_ATTR_IEEE_APP_TABLE };
 	int ret;
 
 	ret = dcb_app_get(dcb, dev, &tab);
@@ -830,9 +830,9 @@ out:
 
 static int dcb_cmd_app_replace(struct dcb *dcb, const char *dev, int argc, char **argv)
 {
-	struct dcb_app_table orig = {};
-	struct dcb_app_table tab = {};
-	struct dcb_app_table new = {};
+	struct dcb_app_table orig = { .attr = DCB_ATTR_IEEE_APP_TABLE };
+	struct dcb_app_table tab = { .attr = DCB_ATTR_IEEE_APP_TABLE };
+	struct dcb_app_table new = { .attr = DCB_ATTR_IEEE_APP_TABLE };
 	int ret;
 
 	ret = dcb_app_get(dcb, dev, &orig);
diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
index 8e7b010dcf75..3aea0bfd1786 100644
--- a/dcb/dcb_app.h
+++ b/dcb/dcb_app.h
@@ -7,6 +7,7 @@ struct dcb;
 struct dcb_app_table {
 	struct dcb_app *apps;
 	size_t n_apps;
+	int attr;
 };
 
 struct dcb_app_add_del {

-- 
2.34.1

[PATCH iproute2-next 3/9] dcb: app: modify dcb-app print functions for dcb-rewr reuse

[Daniel Machon]

Whereas dcb-app requires protocol to be the printed key, dcb-rewr
requires it to be the priority. Existing dcb-app print functions can
easily be adapted to support this, by using the newly introduced dcbnl
attribute in the dcb_app_table struct.

- dcb_app_print_key_*() functions have been renamed to
  dcb_app_print_pid_*() to align with new situation. Also, none of them
  will print the colon anymore.

- dcb_app_print_filtered() will now print either priority or protocol
  first, depending on the dcbnl set/get attribute.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/dcb_app.c | 62 ++++++++++++++++++++++++++++++++---------------------------
 dcb/dcb_app.h | 10 +++++++---
 2 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
index 1d0da35f987d..9bb64f32e12e 100644
--- a/dcb/dcb_app.c
+++ b/dcb/dcb_app.c
@@ -389,40 +389,38 @@ static bool dcb_app_is_port(const struct dcb_app *app)
 	return app->selector == IEEE_8021QAZ_APP_SEL_ANY;
 }
 
-int dcb_app_print_key_dec(__u16 protocol)
+int dcb_app_print_pid_dec(__u16 protocol)
 {
-	return print_uint(PRINT_ANY, NULL, "%d:", protocol);
+	return print_uint(PRINT_ANY, NULL, "%d", protocol);
 }
 
-static int dcb_app_print_key_hex(__u16 protocol)
+static int dcb_app_print_pid_hex(__u16 protocol)
 {
-	return print_uint(PRINT_ANY, NULL, "%x:", protocol);
+	return print_uint(PRINT_ANY, NULL, "%x", protocol);
 }
 
-int dcb_app_print_key_dscp(__u16 protocol)
+int dcb_app_print_pid_dscp(__u16 protocol)
 {
 	const char *name = rtnl_dsfield_get_name(protocol << 2);
 
-
 	if (!is_json_context() && name != NULL)
-		return print_string(PRINT_FP, NULL, "%s:", name);
-	return print_uint(PRINT_ANY, NULL, "%d:", protocol);
+		return print_string(PRINT_FP, NULL, "%s", name);
+	return print_uint(PRINT_ANY, NULL, "%d", protocol);
 }
 
-int dcb_app_print_key_pcp(__u16 protocol)
+int dcb_app_print_pid_pcp(__u16 protocol)
 {
 	/* Print in numerical form, if protocol value is out-of-range */
 	if (protocol > DCB_APP_PCP_MAX)
-		return print_uint(PRINT_ANY, NULL, "%d:", protocol);
+		return print_uint(PRINT_ANY, NULL, "%d", protocol);
 
-	return print_string(PRINT_ANY, NULL, "%s:", pcp_names[protocol]);
+	return print_string(PRINT_ANY, NULL, "%s", pcp_names[protocol]);
 }
 
-static void dcb_app_print_filtered(const struct dcb_app_table *tab,
-				   bool (*filter)(const struct dcb_app *),
-				   int (*print_key)(__u16 protocol),
-				   const char *json_name,
-				   const char *fp_name)
+void dcb_app_print_filtered(const struct dcb_app_table *tab,
+			    bool (*filter)(const struct dcb_app *),
+			    int (*print_pid)(__u16 protocol),
+			    const char *json_name, const char *fp_name)
 {
 	bool first = true;
 	size_t i;
@@ -439,8 +437,14 @@ static void dcb_app_print_filtered(const struct dcb_app_table *tab,
 		}
 
 		open_json_array(PRINT_JSON, NULL);
-		print_key(app->protocol);
-		print_uint(PRINT_ANY, NULL, "%d ", app->priority);
+		if (tab->attr == DCB_ATTR_IEEE_APP_TABLE) {
+			print_pid(app->protocol);
+			print_uint(PRINT_ANY, NULL, ":%d", app->priority);
+		} else {
+			print_uint(PRINT_ANY, NULL, "%d:", app->priority);
+			print_pid(app->protocol);
+		}
+		print_string(PRINT_ANY, NULL, "%s", " ");
 		close_json_array(PRINT_JSON, NULL);
 	}
 
@@ -452,7 +456,7 @@ static void dcb_app_print_filtered(const struct dcb_app_table *tab,
 
 static void dcb_app_print_ethtype_prio(const struct dcb_app_table *tab)
 {
-	dcb_app_print_filtered(tab, dcb_app_is_ethtype,  dcb_app_print_key_hex,
+	dcb_app_print_filtered(tab, dcb_app_is_ethtype, dcb_app_print_pid_hex,
 			       "ethtype_prio", "ethtype-prio");
 }
 
@@ -460,8 +464,8 @@ static void dcb_app_print_pcp_prio(const struct dcb *dcb,
 				   const struct dcb_app_table *tab)
 {
 	dcb_app_print_filtered(tab, dcb_app_is_pcp,
-			       dcb->numeric ? dcb_app_print_key_dec
-					    : dcb_app_print_key_pcp,
+			       dcb->numeric ? dcb_app_print_pid_dec :
+					      dcb_app_print_pid_pcp,
 			       "pcp_prio", "pcp-prio");
 }
 
@@ -469,26 +473,28 @@ static void dcb_app_print_dscp_prio(const struct dcb *dcb,
 				    const struct dcb_app_table *tab)
 {
 	dcb_app_print_filtered(tab, dcb_app_is_dscp,
-			       dcb->numeric ? dcb_app_print_key_dec
-					    : dcb_app_print_key_dscp,
+			       dcb->numeric ? dcb_app_print_pid_dec :
+					      dcb_app_print_pid_dscp,
 			       "dscp_prio", "dscp-prio");
 }
 
 static void dcb_app_print_stream_port_prio(const struct dcb_app_table *tab)
 {
-	dcb_app_print_filtered(tab, dcb_app_is_stream_port, dcb_app_print_key_dec,
-			       "stream_port_prio", "stream-port-prio");
+	dcb_app_print_filtered(tab, dcb_app_is_stream_port,
+			       dcb_app_print_pid_dec, "stream_port_prio",
+			       "stream-port-prio");
 }
 
 static void dcb_app_print_dgram_port_prio(const struct dcb_app_table *tab)
 {
-	dcb_app_print_filtered(tab, dcb_app_is_dgram_port, dcb_app_print_key_dec,
-			       "dgram_port_prio", "dgram-port-prio");
+	dcb_app_print_filtered(tab, dcb_app_is_dgram_port,
+			       dcb_app_print_pid_dec, "dgram_port_prio",
+			       "dgram-port-prio");
 }
 
 static void dcb_app_print_port_prio(const struct dcb_app_table *tab)
 {
-	dcb_app_print_filtered(tab, dcb_app_is_port, dcb_app_print_key_dec,
+	dcb_app_print_filtered(tab, dcb_app_is_port, dcb_app_print_pid_dec,
 			       "port_prio", "port-prio");
 }
 
diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
index 3aea0bfd1786..8f048605c3a8 100644
--- a/dcb/dcb_app.h
+++ b/dcb/dcb_app.h
@@ -41,9 +41,13 @@ void dcb_app_table_remove_existing(struct dcb_app_table *a,
 bool dcb_app_is_pcp(const struct dcb_app *app);
 bool dcb_app_is_dscp(const struct dcb_app *app);
 
-int dcb_app_print_key_dec(__u16 protocol);
-int dcb_app_print_key_dscp(__u16 protocol);
-int dcb_app_print_key_pcp(__u16 protocol);
+int dcb_app_print_pid_dec(__u16 protocol);
+int dcb_app_print_pid_dscp(__u16 protocol);
+int dcb_app_print_pid_pcp(__u16 protocol);
+void dcb_app_print_filtered(const struct dcb_app_table *tab,
+			    bool (*filter)(const struct dcb_app *),
+			    int (*print_pid)(__u16 protocol),
+			    const char *json_name, const char *fp_name);
 
 int dcb_app_parse_pcp(__u32 *key, const char *arg);
 int dcb_app_parse_dscp(__u32 *key, const char *arg);

-- 
2.34.1

[PATCH iproute2-next 4/9] dcb: app: modify dcb_app_table_remove_replaced() for dcb-rewr reuse

[Daniel Machon]

When doing a replace command, entries are checked against selector and
protocol. Rewrite requires the check to be against selector and
priority.

Modify the existing dcb_app_table_remove_replace function for dcb-rewr
reuse, by using the newly introduced dcbnl attribute in the
dcb_app_table struct.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/dcb_app.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
index 9bb64f32e12e..23d6bb2a0013 100644
--- a/dcb/dcb_app.c
+++ b/dcb/dcb_app.c
@@ -160,15 +160,27 @@ void dcb_app_table_remove_replaced(struct dcb_app_table *a,
 		for (ib = 0; ib < b->n_apps; ib++) {
 			const struct dcb_app *ab = &b->apps[ib];
 
-			if (aa->selector == ab->selector &&
-			    aa->protocol == ab->protocol)
-				present = true;
-			else
+			if (aa->selector != ab->selector)
 				continue;
 
-			if (aa->priority == ab->priority) {
-				found = true;
-				break;
+			if (a->attr == DCB_ATTR_IEEE_APP_TABLE) {
+				if (aa->protocol == ab->protocol)
+					present = true;
+				else
+					continue;
+				if (aa->priority == ab->priority) {
+					found = true;
+					break;
+				}
+			} else {
+				if (aa->priority == ab->priority)
+					present = true;
+				else
+					continue;
+				if (aa->protocol == ab->protocol) {
+					found = true;
+					break;
+				}
 			}
 		}
 

-- 
2.34.1

[PATCH iproute2-next 5/9] dcb: app: modify dcb_app_parse_mapping_cb for dcb-rewr reuse

[Daniel Machon]

When parsing APP table entries, priority and protocol is assigned from
value and key, respectively. Rewrite requires it opposite.

Modify the existing dcb_app_parse_mapping_cb for dcb-rewr reuse, by
using the newly introduced dcbnl attribute in the dcb_app_table struct.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/dcb_app.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
index 23d6bb2a0013..46af67112748 100644
--- a/dcb/dcb_app.c
+++ b/dcb/dcb_app.c
@@ -232,10 +232,17 @@ void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
 	struct dcb_app_parse_mapping *pm = data;
 	struct dcb_app app = {
 		.selector = pm->selector,
-		.priority = value,
-		.protocol = key,
 	};
 
+	if (pm->tab->attr == DCB_ATTR_IEEE_APP_TABLE) {
+		app.priority = value;
+		app.protocol = key;
+
+	} else {
+		app.priority = key;
+		app.protocol = value;
+	}
+
 	if (pm->err)
 		return;
 

-- 
2.34.1

[PATCH iproute2-next 6/9] dcb: rewr: add new dcb-rewr subcommand

[Daniel Machon]

Add a new subcommand 'rewr' for configuring the in-kernel DCB rewrite
table. The rewr-table of the kernel is similar to the APP-table, and so
is this new subcommand. Therefore, much of the existing bookkeeping code
from dcb-app, can be reused in the dcb-rewr implementation.

Initially, only support for configuring PCP and DSCP-based rewrite has
been added.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/Makefile   |   3 +-
 dcb/dcb.c      |   4 +-
 dcb/dcb.h      |   3 +
 dcb/dcb_app.h  |   1 +
 dcb/dcb_rewr.c | 332 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 341 insertions(+), 2 deletions(-)

diff --git a/dcb/Makefile b/dcb/Makefile
index dd41a559a0c8..10794c9dc19f 100644
--- a/dcb/Makefile
+++ b/dcb/Makefile
@@ -8,7 +8,8 @@ DCBOBJ = dcb.o \
          dcb_ets.o \
          dcb_maxrate.o \
          dcb_pfc.o \
-         dcb_apptrust.o
+         dcb_apptrust.o \
+         dcb_rewr.o
 TARGETS += dcb
 LDLIBS += -lm
 
diff --git a/dcb/dcb.c b/dcb/dcb.c
index 9b996abac529..fe0a0f04143d 100644
--- a/dcb/dcb.c
+++ b/dcb/dcb.c
@@ -470,7 +470,7 @@ static void dcb_help(void)
 	fprintf(stderr,
 		"Usage: dcb [ OPTIONS ] OBJECT { COMMAND | help }\n"
 		"       dcb [ -f | --force ] { -b | --batch } filename [ -n | --netns ] netnsname\n"
-		"where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc }\n"
+		"where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc | rewr }\n"
 		"       OPTIONS := [ -V | --Version | -i | --iec | -j | --json\n"
 		"                  | -N | --Numeric | -p | --pretty\n"
 		"                  | -s | --statistics | -v | --verbose]\n");
@@ -485,6 +485,8 @@ static int dcb_cmd(struct dcb *dcb, int argc, char **argv)
 		return dcb_cmd_app(dcb, argc - 1, argv + 1);
 	} else if (strcmp(*argv, "apptrust") == 0) {
 		return dcb_cmd_apptrust(dcb, argc - 1, argv + 1);
+	} else if (strcmp(*argv, "rewr") == 0) {
+		return dcb_cmd_rewr(dcb, argc - 1, argv + 1);
 	} else if (matches(*argv, "buffer") == 0) {
 		return dcb_cmd_buffer(dcb, argc - 1, argv + 1);
 	} else if (matches(*argv, "dcbx") == 0) {
diff --git a/dcb/dcb.h b/dcb/dcb.h
index 4c8a4aa25e0c..39a04f1c59df 100644
--- a/dcb/dcb.h
+++ b/dcb/dcb.h
@@ -56,6 +56,9 @@ void dcb_print_array_on_off(const __u8 *array, size_t size);
 void dcb_print_array_kw(const __u8 *array, size_t array_size,
 			const char *const kw[], size_t kw_size);
 
+/* dcb_rewr.c */
+int dcb_cmd_rewr(struct dcb *dcb, int argc, char **argv);
+
 /* dcb_apptrust.c */
 
 int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv);
diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
index 8f048605c3a8..02c9eb03f6c2 100644
--- a/dcb/dcb_app.h
+++ b/dcb/dcb_app.h
@@ -22,6 +22,7 @@ struct dcb_app_parse_mapping {
 };
 
 #define DCB_APP_PCP_MAX 15
+#define DCB_APP_DSCP_MAX 63
 
 int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
 
diff --git a/dcb/dcb_rewr.c b/dcb/dcb_rewr.c
new file mode 100644
index 000000000000..731ba78977e2
--- /dev/null
+++ b/dcb/dcb_rewr.c
@@ -0,0 +1,332 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <errno.h>
+#include <linux/dcbnl.h>
+#include <stdio.h>
+
+#include "dcb.h"
+#include "utils.h"
+
+static void dcb_rewr_help_add(void)
+{
+	fprintf(stderr,
+		"Usage: dcb rewr { add | del | replace } dev STRING\n"
+		"           [ prio-pcp PRIO:PCP   ]\n"
+		"           [ prio-dscp PRIO:DSCP ]\n"
+		"\n"
+		" where PRIO := { 0 .. 7               }\n"
+		"       PCP  := { 0(nd/de) .. 7(nd/de) }\n"
+		"       DSCP := { 0 .. 63              }\n"
+		"\n"
+	);
+}
+
+static void dcb_rewr_help_show_flush(void)
+{
+	fprintf(stderr,
+		"Usage: dcb rewr { show | flush } dev STRING\n"
+		"           [ prio-pcp  ]\n"
+		"           [ prio-dscp ]\n"
+		"\n"
+	);
+}
+
+static void dcb_rewr_help(void)
+{
+	fprintf(stderr,
+		"Usage: dcb rewr help\n"
+		"\n"
+	);
+	dcb_rewr_help_show_flush();
+	dcb_rewr_help_add();
+}
+
+static int dcb_rewr_parse_mapping_prio_pcp(__u32 key, char *value, void *data)
+{
+	__u32 pcp;
+
+	if (dcb_app_parse_pcp(&pcp, value))
+		return -EINVAL;
+
+	return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1, "PCP",
+				 pcp, DCB_APP_PCP_MAX, dcb_app_parse_mapping_cb,
+				 data);
+}
+
+static int dcb_rewr_parse_mapping_prio_dscp(__u32 key, char *value, void *data)
+{
+	__u32 dscp;
+
+	if (dcb_app_parse_dscp(&dscp, value))
+		return -EINVAL;
+
+	return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1, "DSCP",
+				 dscp, DCB_APP_DSCP_MAX,
+				 dcb_app_parse_mapping_cb, data);
+}
+
+static void dcb_rewr_print_prio_pcp(const struct dcb *dcb,
+				    const struct dcb_app_table *tab)
+{
+	dcb_app_print_filtered(tab, dcb_app_is_pcp,
+			       dcb->numeric ? dcb_app_print_pid_dec :
+					      dcb_app_print_pid_pcp,
+			       "prio_pcp", "prio-pcp");
+}
+
+static void dcb_rewr_print_prio_dscp(const struct dcb *dcb,
+				     const struct dcb_app_table *tab)
+{
+	dcb_app_print_filtered(tab, dcb_app_is_dscp,
+			       dcb->numeric ? dcb_app_print_pid_dec :
+					      dcb_app_print_pid_dscp,
+			       "prio_dscp", "prio-dscp");
+}
+
+static void dcb_rewr_print(const struct dcb *dcb,
+			   const struct dcb_app_table *tab)
+{
+	dcb_rewr_print_prio_pcp(dcb, tab);
+	dcb_rewr_print_prio_dscp(dcb, tab);
+}
+
+static int dcb_cmd_rewr_parse_add_del(struct dcb *dcb, const char *dev,
+				      int argc, char **argv,
+				      struct dcb_app_table *tab)
+{
+	struct dcb_app_parse_mapping pm = { .tab = tab };
+	int ret;
+
+	if (!argc) {
+		dcb_rewr_help_add();
+		return 0;
+	}
+
+	do {
+		if (strcmp(*argv, "help") == 0) {
+			dcb_rewr_help_add();
+			return 0;
+		} else if (strcmp(*argv, "prio-pcp") == 0) {
+			NEXT_ARG();
+			pm.selector = DCB_APP_SEL_PCP;
+			ret = parse_mapping(&argc, &argv, false,
+					    &dcb_rewr_parse_mapping_prio_pcp,
+					    &pm);
+		} else if (strcmp(*argv, "prio-dscp") == 0) {
+			NEXT_ARG();
+			pm.selector = IEEE_8021QAZ_APP_SEL_DSCP;
+			ret = parse_mapping(&argc, &argv, false,
+					    &dcb_rewr_parse_mapping_prio_dscp,
+					    &pm);
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			dcb_rewr_help_add();
+			return -EINVAL;
+		}
+
+		if (ret != 0) {
+			fprintf(stderr, "Invalid mapping %s\n", *argv);
+			return ret;
+		}
+		if (pm.err)
+			return pm.err;
+	} while (argc > 0);
+
+	return 0;
+}
+
+static int dcb_cmd_rewr_add(struct dcb *dcb, const char *dev, int argc,
+			    char **argv)
+{
+	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
+	int ret;
+
+	ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
+	if (ret != 0)
+		return ret;
+
+	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_SET, &tab, NULL);
+	dcb_app_table_fini(&tab);
+	return ret;
+}
+
+static int dcb_cmd_rewr_del(struct dcb *dcb, const char *dev, int argc,
+			    char **argv)
+{
+	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
+	int ret;
+
+	ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
+	if (ret != 0)
+		return ret;
+
+	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab, NULL);
+	dcb_app_table_fini(&tab);
+	return ret;
+}
+
+static int dcb_cmd_rewr_replace(struct dcb *dcb, const char *dev, int argc,
+				char **argv)
+{
+	struct dcb_app_table orig = { .attr = DCB_ATTR_DCB_REWR_TABLE };
+	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
+	struct dcb_app_table new = { .attr = DCB_ATTR_DCB_REWR_TABLE };
+	int ret;
+
+	ret = dcb_app_get(dcb, dev, &orig);
+	if (ret != 0)
+		return ret;
+
+	ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
+	if (ret != 0)
+		goto out;
+
+	/* Attempts to add an existing entry would be rejected, so drop
+	 * these entries from tab.
+	 */
+	ret = dcb_app_table_copy(&new, &tab);
+	if (ret != 0)
+		goto out;
+	dcb_app_table_remove_existing(&new, &orig);
+
+	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_SET, &new, NULL);
+	if (ret != 0) {
+		fprintf(stderr, "Could not add new rewrite entries\n");
+		goto out;
+	}
+
+	/* Remove the obsolete entries. */
+	dcb_app_table_remove_replaced(&orig, &tab);
+	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &orig, NULL);
+	if (ret != 0) {
+		fprintf(stderr, "Could not remove replaced rewrite entries\n");
+		goto out;
+	}
+
+out:
+	dcb_app_table_fini(&new);
+	dcb_app_table_fini(&tab);
+	dcb_app_table_fini(&orig);
+	return 0;
+}
+
+
+static int dcb_cmd_rewr_show(struct dcb *dcb, const char *dev, int argc,
+			     char **argv)
+{
+	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
+	int ret;
+
+	ret = dcb_app_get(dcb, dev, &tab);
+	if (ret != 0)
+		return ret;
+
+	dcb_app_table_sort(&tab);
+
+	open_json_object(NULL);
+
+	if (!argc) {
+		dcb_rewr_print(dcb, &tab);
+		goto out;
+	}
+
+	do {
+		if (strcmp(*argv, "help") == 0) {
+			dcb_rewr_help_show_flush();
+			goto out;
+		} else if (strcmp(*argv, "prio-pcp") == 0) {
+			dcb_rewr_print_prio_pcp(dcb, &tab);
+		} else if (strcmp(*argv, "prio-dscp") == 0) {
+			dcb_rewr_print_prio_dscp(dcb, &tab);
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			dcb_rewr_help_show_flush();
+			ret = -EINVAL;
+			goto out;
+		}
+
+		NEXT_ARG_FWD();
+	} while (argc > 0);
+
+out:
+	close_json_object();
+	dcb_app_table_fini(&tab);
+	return ret;
+}
+
+static int dcb_cmd_rewr_flush(struct dcb *dcb, const char *dev, int argc,
+			      char **argv)
+{
+	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
+	int ret;
+
+	ret = dcb_app_get(dcb, dev, &tab);
+	if (ret != 0)
+		return ret;
+
+	if (!argc) {
+		ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
+				      NULL);
+		goto out;
+	}
+
+	do {
+		if (strcmp(*argv, "help") == 0) {
+			dcb_rewr_help_show_flush();
+			goto out;
+		} else if (strcmp(*argv, "prio-pcp") == 0) {
+			ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
+					      &dcb_app_is_pcp);
+			if (ret != 0)
+				goto out;
+		} else if (strcmp(*argv, "prio-dscp") == 0) {
+			ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
+					      &dcb_app_is_dscp);
+			if (ret != 0)
+				goto out;
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			dcb_rewr_help_show_flush();
+			ret = -EINVAL;
+			goto out;
+		}
+
+		NEXT_ARG_FWD();
+	} while (argc > 0);
+
+out:
+	dcb_app_table_fini(&tab);
+	return ret;
+}
+
+int dcb_cmd_rewr(struct dcb *dcb, int argc, char **argv)
+{
+	if (!argc || strcmp(*argv, "help") == 0) {
+		dcb_rewr_help();
+		return 0;
+	} else if (strcmp(*argv, "show") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_show,
+					 dcb_rewr_help_show_flush);
+	} else if (strcmp(*argv, "flush") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_flush,
+					 dcb_rewr_help_show_flush);
+	} else if (strcmp(*argv, "add") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_add,
+					 dcb_rewr_help_add);
+	} else if (strcmp(*argv, "del") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_del,
+					 dcb_rewr_help_add);
+	} else if (strcmp(*argv, "replace") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_replace,
+					 dcb_rewr_help_add);
+	} else {
+		fprintf(stderr, "What is \"%s\"?\n", *argv);
+		dcb_rewr_help();
+		return -EINVAL;
+	}
+}

-- 
2.34.1

[PATCH iproute2-next 7/9] man: dcb-rewr: add new manpage for dcb-rewr

[Daniel Machon]

Add a new manpage for dcb-rewr. Most of the content is copied over from
dcb-app, as the same set of commands and parameters (in reverse) applies
to dcb-rewr.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 man/man8/dcb-rewr.8 | 206 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 206 insertions(+)

diff --git a/man/man8/dcb-rewr.8 b/man/man8/dcb-rewr.8
new file mode 100644
index 000000000000..b859dfea37c1
--- /dev/null
+++ b/man/man8/dcb-rewr.8
@@ -0,0 +1,206 @@
+.TH DCB-REWR 8 "15 may 2023" "iproute2" "Linux"
+.SH NAME
+dcb-rewr \- show / manipulate the rewrite table of
+the DCB (Data Center Bridging) subsystem
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+
+.ti -8
+.B dcb
+.RI "[ " OPTIONS " ] "
+.B rewr
+.RI "{ " COMMAND " | " help " }"
+.sp
+
+.ti -8
+.B dcb rewr " { " show " | " flush " } " dev
+.RI DEV
+.RB "[ " prio-dscp " ]"
+.RB "[ " prio-pcp " ]"
+
+.ti -8
+.B dcb rewr " { " add " | " del " | " replace " } " dev
+.RI DEV
+.RB "[ " prio-dscp " " \fIDSCP-MAP\fB " ]"
+.RB "[ " prio-pcp " " \fIPCP-MAP\fB " ]"
+
+.ti -8
+.IR DSCP-MAP " := [ " DSCP-MAP " ] " DSCP-MAPPING
+
+.ti -8
+.IR DSCP-MAPPING " := " \fIPRIO \fB:\fR "{ " DSCP " | " \fBall\fR " }"
+
+.ti -8
+.IR PCP-MAP " := [ " PCP-MAP " ] " PCP-MAPPING
+
+.ti -8
+.IR PCP-MAPPING " := " \fIPRIO \fB:\fR PCP\fR
+
+.ti -8
+.IR DSCP " := { " \fB0\fR " .. " \fB63\fR " }"
+
+.ti -8
+.IR PCP " := { " \fB0(nd/de)\fR " .. " \fB7(nd/de)\fR " }"
+
+.ti -8
+.IR PRIO " := { " \fB0\fR " .. " \fB7\fR " }"
+
+.SH DESCRIPTION
+
+.B dcb rewr
+is used to configure the rewrite table, in the DCB (Data Center Bridging)
+subsystem.  The rewrite table is used to rewrite certain values in the packet
+headers, based on packet priority.
+
+DCB rewrite entries are, like DCB APP entries, 3-tuples of selector, protocol
+ID, and priority. Selector is an enumeration that picks one of the
+prioritization namespaces. Currently, only the DSCP and PCP selector namespaces
+are supported by dcb rewr.
+
+The rewrite table is a list of DCB rewrite rules, that applies to packets
+with matching priority.  Notably, it is valid to have conflicting rewrite
+assignment for the same selector and priority. For example, the set of two
+rewrite entries (DSCP, 10, 1) and (DSCP, 11, 1), where packets with priority 1
+should have its DSCP value rewritten to both 10 and 11, form a well-defined
+rewrite table.
+.B dcb rewr
+tool allows low-level management of the rewrite table by adding and deleting
+individual rewrite 3-tuples through
+.B add
+and
+.B del
+commands. On the other hand, the command
+.B replace
+does what one would typically want in this situation--first adds the new
+configuration, and then removes the obsolete one, so that only one
+rewrite rule is in effect for a given selector and priority.
+
+.SH COMMANDS
+
+.TP
+.B show
+Display all entries with a given selector. When no selector is given, shows all
+rewrite table entries categorized per selector.
+
+.TP
+.B flush
+Remove all entries with a given selector. When no selector is given, removes all
+rewrite table entries.
+
+.TP
+.B add
+.TQ
+.B del
+Add and, respectively, remove individual rewrite 3-tuples to and from the DCB
+rewrite table.
+
+.TP
+.B replace
+Take the list of entries mentioned as parameter, and add those that are not
+present in the rewrite table yet. Then remove those entries, whose selector and
+priority have been mentioned as parameter, but not with the exact same
+protocol ID. This has the effect of, for the given selector and priority,
+causing that the table only contains the protocol ID (or ID's) given as
+parameter.
+
+.SH PARAMETERS
+
+The following table shows parameters in a way that they would be used with
+\fBadd\fR, \fBdel\fR and \fBreplace\fR commands. For \fBshow\fR and
+\fBflush\fR, the parameter name is to be used as a simple keyword without
+further arguments.
+
+.TP
+.B prio-dscp \fIDSCP-MAP
+\fIDSCP-MAP\fR uses the array parameter syntax, see
+.BR dcb (8)
+for details. Keys are priorities, values are DSCP points for traffic
+with matching priority. DSCP points can be written either directly as numeric
+values, or using symbolic names specified in
+.B /etc/iproute2/rt_dsfield
+(however note that file specifies full 8-bit dsfield values, whereas
+.B dcb rewr
+will only use the higher six bits).
+.B dcb rewr show
+will similarly format DSCP values as symbolic names if possible. The
+command line option
+.B -N
+turns the show translation off.
+
+.TP
+.B prio-pcp \fIPCP-MAP
+\fIPCP-MAP\fR uses the array parameter syntax, see
+.BR dcb (8)
+for details. Keys are priorities. Values are PCP/DEI for traffic with
+matching priority. PCP/DEI values are written as a combination of numeric- and
+symbolic values, to accommodate for both. PCP always in numeric form e.g 0 ..
+7 and DEI in symbolic form e.g 'de' (drop-eligible), indicating that the DEI
+bit is 1 or 'nd' (not-drop-eligible), indicating that the DEI bit is 0.  In
+combination 1:2de translates to a mapping of priority 1 to PCP=2 and DEI=1.
+
+.SH EXAMPLE & USAGE
+
+Add a rule to rewrite DSCP to 0, 24 and 48 for traffic with priority 0, 3 and
+6, respectively:
+.P
+# dcb rewr add dev eth0 prio-dscp 0:0 3:24 6:48
+
+Add a rule to rewrite DSCP to 25 for traffic with priority 3:
+.P
+# dcb rewr add dev eth0 prio-dscp 3:25
+.br
+# dcb rewr show dev eth0 prio-dscp
+.br
+prio-dscp 0:0 3:CS3 3:25 6:CS6
+.br
+# dcb -N rewr show dev eth0 prio-dscp
+.br
+prio-dscp 0:0 3:24 3:25 6:48
+
+Reconfigure the table so that only one rule exists for rewriting traffic with
+priority 3.
+
+.P
+# dcb rewr replace dev eth0 prio-dscp 3:26
+.br
+# dcb rewr -N show dev eth0 prio-dscp
+.br
+prio-dscp 0:0 3:26 6:48
+
+Flush all DSCP rules:
+
+.P
+# dcb rewr flush dev eth0 prio-dscp
+.br
+# dcb rewr show dev eth0 prio-dscp
+.br
+(nothing)
+
+Add a rule to rewrite PCP to 1 and DEI to 0 for traffic with priority 1 and a
+rule to rewrite PCP to 2 and DEI to 1 for traffic with priority 2:
+
+.P
+# dcb rewr add dev eth0 prio-pcp 1:1nd 2:2de
+.br
+# dcb rewr show dev eth0 prio-pcp
+.br
+prio-pcp 1:1nd 2:2de
+
+.SH EXIT STATUS
+Exit status is 0 if command was successful or a positive integer upon failure.
+
+.SH SEE ALSO
+.BR dcb (8)
+.BR dcb-app (8)
+.BR dcb-apptrust (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
+Daniel Machon <daniel.machon@microchip.com>

-- 
2.34.1

[PATCH iproute2-next 8/9] man: dcb: add additional references under 'SEE ALSO'

[Daniel Machon]

Add dcb-apptrust and dcb-rewr to the 'SEE ALSO' section of the dcb
manpage.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 man/man8/dcb.8 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/man/man8/dcb.8 b/man/man8/dcb.8
index 24944b73816b..a1d6505e93b4 100644
--- a/man/man8/dcb.8
+++ b/man/man8/dcb.8
@@ -140,10 +140,12 @@ Exit status is 0 if command was successful or a positive integer upon failure.
 
 .SH SEE ALSO
 .BR dcb-app (8),
+.BR dcb-apptrust (8),
 .BR dcb-buffer (8),
 .BR dcb-ets (8),
 .BR dcb-maxrate (8),
-.BR dcb-pfc (8)
+.BR dcb-pfc (8),
+.BR dcb-rewr (8)
 .br
 
 .SH REPORTING BUGS

-- 
2.34.1

[PATCH iproute2-next 9/9] man: dcb-app: clean up a few mistakes

[Daniel Machon]

While referencing the dcb-app manpage, I spotted a few mistakes. Lets
fix them.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 man/man8/dcb-app.8 | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/man/man8/dcb-app.8 b/man/man8/dcb-app.8
index ecb38591168e..ebec67c90801 100644
--- a/man/man8/dcb-app.8
+++ b/man/man8/dcb-app.8
@@ -1,4 +1,4 @@
-.TH DCB-ETS 8 "6 December 2020" "iproute2" "Linux"
+.TH DCB-APP 8 "6 December 2020" "iproute2" "Linux"
 .SH NAME
 dcb-app \- show / manipulate application priority table of
 the DCB (Data Center Bridging) subsystem
@@ -26,7 +26,7 @@ the DCB (Data Center Bridging) subsystem
 .RB "[ " pcp-prio " ]"
 
 .ti -8
-.B dcb ets " { " add " | " del " | " replace " } " dev
+.B dcb app " { " add " | " del " | " replace " } " dev
 .RI DEV
 .RB "[ " default-prio " " \fIPRIO-LIST\fB " ]"
 .RB "[ " ethtype-prio " " \fIET-MAP\fB " ]"
@@ -106,7 +106,7 @@ individual APP 3-tuples through
 .B add
 and
 .B del
-commands. On the other other hand, the command
+commands. On the other hand, the command
 .B replace
 does what one would typically want in this situation--first adds the new
 configuration, and then removes the obsolete one, so that only one
@@ -184,7 +184,7 @@ for details. Keys are DSCP points, values are priorities assigned to
 traffic with matching DSCP. DSCP points can be written either directly as
 numeric values, or using symbolic names specified in
 .B /etc/iproute2/rt_dsfield
-(however note that that file specifies full 8-bit dsfield values, whereas
+(however note that file specifies full 8-bit dsfield values, whereas
 .B dcb app
 will only use the higher six bits).
 .B dcb app show
@@ -230,7 +230,7 @@ priority 4:
 .P
 # dcb app replace dev eth0 dscp-prio 24:4
 .br
-# dcb app show dev eth0 dscp-prio
+# dcb app -N show dev eth0 dscp-prio
 .br
 dscp-prio 0:0 24:4 48:6
 

-- 
2.34.1

Re: [PATCH iproute2-next 3/9] dcb: app: modify dcb-app print functions for dcb-rewr reuse

[Stephen Hemminger]

On Mon, 22 May 2023 20:41:06 +0200
Daniel Machon <daniel.machon@microchip.com> wrote:

> +int dcb_app_print_pid_dec(__u16 protocol)
>  {
> -	return print_uint(PRINT_ANY, NULL, "%d:", protocol);
> +	return print_uint(PRINT_ANY, NULL, "%d", protocol);
>  }

Should be %u for unsigned value.

Re: [PATCH iproute2-next 1/9] dcb: app: expose dcb-app functions in new header

[Petr Machata]

Daniel Machon <daniel.machon@microchip.com> writes:

> Add new headerfile dcb-app.h that exposes the functions required later
> by dcb-rewr. The new dcb-rewr implementation will reuse much of the
> existing dcb-app code.
>
> I thought this called for a separate header file, instead of polluting
> the existing dcb.h file.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/dcb.h     |  9 ++-------
>  dcb/dcb_app.c | 54 ++++++++++++++++++------------------------------------
>  dcb/dcb_app.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+), 43 deletions(-)
>
> diff --git a/dcb/dcb.h b/dcb/dcb.h
> index d40664f29dad..4c8a4aa25e0c 100644
> --- a/dcb/dcb.h
> +++ b/dcb/dcb.h
> @@ -6,6 +6,8 @@
>  #include <stdbool.h>
>  #include <stddef.h>
>  
> +#include "dcb_app.h"
> +
>  /* dcb.c */
>  
>  struct dcb {
> @@ -54,13 +56,6 @@ void dcb_print_array_on_off(const __u8 *array, size_t size);
>  void dcb_print_array_kw(const __u8 *array, size_t array_size,
>  			const char *const kw[], size_t kw_size);
>  
> -/* dcb_app.c */
> -
> -int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
> -enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector);
> -bool dcb_app_attr_type_validate(enum ieee_attrs_app type);
> -bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
> -

Why the move to a dedicated header? dcb.h ends up being the only client
and everybody consumes the prototypes through that file anyway. I don't
fine it necessary.

>  /* dcb_apptrust.c */
>  
>  int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv);
> diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
> index eeb78e70f63f..df339babd8e6 100644
> --- a/dcb/dcb_app.c
> +++ b/dcb/dcb_app.c
> @@ -10,8 +10,6 @@
>  #include "utils.h"
>  #include "rt_names.h"
>  
> -#define DCB_APP_PCP_MAX 15
> -
>  static const char *const pcp_names[DCB_APP_PCP_MAX + 1] = {
>  	"0nd", "1nd", "2nd", "3nd", "4nd", "5nd", "6nd", "7nd",
>  	"0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de"
> @@ -22,6 +20,7 @@ static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = {
>  	[DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP"
>  };
>  
> +

This looks like a leftover.

>  static void dcb_app_help_add(void)
>  {
>  	fprintf(stderr,
> @@ -68,11 +67,6 @@ static void dcb_app_help(void)
>  	dcb_app_help_add();
>  }
>  
> -struct dcb_app_table {
> -	struct dcb_app *apps;
> -	size_t n_apps;
> -};
> -
>  enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector)
>  {
>  	switch (selector) {
> @@ -105,7 +99,7 @@ bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector)
>  	return dcb_app_attr_type_get(selector) == type;
>  }
>  
> -static void dcb_app_table_fini(struct dcb_app_table *tab)
> +void dcb_app_table_fini(struct dcb_app_table *tab)
>  {
>  	free(tab->apps);
>  }
> @@ -124,8 +118,8 @@ static int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app)
>  	return 0;
>  }
>  
> -static void dcb_app_table_remove_existing(struct dcb_app_table *a,
> -					  const struct dcb_app_table *b)
> +void dcb_app_table_remove_existing(struct dcb_app_table *a,
> +				   const struct dcb_app_table *b)
>  {
>  	size_t ia, ja;
>  	size_t ib;
> @@ -152,8 +146,8 @@ static void dcb_app_table_remove_existing(struct dcb_app_table *a,
>  	a->n_apps = ja;
>  }
>  
> -static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
> -					  const struct dcb_app_table *b)
> +void dcb_app_table_remove_replaced(struct dcb_app_table *a,
> +				   const struct dcb_app_table *b)
>  {
>  	size_t ia, ja;
>  	size_t ib;
> @@ -189,8 +183,7 @@ static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
>  	a->n_apps = ja;
>  }
>  
> -static int dcb_app_table_copy(struct dcb_app_table *a,
> -			      const struct dcb_app_table *b)
> +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b)
>  {
>  	size_t i;
>  	int ret;
> @@ -217,18 +210,12 @@ static int dcb_app_cmp_cb(const void *a, const void *b)
>  	return dcb_app_cmp(a, b);
>  }
>  
> -static void dcb_app_table_sort(struct dcb_app_table *tab)
> +void dcb_app_table_sort(struct dcb_app_table *tab)
>  {
>  	qsort(tab->apps, tab->n_apps, sizeof(*tab->apps), dcb_app_cmp_cb);
>  }
>  
> -struct dcb_app_parse_mapping {
> -	__u8 selector;
> -	struct dcb_app_table *tab;
> -	int err;
> -};
> -
> -static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
> +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
>  {
>  	struct dcb_app_parse_mapping *pm = data;
>  	struct dcb_app app = {
> @@ -260,7 +247,7 @@ static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data
>  				 dcb_app_parse_mapping_cb, data);
>  }
>  
> -static int dcb_app_parse_pcp(__u32 *key, const char *arg)
> +int dcb_app_parse_pcp(__u32 *key, const char *arg)
>  {
>  	int i;
>  
> @@ -286,7 +273,7 @@ static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data)
>  				 dcb_app_parse_mapping_cb, data);
>  }
>  
> -static int dcb_app_parse_dscp(__u32 *key, const char *arg)
> +int dcb_app_parse_dscp(__u32 *key, const char *arg)
>  {
>  	if (parse_mapping_num_all(key, arg) == 0)
>  		return 0;
> @@ -377,12 +364,12 @@ static bool dcb_app_is_default(const struct dcb_app *app)
>  	       app->protocol == 0;
>  }
>  
> -static bool dcb_app_is_dscp(const struct dcb_app *app)
> +bool dcb_app_is_dscp(const struct dcb_app *app)
>  {
>  	return app->selector == IEEE_8021QAZ_APP_SEL_DSCP;
>  }
>  
> -static bool dcb_app_is_pcp(const struct dcb_app *app)
> +bool dcb_app_is_pcp(const struct dcb_app *app)
>  {
>  	return app->selector == DCB_APP_SEL_PCP;
>  }
> @@ -402,7 +389,7 @@ static bool dcb_app_is_port(const struct dcb_app *app)
>  	return app->selector == IEEE_8021QAZ_APP_SEL_ANY;
>  }
>  
> -static int dcb_app_print_key_dec(__u16 protocol)
> +int dcb_app_print_key_dec(__u16 protocol)
>  {
>  	return print_uint(PRINT_ANY, NULL, "%d:", protocol);
>  }
> @@ -412,7 +399,7 @@ static int dcb_app_print_key_hex(__u16 protocol)
>  	return print_uint(PRINT_ANY, NULL, "%x:", protocol);
>  }
>  
> -static int dcb_app_print_key_dscp(__u16 protocol)
> +int dcb_app_print_key_dscp(__u16 protocol)
>  {
>  	const char *name = rtnl_dsfield_get_name(protocol << 2);
>  
> @@ -422,7 +409,7 @@ static int dcb_app_print_key_dscp(__u16 protocol)
>  	return print_uint(PRINT_ANY, NULL, "%d:", protocol);
>  }
>  
> -static int dcb_app_print_key_pcp(__u16 protocol)
> +int dcb_app_print_key_pcp(__u16 protocol)
>  {
>  	/* Print in numerical form, if protocol value is out-of-range */
>  	if (protocol > DCB_APP_PCP_MAX)
> @@ -577,7 +564,7 @@ static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data)
>  	return MNL_CB_OK;
>  }
>  
> -static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab)
> +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab)
>  {
>  	uint16_t payload_len;
>  	void *payload;
> @@ -594,11 +581,6 @@ static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *t
>  	return 0;
>  }
>  
> -struct dcb_app_add_del {
> -	const struct dcb_app_table *tab;
> -	bool (*filter)(const struct dcb_app *app);
> -};
> -

This structure is a protocol between dcb_app_add_del() and
dcb_app_add_del_cb(). I don't think your patchset uses it elsewhere, so
it can be kept private.

>  static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
>  {
>  	struct dcb_app_add_del *add_del = data;
> @@ -620,7 +602,7 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
>  	return 0;
>  }
>  
> -static int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
> +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
>  			   const struct dcb_app_table *tab,
>  			   bool (*filter)(const struct dcb_app *))

This has wrong indentation.

>  {
> diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
> new file mode 100644
> index 000000000000..8e7b010dcf75
> --- /dev/null
> +++ b/dcb/dcb_app.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __DCB_APP_H_
> +#define __DCB_APP_H_
> +
> +struct dcb;
> +
> +struct dcb_app_table {
> +	struct dcb_app *apps;
> +	size_t n_apps;
> +};
> +
> +struct dcb_app_add_del {
> +	const struct dcb_app_table *tab;
> +	bool (*filter)(const struct dcb_app *app);
> +};
> +
> +struct dcb_app_parse_mapping {
> +	__u8 selector;
> +	struct dcb_app_table *tab;
> +	int err;
> +};
> +
> +#define DCB_APP_PCP_MAX 15
> +
> +int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
> +
> +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab);
> +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
> +		    const struct dcb_app_table *tab,
> +		    bool (*filter)(const struct dcb_app *));
> +
> +void dcb_app_table_sort(struct dcb_app_table *tab);
> +void dcb_app_table_fini(struct dcb_app_table *tab);
> +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b);
> +void dcb_app_table_remove_replaced(struct dcb_app_table *a,
> +				   const struct dcb_app_table *b);
> +void dcb_app_table_remove_existing(struct dcb_app_table *a,
> +				   const struct dcb_app_table *b);
> +
> +bool dcb_app_is_pcp(const struct dcb_app *app);
> +bool dcb_app_is_dscp(const struct dcb_app *app);
> +
> +int dcb_app_print_key_dec(__u16 protocol);
> +int dcb_app_print_key_dscp(__u16 protocol);
> +int dcb_app_print_key_pcp(__u16 protocol);
> +
> +int dcb_app_parse_pcp(__u32 *key, const char *arg);
> +int dcb_app_parse_dscp(__u32 *key, const char *arg);
> +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data);
> +
> +bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
> +bool dcb_app_attr_type_validate(enum ieee_attrs_app type);
> +enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector);
> +
> +#endif

Re: [PATCH iproute2-next 3/9] dcb: app: modify dcb-app print functions for dcb-rewr reuse

[Petr Machata]

Daniel Machon <daniel.machon@microchip.com> writes:

> -static void dcb_app_print_filtered(const struct dcb_app_table *tab,
> -				   bool (*filter)(const struct dcb_app *),
> -				   int (*print_key)(__u16 protocol),
> -				   const char *json_name,
> -				   const char *fp_name)
> +void dcb_app_print_filtered(const struct dcb_app_table *tab,
> +			    bool (*filter)(const struct dcb_app *),
> +			    int (*print_pid)(__u16 protocol),
> +			    const char *json_name, const char *fp_name)
>  {
>  	bool first = true;
>  	size_t i;
> @@ -439,8 +437,14 @@ static void dcb_app_print_filtered(const struct dcb_app_table *tab,
>  		}
>  
>  		open_json_array(PRINT_JSON, NULL);
> -		print_key(app->protocol);
> -		print_uint(PRINT_ANY, NULL, "%d ", app->priority);
> +		if (tab->attr == DCB_ATTR_IEEE_APP_TABLE) {
> +			print_pid(app->protocol);
> +			print_uint(PRINT_ANY, NULL, ":%d", app->priority);
> +		} else {
> +			print_uint(PRINT_ANY, NULL, "%d:", app->priority);
> +			print_pid(app->protocol);
> +		}

I really dislike the attribute dispatch. I feels too much like mixing
abstraction layers. I think the callback should take a full struct
dcb_app pointer and format it as appropriate. Then you can model the
rewrite table differently from the app table by providing a callback
that invokes the print_ helpers in the correct order.

The app->protocol field as such is not really necessary IMHO, because
the function that invokes the helpers understands what kind of table it
is dealing with and could provide it as a parameter. But OK, I guess it
makes sense and probably saves some boilerplate parameterization.

> +		print_string(PRINT_ANY, NULL, "%s", " ");
>  		close_json_array(PRINT_JSON, NULL);
>  	}
>  

Re: [PATCH iproute2-next 4/9] dcb: app: modify dcb_app_table_remove_replaced() for dcb-rewr reuse

[Petr Machata]

Daniel Machon <daniel.machon@microchip.com> writes:

> When doing a replace command, entries are checked against selector and
> protocol. Rewrite requires the check to be against selector and
> priority.
>
> Modify the existing dcb_app_table_remove_replace function for dcb-rewr
> reuse, by using the newly introduced dcbnl attribute in the
> dcb_app_table struct.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/dcb_app.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
> index 9bb64f32e12e..23d6bb2a0013 100644
> --- a/dcb/dcb_app.c
> +++ b/dcb/dcb_app.c
> @@ -160,15 +160,27 @@ void dcb_app_table_remove_replaced(struct dcb_app_table *a,
>  		for (ib = 0; ib < b->n_apps; ib++) {
>  			const struct dcb_app *ab = &b->apps[ib];
>  
> -			if (aa->selector == ab->selector &&
> -			    aa->protocol == ab->protocol)
> -				present = true;
> -			else
> +			if (aa->selector != ab->selector)
>  				continue;
>  
> -			if (aa->priority == ab->priority) {
> -				found = true;
> -				break;
> +			if (a->attr == DCB_ATTR_IEEE_APP_TABLE) {
> +				if (aa->protocol == ab->protocol)
> +					present = true;
> +				else
> +					continue;
> +				if (aa->priority == ab->priority) {
> +					found = true;
> +					break;
> +				}
> +			} else {
> +				if (aa->priority == ab->priority)
> +					present = true;
> +				else
> +					continue;
> +				if (aa->protocol == ab->protocol) {
> +					found = true;
> +					break;
> +				}
>  			}
>  		}

Same point about the attribute dispatch. How about this? (Not tested
though.)

	static bool dcb_app_pid_eq(const struct dcb_app *aa, const struct dcb_app *ab)
	{
		return aa->selector == ab->selector &&
		       aa->protocol == ab->protocol;
	}

	static bool dcb_app_prio_eq(const struct dcb_app *aa, const struct dcb_app *ab)
	{
		return aa->selector == ab->selector &&
		       aa->priority == ab->priority;
	}

	static void __dcb_app_table_remove_replaced(struct dcb_app_table *a,
						    const struct dcb_app_table *b,
						    bool (*key_eq)(const struct dcb_app *aa,
								const struct dcb_app *ab),
						    bool (*val_eq)(const struct dcb_app *aa,
								const struct dcb_app *ab))
	{
		size_t ia, ja;
		size_t ib;

		for (ia = 0, ja = 0; ia < a->n_apps; ia++) {
			struct dcb_app *aa = &a->apps[ia];
			bool present = false;
			bool found = false;

			for (ib = 0; ib < b->n_apps; ib++) {
				const struct dcb_app *ab = &b->apps[ib];

				if (key_eq(aa, ab))
					present = true;
				else
					continue;

				if (val_eq(aa, ab)) {
					found = true;
					break;
				}
			}

			/* Entries that remain in A will be removed, so keep in the
                         * table only APP entries whose sel/pid is mentioned in B,
			 * but that do not have the full sel/pid/prio match.
			 */
			if (present && !found)
				a->apps[ja++] = *aa;
		}

		a->n_apps = ja;
	}

	void dcb_app_table_remove_replaced(struct dcb_app_table *a,
					const struct dcb_app_table *b)
	{
		__dcb_app_table_remove_replaced(a, b, dcb_app_pid_eq, dcb_app_prio_eq);
	}

	void dcb_rwr_table_remove_replaced(struct dcb_app_table *a,
					const struct dcb_app_table *b)
	{
		__dcb_app_table_remove_replaced(a, b, dcb_app_prio_eq, dcb_app_pid_eq);
	}

Alternatively have key / value extractor callbacks and compare those
instead of directly priority and protocol.

And actually now that I think about it more, a key_eq / get_key callback
is all we need. Instead of val_eq / get_val, we can just compare the
full app. We know the key matches already, so whatever it actually is,
it will not prevent the second match.

Dunno. I just don't want the attribute field become a polymorphic type
tag of the structure. DCB is using these callbacks quite a bit all over
the place, so code like this will be right at home.

I was actually looking at dcb_app_table_remove_existing(), which is
tantalizingly close to being a special case of the above where key_eq
just always returns true and val_eq compares all fields. But alas for
empty tables it would do the wrong thing.

Re: [PATCH iproute2-next 5/9] dcb: app: modify dcb_app_parse_mapping_cb for dcb-rewr reuse

[Petr Machata]

Daniel Machon <daniel.machon@microchip.com> writes:

> When parsing APP table entries, priority and protocol is assigned from
> value and key, respectively. Rewrite requires it opposite.
>
> Modify the existing dcb_app_parse_mapping_cb for dcb-rewr reuse, by
> using the newly introduced dcbnl attribute in the dcb_app_table struct.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/dcb_app.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
> index 23d6bb2a0013..46af67112748 100644
> --- a/dcb/dcb_app.c
> +++ b/dcb/dcb_app.c
> @@ -232,10 +232,17 @@ void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
>  	struct dcb_app_parse_mapping *pm = data;
>  	struct dcb_app app = {
>  		.selector = pm->selector,
> -		.priority = value,
> -		.protocol = key,
>  	};
>  
> +	if (pm->tab->attr == DCB_ATTR_IEEE_APP_TABLE) {
> +		app.priority = value;
> +		app.protocol = key;
> +
> +	} else {
> +		app.priority = key;
> +		app.protocol = value;
> +	}
> +
>  	if (pm->err)
>  		return;

? (Or thereabouts... again, not tested.)

modified   dcb/dcb_app.c
@@ -225,22 +225,40 @@ static void dcb_app_table_sort(struct dcb_app_table *tab)
 struct dcb_app_parse_mapping {
 	__u8 selector;
 	struct dcb_app_table *tab;
+	int (*push)(struct dcb_app_table *tab,
+		    __u8 selector, __u32 key, __u64 value);
 	int err;
 };
 
-static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
+static int dcb_app_push_app(struct dcb_app_table *tab,
+			    __u8 selector, __u32 key, __u64 value)
 {
-	struct dcb_app_parse_mapping *pm = data;
 	struct dcb_app app = {
-		.selector = pm->selector,
+		.selector = selector,
 		.priority = value,
 		.protocol = key,
 	};
+	return dcb_app_table_push(tab, &app);
+}
+
+static int dcb_app_push_rewr(struct dcb_app_table *tab,
+			     __u8 selector, __u32 key, __u64 value)
+{
+	struct dcb_app app = {
+		.selector = selector,
+		.priority = key,
+		.protocol = value,
+	};
+	return dcb_app_table_push(tab, &app);
+}
+
+static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
+{
+	struct dcb_app_parse_mapping *pm = data;
 
 	if (pm->err)
 		return;
-
-	pm->err = dcb_app_table_push(pm->tab, &app);
+	pm->err = pm->push(pm->tab, pm->selector, key, value);
 }
 
 static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data)
@@ -640,6 +658,7 @@ static int dcb_cmd_app_parse_add_del(struct dcb *dcb, const char *dev,
 {
 	struct dcb_app_parse_mapping pm = {
 		.tab = tab,
+		.push = dcb_app_push_app,
 	};
 	int ret;

Re: [PATCH iproute2-next 6/9] dcb: rewr: add new dcb-rewr subcommand

[Petr Machata]

Daniel Machon <daniel.machon@microchip.com> writes:

> Add a new subcommand 'rewr' for configuring the in-kernel DCB rewrite
> table. The rewr-table of the kernel is similar to the APP-table, and so
> is this new subcommand. Therefore, much of the existing bookkeeping code
> from dcb-app, can be reused in the dcb-rewr implementation.
>
> Initially, only support for configuring PCP and DSCP-based rewrite has
> been added.

That's reasonable.

> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/Makefile   |   3 +-
>  dcb/dcb.c      |   4 +-
>  dcb/dcb.h      |   3 +
>  dcb/dcb_app.h  |   1 +
>  dcb/dcb_rewr.c | 332 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 341 insertions(+), 2 deletions(-)
>
> diff --git a/dcb/Makefile b/dcb/Makefile
> index dd41a559a0c8..10794c9dc19f 100644
> --- a/dcb/Makefile
> +++ b/dcb/Makefile
> @@ -8,7 +8,8 @@ DCBOBJ = dcb.o \
>           dcb_ets.o \
>           dcb_maxrate.o \
>           dcb_pfc.o \
> -         dcb_apptrust.o
> +         dcb_apptrust.o \
> +         dcb_rewr.o
>  TARGETS += dcb
>  LDLIBS += -lm
>  
> diff --git a/dcb/dcb.c b/dcb/dcb.c
> index 9b996abac529..fe0a0f04143d 100644
> --- a/dcb/dcb.c
> +++ b/dcb/dcb.c
> @@ -470,7 +470,7 @@ static void dcb_help(void)
>  	fprintf(stderr,
>  		"Usage: dcb [ OPTIONS ] OBJECT { COMMAND | help }\n"
>  		"       dcb [ -f | --force ] { -b | --batch } filename [ -n | --netns ] netnsname\n"
> -		"where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc }\n"
> +		"where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc | rewr }\n"
>  		"       OPTIONS := [ -V | --Version | -i | --iec | -j | --json\n"
>  		"                  | -N | --Numeric | -p | --pretty\n"
>  		"                  | -s | --statistics | -v | --verbose]\n");
> @@ -485,6 +485,8 @@ static int dcb_cmd(struct dcb *dcb, int argc, char **argv)
>  		return dcb_cmd_app(dcb, argc - 1, argv + 1);
>  	} else if (strcmp(*argv, "apptrust") == 0) {
>  		return dcb_cmd_apptrust(dcb, argc - 1, argv + 1);
> +	} else if (strcmp(*argv, "rewr") == 0) {
> +		return dcb_cmd_rewr(dcb, argc - 1, argv + 1);
>  	} else if (matches(*argv, "buffer") == 0) {
>  		return dcb_cmd_buffer(dcb, argc - 1, argv + 1);
>  	} else if (matches(*argv, "dcbx") == 0) {
> diff --git a/dcb/dcb.h b/dcb/dcb.h
> index 4c8a4aa25e0c..39a04f1c59df 100644
> --- a/dcb/dcb.h
> +++ b/dcb/dcb.h
> @@ -56,6 +56,9 @@ void dcb_print_array_on_off(const __u8 *array, size_t size);
>  void dcb_print_array_kw(const __u8 *array, size_t array_size,
>  			const char *const kw[], size_t kw_size);
>  
> +/* dcb_rewr.c */
> +int dcb_cmd_rewr(struct dcb *dcb, int argc, char **argv);
> +
>  /* dcb_apptrust.c */
>  
>  int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv);
> diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
> index 8f048605c3a8..02c9eb03f6c2 100644
> --- a/dcb/dcb_app.h
> +++ b/dcb/dcb_app.h
> @@ -22,6 +22,7 @@ struct dcb_app_parse_mapping {
>  };
>  
>  #define DCB_APP_PCP_MAX 15
> +#define DCB_APP_DSCP_MAX 63

It would be nice to have dcb_app_parse_mapping_dscp_prio() use that
define now that it exists. Back then I figured the value 63 in the
context that mentions DSCP is clear enough, and the value itself being
grounded in IEEE won't change, but... um, yeah, if the define exists,
let's use it :)

>  
>  int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
>  
> diff --git a/dcb/dcb_rewr.c b/dcb/dcb_rewr.c
> new file mode 100644
> index 000000000000..731ba78977e2
> --- /dev/null
> +++ b/dcb/dcb_rewr.c
> @@ -0,0 +1,332 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <errno.h>
> +#include <linux/dcbnl.h>
> +#include <stdio.h>
> +
> +#include "dcb.h"
> +#include "utils.h"
> +
> +static void dcb_rewr_help_add(void)
> +{
> +	fprintf(stderr,
> +		"Usage: dcb rewr { add | del | replace } dev STRING\n"
> +		"           [ prio-pcp PRIO:PCP   ]\n"
> +		"           [ prio-dscp PRIO:DSCP ]\n"
> +		"\n"
> +		" where PRIO := { 0 .. 7               }\n"
> +		"       PCP  := { 0(nd/de) .. 7(nd/de) }\n"
> +		"       DSCP := { 0 .. 63              }\n"

I was wondering if you had done something with this instance of 63 ;)

Can you also drop all those extra spaces? Here and elsewhere. These
tabular layouts only ever lead to later reformatting as longer lines
break it.

> +		"\n"
> +	);
> +}
> +
> +static void dcb_rewr_help_show_flush(void)
> +{
> +	fprintf(stderr,
> +		"Usage: dcb rewr { show | flush } dev STRING\n"
> +		"           [ prio-pcp  ]\n"
> +		"           [ prio-dscp ]\n"
> +		"\n"
> +	);
> +}
> +
> +static void dcb_rewr_help(void)
> +{
> +	fprintf(stderr,
> +		"Usage: dcb rewr help\n"
> +		"\n"
> +	);
> +	dcb_rewr_help_show_flush();
> +	dcb_rewr_help_add();
> +}
> +
> +static int dcb_rewr_parse_mapping_prio_pcp(__u32 key, char *value, void *data)
> +{
> +	__u32 pcp;
> +
> +	if (dcb_app_parse_pcp(&pcp, value))
> +		return -EINVAL;
> +
> +	return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1, "PCP",
> +				 pcp, DCB_APP_PCP_MAX, dcb_app_parse_mapping_cb,
> +				 data);

See, the way it's formatted in app makes it clear what's what. Consider:

	return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1,
				 "PCP", pcp, DCB_APP_PCP_MAX,
				 dcb_app_parse_mapping_cb, data);

PRIO has proposed value of "key" and goes up to IEEE_8021QAZ_MAX_TCS - 1,
PCP has "pcp", goes up to DCB_APP_PCP_MAX, please use this callback and
invoke it with "data".

The expression in this patch takes the same amount of space, but it's
much less clear what is what.

The same applies below.

> +}
> +
> +static int dcb_rewr_parse_mapping_prio_dscp(__u32 key, char *value, void *data)
> +{
> +	__u32 dscp;
> +
> +	if (dcb_app_parse_dscp(&dscp, value))
> +		return -EINVAL;
> +
> +	return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1, "DSCP",
> +				 dscp, DCB_APP_DSCP_MAX,
> +				 dcb_app_parse_mapping_cb, data);
> +}
> +
> +static void dcb_rewr_print_prio_pcp(const struct dcb *dcb,
> +				    const struct dcb_app_table *tab)
> +{
> +	dcb_app_print_filtered(tab, dcb_app_is_pcp,
> +			       dcb->numeric ? dcb_app_print_pid_dec :
> +					      dcb_app_print_pid_pcp,
> +			       "prio_pcp", "prio-pcp");
> +}
> +
> +static void dcb_rewr_print_prio_dscp(const struct dcb *dcb,
> +				     const struct dcb_app_table *tab)
> +{
> +	dcb_app_print_filtered(tab, dcb_app_is_dscp,
> +			       dcb->numeric ? dcb_app_print_pid_dec :
> +					      dcb_app_print_pid_dscp,
> +			       "prio_dscp", "prio-dscp");
> +}
> +
> +static void dcb_rewr_print(const struct dcb *dcb,
> +			   const struct dcb_app_table *tab)
> +{
> +	dcb_rewr_print_prio_pcp(dcb, tab);
> +	dcb_rewr_print_prio_dscp(dcb, tab);
> +}
> +
> +static int dcb_cmd_rewr_parse_add_del(struct dcb *dcb, const char *dev,
> +				      int argc, char **argv,
> +				      struct dcb_app_table *tab)
> +{
> +	struct dcb_app_parse_mapping pm = { .tab = tab };
> +	int ret;
> +
> +	if (!argc) {
> +		dcb_rewr_help_add();
> +		return 0;
> +	}
> +
> +	do {
> +		if (strcmp(*argv, "help") == 0) {
> +			dcb_rewr_help_add();
> +			return 0;
> +		} else if (strcmp(*argv, "prio-pcp") == 0) {
> +			NEXT_ARG();
> +			pm.selector = DCB_APP_SEL_PCP;
> +			ret = parse_mapping(&argc, &argv, false,
> +					    &dcb_rewr_parse_mapping_prio_pcp,
> +					    &pm);
> +		} else if (strcmp(*argv, "prio-dscp") == 0) {
> +			NEXT_ARG();
> +			pm.selector = IEEE_8021QAZ_APP_SEL_DSCP;
> +			ret = parse_mapping(&argc, &argv, false,
> +					    &dcb_rewr_parse_mapping_prio_dscp,
> +					    &pm);
> +		} else {
> +			fprintf(stderr, "What is \"%s\"?\n", *argv);
> +			dcb_rewr_help_add();
> +			return -EINVAL;
> +		}
> +
> +		if (ret != 0) {
> +			fprintf(stderr, "Invalid mapping %s\n", *argv);
> +			return ret;
> +		}
> +		if (pm.err)
> +			return pm.err;
> +	} while (argc > 0);
> +
> +	return 0;
> +}
> +
> +static int dcb_cmd_rewr_add(struct dcb *dcb, const char *dev, int argc,
> +			    char **argv)
> +{
> +	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> +	int ret;
> +
> +	ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_SET, &tab, NULL);
> +	dcb_app_table_fini(&tab);
> +	return ret;
> +}
> +
> +static int dcb_cmd_rewr_del(struct dcb *dcb, const char *dev, int argc,
> +			    char **argv)
> +{
> +	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> +	int ret;
> +
> +	ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab, NULL);
> +	dcb_app_table_fini(&tab);
> +	return ret;
> +}
> +
> +static int dcb_cmd_rewr_replace(struct dcb *dcb, const char *dev, int argc,
> +				char **argv)
> +{
> +	struct dcb_app_table orig = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> +	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> +	struct dcb_app_table new = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> +	int ret;
> +
> +	ret = dcb_app_get(dcb, dev, &orig);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
> +	if (ret != 0)
> +		goto out;
> +
> +	/* Attempts to add an existing entry would be rejected, so drop
> +	 * these entries from tab.
> +	 */
> +	ret = dcb_app_table_copy(&new, &tab);
> +	if (ret != 0)
> +		goto out;
> +	dcb_app_table_remove_existing(&new, &orig);
> +
> +	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_SET, &new, NULL);
> +	if (ret != 0) {
> +		fprintf(stderr, "Could not add new rewrite entries\n");
> +		goto out;
> +	}
> +
> +	/* Remove the obsolete entries. */
> +	dcb_app_table_remove_replaced(&orig, &tab);
> +	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &orig, NULL);
> +	if (ret != 0) {
> +		fprintf(stderr, "Could not remove replaced rewrite entries\n");
> +		goto out;
> +	}
> +
> +out:
> +	dcb_app_table_fini(&new);
> +	dcb_app_table_fini(&tab);
> +	dcb_app_table_fini(&orig);
> +	return 0;
> +}
> +
> +
> +static int dcb_cmd_rewr_show(struct dcb *dcb, const char *dev, int argc,
> +			     char **argv)
> +{
> +	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> +	int ret;
> +
> +	ret = dcb_app_get(dcb, dev, &tab);
> +	if (ret != 0)
> +		return ret;
> +
> +	dcb_app_table_sort(&tab);
> +
> +	open_json_object(NULL);
> +
> +	if (!argc) {
> +		dcb_rewr_print(dcb, &tab);
> +		goto out;
> +	}
> +
> +	do {
> +		if (strcmp(*argv, "help") == 0) {
> +			dcb_rewr_help_show_flush();
> +			goto out;
> +		} else if (strcmp(*argv, "prio-pcp") == 0) {
> +			dcb_rewr_print_prio_pcp(dcb, &tab);
> +		} else if (strcmp(*argv, "prio-dscp") == 0) {
> +			dcb_rewr_print_prio_dscp(dcb, &tab);
> +		} else {
> +			fprintf(stderr, "What is \"%s\"?\n", *argv);
> +			dcb_rewr_help_show_flush();
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		NEXT_ARG_FWD();
> +	} while (argc > 0);
> +
> +out:
> +	close_json_object();
> +	dcb_app_table_fini(&tab);
> +	return ret;
> +}
> +
> +static int dcb_cmd_rewr_flush(struct dcb *dcb, const char *dev, int argc,
> +			      char **argv)
> +{
> +	struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> +	int ret;
> +
> +	ret = dcb_app_get(dcb, dev, &tab);
> +	if (ret != 0)
> +		return ret;
> +
> +	if (!argc) {
> +		ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
> +				      NULL);
> +		goto out;
> +	}
> +
> +	do {
> +		if (strcmp(*argv, "help") == 0) {
> +			dcb_rewr_help_show_flush();
> +			goto out;
> +		} else if (strcmp(*argv, "prio-pcp") == 0) {
> +			ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
> +					      &dcb_app_is_pcp);
> +			if (ret != 0)
> +				goto out;
> +		} else if (strcmp(*argv, "prio-dscp") == 0) {
> +			ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
> +					      &dcb_app_is_dscp);
> +			if (ret != 0)
> +				goto out;
> +		} else {
> +			fprintf(stderr, "What is \"%s\"?\n", *argv);
> +			dcb_rewr_help_show_flush();
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		NEXT_ARG_FWD();
> +	} while (argc > 0);
> +
> +out:
> +	dcb_app_table_fini(&tab);
> +	return ret;
> +}
> +
> +int dcb_cmd_rewr(struct dcb *dcb, int argc, char **argv)
> +{
> +	if (!argc || strcmp(*argv, "help") == 0) {
> +		dcb_rewr_help();
> +		return 0;
> +	} else if (strcmp(*argv, "show") == 0) {
> +		NEXT_ARG_FWD();
> +		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_show,
> +					 dcb_rewr_help_show_flush);
> +	} else if (strcmp(*argv, "flush") == 0) {
> +		NEXT_ARG_FWD();
> +		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_flush,
> +					 dcb_rewr_help_show_flush);
> +	} else if (strcmp(*argv, "add") == 0) {
> +		NEXT_ARG_FWD();
> +		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_add,
> +					 dcb_rewr_help_add);
> +	} else if (strcmp(*argv, "del") == 0) {
> +		NEXT_ARG_FWD();
> +		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_del,
> +					 dcb_rewr_help_add);
> +	} else if (strcmp(*argv, "replace") == 0) {
> +		NEXT_ARG_FWD();
> +		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_replace,
> +					 dcb_rewr_help_add);
> +	} else {
> +		fprintf(stderr, "What is \"%s\"?\n", *argv);
> +		dcb_rewr_help();
> +		return -EINVAL;
> +	}
> +}

Re: [PATCH iproute2-next 9/9] man: dcb-app: clean up a few mistakes

[Petr Machata]

Daniel Machon <daniel.machon@microchip.com> writes:

> While referencing the dcb-app manpage, I spotted a few mistakes. Lets
> fix them.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  man/man8/dcb-app.8 | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/man/man8/dcb-app.8 b/man/man8/dcb-app.8
> index ecb38591168e..ebec67c90801 100644
> --- a/man/man8/dcb-app.8
> +++ b/man/man8/dcb-app.8
> @@ -1,4 +1,4 @@
> -.TH DCB-ETS 8 "6 December 2020" "iproute2" "Linux"
> +.TH DCB-APP 8 "6 December 2020" "iproute2" "Linux"
>  .SH NAME
>  dcb-app \- show / manipulate application priority table of
>  the DCB (Data Center Bridging) subsystem
> @@ -26,7 +26,7 @@ the DCB (Data Center Bridging) subsystem
>  .RB "[ " pcp-prio " ]"
>  
>  .ti -8
> -.B dcb ets " { " add " | " del " | " replace " } " dev
> +.B dcb app " { " add " | " del " | " replace " } " dev
>  .RI DEV
>  .RB "[ " default-prio " " \fIPRIO-LIST\fB " ]"
>  .RB "[ " ethtype-prio " " \fIET-MAP\fB " ]"
> @@ -106,7 +106,7 @@ individual APP 3-tuples through
>  .B add
>  and
>  .B del
> -commands. On the other other hand, the command
> +commands. On the other hand, the command

Heh, neat typo.

>  .B replace
>  does what one would typically want in this situation--first adds the new
>  configuration, and then removes the obsolete one, so that only one
> @@ -184,7 +184,7 @@ for details. Keys are DSCP points, values are priorities assigned to
>  traffic with matching DSCP. DSCP points can be written either directly as
>  numeric values, or using symbolic names specified in
>  .B /etc/iproute2/rt_dsfield
> -(however note that that file specifies full 8-bit dsfield values, whereas
> +(however note that file specifies full 8-bit dsfield values, whereas

Nope, this one's correct. The first that is a conjunction, the second
one... a pronoun? Not sure.

Maybe make it "note that the file"? It's clear what file we are talking
about and the grating "that that" goes away. Pretty sure it's a
stylistic faux pas.

>  .B dcb app
>  will only use the higher six bits).
>  .B dcb app show
> @@ -230,7 +230,7 @@ priority 4:
>  .P
>  # dcb app replace dev eth0 dscp-prio 24:4
>  .br
> -# dcb app show dev eth0 dscp-prio
> +# dcb app -N show dev eth0 dscp-prio
>  .br
>  dscp-prio 0:0 24:4 48:6

Re: [PATCH iproute2-next 7/9] man: dcb-rewr: add new manpage for dcb-rewr

[Petr Machata]

Daniel Machon <daniel.machon@microchip.com> writes:

> +.TP
> +.B prio-dscp \fIDSCP-MAP
> +\fIDSCP-MAP\fR uses the array parameter syntax, see
> +.BR dcb (8)
> +for details. Keys are priorities, values are DSCP points for traffic
> +with matching priority. DSCP points can be written either directly as numeric
> +values, or using symbolic names specified in
> +.B /etc/iproute2/rt_dsfield
> +(however note that file specifies full 8-bit dsfield values, whereas

... and please whatever form the "that that" ends up having in 9/9, use
it here as well.

Re: [PATCH iproute2-next 1/9] dcb: app: expose dcb-app functions in new header

[Daniel.Machon]

> > Add new headerfile dcb-app.h that exposes the functions required later
> > by dcb-rewr. The new dcb-rewr implementation will reuse much of the
> > existing dcb-app code.
> >
> > I thought this called for a separate header file, instead of polluting
> > the existing dcb.h file.
> >
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  dcb/dcb.h     |  9 ++-------
> >  dcb/dcb_app.c | 54 ++++++++++++++++++------------------------------------
> >  dcb/dcb_app.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 75 insertions(+), 43 deletions(-)
> >
> > diff --git a/dcb/dcb.h b/dcb/dcb.h
> > index d40664f29dad..4c8a4aa25e0c 100644
> > --- a/dcb/dcb.h
> > +++ b/dcb/dcb.h
> > @@ -6,6 +6,8 @@
> >  #include <stdbool.h>
> >  #include <stddef.h>
> >
> > +#include "dcb_app.h"
> > +
> >  /* dcb.c */
> >
> >  struct dcb {
> > @@ -54,13 +56,6 @@ void dcb_print_array_on_off(const __u8 *array, size_t size);
> >  void dcb_print_array_kw(const __u8 *array, size_t array_size,
> >                       const char *const kw[], size_t kw_size);
> >
> > -/* dcb_app.c */
> > -
> > -int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
> > -enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector);
> > -bool dcb_app_attr_type_validate(enum ieee_attrs_app type);
> > -bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
> > -
> 
> Why the move to a dedicated header? dcb.h ends up being the only client
> and everybody consumes the prototypes through that file anyway. I don't
> fine it necessary.

I did try to rationalize that a bit in the commit description. I thought
the amount of exposed app functions ended up polutting the dcb header.
Maybe it is not that bad - can move them back in the next v.

> 
> >  /* dcb_apptrust.c */
> >
> >  int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv);
> > diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
> > index eeb78e70f63f..df339babd8e6 100644
> > --- a/dcb/dcb_app.c
> > +++ b/dcb/dcb_app.c
> > @@ -10,8 +10,6 @@
> >  #include "utils.h"
> >  #include "rt_names.h"
> >
> > -#define DCB_APP_PCP_MAX 15
> > -
> >  static const char *const pcp_names[DCB_APP_PCP_MAX + 1] = {
> >       "0nd", "1nd", "2nd", "3nd", "4nd", "5nd", "6nd", "7nd",
> >       "0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de"
> > @@ -22,6 +20,7 @@ static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = {
> >       [DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP"
> >  };
> >
> > +
> 
> This looks like a leftover.
> 
> >  static void dcb_app_help_add(void)
> >  {
> >       fprintf(stderr,
> > @@ -68,11 +67,6 @@ static void dcb_app_help(void)
> >       dcb_app_help_add();
> >  }
> >
> > -struct dcb_app_table {
> > -     struct dcb_app *apps;
> > -     size_t n_apps;
> > -};
> > -
> >  enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector)
> >  {
> >       switch (selector) {
> > @@ -105,7 +99,7 @@ bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector)
> >       return dcb_app_attr_type_get(selector) == type;
> >  }
> >
> > -static void dcb_app_table_fini(struct dcb_app_table *tab)
> > +void dcb_app_table_fini(struct dcb_app_table *tab)
> >  {
> >       free(tab->apps);
> >  }
> > @@ -124,8 +118,8 @@ static int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app)
> >       return 0;
> >  }
> >
> > -static void dcb_app_table_remove_existing(struct dcb_app_table *a,
> > -                                       const struct dcb_app_table *b)
> > +void dcb_app_table_remove_existing(struct dcb_app_table *a,
> > +                                const struct dcb_app_table *b)
> >  {
> >       size_t ia, ja;
> >       size_t ib;
> > @@ -152,8 +146,8 @@ static void dcb_app_table_remove_existing(struct dcb_app_table *a,
> >       a->n_apps = ja;
> >  }
> >
> > -static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
> > -                                       const struct dcb_app_table *b)
> > +void dcb_app_table_remove_replaced(struct dcb_app_table *a,
> > +                                const struct dcb_app_table *b)
> >  {
> >       size_t ia, ja;
> >       size_t ib;
> > @@ -189,8 +183,7 @@ static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
> >       a->n_apps = ja;
> >  }
> >
> > -static int dcb_app_table_copy(struct dcb_app_table *a,
> > -                           const struct dcb_app_table *b)
> > +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b)
> >  {
> >       size_t i;
> >       int ret;
> > @@ -217,18 +210,12 @@ static int dcb_app_cmp_cb(const void *a, const void *b)
> >       return dcb_app_cmp(a, b);
> >  }
> >
> > -static void dcb_app_table_sort(struct dcb_app_table *tab)
> > +void dcb_app_table_sort(struct dcb_app_table *tab)
> >  {
> >       qsort(tab->apps, tab->n_apps, sizeof(*tab->apps), dcb_app_cmp_cb);
> >  }
> >
> > -struct dcb_app_parse_mapping {
> > -     __u8 selector;
> > -     struct dcb_app_table *tab;
> > -     int err;
> > -};
> > -
> > -static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
> > +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
> >  {
> >       struct dcb_app_parse_mapping *pm = data;
> >       struct dcb_app app = {
> > @@ -260,7 +247,7 @@ static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data
> >                                dcb_app_parse_mapping_cb, data);
> >  }
> >
> > -static int dcb_app_parse_pcp(__u32 *key, const char *arg)
> > +int dcb_app_parse_pcp(__u32 *key, const char *arg)
> >  {
> >       int i;
> >
> > @@ -286,7 +273,7 @@ static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data)
> >                                dcb_app_parse_mapping_cb, data);
> >  }
> >
> > -static int dcb_app_parse_dscp(__u32 *key, const char *arg)
> > +int dcb_app_parse_dscp(__u32 *key, const char *arg)
> >  {
> >       if (parse_mapping_num_all(key, arg) == 0)
> >               return 0;
> > @@ -377,12 +364,12 @@ static bool dcb_app_is_default(const struct dcb_app *app)
> >              app->protocol == 0;
> >  }
> >
> > -static bool dcb_app_is_dscp(const struct dcb_app *app)
> > +bool dcb_app_is_dscp(const struct dcb_app *app)
> >  {
> >       return app->selector == IEEE_8021QAZ_APP_SEL_DSCP;
> >  }
> >
> > -static bool dcb_app_is_pcp(const struct dcb_app *app)
> > +bool dcb_app_is_pcp(const struct dcb_app *app)
> >  {
> >       return app->selector == DCB_APP_SEL_PCP;
> >  }
> > @@ -402,7 +389,7 @@ static bool dcb_app_is_port(const struct dcb_app *app)
> >       return app->selector == IEEE_8021QAZ_APP_SEL_ANY;
> >  }
> >
> > -static int dcb_app_print_key_dec(__u16 protocol)
> > +int dcb_app_print_key_dec(__u16 protocol)
> >  {
> >       return print_uint(PRINT_ANY, NULL, "%d:", protocol);
> >  }
> > @@ -412,7 +399,7 @@ static int dcb_app_print_key_hex(__u16 protocol)
> >       return print_uint(PRINT_ANY, NULL, "%x:", protocol);
> >  }
> >
> > -static int dcb_app_print_key_dscp(__u16 protocol)
> > +int dcb_app_print_key_dscp(__u16 protocol)
> >  {
> >       const char *name = rtnl_dsfield_get_name(protocol << 2);
> >
> > @@ -422,7 +409,7 @@ static int dcb_app_print_key_dscp(__u16 protocol)
> >       return print_uint(PRINT_ANY, NULL, "%d:", protocol);
> >  }
> >
> > -static int dcb_app_print_key_pcp(__u16 protocol)
> > +int dcb_app_print_key_pcp(__u16 protocol)
> >  {
> >       /* Print in numerical form, if protocol value is out-of-range */
> >       if (protocol > DCB_APP_PCP_MAX)
> > @@ -577,7 +564,7 @@ static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data)
> >       return MNL_CB_OK;
> >  }
> >
> > -static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab)
> > +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab)
> >  {
> >       uint16_t payload_len;
> >       void *payload;
> > @@ -594,11 +581,6 @@ static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *t
> >       return 0;
> >  }
> >
> > -struct dcb_app_add_del {
> > -     const struct dcb_app_table *tab;
> > -     bool (*filter)(const struct dcb_app *app);
> > -};
> > -
> 
> This structure is a protocol between dcb_app_add_del() and
> dcb_app_add_del_cb(). I don't think your patchset uses it elsewhere, so
> it can be kept private.

Yep. You are right.

> 
> >  static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
> >  {
> >       struct dcb_app_add_del *add_del = data;
> > @@ -620,7 +602,7 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
> >       return 0;
> >  }
> >
> > -static int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
> > +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
> >                          const struct dcb_app_table *tab,
> >                          bool (*filter)(const struct dcb_app *))
> 
> This has wrong indentation.

Ouch. Will fix in next v.

> 
> >  {
> > diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
> > new file mode 100644
> > index 000000000000..8e7b010dcf75
> > --- /dev/null
> > +++ b/dcb/dcb_app.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __DCB_APP_H_
> > +#define __DCB_APP_H_
> > +
> > +struct dcb;
> > +
> > +struct dcb_app_table {
> > +     struct dcb_app *apps;
> > +     size_t n_apps;
> > +};
> > +
> > +struct dcb_app_add_del {
> > +     const struct dcb_app_table *tab;
> > +     bool (*filter)(const struct dcb_app *app);
> > +};
> > +
> > +struct dcb_app_parse_mapping {
> > +     __u8 selector;
> > +     struct dcb_app_table *tab;
> > +     int err;
> > +};
> > +
> > +#define DCB_APP_PCP_MAX 15
> > +
> > +int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
> > +
> > +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab);
> > +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command,
> > +                 const struct dcb_app_table *tab,
> > +                 bool (*filter)(const struct dcb_app *));
> > +
> > +void dcb_app_table_sort(struct dcb_app_table *tab);
> > +void dcb_app_table_fini(struct dcb_app_table *tab);
> > +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b);
> > +void dcb_app_table_remove_replaced(struct dcb_app_table *a,
> > +                                const struct dcb_app_table *b);
> > +void dcb_app_table_remove_existing(struct dcb_app_table *a,
> > +                                const struct dcb_app_table *b);
> > +
> > +bool dcb_app_is_pcp(const struct dcb_app *app);
> > +bool dcb_app_is_dscp(const struct dcb_app *app);
> > +
> > +int dcb_app_print_key_dec(__u16 protocol);
> > +int dcb_app_print_key_dscp(__u16 protocol);
> > +int dcb_app_print_key_pcp(__u16 protocol);
> > +
> > +int dcb_app_parse_pcp(__u32 *key, const char *arg);
> > +int dcb_app_parse_dscp(__u32 *key, const char *arg);
> > +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data);
> > +
> > +bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
> > +bool dcb_app_attr_type_validate(enum ieee_attrs_app type);
> > +enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector);
> > +
> > +#endif
> 

Re: [PATCH iproute2-next 3/9] dcb: app: modify dcb-app print functions for dcb-rewr reuse

[Daniel.Machon]

> Daniel Machon <daniel.machon@microchip.com> writes:
> 
> > -static void dcb_app_print_filtered(const struct dcb_app_table *tab,
> > -                                bool (*filter)(const struct dcb_app *),
> > -                                int (*print_key)(__u16 protocol),
> > -                                const char *json_name,
> > -                                const char *fp_name)
> > +void dcb_app_print_filtered(const struct dcb_app_table *tab,
> > +                         bool (*filter)(const struct dcb_app *),
> > +                         int (*print_pid)(__u16 protocol),
> > +                         const char *json_name, const char *fp_name)
> >  {
> >       bool first = true;
> >       size_t i;
> > @@ -439,8 +437,14 @@ static void dcb_app_print_filtered(const struct dcb_app_table *tab,
> >               }
> >
> >               open_json_array(PRINT_JSON, NULL);
> > -             print_key(app->protocol);
> > -             print_uint(PRINT_ANY, NULL, "%d ", app->priority);
> > +             if (tab->attr == DCB_ATTR_IEEE_APP_TABLE) {
> > +                     print_pid(app->protocol);
> > +                     print_uint(PRINT_ANY, NULL, ":%d", app->priority);
> > +             } else {
> > +                     print_uint(PRINT_ANY, NULL, "%d:", app->priority);
> > +                     print_pid(app->protocol);
> > +             }
> 
> I really dislike the attribute dispatch. I feels too much like mixing
> abstraction layers. I think the callback should take a full struct
> dcb_app pointer and format it as appropriate. Then you can model the
> rewrite table differently from the app table by providing a callback
> that invokes the print_ helpers in the correct order.
> 
> The app->protocol field as such is not really necessary IMHO, because
> the function that invokes the helpers understands what kind of table it
> is dealing with and could provide it as a parameter. But OK, I guess it
> makes sense and probably saves some boilerplate parameterization.

Roger. And actually, yeah, the callbacks are used heavily throughout
DCB, so that fits better. Will incorporate CB approach in next v. I
think this applies more or less to your comments in patch #3, #4 and #5
too :)

Re: [PATCH iproute2-next 6/9] dcb: rewr: add new dcb-rewr subcommand

[Daniel.Machon]

> Daniel Machon <daniel.machon@microchip.com> writes:
> 
> > Add a new subcommand 'rewr' for configuring the in-kernel DCB rewrite
> > table. The rewr-table of the kernel is similar to the APP-table, and so
> > is this new subcommand. Therefore, much of the existing bookkeeping code
> > from dcb-app, can be reused in the dcb-rewr implementation.
> >
> > Initially, only support for configuring PCP and DSCP-based rewrite has
> > been added.
> 
> That's reasonable.

Good :)

> 
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  dcb/Makefile   |   3 +-
> >  dcb/dcb.c      |   4 +-
> >  dcb/dcb.h      |   3 +
> >  dcb/dcb_app.h  |   1 +
> >  dcb/dcb_rewr.c | 332 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 341 insertions(+), 2 deletions(-)
> >
> > diff --git a/dcb/Makefile b/dcb/Makefile
> > index dd41a559a0c8..10794c9dc19f 100644
> > --- a/dcb/Makefile
> > +++ b/dcb/Makefile
> > @@ -8,7 +8,8 @@ DCBOBJ = dcb.o \
> >           dcb_ets.o \
> >           dcb_maxrate.o \
> >           dcb_pfc.o \
> > -         dcb_apptrust.o
> > +         dcb_apptrust.o \
> > +         dcb_rewr.o
> >  TARGETS += dcb
> >  LDLIBS += -lm
> >
> > diff --git a/dcb/dcb.c b/dcb/dcb.c
> > index 9b996abac529..fe0a0f04143d 100644
> > --- a/dcb/dcb.c
> > +++ b/dcb/dcb.c
> > @@ -470,7 +470,7 @@ static void dcb_help(void)
> >       fprintf(stderr,
> >               "Usage: dcb [ OPTIONS ] OBJECT { COMMAND | help }\n"
> >               "       dcb [ -f | --force ] { -b | --batch } filename [ -n | --netns ] netnsname\n"
> > -             "where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc }\n"
> > +             "where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc | rewr }\n"
> >               "       OPTIONS := [ -V | --Version | -i | --iec | -j | --json\n"
> >               "                  | -N | --Numeric | -p | --pretty\n"
> >               "                  | -s | --statistics | -v | --verbose]\n");
> > @@ -485,6 +485,8 @@ static int dcb_cmd(struct dcb *dcb, int argc, char **argv)
> >               return dcb_cmd_app(dcb, argc - 1, argv + 1);
> >       } else if (strcmp(*argv, "apptrust") == 0) {
> >               return dcb_cmd_apptrust(dcb, argc - 1, argv + 1);
> > +     } else if (strcmp(*argv, "rewr") == 0) {
> > +             return dcb_cmd_rewr(dcb, argc - 1, argv + 1);
> >       } else if (matches(*argv, "buffer") == 0) {
> >               return dcb_cmd_buffer(dcb, argc - 1, argv + 1);
> >       } else if (matches(*argv, "dcbx") == 0) {
> > diff --git a/dcb/dcb.h b/dcb/dcb.h
> > index 4c8a4aa25e0c..39a04f1c59df 100644
> > --- a/dcb/dcb.h
> > +++ b/dcb/dcb.h
> > @@ -56,6 +56,9 @@ void dcb_print_array_on_off(const __u8 *array, size_t size);
> >  void dcb_print_array_kw(const __u8 *array, size_t array_size,
> >                       const char *const kw[], size_t kw_size);
> >
> > +/* dcb_rewr.c */
> > +int dcb_cmd_rewr(struct dcb *dcb, int argc, char **argv);
> > +
> >  /* dcb_apptrust.c */
> >
> >  int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv);
> > diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h
> > index 8f048605c3a8..02c9eb03f6c2 100644
> > --- a/dcb/dcb_app.h
> > +++ b/dcb/dcb_app.h
> > @@ -22,6 +22,7 @@ struct dcb_app_parse_mapping {
> >  };
> >
> >  #define DCB_APP_PCP_MAX 15
> > +#define DCB_APP_DSCP_MAX 63
> 
> It would be nice to have dcb_app_parse_mapping_dscp_prio() use that
> define now that it exists. Back then I figured the value 63 in the
> context that mentions DSCP is clear enough, and the value itself being
> grounded in IEEE won't change, but... um, yeah, if the define exists,
> let's use it :)

I introduced it for PCP back then, so why not for DSCP also. Will update
use in dcb-app also.

> 
> >
> >  int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
> >
> > diff --git a/dcb/dcb_rewr.c b/dcb/dcb_rewr.c
> > new file mode 100644
> > index 000000000000..731ba78977e2
> > --- /dev/null
> > +++ b/dcb/dcb_rewr.c
> > @@ -0,0 +1,332 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include <errno.h>
> > +#include <linux/dcbnl.h>
> > +#include <stdio.h>
> > +
> > +#include "dcb.h"
> > +#include "utils.h"
> > +
> > +static void dcb_rewr_help_add(void)
> > +{
> > +     fprintf(stderr,
> > +             "Usage: dcb rewr { add | del | replace } dev STRING\n"
> > +             "           [ prio-pcp PRIO:PCP   ]\n"
> > +             "           [ prio-dscp PRIO:DSCP ]\n"
> > +             "\n"
> > +             " where PRIO := { 0 .. 7               }\n"
> > +             "       PCP  := { 0(nd/de) .. 7(nd/de) }\n"
> > +             "       DSCP := { 0 .. 63              }\n"
> 
> I was wondering if you had done something with this instance of 63 ;)
> 
> Can you also drop all those extra spaces? Here and elsewhere. These
> tabular layouts only ever lead to later reformatting as longer lines
> break it.

Sure.

> 
> > +             "\n"
> > +     );
> > +}
> > +
> > +static void dcb_rewr_help_show_flush(void)
> > +{
> > +     fprintf(stderr,
> > +             "Usage: dcb rewr { show | flush } dev STRING\n"
> > +             "           [ prio-pcp  ]\n"
> > +             "           [ prio-dscp ]\n"
> > +             "\n"
> > +     );
> > +}
> > +
> > +static void dcb_rewr_help(void)
> > +{
> > +     fprintf(stderr,
> > +             "Usage: dcb rewr help\n"
> > +             "\n"
> > +     );
> > +     dcb_rewr_help_show_flush();
> > +     dcb_rewr_help_add();
> > +}
> > +
> > +static int dcb_rewr_parse_mapping_prio_pcp(__u32 key, char *value, void *data)
> > +{
> > +     __u32 pcp;
> > +
> > +     if (dcb_app_parse_pcp(&pcp, value))
> > +             return -EINVAL;
> > +
> > +     return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1, "PCP",
> > +                              pcp, DCB_APP_PCP_MAX, dcb_app_parse_mapping_cb,
> > +                              data);
> 
> See, the way it's formatted in app makes it clear what's what. Consider:
> 
>         return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1,
>                                  "PCP", pcp, DCB_APP_PCP_MAX,
>                                  dcb_app_parse_mapping_cb, data);
> 
> PRIO has proposed value of "key" and goes up to IEEE_8021QAZ_MAX_TCS - 1,
> PCP has "pcp", goes up to DCB_APP_PCP_MAX, please use this callback and
> invoke it with "data".
> 
> The expression in this patch takes the same amount of space, but it's
> much less clear what is what.
> 
> The same applies below.

Ack.

> 
> > +}
> > +
> > +static int dcb_rewr_parse_mapping_prio_dscp(__u32 key, char *value, void *data)
> > +{
> > +     __u32 dscp;
> > +
> > +     if (dcb_app_parse_dscp(&dscp, value))
> > +             return -EINVAL;
> > +
> > +     return dcb_parse_mapping("PRIO", key, IEEE_8021QAZ_MAX_TCS - 1, "DSCP",
> > +                              dscp, DCB_APP_DSCP_MAX,
> > +                              dcb_app_parse_mapping_cb, data);
> > +}
> > +
> > +static void dcb_rewr_print_prio_pcp(const struct dcb *dcb,
> > +                                 const struct dcb_app_table *tab)
> > +{
> > +     dcb_app_print_filtered(tab, dcb_app_is_pcp,
> > +                            dcb->numeric ? dcb_app_print_pid_dec :
> > +                                           dcb_app_print_pid_pcp,
> > +                            "prio_pcp", "prio-pcp");
> > +}
> > +
> > +static void dcb_rewr_print_prio_dscp(const struct dcb *dcb,
> > +                                  const struct dcb_app_table *tab)
> > +{
> > +     dcb_app_print_filtered(tab, dcb_app_is_dscp,
> > +                            dcb->numeric ? dcb_app_print_pid_dec :
> > +                                           dcb_app_print_pid_dscp,
> > +                            "prio_dscp", "prio-dscp");
> > +}
> > +
> > +static void dcb_rewr_print(const struct dcb *dcb,
> > +                        const struct dcb_app_table *tab)
> > +{
> > +     dcb_rewr_print_prio_pcp(dcb, tab);
> > +     dcb_rewr_print_prio_dscp(dcb, tab);
> > +}
> > +
> > +static int dcb_cmd_rewr_parse_add_del(struct dcb *dcb, const char *dev,
> > +                                   int argc, char **argv,
> > +                                   struct dcb_app_table *tab)
> > +{
> > +     struct dcb_app_parse_mapping pm = { .tab = tab };
> > +     int ret;
> > +
> > +     if (!argc) {
> > +             dcb_rewr_help_add();
> > +             return 0;
> > +     }
> > +
> > +     do {
> > +             if (strcmp(*argv, "help") == 0) {
> > +                     dcb_rewr_help_add();
> > +                     return 0;
> > +             } else if (strcmp(*argv, "prio-pcp") == 0) {
> > +                     NEXT_ARG();
> > +                     pm.selector = DCB_APP_SEL_PCP;
> > +                     ret = parse_mapping(&argc, &argv, false,
> > +                                         &dcb_rewr_parse_mapping_prio_pcp,
> > +                                         &pm);
> > +             } else if (strcmp(*argv, "prio-dscp") == 0) {
> > +                     NEXT_ARG();
> > +                     pm.selector = IEEE_8021QAZ_APP_SEL_DSCP;
> > +                     ret = parse_mapping(&argc, &argv, false,
> > +                                         &dcb_rewr_parse_mapping_prio_dscp,
> > +                                         &pm);
> > +             } else {
> > +                     fprintf(stderr, "What is \"%s\"?\n", *argv);
> > +                     dcb_rewr_help_add();
> > +                     return -EINVAL;
> > +             }
> > +
> > +             if (ret != 0) {
> > +                     fprintf(stderr, "Invalid mapping %s\n", *argv);
> > +                     return ret;
> > +             }
> > +             if (pm.err)
> > +                     return pm.err;
> > +     } while (argc > 0);
> > +
> > +     return 0;
> > +}
> > +
> > +static int dcb_cmd_rewr_add(struct dcb *dcb, const char *dev, int argc,
> > +                         char **argv)
> > +{
> > +     struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> > +     int ret;
> > +
> > +     ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
> > +     if (ret != 0)
> > +             return ret;
> > +
> > +     ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_SET, &tab, NULL);
> > +     dcb_app_table_fini(&tab);
> > +     return ret;
> > +}
> > +
> > +static int dcb_cmd_rewr_del(struct dcb *dcb, const char *dev, int argc,
> > +                         char **argv)
> > +{
> > +     struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> > +     int ret;
> > +
> > +     ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
> > +     if (ret != 0)
> > +             return ret;
> > +
> > +     ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab, NULL);
> > +     dcb_app_table_fini(&tab);
> > +     return ret;
> > +}
> > +
> > +static int dcb_cmd_rewr_replace(struct dcb *dcb, const char *dev, int argc,
> > +                             char **argv)
> > +{
> > +     struct dcb_app_table orig = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> > +     struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> > +     struct dcb_app_table new = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> > +     int ret;
> > +
> > +     ret = dcb_app_get(dcb, dev, &orig);
> > +     if (ret != 0)
> > +             return ret;
> > +
> > +     ret = dcb_cmd_rewr_parse_add_del(dcb, dev, argc, argv, &tab);
> > +     if (ret != 0)
> > +             goto out;
> > +
> > +     /* Attempts to add an existing entry would be rejected, so drop
> > +      * these entries from tab.
> > +      */
> > +     ret = dcb_app_table_copy(&new, &tab);
> > +     if (ret != 0)
> > +             goto out;
> > +     dcb_app_table_remove_existing(&new, &orig);
> > +
> > +     ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_SET, &new, NULL);
> > +     if (ret != 0) {
> > +             fprintf(stderr, "Could not add new rewrite entries\n");
> > +             goto out;
> > +     }
> > +
> > +     /* Remove the obsolete entries. */
> > +     dcb_app_table_remove_replaced(&orig, &tab);
> > +     ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &orig, NULL);
> > +     if (ret != 0) {
> > +             fprintf(stderr, "Could not remove replaced rewrite entries\n");
> > +             goto out;
> > +     }
> > +
> > +out:
> > +     dcb_app_table_fini(&new);
> > +     dcb_app_table_fini(&tab);
> > +     dcb_app_table_fini(&orig);
> > +     return 0;
> > +}
> > +
> > +
> > +static int dcb_cmd_rewr_show(struct dcb *dcb, const char *dev, int argc,
> > +                          char **argv)
> > +{
> > +     struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> > +     int ret;
> > +
> > +     ret = dcb_app_get(dcb, dev, &tab);
> > +     if (ret != 0)
> > +             return ret;
> > +
> > +     dcb_app_table_sort(&tab);
> > +
> > +     open_json_object(NULL);
> > +
> > +     if (!argc) {
> > +             dcb_rewr_print(dcb, &tab);
> > +             goto out;
> > +     }
> > +
> > +     do {
> > +             if (strcmp(*argv, "help") == 0) {
> > +                     dcb_rewr_help_show_flush();
> > +                     goto out;
> > +             } else if (strcmp(*argv, "prio-pcp") == 0) {
> > +                     dcb_rewr_print_prio_pcp(dcb, &tab);
> > +             } else if (strcmp(*argv, "prio-dscp") == 0) {
> > +                     dcb_rewr_print_prio_dscp(dcb, &tab);
> > +             } else {
> > +                     fprintf(stderr, "What is \"%s\"?\n", *argv);
> > +                     dcb_rewr_help_show_flush();
> > +                     ret = -EINVAL;
> > +                     goto out;
> > +             }
> > +
> > +             NEXT_ARG_FWD();
> > +     } while (argc > 0);
> > +
> > +out:
> > +     close_json_object();
> > +     dcb_app_table_fini(&tab);
> > +     return ret;
> > +}
> > +
> > +static int dcb_cmd_rewr_flush(struct dcb *dcb, const char *dev, int argc,
> > +                           char **argv)
> > +{
> > +     struct dcb_app_table tab = { .attr = DCB_ATTR_DCB_REWR_TABLE };
> > +     int ret;
> > +
> > +     ret = dcb_app_get(dcb, dev, &tab);
> > +     if (ret != 0)
> > +             return ret;
> > +
> > +     if (!argc) {
> > +             ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
> > +                                   NULL);
> > +             goto out;
> > +     }
> > +
> > +     do {
> > +             if (strcmp(*argv, "help") == 0) {
> > +                     dcb_rewr_help_show_flush();
> > +                     goto out;
> > +             } else if (strcmp(*argv, "prio-pcp") == 0) {
> > +                     ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
> > +                                           &dcb_app_is_pcp);
> > +                     if (ret != 0)
> > +                             goto out;
> > +             } else if (strcmp(*argv, "prio-dscp") == 0) {
> > +                     ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
> > +                                           &dcb_app_is_dscp);
> > +                     if (ret != 0)
> > +                             goto out;
> > +             } else {
> > +                     fprintf(stderr, "What is \"%s\"?\n", *argv);
> > +                     dcb_rewr_help_show_flush();
> > +                     ret = -EINVAL;
> > +                     goto out;
> > +             }
> > +
> > +             NEXT_ARG_FWD();
> > +     } while (argc > 0);
> > +
> > +out:
> > +     dcb_app_table_fini(&tab);
> > +     return ret;
> > +}
> > +
> > +int dcb_cmd_rewr(struct dcb *dcb, int argc, char **argv)
> > +{
> > +     if (!argc || strcmp(*argv, "help") == 0) {
> > +             dcb_rewr_help();
> > +             return 0;
> > +     } else if (strcmp(*argv, "show") == 0) {
> > +             NEXT_ARG_FWD();
> > +             return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_show,
> > +                                      dcb_rewr_help_show_flush);
> > +     } else if (strcmp(*argv, "flush") == 0) {
> > +             NEXT_ARG_FWD();
> > +             return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_flush,
> > +                                      dcb_rewr_help_show_flush);
> > +     } else if (strcmp(*argv, "add") == 0) {
> > +             NEXT_ARG_FWD();
> > +             return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_add,
> > +                                      dcb_rewr_help_add);
> > +     } else if (strcmp(*argv, "del") == 0) {
> > +             NEXT_ARG_FWD();
> > +             return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_del,
> > +                                      dcb_rewr_help_add);
> > +     } else if (strcmp(*argv, "replace") == 0) {
> > +             NEXT_ARG_FWD();
> > +             return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_rewr_replace,
> > +                                      dcb_rewr_help_add);
> > +     } else {
> > +             fprintf(stderr, "What is \"%s\"?\n", *argv);
> > +             dcb_rewr_help();
> > +             return -EINVAL;
> > +     }
> > +}
> 

Re: [PATCH iproute2-next 9/9] man: dcb-app: clean up a few mistakes

[Daniel.Machon]

 > Daniel Machon <daniel.machon@microchip.com> writes:
> 
> > While referencing the dcb-app manpage, I spotted a few mistakes. Lets
> > fix them.
> >
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  man/man8/dcb-app.8 | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/man/man8/dcb-app.8 b/man/man8/dcb-app.8
> > index ecb38591168e..ebec67c90801 100644
> > --- a/man/man8/dcb-app.8
> > +++ b/man/man8/dcb-app.8
> > @@ -1,4 +1,4 @@
> > -.TH DCB-ETS 8 "6 December 2020" "iproute2" "Linux"
> > +.TH DCB-APP 8 "6 December 2020" "iproute2" "Linux"
> >  .SH NAME
> >  dcb-app \- show / manipulate application priority table of
> >  the DCB (Data Center Bridging) subsystem
> > @@ -26,7 +26,7 @@ the DCB (Data Center Bridging) subsystem
> >  .RB "[ " pcp-prio " ]"
> >
> >  .ti -8
> > -.B dcb ets " { " add " | " del " | " replace " } " dev
> > +.B dcb app " { " add " | " del " | " replace " } " dev
> >  .RI DEV
> >  .RB "[ " default-prio " " \fIPRIO-LIST\fB " ]"
> >  .RB "[ " ethtype-prio " " \fIET-MAP\fB " ]"
> > @@ -106,7 +106,7 @@ individual APP 3-tuples through
> >  .B add
> >  and
> >  .B del
> > -commands. On the other other hand, the command
> > +commands. On the other hand, the command
> 
> Heh, neat typo.

Liked that one too.

> 
> >  .B replace
> >  does what one would typically want in this situation--first adds the new
> >  configuration, and then removes the obsolete one, so that only one
> > @@ -184,7 +184,7 @@ for details. Keys are DSCP points, values are priorities assigned to
> >  traffic with matching DSCP. DSCP points can be written either directly as
> >  numeric values, or using symbolic names specified in
> >  .B /etc/iproute2/rt_dsfield
> > -(however note that that file specifies full 8-bit dsfield values, whereas
> > +(however note that file specifies full 8-bit dsfield values, whereas
> 
> Nope, this one's correct. The first that is a conjunction, the second
> one... a pronoun? Not sure.

Argh. You are right. Will change it here and in dcb-rewr.8 too.

> 
> Maybe make it "note that the file"? It's clear what file we are talking
> about and the grating "that that" goes away. Pretty sure it's a
> stylistic faux pas.

> 
> >  .B dcb app
> >  will only use the higher six bits).
> >  .B dcb app show
> > @@ -230,7 +230,7 @@ priority 4:
> >  .P
> >  # dcb app replace dev eth0 dscp-prio 24:4
> >  .br
> > -# dcb app show dev eth0 dscp-prio
> > +# dcb app -N show dev eth0 dscp-prio
> >  .br
> >  dscp-prio 0:0 24:4 48:6
> 

Re: [PATCH iproute2-next 1/9] dcb: app: expose dcb-app functions in new header

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

>> > -/* dcb_app.c */
>> > -
>> > -int dcb_cmd_app(struct dcb *dcb, int argc, char **argv);
>> > -enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector);
>> > -bool dcb_app_attr_type_validate(enum ieee_attrs_app type);
>> > -bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
>> > -
>> 
>> Why the move to a dedicated header? dcb.h ends up being the only client
>> and everybody consumes the prototypes through that file anyway. I don't
>> fine it necessary.
>
> I did try to rationalize that a bit in the commit description. I thought
> the amount of exposed app functions ended up polutting the dcb header.

I think it's not too bad. The dcb.c section of the header is similarly
large as the app section will be. Even with all the stuff that you
publish, the header is still, what, 150 lines maybe? I find that the
fragmentation isn't necessary and the current setup is just super
simple.

> Maybe it is not that bad - can move them back in the next v.

Re: [PATCH iproute2-next 3/9] dcb: app: modify dcb-app print functions for dcb-rewr reuse

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

>> Daniel Machon <daniel.machon@microchip.com> writes:
>> 
>> > -static void dcb_app_print_filtered(const struct dcb_app_table *tab,
>> > -                                bool (*filter)(const struct dcb_app *),
>> > -                                int (*print_key)(__u16 protocol),
>> > -                                const char *json_name,
>> > -                                const char *fp_name)
>> > +void dcb_app_print_filtered(const struct dcb_app_table *tab,
>> > +                         bool (*filter)(const struct dcb_app *),
>> > +                         int (*print_pid)(__u16 protocol),
>> > +                         const char *json_name, const char *fp_name)
>> >  {
>> >       bool first = true;
>> >       size_t i;
>> > @@ -439,8 +437,14 @@ static void dcb_app_print_filtered(const struct dcb_app_table *tab,
>> >               }
>> >
>> >               open_json_array(PRINT_JSON, NULL);
>> > -             print_key(app->protocol);
>> > -             print_uint(PRINT_ANY, NULL, "%d ", app->priority);
>> > +             if (tab->attr == DCB_ATTR_IEEE_APP_TABLE) {
>> > +                     print_pid(app->protocol);
>> > +                     print_uint(PRINT_ANY, NULL, ":%d", app->priority);
>> > +             } else {
>> > +                     print_uint(PRINT_ANY, NULL, "%d:", app->priority);
>> > +                     print_pid(app->protocol);
>> > +             }
>> 
>> I really dislike the attribute dispatch. I feels too much like mixing
>> abstraction layers. I think the callback should take a full struct
>> dcb_app pointer and format it as appropriate. Then you can model the
>> rewrite table differently from the app table by providing a callback
>> that invokes the print_ helpers in the correct order.
>> 
>> The app->protocol field as such is not really necessary IMHO, because
>> the function that invokes the helpers understands what kind of table it
>> is dealing with and could provide it as a parameter. But OK, I guess it
>> makes sense and probably saves some boilerplate parameterization.
>
> Roger. And actually, yeah, the callbacks are used heavily throughout
> DCB, so that fits better. Will incorporate CB approach in next v. I
> think this applies more or less to your comments in patch #3, #4 and #5
> too :)

Yeah, I wasn't sure myself how much of a pain the callback approach
brings, so wanted to make sure it's not bending-backwards bad. Hence all
that prototype code :)

Re: [PATCH iproute2-next 3/9] dcb: app: modify dcb-app print functions for dcb-rewr reuse

[Daniel Machon]

 > On Mon, 22 May 2023 20:41:06 +0200
> Daniel Machon <daniel.machon@microchip.com> wrote:
> 
> > +int dcb_app_print_pid_dec(__u16 protocol)
> >  {
> > -     return print_uint(PRINT_ANY, NULL, "%d:", protocol);
> > +     return print_uint(PRINT_ANY, NULL, "%d", protocol);
> >  }
> 
> Should be %u for unsigned value.

Thanks Stephen,
Will make sure to change that in v2.

/Daniel