| Message ID | 20220915095757.2861822-3-daniel.machon@microchip.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add PCP selector and new APPTRUST attribute | expand |
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); > + }
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);
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);
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(-)