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: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for
 apptrust
Thread-Topic: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for
 apptrust
Thread-Index: AQHY1DNZUjZtLK0dzEirZyaNJNiZ+634H/uAgAQj3gA=
Date:   Mon, 3 Oct 2022 06:52:40 +0000
Message-ID: <YzqJEESxhwkcayjs@DEN-LT-70577>
References: <20220929185207.2183473-1-daniel.machon@microchip.com>
 <20220929185207.2183473-5-daniel.machon@microchip.com>
 <87zgegu9kq.fsf@nvidia.com>
In-Reply-To: <87zgegu9kq.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?PdCDmJfpfRTkWX6vxT1JJdofJD5+c6GyatKIaFTxcWNZZFJQM+lEUQ4j06ZW?=
 =?us-ascii?Q?eiBe5D1bBl5gOkDuAZMQLldAhMaVvU0leJBVCJ6c4OGI4xG0UN27sWyq8IVF?=
 =?us-ascii?Q?6WWZEWsGTg1EjruixsoRHBHMQSSPTj3ByOgtvZch1x0bqGxS1AeDpwvplPt4?=
 =?us-ascii?Q?5ZaSrP5sHa0g3Fbn0WQcIhiRmfqMiUDN/6ZQyHlrgh78ZIgds4BpqdIaNKZx?=
 =?us-ascii?Q?d8oY3tfxUG5GJJgBZRxkEPSG4BSyXFalAhZ2LH8FwMv7qF0f+wmu3QqGBeJ2?=
 =?us-ascii?Q?2x05kUEq4AQSlXEGiCczYfyJP3FldCwEhOKtUO3KJjdv52SKUIvGQ2kQHKwq?=
 =?us-ascii?Q?ARrSdChL8Fb2FpNEntWSM/UQ4J3FHHtV4J+PDLWvsYJYIXFV8WGmJyt+X+9H?=
 =?us-ascii?Q?XQJiUFdyVoZVk6QVX3G22KjTzDraXwqgYz4cR4UC2hgDA9eRIdw+slggW8V7?=
 =?us-ascii?Q?U5OEnT1xAynPQTR7mTvtUQYTdkoI4FtLKFJNleFN2kYo0WG8V7okbK5o6Han?=
 =?us-ascii?Q?+y55tchxyUINbO4X6tWrIpSiO/jluFyq7U3X+swdLPYrOtxU6Ao/mSo4lM7M?=
 =?us-ascii?Q?iWZkPmxRtcb7ioLFxe4wJ+7iOxtIxQ+L6nuFB6zIZmXTYBQZ9W0aaWJYDrUj?=
 =?us-ascii?Q?eUby1zERymv8+nbIuHhStWBx96Ie3l2VgS9LRKDkqfxUv2Un6cC7X078ozOt?=
 =?us-ascii?Q?1ey8wc3oUSsirq/vR955H0KRuUbh/UsfLv0ZNoJSDQONCbWywC5ITYSvZw7O?=
 =?us-ascii?Q?Vk9HSHYNodfBZijQu/hsKsVaqXEzWhGfkSjxeVUv0GsRuJ6CFiZ0180GGk+F?=
 =?us-ascii?Q?zrbqicO/84v9Z2F8q+je1NOGTdt1BncSINqZgvwV1yfNvLnjPV1uDCfnA68f?=
 =?us-ascii?Q?5JXiA0jvyoYfG65HhrZzwGXisogXKJdGpPM5yXDRcalubvOUsq/FUiYboUVa?=
 =?us-ascii?Q?QH/bDY39awJSp01pHTK7MvjZ1D29v6PELNzfE6d90jyTgeb48pFGeBt8Ug7z?=
 =?us-ascii?Q?fcChjEK7Jdiasp7xuu2hfShy62AZtT8ZMhDvaDYaPIkKLl006B9dUAycYcS4?=
 =?us-ascii?Q?krfOMkr48EALTGT3UX2fQJrjlJ9TB9ClFsaYrfjNvec1iYKVPLcrlutzS+kx?=
 =?us-ascii?Q?dh4FfFkqefK+YYoLVq6FM+ZNlfRj0BUiIgeUxE+50iIDlZ2pnkiEpPD3zj49?=
 =?us-ascii?Q?H4ajVNLJWsqaf1Wp28wFhpEn0sx6cfCya+z+Q11x50f30UeQB4oCCMjXNJ4P?=
 =?us-ascii?Q?k3jHteaoLObVPD0G96lzOlrQ2nDoXV1GtI0iOnFwhdMdoJimCTARLPoXhiHX?=
 =?us-ascii?Q?6mpIH4s7qlVFzkPxUtlJLtq+Nzwi1fdqUo9gBbho4Ca+nqQo4CDvTpqDhjD4?=
 =?us-ascii?Q?0afqcZNwPLofncaJ5OpYyYyt5yIx2WOZmL+OHlvPEcHWt45yVUkhjDNkCWws?=
 =?us-ascii?Q?8fjtcL+5cWgQTUdQnXkt+nav87aasDZkRagYvkaNBffCgi+hk4kJFZQAxM4C?=
 =?us-ascii?Q?GSV7FKQDWlyK0CFrSu/nj8oyVskU+3iZanaeMlwDJPTNNUvpe046baleBjf8?=
 =?us-ascii?Q?JtdR0CAwx+XNwrty/cHGBVyrvcgG3uQUXu99szfPS6sAVMr96OPkUpOziHfb?=
 =?us-ascii?Q?+UFFr/1m8BxDGGSvlM5yyO8=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <60338EEAB13D4E4F84E1B1B2752D1930@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5580.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5ac33ba4-3d5d-4de4-c8c2-08daa50bdd75
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Oct 2022 06:52:40.4429
 (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: 22b6suNOsO5JVSS3Vs/CysvHqdezmNG/3dLAAy85aqYAgp08P/WEk5OGmWWq09p2B0EMzuoFf/rrqBDt9KJ9ABwnD/l+O09renwzH1dY/JY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7003
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,SPF_HELO_PASS,
        SPF_PASS 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

> > Make use of set/getapptrust() to implement per-selector trust and trust
> > order.
> >
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
> >  .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
> >  .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
> >  .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
> >  .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
> >  5 files changed, 127 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drive=
rs/net/ethernet/microchip/sparx5/sparx5_dcb.c
> > index db17c124dac8..10aeb422b1ae 100644
> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> > @@ -8,6 +8,22 @@
> >
> >  #include "sparx5_port.h"
> >
> > +static const struct sparx5_dcb_apptrust {
> > +     u8 selectors[256];
> > +     int nselectors;
> > +     char *names;
>=20
> I think this should be just "name".

I dont think so. This is a str representation of all the selector values.
"names" makes more sense to me.

>=20
> > +} *apptrust[SPX5_PORTS];
> > +
> > +/* Sparx5 supported apptrust configurations */
> > +static const struct sparx5_dcb_apptrust apptrust_conf[4] =3D {
> > +     /* Empty *must* be first */
> > +     { { 0                         }, 0, "empty"    },
> > +     { { IEEE_8021QAZ_APP_SEL_DSCP }, 1, "dscp"     },
> > +     { { DCB_APP_SEL_PCP           }, 1, "pcp"      },
> > +     { { IEEE_8021QAZ_APP_SEL_DSCP,
> > +         DCB_APP_SEL_PCP           }, 2, "dscp pcp" },
> > +};
> > +
> >  /* Validate app entry.
> >   *
> >   * Check for valid selectors and valid protocol and priority ranges.
> > @@ -37,12 +53,59 @@ static int sparx5_dcb_app_validate(struct net_devic=
e *dev,
> >       return err;
> >  }
> >
> > +/* Validate apptrust configuration.
> > + *
> > + * Return index of supported apptrust configuration if valid, otherwis=
e return
> > + * error.
> > + */
> > +static int sparx5_dcb_apptrust_validate(struct net_device *dev, u8 *se=
lectors,
> > +                                     int nselectors, int *err)
> > +{
> > +     bool match;
> > +     int i, ii;
> > +
> > +     for (i =3D 0; i < ARRAY_SIZE(apptrust_conf); i++) {
>=20
> I would do this here:
>=20
>     if (apptrust_conf[i].nselectors !=3D nselectors) continue;
>=20
> to avoid having to think about the semantics of comparing to all those
> zeroes in apptrust_conf.selectors array.

Yes, that would be good.

>=20
> > +             match =3D true;
> > +             for (ii =3D 0; ii < nselectors; ii++) {
> > +                     if (apptrust_conf[i].selectors[ii] !=3D
> > +                         *(selectors + ii)) {
> > +                             match =3D false;
> > +                             break;
> > +                     }
> > +             }
> > +             if (match)
> > +                     break;
> > +     }
> > +
> > +     /* Requested trust configuration is not supported */
> > +     if (!match) {
> > +             netdev_err(dev, "Valid apptrust configurations are:\n");
> > +             for (i =3D 0; i < ARRAY_SIZE(apptrust_conf); i++)
> > +                     pr_info("order: %s\n", apptrust_conf[i].names);
> > +             *err =3D -EOPNOTSUPP;
> > +     }
> > +
> > +     return i;
> > +}=