[PATCH net-next v2 0/6] Add new PCP and APPTRUST attributes to dcbnl

[PATCH net-next v2 0/6] Add new PCP and APPTRUST attributes to dcbnl

[Daniel Machon]

This patch series adds new extension attributes to dcbnl, for PCP queue
classification and per-selector trust and trust order. Additionally, the
microchip sparx5 driver has been dcb-enabled to make use of the new
attributes to offload PCP, DSCP and Default prio to the switch, and
implement trust order of selectors.

For pre-RFC discussion see:
https://lore.kernel.org/netdev/Yv9VO1DYAxNduw6A@DEN-LT-70577/

For RFC series see:
https://lore.kernel.org/netdev/20220915095757.2861822-1-daniel.machon@microchip.com/

In summary: there currently exist no convenient way to offload per-port
PCP-based queue classification to hardware. Similarly, there is no way
to indicate the notion of trust for APP table selectors. This patch
series addresses both topics.

PCP based queue classification:
  - 8021Q standardizes the Priority Code Point table (see 6.9.3 of IEEE
    Std 802.1Q-2018).  This patch series makes it possible, to offload
    the PCP classification to said table.  The new PCP selector is not a
    standard part of the APP managed object, therefore it is
    encapsulated in a new non-std extension attribute.

Selector trust:
  - ASIC's often has the notion of trust DSCP and trust PCP. The new
    attribute makes it possible to specify a trust order of app
    selectors, which drivers can then react on.

DCB-enable sparx5 driver:
 - Now supports offloading of DSCP, PCP and default priority. Only one
   mapping of protocol:priority is allowed. Consecutive mappings of the
   same protocol to some new priority, will overwrite the previous. This
   is to keep a consistent view of the app table and the hardware.
 - Now supports dscp and pcp trust, by use of the introduced
   dcbnl_set/getapptrust ops. Sparx5 supports trust orders: [], [dscp],
   [pcp] and [dscp, pcp]. For now, only DSCP and PCP selectors are
   supported by the driver, everything else is bounced.

Patch #1 introduces a new PCP selector to the APP object, which makes it
possible to encode PCP and DEI in the app triplet and offload it to the
PCP table of the ASIC.

Patch #2 Introduces the new extension attributes
DCB_ATTR_DCB_APP_TRUST_TABLE and DCB_ATTR_DCB_APP_TRUST. Trusted
selectors are passed in the nested DCB_ATTR_DCB_APP_TRUST_TABLE
attribute, and assembled into an array of selectors:

  u8 selectors[256];

where lower indexes has higher precedence.  In the array, selectors are
stored consecutively, starting from index zero. With a maximum number of
256 unique selectors, the list has the same maximum size.

Patch #3 Sets up the dcbnl ops hook, and adds support for offloading pcp
app entries, to the PCP table of the switch.

Patch #4 Makes use of the dcbnl_set/getapptrust ops, to set a per-port
trust order.

Patch #5 Adds support for offloading dscp app entries to the DSCP table
of the switch.

Patch #6 Adds support for offloading default prio app entries to the
switch.

================================================================================

RFC v1:
https://lore.kernel.org/netdev/20220908120442.3069771-1-daniel.machon@microchip.com/

RFC v1 -> RFC v2:
  - Added new nested attribute type DCB_ATTR_DCB_APP_TRUST_TABLE.
  - Renamed attributes from DCB_ATTR_IEEE_* to DCB_ATTR_DCB_*.
  - Renamed ieee_set/getapptrust to dcbnl_set/getapptrust.
  - Added -EOPNOTSUPP if dcbnl_setapptrust is not set.
  - Added sanitization of selector array, before passing to driver.

RFC v2 -> (non-RFC) v1
 - Added additional check for selector validity.
 - Fixed a few style errors.
 - using nla_start_nest() instead of nla_start_nest_no_flag().
 - Moved DCB_ATTR_DCB_APP_TRUST into new enum.
 - Added new DCB_ATTR_DCB_APP extension attribute, for non-std selector
   values.
 - Added support for offloading dscp, pcp and default prio in the sparx5
   driver.
 - Added support for per-selector trust and trust order in the sparx5
   driver.

v1 -> v2
  - Fixed compiler and kdoc warning

Daniel Machon (6):
  net: dcb: add new pcp selector to app object
  net: dcb: add new apptrust attribute
  net: microchip: sparx5: add support for offloading pcp table
  net: microchip: sparx5: add support for apptrust
  net: microchip: sparx5: add support for offloading dscp table
  net: microchip: sparx5: add support for offloading default prio

 .../net/ethernet/microchip/sparx5/Makefile    |   2 +-
 .../ethernet/microchip/sparx5/sparx5_dcb.c    | 287 ++++++++++++++++++
 .../ethernet/microchip/sparx5/sparx5_main.h   |   4 +
 .../microchip/sparx5/sparx5_main_regs.h       | 127 +++++++-
 .../ethernet/microchip/sparx5/sparx5_netdev.c |   1 +
 .../ethernet/microchip/sparx5/sparx5_port.c   |  99 ++++++
 .../ethernet/microchip/sparx5/sparx5_port.h   |  33 ++
 .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
 include/net/dcbnl.h                           |   4 +
 include/uapi/linux/dcbnl.h                    |  15 +
 net/dcb/dcbnl.c                               | 126 +++++++-
 11 files changed, 691 insertions(+), 11 deletions(-)
 create mode 100644 drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c

--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Daniel Machon]

Add new PCP selector for the 8021Qaz APP managed object.

As the PCP selector is not part of the 8021Qaz standard, a new non-std
extension attribute DCB_ATTR_DCB_APP has been introduced. Also two
helper functions to translate between selector and app attribute type
has been added.

The purpose of adding the PCP selector, is to be able to offload
PCP-based queue classification to the 8021Q Priority Code Point table,
see 6.9.3 of IEEE Std 802.1Q-2018.

PCP and DEI is encoded in the protocol field as 8*dei+pcp, so that a
mapping of PCP 2 and DEI 1 to priority 3 is encoded as {255, 10, 3}.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 include/uapi/linux/dcbnl.h |  6 +++++
 net/dcb/dcbnl.c            | 49 ++++++++++++++++++++++++++++++++++----
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
index a791a94013a6..9f68dc501cc1 100644
--- a/include/uapi/linux/dcbnl.h
+++ b/include/uapi/linux/dcbnl.h
@@ -218,6 +218,9 @@ struct cee_pfc {
 #define IEEE_8021QAZ_APP_SEL_ANY	4
 #define IEEE_8021QAZ_APP_SEL_DSCP       5

+/* Non-std selector values */
+#define DCB_APP_SEL_PCP		24
+
 /* This structure contains the IEEE 802.1Qaz APP managed object. This
  * object is also used for the CEE std as well.
  *
@@ -247,6 +250,8 @@ struct dcb_app {
 	__u16	protocol;
 };

+#define IEEE_8021QAZ_APP_SEL_MAX 255
+
 /**
  * struct dcb_peer_app_info - APP feature information sent by the peer
  *
@@ -425,6 +430,7 @@ enum ieee_attrs {
 enum ieee_attrs_app {
 	DCB_ATTR_IEEE_APP_UNSPEC,
 	DCB_ATTR_IEEE_APP,
+	DCB_ATTR_DCB_APP,
 	__DCB_ATTR_IEEE_APP_MAX
 };
 #define DCB_ATTR_IEEE_APP_MAX (__DCB_ATTR_IEEE_APP_MAX - 1)
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index dc4fb699b56c..580d26acfc84 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -179,6 +179,46 @@ static const struct nla_policy dcbnl_featcfg_nest[DCB_FEATCFG_ATTR_MAX + 1] = {
 static LIST_HEAD(dcb_app_list);
 static DEFINE_SPINLOCK(dcb_lock);

+static int dcbnl_app_attr_type_get(u8 selector)
+{
+	enum ieee_attrs_app type;
+
+	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:
+		type = DCB_ATTR_IEEE_APP;
+		break;
+	case DCB_APP_SEL_PCP:
+		type = DCB_ATTR_DCB_APP;
+		break;
+	default:
+		type = DCB_ATTR_IEEE_APP_UNSPEC;
+		break;
+	}
+
+	return type;
+}
+
+static int dcbnl_app_attr_type_validate(enum ieee_attrs_app type)
+{
+	bool ret;
+
+	switch (type) {
+	case DCB_ATTR_IEEE_APP:
+	case DCB_ATTR_DCB_APP:
+		ret = true;
+		break;
+	default:
+		ret = false;
+		break;
+	}
+
+	return ret;
+}
+
 static struct sk_buff *dcbnl_newmsg(int type, u8 cmd, u32 port, u32 seq,
 				    u32 flags, struct nlmsghdr **nlhp)
 {
@@ -1116,8 +1156,9 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
 	spin_lock_bh(&dcb_lock);
 	list_for_each_entry(itr, &dcb_app_list, list) {
 		if (itr->ifindex == netdev->ifindex) {
-			err = nla_put(skb, DCB_ATTR_IEEE_APP, sizeof(itr->app),
-					 &itr->app);
+			enum ieee_attrs_app type =
+				dcbnl_app_attr_type_get(itr->app.selector);
+			err = nla_put(skb, type, sizeof(itr->app), &itr->app);
 			if (err) {
 				spin_unlock_bh(&dcb_lock);
 				return -EMSGSIZE;
@@ -1495,7 +1536,7 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
 		nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
 			struct dcb_app *app_data;

-			if (nla_type(attr) != DCB_ATTR_IEEE_APP)
+			if (!dcbnl_app_attr_type_validate(nla_type(attr)))
 				continue;

 			if (nla_len(attr) < sizeof(struct dcb_app)) {
@@ -1556,7 +1597,7 @@ static int dcbnl_ieee_del(struct net_device *netdev, struct nlmsghdr *nlh,
 		nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
 			struct dcb_app *app_data;

-			if (nla_type(attr) != DCB_ATTR_IEEE_APP)
+			if (!dcbnl_app_attr_type_validate(nla_type(attr)))
 				continue;
 			app_data = nla_data(attr);
 			if (ops->ieee_delapp)
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v2 2/6] net: dcb: add new apptrust attribute

[Daniel Machon]

Add new apptrust extension attributes to the 8021Qaz APP managed object.

Two new attributes, DCB_ATTR_DCB_APP_TRUST_TABLE and
DCB_ATTR_DCB_APP_TRUST, has been added. Trusted selectors are passed in
the nested attribute DCB_ATTR_DCB_APP_TRUST, in order of precedence.

The new attributes are meant to allow drivers, whose hw supports the
notion of trust, to be able to set whether a particular app selector is
trusted - and in which order.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 include/net/dcbnl.h        |  4 ++
 include/uapi/linux/dcbnl.h |  9 +++++
 net/dcb/dcbnl.c            | 77 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/include/net/dcbnl.h b/include/net/dcbnl.h
index 2b2d86fb3131..8841ab6c2de7 100644
--- a/include/net/dcbnl.h
+++ b/include/net/dcbnl.h
@@ -109,6 +109,10 @@ struct dcbnl_rtnl_ops {
 	/* buffer settings */
 	int (*dcbnl_getbuffer)(struct net_device *, struct dcbnl_buffer *);
 	int (*dcbnl_setbuffer)(struct net_device *, struct dcbnl_buffer *);
+
+	/* apptrust */
+	int (*dcbnl_setapptrust)(struct net_device *, u8 *, int);
+	int (*dcbnl_getapptrust)(struct net_device *, u8 *, int *);
 };

 #endif /* __NET_DCBNL_H__ */
diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
index 9f68dc501cc1..f892cd945695 100644
--- a/include/uapi/linux/dcbnl.h
+++ b/include/uapi/linux/dcbnl.h
@@ -410,6 +410,7 @@ enum dcbnl_attrs {
  * @DCB_ATTR_IEEE_PEER_ETS: peer ETS configuration - get only
  * @DCB_ATTR_IEEE_PEER_PFC: peer PFC configuration - get only
  * @DCB_ATTR_IEEE_PEER_APP: peer APP tlv - get only
+ * @DCB_ATTR_DCB_APP_TRUST_TABLE: selector trust order
  */
 enum ieee_attrs {
 	DCB_ATTR_IEEE_UNSPEC,
@@ -423,6 +424,7 @@ enum ieee_attrs {
 	DCB_ATTR_IEEE_QCN,
 	DCB_ATTR_IEEE_QCN_STATS,
 	DCB_ATTR_DCB_BUFFER,
+	DCB_ATTR_DCB_APP_TRUST_TABLE,
 	__DCB_ATTR_IEEE_MAX
 };
 #define DCB_ATTR_IEEE_MAX (__DCB_ATTR_IEEE_MAX - 1)
@@ -435,6 +437,13 @@ enum ieee_attrs_app {
 };
 #define DCB_ATTR_IEEE_APP_MAX (__DCB_ATTR_IEEE_APP_MAX - 1)

+enum dcbnl_attrs_apptrust {
+	DCB_ATTR_DCB_APP_TRUST_UNSPEC,
+	DCB_ATTR_DCB_APP_TRUST,
+	__DCB_ATTR_DCB_APP_TRUST_MAX
+};
+#define DCB_ATTR_DCB_APP_TRUST_MAX (__DCB_ATTR_DCB_APP_TRUST_MAX - 1)
+
 /**
  * enum cee_attrs - CEE DCBX get attributes.
  *
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index 580d26acfc84..ad84f70e3eb3 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -166,6 +166,7 @@ static const struct nla_policy dcbnl_ieee_policy[DCB_ATTR_IEEE_MAX + 1] = {
 	[DCB_ATTR_IEEE_QCN]         = {.len = sizeof(struct ieee_qcn)},
 	[DCB_ATTR_IEEE_QCN_STATS]   = {.len = sizeof(struct ieee_qcn_stats)},
 	[DCB_ATTR_DCB_BUFFER]       = {.len = sizeof(struct dcbnl_buffer)},
+	[DCB_ATTR_DCB_APP_TRUST_TABLE] = {.type = NLA_NESTED},
 };

 /* DCB number of traffic classes nested attributes. */
@@ -1070,11 +1071,11 @@ static int dcbnl_build_peer_app(struct net_device *netdev, struct sk_buff* skb,
 /* Handle IEEE 802.1Qaz/802.1Qau/802.1Qbb GET commands. */
 static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
 {
-	struct nlattr *ieee, *app;
-	struct dcb_app_type *itr;
 	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
+	struct nlattr *ieee, *app, *apptrust;
+	struct dcb_app_type *itr;
+	int err, i;
 	int dcbx;
-	int err;

 	if (nla_put_string(skb, DCB_ATTR_IFNAME, netdev->name))
 		return -EMSGSIZE;
@@ -1174,6 +1175,24 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
 	spin_unlock_bh(&dcb_lock);
 	nla_nest_end(skb, app);

+	if (ops->dcbnl_getapptrust) {
+		u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0};
+		int nselectors;
+
+		apptrust = nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE);
+		if (!app)
+			return -EMSGSIZE;
+
+		err = ops->dcbnl_getapptrust(netdev, selectors, &nselectors);
+		if (err)
+			return -EMSGSIZE;
+
+		for (i = 0; i < nselectors; i++)
+			nla_put_u8(skb, DCB_ATTR_DCB_APP_TRUST, selectors[i]);
+
+		nla_nest_end(skb, apptrust);
+	}
+
 	/* get peer info if available */
 	if (ops->ieee_peer_getets) {
 		struct ieee_ets ets;
@@ -1467,8 +1486,8 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
 {
 	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
 	struct nlattr *ieee[DCB_ATTR_IEEE_MAX + 1];
+	int err, i;
 	int prio;
-	int err;

 	if (!ops)
 		return -EOPNOTSUPP;
@@ -1554,6 +1573,56 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
 		}
 	}

+	if (ieee[DCB_ATTR_DCB_APP_TRUST_TABLE]) {
+		u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0};
+		struct nlattr *attr;
+		int nselectors = 0;
+		u8 selector;
+		int rem;
+
+		if (!ops->dcbnl_setapptrust) {
+			err = -EOPNOTSUPP;
+			goto err;
+		}
+
+		nla_for_each_nested(attr, ieee[DCB_ATTR_DCB_APP_TRUST_TABLE],
+				    rem) {
+			if (nla_type(attr) != DCB_ATTR_DCB_APP_TRUST ||
+			    nla_len(attr) != 1 ||
+			    nselectors >= sizeof(selectors)) {
+				err = -EINVAL;
+				goto err;
+			}
+
+			selector = nla_get_u8(attr);
+			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:
+			case DCB_APP_SEL_PCP:
+				break;
+			default:
+				err = -EINVAL;
+				goto err;
+			}
+			/* Duplicate selector ? */
+			for (i = 0; i < nselectors; i++) {
+				if (selectors[i] == selector) {
+					err = -EINVAL;
+					goto err;
+				}
+			}
+
+			selectors[nselectors++] = selector;
+		}
+
+		err = ops->dcbnl_setapptrust(netdev, selectors, nselectors);
+		if (err)
+			goto err;
+	}
+
 err:
 	err = nla_put_u8(skb, DCB_ATTR_IEEE, err);
 	dcbnl_ieee_notify(netdev, RTM_SETDCB, DCB_CMD_IEEE_SET, seq, 0);
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v2 3/6] net: microchip: sparx5: add support for offloading pcp table

[Daniel Machon]

Add new registers and functions to support offload of pcp app entries.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 .../net/ethernet/microchip/sparx5/Makefile    |   2 +-
 .../ethernet/microchip/sparx5/sparx5_dcb.c    | 101 ++++++++++++++
 .../ethernet/microchip/sparx5/sparx5_main.h   |   1 +
 .../microchip/sparx5/sparx5_main_regs.h       | 127 +++++++++++++++++-
 .../ethernet/microchip/sparx5/sparx5_netdev.c |   1 +
 .../ethernet/microchip/sparx5/sparx5_port.c   |  37 +++++
 .../ethernet/microchip/sparx5/sparx5_port.h   |  17 +++
 7 files changed, 283 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c

diff --git a/drivers/net/ethernet/microchip/sparx5/Makefile b/drivers/net/ethernet/microchip/sparx5/Makefile
index d1c6ad966747..e3460e16f8fc 100644
--- a/drivers/net/ethernet/microchip/sparx5/Makefile
+++ b/drivers/net/ethernet/microchip/sparx5/Makefile
@@ -8,4 +8,4 @@ obj-$(CONFIG_SPARX5_SWITCH) += sparx5-switch.o
 sparx5-switch-objs  := sparx5_main.o sparx5_packet.o \
  sparx5_netdev.o sparx5_phylink.o sparx5_port.o sparx5_mactable.o sparx5_vlan.o \
  sparx5_switchdev.o sparx5_calendar.o sparx5_ethtool.o sparx5_fdma.o \
- sparx5_ptp.o sparx5_pgid.o sparx5_tc.o sparx5_qos.o
+ sparx5_ptp.o sparx5_pgid.o sparx5_tc.o sparx5_qos.o sparx5_dcb.o
\ No newline at end of file
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
new file mode 100644
index 000000000000..db17c124dac8
--- /dev/null
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Microchip Sparx5 Switch driver
+ *
+ * Copyright (c) 2022 Microchip Technology Inc. and its subsidiaries.
+ */
+
+#include <net/dcbnl.h>
+
+#include "sparx5_port.h"
+
+/* Validate app entry.
+ *
+ * Check for valid selectors and valid protocol and priority ranges.
+ */
+static int sparx5_dcb_app_validate(struct net_device *dev,
+				   const struct dcb_app *app)
+{
+	int err = 0;
+
+	switch (app->selector) {
+	/* Pcp checks */
+	case DCB_APP_SEL_PCP:
+		if (app->protocol > 15)
+			err = -EINVAL;
+		else if (app->priority >= SPX5_PRIOS)
+			err = -ERANGE;
+		break;
+	default:
+		err = -EINVAL;
+		break;
+	}
+
+	if (err)
+		netdev_err(dev, "Invalid entry: %d:%d\n", app->protocol,
+			   app->priority);
+
+	return err;
+}
+
+static int sparx5_dcb_app_update(struct net_device *dev)
+{
+	struct dcb_app app_itr = { .selector = DCB_APP_SEL_PCP };
+	struct sparx5_port *port = netdev_priv(dev);
+	struct sparx5_port_qos_pcp_map *pcp_map;
+	struct sparx5_port_qos qos = {0};
+	int i;
+
+	pcp_map = &qos.pcp.map;
+
+	/* Get pcp ingress mapping */
+	for (i = 0; i < ARRAY_SIZE(pcp_map->map); i++) {
+		app_itr.protocol = i;
+		pcp_map->map[i] = dcb_getapp(dev, &app_itr);
+	}
+
+	return sparx5_port_qos_set(port, &qos);
+}
+
+static int sparx5_dcb_ieee_setapp(struct net_device *dev, struct dcb_app *app)
+{
+	struct dcb_app app_itr;
+	int err = 0;
+	u8 prio;
+
+	err = sparx5_dcb_app_validate(dev, app);
+	if (err)
+		goto out;
+
+	/* Delete current mapping, if it exists */
+	prio = dcb_getapp(dev, app);
+	if (prio) {
+		app_itr = *app;
+		app_itr.priority = prio;
+		dcb_ieee_delapp(dev, &app_itr);
+	}
+
+	err = dcb_ieee_setapp(dev, app);
+	if (err)
+		goto out;
+
+	sparx5_dcb_app_update(dev);
+
+out:
+	return err;
+}
+
+static int sparx5_dcb_ieee_delapp(struct net_device *dev, struct dcb_app *app)
+{
+	int err;
+
+	err = dcb_ieee_delapp(dev, app);
+	if (err < 0)
+		return err;
+
+	return sparx5_dcb_app_update(dev);
+}
+
+const struct dcbnl_rtnl_ops sparx5_dcbnl_ops = {
+	.ieee_setapp = sparx5_dcb_ieee_setapp,
+	.ieee_delapp = sparx5_dcb_ieee_delapp,
+};
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 8b42cad0e49c..0d8e04c64584 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -418,6 +418,7 @@ static inline bool sparx5_is_baser(phy_interface_t interface)
 extern const struct phylink_mac_ops sparx5_phylink_mac_ops;
 extern const struct phylink_pcs_ops sparx5_phylink_pcs_ops;
 extern const struct ethtool_ops sparx5_ethtool_ops;
+extern const struct dcbnl_rtnl_ops sparx5_dcbnl_ops;

 /* Calculate raw offset */
 static inline __pure int spx5_offset(int id, int tinst, int tcnt,
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
index fa2eb70f487a..e8c3a0d6074f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
@@ -4,8 +4,8 @@
  * Copyright (c) 2021 Microchip Technology Inc.
  */

-/* This file is autogenerated by cml-utils 2022-02-26 14:15:01 +0100.
- * Commit ID: 98bdd3d171cc2a1afd30d241d41a4281d471a48c (dirty)
+/* This file is autogenerated by cml-utils 2022-09-28 11:17:02 +0200.
+ * Commit ID: 385c8a11d71a9f6a60368d3a3cb648fa257b479a
  */

 #ifndef _SPARX5_MAIN_REGS_H_
@@ -426,6 +426,96 @@ enum sparx5_target {
 #define ANA_CL_VLAN_CTRL_2_VLAN_PUSH_CNT_GET(x)\
 	FIELD_GET(ANA_CL_VLAN_CTRL_2_VLAN_PUSH_CNT, x)

+/*      ANA_CL:PORT:PCP_DEI_MAP_CFG */
+#define ANA_CL_PCP_DEI_MAP_CFG(g, r) __REG(TARGET_ANA_CL, 0, 1, 131072, g, 70, 512, 108, r, 16, 4)
+
+#define ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_DP_VAL    GENMASK(4, 3)
+#define ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_DP_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_DP_VAL, x)
+#define ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_DP_VAL_GET(x)\
+	FIELD_GET(ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_DP_VAL, x)
+
+#define ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_QOS_VAL   GENMASK(2, 0)
+#define ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_QOS_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_QOS_VAL, x)
+#define ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_QOS_VAL_GET(x)\
+	FIELD_GET(ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_QOS_VAL, x)
+
+/*      ANA_CL:PORT:QOS_CFG */
+#define ANA_CL_QOS_CFG(g)         __REG(TARGET_ANA_CL, 0, 1, 131072, g, 70, 512, 172, 0, 1, 4)
+
+#define ANA_CL_QOS_CFG_DEFAULT_COSID_ENA         BIT(17)
+#define ANA_CL_QOS_CFG_DEFAULT_COSID_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DEFAULT_COSID_ENA, x)
+#define ANA_CL_QOS_CFG_DEFAULT_COSID_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DEFAULT_COSID_ENA, x)
+
+#define ANA_CL_QOS_CFG_DEFAULT_COSID_VAL         GENMASK(16, 14)
+#define ANA_CL_QOS_CFG_DEFAULT_COSID_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DEFAULT_COSID_VAL, x)
+#define ANA_CL_QOS_CFG_DEFAULT_COSID_VAL_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DEFAULT_COSID_VAL, x)
+
+#define ANA_CL_QOS_CFG_DSCP_REWR_MODE_SEL        GENMASK(13, 12)
+#define ANA_CL_QOS_CFG_DSCP_REWR_MODE_SEL_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DSCP_REWR_MODE_SEL, x)
+#define ANA_CL_QOS_CFG_DSCP_REWR_MODE_SEL_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DSCP_REWR_MODE_SEL, x)
+
+#define ANA_CL_QOS_CFG_DSCP_TRANSLATE_ENA        BIT(11)
+#define ANA_CL_QOS_CFG_DSCP_TRANSLATE_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DSCP_TRANSLATE_ENA, x)
+#define ANA_CL_QOS_CFG_DSCP_TRANSLATE_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DSCP_TRANSLATE_ENA, x)
+
+#define ANA_CL_QOS_CFG_DSCP_KEEP_ENA             BIT(10)
+#define ANA_CL_QOS_CFG_DSCP_KEEP_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DSCP_KEEP_ENA, x)
+#define ANA_CL_QOS_CFG_DSCP_KEEP_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DSCP_KEEP_ENA, x)
+
+#define ANA_CL_QOS_CFG_KEEP_ENA                  BIT(9)
+#define ANA_CL_QOS_CFG_KEEP_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_KEEP_ENA, x)
+#define ANA_CL_QOS_CFG_KEEP_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_KEEP_ENA, x)
+
+#define ANA_CL_QOS_CFG_PCP_DEI_DP_ENA            BIT(8)
+#define ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_PCP_DEI_DP_ENA, x)
+#define ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_PCP_DEI_DP_ENA, x)
+
+#define ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA           BIT(7)
+#define ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA, x)
+#define ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA, x)
+
+#define ANA_CL_QOS_CFG_DSCP_DP_ENA               BIT(6)
+#define ANA_CL_QOS_CFG_DSCP_DP_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DSCP_DP_ENA, x)
+#define ANA_CL_QOS_CFG_DSCP_DP_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DSCP_DP_ENA, x)
+
+#define ANA_CL_QOS_CFG_DSCP_QOS_ENA              BIT(5)
+#define ANA_CL_QOS_CFG_DSCP_QOS_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DSCP_QOS_ENA, x)
+#define ANA_CL_QOS_CFG_DSCP_QOS_ENA_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DSCP_QOS_ENA, x)
+
+#define ANA_CL_QOS_CFG_DEFAULT_DP_VAL            GENMASK(4, 3)
+#define ANA_CL_QOS_CFG_DEFAULT_DP_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DEFAULT_DP_VAL, x)
+#define ANA_CL_QOS_CFG_DEFAULT_DP_VAL_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DEFAULT_DP_VAL, x)
+
+#define ANA_CL_QOS_CFG_DEFAULT_QOS_VAL           GENMASK(2, 0)
+#define ANA_CL_QOS_CFG_DEFAULT_QOS_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_QOS_CFG_DEFAULT_QOS_VAL, x)
+#define ANA_CL_QOS_CFG_DEFAULT_QOS_VAL_GET(x)\
+	FIELD_GET(ANA_CL_QOS_CFG_DEFAULT_QOS_VAL, x)
+
 /*      ANA_CL:PORT:CAPTURE_BPDU_CFG */
 #define ANA_CL_CAPTURE_BPDU_CFG(g) __REG(TARGET_ANA_CL, 0, 1, 131072, g, 70, 512, 196, 0, 1, 4)

@@ -438,6 +528,39 @@ enum sparx5_target {
 #define ANA_CL_OWN_UPSID_OWN_UPSID_GET(x)\
 	FIELD_GET(ANA_CL_OWN_UPSID_OWN_UPSID, x)

+/*      ANA_CL:COMMON:DSCP_CFG */
+#define ANA_CL_DSCP_CFG(r)        __REG(TARGET_ANA_CL, 0, 1, 166912, 0, 1, 756, 256, r, 64, 4)
+
+#define ANA_CL_DSCP_CFG_DSCP_TRANSLATE_VAL       GENMASK(12, 7)
+#define ANA_CL_DSCP_CFG_DSCP_TRANSLATE_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_DSCP_CFG_DSCP_TRANSLATE_VAL, x)
+#define ANA_CL_DSCP_CFG_DSCP_TRANSLATE_VAL_GET(x)\
+	FIELD_GET(ANA_CL_DSCP_CFG_DSCP_TRANSLATE_VAL, x)
+
+#define ANA_CL_DSCP_CFG_DSCP_QOS_VAL             GENMASK(6, 4)
+#define ANA_CL_DSCP_CFG_DSCP_QOS_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_DSCP_CFG_DSCP_QOS_VAL, x)
+#define ANA_CL_DSCP_CFG_DSCP_QOS_VAL_GET(x)\
+	FIELD_GET(ANA_CL_DSCP_CFG_DSCP_QOS_VAL, x)
+
+#define ANA_CL_DSCP_CFG_DSCP_DP_VAL              GENMASK(3, 2)
+#define ANA_CL_DSCP_CFG_DSCP_DP_VAL_SET(x)\
+	FIELD_PREP(ANA_CL_DSCP_CFG_DSCP_DP_VAL, x)
+#define ANA_CL_DSCP_CFG_DSCP_DP_VAL_GET(x)\
+	FIELD_GET(ANA_CL_DSCP_CFG_DSCP_DP_VAL, x)
+
+#define ANA_CL_DSCP_CFG_DSCP_REWR_ENA            BIT(1)
+#define ANA_CL_DSCP_CFG_DSCP_REWR_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_DSCP_CFG_DSCP_REWR_ENA, x)
+#define ANA_CL_DSCP_CFG_DSCP_REWR_ENA_GET(x)\
+	FIELD_GET(ANA_CL_DSCP_CFG_DSCP_REWR_ENA, x)
+
+#define ANA_CL_DSCP_CFG_DSCP_TRUST_ENA           BIT(0)
+#define ANA_CL_DSCP_CFG_DSCP_TRUST_ENA_SET(x)\
+	FIELD_PREP(ANA_CL_DSCP_CFG_DSCP_TRUST_ENA, x)
+#define ANA_CL_DSCP_CFG_DSCP_TRUST_ENA_GET(x)\
+	FIELD_GET(ANA_CL_DSCP_CFG_DSCP_TRUST_ENA, x)
+
 /*      ANA_L2:COMMON:AUTO_LRN_CFG */
 #define ANA_L2_AUTO_LRN_CFG       __REG(TARGET_ANA_L2, 0, 1, 566024, 0, 1, 700, 24, 0, 1, 4)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
index 19516ccad533..afe117736e25 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
@@ -258,6 +258,7 @@ struct net_device *sparx5_create_netdev(struct sparx5 *sparx5, u32 portno)

 	ndev->netdev_ops = &sparx5_port_netdev_ops;
 	ndev->ethtool_ops = &sparx5_ethtool_ops;
+	ndev->dcbnl_ops = &sparx5_dcbnl_ops;

 	eth_hw_addr_gen(ndev, sparx5->base_mac, portno + 1);

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index 32709d21ab2f..9ffaaf34d196 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -6,6 +6,7 @@

 #include <linux/module.h>
 #include <linux/phy/phy.h>
+#include <net/dcbnl.h>

 #include "sparx5_main_regs.h"
 #include "sparx5_main.h"
@@ -1144,3 +1145,39 @@ void sparx5_port_enable(struct sparx5_port *port, bool enable)
 		 sparx5,
 		 QFWD_SWITCH_PORT_MODE(port->portno));
 }
+
+int sparx5_port_qos_set(struct sparx5_port *port,
+			struct sparx5_port_qos *qos)
+{
+	sparx5_port_qos_pcp_set(port, &qos->pcp);
+
+	return 0;
+}
+
+int sparx5_port_qos_pcp_set(const struct sparx5_port *port,
+			    struct sparx5_port_qos_pcp *qos)
+{
+	struct sparx5 *sparx5 = port->sparx5;
+	u8 *pcp_itr = qos->map.map;
+	u8 pcp, dp;
+	int i;
+
+	/* Enable/disable pcp and dp for qos classification. */
+	spx5_rmw(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(1) |
+		 ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(1),
+		 ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA | ANA_CL_QOS_CFG_PCP_DEI_DP_ENA,
+		 sparx5, ANA_CL_QOS_CFG(port->portno));
+
+	/* Map each pcp and dei value to priority and dp */
+	for (i = 0; i < ARRAY_SIZE(qos->map.map); i++) {
+		pcp = *(pcp_itr + i);
+		dp = (i <= 7) ? 0 : 1;
+		spx5_rmw(ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_QOS_VAL_SET(pcp) |
+			 ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_DP_VAL_SET(dp),
+			 ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_QOS_VAL |
+			 ANA_CL_PCP_DEI_MAP_CFG_PCP_DEI_DP_VAL, sparx5,
+			 ANA_CL_PCP_DEI_MAP_CFG(port->portno, i));
+	}
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
index 2f8043eac71b..9c5fb6b651db 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
@@ -91,4 +91,21 @@ int sparx5_get_port_status(struct sparx5 *sparx5,
 void sparx5_port_enable(struct sparx5_port *port, bool enable);
 int sparx5_port_fwd_urg(struct sparx5 *sparx5, u32 speed);

+struct sparx5_port_qos_pcp_map {
+	u8 map[16];
+};
+
+struct sparx5_port_qos_pcp {
+	struct sparx5_port_qos_pcp_map map;
+};
+
+struct sparx5_port_qos {
+	struct sparx5_port_qos_pcp pcp;
+};
+
+int sparx5_port_qos_set(struct sparx5_port *port, struct sparx5_port_qos *qos);
+
+int sparx5_port_qos_pcp_set(const struct sparx5_port *port,
+			    struct sparx5_port_qos_pcp *qos);
+
 #endif	/* __SPARX5_PORT_H__ */
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v2 4/6] net: microchip: sparx5: add support for apptrust

[Daniel Machon]

Make use of set/getapptrust() to implement per-selector trust and trust
order.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
 .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
 .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
 .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
 .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
 5 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
index db17c124dac8..10aeb422b1ae 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
@@ -8,6 +8,22 @@

 #include "sparx5_port.h"

+static const struct sparx5_dcb_apptrust {
+	u8 selectors[256];
+	int nselectors;
+	char *names;
+} *apptrust[SPX5_PORTS];
+
+/* Sparx5 supported apptrust configurations */
+static const struct sparx5_dcb_apptrust apptrust_conf[4] = {
+	/* Empty *must* be first */
+	{ { 0                         }, 0, "empty"    },
+	{ { IEEE_8021QAZ_APP_SEL_DSCP }, 1, "dscp"     },
+	{ { DCB_APP_SEL_PCP           }, 1, "pcp"      },
+	{ { IEEE_8021QAZ_APP_SEL_DSCP,
+	    DCB_APP_SEL_PCP           }, 2, "dscp pcp" },
+};
+
 /* Validate app entry.
  *
  * Check for valid selectors and valid protocol and priority ranges.
@@ -37,12 +53,59 @@ static int sparx5_dcb_app_validate(struct net_device *dev,
 	return err;
 }

+/* Validate apptrust configuration.
+ *
+ * Return index of supported apptrust configuration if valid, otherwise return
+ * error.
+ */
+static int sparx5_dcb_apptrust_validate(struct net_device *dev, u8 *selectors,
+					int nselectors, int *err)
+{
+	bool match;
+	int i, ii;
+
+	for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++) {
+		match = true;
+		for (ii = 0; ii < nselectors; ii++) {
+			if (apptrust_conf[i].selectors[ii] !=
+			    *(selectors + ii)) {
+				match = false;
+				break;
+			}
+		}
+		if (match)
+			break;
+	}
+
+	/* Requested trust configuration is not supported */
+	if (!match) {
+		netdev_err(dev, "Valid apptrust configurations are:\n");
+		for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++)
+			pr_info("order: %s\n", apptrust_conf[i].names);
+		*err = -EOPNOTSUPP;
+	}
+
+	return i;
+}
+
+static bool sparx5_dcb_apptrust_contains(int portno, u8 selector)
+{
+	int i;
+
+	for (i = 0; i < IEEE_8021QAZ_APP_SEL_MAX + 1; i++)
+		if (apptrust[portno]->selectors[i] == selector)
+			return true;
+
+	return false;
+}
+
 static int sparx5_dcb_app_update(struct net_device *dev)
 {
 	struct dcb_app app_itr = { .selector = DCB_APP_SEL_PCP };
 	struct sparx5_port *port = netdev_priv(dev);
 	struct sparx5_port_qos_pcp_map *pcp_map;
 	struct sparx5_port_qos qos = {0};
+	int portno = port->portno;
 	int i;

 	pcp_map = &qos.pcp.map;
@@ -53,6 +116,12 @@ static int sparx5_dcb_app_update(struct net_device *dev)
 		pcp_map->map[i] = dcb_getapp(dev, &app_itr);
 	}

+	/* Enable use of pcp for queue classification ? */
+	if (sparx5_dcb_apptrust_contains(portno, DCB_APP_SEL_PCP)) {
+		qos.pcp.qos_enable = true;
+		qos.pcp.dp_enable = qos.pcp.qos_enable;
+	}
+
 	return sparx5_port_qos_set(port, &qos);
 }

@@ -95,7 +164,54 @@ static int sparx5_dcb_ieee_delapp(struct net_device *dev, struct dcb_app *app)
 	return sparx5_dcb_app_update(dev);
 }

+static int sparx5_dcb_setapptrust(struct net_device *dev, u8 *selectors,
+				  int nselectors)
+{
+	struct sparx5_port *port = netdev_priv(dev);
+	int err = 0, idx;
+
+	idx = sparx5_dcb_apptrust_validate(dev, selectors, nselectors, &err);
+	if (err < 0)
+		return err;
+
+	apptrust[port->portno] = &apptrust_conf[idx];
+
+	return sparx5_dcb_app_update(dev);
+}
+
+static int sparx5_dcb_getapptrust(struct net_device *dev, u8 *selectors,
+				  int *nselectors)
+{
+	struct sparx5_port *port = netdev_priv(dev);
+	const struct sparx5_dcb_apptrust *trust;
+
+	trust = apptrust[port->portno];
+
+	memcpy(selectors, trust->selectors, trust->nselectors);
+	*nselectors = trust->nselectors;
+
+	return 0;
+}
+
+int sparx5_dcb_init(struct sparx5 *sparx5)
+{
+	struct sparx5_port *port;
+	int i;
+
+	for (i = 0; i < SPX5_PORTS; i++) {
+		port = sparx5->ports[i];
+		if (!port)
+			continue;
+		/* Initialize [dscp, pcp] default trust */
+		apptrust[port->portno] = &apptrust_conf[3];
+	}
+
+	return sparx5_dcb_app_update(port->ndev);
+}
+
 const struct dcbnl_rtnl_ops sparx5_dcbnl_ops = {
 	.ieee_setapp = sparx5_dcb_ieee_setapp,
 	.ieee_delapp = sparx5_dcb_ieee_delapp,
+	.dcbnl_setapptrust = sparx5_dcb_setapptrust,
+	.dcbnl_getapptrust = sparx5_dcb_getapptrust,
 };
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 0d8e04c64584..d07ef2e8b321 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -357,6 +357,9 @@ int sparx5_config_dsm_calendar(struct sparx5 *sparx5);
 void sparx5_get_stats64(struct net_device *ndev, struct rtnl_link_stats64 *stats);
 int sparx_stats_init(struct sparx5 *sparx5);

+/* sparx5_dcb.c */
+int sparx5_dcb_init(struct sparx5 *sparx5);
+
 /* sparx5_netdev.c */
 void sparx5_set_port_ifh_timestamp(void *ifh_hdr, u64 timestamp);
 void sparx5_set_port_ifh_rew_op(void *ifh_hdr, u32 rew_op);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index 9ffaaf34d196..99e86e87aa16 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -1163,8 +1163,8 @@ int sparx5_port_qos_pcp_set(const struct sparx5_port *port,
 	int i;

 	/* Enable/disable pcp and dp for qos classification. */
-	spx5_rmw(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(1) |
-		 ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(1),
+	spx5_rmw(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(qos->qos_enable) |
+		 ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(qos->dp_enable),
 		 ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA | ANA_CL_QOS_CFG_PCP_DEI_DP_ENA,
 		 sparx5, ANA_CL_QOS_CFG(port->portno));

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
index 9c5fb6b651db..fae9f5464548 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
@@ -97,6 +97,8 @@ struct sparx5_port_qos_pcp_map {

 struct sparx5_port_qos_pcp {
 	struct sparx5_port_qos_pcp_map map;
+	bool qos_enable;
+	bool dp_enable;
 };

 struct sparx5_port_qos {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
index 1e79d0ef0cb8..379e540e5e6a 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
@@ -389,6 +389,10 @@ int sparx5_qos_init(struct sparx5 *sparx5)
 	if (ret < 0)
 		return ret;

+	ret = sparx5_dcb_init(sparx5);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }

--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v2 5/6] net: microchip: sparx5: add support for offloading dscp table

[Daniel Machon]

Add support for offloading dscp app entries. Dscp values are global for
all ports on the sparx5 switch. Therefore, we replicate each dscp app
entry per-port.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 .../ethernet/microchip/sparx5/sparx5_dcb.c    | 62 ++++++++++++++++++-
 .../ethernet/microchip/sparx5/sparx5_port.c   | 39 ++++++++++++
 .../ethernet/microchip/sparx5/sparx5_port.h   |  9 +++
 3 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
index 10aeb422b1ae..0cc46672b59c 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
@@ -34,6 +34,13 @@ static int sparx5_dcb_app_validate(struct net_device *dev,
 	int err = 0;

 	switch (app->selector) {
+	/* Dscp checks */
+	case IEEE_8021QAZ_APP_SEL_DSCP:
+		if (app->protocol > 63)
+			err = -EINVAL;
+		else if (app->priority >= SPX5_PRIOS)
+			err = -ERANGE;
+		break;
 	/* Pcp checks */
 	case DCB_APP_SEL_PCP:
 		if (app->protocol > 15)
@@ -104,12 +111,20 @@ static int sparx5_dcb_app_update(struct net_device *dev)
 	struct dcb_app app_itr = { .selector = DCB_APP_SEL_PCP };
 	struct sparx5_port *port = netdev_priv(dev);
 	struct sparx5_port_qos_pcp_map *pcp_map;
+	struct dcb_ieee_app_dscp_map *dscp_map;
 	struct sparx5_port_qos qos = {0};
 	int portno = port->portno;
 	int i;

+	dscp_map = &qos.dscp.map;
 	pcp_map = &qos.pcp.map;

+	/* Get dscp ingress mapping */
+	dcb_ieee_getapp_dscp_prio_mask_map(dev, dscp_map);
+	for (i = 0; i < ARRAY_SIZE(dscp_map->map); i++)
+		if (dscp_map->map[i])
+			dscp_map->map[i] = fls(dscp_map->map[i]) - 1;
+
 	/* Get pcp ingress mapping */
 	for (i = 0; i < ARRAY_SIZE(pcp_map->map); i++) {
 		app_itr.protocol = i;
@@ -122,9 +137,44 @@ static int sparx5_dcb_app_update(struct net_device *dev)
 		qos.pcp.dp_enable = qos.pcp.qos_enable;
 	}

+	/* Enable use of dscp for queue classification ? */
+	if (sparx5_dcb_apptrust_contains(portno, IEEE_8021QAZ_APP_SEL_DSCP)) {
+		qos.dscp.qos_enable = true;
+		qos.dscp.dp_enable = qos.dscp.qos_enable;
+	}
+
 	return sparx5_port_qos_set(port, &qos);
 }

+/* Set or delete dscp app entry.
+ *
+ * Dscp mapping is global for all ports, so set and delete app entries are
+ * replicated for each port.
+ */
+static int sparx5_dcb_ieee_dscp_setdel_app(struct net_device *dev,
+					   struct dcb_app *app, bool del)
+{
+	struct sparx5_port *port = netdev_priv(dev);
+	struct dcb_app apps[SPX5_PORTS];
+	struct sparx5_port *port_itr;
+	int err, i;
+
+	for (i = 0; i < SPX5_PORTS; i++) {
+		port_itr = port->sparx5->ports[i];
+		if (!port_itr)
+			continue;
+		memcpy(&apps[i], app, sizeof(struct dcb_app));
+		if (del)
+			err = dcb_ieee_delapp(port_itr->ndev, &apps[i]);
+		else
+			err = dcb_ieee_setapp(port_itr->ndev, &apps[i]);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static int sparx5_dcb_ieee_setapp(struct net_device *dev, struct dcb_app *app)
 {
 	struct dcb_app app_itr;
@@ -143,7 +193,11 @@ static int sparx5_dcb_ieee_setapp(struct net_device *dev, struct dcb_app *app)
 		dcb_ieee_delapp(dev, &app_itr);
 	}

-	err = dcb_ieee_setapp(dev, app);
+	if (app->selector == IEEE_8021QAZ_APP_SEL_DSCP)
+		err = sparx5_dcb_ieee_dscp_setdel_app(dev, app, false);
+	else
+		err = dcb_ieee_setapp(dev, app);
+
 	if (err)
 		goto out;

@@ -157,7 +211,11 @@ static int sparx5_dcb_ieee_delapp(struct net_device *dev, struct dcb_app *app)
 {
 	int err;

-	err = dcb_ieee_delapp(dev, app);
+	if (app->selector == IEEE_8021QAZ_APP_SEL_DSCP)
+		err = sparx5_dcb_ieee_dscp_setdel_app(dev, app, true);
+	else
+		err = dcb_ieee_delapp(dev, app);
+
 	if (err < 0)
 		return err;

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index 99e86e87aa16..fb5e321c4896 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -1149,6 +1149,7 @@ void sparx5_port_enable(struct sparx5_port *port, bool enable)
 int sparx5_port_qos_set(struct sparx5_port *port,
 			struct sparx5_port_qos *qos)
 {
+	sparx5_port_qos_dscp_set(port, &qos->dscp);
 	sparx5_port_qos_pcp_set(port, &qos->pcp);

 	return 0;
@@ -1181,3 +1182,41 @@ int sparx5_port_qos_pcp_set(const struct sparx5_port *port,

 	return 0;
 }
+
+int sparx5_port_qos_dscp_set(const struct sparx5_port *port,
+			     struct sparx5_port_qos_dscp *qos)
+{
+	struct sparx5 *sparx5 = port->sparx5;
+	u8 *dscp = qos->map.map;
+	int i;
+
+	/* Enable/disable dscp and dp for qos classification.
+	 * Disable rewrite of dscp values for now.
+	 */
+	spx5_rmw(ANA_CL_QOS_CFG_DSCP_QOS_ENA_SET(qos->qos_enable) |
+		 ANA_CL_QOS_CFG_DSCP_DP_ENA_SET(qos->dp_enable) |
+		 ANA_CL_QOS_CFG_DSCP_KEEP_ENA_SET(1),
+		 ANA_CL_QOS_CFG_DSCP_QOS_ENA | ANA_CL_QOS_CFG_DSCP_DP_ENA |
+		 ANA_CL_QOS_CFG_DSCP_KEEP_ENA, sparx5,
+		 ANA_CL_QOS_CFG(port->portno));
+
+	/* Map each dscp value to priority and dp */
+	for (i = 0; i < ARRAY_SIZE(qos->map.map); i++) {
+		spx5_rmw(ANA_CL_DSCP_CFG_DSCP_QOS_VAL_SET(*(dscp + i)) |
+			 ANA_CL_DSCP_CFG_DSCP_DP_VAL_SET(0),
+			 ANA_CL_DSCP_CFG_DSCP_QOS_VAL |
+			 ANA_CL_DSCP_CFG_DSCP_DP_VAL, sparx5,
+			 ANA_CL_DSCP_CFG(i));
+	}
+
+	/* Set per-dscp trust */
+	for (i = 0; i <  ARRAY_SIZE(qos->map.map); i++) {
+		if (qos->qos_enable) {
+			spx5_rmw(ANA_CL_DSCP_CFG_DSCP_TRUST_ENA_SET(1),
+				 ANA_CL_DSCP_CFG_DSCP_TRUST_ENA, sparx5,
+				 ANA_CL_DSCP_CFG(i));
+		}
+	}
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
index fae9f5464548..00def02455a7 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
@@ -101,8 +101,15 @@ struct sparx5_port_qos_pcp {
 	bool dp_enable;
 };

+struct sparx5_port_qos_dscp {
+	struct dcb_ieee_app_dscp_map map;
+	bool qos_enable;
+	bool dp_enable;
+};
+
 struct sparx5_port_qos {
 	struct sparx5_port_qos_pcp pcp;
+	struct sparx5_port_qos_dscp dscp;
 };

 int sparx5_port_qos_set(struct sparx5_port *port, struct sparx5_port_qos *qos);
@@ -110,4 +117,6 @@ int sparx5_port_qos_set(struct sparx5_port *port, struct sparx5_port_qos *qos);
 int sparx5_port_qos_pcp_set(const struct sparx5_port *port,
 			    struct sparx5_port_qos_pcp *qos);

+int sparx5_port_qos_dscp_set(const struct sparx5_port *port,
+			     struct sparx5_port_qos_dscp *qos);
 #endif	/* __SPARX5_PORT_H__ */
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v2 6/6] net: microchip: sparx5: add support for offloading default prio

[Daniel Machon]

Add support for offloading default prio {ETHERTYPE, 0, prio}.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 .../ethernet/microchip/sparx5/sparx5_dcb.c    | 12 ++++++++++
 .../ethernet/microchip/sparx5/sparx5_port.c   | 23 +++++++++++++++++++
 .../ethernet/microchip/sparx5/sparx5_port.h   |  5 ++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
index 0cc46672b59c..50df24972643 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
@@ -34,6 +34,13 @@ static int sparx5_dcb_app_validate(struct net_device *dev,
 	int err = 0;

 	switch (app->selector) {
+	/* Default priority checks */
+	case IEEE_8021QAZ_APP_SEL_ETHERTYPE:
+		if (app->protocol != 0)
+			err = -EINVAL;
+		else if (app->priority >= SPX5_PRIOS)
+			err = -ERANGE;
+		break;
 	/* Dscp checks */
 	case IEEE_8021QAZ_APP_SEL_DSCP:
 		if (app->protocol > 63)
@@ -119,6 +126,11 @@ static int sparx5_dcb_app_update(struct net_device *dev)
 	dscp_map = &qos.dscp.map;
 	pcp_map = &qos.pcp.map;

+	/* Get default prio. */
+	qos.default_prio = dcb_ieee_getapp_default_prio_mask(dev);
+	if (qos.default_prio)
+		qos.default_prio = fls(qos.default_prio) - 1;
+
 	/* Get dscp ingress mapping */
 	dcb_ieee_getapp_dscp_prio_mask_map(dev, dscp_map);
 	for (i = 0; i < ARRAY_SIZE(dscp_map->map); i++)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index fb5e321c4896..73ebe76d7e50 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -1151,6 +1151,7 @@ int sparx5_port_qos_set(struct sparx5_port *port,
 {
 	sparx5_port_qos_dscp_set(port, &qos->dscp);
 	sparx5_port_qos_pcp_set(port, &qos->pcp);
+	sparx5_port_qos_default_set(port, qos);

 	return 0;
 }
@@ -1220,3 +1221,25 @@ int sparx5_port_qos_dscp_set(const struct sparx5_port *port,

 	return 0;
 }
+
+int sparx5_port_qos_default_set(const struct sparx5_port *port,
+				const struct sparx5_port_qos *qos)
+{
+	struct sparx5 *sparx5 = port->sparx5;
+
+	/* Set default prio and dp level */
+	spx5_rmw(ANA_CL_QOS_CFG_DEFAULT_QOS_VAL_SET(qos->default_prio) |
+		 ANA_CL_QOS_CFG_DEFAULT_DP_VAL_SET(0),
+		 ANA_CL_QOS_CFG_DEFAULT_QOS_VAL |
+		 ANA_CL_QOS_CFG_DEFAULT_DP_VAL,
+		 sparx5, ANA_CL_QOS_CFG(port->portno));
+
+	/* Set default pcp and dei for untagged frames */
+	spx5_rmw(ANA_CL_VLAN_CTRL_PORT_PCP_SET(0) |
+		 ANA_CL_VLAN_CTRL_PORT_DEI_SET(0),
+		 ANA_CL_VLAN_CTRL_PORT_PCP |
+		 ANA_CL_VLAN_CTRL_PORT_DEI,
+		 sparx5, ANA_CL_VLAN_CTRL(port->portno));
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
index 00def02455a7..698d7d5a5c4e 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
@@ -110,6 +110,7 @@ struct sparx5_port_qos_dscp {
 struct sparx5_port_qos {
 	struct sparx5_port_qos_pcp pcp;
 	struct sparx5_port_qos_dscp dscp;
+	u8 default_prio;
 };

 int sparx5_port_qos_set(struct sparx5_port *port, struct sparx5_port_qos *qos);
@@ -119,4 +120,8 @@ int sparx5_port_qos_pcp_set(const struct sparx5_port *port,

 int sparx5_port_qos_dscp_set(const struct sparx5_port *port,
 			     struct sparx5_port_qos_dscp *qos);
+
+int sparx5_port_qos_default_set(const struct sparx5_port *port,
+				const struct sparx5_port_qos *qos);
+
 #endif	/* __SPARX5_PORT_H__ */
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

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

> Add new PCP selector for the 8021Qaz APP managed object.
>
> As the PCP selector is not part of the 8021Qaz standard, a new non-std
> extension attribute DCB_ATTR_DCB_APP has been introduced. Also two
> helper functions to translate between selector and app attribute type
> has been added.
>
> The purpose of adding the PCP selector, is to be able to offload
> PCP-based queue classification to the 8021Q Priority Code Point table,
> see 6.9.3 of IEEE Std 802.1Q-2018.

Just a note: the "dcb app" block deals with packet prioritization.
Classification is handled through "dcb ets prio-tc", or offloaded egress
qdiscs or whatever, regardless of how the priority was derived.

> PCP and DEI is encoded in the protocol field as 8*dei+pcp, so that a
> mapping of PCP 2 and DEI 1 to priority 3 is encoded as {255, 10, 3}.

It would be good to shout out that the new selector value is 255.
Also it would be good to be explicit about how the same struct dcb_app
is used for both standard and non-standard attributes, because there
currently is no overlap between the two namespaces.

> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  include/uapi/linux/dcbnl.h |  6 +++++
>  net/dcb/dcbnl.c            | 49 ++++++++++++++++++++++++++++++++++----
>  2 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
> index a791a94013a6..9f68dc501cc1 100644
> --- a/include/uapi/linux/dcbnl.h
> +++ b/include/uapi/linux/dcbnl.h
> @@ -218,6 +218,9 @@ struct cee_pfc {
>  #define IEEE_8021QAZ_APP_SEL_ANY	4
>  #define IEEE_8021QAZ_APP_SEL_DSCP       5
>
> +/* Non-std selector values */
> +#define DCB_APP_SEL_PCP		24
> +
>  /* This structure contains the IEEE 802.1Qaz APP managed object. This
>   * object is also used for the CEE std as well.
>   *
> @@ -247,6 +250,8 @@ struct dcb_app {
>  	__u16	protocol;
>  };
>
> +#define IEEE_8021QAZ_APP_SEL_MAX 255

This is only necessary for the trust table code, so I would punt this
into the next patch.

> +
>  /**
>   * struct dcb_peer_app_info - APP feature information sent by the peer
>   *
> @@ -425,6 +430,7 @@ enum ieee_attrs {
>  enum ieee_attrs_app {
>  	DCB_ATTR_IEEE_APP_UNSPEC,
>  	DCB_ATTR_IEEE_APP,
> +	DCB_ATTR_DCB_APP,
>  	__DCB_ATTR_IEEE_APP_MAX
>  };
>  #define DCB_ATTR_IEEE_APP_MAX (__DCB_ATTR_IEEE_APP_MAX - 1)
> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index dc4fb699b56c..580d26acfc84 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -179,6 +179,46 @@ static const struct nla_policy dcbnl_featcfg_nest[DCB_FEATCFG_ATTR_MAX + 1] = {
>  static LIST_HEAD(dcb_app_list);
>  static DEFINE_SPINLOCK(dcb_lock);
>
> +static int dcbnl_app_attr_type_get(u8 selector)

The return value can be directly enum ieee_attrs_app;

> +{
> +	enum ieee_attrs_app type;
> +
> +	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:
> +		type = DCB_ATTR_IEEE_APP;
> +		break;

Just return DCB_ATTR_IEEE_APP? Similarly below.

> +	case DCB_APP_SEL_PCP:
> +		type = DCB_ATTR_DCB_APP;
> +		break;
> +	default:
> +		type = DCB_ATTR_IEEE_APP_UNSPEC;
> +		break;
> +	}
> +
> +	return type;
> +}
> +
> +static int dcbnl_app_attr_type_validate(enum ieee_attrs_app type)
> +{
> +	bool ret;
> +
> +	switch (type) {
> +	case DCB_ATTR_IEEE_APP:
> +	case DCB_ATTR_DCB_APP:
> +		ret = true;
> +		break;
> +	default:
> +		ret = false;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
>  static struct sk_buff *dcbnl_newmsg(int type, u8 cmd, u32 port, u32 seq,
>  				    u32 flags, struct nlmsghdr **nlhp)
>  {
> @@ -1116,8 +1156,9 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
>  	spin_lock_bh(&dcb_lock);
>  	list_for_each_entry(itr, &dcb_app_list, list) {
>  		if (itr->ifindex == netdev->ifindex) {
> -			err = nla_put(skb, DCB_ATTR_IEEE_APP, sizeof(itr->app),
> -					 &itr->app);
> +			enum ieee_attrs_app type =
> +				dcbnl_app_attr_type_get(itr->app.selector);
> +			err = nla_put(skb, type, sizeof(itr->app), &itr->app);
>  			if (err) {
>  				spin_unlock_bh(&dcb_lock);
>  				return -EMSGSIZE;
> @@ -1495,7 +1536,7 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
>  		nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
>  			struct dcb_app *app_data;
>
> -			if (nla_type(attr) != DCB_ATTR_IEEE_APP)
> +			if (!dcbnl_app_attr_type_validate(nla_type(attr)))

Oh no! It wasn't validating the DCB_ATTR_IEEE_APP_TABLE nest against a
policy! Instead it was just skipping whatever is not DCB_ATTR_IEEE_APP.

So userspace was permitted to shove random crap down here, and it would
just quietly be ignored. We can't start reinterpreting some of that crap
as information. We also can't start bouncing it.

This needs to be done differently.

One API "hole" that I see is that payload with size < struct dcb_app
gets bounced.

We can pack the new stuff into a smaller payload. The inner attribute
would not be DCB_ATTR_DCB_APP, but say DCB_ATTR_DCB_PCP, which would
imply the selector. The payload can be struct { u8 prio; u16 proto; }.
This would have been bounced by the old UAPI, so we know no userspace
makes use of that.

We can treat the output similarly.

>  				continue;
>
>  			if (nla_len(attr) < sizeof(struct dcb_app)) {
> @@ -1556,7 +1597,7 @@ static int dcbnl_ieee_del(struct net_device *netdev, struct nlmsghdr *nlh,
>  		nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
>  			struct dcb_app *app_data;
>
> -			if (nla_type(attr) != DCB_ATTR_IEEE_APP)
> +			if (!dcbnl_app_attr_type_validate(nla_type(attr)))

Likewise here, unfortunately.

>  				continue;
>  			app_data = nla_data(attr);
>  			if (ops->ieee_delapp)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 2/6] net: dcb: add new apptrust attribute

[Petr Machata]

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

> Add new apptrust extension attributes to the 8021Qaz APP managed object.
>
> Two new attributes, DCB_ATTR_DCB_APP_TRUST_TABLE and
> DCB_ATTR_DCB_APP_TRUST, has been added. Trusted selectors are passed in
> the nested attribute DCB_ATTR_DCB_APP_TRUST, in order of precedence.
>
> The new attributes are meant to allow drivers, whose hw supports the
> notion of trust, to be able to set whether a particular app selector is
> trusted - and in which order.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  include/net/dcbnl.h        |  4 ++
>  include/uapi/linux/dcbnl.h |  9 +++++
>  net/dcb/dcbnl.c            | 77 ++++++++++++++++++++++++++++++++++++--
>  3 files changed, 86 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/dcbnl.h b/include/net/dcbnl.h
> index 2b2d86fb3131..8841ab6c2de7 100644
> --- a/include/net/dcbnl.h
> +++ b/include/net/dcbnl.h
> @@ -109,6 +109,10 @@ struct dcbnl_rtnl_ops {
>  	/* buffer settings */
>  	int (*dcbnl_getbuffer)(struct net_device *, struct dcbnl_buffer *);
>  	int (*dcbnl_setbuffer)(struct net_device *, struct dcbnl_buffer *);
> +
> +	/* apptrust */
> +	int (*dcbnl_setapptrust)(struct net_device *, u8 *, int);
> +	int (*dcbnl_getapptrust)(struct net_device *, u8 *, int *);
>  };
>
>  #endif /* __NET_DCBNL_H__ */
> diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
> index 9f68dc501cc1..f892cd945695 100644
> --- a/include/uapi/linux/dcbnl.h
> +++ b/include/uapi/linux/dcbnl.h
> @@ -410,6 +410,7 @@ enum dcbnl_attrs {
>   * @DCB_ATTR_IEEE_PEER_ETS: peer ETS configuration - get only
>   * @DCB_ATTR_IEEE_PEER_PFC: peer PFC configuration - get only
>   * @DCB_ATTR_IEEE_PEER_APP: peer APP tlv - get only
> + * @DCB_ATTR_DCB_APP_TRUST_TABLE: selector trust order
>   */
>  enum ieee_attrs {
>  	DCB_ATTR_IEEE_UNSPEC,
> @@ -423,6 +424,7 @@ enum ieee_attrs {
>  	DCB_ATTR_IEEE_QCN,
>  	DCB_ATTR_IEEE_QCN_STATS,
>  	DCB_ATTR_DCB_BUFFER,
> +	DCB_ATTR_DCB_APP_TRUST_TABLE,
>  	__DCB_ATTR_IEEE_MAX
>  };
>  #define DCB_ATTR_IEEE_MAX (__DCB_ATTR_IEEE_MAX - 1)
> @@ -435,6 +437,13 @@ enum ieee_attrs_app {
>  };
>  #define DCB_ATTR_IEEE_APP_MAX (__DCB_ATTR_IEEE_APP_MAX - 1)
>
> +enum dcbnl_attrs_apptrust {
> +	DCB_ATTR_DCB_APP_TRUST_UNSPEC,
> +	DCB_ATTR_DCB_APP_TRUST,
> +	__DCB_ATTR_DCB_APP_TRUST_MAX
> +};
> +#define DCB_ATTR_DCB_APP_TRUST_MAX (__DCB_ATTR_DCB_APP_TRUST_MAX - 1)
> +
>  /**
>   * enum cee_attrs - CEE DCBX get attributes.
>   *
> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index 580d26acfc84..ad84f70e3eb3 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -166,6 +166,7 @@ static const struct nla_policy dcbnl_ieee_policy[DCB_ATTR_IEEE_MAX + 1] = {
>  	[DCB_ATTR_IEEE_QCN]         = {.len = sizeof(struct ieee_qcn)},
>  	[DCB_ATTR_IEEE_QCN_STATS]   = {.len = sizeof(struct ieee_qcn_stats)},
>  	[DCB_ATTR_DCB_BUFFER]       = {.len = sizeof(struct dcbnl_buffer)},
> +	[DCB_ATTR_DCB_APP_TRUST_TABLE] = {.type = NLA_NESTED},
>  };
>
>  /* DCB number of traffic classes nested attributes. */
> @@ -1070,11 +1071,11 @@ static int dcbnl_build_peer_app(struct net_device *netdev, struct sk_buff* skb,
>  /* Handle IEEE 802.1Qaz/802.1Qau/802.1Qbb GET commands. */
>  static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
>  {
> -	struct nlattr *ieee, *app;
> -	struct dcb_app_type *itr;
>  	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
> +	struct nlattr *ieee, *app, *apptrust;
> +	struct dcb_app_type *itr;
> +	int err, i;
>  	int dcbx;
> -	int err;
>
>  	if (nla_put_string(skb, DCB_ATTR_IFNAME, netdev->name))
>  		return -EMSGSIZE;
> @@ -1174,6 +1175,24 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
>  	spin_unlock_bh(&dcb_lock);
>  	nla_nest_end(skb, app);
>
> +	if (ops->dcbnl_getapptrust) {
> +		u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0};

BTW, the MAX value is currently 255, which made some sort of sense when
that was the value used for PCP. But we currently only need 24, and
actually like... 6 or whatever? Since the selectors are not supposed to
duplicate, and there are only about that number of them?

Though actually since the new attribute route won't work (as explained
in the other e-mail), it's an open question what the PCP selector value
will be.

> +		int nselectors;
> +
> +		apptrust = nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE);
> +		if (!app)
> +			return -EMSGSIZE;
> +
> +		err = ops->dcbnl_getapptrust(netdev, selectors, &nselectors);
> +		if (err)
> +			return -EMSGSIZE;
> +
> +		for (i = 0; i < nselectors; i++)
> +			nla_put_u8(skb, DCB_ATTR_DCB_APP_TRUST, selectors[i]);
> +
> +		nla_nest_end(skb, apptrust);
> +	}
> +
>  	/* get peer info if available */
>  	if (ops->ieee_peer_getets) {
>  		struct ieee_ets ets;
> @@ -1467,8 +1486,8 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
>  {
>  	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
>  	struct nlattr *ieee[DCB_ATTR_IEEE_MAX + 1];
> +	int err, i;
>  	int prio;
> -	int err;
>
>  	if (!ops)
>  		return -EOPNOTSUPP;
> @@ -1554,6 +1573,56 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
>  		}
>  	}
>
> +	if (ieee[DCB_ATTR_DCB_APP_TRUST_TABLE]) {
> +		u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0};
> +		struct nlattr *attr;
> +		int nselectors = 0;
> +		u8 selector;
> +		int rem;
> +
> +		if (!ops->dcbnl_setapptrust) {
> +			err = -EOPNOTSUPP;
> +			goto err;
> +		}
> +
> +		nla_for_each_nested(attr, ieee[DCB_ATTR_DCB_APP_TRUST_TABLE],
> +				    rem) {
> +			if (nla_type(attr) != DCB_ATTR_DCB_APP_TRUST ||
> +			    nla_len(attr) != 1 ||
> +			    nselectors >= sizeof(selectors)) {
> +				err = -EINVAL;
> +				goto err;
> +			}
> +
> +			selector = nla_get_u8(attr);
> +			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:
> +			case DCB_APP_SEL_PCP:
> +				break;
> +			default:
> +				err = -EINVAL;
> +				goto err;
> +			}
> +			/* Duplicate selector ? */
> +			for (i = 0; i < nselectors; i++) {
> +				if (selectors[i] == selector) {
> +					err = -EINVAL;
> +					goto err;
> +				}
> +			}
> +
> +			selectors[nselectors++] = selector;
> +		}
> +
> +		err = ops->dcbnl_setapptrust(netdev, selectors, nselectors);
> +		if (err)
> +			goto err;
> +	}
> +
>  err:
>  	err = nla_put_u8(skb, DCB_ATTR_IEEE, err);
>  	dcbnl_ieee_notify(netdev, RTM_SETDCB, DCB_CMD_IEEE_SET, seq, 0);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

Petr Machata <petrm@nvidia.com> writes:

> We can pack the new stuff into a smaller payload. The inner attribute
> would not be DCB_ATTR_DCB_APP, but say DCB_ATTR_DCB_PCP, which would
> imply the selector. The payload can be struct { u8 prio; u16 proto; }.
> This would have been bounced by the old UAPI, so we know no userspace
> makes use of that.

Except this will end up having size of 4 anyway due to alignment. We'll
have to make it a struct { ... } __attribute__((__packed__));

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for apptrust

[Petr Machata]

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

> Make use of set/getapptrust() to implement per-selector trust and trust
> order.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
>  .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
>  .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
>  .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
>  .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
>  5 files changed, 127 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> index db17c124dac8..10aeb422b1ae 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> @@ -8,6 +8,22 @@
>
>  #include "sparx5_port.h"
>
> +static const struct sparx5_dcb_apptrust {
> +	u8 selectors[256];
> +	int nselectors;
> +	char *names;

I think this should be just "name".

> +} *apptrust[SPX5_PORTS];
> +
> +/* Sparx5 supported apptrust configurations */
> +static const struct sparx5_dcb_apptrust apptrust_conf[4] = {
> +	/* Empty *must* be first */
> +	{ { 0                         }, 0, "empty"    },
> +	{ { IEEE_8021QAZ_APP_SEL_DSCP }, 1, "dscp"     },
> +	{ { DCB_APP_SEL_PCP           }, 1, "pcp"      },
> +	{ { IEEE_8021QAZ_APP_SEL_DSCP,
> +	    DCB_APP_SEL_PCP           }, 2, "dscp pcp" },
> +};
> +
>  /* Validate app entry.
>   *
>   * Check for valid selectors and valid protocol and priority ranges.
> @@ -37,12 +53,59 @@ static int sparx5_dcb_app_validate(struct net_device *dev,
>  	return err;
>  }
>
> +/* Validate apptrust configuration.
> + *
> + * Return index of supported apptrust configuration if valid, otherwise return
> + * error.
> + */
> +static int sparx5_dcb_apptrust_validate(struct net_device *dev, u8 *selectors,
> +					int nselectors, int *err)
> +{
> +	bool match;
> +	int i, ii;
> +
> +	for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++) {

I would do this here:

    if (apptrust_conf[i].nselectors != nselectors) continue;

to avoid having to think about the semantics of comparing to all those
zeroes in apptrust_conf.selectors array.

> +		match = true;
> +		for (ii = 0; ii < nselectors; ii++) {
> +			if (apptrust_conf[i].selectors[ii] !=
> +			    *(selectors + ii)) {
> +				match = false;
> +				break;
> +			}
> +		}
> +		if (match)
> +			break;
> +	}
> +
> +	/* Requested trust configuration is not supported */
> +	if (!match) {
> +		netdev_err(dev, "Valid apptrust configurations are:\n");
> +		for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++)
> +			pr_info("order: %s\n", apptrust_conf[i].names);
> +		*err = -EOPNOTSUPP;
> +	}
> +
> +	return i;
> +}
> +
> +static bool sparx5_dcb_apptrust_contains(int portno, u8 selector)
> +{
> +	int i;
> +
> +	for (i = 0; i < IEEE_8021QAZ_APP_SEL_MAX + 1; i++)
> +		if (apptrust[portno]->selectors[i] == selector)
> +			return true;
> +
> +	return false;
> +}
> +
>  static int sparx5_dcb_app_update(struct net_device *dev)
>  {
>  	struct dcb_app app_itr = { .selector = DCB_APP_SEL_PCP };
>  	struct sparx5_port *port = netdev_priv(dev);
>  	struct sparx5_port_qos_pcp_map *pcp_map;
>  	struct sparx5_port_qos qos = {0};
> +	int portno = port->portno;
>  	int i;
>
>  	pcp_map = &qos.pcp.map;
> @@ -53,6 +116,12 @@ static int sparx5_dcb_app_update(struct net_device *dev)
>  		pcp_map->map[i] = dcb_getapp(dev, &app_itr);
>  	}
>
> +	/* Enable use of pcp for queue classification ? */
> +	if (sparx5_dcb_apptrust_contains(portno, DCB_APP_SEL_PCP)) {
> +		qos.pcp.qos_enable = true;
> +		qos.pcp.dp_enable = qos.pcp.qos_enable;
> +	}
> +
>  	return sparx5_port_qos_set(port, &qos);
>  }
>
> @@ -95,7 +164,54 @@ static int sparx5_dcb_ieee_delapp(struct net_device *dev, struct dcb_app *app)
>  	return sparx5_dcb_app_update(dev);
>  }
>
> +static int sparx5_dcb_setapptrust(struct net_device *dev, u8 *selectors,
> +				  int nselectors)
> +{
> +	struct sparx5_port *port = netdev_priv(dev);
> +	int err = 0, idx;
> +
> +	idx = sparx5_dcb_apptrust_validate(dev, selectors, nselectors, &err);
> +	if (err < 0)
> +		return err;
> +
> +	apptrust[port->portno] = &apptrust_conf[idx];
> +
> +	return sparx5_dcb_app_update(dev);
> +}
> +
> +static int sparx5_dcb_getapptrust(struct net_device *dev, u8 *selectors,
> +				  int *nselectors)
> +{
> +	struct sparx5_port *port = netdev_priv(dev);
> +	const struct sparx5_dcb_apptrust *trust;
> +
> +	trust = apptrust[port->portno];
> +
> +	memcpy(selectors, trust->selectors, trust->nselectors);
> +	*nselectors = trust->nselectors;
> +
> +	return 0;
> +}
> +
> +int sparx5_dcb_init(struct sparx5 *sparx5)
> +{
> +	struct sparx5_port *port;
> +	int i;
> +
> +	for (i = 0; i < SPX5_PORTS; i++) {
> +		port = sparx5->ports[i];
> +		if (!port)
> +			continue;
> +		/* Initialize [dscp, pcp] default trust */
> +		apptrust[port->portno] = &apptrust_conf[3];
> +	}
> +
> +	return sparx5_dcb_app_update(port->ndev);
> +}
> +
>  const struct dcbnl_rtnl_ops sparx5_dcbnl_ops = {
>  	.ieee_setapp = sparx5_dcb_ieee_setapp,
>  	.ieee_delapp = sparx5_dcb_ieee_delapp,
> +	.dcbnl_setapptrust = sparx5_dcb_setapptrust,
> +	.dcbnl_getapptrust = sparx5_dcb_getapptrust,
>  };
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index 0d8e04c64584..d07ef2e8b321 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -357,6 +357,9 @@ int sparx5_config_dsm_calendar(struct sparx5 *sparx5);
>  void sparx5_get_stats64(struct net_device *ndev, struct rtnl_link_stats64 *stats);
>  int sparx_stats_init(struct sparx5 *sparx5);
>
> +/* sparx5_dcb.c */
> +int sparx5_dcb_init(struct sparx5 *sparx5);
> +
>  /* sparx5_netdev.c */
>  void sparx5_set_port_ifh_timestamp(void *ifh_hdr, u64 timestamp);
>  void sparx5_set_port_ifh_rew_op(void *ifh_hdr, u32 rew_op);
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
> index 9ffaaf34d196..99e86e87aa16 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
> @@ -1163,8 +1163,8 @@ int sparx5_port_qos_pcp_set(const struct sparx5_port *port,
>  	int i;
>
>  	/* Enable/disable pcp and dp for qos classification. */
> -	spx5_rmw(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(1) |
> -		 ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(1),
> +	spx5_rmw(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(qos->qos_enable) |
> +		 ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(qos->dp_enable),
>  		 ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA | ANA_CL_QOS_CFG_PCP_DEI_DP_ENA,
>  		 sparx5, ANA_CL_QOS_CFG(port->portno));
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
> index 9c5fb6b651db..fae9f5464548 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
> @@ -97,6 +97,8 @@ struct sparx5_port_qos_pcp_map {
>
>  struct sparx5_port_qos_pcp {
>  	struct sparx5_port_qos_pcp_map map;
> +	bool qos_enable;
> +	bool dp_enable;
>  };
>
>  struct sparx5_port_qos {
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
> index 1e79d0ef0cb8..379e540e5e6a 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
> @@ -389,6 +389,10 @@ int sparx5_qos_init(struct sparx5 *sparx5)
>  	if (ret < 0)
>  		return ret;
>
> +	ret = sparx5_dcb_init(sparx5);
> +	if (ret < 0)
> +		return ret;
> +
>  	return 0;
>  }


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Jakub Kicinski]

On Fri, 30 Sep 2022 14:20:50 +0200 Petr Machata wrote:
> > @@ -1495,7 +1536,7 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
> >  		nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
> >  			struct dcb_app *app_data;
> >
> > -			if (nla_type(attr) != DCB_ATTR_IEEE_APP)
> > +			if (!dcbnl_app_attr_type_validate(nla_type(attr)))  
> 
> Oh no! It wasn't validating the DCB_ATTR_IEEE_APP_TABLE nest against a
> policy! Instead it was just skipping whatever is not DCB_ATTR_IEEE_APP.
> 
> So userspace was permitted to shove random crap down here, and it would
> just quietly be ignored. We can't start reinterpreting some of that crap
> as information. We also can't start bouncing it.

Are you saying that we can't start interpreting new attr types?

"Traditionally" netlink ignored new attr types so from that perspective
starting to interpret new types is pretty "run of the mill" for netlink.
IOW *_deprecated() parsing routines do not use NL_VALIDATE_MAXTYPE.

That does put netlink in a bit of a special category when it comes to
input validation, but really putting in a random but valid attr is much
harder than not initializing a struct member. Is there user space which
does that?

Sorry if I'm misinterpreting the situation.

> This needs to be done differently.
> 
> One API "hole" that I see is that payload with size < struct dcb_app
> gets bounced.
> 
> We can pack the new stuff into a smaller payload. The inner attribute
> would not be DCB_ATTR_DCB_APP, but say DCB_ATTR_DCB_PCP, which would
> imply the selector. The payload can be struct { u8 prio; u16 proto; }.
> This would have been bounced by the old UAPI, so we know no userspace
> makes use of that.
> 
> We can treat the output similarly.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Daniel.Machon]

> > Add new PCP selector for the 8021Qaz APP managed object.
> >
> > As the PCP selector is not part of the 8021Qaz standard, a new non-std
> > extension attribute DCB_ATTR_DCB_APP has been introduced. Also two
> > helper functions to translate between selector and app attribute type
> > has been added.
> >
> > The purpose of adding the PCP selector, is to be able to offload
> > PCP-based queue classification to the 8021Q Priority Code Point table,
> > see 6.9.3 of IEEE Std 802.1Q-2018.
> 
> Just a note: the "dcb app" block deals with packet prioritization.
> Classification is handled through "dcb ets prio-tc", or offloaded egress
> qdiscs or whatever, regardless of how the priority was derived.
> 
> > PCP and DEI is encoded in the protocol field as 8*dei+pcp, so that a
> > mapping of PCP 2 and DEI 1 to priority 3 is encoded as {255, 10, 3}.
> 
> It would be good to shout out that the new selector value is 255.
> Also it would be good to be explicit about how the same struct dcb_app
> is used for both standard and non-standard attributes, because there
> currently is no overlap between the two namespaces.
> 
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  include/uapi/linux/dcbnl.h |  6 +++++
> >  net/dcb/dcbnl.c            | 49 ++++++++++++++++++++++++++++++++++----
> >  2 files changed, 51 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
> > index a791a94013a6..9f68dc501cc1 100644
> > --- a/include/uapi/linux/dcbnl.h
> > +++ b/include/uapi/linux/dcbnl.h
> > @@ -218,6 +218,9 @@ struct cee_pfc {
> >  #define IEEE_8021QAZ_APP_SEL_ANY     4
> >  #define IEEE_8021QAZ_APP_SEL_DSCP       5
> >
> > +/* Non-std selector values */
> > +#define DCB_APP_SEL_PCP              24
> > +
> >  /* This structure contains the IEEE 802.1Qaz APP managed object. This
> >   * object is also used for the CEE std as well.
> >   *
> > @@ -247,6 +250,8 @@ struct dcb_app {
> >       __u16   protocol;
> >  };
> >
> > +#define IEEE_8021QAZ_APP_SEL_MAX 255
> 
> This is only necessary for the trust table code, so I would punt this
> into the next patch.

Will be fixed in next v.

> 
> > +
> >  /**
> >   * struct dcb_peer_app_info - APP feature information sent by the peer
> >   *
> > @@ -425,6 +430,7 @@ enum ieee_attrs {
> >  enum ieee_attrs_app {
> >       DCB_ATTR_IEEE_APP_UNSPEC,
> >       DCB_ATTR_IEEE_APP,
> > +     DCB_ATTR_DCB_APP,
> >       __DCB_ATTR_IEEE_APP_MAX
> >  };
> >  #define DCB_ATTR_IEEE_APP_MAX (__DCB_ATTR_IEEE_APP_MAX - 1)
> > diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> > index dc4fb699b56c..580d26acfc84 100644
> > --- a/net/dcb/dcbnl.c
> > +++ b/net/dcb/dcbnl.c
> > @@ -179,6 +179,46 @@ static const struct nla_policy dcbnl_featcfg_nest[DCB_FEATCFG_ATTR_MAX + 1] = {
> >  static LIST_HEAD(dcb_app_list);
> >  static DEFINE_SPINLOCK(dcb_lock);
> >
> > +static int dcbnl_app_attr_type_get(u8 selector)
> 
> The return value can be directly enum ieee_attrs_app;

Will be fixed in next v.

> 
> > +{
> > +     enum ieee_attrs_app type;
> > +
> > +     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:
> > +             type = DCB_ATTR_IEEE_APP;
> > +             break;
> 
> Just return DCB_ATTR_IEEE_APP? Similarly below.

That's fine.

> 
> > +     case DCB_APP_SEL_PCP:
> > +             type = DCB_ATTR_DCB_APP;
> > +             break;
> > +     default:
> > +             type = DCB_ATTR_IEEE_APP_UNSPEC;
> > +             break;
> > +     }
> > +
> > +     return type;
> > +}
> > +
> > +static int dcbnl_app_attr_type_validate(enum ieee_attrs_app type)
> > +{
> > +     bool ret;
> > +
> > +     switch (type) {
> > +     case DCB_ATTR_IEEE_APP:
> > +     case DCB_ATTR_DCB_APP:
> > +             ret = true;
> > +             break;
> > +     default:
> > +             ret = false;
> > +             break;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> >  static struct sk_buff *dcbnl_newmsg(int type, u8 cmd, u32 port, u32 seq,
> >                                   u32 flags, struct nlmsghdr **nlhp)
> >  {
> > @@ -1116,8 +1156,9 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
> >       spin_lock_bh(&dcb_lock);
> >       list_for_each_entry(itr, &dcb_app_list, list) {
> >               if (itr->ifindex == netdev->ifindex) {
> > -                     err = nla_put(skb, DCB_ATTR_IEEE_APP, sizeof(itr->app),
> > -                                      &itr->app);
> > +                     enum ieee_attrs_app type =
> > +                             dcbnl_app_attr_type_get(itr->app.selector);
> > +                     err = nla_put(skb, type, sizeof(itr->app), &itr->app);
> >                       if (err) {
> >                               spin_unlock_bh(&dcb_lock);
> >                               return -EMSGSIZE;
> > @@ -1495,7 +1536,7 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
> >               nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
> >                       struct dcb_app *app_data;
> >
> > -                     if (nla_type(attr) != DCB_ATTR_IEEE_APP)
> > +                     if (!dcbnl_app_attr_type_validate(nla_type(attr)))
> 
> Oh no! It wasn't validating the DCB_ATTR_IEEE_APP_TABLE nest against a
> policy! Instead it was just skipping whatever is not DCB_ATTR_IEEE_APP.
> 
> So userspace was permitted to shove random crap down here, and it would
> just quietly be ignored. We can't start reinterpreting some of that crap
> as information. We also can't start bouncing it.
> 
> This needs to be done differently.
> 
> One API "hole" that I see is that payload with size < struct dcb_app
> gets bounced.
> 
> We can pack the new stuff into a smaller payload. The inner attribute
> would not be DCB_ATTR_DCB_APP, but say DCB_ATTR_DCB_PCP, which would
> imply the selector. The payload can be struct { u8 prio; u16 proto; }.
> This would have been bounced by the old UAPI, so we know no userspace
> makes use of that.

Right, I see your point. But. First thought; this starts to look a little
hackish.

Looking through the 802.1Q-2018 std again, sel bits 0, 6 and 7 are 
reserved (implicit for future standard implementation?). Do we know of
any cases, where a new standard version would introduce new values beyond
what was reserved in the first place for future use? I dont know myself.

I am just trying to raise a question of whether using the std APP attr
with a new high (255) selector, really could be preferred over this new
non-std APP attr with new packed payload.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for apptrust

[Daniel.Machon]

> > Make use of set/getapptrust() to implement per-selector trust and trust
> > order.
> >
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
> >  .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
> >  .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
> >  .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
> >  .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
> >  5 files changed, 127 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> > index db17c124dac8..10aeb422b1ae 100644
> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> > @@ -8,6 +8,22 @@
> >
> >  #include "sparx5_port.h"
> >
> > +static const struct sparx5_dcb_apptrust {
> > +     u8 selectors[256];
> > +     int nselectors;
> > +     char *names;
> 
> I think this should be just "name".

I dont think so. This is a str representation of all the selector values.
"names" makes more sense to me.

> 
> > +} *apptrust[SPX5_PORTS];
> > +
> > +/* Sparx5 supported apptrust configurations */
> > +static const struct sparx5_dcb_apptrust apptrust_conf[4] = {
> > +     /* Empty *must* be first */
> > +     { { 0                         }, 0, "empty"    },
> > +     { { IEEE_8021QAZ_APP_SEL_DSCP }, 1, "dscp"     },
> > +     { { DCB_APP_SEL_PCP           }, 1, "pcp"      },
> > +     { { IEEE_8021QAZ_APP_SEL_DSCP,
> > +         DCB_APP_SEL_PCP           }, 2, "dscp pcp" },
> > +};
> > +
> >  /* Validate app entry.
> >   *
> >   * Check for valid selectors and valid protocol and priority ranges.
> > @@ -37,12 +53,59 @@ static int sparx5_dcb_app_validate(struct net_device *dev,
> >       return err;
> >  }
> >
> > +/* Validate apptrust configuration.
> > + *
> > + * Return index of supported apptrust configuration if valid, otherwise return
> > + * error.
> > + */
> > +static int sparx5_dcb_apptrust_validate(struct net_device *dev, u8 *selectors,
> > +                                     int nselectors, int *err)
> > +{
> > +     bool match;
> > +     int i, ii;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++) {
> 
> I would do this here:
> 
>     if (apptrust_conf[i].nselectors != nselectors) continue;
> 
> to avoid having to think about the semantics of comparing to all those
> zeroes in apptrust_conf.selectors array.

Yes, that would be good.

> 
> > +             match = true;
> > +             for (ii = 0; ii < nselectors; ii++) {
> > +                     if (apptrust_conf[i].selectors[ii] !=
> > +                         *(selectors + ii)) {
> > +                             match = false;
> > +                             break;
> > +                     }
> > +             }
> > +             if (match)
> > +                     break;
> > +     }
> > +
> > +     /* Requested trust configuration is not supported */
> > +     if (!match) {
> > +             netdev_err(dev, "Valid apptrust configurations are:\n");
> > +             for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++)
> > +                     pr_info("order: %s\n", apptrust_conf[i].names);
> > +             *err = -EOPNOTSUPP;
> > +     }
> > +
> > +     return i;
> > +}
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Fri, 30 Sep 2022 14:20:50 +0200 Petr Machata wrote:
>> > @@ -1495,7 +1536,7 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
>> >  		nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
>> >  			struct dcb_app *app_data;
>> >
>> > -			if (nla_type(attr) != DCB_ATTR_IEEE_APP)
>> > +			if (!dcbnl_app_attr_type_validate(nla_type(attr)))  
>> 
>> Oh no! It wasn't validating the DCB_ATTR_IEEE_APP_TABLE nest against a
>> policy! Instead it was just skipping whatever is not DCB_ATTR_IEEE_APP.
>> 
>> So userspace was permitted to shove random crap down here, and it would
>> just quietly be ignored. We can't start reinterpreting some of that crap
>> as information. We also can't start bouncing it.
>
> Are you saying that we can't start interpreting new attr types?
>
> "Traditionally" netlink ignored new attr types so from that perspective
> starting to interpret new types is pretty "run of the mill" for netlink.
> IOW *_deprecated() parsing routines do not use NL_VALIDATE_MAXTYPE.
>
> That does put netlink in a bit of a special category when it comes to
> input validation, but really putting in a random but valid attr is much
> harder than not initializing a struct member. Is there user space which
> does that?
>
> Sorry if I'm misinterpreting the situation.

I assumed the policy is much more strict with changes like this. If you
think it's OK, I'm fine with it as well.

The userspace (lldpad in particular) is doing the opposite thing BTW:
assuming everything in the nest is a DCB_ATTR_IEEE_APP. When we start
emitting the new attribute, it will get confused.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for apptrust

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

>> > Make use of set/getapptrust() to implement per-selector trust and trust
>> > order.
>> >
>> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
>> > ---
>> >  .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
>> >  .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
>> >  .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
>> >  .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
>> >  .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
>> >  5 files changed, 127 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
>> > index db17c124dac8..10aeb422b1ae 100644
>> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
>> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
>> > @@ -8,6 +8,22 @@
>> >
>> >  #include "sparx5_port.h"
>> >
>> > +static const struct sparx5_dcb_apptrust {
>> > +     u8 selectors[256];
>> > +     int nselectors;
>> > +     char *names;
>> 
>> I think this should be just "name".
>
> I dont think so. This is a str representation of all the selector values.
> "names" makes more sense to me.

But it just points to one name, doesn't it? The name of this apptrust
policy...

BTW, I think it should also be a const char *.

>> > +} *apptrust[SPX5_PORTS];
>> > +
>> > +/* Sparx5 supported apptrust configurations */
>> > +static const struct sparx5_dcb_apptrust apptrust_conf[4] = {
>> > +     /* Empty *must* be first */
>> > +     { { 0                         }, 0, "empty"    },
>> > +     { { IEEE_8021QAZ_APP_SEL_DSCP }, 1, "dscp"     },
>> > +     { { DCB_APP_SEL_PCP           }, 1, "pcp"      },
>> > +     { { IEEE_8021QAZ_APP_SEL_DSCP,
>> > +         DCB_APP_SEL_PCP           }, 2, "dscp pcp" },
>> > +};
>> > +

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for apptrust

[Daniel.Machon]

> >> > Make use of set/getapptrust() to implement per-selector trust and trust
> >> > order.
> >> >
> >> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> >> > ---
> >> >  .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
> >> >  .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
> >> >  .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
> >> >  .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
> >> >  .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
> >> >  5 files changed, 127 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> >> > index db17c124dac8..10aeb422b1ae 100644
> >> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> >> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> >> > @@ -8,6 +8,22 @@
> >> >
> >> >  #include "sparx5_port.h"
> >> >
> >> > +static const struct sparx5_dcb_apptrust {
> >> > +     u8 selectors[256];
> >> > +     int nselectors;
> >> > +     char *names;
> >>
> >> I think this should be just "name".
> >
> > I dont think so. This is a str representation of all the selector values.
> > "names" makes more sense to me.
> 
> But it just points to one name, doesn't it? The name of this apptrust
> policy...

It points to a str of space-separated selector names.
I inteded it to be a str repr. of the selector values, and not a str
identifier of the apptrust policy. Maybe sel(ector)_names?

> 
> BTW, I think it should also be a const char *.

Ack.

> 
> >> > +} *apptrust[SPX5_PORTS];
> >> > +
> >> > +/* Sparx5 supported apptrust configurations */
> >> > +static const struct sparx5_dcb_apptrust apptrust_conf[4] = {
> >> > +     /* Empty *must* be first */
> >> > +     { { 0                         }, 0, "empty"    },
> >> > +     { { IEEE_8021QAZ_APP_SEL_DSCP }, 1, "dscp"     },
> >> > +     { { DCB_APP_SEL_PCP           }, 1, "pcp"      },
> >> > +     { { IEEE_8021QAZ_APP_SEL_DSCP,
> >> > +         DCB_APP_SEL_PCP           }, 2, "dscp pcp" },
> >> > +};
> >> > +
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

> Right, I see your point. But. First thought; this starts to look a little
> hackish.

So it is. That's what poking backward compatible holes in an existing
API gets you. Look at modern C++ syntax for an extreme example :)

But read Jakub's email. It looks like we don't actually need to worry
about this.

> Looking through the 802.1Q-2018 std again, sel bits 0, 6 and 7 are 
> reserved (implicit for future standard implementation?). Do we know of
> any cases, where a new standard version would introduce new values beyond
> what was reserved in the first place for future use? I dont know myself.
>
> I am just trying to raise a question of whether using the std APP attr
> with a new high (255) selector, really could be preferred over this new
> non-std APP attr with new packed payload.

Yeah. We'll need to patch lldpad anyway. We can basically choose which
way we patch it. And BTW, using the too-short attribute payload of
course breaks it _as well_, because they don't do any payload size
validation.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Daniel.Machon]

Den Mon, Oct 03, 2022 at 10:22:48AM +0200 skrev Petr Machata:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> <Daniel.Machon@microchip.com> writes:
> 
> > Right, I see your point. But. First thought; this starts to look a little
> > hackish.
> 
> So it is. That's what poking backward compatible holes in an existing
> API gets you. Look at modern C++ syntax for an extreme example :)
> 
> But read Jakub's email. It looks like we don't actually need to worry
> about this.
> 
> > Looking through the 802.1Q-2018 std again, sel bits 0, 6 and 7 are
> > reserved (implicit for future standard implementation?). Do we know of
> > any cases, where a new standard version would introduce new values beyond
> > what was reserved in the first place for future use? I dont know myself.
> >
> > I am just trying to raise a question of whether using the std APP attr
> > with a new high (255) selector, really could be preferred over this new
> > non-std APP attr with new packed payload.
> 
> Yeah. We'll need to patch lldpad anyway. We can basically choose which
> way we patch it. And BTW, using the too-short attribute payload of
> course breaks it _as well_, because they don't do any payload size
> validation.

Right, unless we reconstruct std app entry payload from the "too-short"
attribute payload, before adding it the the app list or passing it to the 
driver.

Anyway. Considering Jakub's mail. I think this patch version with a non-std
attribute to do non-std app table contributions separates non-std from std
stuff nicely and is preffered over just adding the new selector. So if we can 
agree on this, I will prepare a new v. with the other changes suggested.

Wrt. lldpad we can then patch it to react on attrs or selectors > 7.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for apptrust

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

>> >> > Make use of set/getapptrust() to implement per-selector trust and trust
>> >> > order.
>> >> >
>> >> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
>> >> > ---
>> >> >  .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
>> >> >  .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
>> >> >  .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
>> >> >  .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
>> >> >  .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
>> >> >  5 files changed, 127 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
>> >> > index db17c124dac8..10aeb422b1ae 100644
>> >> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
>> >> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
>> >> > @@ -8,6 +8,22 @@
>> >> >
>> >> >  #include "sparx5_port.h"
>> >> >
>> >> > +static const struct sparx5_dcb_apptrust {
>> >> > +     u8 selectors[256];
>> >> > +     int nselectors;
>> >> > +     char *names;
>> >>
>> >> I think this should be just "name".
>> >
>> > I dont think so. This is a str representation of all the selector values.
>> > "names" makes more sense to me.
>> 
>> But it just points to one name, doesn't it? The name of this apptrust
>> policy...
>
> It points to a str of space-separated selector names. I inteded it to
> be a str repr. of the selector values, and not a str identifier of the
> apptrust policy.

Ah, that's what you mean. Sure, NP :)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Jakub Kicinski]

On Mon, 3 Oct 2022 09:52:59 +0200 Petr Machata wrote:
> I assumed the policy is much more strict with changes like this. If you
> think it's OK, I'm fine with it as well.
> 
> The userspace (lldpad in particular) is doing the opposite thing BTW:
> assuming everything in the nest is a DCB_ATTR_IEEE_APP. When we start
> emitting the new attribute, it will get confused.

Can you add an attribute or a flag to the request which would turn
emitting the new attrs on?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Daniel.Machon]

> On Mon, 3 Oct 2022 09:52:59 +0200 Petr Machata wrote:
> > I assumed the policy is much more strict with changes like this. If you
> > think it's OK, I'm fine with it as well.
> >
> > The userspace (lldpad in particular) is doing the opposite thing BTW:
> > assuming everything in the nest is a DCB_ATTR_IEEE_APP. When we start
> > emitting the new attribute, it will get confused.
> 
> Can you add an attribute or a flag to the request which would turn
> emitting the new attrs on?

As for this sparx5 impl. the new attrs or any other app attr, will not be
emitted by lldpad, since APP tx cannot be enabled when set/get_dcbx is not 
supported. IIUIC.

If lldpad was idd able to emit the new pcp app entries, they would be
emitted as invalid TLV's (assuming 255 or 24 selector value), because the
selector would be either zero or seven, which is currently not used for
any selector by the std. We then have time to patch lldpad to do whatever
with the new attr. Wouldn't this be acceptable?

As for iproute2/dcb, the new attr will just get ignored with a warning.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Jakub Kicinski]

On Mon, 3 Oct 2022 21:59:49 +0000 Daniel.Machon@microchip.com wrote:
> If lldpad was idd able to emit the new pcp app entries, they would be

idd?

> emitted as invalid TLV's (assuming 255 or 24 selector value), because the
> selector would be either zero or seven, which is currently not used for
> any selector by the std. We then have time to patch lldpad to do whatever
> with the new attr. Wouldn't this be acceptable?

I'm not sure I can provide sensible advice given I don't really know
how the information flow looks in case of DCB.

First off - we're talking about netlink TLVs not LLDP / DCB wire message
TLVs?

What I was saying is that if lldpad dumps the information from the
kernel and gets confused by certain TLVs - we can add an opt-in
attribute to whatever Netlink request lldpad uses, and only add the new
attrs if that opt-in attribute is present. Normal GET or DUMP requests
can both take input attributes.

Old lldpad will not send this attribute to the kernel - the kernel will
not respond with confusing attrs. The new lldpad can be patched to send
the attribute and will get all the attrs (if it actually cares).

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Mon, 3 Oct 2022 09:52:59 +0200 Petr Machata wrote:
>> I assumed the policy is much more strict with changes like this. If you
>> think it's OK, I'm fine with it as well.
>> 
>> The userspace (lldpad in particular) is doing the opposite thing BTW:
>> assuming everything in the nest is a DCB_ATTR_IEEE_APP. When we start
>> emitting the new attribute, it will get confused.
>
> Can you add an attribute or a flag to the request which would turn
> emitting the new attrs on?

The question is whether it's better to do it anyway. My opinion is that
if a userspace decides to make assumptions about the contents of a TLV,
and neglects to validate the actual TLV type, it's on them, and I'm not
obligated to keep them working. We know about this case, but really any
attribute addition at all could potentially trip some userspace if they
expected something else at this offset.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

Petr Machata <petrm@nvidia.com> writes:

> Jakub Kicinski <kuba@kernel.org> writes:
>
>> On Mon, 3 Oct 2022 09:52:59 +0200 Petr Machata wrote:
>>> I assumed the policy is much more strict with changes like this. If you
>>> think it's OK, I'm fine with it as well.
>>> 
>>> The userspace (lldpad in particular) is doing the opposite thing BTW:
>>> assuming everything in the nest is a DCB_ATTR_IEEE_APP. When we start
>>> emitting the new attribute, it will get confused.
>>
>> Can you add an attribute or a flag to the request which would turn
>> emitting the new attrs on?
>
> The question is whether it's better to do it anyway. My opinion is that
> if a userspace decides to make assumptions about the contents of a TLV,
> and neglects to validate the actual TLV type, it's on them, and I'm not
> obligated to keep them working. We know about this case, but really any
> attribute addition at all could potentially trip some userspace if they
> expected something else at this offset.

And re the flag: I think struct dcbmsg.dcb_pad was meant to be the place
to keep flags when the need arises, but it is not validated anywhere, so
we cannot use it. It could be a new command, but I'm not a fan. So if we
need to discriminate userspaces, I think it should be a new attribute.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Mon, 3 Oct 2022 21:59:49 +0000 Daniel.Machon@microchip.com wrote:
>> If lldpad was idd able to emit the new pcp app entries, they would be
>
> idd?
>
>> emitted as invalid TLV's (assuming 255 or 24 selector value), because the
>> selector would be either zero or seven, which is currently not used for
>> any selector by the std. We then have time to patch lldpad to do whatever
>> with the new attr. Wouldn't this be acceptable?
>
> I'm not sure I can provide sensible advice given I don't really know
> how the information flow looks in case of DCB.
>
> First off - we're talking about netlink TLVs not LLDP / DCB wire message
> TLVs?

DCB wire message in this case.

> What I was saying is that if lldpad dumps the information from the
> kernel and gets confused by certain TLVs - we can add an opt-in
> attribute to whatever Netlink request lldpad uses, and only add the new
> attrs if that opt-in attribute is present. Normal GET or DUMP requests
> can both take input attributes.
>
> Old lldpad will not send this attribute to the kernel - the kernel will
> not respond with confusing attrs. The new lldpad can be patched to send
> the attribute and will get all the attrs (if it actually cares).

Another aspect is that lldpad will never create these entries on its
own, until it gets support for it, at which point these issues would
presumably get fixed as well. The only scenario in which it breaks is
when an admin messes with the APP entries through iproute2, but then
uses lldpad. Which doesn't make sense to me as a use case.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Jakub Kicinski]

On Tue, 4 Oct 2022 12:52:35 +0200 Petr Machata wrote:
> > The question is whether it's better to do it anyway. My opinion is that
> > if a userspace decides to make assumptions about the contents of a TLV,
> > and neglects to validate the actual TLV type, it's on them, and I'm not
> > obligated to keep them working. We know about this case, but really any
> > attribute addition at all could potentially trip some userspace if they
> > expected something else at this offset.  
> 
> And re the flag: I think struct dcbmsg.dcb_pad was meant to be the place
> to keep flags when the need arises, but it is not validated anywhere, so
> we cannot use it. It could be a new command, but I'm not a fan. So if we
> need to discriminate userspaces, I think it should be a new attribute.

All good points. I'm fine with not gating the attributes if that's your
preference. Your call.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

[Petr Machata]

<Daniel.Machon@microchip.com> writes:

> Den Mon, Oct 03, 2022 at 10:22:48AM +0200 skrev Petr Machata:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>> 
>> <Daniel.Machon@microchip.com> writes:
>> 
>> > Right, I see your point. But. First thought; this starts to look a little
>> > hackish.
>> 
>> So it is. That's what poking backward compatible holes in an existing
>> API gets you. Look at modern C++ syntax for an extreme example :)
>> 
>> But read Jakub's email. It looks like we don't actually need to worry
>> about this.
>> 
>> > Looking through the 802.1Q-2018 std again, sel bits 0, 6 and 7 are
>> > reserved (implicit for future standard implementation?). Do we know of
>> > any cases, where a new standard version would introduce new values beyond
>> > what was reserved in the first place for future use? I dont know myself.
>> >
>> > I am just trying to raise a question of whether using the std APP attr
>> > with a new high (255) selector, really could be preferred over this new
>> > non-std APP attr with new packed payload.
>> 
>> Yeah. We'll need to patch lldpad anyway. We can basically choose which
>> way we patch it. And BTW, using the too-short attribute payload of
>> course breaks it _as well_, because they don't do any payload size
>> validation.
>
> Right, unless we reconstruct std app entry payload from the "too-short"
> attribute payload, before adding it the the app list or passing it to the 
> driver.

The issue is not in drivers, but in lldpad itself. They just iterate the
attribute stream, assume that everything is an APP, and treat is as
such, not even validating the length (I mean, why would they, it's
supposed to be an APP after all...).

So we trip lldpad however we set the API up.

So let's trip it in a way that makes for a reasonable API.

> Anyway. Considering Jakub's mail. I think this patch version with a non-std
> attribute to do non-std app table contributions separates non-std from std
> stuff nicely and is preffered over just adding the new selector. So if we can 
> agree on this, I will prepare a new v. with the other changes suggested.
>
> Wrt. lldpad we can then patch it to react on attrs or selectors > 7.

Yep, agreed.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel