| Message ID | 20221028100320.786984-2-daniel.machon@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add new PCP and APPTRUST attributes to dcbnl | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next |
Daniel Machon <daniel.machon@microchip.com> writes: > diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c > index dc4fb699b56c..68e033a459af 100644 > --- a/net/dcb/dcbnl.c > +++ b/net/dcb/dcbnl.c > @@ -179,6 +179,57 @@ 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 enum ieee_attrs_app dcbnl_app_attr_type_get(u8 selector) > +{ > + switch (selector) { > + case IEEE_8021QAZ_APP_SEL_ETHERTYPE: > + case IEEE_8021QAZ_APP_SEL_STREAM: > + case IEEE_8021QAZ_APP_SEL_DGRAM: > + case IEEE_8021QAZ_APP_SEL_ANY: > + case IEEE_8021QAZ_APP_SEL_DSCP: > + return DCB_ATTR_IEEE_APP; > + case DCB_APP_SEL_PCP: > + return DCB_ATTR_DCB_APP; > + default: > + return DCB_ATTR_IEEE_APP_UNSPEC; > + } > +} > + > +static bool dcbnl_app_attr_type_validate(enum ieee_attrs_app type) > +{ > + switch (type) { > + case DCB_ATTR_IEEE_APP: > + case DCB_ATTR_DCB_APP: > + return true; > + default: > + return false; > + } > +} > + > +static bool dcbnl_app_selector_validate(enum ieee_attrs_app type, u32 selector) > +{ > + switch (selector) { > + case IEEE_8021QAZ_APP_SEL_ETHERTYPE: > + case IEEE_8021QAZ_APP_SEL_STREAM: > + case IEEE_8021QAZ_APP_SEL_DGRAM: > + case IEEE_8021QAZ_APP_SEL_ANY: > + case IEEE_8021QAZ_APP_SEL_DSCP: > + /* IEEE std selectors in IEEE std attribute */ > + if (type == DCB_ATTR_IEEE_APP) > + return true; > + else > + return false; AKA return type == DCB_ATTR_IEEE_APP; > + case DCB_APP_SEL_PCP: > + /* Non-std selectors in non-std attribute */ > + if (type == DCB_ATTR_DCB_APP) > + return true; > + else > + return false; Likewise here. > + default: > + return false; > + } Also, it really looks like the following would be equivalent? static bool dcbnl_app_selector_validate(enum ieee_attrs_app type, u32 selector) { return dcbnl_app_attr_type_get(selector) == type; } Also, shouldn't it be u8 selector? > +} > + > static struct sk_buff *dcbnl_newmsg(int type, u8 cmd, u32 port, u32 seq, > u32 flags, struct nlmsghdr **nlhp) > {
> > diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c > > index dc4fb699b56c..68e033a459af 100644 > > --- a/net/dcb/dcbnl.c > > +++ b/net/dcb/dcbnl.c > > @@ -179,6 +179,57 @@ 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 enum ieee_attrs_app dcbnl_app_attr_type_get(u8 selector) > > +{ > > + switch (selector) { > > + case IEEE_8021QAZ_APP_SEL_ETHERTYPE: > > + case IEEE_8021QAZ_APP_SEL_STREAM: > > + case IEEE_8021QAZ_APP_SEL_DGRAM: > > + case IEEE_8021QAZ_APP_SEL_ANY: > > + case IEEE_8021QAZ_APP_SEL_DSCP: > > + return DCB_ATTR_IEEE_APP; > > + case DCB_APP_SEL_PCP: > > + return DCB_ATTR_DCB_APP; > > + default: > > + return DCB_ATTR_IEEE_APP_UNSPEC; > > + } > > +} > > + > > +static bool dcbnl_app_attr_type_validate(enum ieee_attrs_app type) > > +{ > > + switch (type) { > > + case DCB_ATTR_IEEE_APP: > > + case DCB_ATTR_DCB_APP: > > + return true; > > + default: > > + return false; > > + } > > +} > > + > > +static bool dcbnl_app_selector_validate(enum ieee_attrs_app type, u32 selector) > > +{ > > + switch (selector) { > > + case IEEE_8021QAZ_APP_SEL_ETHERTYPE: > > + case IEEE_8021QAZ_APP_SEL_STREAM: > > + case IEEE_8021QAZ_APP_SEL_DGRAM: > > + case IEEE_8021QAZ_APP_SEL_ANY: > > + case IEEE_8021QAZ_APP_SEL_DSCP: > > + /* IEEE std selectors in IEEE std attribute */ > > + if (type == DCB_ATTR_IEEE_APP) > > + return true; > > + else > > + return false; > > AKA return type == DCB_ATTR_IEEE_APP; > > > + case DCB_APP_SEL_PCP: > > + /* Non-std selectors in non-std attribute */ > > + if (type == DCB_ATTR_DCB_APP) > > + return true; > > + else > > + return false; > > Likewise here. > > > + default: > > + return false; > > + } > > Also, it really looks like the following would be equivalent? > > static bool dcbnl_app_selector_validate(enum ieee_attrs_app type, u32 selector) > { > return dcbnl_app_attr_type_get(selector) == type; > } Ah, yes. Less code. Much better indeed, thanks :-) > > Also, shouldn't it be u8 selector? Will be fixed in next version.
diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h index a791a94013a6..dc7ef96207ca 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 255 + /* 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..68e033a459af 100644 --- a/net/dcb/dcbnl.c +++ b/net/dcb/dcbnl.c @@ -179,6 +179,57 @@ 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 enum ieee_attrs_app dcbnl_app_attr_type_get(u8 selector) +{ + switch (selector) { + case IEEE_8021QAZ_APP_SEL_ETHERTYPE: + case IEEE_8021QAZ_APP_SEL_STREAM: + case IEEE_8021QAZ_APP_SEL_DGRAM: + case IEEE_8021QAZ_APP_SEL_ANY: + case IEEE_8021QAZ_APP_SEL_DSCP: + return DCB_ATTR_IEEE_APP; + case DCB_APP_SEL_PCP: + return DCB_ATTR_DCB_APP; + default: + return DCB_ATTR_IEEE_APP_UNSPEC; + } +} + +static bool dcbnl_app_attr_type_validate(enum ieee_attrs_app type) +{ + switch (type) { + case DCB_ATTR_IEEE_APP: + case DCB_ATTR_DCB_APP: + return true; + default: + return false; + } +} + +static bool dcbnl_app_selector_validate(enum ieee_attrs_app type, u32 selector) +{ + switch (selector) { + case IEEE_8021QAZ_APP_SEL_ETHERTYPE: + case IEEE_8021QAZ_APP_SEL_STREAM: + case IEEE_8021QAZ_APP_SEL_DGRAM: + case IEEE_8021QAZ_APP_SEL_ANY: + case IEEE_8021QAZ_APP_SEL_DSCP: + /* IEEE std selectors in IEEE std attribute */ + if (type == DCB_ATTR_IEEE_APP) + return true; + else + return false; + case DCB_APP_SEL_PCP: + /* Non-std selectors in non-std attribute */ + if (type == DCB_ATTR_DCB_APP) + return true; + else + return false; + default: + return false; + } +} + static struct sk_buff *dcbnl_newmsg(int type, u8 cmd, u32 port, u32 seq, u32 flags, struct nlmsghdr **nlhp) { @@ -1116,8 +1167,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; @@ -1493,9 +1545,10 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh, int rem; nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) { + enum ieee_attrs_app type = nla_type(attr); struct dcb_app *app_data; - if (nla_type(attr) != DCB_ATTR_IEEE_APP) + if (!dcbnl_app_attr_type_validate(type)) continue; if (nla_len(attr) < sizeof(struct dcb_app)) { @@ -1504,6 +1557,11 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh, } app_data = nla_data(attr); + + if (!dcbnl_app_selector_validate(type, + app_data->selector)) + return -EINVAL; + if (ops->ieee_setapp) err = ops->ieee_setapp(netdev, app_data); else @@ -1554,11 +1612,18 @@ static int dcbnl_ieee_del(struct net_device *netdev, struct nlmsghdr *nlh, int rem; nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) { + enum ieee_attrs_app type = nla_type(attr); struct dcb_app *app_data; - if (nla_type(attr) != DCB_ATTR_IEEE_APP) + if (!dcbnl_app_attr_type_validate(type)) continue; + app_data = nla_data(attr); + + if (!dcbnl_app_selector_validate(type, + app_data->selector)) + return -EINVAL; + if (ops->ieee_delapp) err = ops->ieee_delapp(netdev, app_data); else
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 new selector has been given a value of 255, to minimize the risk of future overlap of std- and non-std attributes. The new DCB_ATTR_DCB_APP is sent alongside the ieee std attribute in the app table. This means that the dcb_app struct can now both contain std- and non-std app attributes. Currently there is no overlap between the selector values of the two attributes. 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 | 73 +++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 4 deletions(-)