| Message ID | 20221129000550.3833570-3-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: devlink: return the driver name in devlink_nl_info_fill | expand |
> -----Original Message----- > From: Vincent Mailhol <vincent.mailhol@gmail.com> On Behalf Of Vincent > Mailhol > Sent: Monday, November 28, 2022 4:06 PM > To: Jiri Pirko <jiri@nvidia.com>; netdev@vger.kernel.org; Jakub Kicinski > <kuba@kernel.org> > Cc: David S . Miller <davem@davemloft.net>; Eric Dumazet > <edumazet@google.com>; Paolo Abeni <pabeni@redhat.com>; linux- > kernel@vger.kernel.org; Boris Brezillon <bbrezillon@kernel.org>; Arnaud Ebalard > <arno@natisbad.org>; Srujana Challa <schalla@marvell.com>; Kurt Kanzenbach > <kurt@linutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > <f.fainelli@gmail.com>; Vladimir Oltean <olteanv@gmail.com>; Michael Chan > <michael.chan@broadcom.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; > Dimitris Michailidis <dmichail@fungible.com>; Yisen Zhuang > <yisen.zhuang@huawei.com>; Salil Mehta <salil.mehta@huawei.com>; > Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L > <anthony.l.nguyen@intel.com>; Sunil Goutham <sgoutham@marvell.com>; Linu > Cherian <lcherian@marvell.com>; Geetha sowjanya <gakula@marvell.com>; > Jerin Jacob <jerinj@marvell.com>; hariprasad <hkelam@marvell.com>; > Subbaraya Sundeep <sbhatta@marvell.com>; Taras Chornyi > <tchornyi@marvell.com>; Saeed Mahameed <saeedm@nvidia.com>; Leon > Romanovsky <leon@kernel.org>; Ido Schimmel <idosch@nvidia.com>; Petr > Machata <petrm@nvidia.com>; Simon Horman <simon.horman@corigine.com>; > Shannon Nelson <snelson@pensando.io>; drivers@pensando.io; Ariel Elior > <aelior@marvell.com>; Manish Chopra <manishc@marvell.com>; Jonathan > Lemon <jonathan.lemon@gmail.com>; Vadim Fedorenko <vadfed@fb.com>; > Richard Cochran <richardcochran@gmail.com>; Vadim Pasternak > <vadimp@mellanox.com>; Shalom Toledo <shalomt@mellanox.com>; linux- > crypto@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux- > rdma@vger.kernel.org; oss-drivers@corigine.com; Jiri Pirko > <jiri@mellanox.com>; Herbert Xu <herbert@gondor.apana.org.au>; Hao Chen > <chenhao288@hisilicon.com>; Guangbin Huang > <huangguangbin2@huawei.com>; Minghao Chi <chi.minghao@zte.com.cn>; > Shijith Thotton <sthotton@marvell.com>; Vincent Mailhol > <mailhol.vincent@wanadoo.fr> > Subject: [PATCH net-next v5 2/4] net: devlink: remove > devlink_info_driver_name_put() > > Now that the core sets the driver name attribute, drivers are not > supposed to call devlink_info_driver_name_put() anymore. Remove it. > You could combine this patch with the previous one so that in the event of a cherry-pick its not possible to have this function while the core inserts the driver name automatically. I think that also makes it very clear that there are no remaining in-tree drivers still calling the function. I don't have a strong preference if we prefer it being separated. Thanks, Jake
Hi Jacob, Thanks for the review! On Tue. 29 Nov. 2022 at 09:23, Keller, Jacob E <jacob.e.keller@intel.com> wrote: > > -----Original Message----- > > From: Vincent Mailhol <vincent.mailhol@gmail.com> On Behalf Of Vincent > > Mailhol > > Sent: Monday, November 28, 2022 4:06 PM > > To: Jiri Pirko <jiri@nvidia.com>; netdev@vger.kernel.org; Jakub Kicinski > > <kuba@kernel.org> > > Cc: David S . Miller <davem@davemloft.net>; Eric Dumazet > > <edumazet@google.com>; Paolo Abeni <pabeni@redhat.com>; linux- > > kernel@vger.kernel.org; Boris Brezillon <bbrezillon@kernel.org>; Arnaud Ebalard > > <arno@natisbad.org>; Srujana Challa <schalla@marvell.com>; Kurt Kanzenbach > > <kurt@linutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > > <f.fainelli@gmail.com>; Vladimir Oltean <olteanv@gmail.com>; Michael Chan > > <michael.chan@broadcom.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; > > Dimitris Michailidis <dmichail@fungible.com>; Yisen Zhuang > > <yisen.zhuang@huawei.com>; Salil Mehta <salil.mehta@huawei.com>; > > Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L > > <anthony.l.nguyen@intel.com>; Sunil Goutham <sgoutham@marvell.com>; Linu > > Cherian <lcherian@marvell.com>; Geetha sowjanya <gakula@marvell.com>; > > Jerin Jacob <jerinj@marvell.com>; hariprasad <hkelam@marvell.com>; > > Subbaraya Sundeep <sbhatta@marvell.com>; Taras Chornyi > > <tchornyi@marvell.com>; Saeed Mahameed <saeedm@nvidia.com>; Leon > > Romanovsky <leon@kernel.org>; Ido Schimmel <idosch@nvidia.com>; Petr > > Machata <petrm@nvidia.com>; Simon Horman <simon.horman@corigine.com>; > > Shannon Nelson <snelson@pensando.io>; drivers@pensando.io; Ariel Elior > > <aelior@marvell.com>; Manish Chopra <manishc@marvell.com>; Jonathan > > Lemon <jonathan.lemon@gmail.com>; Vadim Fedorenko <vadfed@fb.com>; > > Richard Cochran <richardcochran@gmail.com>; Vadim Pasternak > > <vadimp@mellanox.com>; Shalom Toledo <shalomt@mellanox.com>; linux- > > crypto@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux- > > rdma@vger.kernel.org; oss-drivers@corigine.com; Jiri Pirko > > <jiri@mellanox.com>; Herbert Xu <herbert@gondor.apana.org.au>; Hao Chen > > <chenhao288@hisilicon.com>; Guangbin Huang > > <huangguangbin2@huawei.com>; Minghao Chi <chi.minghao@zte.com.cn>; > > Shijith Thotton <sthotton@marvell.com>; Vincent Mailhol > > <mailhol.vincent@wanadoo.fr> > > Subject: [PATCH net-next v5 2/4] net: devlink: remove > > devlink_info_driver_name_put() > > > > Now that the core sets the driver name attribute, drivers are not > > supposed to call devlink_info_driver_name_put() anymore. Remove it. > > > > You could combine this patch with the previous one so that in the event of a cherry-pick its not possible to have this function while the core inserts the driver name automatically. > > I think that also makes it very clear that there are no remaining in-tree drivers still calling the function. > > I don't have a strong preference if we prefer it being separated. The first patch is already pretty long. I do not expect this to be cherry-picked because it does not fix any existing bug (and if people really want to cherry pick, then they just have to cherry pick both). On the contrary, splitting makes it easier to review, I think. Unless the maintainers as me to squash, I want to keep it as-is. Yours sincerely, Vincent Mailhol
> -----Original Message----- > From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr> > Sent: Monday, November 28, 2022 5:11 PM > To: Keller, Jacob E <jacob.e.keller@intel.com> > Cc: Jiri Pirko <jiri@nvidia.com>; netdev@vger.kernel.org; Jakub Kicinski > <kuba@kernel.org>; David S . Miller <davem@davemloft.net>; Eric Dumazet > <edumazet@google.com>; Paolo Abeni <pabeni@redhat.com>; linux- > kernel@vger.kernel.org; Boris Brezillon <bbrezillon@kernel.org>; Arnaud Ebalard > <arno@natisbad.org>; Srujana Challa <schalla@marvell.com>; Kurt Kanzenbach > <kurt@linutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > <f.fainelli@gmail.com>; Vladimir Oltean <olteanv@gmail.com>; Michael Chan > <michael.chan@broadcom.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; > Dimitris Michailidis <dmichail@fungible.com>; Yisen Zhuang > <yisen.zhuang@huawei.com>; Salil Mehta <salil.mehta@huawei.com>; > Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L > <anthony.l.nguyen@intel.com>; Sunil Goutham <sgoutham@marvell.com>; Linu > Cherian <lcherian@marvell.com>; Geetha sowjanya <gakula@marvell.com>; > Jerin Jacob <jerinj@marvell.com>; hariprasad <hkelam@marvell.com>; > Subbaraya Sundeep <sbhatta@marvell.com>; Taras Chornyi > <tchornyi@marvell.com>; Saeed Mahameed <saeedm@nvidia.com>; Leon > Romanovsky <leon@kernel.org>; Ido Schimmel <idosch@nvidia.com>; Petr > Machata <petrm@nvidia.com>; Simon Horman <simon.horman@corigine.com>; > Shannon Nelson <snelson@pensando.io>; drivers@pensando.io; Ariel Elior > <aelior@marvell.com>; Manish Chopra <manishc@marvell.com>; Jonathan > Lemon <jonathan.lemon@gmail.com>; Vadim Fedorenko <vadfed@fb.com>; > Richard Cochran <richardcochran@gmail.com>; Vadim Pasternak > <vadimp@mellanox.com>; linux-crypto@vger.kernel.org; intel-wired- > lan@lists.osuosl.org; linux-rdma@vger.kernel.org; oss-drivers@corigine.com; Jiri > Pirko <jiri@mellanox.com>; Herbert Xu <herbert@gondor.apana.org.au>; > Guangbin Huang <huangguangbin2@huawei.com>; Minghao Chi > <chi.minghao@zte.com.cn>; Shijith Thotton <sthotton@marvell.com> > Subject: Re: [PATCH net-next v5 2/4] net: devlink: remove > devlink_info_driver_name_put() > > Hi Jacob, > > Thanks for the review! > > On Tue. 29 Nov. 2022 at 09:23, Keller, Jacob E <jacob.e.keller@intel.com> wrote: > > > -----Original Message----- > > > From: Vincent Mailhol <vincent.mailhol@gmail.com> On Behalf Of Vincent > > > Mailhol > > > Sent: Monday, November 28, 2022 4:06 PM > > > To: Jiri Pirko <jiri@nvidia.com>; netdev@vger.kernel.org; Jakub Kicinski > > > <kuba@kernel.org> > > > Cc: David S . Miller <davem@davemloft.net>; Eric Dumazet > > > <edumazet@google.com>; Paolo Abeni <pabeni@redhat.com>; linux- > > > kernel@vger.kernel.org; Boris Brezillon <bbrezillon@kernel.org>; Arnaud > Ebalard > > > <arno@natisbad.org>; Srujana Challa <schalla@marvell.com>; Kurt > Kanzenbach > > > <kurt@linutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > > > <f.fainelli@gmail.com>; Vladimir Oltean <olteanv@gmail.com>; Michael Chan > > > <michael.chan@broadcom.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; > > > Dimitris Michailidis <dmichail@fungible.com>; Yisen Zhuang > > > <yisen.zhuang@huawei.com>; Salil Mehta <salil.mehta@huawei.com>; > > > Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L > > > <anthony.l.nguyen@intel.com>; Sunil Goutham <sgoutham@marvell.com>; > Linu > > > Cherian <lcherian@marvell.com>; Geetha sowjanya <gakula@marvell.com>; > > > Jerin Jacob <jerinj@marvell.com>; hariprasad <hkelam@marvell.com>; > > > Subbaraya Sundeep <sbhatta@marvell.com>; Taras Chornyi > > > <tchornyi@marvell.com>; Saeed Mahameed <saeedm@nvidia.com>; Leon > > > Romanovsky <leon@kernel.org>; Ido Schimmel <idosch@nvidia.com>; Petr > > > Machata <petrm@nvidia.com>; Simon Horman > <simon.horman@corigine.com>; > > > Shannon Nelson <snelson@pensando.io>; drivers@pensando.io; Ariel Elior > > > <aelior@marvell.com>; Manish Chopra <manishc@marvell.com>; Jonathan > > > Lemon <jonathan.lemon@gmail.com>; Vadim Fedorenko <vadfed@fb.com>; > > > Richard Cochran <richardcochran@gmail.com>; Vadim Pasternak > > > <vadimp@mellanox.com>; Shalom Toledo <shalomt@mellanox.com>; linux- > > > crypto@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux- > > > rdma@vger.kernel.org; oss-drivers@corigine.com; Jiri Pirko > > > <jiri@mellanox.com>; Herbert Xu <herbert@gondor.apana.org.au>; Hao > Chen > > > <chenhao288@hisilicon.com>; Guangbin Huang > > > <huangguangbin2@huawei.com>; Minghao Chi <chi.minghao@zte.com.cn>; > > > Shijith Thotton <sthotton@marvell.com>; Vincent Mailhol > > > <mailhol.vincent@wanadoo.fr> > > > Subject: [PATCH net-next v5 2/4] net: devlink: remove > > > devlink_info_driver_name_put() > > > > > > Now that the core sets the driver name attribute, drivers are not > > > supposed to call devlink_info_driver_name_put() anymore. Remove it. > > > > > > > You could combine this patch with the previous one so that in the event of a > cherry-pick its not possible to have this function while the core inserts the driver > name automatically. > > > > I think that also makes it very clear that there are no remaining in-tree drivers > still calling the function. > > > > I don't have a strong preference if we prefer it being separated. > > The first patch is already pretty long. I do not expect this to be > cherry-picked because it does not fix any existing bug (and if people > really want to cherry pick, then they just have to cherry pick both). > On the contrary, splitting makes it easier to review, I think. > > Unless the maintainers as me to squash, I want to keep it as-is. > That works for me! Thanks, Jake > > Yours sincerely, > Vincent Mailhol
Tue, Nov 29, 2022 at 01:05:48AM CET, mailhol.vincent@wanadoo.fr wrote: >Now that the core sets the driver name attribute, drivers are not >supposed to call devlink_info_driver_name_put() anymore. Remove it. > >Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> I agree with Jacob that this could be easily squashed to the previous patch. One way or another: Reviewed-by: Jiri Pirko <jiri@nvidia.com>
On Tue. 29 Nov. 2022 at 17:25, Jiri Pirko <jiri@resnulli.us> wrote: > Tue, Nov 29, 2022 at 01:05:48AM CET, mailhol.vincent@wanadoo.fr wrote: > >Now that the core sets the driver name attribute, drivers are not > >supposed to call devlink_info_driver_name_put() anymore. Remove it. > > > >Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > > I agree with Jacob that this could be easily squashed to the previous > patch. One way or another: OK. Let's have the majority decide: I will squash patches 1 and 2 and send a v6. > Reviewed-by: Jiri Pirko <jiri@nvidia.com> Thank you for the review.
On Tue, 29 Nov 2022 18:37:41 +0900 Vincent MAILHOL wrote: > > I agree with Jacob that this could be easily squashed to the previous > > patch. One way or another: > > OK. Let's have the majority decide: I will squash patches 1 and 2 and send a v6. FTR I prefer v5, much easier to review the code when driver changes are separated from core changes. But doesn't matter.
diff --git a/include/net/devlink.h b/include/net/devlink.h index 074a79b8933f..52d5fb67e9b8 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -1746,8 +1746,6 @@ int devlink_region_snapshot_create(struct devlink_region *region, u8 *data, u32 snapshot_id); int devlink_info_serial_number_put(struct devlink_info_req *req, const char *sn); -int devlink_info_driver_name_put(struct devlink_info_req *req, - const char *name); int devlink_info_board_serial_number_put(struct devlink_info_req *req, const char *bsn); diff --git a/net/core/devlink.c b/net/core/devlink.c index 6478135d9ba1..3babc16eeb6b 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -6633,14 +6633,6 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb, return err; } -int devlink_info_driver_name_put(struct devlink_info_req *req, const char *name) -{ - if (!req->msg) - return 0; - return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME, name); -} -EXPORT_SYMBOL_GPL(devlink_info_driver_name_put); - int devlink_info_serial_number_put(struct devlink_info_req *req, const char *sn) { if (!req->msg) @@ -6756,7 +6748,8 @@ static int devlink_nl_driver_info_get(struct device_driver *drv, return 0; if (drv->name[0]) - return devlink_info_driver_name_put(req, drv->name); + return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME, + drv->name); return 0; }
Now that the core sets the driver name attribute, drivers are not supposed to call devlink_info_driver_name_put() anymore. Remove it. Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> --- include/net/devlink.h | 2 -- net/core/devlink.c | 11 ++--------- 2 files changed, 2 insertions(+), 11 deletions(-)