Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   <Daniel.Machon@microchip.com>
To:     <petrm@nvidia.com>
CC:     <netdev@vger.kernel.org>, <davem@davemloft.net>,
        <maxime.chevallier@bootlin.com>, <thomas.petazzoni@bootlin.com>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <Lars.Povlsen@microchip.com>, <Steen.Hegelund@microchip.com>,
        <UNGLinuxDriver@microchip.com>, <joe@perches.com>,
        <linux@armlinux.org.uk>, <Horatiu.Vultur@microchip.com>,
        <Julia.Lawall@inria.fr>, <vladimir.oltean@nxp.com>,
        <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
Subject: Re: [net-next v3 1/6] net: dcb: add new pcp selector to app object
Thread-Topic: [net-next v3 1/6] net: dcb: add new pcp selector to app object
Thread-Index: AQHY54e/oXdDISkOjUKo8XH7CRcTxq4gd3gAgAAV3AA=
Date:   Wed, 26 Oct 2022 11:19:26 +0000
Message-ID: <Y1kaErnPh5h4otWe@DEN-LT-70577>
References: <20221024091333.1048061-1-daniel.machon@microchip.com>
 <20221024091333.1048061-2-daniel.machon@microchip.com>
 <874jvq28l3.fsf@nvidia.com>
In-Reply-To: <874jvq28l3.fsf@nvidia.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Y9Fntu87VXdIM4rrAZr/V+rdwR4MN6PY9BawfbwNf74lB1Kq88J7ET4ZX054?=
 =?us-ascii?Q?02nqh96SbtVoBRpEJnjnCcEF94H3SEQgJDsHvUOzVnEXbHyo5JxtyBmDEUQw?=
 =?us-ascii?Q?564+0qATOdclbuFlcggN0I0H6T5W8RftKWvct1oMGSlqqdL5JDuqDECLOwF/?=
 =?us-ascii?Q?cq3dHtPI2zUyh40ERnh8L7lGkXa/lGfxUqSA6J9S2PIuJZ3Yo2WpmqDnJrzZ?=
 =?us-ascii?Q?hyxIRbgDK2CNUl1bqQ8NzOTAUMwuW+OlXmv3RzdiN4cUcpMLE3ycXbg2ZXAD?=
 =?us-ascii?Q?HLfIRBbMo5rPuMcfoGWa94+1/x+hzcNMSS8bLy0m2RQKljU4VntaCRk4/eyA?=
 =?us-ascii?Q?SurKuIaS2oPi19CAs7qf5jiBHNJlu+1bfBKGbtWCi+izMG7uIwbA0vwS1lOh?=
 =?us-ascii?Q?/LEET5OQO/RSOZJM0sODdD0FkJ/23ri8fRaCUucSvPQ6w8rDil/5W5VKt5U1?=
 =?us-ascii?Q?SQ6kDmL1jJOqiQQbRSTmHtCU1VyasUECX3rdHFg6Xf55rXcHpUFqOgWIwWKw?=
 =?us-ascii?Q?bMUasJgMDBSqbsneAc7BeLsmq39gk6gIG3ZQjxWo/wn/47jGYT9mUax/B7/1?=
 =?us-ascii?Q?GeGsyHP3DQEw7QufBDUkAK3KQ+EPuWlM4Gw1rEOIDLhRov4gRy4s+JzYzB4+?=
 =?us-ascii?Q?CMzjhPf4ZHuibiboD9ZtXXLknsWbY6px5JZwVqU34pvvllLDwh3DVTW0tIng?=
 =?us-ascii?Q?dc40C+SYRFoN4wgp72OBKCfKi7TVxr1qlqOXhbqJET3IVSGeCinDet8uMni0?=
 =?us-ascii?Q?FSjP497Jv4KahPoOUFlqa1j9TwnjT6CT3cTE1bvzp7+rbyNnLewwoiy9NNf2?=
 =?us-ascii?Q?FGE7KmGSpB0vRscTNIjz1Gqk99+ijyHWbXb4zNpm3RmVvHtoHL+43gHz7/eK?=
 =?us-ascii?Q?vlAlMBSIL8GP1hEDcXRgNndj0mwEmqKZfIfq5U+1aiiObC2vtNtG3ZwomTM0?=
 =?us-ascii?Q?R2+dD3EYKe0U26UDoga950iYbO0grI2kdHcWn8Kx9RFHegK73+YE+qFJ3sqS?=
 =?us-ascii?Q?NKIZakZ9bXh/VU+EZEflRTLuLZ9BLlqxV7er49vLUaiqoXCBCGEszbIATSYJ?=
 =?us-ascii?Q?upT1s4jKbKTisB2mmsRCiVEn/yfmch+2LJkxW7cleTP11vDTwyftzysRdOcT?=
 =?us-ascii?Q?uCUxbdh4cuKxzE43z44fTV+LcUrbzF7jddZvx7eBR4RHzSxpFNP7h12bSkuF?=
 =?us-ascii?Q?BEKTfy9uBbpO6ACV6y8m27kX1r9LD3f8koLBmll0onKD6w7i4gCmHt/5QNOY?=
 =?us-ascii?Q?cL/YcZv9db+ocvV0mqeaTZTl7Bqojj56MXJh3Flg3P0qbG4ypuztPOxac3Kv?=
 =?us-ascii?Q?2D+/LY4QjNRWQPm+8moS1yS6PKyCdCNSALWwAx/kACkIJgWZ4Fwpq7JweINx?=
 =?us-ascii?Q?yx713l61uDJjqJ0OOImxtyVgofYPJi1nHG+emK3twoe2hVPFIvSuYKUmcZNH?=
 =?us-ascii?Q?OkBOLCG1sEK22GJxl0GIwfoLZJvmn3qLo3qC7knT18ltHvsqX4CCUIWoRbAz?=
 =?us-ascii?Q?Tw89NT5DOp3EQMYD9/YCUSLKkfkX23FV/h/BvGtV3/GwxPisWtx1V8o2l6JR?=
 =?us-ascii?Q?9csDNewG64ghI8lDD1CxfyaIryWS/aExHOLAsHgWL8T2TXpOIZvlI/KgcGM1?=
 =?us-ascii?Q?tw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <FAA1B9A57DE24440BBFE276EC19C7CC2@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO6PR11MB5569.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 01127928-b20b-4c92-1f37-08dab743f14d
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Oct 2022 11:19:26.4776
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: TPPMSBFR0bY23qSu6tE2uXDxzvyYFhRFXgeUGtuObAD3YvN+0eSwWySQIdZU8pszoiUeAu4xqOeMO/Y60ykcxz13n/JAvrcaejB7SGJRu58=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4587
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> > 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 th=
e
> > 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>
>=20
> >  static struct sk_buff *dcbnl_newmsg(int type, u8 cmd, u32 port, u32 se=
q,
> >                                   u32 flags, struct nlmsghdr **nlhp)
> >  {
> > @@ -1116,8 +1143,9 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, s=
truct net_device *netdev)
> >       spin_lock_bh(&dcb_lock);
> >       list_for_each_entry(itr, &dcb_app_list, list) {
> >               if (itr->ifindex =3D=3D netdev->ifindex) {
> > -                     err =3D nla_put(skb, DCB_ATTR_IEEE_APP, sizeof(it=
r->app),
> > -                                      &itr->app);
> > +                     enum ieee_attrs_app type =3D
> > +                             dcbnl_app_attr_type_get(itr->app.selector=
);
> > +                     err =3D nla_put(skb, type, sizeof(itr->app), &itr=
->app);
> >                       if (err) {
> >                               spin_unlock_bh(&dcb_lock);
> >                               return -EMSGSIZE;
> > @@ -1495,7 +1523,7 @@ static int dcbnl_ieee_set(struct net_device *netd=
ev, struct nlmsghdr *nlh,
> >               nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], =
rem) {
> >                       struct dcb_app *app_data;
> >
> > -                     if (nla_type(attr) !=3D DCB_ATTR_IEEE_APP)
> > +                     if (!dcbnl_app_attr_type_validate(nla_type(attr))=
)
> >                               continue;
> >
> >                       if (nla_len(attr) < sizeof(struct dcb_app)) {
> > @@ -1556,7 +1584,7 @@ static int dcbnl_ieee_del(struct net_device *netd=
ev, struct nlmsghdr *nlh,
> >               nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], =
rem) {
> >                       struct dcb_app *app_data;
> >
> > -                     if (nla_type(attr) !=3D DCB_ATTR_IEEE_APP)
> > +                     if (!dcbnl_app_attr_type_validate(nla_type(attr))=
)
> >                               continue;
> >                       app_data =3D nla_data(attr);
> >                       if (ops->ieee_delapp)
>=20
> I'm missing a validation that DCB_APP_SEL_PCP is always sent in
> DCB_ATTR_DCB_APP encapsulation. Wouldn't the current code permit
> sending it in the IEEE encap? This should be forbidden.

Right. Current impl. does not check that the non-std selectors received, ar=
e
sent with a DCB_ATTR_DCB_APP type.  We could introduce a new check
dcbnl_app_attr_selector_validate() that checks combination of type and=20
selector, after the type and nla_len(attr) has been checked, so that:

 validate type -> validate nla_len(attr) -> validate selector

> And vice versa: I'm not sure we want to permit sending the standard
> attributes in the DCB encap.

dcbnl_app_attr_type_get() in dcbnl_ieee_fill() takes care of this. IEEE are
always sent in DCB_ATTR_IEEE and non-std are sent in DCB_ATTR_DCB.

/ Daniel