Mon, Jul 29, 2019 at 08:59:06PM CEST, jakub.kicin...@netronome.com wrote:
>On Sat, 27 Jul 2019 11:44:59 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <j...@mellanox.com>
>>
>> When user does create new netdevsim instance using sysfs bus file,
>> create the devlink instance and related netdev instance in the namespace
>> of the caller.
>>
>> Signed-off-by: Jiri Pirko <j...@mellanox.com>
>
>> diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
>> index 1a0ff3d7747b..6aeed0c600f8 100644
>> --- a/drivers/net/netdevsim/bus.c
>> +++ b/drivers/net/netdevsim/bus.c
>> @@ -283,6 +283,7 @@ nsim_bus_dev_new(unsigned int id, unsigned int
>> port_count)
>> nsim_bus_dev->dev.bus = &nsim_bus;
>> nsim_bus_dev->dev.type = &nsim_bus_dev_type;
>> nsim_bus_dev->port_count = port_count;
>> + nsim_bus_dev->initial_net = current->nsproxy->net_ns;
>>
>> err = device_register(&nsim_bus_dev->dev);
>> if (err)
>
>The saved initial_net is only to carry the net info from the adding
>process to the probe callback, because probe can be asynchronous?
Exactly.
>I'm not entirely sure netdevsim can probe asynchronously in practice
>given we never return EPROBE_DEFER, but would you mind at least adding
>a comment stating that next to the definition of the field, otherwise
>I worry someone may mistakenly use this field instead of the up-to-date
>devlink's netns.
Will do.
>
>> diff --git a/drivers/net/netdevsim/netdevsim.h
>> b/drivers/net/netdevsim/netdevsim.h
>> index 79c05af2a7c0..cdf53d0e0c49 100644
>> --- a/drivers/net/netdevsim/netdevsim.h
>> +++ b/drivers/net/netdevsim/netdevsim.h
>> @@ -19,6 +19,7 @@
>> #include <linux/netdevice.h>
>> #include <linux/u64_stats_sync.h>
>> #include <net/devlink.h>
>> +#include <net/net_namespace.h>
>
>You can just do a forward declaration, no need to pull in the header.
Sure, but why?
>
>> #include <net/xdp.h>
>>
>> #define DRV_NAME "netdevsim"
>
>> @@ -213,6 +215,7 @@ struct nsim_bus_dev {
>> struct device dev;
>> struct list_head list;
>> unsigned int port_count;
>> + struct net *initial_net;
>> unsigned int num_vfs;
>> struct nsim_vf_config *vfconfigs;
>> };
>
>Otherwise makes perfect sense, with the above nits addressed feel free
>to add:
>
>Acked-by: Jakub Kicinski <jakub.kicin...@netronome.com>
