Re: [PATCH net-next 2/2] devlink: Allocate devlink directly in requested net namespace

On Thu, Jul 29, 2021 at 01:55:29PM +0200, Jiri Pirko wrote:

Thu, Jul 29, 2021 at 10:15:26AM CEST, leon@kernel.org wrote:

quoted

From: Leon Romanovsky <leonro@nvidia.com>

There is no need in extra call indirection and check from impossible
flow where someone tries to set namespace without prior call
to devlink_alloc().

Instead of this extra logic and additional EXPORT_SYMBOL, use specialized
devlink allocation function that receives net namespace as an argument.

Such specialized API allows clear view when devlink initialized in wrong
net namespace and/or kernel users don't try to change devlink namespace
under the hood.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
drivers/net/netdevsim/dev.c |  4 ++--
include/net/devlink.h       | 14 ++++++++++++--
net/core/devlink.c          | 26 ++++++++------------------
3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 6348307bfa84..d538a39d4225 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c

@@ -1431,10 +1431,10 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)

	struct devlink *devlink;
	int err;

-	devlink = devlink_alloc(&nsim_dev_devlink_ops, sizeof(*nsim_dev));
+	devlink = devlink_alloc_ns(&nsim_dev_devlink_ops, sizeof(*nsim_dev),
+				   nsim_bus_dev->initial_net);
	if (!devlink)
		return -ENOMEM;
-	devlink_net_set(devlink, nsim_bus_dev->initial_net);
	nsim_dev = devlink_priv(devlink);
	nsim_dev->nsim_bus_dev = nsim_bus_dev;
	nsim_dev->switch_id.id_len = sizeof(nsim_dev->switch_id.id);

diff --git a/include/net/devlink.h b/include/net/devlink.h
index e48a62320407..b4691c40320f 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h

@@ -1540,8 +1540,18 @@ static inline struct devlink *netdev_to_devlink(struct net_device *dev)

struct ib_device;

struct net *devlink_net(const struct devlink *devlink);
-void devlink_net_set(struct devlink *devlink, struct net *net);
-struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size);
+/* This RAW call is intended for software devices that can

Not sure what "RAW call" is, perhaps you can just avoid this here.

I wanted to emphasize the message that we shouldn't see this call in
regular drivers and it is very specific to SW drivers.

I'll drop "RAW".

Otherwise, this patch looks fine to me:
Reviewed-by: Jiri Pirko <redacted>

Thanks

[...]