* [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute @ 2022-09-15 9:57 Daniel Machon 2022-09-15 9:57 ` [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object Daniel Machon ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Daniel Machon @ 2022-09-15 9:57 UTC (permalink / raw) To: netdev Cc: Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, petrm, kuba, vinicius.gomes, thomas.petazzoni, Daniel Machon This patch series adds support for offloading PCP-based queue classification and introduces a new APPTRUST extension attribute to the 8021Qaz APP managed object. Prior to implemenation, it has been discussed on the netdev mailing list here: https://lore.kernel.org/netdev/Yv9VO1DYAxNduw6A@DEN-LT-70577/ In summary: there currently exist no conveinent 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 has been assigned a value of 255 to avoid any clashes with future DCB standard extensions. Selector trust: ASIC's often has the notion of trust DSCP and trust PCP. This new object makes it possible to specify a trust order of app selectors, which drivers can then react on. 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. The userspace part of this will be posted in a separate patch series. ================================================================================ 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. Daniel Machon (2): net: dcb: add new pcp selector to app object net: dcb: add new apptrust attribute include/net/dcbnl.h | 5 +++ include/uapi/linux/dcbnl.h | 5 +++ net/dcb/dcbnl.c | 66 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 73 insertions(+), 3 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object 2022-09-15 9:57 [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Daniel Machon @ 2022-09-15 9:57 ` Daniel Machon 2022-09-19 9:45 ` Petr Machata 2022-09-15 9:57 ` [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute Daniel Machon 2022-09-19 7:54 ` [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Petr Machata 2 siblings, 1 reply; 11+ messages in thread From: Daniel Machon @ 2022-09-15 9:57 UTC (permalink / raw) To: netdev Cc: Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, petrm, kuba, vinicius.gomes, thomas.petazzoni, Daniel Machon Add new PCP selector for the 8021Qaz APP managed object. 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}. While PCP is not a standard 8021Qaz selector, it seems very convenient to add it to the APP object, as this is where similar priority mapping is handled, and it perfectly fits the {selector, protocol, priority} triplet. Signed-off-by: Daniel Machon <daniel.machon@microchip.com> --- include/uapi/linux/dcbnl.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h index a791a94013a6..8eab16e5bc13 100644 --- a/include/uapi/linux/dcbnl.h +++ b/include/uapi/linux/dcbnl.h @@ -217,6 +217,7 @@ struct cee_pfc { #define IEEE_8021QAZ_APP_SEL_DGRAM 3 #define IEEE_8021QAZ_APP_SEL_ANY 4 #define IEEE_8021QAZ_APP_SEL_DSCP 5 +#define IEEE_8021QAZ_APP_SEL_PCP 255 /* This structure contains the IEEE 802.1Qaz APP managed object. This * object is also used for the CEE std as well. -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object 2022-09-15 9:57 ` [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object Daniel Machon @ 2022-09-19 9:45 ` Petr Machata 2022-09-28 13:52 ` Daniel.Machon 0 siblings, 1 reply; 11+ messages in thread From: Petr Machata @ 2022-09-19 9:45 UTC (permalink / raw) To: Daniel Machon Cc: netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, petrm, kuba, vinicius.gomes, thomas.petazzoni Daniel Machon <daniel.machon@microchip.com> writes: > diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h > index a791a94013a6..8eab16e5bc13 100644 > --- a/include/uapi/linux/dcbnl.h > +++ b/include/uapi/linux/dcbnl.h > @@ -217,6 +217,7 @@ struct cee_pfc { > #define IEEE_8021QAZ_APP_SEL_DGRAM 3 > #define IEEE_8021QAZ_APP_SEL_ANY 4 > #define IEEE_8021QAZ_APP_SEL_DSCP 5 > +#define IEEE_8021QAZ_APP_SEL_PCP 255 > > /* This structure contains the IEEE 802.1Qaz APP managed object. This > * object is also used for the CEE std as well. One more thought: please verify how this behaves with openlldpad. It's a fairly major user of this API. I guess it is OK if it refuses to run or bails out in face of the PCP APP entries. On its own it will never introduce them, so this clear and noisy diagnostic when a user messes with the system through a different channels is OK IMHO. But it shouldn't silently reinterpret the 255 to mean something else. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object 2022-09-19 9:45 ` Petr Machata @ 2022-09-28 13:52 ` Daniel.Machon 2022-09-28 15:50 ` Petr Machata 0 siblings, 1 reply; 11+ messages in thread From: Daniel.Machon @ 2022-09-28 13:52 UTC (permalink / raw) To: petrm Cc: netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, kuba, vinicius.gomes, thomas.petazzoni Den Mon, Sep 19, 2022 at 11:45:41AM +0200 skrev Petr Machata: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Daniel Machon <daniel.machon@microchip.com> writes: > > > diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h > > index a791a94013a6..8eab16e5bc13 100644 > > --- a/include/uapi/linux/dcbnl.h > > +++ b/include/uapi/linux/dcbnl.h > > @@ -217,6 +217,7 @@ struct cee_pfc { > > #define IEEE_8021QAZ_APP_SEL_DGRAM 3 > > #define IEEE_8021QAZ_APP_SEL_ANY 4 > > #define IEEE_8021QAZ_APP_SEL_DSCP 5 > > +#define IEEE_8021QAZ_APP_SEL_PCP 255 > > > > /* This structure contains the IEEE 802.1Qaz APP managed object. This > > * object is also used for the CEE std as well. > > One more thought: please verify how this behaves with openlldpad. > It's a fairly major user of this API. > > I guess it is OK if it refuses to run or bails out in face of the PCP > APP entries. On its own it will never introduce them, so this clear and > noisy diagnostic when a user messes with the system through a different > channels is OK IMHO. > > But it shouldn't silently reinterpret the 255 to mean something else. Hi Petr, Looks like we are in trouble here: https://github.com/openSUSE/lldpad/blob/master/lldp_8021qaz.c#L911 protocol is shifted and masked with selector to fit in u8. Same u8 value is being transmitted in the APP TLVs. A dscp mapping of 10:7 will become (7 << 5) | 5 = e5 A pcp mapping of 1:1 will become (1 << 5) | ff = ff (always) Looks like the loop does not even check for DCB_ATTR_IEEE_APP, so putting the pcp stuff in a non-standard attribute in the DCB_ATTR_IEEE_APP_TABLE wont work either. The pcp selector will have to fit in 5 bits (0x1f instead of 0xff) to not interfere with the priority in lldapd. Thoughts? / Daniel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object 2022-09-28 13:52 ` Daniel.Machon @ 2022-09-28 15:50 ` Petr Machata 0 siblings, 0 replies; 11+ messages in thread From: Petr Machata @ 2022-09-28 15:50 UTC (permalink / raw) To: Daniel.Machon Cc: petrm, netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, kuba, vinicius.gomes, thomas.petazzoni <Daniel.Machon@microchip.com> writes: > Den Mon, Sep 19, 2022 at 11:45:41AM +0200 skrev Petr Machata: >> >> Daniel Machon <daniel.machon@microchip.com> writes: >> >> > diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h >> > index a791a94013a6..8eab16e5bc13 100644 >> > --- a/include/uapi/linux/dcbnl.h >> > +++ b/include/uapi/linux/dcbnl.h >> > @@ -217,6 +217,7 @@ struct cee_pfc { >> > #define IEEE_8021QAZ_APP_SEL_DGRAM 3 >> > #define IEEE_8021QAZ_APP_SEL_ANY 4 >> > #define IEEE_8021QAZ_APP_SEL_DSCP 5 >> > +#define IEEE_8021QAZ_APP_SEL_PCP 255 >> > >> > /* This structure contains the IEEE 802.1Qaz APP managed object. This >> > * object is also used for the CEE std as well. >> >> One more thought: please verify how this behaves with openlldpad. >> It's a fairly major user of this API. >> >> I guess it is OK if it refuses to run or bails out in face of the PCP >> APP entries. On its own it will never introduce them, so this clear and >> noisy diagnostic when a user messes with the system through a different >> channels is OK IMHO. >> >> But it shouldn't silently reinterpret the 255 to mean something else. > > Hi Petr, > > Looks like we are in trouble here: > > https://github.com/openSUSE/lldpad/blob/master/lldp_8021qaz.c#L911 > > protocol is shifted and masked with selector to fit in u8. Same u8 > value is being transmitted in the APP TLVs. > > A dscp mapping of 10:7 will become (7 << 5) | 5 = e5 > A pcp mapping of 1:1 will become (1 << 5) | ff = ff (always) > > Looks like the loop does not even check for DCB_ATTR_IEEE_APP, so putting > the pcp stuff in a non-standard attribute in the DCB_ATTR_IEEE_APP_TABLE > wont work either. Ho hum. Yeah, they are reconstructing the APP TLV in place. The format is three bits of priority, two bits reserved, three bits of selector. Hence the priority << 5. I guess the question is how far do we go to maintain the exact same behavior for broken userspace. Attributes exist exactly to make future extensions possible. If a userspace decides to reinterpret random bytes, I feel like that's on them. But checking my what-would-Linus-do wristband, I'm not 100% sure ;) > The pcp selector will have to fit in 5 bits (0x1f instead of 0xff) to not > interfere with the priority in lldapd. Yeah, but then it ends up shifting into the reserved field of the TLV, which is also a breakage. Plus, if ever the standard needs more space to support 16 priorities or 16 or 32 selectors, the reserved bits are where they go. So 31 as a selector value is not far enough from the standard stuff to be safe as an extension value. Um, like, I think we are not in the wrong here, and userspace goes above and beyond to be broken. So adding a new attribute and patching openlldp to ignore / bounce the non-standard stuff seems OK. Within the new attribute, we can use a value such as 24, because 24&7 == 0, which is currently reserved, and IMHO likely to stay that way. So old openlldp on new Linux with the PCP rules configured would send broken APP TLVs, but they would be broken in a fairly conspicuous manner. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute 2022-09-15 9:57 [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Daniel Machon 2022-09-15 9:57 ` [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object Daniel Machon @ 2022-09-15 9:57 ` Daniel Machon 2022-09-15 9:51 ` Vladimir Oltean 2022-09-19 7:30 ` Petr Machata 2022-09-19 7:54 ` [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Petr Machata 2 siblings, 2 replies; 11+ messages in thread From: Daniel Machon @ 2022-09-15 9:57 UTC (permalink / raw) To: netdev Cc: Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, petrm, kuba, vinicius.gomes, thomas.petazzoni, 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 (TRUST_TABLE), 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 to be trusted - and also the order of precedence of selectors. Signed-off-by: Daniel Machon <daniel.machon@microchip.com> --- include/net/dcbnl.h | 4 +++ include/uapi/linux/dcbnl.h | 4 +++ net/dcb/dcbnl.c | 64 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 69 insertions(+), 3 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 8eab16e5bc13..ede72aefd88f 100644 --- a/include/uapi/linux/dcbnl.h +++ b/include/uapi/linux/dcbnl.h @@ -248,6 +248,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 * @@ -419,6 +421,8 @@ enum ieee_attrs { DCB_ATTR_IEEE_QCN, DCB_ATTR_IEEE_QCN_STATS, DCB_ATTR_DCB_BUFFER, + DCB_ATTR_DCB_APP_TRUST_TABLE, + DCB_ATTR_DCB_APP_TRUST, __DCB_ATTR_IEEE_MAX }; #define DCB_ATTR_IEEE_MAX (__DCB_ATTR_IEEE_MAX - 1) diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c index dc4fb699b56c..6f5d2b295d09 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. */ @@ -1030,11 +1031,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 nlattr *ieee, *app, *apptrust; struct dcb_app_type *itr; const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops; int dcbx; - int err; + int err, i; if (nla_put_string(skb, DCB_ATTR_IFNAME, netdev->name)) return -EMSGSIZE; @@ -1133,6 +1134,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_noflag(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; @@ -1427,7 +1446,7 @@ 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 prio; - int err; + int err, i; if (!ops) return -EOPNOTSUPP; @@ -1513,6 +1532,45 @@ 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); + /* 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute 2022-09-15 9:57 ` [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute Daniel Machon @ 2022-09-15 9:51 ` Vladimir Oltean 2022-09-19 7:30 ` Petr Machata 1 sibling, 0 replies; 11+ messages in thread From: Vladimir Oltean @ 2022-09-15 9:51 UTC (permalink / raw) To: Daniel Machon Cc: netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, petrm, kuba, vinicius.gomes, thomas.petazzoni On Thu, Sep 15, 2022 at 11:57:57AM +0200, Daniel Machon wrote: > + if (ops->dcbnl_getapptrust) { > + u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0}; > + int nselectors; > + > + apptrust = nla_nest_start_noflag(skb, > + DCB_ATTR_DCB_APP_TRUST_TABLE); > + if (!app) > + return -EMSGSIZE; Should new code be creating nests without the NLA_F_NESTED flag? > + > + 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); > + } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute 2022-09-15 9:57 ` [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute Daniel Machon 2022-09-15 9:51 ` Vladimir Oltean @ 2022-09-19 7:30 ` Petr Machata 1 sibling, 0 replies; 11+ messages in thread From: Petr Machata @ 2022-09-19 7:30 UTC (permalink / raw) To: Daniel Machon Cc: netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, petrm, kuba, vinicius.gomes, thomas.petazzoni 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 (TRUST_TABLE), 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 > to be trusted - and also the order of precedence of selectors. > > Signed-off-by: Daniel Machon <daniel.machon@microchip.com> > --- > include/net/dcbnl.h | 4 +++ > include/uapi/linux/dcbnl.h | 4 +++ > net/dcb/dcbnl.c | 64 ++++++++++++++++++++++++++++++++++++-- > 3 files changed, 69 insertions(+), 3 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 8eab16e5bc13..ede72aefd88f 100644 > --- a/include/uapi/linux/dcbnl.h > +++ b/include/uapi/linux/dcbnl.h > @@ -248,6 +248,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 > * > @@ -419,6 +421,8 @@ enum ieee_attrs { > DCB_ATTR_IEEE_QCN, > DCB_ATTR_IEEE_QCN_STATS, > DCB_ATTR_DCB_BUFFER, > + DCB_ATTR_DCB_APP_TRUST_TABLE, > + DCB_ATTR_DCB_APP_TRUST, > __DCB_ATTR_IEEE_MAX > }; I find it odd that APP_TRUST is at the same level as APP_TRUST_TABLE. I think it would be better to have a separate enum ala what APP_TABLE has with enum ieee_attr_app. > #define DCB_ATTR_IEEE_MAX (__DCB_ATTR_IEEE_MAX - 1) > diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c > index dc4fb699b56c..6f5d2b295d09 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. */ > @@ -1030,11 +1031,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 nlattr *ieee, *app, *apptrust; > struct dcb_app_type *itr; > const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops; > int dcbx; > - int err; > + int err, i; Move "err, i" up before dcbx so that it observes the reverse xmass tree principle, please. > > if (nla_put_string(skb, DCB_ATTR_IFNAME, netdev->name)) > return -EMSGSIZE; > @@ -1133,6 +1134,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_noflag(skb, > + DCB_ATTR_DCB_APP_TRUST_TABLE); > + if (!app) > + return -EMSGSIZE; I agree with Vladimir that this should not be _noflag. > + > + 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; > @@ -1427,7 +1446,7 @@ 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 prio; > - int err; > + int err, i; RXT this as well, please. > > if (!ops) > return -EOPNOTSUPP; > @@ -1513,6 +1532,45 @@ 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; > + } I'm assuming you tested this code, I just wrote it into the email client, so all guarantees are off :) > + > + selector = nla_get_u8(attr); > + /* Duplicate selector ? */ > + for (i = 0; i < nselectors; i++) { > + if (selectors[i] == selector) { > + err = -EINVAL; > + goto err; > + } > + } This should validate the selector values as well IMHO. Maybe something like this? switch (selector) { ... case IEEE_8021QAZ_APP_SEL_DGRAM: case IEEE_8021QAZ_APP_SEL_ANY: case IEEE_8021QAZ_APP_SEL_DSCP: case IEEE_8021QAZ_APP_SEL_PCP: break; default: 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); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute 2022-09-15 9:57 [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Daniel Machon 2022-09-15 9:57 ` [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object Daniel Machon 2022-09-15 9:57 ` [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute Daniel Machon @ 2022-09-19 7:54 ` Petr Machata 2022-09-19 8:13 ` Daniel.Machon 2 siblings, 1 reply; 11+ messages in thread From: Petr Machata @ 2022-09-19 7:54 UTC (permalink / raw) To: Daniel Machon Cc: netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, petrm, kuba, vinicius.gomes, thomas.petazzoni Thanks, this looks good to me overall, despite the several points Vladimir and I raised. I think it would be good to send this as non-RFC. Note that for the non-RFC version, an actual user of the interface needs to be present as well. So one of the offloading drivers should be adapted to make use of the APP_TRUST and the new PCP selector. mlxsw would like to make use of both, but I don't know when I will have time to implement that. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute 2022-09-19 7:54 ` [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Petr Machata @ 2022-09-19 8:13 ` Daniel.Machon 2022-09-19 15:11 ` Petr Machata 0 siblings, 1 reply; 11+ messages in thread From: Daniel.Machon @ 2022-09-19 8:13 UTC (permalink / raw) To: petrm Cc: netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, kuba, vinicius.gomes, thomas.petazzoni Den Mon, Sep 19, 2022 at 09:54:23AM +0200 skrev Petr Machata: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Thanks, this looks good to me overall, despite the several points > Vladimir and I raised. I think it would be good to send this as non-RFC. > > Note that for the non-RFC version, an actual user of the interface needs > to be present as well. So one of the offloading drivers should be > adapted to make use of the APP_TRUST and the new PCP selector. > mlxsw would like to make use of both, but I don't know when I will have > time to implement that. Sounds good, and thanks for reviewing to you both. I will go ahead and add support for this in the sparx5 driver - most of it is already implemented during the tests anyway. Should the driver support be posted together with said non-RFC patch series? / Daniel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute 2022-09-19 8:13 ` Daniel.Machon @ 2022-09-19 15:11 ` Petr Machata 0 siblings, 0 replies; 11+ messages in thread From: Petr Machata @ 2022-09-19 15:11 UTC (permalink / raw) To: Daniel.Machon Cc: petrm, netdev, Allan.Nielsen, UNGLinuxDriver, maxime.chevallier, vladimir.oltean, kuba, vinicius.gomes, thomas.petazzoni <Daniel.Machon@microchip.com> writes: > Den Mon, Sep 19, 2022 at 09:54:23AM +0200 skrev Petr Machata: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> Thanks, this looks good to me overall, despite the several points >> Vladimir and I raised. I think it would be good to send this as non-RFC. >> >> Note that for the non-RFC version, an actual user of the interface needs >> to be present as well. So one of the offloading drivers should be >> adapted to make use of the APP_TRUST and the new PCP selector. >> mlxsw would like to make use of both, but I don't know when I will have >> time to implement that. > > Sounds good, and thanks for reviewing to you both. > I will go ahead and add support for this in the sparx5 driver - most of it > is already implemented during the tests anyway. > > Should the driver support be posted together with said non-RFC patch > series? Yes. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-09-28 16:23 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-15 9:57 [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Daniel Machon 2022-09-15 9:57 ` [RFC PATCH v2 net-next 1/2] net: dcb: add new pcp selector to app object Daniel Machon 2022-09-19 9:45 ` Petr Machata 2022-09-28 13:52 ` Daniel.Machon 2022-09-28 15:50 ` Petr Machata 2022-09-15 9:57 ` [RFC PATCH v2 net-next 2/2] net: dcb: add new apptrust attribute Daniel Machon 2022-09-15 9:51 ` Vladimir Oltean 2022-09-19 7:30 ` Petr Machata 2022-09-19 7:54 ` [RFC PATCH v2 net-next 0/2] Add PCP selector and new APPTRUST attribute Petr Machata 2022-09-19 8:13 ` Daniel.Machon 2022-09-19 15:11 ` Petr Machata
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).