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>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <Lars.Povlsen@microchip.com>, <Steen.Hegelund@microchip.com>,
        <UNGLinuxDriver@microchip.com>, <joe@perches.com>,
        <error27@gmail.com>, <Horatiu.Vultur@microchip.com>,
        <Julia.Lawall@inria.fr>, <vladimir.oltean@nxp.com>,
        <maxime.chevallier@bootlin.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 2/6] net: dcb: add new common function for
 set/del of app/rewr entries
Thread-Topic: [PATCH net-next 2/6] net: dcb: add new common function for
 set/del of app/rewr entries
Thread-Index: AQHZJsEEnFbJ+CUja0WnZfWTFKCrSq6cb8yAgASXnoA=
Date:   Mon, 16 Jan 2023 13:00:10 +0000
Message-ID: <Y8VKWcqGEjPOvCQE@DEN-LT-70577>
References: <20230112201554.752144-1-daniel.machon@microchip.com>
 <20230112201554.752144-3-daniel.machon@microchip.com>
 <875ydazcfa.fsf@nvidia.com>
In-Reply-To: <875ydazcfa.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?sMSdklfJ0pcP6yWanZVROG03v4T5D7DUJUuLhSKFm8VhpLJfdqpa5HvVY+5T?=
 =?us-ascii?Q?avJw9B+W/7RRuoP6Qge2sC28ty0mj8VyAwuoMhGSS90EFh2Q1XHvJE6IPu/m?=
 =?us-ascii?Q?AOxI35Z4kn8S2d6MvTuWqmaXrjR23fTIijAMLdLuLgNOppX+idQ8KUhzpueE?=
 =?us-ascii?Q?sletCtt2xt11eb0qXlNwW1zbrG1IjtJKFRiF2zdhfQTJXD6XJSruJahJAzZd?=
 =?us-ascii?Q?+ape/3xQzHcPBATQgfcrWFAz63EKS2EhgfbFCV9u/P8AdtdolH2OomnWweZ4?=
 =?us-ascii?Q?NnVusy8+96y9U1/4IIbJwCkKk0KQP14NLjdzmy4jtem24sr2bxSM8FXSN/TM?=
 =?us-ascii?Q?Qzgd1519d8JCeDahVI6Bv+Q9jm8yyptYcSN5lRgQs6gEU7VTy7JqkWijqLey?=
 =?us-ascii?Q?wQZubGqcxHQjnDJYgFe/2B56xf3crAgcAK+4fPBc0WJaXzSp8Esm4QBoVzMZ?=
 =?us-ascii?Q?3VHnSYU+MKw79XoO2J4F0Sh6mj82boBbaL6CfzApK4F1OCJAsZ3GUW09Z6Yt?=
 =?us-ascii?Q?FHp5dTgLsR7WxU5FDivrlY/vIq6dK1onbQfyRH95LqgBlHr1nPfj4OOq+zWR?=
 =?us-ascii?Q?0jBpliRFiSjUOK2vfNc97Amc26X3mzsUM6p0fWs0I8dziYbtmceuzScF6/Wz?=
 =?us-ascii?Q?IJITbkqa31CI6StB6XDQzEEt0Oi1Ve4S61H+j+tnx94Q168nRBQ85wO/L9lp?=
 =?us-ascii?Q?hYo5CJ4eDTH9/zlqdqUSsuHjbnZy6fcc3In8EpjV0y76yE34RrdTftaq0B8l?=
 =?us-ascii?Q?Z4naRh7UAXdqsKjBuI10k8pFWBLXE1xXNmTs6I8OPsJPg4oezYKrKaw+lsC7?=
 =?us-ascii?Q?U2sRbTfuburVTirzW6zgIhqnqHnKaLPJtSrBmDEGh4GjtVcn9JwtYFeDP0b4?=
 =?us-ascii?Q?opfs+aEhh00AbyPlHnNOcrRHGFU73vCdI+ki3jYp6NUdwHwcjjPtvZNkwK3V?=
 =?us-ascii?Q?HEA8B+wv9gMqw9o27f0xcSm6GTPdU6B8R6fZZnGIVvksHF7sl826f/jlq0ZO?=
 =?us-ascii?Q?ndJ5+5K1PJjG2jJstBPo8G6kv88tYk+EjWZBydXQQoRE7PMmSS9v8cKLxf7K?=
 =?us-ascii?Q?JBBZYw1BILYyLM4uaIVD+nFbe7AfF4jRrNGq53CtI4n1ho742mA46beWaEj+?=
 =?us-ascii?Q?ZSfOauQvn+L4TIchrLvApXKAGmvbKClpGYv6aM1Nb5P4VRhUz60V8C66cfwD?=
 =?us-ascii?Q?le8xS9gBNFwPCGOxnAQ7x0cCn98xGr5C6qguIUcBLzHNRDFckU/Dzct07eQj?=
 =?us-ascii?Q?pn888wEpT2QY49UkGRfPDbQmfds7Aw8MoxX+SHF2Di6QDqystVDmPEU8vWWh?=
 =?us-ascii?Q?lzL7mVKbPqWs2BE5xEEQPUSVlpltdbjSQRbUaQor2mPdzURgEkaXyg4JpOla?=
 =?us-ascii?Q?M26rictNiIMAR5wf8PtTkMeIl4DbCgXQthoCHgsM/ombjkIMExE4JWU3pbIo?=
 =?us-ascii?Q?nuggS1NmqZOSBVg6qnGqbyVgqMjs9Jb8/AtF6rI63ue+MdB8TrLvAswvtt1P?=
 =?us-ascii?Q?RKhwamta3eeQLoVSKpFVFeDBmqQHj+SKSGHqARarbSHU0BysWR4FX11xLcDt?=
 =?us-ascii?Q?3UCFcK2kYq91QN2p7anwCOJ1hXlxUfOr1a17xSBbH3yKf/t09xRbE7uex1CN?=
 =?us-ascii?Q?4egevp7LmVfwhsSeliFsWk4=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <F2524BD175D2094BB69A8F3978D0678E@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: 453dd267-5859-4a36-1a23-08daf7c199ed
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jan 2023 13:00:10.9079
 (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: LfBcFxLlfiFsClAX25q84MRl0xZik4AoOY6O3d2D/9SlKLyP1+2TBE2YrSUXkU5qNv+OPyjTSNdd7y/T8zVT94OsbRqiJQyijIsHgMFZsRg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6706
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 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

> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  net/dcb/dcbnl.c | 104 +++++++++++++++++++++++-------------------------
> >  1 file changed, 49 insertions(+), 55 deletions(-)
> >
> > diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> > index a76bdf6f0198..6d19564e19a8 100644
> > --- a/net/dcb/dcbnl.c
> > +++ b/net/dcb/dcbnl.c
> > @@ -1099,6 +1099,45 @@ static int dcbnl_getapptrust(struct net_device *=
netdev, struct sk_buff *skb)
> >       return err;
> >  }
> >
> > +/* Set or delete APP table or rewrite table entries. The APP struct is=
 validated
> > + * and the appropriate callback function is called.
> > + */
> > +static int dcbnl_apprewr_setdel(struct nlattr *attr, struct net_device=
 *netdev,
> > +                             int (*setdel)(struct net_device *dev,
> > +                                           struct dcb_app *app),
> > +                             int (*ops_setdel)(struct net_device *dev,
> > +                                               struct dcb_app *app))
>=20
> The name makes it look like it's rewrite-specific. Maybe make it
> _app_table_? Given both DCB app and DCB rewrite use app table as the
> database... Dunno. Not a big deal.

Yeah, the other shared funcs are called _app_* too, so lets stick with
that naming. Agreed.

> >  /* Handle IEEE 802.1Qaz/802.1Qau/802.1Qbb GET commands. */
> >  static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *net=
dev)
> >  {
> > @@ -1568,36 +1607,11 @@ static int dcbnl_ieee_set(struct net_device *ne=
tdev, struct nlmsghdr *nlh,
> >       }
> >
> >       if (ieee[DCB_ATTR_IEEE_APP_TABLE]) {
> > -             struct nlattr *attr;
> > -             int rem;
> > -
> > -             nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], =
rem) {
> > -                     enum ieee_attrs_app type =3D nla_type(attr);
> > -                     struct dcb_app *app_data;
> > -
> > -                     if (!dcbnl_app_attr_type_validate(type))
> > -                             continue;
> > -
> > -                     if (nla_len(attr) < sizeof(struct dcb_app)) {
> > -                             err =3D -ERANGE;
> > -                             goto err;
> > -                     }
> > -
> > -                     app_data =3D nla_data(attr);
> > -
> > -                     if (!dcbnl_app_selector_validate(type,
> > -                                                      app_data->select=
or)) {
> > -                             err =3D -EINVAL;
> > -                             goto err;
> > -                     }
> > -
> > -                     if (ops->ieee_setapp)
> > -                             err =3D ops->ieee_setapp(netdev, app_data=
);
> > -                     else
> > -                             err =3D dcb_ieee_setapp(netdev, app_data)=
;
> > -                     if (err)
> > -                             goto err;
> > -             }
> > +             err =3D dcbnl_apprewr_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE=
],
> > +                                        netdev, dcb_ieee_setapp,
> > +                                        ops->ieee_setapp);
>=20
> This could pre-resolve the callback to use and pass one pointer:
>=20
>                 err =3D dcbnl_apprewr_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE=
], netdev,
>                                            ops->ieee_setapp ?: dcb_ieee_s=
etapp);

Agreed.=20

Will make the suggested changes in v2 - thank you Petr.

/Daniel