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

[<Daniel.Machon@microchip.com>]

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