[PATCH iproute2-next 0/2] Add pcp-prio and new apptrust subcommand

[PATCH iproute2-next 0/2] Add pcp-prio and new apptrust subcommand

[Daniel Machon]

This patch series makes use of the newly introduced [1] DCB_APP_SEL_PCP
selector, for PCP/DEI prioritization, and DCB_ATTR_IEEE_APP_TRUST
attribute for configuring per-selector trust and trust-order.

========================================================================
New parameter "pcp-prio" to existing "app" subcommand:
========================================================================

A new pcp-prio parameter has been added to the app subcommand, which can
be used to classify traffic based on PCP and DEI from the VLAN header.
PCP and DEI is specified in a combination of numerical and symbolic
form, where 'de' (as specified in the PCP Encoding Table, 802.1Q) means
DEI=1.

Map PCP 1 and DEI 0 to priority 1 $ dcb app add dev eth0 pcp-prio 1:1

Map PCP 1 and DEI 1 to priority 1 $ dcb app add dev eth0 pcp-prio 1de:1

In a hardware offloaded context, 'de' can be used by drivers, to map the
DEI bit directly to a drop-precedence.

========================================================================
New apptrust subcommand for configuring per-selector trust and trust
order:
========================================================================

This new command currently has a single parameter, which lets you
specify an ordered list of trusted selectors. The microchip sparx5
driver is already enabled to offload said list of trusted selectors. The
new command has been given the name apptrust, to indicate that the trust
covers APP table selectors only. I found that 'apptrust' was better than
plain 'trust' as the latter does not indicate the scope of what is to be
trusted.

Example:

Trust selectors dscp and pcp, in that order: $ dcb apptrust set dev eth0
order dscp pcp

Trust selectors ethertype, stream and pcp, in that order $ dcb apptrust
set dev eth0 order eth stream pcp

Show the trust order $ dcb apptrust show dev eth0 order trust-order: eth
stream pcp

A concern was raised here [2], that 'apptrust' would not work well with
matches(), so instead strcmp() has been used to match for the new
subcommand, as suggested here [3]. Same goes with pcp-prio parameter for
dcb app.

The man page for dcb_app has been extended to cover the new pcp-prio
parameter, and a new man page for dcb_apptrust has been created.

[1] https://lore.kernel.org/netdev/20221101094834.2726202-1-daniel.machon@microchip.com/
[2] https://lore.kernel.org/netdev/20220909080631.6941a770@hermes.local/
[3] https://lore.kernel.org/netdev/Y0fP+9C0tE7P2xyK@shredder/

Daniel Machon (2):
  dcb: add new pcp-prio parameter to dcb app
  dcb: add new subcommand for apptrust

 dcb/Makefile            |   3 +-
 dcb/dcb.c               |   4 +-
 dcb/dcb.h               |   7 +
 dcb/dcb_app.c           | 138 ++++++++++++++++++-
 dcb/dcb_apptrust.c      | 291 ++++++++++++++++++++++++++++++++++++++++
 man/man8/dcb-app.8      |  27 ++++
 man/man8/dcb-apptrust.8 | 118 ++++++++++++++++
 7 files changed, 580 insertions(+), 8 deletions(-)
 create mode 100644 dcb/dcb_apptrust.c
 create mode 100644 man/man8/dcb-apptrust.8

-- 
2.34.1

[PATCH iproute2-next 1/2] dcb: add new pcp-prio parameter to dcb app

[Daniel Machon]

Add new pcp-prio parameter to the app subcommand, which can be used to
classify traffic based on PCP and DEI from the VLAN header. PCP and DEI
is specified in a combination of numerical and symbolic form, where 'de'
(as specified in the PCP Encoding Table, 802.1Q) means DEI=1.

Map PCP 1 and DEI 0 to priority 1 $ dcb app add dev eth0 pcp-prio 1:1

Map PCP 1 and DEI 1 to priority 1 $ dcb app add dev eth0 pcp-prio 1de:1

Internally, PCP and DEI is encoded in the protocol field of the dcb_app
struct. Each combination of PCP and DEI maps to a priority, thus needing
a range of  0-15. A well formed dcb_app entry for PCP/DEI
prioritization, could look like:

    struct dcb_app pcp = {
        .selector = DCB_APP_SEL_PCP,
	.priority = 7,
        .protocol = 15
    }

For mapping PCP=7 and DEI=1 to Prio=7.

Also, two helper functions for translating between std and non-std APP
selectors, have been added to dcb_app.c and exposed through dcb.h.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/dcb.h          |   3 +
 dcb/dcb_app.c      | 138 +++++++++++++++++++++++++++++++++++++++++++--
 man/man8/dcb-app.8 |  27 +++++++++
 3 files changed, 162 insertions(+), 6 deletions(-)

diff --git a/dcb/dcb.h b/dcb/dcb.h
index 244c3d3c30e3..05eddcbbcfdf 100644
--- a/dcb/dcb.h
+++ b/dcb/dcb.h
@@ -57,6 +57,9 @@ void dcb_print_array_kw(const __u8 *array, size_t array_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_buffer.c */
 
diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
index dad34554017a..0d4a82e1e502 100644
--- a/dcb/dcb_app.c
+++ b/dcb/dcb_app.c
@@ -10,6 +10,16 @@
 #include "utils.h"
 #include "rt_names.h"
 
+static const char *const pcp_names[16] = {
+	"0",   "1",   "2",   "3",   "4",   "5",   "6",   "7",
+	"0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de"
+};
+
+static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = {
+	[DCB_ATTR_IEEE_APP] = "DCB_ATTR_IEEE_APP",
+	[DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP"
+};
+
 static void dcb_app_help_add(void)
 {
 	fprintf(stderr,
@@ -20,11 +30,13 @@ static void dcb_app_help_add(void)
 		"           [ dgram-port-prio PORT:PRIO ]\n"
 		"           [ port-prio PORT:PRIO ]\n"
 		"           [ dscp-prio INTEGER:PRIO ]\n"
+		"           [ pcp-prio INTEGER:PRIO ]\n"
 		"\n"
 		" where PRIO := { 0 .. 7 }\n"
 		"       ET := { 0x600 .. 0xffff }\n"
 		"       PORT := { 1 .. 65535 }\n"
 		"       DSCP := { 0 .. 63 }\n"
+		"       PCP := { 0(de) .. 7(de) }\n"
 		"\n"
 	);
 }
@@ -39,6 +51,7 @@ static void dcb_app_help_show_flush(void)
 		"           [ dgram-port-prio ]\n"
 		"           [ port-prio ]\n"
 		"           [ dscp-prio ]\n"
+		"           [ pcp-prio ]\n"
 		"\n"
 	);
 }
@@ -58,6 +71,38 @@ struct dcb_app_table {
 	size_t n_apps;
 };
 
+enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector)
+{
+	switch (selector) {
+	case IEEE_8021QAZ_APP_SEL_ETHERTYPE:
+	case IEEE_8021QAZ_APP_SEL_STREAM:
+	case IEEE_8021QAZ_APP_SEL_DGRAM:
+	case IEEE_8021QAZ_APP_SEL_ANY:
+	case IEEE_8021QAZ_APP_SEL_DSCP:
+		return DCB_ATTR_IEEE_APP;
+	case DCB_APP_SEL_PCP:
+		return DCB_ATTR_DCB_APP;
+	default:
+		return DCB_ATTR_IEEE_APP_UNSPEC;
+	}
+}
+
+bool dcb_app_attr_type_validate(enum ieee_attrs_app type)
+{
+	switch (type) {
+	case DCB_ATTR_IEEE_APP:
+	case DCB_ATTR_DCB_APP:
+		return true;
+	default:
+		return false;
+	}
+}
+
+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)
 {
 	free(tab->apps);
@@ -213,6 +258,32 @@ 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 ret, res;
+
+	res = parse_one_of("pcp-names", arg, pcp_names,
+			   ARRAY_SIZE(pcp_names), &ret);
+	if (ret < 0)
+		return ret;
+
+	*key = res;
+
+	return 0;
+}
+
+static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data)
+{
+	__u8 prio;
+
+	if (get_u8(&prio, value, 0))
+		return -EINVAL;
+
+	return dcb_parse_mapping("PCP", key, 15,
+				 "PRIO", prio, IEEE_8021QAZ_MAX_TCS - 1,
+				 dcb_app_parse_mapping_cb, data);
+}
+
 static int dcb_app_parse_dscp(__u32 *key, const char *arg)
 {
 	if (parse_mapping_num_all(key, arg) == 0)
@@ -309,6 +380,11 @@ static 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)
+{
+	return app->selector == DCB_APP_SEL_PCP;
+}
+
 static bool dcb_app_is_stream_port(const struct dcb_app *app)
 {
 	return app->selector == IEEE_8021QAZ_APP_SEL_STREAM;
@@ -344,6 +420,17 @@ 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)
+{
+	/* Print in numerical form, if protocol value is out-of-range */
+	if (protocol > 15) {
+		fprintf(stderr, "Unknown PCP key: %d\n", protocol);
+		return print_uint(PRINT_ANY, NULL, "%d:", 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),
@@ -382,6 +469,15 @@ static void dcb_app_print_ethtype_prio(const struct dcb_app_table *tab)
 			       "ethtype_prio", "ethtype-prio");
 }
 
+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,
+			       "pcp_prio", "pcp-prio");
+}
+
 static void dcb_app_print_dscp_prio(const struct dcb *dcb,
 				    const struct dcb_app_table *tab)
 {
@@ -439,26 +535,41 @@ static void dcb_app_print(const struct dcb *dcb, const struct dcb_app_table *tab
 	dcb_app_print_stream_port_prio(tab);
 	dcb_app_print_dgram_port_prio(tab);
 	dcb_app_print_port_prio(tab);
+	dcb_app_print_pcp_prio(dcb, tab);
 }
 
 static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data)
 {
 	struct dcb_app_table *tab = data;
 	struct dcb_app *app;
+	uint16_t type;
 	int ret;
 
-	if (mnl_attr_get_type(attr) != DCB_ATTR_IEEE_APP) {
-		fprintf(stderr, "Unknown attribute in DCB_ATTR_IEEE_APP_TABLE: %d\n",
-			mnl_attr_get_type(attr));
+	type = mnl_attr_get_type(attr);
+
+	if (!dcb_app_attr_type_validate(type)) {
+		fprintf(stderr,
+			"Unknown attribute in DCB_ATTR_IEEE_APP_TABLE: %d\n",
+			type);
 		return MNL_CB_OK;
 	}
 	if (mnl_attr_get_payload_len(attr) < sizeof(struct dcb_app)) {
-		fprintf(stderr, "DCB_ATTR_IEEE_APP payload expected to have size %zd, not %d\n",
-			sizeof(struct dcb_app), mnl_attr_get_payload_len(attr));
+		fprintf(stderr,
+			"%s payload expected to have size %zd, not %d\n",
+			ieee_attrs_app_names[type], sizeof(struct dcb_app),
+			mnl_attr_get_payload_len(attr));
 		return MNL_CB_OK;
 	}
 
 	app = mnl_attr_get_payload(attr);
+
+	/* Check that selector is encapsulated in the right attribute */
+	if (!dcb_app_selector_validate(type, app->selector)) {
+		fprintf(stderr, "Wrong selector for type: %s\n",
+			ieee_attrs_app_names[type]);
+		return MNL_CB_OK;
+	}
+
 	ret = dcb_app_table_push(tab, app);
 	if (ret != 0)
 		return MNL_CB_ERROR;
@@ -491,6 +602,7 @@ struct dcb_app_add_del {
 static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
 {
 	struct dcb_app_add_del *add_del = data;
+	enum ieee_attrs_app type;
 	struct nlattr *nest;
 	size_t i;
 
@@ -498,9 +610,10 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
 
 	for (i = 0; i < add_del->tab->n_apps; i++) {
 		const struct dcb_app *app = &add_del->tab->apps[i];
+		type = dcb_app_attr_type_get(app->selector);
 
 		if (add_del->filter == NULL || add_del->filter(app))
-			mnl_attr_put(nlh, DCB_ATTR_IEEE_APP, sizeof(*app), app);
+			mnl_attr_put(nlh, type, sizeof(*app), app);
 	}
 
 	mnl_attr_nest_end(nlh, nest);
@@ -577,6 +690,12 @@ static int dcb_cmd_app_parse_add_del(struct dcb *dcb, const char *dev,
 			ret = parse_mapping(&argc, &argv, false,
 					    &dcb_app_parse_mapping_port_prio,
 					    &pm);
+		} else if (strcmp(*argv, "pcp-prio") == 0) {
+			NEXT_ARG();
+			pm.selector = DCB_APP_SEL_PCP;
+			ret = parse_mapping_gen(&argc, &argv, &dcb_app_parse_pcp,
+						&dcb_app_parse_mapping_pcp_prio,
+						&pm);
 		} else {
 			fprintf(stderr, "What is \"%s\"?\n", *argv);
 			dcb_app_help_add();
@@ -656,6 +775,8 @@ static int dcb_cmd_app_show(struct dcb *dcb, const char *dev, int argc, char **a
 			dcb_app_print_port_prio(&tab);
 		} else if (matches(*argv, "default-prio") == 0) {
 			dcb_app_print_default_prio(&tab);
+		} else if (strcmp(*argv, "pcp-prio") == 0) {
+			dcb_app_print_pcp_prio(dcb, &tab);
 		} else {
 			fprintf(stderr, "What is \"%s\"?\n", *argv);
 			dcb_app_help_show_flush();
@@ -705,6 +826,11 @@ static int dcb_cmd_app_flush(struct dcb *dcb, const char *dev, int argc, char **
 					      &dcb_app_is_dscp);
 			if (ret != 0)
 				goto out;
+		} else if (strcmp(*argv, "pcp-prio") == 0) {
+			ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
+					      &dcb_app_is_pcp);
+			if (ret != 0)
+				goto out;
 		} else {
 			fprintf(stderr, "What is \"%s\"?\n", *argv);
 			dcb_app_help_show_flush();
diff --git a/man/man8/dcb-app.8 b/man/man8/dcb-app.8
index 9780fe4b60fa..054ba9801d81 100644
--- a/man/man8/dcb-app.8
+++ b/man/man8/dcb-app.8
@@ -23,6 +23,7 @@ the DCB (Data Center Bridging) subsystem
 .RB "[ " dgram-port-prio " ]"
 .RB "[ " port-prio " ]"
 .RB "[ " dscp-prio " ]"
+.RB "[ " pcp-prio " ]"
 
 .ti -8
 .B dcb ets " { " add " | " del " | " replace " } " dev
@@ -33,6 +34,7 @@ the DCB (Data Center Bridging) subsystem
 .RB "[ " dgram-port-prio " " \fIPORT-MAP\fB " ]"
 .RB "[ " port-prio " " \fIPORT-MAP\fB " ]"
 .RB "[ " dscp-prio " " \fIDSCP-MAP\fB " ]"
+.RB "[ " pcp-prio " " \fIPCP-MAP\fB " ]"
 
 .ti -8
 .IR PRIO-LIST " := [ " PRIO-LIST " ] " PRIO
@@ -64,6 +66,9 @@ the DCB (Data Center Bridging) subsystem
 .ti -8
 .IR DSCP " := { " \fB0\fR " .. " \fB63\fR " }"
 
+.ti -8
+.IR PCP " := { " \fB0\fR " .. " \fB7\fR " }"
+
 .ti -8
 .IR PRIO " := { " \fB0\fR " .. " \fB7\fR " }"
 
@@ -182,6 +187,18 @@ command line option
 .B -N
 turns the show translation off.
 
+.TP
+.B pcp-prio \fIPCP-MAP
+\fIPCP-MAP\fR uses the array parameter syntax, see
+.BR dcb (8)
+for details. Keys are PCP/DEI values. Values are priorities assigned to traffic
+with matching PCP and DEI. PCP/DEI values are written as a combination of
+numeric- and symbolic values, to accommodate for both PCP and DEI. PCP always
+in numerical form e.g 1 .. 7 and DEI in symbolic form e.g 'de', indicating that
+the DEI bit is 1.  In combination 2de:1 translates to a mapping of PCP=2 and
+DEI=1 to priority 1. In a hardware offloaded context, the DEI bit can be mapped
+directly to drop-precedence (DP) by the driver.
+
 .SH EXAMPLE & USAGE
 
 Prioritize traffic with DSCP 0 to priority 0, 24 to 3 and 48 to 6:
@@ -221,6 +238,16 @@ Flush all DSCP rules:
 .br
 (nothing)
 
+Add a rule to map traffic with PCP 1 and DEI 0 to priority 1 and PCP 2 and DEI 1
+to priority 2:
+
+.P
+# dcb app add dev eth0 pcp-prio 1:1 2de:2
+.br
+# dcb app show dev eth0 pcp-prio
+.br
+pcp-prio 1:1 2de:2
+
 .SH EXIT STATUS
 Exit status is 0 if command was successful or a positive integer upon failure.
 
-- 
2.34.1

[PATCH iproute2-next 2/2] dcb: add new subcommand for apptrust

[Daniel Machon]

Add new apptrust subcommand for the dcbnl apptrust extension object.

The apptrust command lets you specify a consecutive ordered list of
trusted selectors, which can be used by drivers to determine which
selectors are eligible (trusted) for packet prioritization, and in which
order.

Selectors are sent in a new nested attribute:
DCB_ATTR_IEEE_APP_TRUST_TABLE.  The nest contains trusted selectors
encapsulated in either DCB_ATTR_IEEE_APP or DCB_ATTR_DCB_APP attributes,
for standard and non-standard selectors, respectively.

Example:

Trust selectors dscp and pcp, in that order
$ dcb apptrust set dev eth0 order dscp pcp

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/Makefile            |   3 +-
 dcb/dcb.c               |   4 +-
 dcb/dcb.h               |   4 +
 dcb/dcb_apptrust.c      | 291 ++++++++++++++++++++++++++++++++++++++++
 man/man8/dcb-apptrust.8 | 118 ++++++++++++++++
 5 files changed, 418 insertions(+), 2 deletions(-)
 create mode 100644 dcb/dcb_apptrust.c
 create mode 100644 man/man8/dcb-apptrust.8

diff --git a/dcb/Makefile b/dcb/Makefile
index ca65d4670c80..dd41a559a0c8 100644
--- a/dcb/Makefile
+++ b/dcb/Makefile
@@ -7,7 +7,8 @@ DCBOBJ = dcb.o \
          dcb_dcbx.o \
          dcb_ets.o \
          dcb_maxrate.o \
-         dcb_pfc.o
+         dcb_pfc.o \
+         dcb_apptrust.o
 TARGETS += dcb
 LDLIBS += -lm
 
diff --git a/dcb/dcb.c b/dcb/dcb.c
index 391fd95455f4..3ffa91d64d0d 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 | buffer | dcbx | ets | maxrate | pfc }\n"
+		"where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc }\n"
 		"       OPTIONS := [ -V | --Version | -i | --iec | -j | --json\n"
 		"                  | -N | --Numeric | -p | --pretty\n"
 		"                  | -s | --statistics | -v | --verbose]\n");
@@ -483,6 +483,8 @@ static int dcb_cmd(struct dcb *dcb, int argc, char **argv)
 		return 0;
 	} else if (matches(*argv, "app") == 0) {
 		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 (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 05eddcbbcfdf..d40664f29dad 100644
--- a/dcb/dcb.h
+++ b/dcb/dcb.h
@@ -61,6 +61,10 @@ 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);
+
 /* dcb_buffer.c */
 
 int dcb_cmd_buffer(struct dcb *dcb, int argc, char **argv);
diff --git a/dcb/dcb_apptrust.c b/dcb/dcb_apptrust.c
new file mode 100644
index 000000000000..14d18dcb7f83
--- /dev/null
+++ b/dcb/dcb_apptrust.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <errno.h>
+#include <linux/dcbnl.h>
+
+#include "dcb.h"
+#include "utils.h"
+
+static void dcb_apptrust_help_set(void)
+{
+	fprintf(stderr,
+		"Usage: dcb apptrust set dev STRING\n"
+		"	[ order [ eth | stream | dgram | any | dscp | pcp ] ]\n"
+		"\n");
+}
+
+static void dcb_apptrust_help_show(void)
+{
+	fprintf(stderr, "Usage: dcb [ -i ] apptrust show dev STRING\n"
+			"           [ order ]\n"
+			"\n");
+}
+
+static void dcb_apptrust_help(void)
+{
+	fprintf(stderr, "Usage: dcb apptrust help\n"
+			"\n");
+	dcb_apptrust_help_show();
+	dcb_apptrust_help_set();
+}
+
+static const char *const selector_names[] = {
+	[IEEE_8021QAZ_APP_SEL_ETHERTYPE] = "eth",
+	[IEEE_8021QAZ_APP_SEL_STREAM]    = "stream",
+	[IEEE_8021QAZ_APP_SEL_DGRAM]     = "dgram",
+	[IEEE_8021QAZ_APP_SEL_ANY]       = "any",
+	[IEEE_8021QAZ_APP_SEL_DSCP]      = "dscp",
+	[DCB_APP_SEL_PCP]                = "pcp",
+};
+
+struct dcb_apptrust_table {
+	__u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1];
+	int nselectors;
+};
+
+static bool dcb_apptrust_contains(const struct dcb_apptrust_table *table,
+				  __u8 selector)
+{
+	int i;
+
+	for (i = 0; i < table->nselectors; i++)
+		if (table->selectors[i] == selector)
+			return true;
+
+	return false;
+}
+
+static void dcb_apptrust_print(const struct dcb_apptrust_table *table)
+{
+	const char *str;
+	__u8 selector;
+	int i;
+
+	open_json_array(PRINT_JSON, "order");
+	print_string(PRINT_FP, NULL, "order: ", NULL);
+
+	for (i = 0; i < table->nselectors; i++) {
+		selector = table->selectors[i];
+		str = selector_names[selector];
+		print_string(PRINT_ANY, NULL, "%s ", str);
+	}
+	print_nl();
+
+	close_json_array(PRINT_JSON, "order");
+}
+
+static int dcb_apptrust_get_cb(const struct nlattr *attr, void *data)
+{
+	struct dcb_apptrust_table *table = data;
+	uint16_t type;
+	__u8 selector;
+
+	type = mnl_attr_get_type(attr);
+
+	if (!dcb_app_attr_type_validate(type)) {
+		fprintf(stderr,
+			"Unknown attribute in DCB_ATTR_IEEE_APP_TRUST_TABLE: %d\n",
+			type);
+		return MNL_CB_OK;
+	}
+
+	if (mnl_attr_get_payload_len(attr) < 1) {
+		fprintf(stderr,
+			"DCB_ATTR_IEEE_APP_TRUST payload expected to have size %zd, not %d\n",
+			sizeof(struct dcb_app), mnl_attr_get_payload_len(attr));
+		return MNL_CB_OK;
+	}
+
+	selector = mnl_attr_get_u8(attr);
+
+	/* Check that selector is encapsulated in the right attribute */
+	if (!dcb_app_selector_validate(type, selector)) {
+		fprintf(stderr, "Wrong type for selector: %s\n",
+			selector_names[selector]);
+		return MNL_CB_OK;
+	}
+
+	table->selectors[table->nselectors++] = selector;
+
+	return MNL_CB_OK;
+}
+
+static int dcb_apptrust_get(struct dcb *dcb, const char *dev,
+			    struct dcb_apptrust_table *table)
+{
+	uint16_t payload_len;
+	void *payload;
+	int ret;
+
+	ret = dcb_get_attribute_va(dcb, dev, DCB_ATTR_DCB_APP_TRUST_TABLE,
+				   &payload, &payload_len);
+	if (ret != 0)
+		return ret;
+
+	ret = mnl_attr_parse_payload(payload, payload_len, dcb_apptrust_get_cb,
+				     table);
+	if (ret != MNL_CB_OK)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int dcb_apptrust_set_cb(struct dcb *dcb, struct nlmsghdr *nlh,
+			       void *data)
+{
+	const struct dcb_apptrust_table *table = data;
+	enum ieee_attrs_app type;
+	struct nlattr *nest;
+	int i;
+
+	nest = mnl_attr_nest_start(nlh, DCB_ATTR_DCB_APP_TRUST_TABLE);
+
+	for (i = 0; i < table->nselectors; i++) {
+		type = dcb_app_attr_type_get(table->selectors[i]);
+		mnl_attr_put_u8(nlh, type, table->selectors[i]);
+	}
+
+	mnl_attr_nest_end(nlh, nest);
+
+	return 0;
+}
+
+static int dcb_apptrust_set(struct dcb *dcb, const char *dev,
+			    const struct dcb_apptrust_table *table)
+{
+	return dcb_set_attribute_va(dcb, DCB_CMD_IEEE_SET, dev,
+				    &dcb_apptrust_set_cb, (void *)table);
+}
+
+static int dcb_apptrust_parse_selector_list(int *argcp, char ***argvp,
+					    struct dcb_apptrust_table *table)
+{
+	char **argv = *argvp;
+	int argc = *argcp;
+	__u8 selector;
+	int ret;
+
+	NEXT_ARG_FWD();
+
+	/* No trusted selectors ? */
+	if (argc == 0)
+		goto out;
+
+	while (argc > 0) {
+		selector = parse_one_of("order", *argv, selector_names,
+					ARRAY_SIZE(selector_names), &ret);
+		if (ret < 0)
+			return -EINVAL;
+
+		if (table->nselectors > IEEE_8021QAZ_APP_SEL_MAX)
+			return -ERANGE;
+
+		if (dcb_apptrust_contains(table, selector)) {
+			fprintf(stderr, "Duplicate selector: %s\n",
+				selector_names[selector]);
+			return -EINVAL;
+		}
+
+		table->selectors[table->nselectors++] = selector;
+
+		NEXT_ARG_FWD();
+	}
+
+out:
+	*argcp = argc;
+	*argvp = argv;
+
+	return 0;
+}
+
+static int dcb_cmd_apptrust_set(struct dcb *dcb, const char *dev, int argc,
+				char **argv)
+{
+	struct dcb_apptrust_table table = { 0 };
+	int ret;
+
+	if (!argc) {
+		dcb_apptrust_help_set();
+		return 0;
+	}
+
+	do {
+		if (strcmp(*argv, "help") == 0) {
+			dcb_apptrust_help_set();
+			return 0;
+		} else if (strcmp(*argv, "order") == 0) {
+			ret = dcb_apptrust_parse_selector_list(&argc, &argv,
+							       &table);
+			if (ret < 0) {
+				fprintf(stderr, "Invalid list of selectors\n");
+				return -EINVAL;
+			}
+			continue;
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			dcb_apptrust_help_set();
+			return -EINVAL;
+		}
+
+		NEXT_ARG_FWD();
+	} while (argc > 0);
+
+	return dcb_apptrust_set(dcb, dev, &table);
+}
+
+static int dcb_cmd_apptrust_show(struct dcb *dcb, const char *dev, int argc,
+				 char **argv)
+{
+	struct dcb_apptrust_table table = { 0 };
+	int ret;
+
+	ret = dcb_apptrust_get(dcb, dev, &table);
+	if (ret)
+		return ret;
+
+	open_json_object(NULL);
+
+	if (!argc) {
+		dcb_apptrust_help();
+		goto out;
+	}
+
+	do {
+		if (strcmp(*argv, "help") == 0) {
+			dcb_apptrust_help_show();
+			return 0;
+		} else if (strcmp(*argv, "order") == 0) {
+			dcb_apptrust_print(&table);
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			dcb_apptrust_help_show();
+			return -EINVAL;
+		}
+
+		NEXT_ARG_FWD();
+	} while (argc > 0);
+
+out:
+	close_json_object();
+	return 0;
+}
+
+int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv)
+{
+	if (!argc || strcmp(*argv, "help") == 0) {
+		dcb_apptrust_help();
+		return 0;
+	} else if (strcmp(*argv, "show") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_apptrust_show,
+					 dcb_apptrust_help_show);
+	} else if (strcmp(*argv, "set") == 0) {
+		NEXT_ARG_FWD();
+		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_apptrust_set,
+					 dcb_apptrust_help_set);
+	} else {
+		fprintf(stderr, "What is \"%s\"?\n", *argv);
+		dcb_apptrust_help();
+		return -EINVAL;
+	}
+}
diff --git a/man/man8/dcb-apptrust.8 b/man/man8/dcb-apptrust.8
new file mode 100644
index 000000000000..9ebe7c17292c
--- /dev/null
+++ b/man/man8/dcb-apptrust.8
@@ -0,0 +1,118 @@
+.TH DCB-APPTRUST 8 "22 November 2022" "iproute2" "Linux"
+.SH NAME
+dcb-apptrust \- show / manipulate per-selector trust and trust order of the application
+priority table of the DCB (Data Center Bridging) subsystem.
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+
+.ti -8
+.B dcb
+.RI "[ " OPTIONS " ] "
+.B apptrust
+.RI "{ " COMMAND " | " help " }"
+.sp
+
+.ti -8
+.B dcb apptrust show dev order
+.RI DEV
+
+.ti -8
+.B dcb apptrust set dev order
+.RI DEV
+.RB "[ " eth " ]"
+.RB "[ " stream " ]"
+.RB "[ " dgram " ]"
+.RB "[ " any " ]"
+.RB "[ " dscp " ]"
+.RB "[ " pcp " ]"
+
+.SH DESCRIPTION
+
+.B dcb apptrust
+is used to configure and manipulate per-selector trust and trust order of the
+Application Priority Table, see
+.BR dcb-app (8)
+for details on how to configure app table entries.
+
+Selector trust can be used by the
+software stack, or drivers (most likely the latter), when querying the APP
+table, to determine if an APP entry should take effect, or not. Additionaly, the
+order of the trusted selectors will dictate which selector should take
+precedence, in the case of multiple different APP selectors being present in the
+APP table.
+
+.SH COMMANDS
+
+.TP
+.B show
+Display all trusted selectors.
+
+.TP
+.B set
+Set new list of trusted selectors. Empty list is effectively the same as
+removing trust entirely.
+
+.SH PARAMETERS
+
+The following describes only the write direction, i.e. as used with the
+\fBset\fR command. For the \fBshow\fR command, the parameter name is to be used
+as a simple keyword without further arguments. This instructs the tool to show
+the values of a given parameter.
+
+.TP
+.B order \fISELECTOR-NAMES
+\fISELECTOR-NAMES\fR is a space-separated list of selector names:\fR
+
+.B eth
+Trust EtherType.
+
+.B stream
+Trust TCP, or Stream Control Transmission Protocol (SCTP).
+
+.B dgram
+Trust UDP, or Datagram Congestion Control Protocol (DCCP).
+
+.B any
+Trust TCP, SCTP, UDP, or DCCP.
+
+.B dscp
+Trust Differentiated Services Code Point (DSCP) values.
+
+.B pcp
+Trust Priority Code Point/Drop Eligible Indicator (PCP/DEI).
+
+
+.SH EXAMPLE & USAGE
+
+Set trust order to: dscp, pcp for eth0:
+.P
+# dcb apptrust set dev eth0 order dscp pcp
+
+Set trust order to: any (stream or dgram), pcp, eth for eth1:
+.P
+# dcb apptrust set dev eth1 order any pcp eth
+
+Show what was set:
+
+.P
+# dcb apptrust show dev eth0
+.br
+order: any pcp eth
+
+.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)
+
+.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

Re: [PATCH iproute2-next 1/2] dcb: add new pcp-prio parameter to dcb app

[Petr Machata]

This looks good to me overall, I just have a few nits.

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

> Add new pcp-prio parameter to the app subcommand, which can be used to
> classify traffic based on PCP and DEI from the VLAN header. PCP and DEI
> is specified in a combination of numerical and symbolic form, where 'de'
> (as specified in the PCP Encoding Table, 802.1Q) means DEI=1.
>
> Map PCP 1 and DEI 0 to priority 1 $ dcb app add dev eth0 pcp-prio 1:1
>
> Map PCP 1 and DEI 1 to priority 1 $ dcb app add dev eth0 pcp-prio 1de:1
>
> Internally, PCP and DEI is encoded in the protocol field of the dcb_app
> struct. Each combination of PCP and DEI maps to a priority, thus needing
> a range of  0-15. A well formed dcb_app entry for PCP/DEI
> prioritization, could look like:
>
>     struct dcb_app pcp = {
>         .selector = DCB_APP_SEL_PCP,
> 	.priority = 7,
>         .protocol = 15
>     }
>
> For mapping PCP=7 and DEI=1 to Prio=7.
>
> Also, two helper functions for translating between std and non-std APP
> selectors, have been added to dcb_app.c and exposed through dcb.h.

Could you include an example or two of how PCP is configured? E.g. the
following was part of the dcb-app submission:

    # dcb app add dev eni1np1 dscp-prio 0:0 CS3:3 CS6:6
    # dcb app show dev eni1np1
    dscp-prio 0:0 CS3:3 CS6:6


> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/dcb.h          |   3 +
>  dcb/dcb_app.c      | 138 +++++++++++++++++++++++++++++++++++++++++++--
>  man/man8/dcb-app.8 |  27 +++++++++
>  3 files changed, 162 insertions(+), 6 deletions(-)
>
> diff --git a/dcb/dcb.h b/dcb/dcb.h
> index 244c3d3c30e3..05eddcbbcfdf 100644
> --- a/dcb/dcb.h
> +++ b/dcb/dcb.h
> @@ -57,6 +57,9 @@ void dcb_print_array_kw(const __u8 *array, size_t array_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_buffer.c */
>  
> diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c
> index dad34554017a..0d4a82e1e502 100644
> --- a/dcb/dcb_app.c
> +++ b/dcb/dcb_app.c
> @@ -10,6 +10,16 @@
>  #include "utils.h"
>  #include "rt_names.h"
>  
> +static const char *const pcp_names[16] = {
> +	"0",   "1",   "2",   "3",   "4",   "5",   "6",   "7",
> +	"0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de"
> +};
> +
> +static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = {
> +	[DCB_ATTR_IEEE_APP] = "DCB_ATTR_IEEE_APP",
> +	[DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP"
> +};
> +
>  static void dcb_app_help_add(void)
>  {
>  	fprintf(stderr,
> @@ -20,11 +30,13 @@ static void dcb_app_help_add(void)
>  		"           [ dgram-port-prio PORT:PRIO ]\n"
>  		"           [ port-prio PORT:PRIO ]\n"
>  		"           [ dscp-prio INTEGER:PRIO ]\n"
> +		"           [ pcp-prio INTEGER:PRIO ]\n"

This should say PCP:PRIO, not INTEGER:PRIO.

>  		"\n"
>  		" where PRIO := { 0 .. 7 }\n"
>  		"       ET := { 0x600 .. 0xffff }\n"
>  		"       PORT := { 1 .. 65535 }\n"
>  		"       DSCP := { 0 .. 63 }\n"
> +		"       PCP := { 0(de) .. 7(de) }\n"
>  		"\n"
>  	);
>  }
> @@ -39,6 +51,7 @@ static void dcb_app_help_show_flush(void)
>  		"           [ dgram-port-prio ]\n"
>  		"           [ port-prio ]\n"
>  		"           [ dscp-prio ]\n"
> +		"           [ pcp-prio ]\n"
>  		"\n"
>  	);
>  }
> @@ -58,6 +71,38 @@ struct dcb_app_table {
>  	size_t n_apps;
>  };
>  
> +enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector)
> +{
> +	switch (selector) {
> +	case IEEE_8021QAZ_APP_SEL_ETHERTYPE:
> +	case IEEE_8021QAZ_APP_SEL_STREAM:
> +	case IEEE_8021QAZ_APP_SEL_DGRAM:
> +	case IEEE_8021QAZ_APP_SEL_ANY:
> +	case IEEE_8021QAZ_APP_SEL_DSCP:
> +		return DCB_ATTR_IEEE_APP;
> +	case DCB_APP_SEL_PCP:
> +		return DCB_ATTR_DCB_APP;
> +	default:
> +		return DCB_ATTR_IEEE_APP_UNSPEC;
> +	}
> +}
> +
> +bool dcb_app_attr_type_validate(enum ieee_attrs_app type)
> +{
> +	switch (type) {
> +	case DCB_ATTR_IEEE_APP:
> +	case DCB_ATTR_DCB_APP:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +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)
>  {
>  	free(tab->apps);
> @@ -213,6 +258,32 @@ 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 ret, res;
> +
> +	res = parse_one_of("pcp-names", arg, pcp_names,
> +			   ARRAY_SIZE(pcp_names), &ret);
> +	if (ret < 0)
> +		return ret;
> +
> +	*key = res;
> +
> +	return 0;
> +}
> +
> +static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data)
> +{
> +	__u8 prio;
> +
> +	if (get_u8(&prio, value, 0))
> +		return -EINVAL;
> +
> +	return dcb_parse_mapping("PCP", key, 15,
> +				 "PRIO", prio, IEEE_8021QAZ_MAX_TCS - 1,
> +				 dcb_app_parse_mapping_cb, data);
> +}
> +
>  static int dcb_app_parse_dscp(__u32 *key, const char *arg)
>  {
>  	if (parse_mapping_num_all(key, arg) == 0)
> @@ -309,6 +380,11 @@ static 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)
> +{
> +	return app->selector == DCB_APP_SEL_PCP;
> +}
> +
>  static bool dcb_app_is_stream_port(const struct dcb_app *app)
>  {
>  	return app->selector == IEEE_8021QAZ_APP_SEL_STREAM;
> @@ -344,6 +420,17 @@ 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)
> +{
> +	/* Print in numerical form, if protocol value is out-of-range */
> +	if (protocol > 15) {
> +		fprintf(stderr, "Unknown PCP key: %d\n", protocol);
> +		return print_uint(PRINT_ANY, NULL, "%d:", 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),
> @@ -382,6 +469,15 @@ static void dcb_app_print_ethtype_prio(const struct dcb_app_table *tab)
>  			       "ethtype_prio", "ethtype-prio");
>  }
>  
> +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,
> +			       "pcp_prio", "pcp-prio");
> +}
> +
>  static void dcb_app_print_dscp_prio(const struct dcb *dcb,
>  				    const struct dcb_app_table *tab)
>  {
> @@ -439,26 +535,41 @@ static void dcb_app_print(const struct dcb *dcb, const struct dcb_app_table *tab
>  	dcb_app_print_stream_port_prio(tab);
>  	dcb_app_print_dgram_port_prio(tab);
>  	dcb_app_print_port_prio(tab);
> +	dcb_app_print_pcp_prio(dcb, tab);
>  }
>  
>  static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data)
>  {
>  	struct dcb_app_table *tab = data;
>  	struct dcb_app *app;
> +	uint16_t type;
>  	int ret;
>  
> -	if (mnl_attr_get_type(attr) != DCB_ATTR_IEEE_APP) {
> -		fprintf(stderr, "Unknown attribute in DCB_ATTR_IEEE_APP_TABLE: %d\n",
> -			mnl_attr_get_type(attr));
> +	type = mnl_attr_get_type(attr);
> +
> +	if (!dcb_app_attr_type_validate(type)) {
> +		fprintf(stderr,
> +			"Unknown attribute in DCB_ATTR_IEEE_APP_TABLE: %d\n",
> +			type);
>  		return MNL_CB_OK;
>  	}
>  	if (mnl_attr_get_payload_len(attr) < sizeof(struct dcb_app)) {
> -		fprintf(stderr, "DCB_ATTR_IEEE_APP payload expected to have size %zd, not %d\n",
> -			sizeof(struct dcb_app), mnl_attr_get_payload_len(attr));
> +		fprintf(stderr,
> +			"%s payload expected to have size %zd, not %d\n",
> +			ieee_attrs_app_names[type], sizeof(struct dcb_app),
> +			mnl_attr_get_payload_len(attr));
>  		return MNL_CB_OK;
>  	}
>  
>  	app = mnl_attr_get_payload(attr);
> +
> +	/* Check that selector is encapsulated in the right attribute */
> +	if (!dcb_app_selector_validate(type, app->selector)) {
> +		fprintf(stderr, "Wrong selector for type: %s\n",
> +			ieee_attrs_app_names[type]);
> +		return MNL_CB_OK;
> +	}
> +
>  	ret = dcb_app_table_push(tab, app);
>  	if (ret != 0)
>  		return MNL_CB_ERROR;
> @@ -491,6 +602,7 @@ struct dcb_app_add_del {
>  static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
>  {
>  	struct dcb_app_add_del *add_del = data;
> +	enum ieee_attrs_app type;
>  	struct nlattr *nest;
>  	size_t i;
>  
> @@ -498,9 +610,10 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data)
>  
>  	for (i = 0; i < add_del->tab->n_apps; i++) {
>  		const struct dcb_app *app = &add_del->tab->apps[i];
> +		type = dcb_app_attr_type_get(app->selector);
>  
>  		if (add_del->filter == NULL || add_del->filter(app))
> -			mnl_attr_put(nlh, DCB_ATTR_IEEE_APP, sizeof(*app), app);
> +			mnl_attr_put(nlh, type, sizeof(*app), app);
>  	}
>  
>  	mnl_attr_nest_end(nlh, nest);
> @@ -577,6 +690,12 @@ static int dcb_cmd_app_parse_add_del(struct dcb *dcb, const char *dev,
>  			ret = parse_mapping(&argc, &argv, false,
>  					    &dcb_app_parse_mapping_port_prio,
>  					    &pm);
> +		} else if (strcmp(*argv, "pcp-prio") == 0) {
> +			NEXT_ARG();
> +			pm.selector = DCB_APP_SEL_PCP;
> +			ret = parse_mapping_gen(&argc, &argv, &dcb_app_parse_pcp,
> +						&dcb_app_parse_mapping_pcp_prio,
> +						&pm);
>  		} else {
>  			fprintf(stderr, "What is \"%s\"?\n", *argv);
>  			dcb_app_help_add();
> @@ -656,6 +775,8 @@ static int dcb_cmd_app_show(struct dcb *dcb, const char *dev, int argc, char **a
>  			dcb_app_print_port_prio(&tab);
>  		} else if (matches(*argv, "default-prio") == 0) {
>  			dcb_app_print_default_prio(&tab);
> +		} else if (strcmp(*argv, "pcp-prio") == 0) {
> +			dcb_app_print_pcp_prio(dcb, &tab);
>  		} else {
>  			fprintf(stderr, "What is \"%s\"?\n", *argv);
>  			dcb_app_help_show_flush();
> @@ -705,6 +826,11 @@ static int dcb_cmd_app_flush(struct dcb *dcb, const char *dev, int argc, char **
>  					      &dcb_app_is_dscp);
>  			if (ret != 0)
>  				goto out;
> +		} else if (strcmp(*argv, "pcp-prio") == 0) {
> +			ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &tab,
> +					      &dcb_app_is_pcp);
> +			if (ret != 0)
> +				goto out;
>  		} else {
>  			fprintf(stderr, "What is \"%s\"?\n", *argv);
>  			dcb_app_help_show_flush();
> diff --git a/man/man8/dcb-app.8 b/man/man8/dcb-app.8
> index 9780fe4b60fa..054ba9801d81 100644
> --- a/man/man8/dcb-app.8
> +++ b/man/man8/dcb-app.8
> @@ -23,6 +23,7 @@ the DCB (Data Center Bridging) subsystem
>  .RB "[ " dgram-port-prio " ]"
>  .RB "[ " port-prio " ]"
>  .RB "[ " dscp-prio " ]"
> +.RB "[ " pcp-prio " ]"
>  
>  .ti -8
>  .B dcb ets " { " add " | " del " | " replace " } " dev
> @@ -33,6 +34,7 @@ the DCB (Data Center Bridging) subsystem
>  .RB "[ " dgram-port-prio " " \fIPORT-MAP\fB " ]"
>  .RB "[ " port-prio " " \fIPORT-MAP\fB " ]"
>  .RB "[ " dscp-prio " " \fIDSCP-MAP\fB " ]"
> +.RB "[ " pcp-prio " " \fIPCP-MAP\fB " ]"
>  
>  .ti -8
>  .IR PRIO-LIST " := [ " PRIO-LIST " ] " PRIO
> @@ -64,6 +66,9 @@ the DCB (Data Center Bridging) subsystem
>  .ti -8
>  .IR DSCP " := { " \fB0\fR " .. " \fB63\fR " }"
>  
> +.ti -8
> +.IR PCP " := { " \fB0\fR " .. " \fB7\fR " }"
> +
>  .ti -8
>  .IR PRIO " := { " \fB0\fR " .. " \fB7\fR " }"
>  
> @@ -182,6 +187,18 @@ command line option
>  .B -N
>  turns the show translation off.
>  
> +.TP
> +.B pcp-prio \fIPCP-MAP
> +\fIPCP-MAP\fR uses the array parameter syntax, see
> +.BR dcb (8)
> +for details. Keys are PCP/DEI values. Values are priorities assigned to traffic
> +with matching PCP and DEI. PCP/DEI values are written as a combination of
> +numeric- and symbolic values, to accommodate for both PCP and DEI. PCP always
> +in numerical form e.g 1 .. 7 and DEI in symbolic form e.g 'de', indicating that

0..7?

> +the DEI bit is 1.  In combination 2de:1 translates to a mapping of PCP=2 and
> +DEI=1 to priority 1. In a hardware offloaded context, the DEI bit can be mapped
> +directly to drop-precedence (DP) by the driver.

The last sentence about drivers doesn't belong to this man page IMHO.
Besides I'm not sure it's even correct: there is no notion of in-ASIC
drop eligibility here, just mapping between 802.1q priority+de headers
and in-ASIC priority.

> +
>  .SH EXAMPLE & USAGE
>  
>  Prioritize traffic with DSCP 0 to priority 0, 24 to 3 and 48 to 6:
> @@ -221,6 +238,16 @@ Flush all DSCP rules:
>  .br
>  (nothing)
>  
> +Add a rule to map traffic with PCP 1 and DEI 0 to priority 1 and PCP 2 and DEI 1
> +to priority 2:
> +
> +.P
> +# dcb app add dev eth0 pcp-prio 1:1 2de:2
> +.br
> +# dcb app show dev eth0 pcp-prio
> +.br
> +pcp-prio 1:1 2de:2
> +
>  .SH EXIT STATUS
>  Exit status is 0 if command was successful or a positive integer upon failure.

Re: [PATCH iproute2-next 1/2] dcb: add new pcp-prio parameter to dcb app

[Petr Machata]

Petr Machata <petrm@nvidia.com> writes:

> This looks good to me overall, I just have a few nits.
>
> Daniel Machon <daniel.machon@microchip.com> writes:
>
>> Add new pcp-prio parameter to the app subcommand, which can be used to
>> classify traffic based on PCP and DEI from the VLAN header. PCP and DEI
>> is specified in a combination of numerical and symbolic form, where 'de'
>> (as specified in the PCP Encoding Table, 802.1Q) means DEI=1.
>>
>> Map PCP 1 and DEI 0 to priority 1 $ dcb app add dev eth0 pcp-prio 1:1
>>
>> Map PCP 1 and DEI 1 to priority 1 $ dcb app add dev eth0 pcp-prio 1de:1
>>
>> Internally, PCP and DEI is encoded in the protocol field of the dcb_app
>> struct. Each combination of PCP and DEI maps to a priority, thus needing
>> a range of  0-15. A well formed dcb_app entry for PCP/DEI
>> prioritization, could look like:
>>
>>     struct dcb_app pcp = {
>>         .selector = DCB_APP_SEL_PCP,
>> 	.priority = 7,
>>         .protocol = 15
>>     }
>>
>> For mapping PCP=7 and DEI=1 to Prio=7.
>>
>> Also, two helper functions for translating between std and non-std APP
>> selectors, have been added to dcb_app.c and exposed through dcb.h.
>
> Could you include an example or two of how PCP is configured? E.g. the
> following was part of the dcb-app submission:
>
>     # dcb app add dev eni1np1 dscp-prio 0:0 CS3:3 CS6:6
>     # dcb app show dev eni1np1
>     dscp-prio 0:0 CS3:3 CS6:6

I just noticed you have the examples up there. Maybe just reformat them
a bit so that they stand out.

Re: [PATCH iproute2-next 2/2] dcb: add new subcommand for apptrust

[Petr Machata]

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

> Add new apptrust subcommand for the dcbnl apptrust extension object.
>
> The apptrust command lets you specify a consecutive ordered list of
> trusted selectors, which can be used by drivers to determine which
> selectors are eligible (trusted) for packet prioritization, and in which
> order.
>
> Selectors are sent in a new nested attribute:
> DCB_ATTR_IEEE_APP_TRUST_TABLE.  The nest contains trusted selectors
> encapsulated in either DCB_ATTR_IEEE_APP or DCB_ATTR_DCB_APP attributes,
> for standard and non-standard selectors, respectively.
>
> Example:
>
> Trust selectors dscp and pcp, in that order
> $ dcb apptrust set dev eth0 order dscp pcp
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/Makefile            |   3 +-
>  dcb/dcb.c               |   4 +-
>  dcb/dcb.h               |   4 +
>  dcb/dcb_apptrust.c      | 291 ++++++++++++++++++++++++++++++++++++++++
>  man/man8/dcb-apptrust.8 | 118 ++++++++++++++++
>  5 files changed, 418 insertions(+), 2 deletions(-)
>  create mode 100644 dcb/dcb_apptrust.c
>  create mode 100644 man/man8/dcb-apptrust.8
>
> diff --git a/dcb/Makefile b/dcb/Makefile
> index ca65d4670c80..dd41a559a0c8 100644
> --- a/dcb/Makefile
> +++ b/dcb/Makefile
> @@ -7,7 +7,8 @@ DCBOBJ = dcb.o \
>           dcb_dcbx.o \
>           dcb_ets.o \
>           dcb_maxrate.o \
> -         dcb_pfc.o
> +         dcb_pfc.o \
> +         dcb_apptrust.o
>  TARGETS += dcb
>  LDLIBS += -lm
>  
> diff --git a/dcb/dcb.c b/dcb/dcb.c
> index 391fd95455f4..3ffa91d64d0d 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 | buffer | dcbx | ets | maxrate | pfc }\n"
> +		"where  OBJECT := { app | apptrust | buffer | dcbx | ets | maxrate | pfc }\n"
>  		"       OPTIONS := [ -V | --Version | -i | --iec | -j | --json\n"
>  		"                  | -N | --Numeric | -p | --pretty\n"
>  		"                  | -s | --statistics | -v | --verbose]\n");
> @@ -483,6 +483,8 @@ static int dcb_cmd(struct dcb *dcb, int argc, char **argv)
>  		return 0;
>  	} else if (matches(*argv, "app") == 0) {
>  		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 (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 05eddcbbcfdf..d40664f29dad 100644
> --- a/dcb/dcb.h
> +++ b/dcb/dcb.h
> @@ -61,6 +61,10 @@ 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);
> +
>  /* dcb_buffer.c */
>  
>  int dcb_cmd_buffer(struct dcb *dcb, int argc, char **argv);
> diff --git a/dcb/dcb_apptrust.c b/dcb/dcb_apptrust.c
> new file mode 100644
> index 000000000000..14d18dcb7f83
> --- /dev/null
> +++ b/dcb/dcb_apptrust.c
> @@ -0,0 +1,291 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <errno.h>
> +#include <linux/dcbnl.h>
> +
> +#include "dcb.h"
> +#include "utils.h"
> +
> +static void dcb_apptrust_help_set(void)
> +{
> +	fprintf(stderr,
> +		"Usage: dcb apptrust set dev STRING\n"
> +		"	[ order [ eth | stream | dgram | any | dscp | pcp ] ]\n"
> +		"\n");
> +}
> +
> +static void dcb_apptrust_help_show(void)
> +{
> +	fprintf(stderr, "Usage: dcb [ -i ] apptrust show dev STRING\n"
> +			"           [ order ]\n"
> +			"\n");
> +}
> +
> +static void dcb_apptrust_help(void)
> +{
> +	fprintf(stderr, "Usage: dcb apptrust help\n"
> +			"\n");
> +	dcb_apptrust_help_show();
> +	dcb_apptrust_help_set();
> +}
> +
> +static const char *const selector_names[] = {
> +	[IEEE_8021QAZ_APP_SEL_ETHERTYPE] = "eth",
> +	[IEEE_8021QAZ_APP_SEL_STREAM]    = "stream",
> +	[IEEE_8021QAZ_APP_SEL_DGRAM]     = "dgram",
> +	[IEEE_8021QAZ_APP_SEL_ANY]       = "any",
> +	[IEEE_8021QAZ_APP_SEL_DSCP]      = "dscp",
> +	[DCB_APP_SEL_PCP]                = "pcp",
> +};

These names should match how dcb-app names them. So ethtype,
stream-port, dgram-port, port, dscp, pcp.

> +
> +struct dcb_apptrust_table {
> +	__u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1];
> +	int nselectors;
> +};
> +
> +static bool dcb_apptrust_contains(const struct dcb_apptrust_table *table,
> +				  __u8 selector)
> +{
> +	int i;
> +
> +	for (i = 0; i < table->nselectors; i++)
> +		if (table->selectors[i] == selector)
> +			return true;
> +
> +	return false;
> +}
> +
> +static void dcb_apptrust_print(const struct dcb_apptrust_table *table)
> +{
> +	const char *str;
> +	__u8 selector;
> +	int i;
> +
> +	open_json_array(PRINT_JSON, "order");
> +	print_string(PRINT_FP, NULL, "order: ", NULL);
> +
> +	for (i = 0; i < table->nselectors; i++) {
> +		selector = table->selectors[i];
> +		str = selector_names[selector];
> +		print_string(PRINT_ANY, NULL, "%s ", str);
> +	}
> +	print_nl();
> +
> +	close_json_array(PRINT_JSON, "order");
> +}
> +
> +static int dcb_apptrust_get_cb(const struct nlattr *attr, void *data)
> +{
> +	struct dcb_apptrust_table *table = data;
> +	uint16_t type;
> +	__u8 selector;
> +
> +	type = mnl_attr_get_type(attr);
> +
> +	if (!dcb_app_attr_type_validate(type)) {
> +		fprintf(stderr,
> +			"Unknown attribute in DCB_ATTR_IEEE_APP_TRUST_TABLE: %d\n",
> +			type);
> +		return MNL_CB_OK;
> +	}
> +
> +	if (mnl_attr_get_payload_len(attr) < 1) {
> +		fprintf(stderr,
> +			"DCB_ATTR_IEEE_APP_TRUST payload expected to have size %zd, not %d\n",
> +			sizeof(struct dcb_app), mnl_attr_get_payload_len(attr));
> +		return MNL_CB_OK;
> +	}
> +
> +	selector = mnl_attr_get_u8(attr);
> +
> +	/* Check that selector is encapsulated in the right attribute */
> +	if (!dcb_app_selector_validate(type, selector)) {
> +		fprintf(stderr, "Wrong type for selector: %s\n",
> +			selector_names[selector]);
> +		return MNL_CB_OK;
> +	}
> +
> +	table->selectors[table->nselectors++] = selector;
> +
> +	return MNL_CB_OK;
> +}
> +
> +static int dcb_apptrust_get(struct dcb *dcb, const char *dev,
> +			    struct dcb_apptrust_table *table)
> +{
> +	uint16_t payload_len;
> +	void *payload;
> +	int ret;
> +
> +	ret = dcb_get_attribute_va(dcb, dev, DCB_ATTR_DCB_APP_TRUST_TABLE,
> +				   &payload, &payload_len);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = mnl_attr_parse_payload(payload, payload_len, dcb_apptrust_get_cb,
> +				     table);
> +	if (ret != MNL_CB_OK)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int dcb_apptrust_set_cb(struct dcb *dcb, struct nlmsghdr *nlh,
> +			       void *data)
> +{
> +	const struct dcb_apptrust_table *table = data;
> +	enum ieee_attrs_app type;
> +	struct nlattr *nest;
> +	int i;
> +
> +	nest = mnl_attr_nest_start(nlh, DCB_ATTR_DCB_APP_TRUST_TABLE);
> +
> +	for (i = 0; i < table->nselectors; i++) {
> +		type = dcb_app_attr_type_get(table->selectors[i]);
> +		mnl_attr_put_u8(nlh, type, table->selectors[i]);
> +	}
> +
> +	mnl_attr_nest_end(nlh, nest);
> +
> +	return 0;
> +}
> +
> +static int dcb_apptrust_set(struct dcb *dcb, const char *dev,
> +			    const struct dcb_apptrust_table *table)
> +{
> +	return dcb_set_attribute_va(dcb, DCB_CMD_IEEE_SET, dev,
> +				    &dcb_apptrust_set_cb, (void *)table);
> +}
> +
> +static int dcb_apptrust_parse_selector_list(int *argcp, char ***argvp,
> +					    struct dcb_apptrust_table *table)
> +{
> +	char **argv = *argvp;
> +	int argc = *argcp;
> +	__u8 selector;
> +	int ret;
> +
> +	NEXT_ARG_FWD();
> +
> +	/* No trusted selectors ? */
> +	if (argc == 0)
> +		goto out;
> +
> +	while (argc > 0) {
> +		selector = parse_one_of("order", *argv, selector_names,
> +					ARRAY_SIZE(selector_names), &ret);
> +		if (ret < 0)
> +			return -EINVAL;

I think this should legitimately conclude the parsing, because it could
be one of the higher-level keywords. Currently there's only one,
"order", but nonetheless. I think it should goto out, and be plonked by
the caller with "what is X?". Similar to how the first argument that
doesn't parse as e.g. DSCP:PRIO bails out and is attempted as a keyword
higher up, and either parsed, or plonked with "what is X".

> +
> +		if (table->nselectors > IEEE_8021QAZ_APP_SEL_MAX)

Yeah, this is purely theoretical, so no need for a message.

> +			return -ERANGE;
> +
> +		if (dcb_apptrust_contains(table, selector)) {
> +			fprintf(stderr, "Duplicate selector: %s\n",
> +				selector_names[selector]);
> +			return -EINVAL;
> +		}
> +
> +		table->selectors[table->nselectors++] = selector;
> +
> +		NEXT_ARG_FWD();
> +	}
> +
> +out:
> +	*argcp = argc;
> +	*argvp = argv;
> +
> +	return 0;
> +}
> +
> +static int dcb_cmd_apptrust_set(struct dcb *dcb, const char *dev, int argc,
> +				char **argv)
> +{
> +	struct dcb_apptrust_table table = { 0 };
> +	int ret;
> +
> +	if (!argc) {
> +		dcb_apptrust_help_set();
> +		return 0;
> +	}
> +
> +	do {
> +		if (strcmp(*argv, "help") == 0) {
> +			dcb_apptrust_help_set();
> +			return 0;
> +		} else if (strcmp(*argv, "order") == 0) {
> +			ret = dcb_apptrust_parse_selector_list(&argc, &argv,
> +							       &table);
> +			if (ret < 0) {
> +				fprintf(stderr, "Invalid list of selectors\n");
> +				return -EINVAL;
> +			}
> +			continue;
> +		} else {
> +			fprintf(stderr, "What is \"%s\"?\n", *argv);
> +			dcb_apptrust_help_set();
> +			return -EINVAL;
> +		}
> +
> +		NEXT_ARG_FWD();
> +	} while (argc > 0);
> +
> +	return dcb_apptrust_set(dcb, dev, &table);
> +}
> +
> +static int dcb_cmd_apptrust_show(struct dcb *dcb, const char *dev, int argc,
> +				 char **argv)
> +{
> +	struct dcb_apptrust_table table = { 0 };
> +	int ret;
> +
> +	ret = dcb_apptrust_get(dcb, dev, &table);
> +	if (ret)
> +		return ret;
> +
> +	open_json_object(NULL);
> +
> +	if (!argc) {
> +		dcb_apptrust_help();

Given no arguments to show, the tool should show everything, not help.

> +		goto out;
> +	}
> +
> +	do {
> +		if (strcmp(*argv, "help") == 0) {
> +			dcb_apptrust_help_show();
> +			return 0;
> +		} else if (strcmp(*argv, "order") == 0) {
> +			dcb_apptrust_print(&table);

This should probably be dcb_apptrust_print_order, so that more stuff can
be added cleanly.

> +		} else {
> +			fprintf(stderr, "What is \"%s\"?\n", *argv);
> +			dcb_apptrust_help_show();
> +			return -EINVAL;
> +		}
> +
> +		NEXT_ARG_FWD();
> +	} while (argc > 0);
> +
> +out:
> +	close_json_object();
> +	return 0;
> +}
> +
> +int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv)
> +{
> +	if (!argc || strcmp(*argv, "help") == 0) {
> +		dcb_apptrust_help();
> +		return 0;
> +	} else if (strcmp(*argv, "show") == 0) {
> +		NEXT_ARG_FWD();
> +		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_apptrust_show,
> +					 dcb_apptrust_help_show);
> +	} else if (strcmp(*argv, "set") == 0) {
> +		NEXT_ARG_FWD();
> +		return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_apptrust_set,
> +					 dcb_apptrust_help_set);
> +	} else {
> +		fprintf(stderr, "What is \"%s\"?\n", *argv);
> +		dcb_apptrust_help();
> +		return -EINVAL;
> +	}
> +}
> diff --git a/man/man8/dcb-apptrust.8 b/man/man8/dcb-apptrust.8
> new file mode 100644
> index 000000000000..9ebe7c17292c
> --- /dev/null
> +++ b/man/man8/dcb-apptrust.8
> @@ -0,0 +1,118 @@
> +.TH DCB-APPTRUST 8 "22 November 2022" "iproute2" "Linux"
> +.SH NAME
> +dcb-apptrust \- show / manipulate per-selector trust and trust order of the application
> +priority table of the DCB (Data Center Bridging) subsystem.
> +.SH SYNOPSIS
> +.sp
> +.ad l
> +.in +8
> +
> +.ti -8
> +.B dcb
> +.RI "[ " OPTIONS " ] "
> +.B apptrust
> +.RI "{ " COMMAND " | " help " }"
> +.sp
> +
> +.ti -8
> +.B dcb apptrust show dev order
> +.RI DEV
> +
> +.ti -8
> +.B dcb apptrust set dev order
> +.RI DEV
> +.RB "[ " eth " ]"
> +.RB "[ " stream " ]"
> +.RB "[ " dgram " ]"
> +.RB "[ " any " ]"
> +.RB "[ " dscp " ]"
> +.RB "[ " pcp " ]"

Taken literally, this prescribes the order, just allows omitting some of
the selectors. I think you'll need to circumscribe like this:

    dcb apptrust set dev order [ SEL-LIST ]
    SEL-LIST := [ SEL-LIST ] SEL
    SEL := { ethtype | stream-port | etc. etc. }

> +.SH DESCRIPTION
> +
> +.B dcb apptrust
> +is used to configure and manipulate per-selector trust and trust order of the
> +Application Priority Table, see
> +.BR dcb-app (8)
> +for details on how to configure app table entries.
> +
> +Selector trust can be used by the
> +software stack, or drivers (most likely the latter), when querying the APP
> +table, to determine if an APP entry should take effect, or not. Additionaly, the
> +order of the trusted selectors will dictate which selector should take
> +precedence, in the case of multiple different APP selectors being present in the
> +APP table.
> +
> +.SH COMMANDS
> +
> +.TP
> +.B show
> +Display all trusted selectors.
> +
> +.TP
> +.B set
> +Set new list of trusted selectors. Empty list is effectively the same as
> +removing trust entirely.
> +
> +.SH PARAMETERS
> +
> +The following describes only the write direction, i.e. as used with the
> +\fBset\fR command. For the \fBshow\fR command, the parameter name is to be used
> +as a simple keyword without further arguments. This instructs the tool to show
> +the values of a given parameter.
> +
> +.TP
> +.B order \fISELECTOR-NAMES
> +\fISELECTOR-NAMES\fR is a space-separated list of selector names:\fR
> +
> +.B eth
> +Trust EtherType.
> +
> +.B stream
> +Trust TCP, or Stream Control Transmission Protocol (SCTP).
> +
> +.B dgram
> +Trust UDP, or Datagram Congestion Control Protocol (DCCP).
> +
> +.B any
> +Trust TCP, SCTP, UDP, or DCCP.
> +
> +.B dscp
> +Trust Differentiated Services Code Point (DSCP) values.
> +
> +.B pcp
> +Trust Priority Code Point/Drop Eligible Indicator (PCP/DEI).

These names need to be updated as well.

> +
> +
> +.SH EXAMPLE & USAGE
> +
> +Set trust order to: dscp, pcp for eth0:
> +.P
> +# dcb apptrust set dev eth0 order dscp pcp
> +
> +Set trust order to: any (stream or dgram), pcp, eth for eth1:
> +.P
> +# dcb apptrust set dev eth1 order any pcp eth
> +
> +Show what was set:
> +
> +.P
> +# dcb apptrust show dev eth0
> +.br
> +order: any pcp eth
> +
> +.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)
> +
> +.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>

Re: [PATCH iproute2-next 1/2] dcb: add new pcp-prio parameter to dcb app

[Petr Machata]

Petr Machata <petrm@nvidia.com> writes:

> This looks good to me overall, I just have a few nits.

Actually, one more fairly fundamental thing that occurred to me. If a
user doesn't care about DEI, they need to do this using two rules: say
1:1 and 1de:1.

I wonder if it would make sense to assume that people are more likely to
not care about DEI at all, and make the 1:1 mean that. Then 1:1 would be
a shorthand for expressing two rules, one for DE=0, one for DE=1.

If the user does care about DEI, they would either say 1de:1 or 1nd:1,
depending on what they want the DEI to be.

If you generally agree with this idea, but don't have spare cycles to
code it up, would you please just make the PCP keys "${prio}de" and
"${prio}nd"? (Or whatever, but have the syntax reflect the DEI state in
both cases.) I think I'll be able to scrape a bit of a free time on some
weekend to add the syntax sugar.

Re: [PATCH iproute2-next 2/2] dcb: add new subcommand for apptrust

[Daniel.Machon]

As always, thank you for the feedback - much appreciated!

> > diff --git a/dcb/dcb_apptrust.c b/dcb/dcb_apptrust.c
> > new file mode 100644
> > index 000000000000..14d18dcb7f83
> > --- /dev/null
> > +++ b/dcb/dcb_apptrust.c
> > @@ -0,0 +1,291 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include <errno.h>
> > +#include <linux/dcbnl.h>
> > +
> > +#include "dcb.h"
> > +#include "utils.h"
> > +
> > +static void dcb_apptrust_help_set(void)
> > +{
> > +     fprintf(stderr,
> > +             "Usage: dcb apptrust set dev STRING\n"
> > +             "       [ order [ eth | stream | dgram | any | dscp | pcp ] ]\n"
> > +             "\n");
> > +}
> > +
> > +static void dcb_apptrust_help_show(void)
> > +{
> > +     fprintf(stderr, "Usage: dcb [ -i ] apptrust show dev STRING\n"
> > +                     "           [ order ]\n"
> > +                     "\n");
> > +}
> > +
> > +static void dcb_apptrust_help(void)
> > +{
> > +     fprintf(stderr, "Usage: dcb apptrust help\n"
> > +                     "\n");
> > +     dcb_apptrust_help_show();
> > +     dcb_apptrust_help_set();
> > +}
> > +
> > +static const char *const selector_names[] = {
> > +     [IEEE_8021QAZ_APP_SEL_ETHERTYPE] = "eth",
> > +     [IEEE_8021QAZ_APP_SEL_STREAM]    = "stream",
> > +     [IEEE_8021QAZ_APP_SEL_DGRAM]     = "dgram",
> > +     [IEEE_8021QAZ_APP_SEL_ANY]       = "any",
> > +     [IEEE_8021QAZ_APP_SEL_DSCP]      = "dscp",
> > +     [DCB_APP_SEL_PCP]                = "pcp",
> > +};
> 
> These names should match how dcb-app names them. So ethtype,
> stream-port, dgram-port, port, dscp, pcp.

My intention was to match the selector names from the uapi header, and
not the dcb-app names (except "eth" should actually have been
"ethertype"). Afterall, we are specifying selector trust. But then
again, the user does not know about selector names, only what is visible
through dcb-app help() and the dcb-app man page. I guess you are right :)

> 
> > +
> > +struct dcb_apptrust_table {
> > +     __u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1];
> > +     int nselectors;
> > +};
> > +
> > +static bool dcb_apptrust_contains(const struct dcb_apptrust_table *table,
> > +                               __u8 selector)
> > +{
> > +     int i;
> > +
> > +     for (i = 0; i < table->nselectors; i++)
> > +             if (table->selectors[i] == selector)
> > +                     return true;
> > +
> > +     return false;
> > +}
> > +
> > +static void dcb_apptrust_print(const struct dcb_apptrust_table *table)
> > +{
> > +     const char *str;
> > +     __u8 selector;
> > +     int i;
> > +
> > +     open_json_array(PRINT_JSON, "order");
> > +     print_string(PRINT_FP, NULL, "order: ", NULL);
> > +
> > +     for (i = 0; i < table->nselectors; i++) {
> > +             selector = table->selectors[i];
> > +             str = selector_names[selector];
> > +             print_string(PRINT_ANY, NULL, "%s ", str);
> > +     }
> > +     print_nl();
> > +
> > +     close_json_array(PRINT_JSON, "order");
> > +}
> > +
> > +static int dcb_apptrust_get_cb(const struct nlattr *attr, void *data)
> > +{
> > +     struct dcb_apptrust_table *table = data;
> > +     uint16_t type;
> > +     __u8 selector;
> > +
> > +     type = mnl_attr_get_type(attr);
> > +
> > +     if (!dcb_app_attr_type_validate(type)) {
> > +             fprintf(stderr,
> > +                     "Unknown attribute in DCB_ATTR_IEEE_APP_TRUST_TABLE: %d\n",
> > +                     type);
> > +             return MNL_CB_OK;
> > +     }
> > +
> > +     if (mnl_attr_get_payload_len(attr) < 1) {
> > +             fprintf(stderr,
> > +                     "DCB_ATTR_IEEE_APP_TRUST payload expected to have size %zd, not %d\n",
> > +                     sizeof(struct dcb_app), mnl_attr_get_payload_len(attr));
> > +             return MNL_CB_OK;
> > +     }
> > +
> > +     selector = mnl_attr_get_u8(attr);
> > +
> > +     /* Check that selector is encapsulated in the right attribute */
> > +     if (!dcb_app_selector_validate(type, selector)) {
> > +             fprintf(stderr, "Wrong type for selector: %s\n",
> > +                     selector_names[selector]);
> > +             return MNL_CB_OK;
> > +     }
> > +
> > +     table->selectors[table->nselectors++] = selector;
> > +
> > +     return MNL_CB_OK;
> > +}
> > +
> > +static int dcb_apptrust_get(struct dcb *dcb, const char *dev,
> > +                         struct dcb_apptrust_table *table)
> > +{
> > +     uint16_t payload_len;
> > +     void *payload;
> > +     int ret;
> > +
> > +     ret = dcb_get_attribute_va(dcb, dev, DCB_ATTR_DCB_APP_TRUST_TABLE,
> > +                                &payload, &payload_len);
> > +     if (ret != 0)
> > +             return ret;
> > +
> > +     ret = mnl_attr_parse_payload(payload, payload_len, dcb_apptrust_get_cb,
> > +                                  table);
> > +     if (ret != MNL_CB_OK)
> > +             return -EINVAL;
> > +
> > +     return 0;
> > +}
> > +
> > +static int dcb_apptrust_set_cb(struct dcb *dcb, struct nlmsghdr *nlh,
> > +                            void *data)
> > +{
> > +     const struct dcb_apptrust_table *table = data;
> > +     enum ieee_attrs_app type;
> > +     struct nlattr *nest;
> > +     int i;
> > +
> > +     nest = mnl_attr_nest_start(nlh, DCB_ATTR_DCB_APP_TRUST_TABLE);
> > +
> > +     for (i = 0; i < table->nselectors; i++) {
> > +             type = dcb_app_attr_type_get(table->selectors[i]);
> > +             mnl_attr_put_u8(nlh, type, table->selectors[i]);
> > +     }
> > +
> > +     mnl_attr_nest_end(nlh, nest);
> > +
> > +     return 0;
> > +}
> > +
> > +static int dcb_apptrust_set(struct dcb *dcb, const char *dev,
> > +                         const struct dcb_apptrust_table *table)
> > +{
> > +     return dcb_set_attribute_va(dcb, DCB_CMD_IEEE_SET, dev,
> > +                                 &dcb_apptrust_set_cb, (void *)table);
> > +}
> > +
> > +static int dcb_apptrust_parse_selector_list(int *argcp, char ***argvp,
> > +                                         struct dcb_apptrust_table *table)
> > +{
> > +     char **argv = *argvp;
> > +     int argc = *argcp;
> > +     __u8 selector;
> > +     int ret;
> > +
> > +     NEXT_ARG_FWD();
> > +
> > +     /* No trusted selectors ? */
> > +     if (argc == 0)
> > +             goto out;
> > +
> > +     while (argc > 0) {
> > +             selector = parse_one_of("order", *argv, selector_names,
> > +                                     ARRAY_SIZE(selector_names), &ret);
> > +             if (ret < 0)
> > +                     return -EINVAL;
> 
> I think this should legitimately conclude the parsing, because it could
> be one of the higher-level keywords. Currently there's only one,
> "order", but nonetheless. I think it should goto out, and be plonked by
> the caller with "what is X?". Similar to how the first argument that
> doesn't parse as e.g. DSCP:PRIO bails out and is attempted as a keyword
> higher up, and either parsed, or plonked with "what is X".

I dont quite follow you on this one. We are parsing the selector list
here. Any offending selector is printed, as well as the entire list of
valid ones. How could it be one of the higher-level keywords? Am I
missing something here? :-)

> 
> > +
> > +             if (table->nselectors > IEEE_8021QAZ_APP_SEL_MAX)
> 
> Yeah, this is purely theoretical, so no need for a message.
> 
> > +                     return -ERANGE;
> > +
> > +             if (dcb_apptrust_contains(table, selector)) {
> > +                     fprintf(stderr, "Duplicate selector: %s\n",
> > +                             selector_names[selector]);
> > +                     return -EINVAL;
> > +             }
> > +
> > +             table->selectors[table->nselectors++] = selector;
> > +
> > +             NEXT_ARG_FWD();
> > +     }
> > +
> > +out:
> > +     *argcp = argc;
> > +     *argvp = argv;
> > +
> > +     return 0;
> > +}
> > +
> > +static int dcb_cmd_apptrust_set(struct dcb *dcb, const char *dev, int argc,
> > +                             char **argv)
> > +{
> > +     struct dcb_apptrust_table table = { 0 };
> > +     int ret;
> > +
> > +     if (!argc) {
> > +             dcb_apptrust_help_set();
> > +             return 0;
> > +     }
> > +
> > +     do {
> > +             if (strcmp(*argv, "help") == 0) {
> > +                     dcb_apptrust_help_set();
> > +                     return 0;
> > +             } else if (strcmp(*argv, "order") == 0) {
> > +                     ret = dcb_apptrust_parse_selector_list(&argc, &argv,
> > +                                                            &table);
> > +                     if (ret < 0) {
> > +                             fprintf(stderr, "Invalid list of selectors\n");
> > +                             return -EINVAL;
> > +                     }
> > +                     continue;
> > +             } else {
> > +                     fprintf(stderr, "What is \"%s\"?\n", *argv);
> > +                     dcb_apptrust_help_set();
> > +                     return -EINVAL;
> > +             }
> > +
> > +             NEXT_ARG_FWD();
> > +     } while (argc > 0);
> > +
> > +     return dcb_apptrust_set(dcb, dev, &table);
> > +}
> > +
> > +static int dcb_cmd_apptrust_show(struct dcb *dcb, const char *dev, int argc,
> > +                              char **argv)
> > +{
> > +     struct dcb_apptrust_table table = { 0 };
> > +     int ret;
> > +
> > +     ret = dcb_apptrust_get(dcb, dev, &table);
> > +     if (ret)
> > +             return ret;
> > +
> > +     open_json_object(NULL);
> > +
> > +     if (!argc) {
> > +             dcb_apptrust_help();
> 
> Given no arguments to show, the tool should show everything, not help.

Ahh. Yes. Will fix that.

> 
> > +             goto out;
> > +     }
> > +
> > +     do {
> > +             if (strcmp(*argv, "help") == 0) {
> > +                     dcb_apptrust_help_show();
> > +                     return 0;
> > +             } else if (strcmp(*argv, "order") == 0) {
> > +                     dcb_apptrust_print(&table);
> 
> This should probably be dcb_apptrust_print_order, so that more stuff can
> be added cleanly.
> 
> > +             } else {
> > +                     fprintf(stderr, "What is \"%s\"?\n", *argv);
> > +                     dcb_apptrust_help_show();
> > +                     return -EINVAL;
> > +             }
> > +
> > +             NEXT_ARG_FWD();
> > +     } while (argc > 0);
> > +
> > +out:
> > +     close_json_object();
> > +     return 0;
> > +}
> > +
> > +int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv)
> > +{
> > +     if (!argc || strcmp(*argv, "help") == 0) {
> > +             dcb_apptrust_help();
> > +             return 0;
> > +     } else if (strcmp(*argv, "show") == 0) {
> > +             NEXT_ARG_FWD();
> > +             return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_apptrust_show,
> > +                                      dcb_apptrust_help_show);
> > +     } else if (strcmp(*argv, "set") == 0) {
> > +             NEXT_ARG_FWD();
> > +             return dcb_cmd_parse_dev(dcb, argc, argv, dcb_cmd_apptrust_set,
> > +                                      dcb_apptrust_help_set);
> > +     } else {
> > +             fprintf(stderr, "What is \"%s\"?\n", *argv);
> > +             dcb_apptrust_help();
> > +             return -EINVAL;
> > +     }
> > +}
> > diff --git a/man/man8/dcb-apptrust.8 b/man/man8/dcb-apptrust.8
> > new file mode 100644
> > index 000000000000..9ebe7c17292c
> > --- /dev/null
> > +++ b/man/man8/dcb-apptrust.8
> > @@ -0,0 +1,118 @@
> > +.TH DCB-APPTRUST 8 "22 November 2022" "iproute2" "Linux"
> > +.SH NAME
> > +dcb-apptrust \- show / manipulate per-selector trust and trust order of the application
> > +priority table of the DCB (Data Center Bridging) subsystem.
> > +.SH SYNOPSIS
> > +.sp
> > +.ad l
> > +.in +8
> > +
> > +.ti -8
> > +.B dcb
> > +.RI "[ " OPTIONS " ] "
> > +.B apptrust
> > +.RI "{ " COMMAND " | " help " }"
> > +.sp
> > +
> > +.ti -8
> > +.B dcb apptrust show dev order
> > +.RI DEV
> > +
> > +.ti -8
> > +.B dcb apptrust set dev order
> > +.RI DEV
> > +.RB "[ " eth " ]"
> > +.RB "[ " stream " ]"
> > +.RB "[ " dgram " ]"
> > +.RB "[ " any " ]"
> > +.RB "[ " dscp " ]"
> > +.RB "[ " pcp " ]"
> 
> Taken literally, this prescribes the order, just allows omitting some of
> the selectors. I think you'll need to circumscribe like this:
> 
>     dcb apptrust set dev order [ SEL-LIST ]
>     SEL-LIST := [ SEL-LIST ] SEL
>     SEL := { ethtype | stream-port | etc. etc. }
> 

Ack. Will be fixed.

> > +.SH DESCRIPTION
> > +
> > +.B dcb apptrust
> > +is used to configure and manipulate per-selector trust and trust order of the
> > +Application Priority Table, see
> > +.BR dcb-app (8)
> > +for details on how to configure app table entries.
> > +
> > +Selector trust can be used by the
> > +software stack, or drivers (most likely the latter), when querying the APP
> > +table, to determine if an APP entry should take effect, or not. Additionaly, the
> > +order of the trusted selectors will dictate which selector should take
> > +precedence, in the case of multiple different APP selectors being present in the
> > +APP table.
> > +
> > +.SH COMMANDS
> > +
> > +.TP
> > +.B show
> > +Display all trusted selectors.
> > +
> > +.TP
> > +.B set
> > +Set new list of trusted selectors. Empty list is effectively the same as
> > +removing trust entirely.
> > +
> > +.SH PARAMETERS
> > +
> > +The following describes only the write direction, i.e. as used with the
> > +\fBset\fR command. For the \fBshow\fR command, the parameter name is to be used
> > +as a simple keyword without further arguments. This instructs the tool to show
> > +the values of a given parameter.
> > +
> > +.TP
> > +.B order \fISELECTOR-NAMES
> > +\fISELECTOR-NAMES\fR is a space-separated list of selector names:\fR
> > +
> > +.B eth
> > +Trust EtherType.
> > +
> > +.B stream
> > +Trust TCP, or Stream Control Transmission Protocol (SCTP).
> > +
> > +.B dgram
> > +Trust UDP, or Datagram Congestion Control Protocol (DCCP).
> > +
> > +.B any
> > +Trust TCP, SCTP, UDP, or DCCP.
> > +
> > +.B dscp
> > +Trust Differentiated Services Code Point (DSCP) values.
> > +
> > +.B pcp
> > +Trust Priority Code Point/Drop Eligible Indicator (PCP/DEI).
> 
> These names need to be updated as well.

Ack. Will be fixed.

/ Daniel 

Re: [PATCH iproute2-next 1/2] dcb: add new pcp-prio parameter to dcb app

[Daniel.Machon]

> Petr Machata <petrm@nvidia.com> writes:
> 
> > This looks good to me overall, I just have a few nits.
> 
> Actually, one more fairly fundamental thing that occurred to me. If a
> user doesn't care about DEI, they need to do this using two rules: say
> 1:1 and 1de:1.
> 
> I wonder if it would make sense to assume that people are more likely to
> not care about DEI at all, and make the 1:1 mean that. Then 1:1 would be
> a shorthand for expressing two rules, one for DE=0, one for DE=1.
> 
> If the user does care about DEI, they would either say 1de:1 or 1nd:1,
> depending on what they want the DEI to be.
> 
> If you generally agree with this idea, but don't have spare cycles to
> code it up, would you please just make the PCP keys "${prio}de" and
> "${prio}nd"? (Or whatever, but have the syntax reflect the DEI state in
> both cases.) I think I'll be able to scrape a bit of a free time on some
> weekend to add the syntax sugar.

I think this could be useful and 'de', 'nd' (not-drop-eligible?) is fine
by me. However, it is perfectly useable for me in its current form so I
wont object if you can find the time to code this addition. Is there any
reason to add the '${prio}nd' keys upfront?

/ Daniel

Re: [PATCH iproute2-next 2/2] dcb: add new subcommand for apptrust

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

>> > +static int dcb_apptrust_parse_selector_list(int *argcp, char ***argvp,
>> > +                                         struct dcb_apptrust_table *table)
>> > +{
>> > +     char **argv = *argvp;
>> > +     int argc = *argcp;
>> > +     __u8 selector;
>> > +     int ret;
>> > +
>> > +     NEXT_ARG_FWD();
>> > +
>> > +     /* No trusted selectors ? */
>> > +     if (argc == 0)
>> > +             goto out;
>> > +
>> > +     while (argc > 0) {
>> > +             selector = parse_one_of("order", *argv, selector_names,
>> > +                                     ARRAY_SIZE(selector_names), &ret);
>> > +             if (ret < 0)
>> > +                     return -EINVAL;
>> 
>> I think this should legitimately conclude the parsing, because it could
>> be one of the higher-level keywords. Currently there's only one,
>> "order", but nonetheless. I think it should goto out, and be plonked by
>> the caller with "what is X?". Similar to how the first argument that
>> doesn't parse as e.g. DSCP:PRIO bails out and is attempted as a keyword
>> higher up, and either parsed, or plonked with "what is X".
>
> I dont quite follow you on this one. We are parsing the selector list
> here. Any offending selector is printed, as well as the entire list of
> valid ones. How could it be one of the higher-level keywords? Am I
> missing something here? :-)

Imagine there's more to specify than order. Say, per-selector rewrite
enablement or something. Then the command-line to specify both at the
same time could look like this:

# dcb apptrust set dev eth0 order dscp pcp rewrite dscp:on pcp:off

I think that currently the "rewrite" keyword will trigger the -EINVAL
return above and the whole command line will be rejected.

Right now it's all the same, because there's only one thing to
configure, but it would be cleaner to handle this case as if there could
be more things to configure.

Re: [PATCH iproute2-next 1/2] dcb: add new pcp-prio parameter to dcb app

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

>> Petr Machata <petrm@nvidia.com> writes:
>> 
>> > This looks good to me overall, I just have a few nits.
>> 
>> Actually, one more fairly fundamental thing that occurred to me. If a
>> user doesn't care about DEI, they need to do this using two rules: say
>> 1:1 and 1de:1.
>> 
>> I wonder if it would make sense to assume that people are more likely to
>> not care about DEI at all, and make the 1:1 mean that. Then 1:1 would be
>> a shorthand for expressing two rules, one for DE=0, one for DE=1.
>> 
>> If the user does care about DEI, they would either say 1de:1 or 1nd:1,
>> depending on what they want the DEI to be.
>> 
>> If you generally agree with this idea, but don't have spare cycles to
>> code it up, would you please just make the PCP keys "${prio}de" and
>> "${prio}nd"? (Or whatever, but have the syntax reflect the DEI state in
>> both cases.) I think I'll be able to scrape a bit of a free time on some
>> weekend to add the syntax sugar.
>
> I think this could be useful and 'de', 'nd' (not-drop-eligible?) is fine

Yeah, no-drop. It could also be de/nde perhaps, I have no strong
preference here.

> by me. However, it is perfectly useable for me in its current form so I
> wont object if you can find the time to code this addition. Is there any
> reason to add the '${prio}nd' keys upfront?

Yes, so that the semantics do not change. Whatever 1:1 means now, that's
what it will have to mean basically forever. That's why I'm asking you
to change to 1de:1 and 1nd:1, so that the 1:1 syntax remains free.