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

Message ID 20221028100320.786984-2-daniel.machon@microchip.com
State New
Headers
Series Add new PCP and APPTRUST attributes to dcbnl |

Commit Message

Daniel Machon Oct. 28, 2022, 10:03 a.m. UTC
  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(-)
  

Comments

Petr Machata Oct. 31, 2022, 11:13 a.m. UTC | #1
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)
>  {
  
Daniel Machon Oct. 31, 2022, 12:39 p.m. UTC | #2
> > 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.
  

Patch

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