[patch net-next v3 0/3] net: devlink: Finish network namespace support

[patch net-next v3 0/3] net: devlink: Finish network namespace support

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Devlink from the beginning counts with network namespaces, but the
instances has been fixed to init_net. The first patch allows user
to move existing devlink instances into namespaces:

$ devlink dev
netdevsim/netdevsim1
$ ip netns add ns1
$ devlink dev set netdevsim/netdevsim1 netns ns1
$ devlink -N ns1 dev
netdevsim/netdevsim1

The last patch allows user to create new netdevsim instance directly
inside network namespace of a caller.

Jiri Pirko (3):
  net: devlink: allow to change namespaces
  net: devlink: export devlink net set/get helpers
  netdevsim: create devlink and netdev instances in namespace

 drivers/net/netdevsim/bus.c       |   1 +
 drivers/net/netdevsim/dev.c       |  17 ++-
 drivers/net/netdevsim/netdev.c    |   4 +-
 drivers/net/netdevsim/netdevsim.h |   8 +-
 include/net/devlink.h             |   3 +
 include/uapi/linux/devlink.h      |   4 +
 net/core/devlink.c                | 182 +++++++++++++++++++++++++++++-
 7 files changed, 205 insertions(+), 14 deletions(-)

-- 
2.21.0

[patch net-next v3 1/3] net: devlink: allow to change namespaces

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

All devlink instances are created in init_net and stay there for a
lifetime. Allow user to be able to move devlink instances into
namespaces.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v2->v3:
- added notifications for all objects
v1->v2:
- change the check for multiple attributes
- add warnon in case there is no attribute passed
---
 include/uapi/linux/devlink.h |   4 +
 net/core/devlink.c           | 166 ++++++++++++++++++++++++++++++++++-
 2 files changed, 167 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index ffc993256527..95f0a1edab99 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -348,6 +348,10 @@ enum devlink_attr {
 	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
 	DEVLINK_ATTR_PORT_PCI_VF_NUMBER,	/* u16 */
 
+	DEVLINK_ATTR_NETNS_FD,			/* u32 */
+	DEVLINK_ATTR_NETNS_PID,			/* u32 */
+	DEVLINK_ATTR_NETNS_ID,			/* u32 */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e3a1ae44f93d..6f8c1b2cdfb2 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -430,8 +430,16 @@ static void devlink_nl_post_doit(const struct genl_ops *ops,
 {
 	struct devlink *devlink;
 
-	devlink = devlink_get_from_info(info);
-	if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
+	/* When devlink changes netns, it would not be found
+	 * by devlink_get_from_info(). So try if it is stored first.
+	 */
+	if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_DEVLINK) {
+		devlink = info->user_ptr[0];
+	} else {
+		devlink = devlink_get_from_info(info);
+		WARN_ON(IS_ERR(devlink));
+	}
+	if (!IS_ERR(devlink) && ~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
 		mutex_unlock(&devlink->lock);
 	mutex_unlock(&devlink_mutex);
 }
@@ -636,6 +644,74 @@ static int devlink_nl_cmd_get_doit(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
 
+static struct net *devlink_netns_get(struct sk_buff *skb,
+				     struct devlink *devlink,
+				     struct genl_info *info)
+{
+	struct nlattr *netns_pid_attr = info->attrs[DEVLINK_ATTR_NETNS_PID];
+	struct nlattr *netns_fd_attr = info->attrs[DEVLINK_ATTR_NETNS_FD];
+	struct nlattr *netns_id_attr = info->attrs[DEVLINK_ATTR_NETNS_ID];
+	struct net *net;
+
+	if (!!netns_pid_attr + !!netns_fd_attr + !!netns_id_attr > 1) {
+		NL_SET_ERR_MSG(info->extack, "multiple netns identifying attributes specified");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (netns_pid_attr) {
+		net = get_net_ns_by_pid(nla_get_u32(netns_pid_attr));
+	} else if (netns_fd_attr) {
+		net = get_net_ns_by_fd(nla_get_u32(netns_fd_attr));
+	} else if (netns_id_attr) {
+		net = get_net_ns_by_id(sock_net(skb->sk),
+				       nla_get_u32(netns_id_attr));
+		if (!net)
+			net = ERR_PTR(-EINVAL);
+	} else {
+		WARN_ON(1);
+		net = ERR_PTR(-EINVAL);
+	}
+	if (IS_ERR(net)) {
+		NL_SET_ERR_MSG(info->extack, "Unknown network namespace");
+		return ERR_PTR(-EINVAL);
+	}
+	if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
+		put_net(net);
+		return ERR_PTR(-EPERM);
+	}
+	return net;
+}
+
+static void devlink_all_add_notify(struct devlink *devlink);
+static void devlink_all_del_notify(struct devlink *devlink);
+
+static void devlink_netns_change(struct devlink *devlink, struct net *net)
+{
+	if (net_eq(devlink_net(devlink), net))
+		return;
+	devlink_all_del_notify(devlink);
+	devlink_net_set(devlink, net);
+	devlink_all_add_notify(devlink);
+}
+
+static int devlink_nl_cmd_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct devlink *devlink = info->user_ptr[0];
+
+	if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
+	    info->attrs[DEVLINK_ATTR_NETNS_FD] ||
+	    info->attrs[DEVLINK_ATTR_NETNS_ID]) {
+		struct net *net;
+
+		net = devlink_netns_get(skb, devlink, info);
+		if (IS_ERR(net))
+			return PTR_ERR(net);
+		devlink_netns_change(devlink, net);
+		put_net(net);
+	}
+	return 0;
+}
+
 static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
 				     struct netlink_callback *cb)
 {
@@ -5184,6 +5260,9 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER] = { .type = NLA_U8 },
 	[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME] = { .type = NLA_NUL_STRING },
 	[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT] = { .type = NLA_NUL_STRING },
+	[DEVLINK_ATTR_NETNS_PID] = { .type = NLA_U32 },
+	[DEVLINK_ATTR_NETNS_FD] = { .type = NLA_U32 },
+	[DEVLINK_ATTR_NETNS_ID] = { .type = NLA_U32 },
 };
 
 static const struct genl_ops devlink_nl_ops[] = {
@@ -5195,6 +5274,13 @@ static const struct genl_ops devlink_nl_ops[] = {
 		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
 		/* can be retrieved by unprivileged users */
 	},
+	{
+		.cmd = DEVLINK_CMD_SET,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = devlink_nl_cmd_set_doit,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+	},
 	{
 		.cmd = DEVLINK_CMD_PORT_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
@@ -6834,6 +6920,56 @@ int devlink_region_snapshot_create(struct devlink_region *region,
 }
 EXPORT_SYMBOL_GPL(devlink_region_snapshot_create);
 
+static void devlink_all_del_notify(struct devlink *devlink)
+{
+	struct devlink_port *devlink_port;
+	struct devlink_region *region;
+	struct devlink_param_item *param_item;
+	struct devlink_snapshot *snapshot;
+
+	list_for_each_entry(region, &devlink->region_list, list) {
+		list_for_each_entry(snapshot, &region->snapshot_list, list)
+			devlink_nl_region_notify(region, snapshot,
+						 DEVLINK_CMD_REGION_DEL);
+		devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_DEL);
+	}
+	list_for_each_entry(devlink_port, &devlink->port_list, list) {
+		list_for_each_entry(param_item, &devlink_port->param_list, list)
+			devlink_param_notify(devlink, devlink_port->index,
+					     param_item, DEVLINK_CMD_PARAM_DEL);
+		devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
+	}
+	list_for_each_entry(param_item, &devlink->param_list, list)
+		devlink_param_notify(devlink, 0, param_item,
+				     DEVLINK_CMD_PARAM_DEL);
+	devlink_notify(devlink, DEVLINK_CMD_DEL);
+}
+
+static void devlink_all_add_notify(struct devlink *devlink)
+{
+	struct devlink_port *devlink_port;
+	struct devlink_region *region;
+	struct devlink_param_item *param_item;
+	struct devlink_snapshot *snapshot;
+
+	devlink_notify(devlink, DEVLINK_CMD_NEW);
+	list_for_each_entry(param_item, &devlink->param_list, list)
+		devlink_param_notify(devlink, 0, param_item,
+				     DEVLINK_CMD_PARAM_NEW);
+	list_for_each_entry(devlink_port, &devlink->port_list, list) {
+		devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
+		list_for_each_entry(param_item, &devlink_port->param_list, list)
+			devlink_param_notify(devlink, devlink_port->index,
+					     param_item, DEVLINK_CMD_PARAM_NEW);
+	}
+	list_for_each_entry(region, &devlink->region_list, list) {
+		devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
+		list_for_each_entry(snapshot, &region->snapshot_list, list)
+			devlink_nl_region_notify(region, snapshot,
+						 DEVLINK_CMD_REGION_NEW);
+	}
+}
+
 static void __devlink_compat_running_version(struct devlink *devlink,
 					     char *buf, size_t len)
 {
@@ -6953,9 +7089,33 @@ int devlink_compat_switch_id_get(struct net_device *dev,
 	return 0;
 }
 
+static void __net_exit devlink_pernet_exit(struct net *net)
+{
+	struct devlink *devlink;
+
+	mutex_lock(&devlink_mutex);
+	list_for_each_entry(devlink, &devlink_list, list)
+		if (net_eq(devlink_net(devlink), net))
+			devlink_netns_change(devlink, &init_net);
+	mutex_unlock(&devlink_mutex);
+}
+
+static struct pernet_operations __net_initdata devlink_pernet_ops = {
+	.exit = devlink_pernet_exit,
+};
+
 static int __init devlink_init(void)
 {
-	return genl_register_family(&devlink_nl_family);
+	int err;
+
+	err = genl_register_family(&devlink_nl_family);
+	if (err)
+		goto out;
+	err = register_pernet_device(&devlink_pernet_ops);
+
+out:
+	WARN_ON(err);
+	return err;
 }
 
 subsys_initcall(devlink_init);
-- 
2.21.0

Re: [patch net-next v3 1/3] net: devlink: allow to change namespaces

[Jakub Kicinski]

On Mon, 12 Aug 2019 15:47:49 +0200, Jiri Pirko wrote:
> @@ -6953,9 +7089,33 @@ int devlink_compat_switch_id_get(struct net_device *dev,
>  	return 0;
>  }
>  
> +static void __net_exit devlink_pernet_exit(struct net *net)
> +{
> +	struct devlink *devlink;
> +
> +	mutex_lock(&devlink_mutex);
> +	list_for_each_entry(devlink, &devlink_list, list)
> +		if (net_eq(devlink_net(devlink), net))
> +			devlink_netns_change(devlink, &init_net);
> +	mutex_unlock(&devlink_mutex);
> +}

Just to be sure - this will not cause any locking issues?
Usually the locking order goes devlink -> rtnl

Re: [patch net-next v3 1/3] net: devlink: allow to change namespaces

[Jiri Pirko]

Tue, Aug 13, 2019 at 03:21:22AM CEST, jakub.kicinski@netronome.com wrote:
>On Mon, 12 Aug 2019 15:47:49 +0200, Jiri Pirko wrote:
>> @@ -6953,9 +7089,33 @@ int devlink_compat_switch_id_get(struct net_device *dev,
>>  	return 0;
>>  }
>>  
>> +static void __net_exit devlink_pernet_exit(struct net *net)
>> +{
>> +	struct devlink *devlink;
>> +
>> +	mutex_lock(&devlink_mutex);
>> +	list_for_each_entry(devlink, &devlink_list, list)
>> +		if (net_eq(devlink_net(devlink), net))
>> +			devlink_netns_change(devlink, &init_net);
>> +	mutex_unlock(&devlink_mutex);
>> +}
>
>Just to be sure - this will not cause any locking issues?
>Usually the locking order goes devlink -> rtnl

rtnl is not taken. Do I miss something?

Re: [patch net-next v3 1/3] net: devlink: allow to change namespaces

[Jakub Kicinski]

On Tue, 13 Aug 2019 08:13:55 +0200, Jiri Pirko wrote:
> Tue, Aug 13, 2019 at 03:21:22AM CEST, jakub.kicinski@netronome.com wrote:
> >On Mon, 12 Aug 2019 15:47:49 +0200, Jiri Pirko wrote:  
> >> @@ -6953,9 +7089,33 @@ int devlink_compat_switch_id_get(struct net_device *dev,
> >>  	return 0;
> >>  }
> >>  
> >> +static void __net_exit devlink_pernet_exit(struct net *net)
> >> +{
> >> +	struct devlink *devlink;
> >> +
> >> +	mutex_lock(&devlink_mutex);
> >> +	list_for_each_entry(devlink, &devlink_list, list)
> >> +		if (net_eq(devlink_net(devlink), net))
> >> +			devlink_netns_change(devlink, &init_net);
> >> +	mutex_unlock(&devlink_mutex);
> >> +}  
> >
> >Just to be sure - this will not cause any locking issues?
> >Usually the locking order goes devlink -> rtnl  
> 
> rtnl is not taken. Do I miss something?

Probably not, just double checking.

[patch net-next v3 2/3] net: devlink: export devlink net set/get helpers

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Allow drivers to set/get net struct for devlink instance. Set is only
allowed for newly allocated devlink instance.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/devlink.h |  3 +++
 net/core/devlink.c    | 18 ++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 451268f64880..c45b10d79b14 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -35,6 +35,7 @@ struct devlink {
 	struct device *dev;
 	possible_net_t _net;
 	struct mutex lock;
+	bool registered;
 	char priv[0] __aligned(NETDEV_ALIGN);
 };
 
@@ -591,6 +592,8 @@ 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);
 int devlink_register(struct devlink *devlink, struct device *dev);
 void devlink_unregister(struct devlink *devlink);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6f8c1b2cdfb2..80a4b3ae9d39 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -92,16 +92,25 @@ static LIST_HEAD(devlink_list);
  */
 static DEFINE_MUTEX(devlink_mutex);
 
-static struct net *devlink_net(const struct devlink *devlink)
+struct net *devlink_net(const struct devlink *devlink)
 {
 	return read_pnet(&devlink->_net);
 }
+EXPORT_SYMBOL_GPL(devlink_net);
 
-static void devlink_net_set(struct devlink *devlink, struct net *net)
+static void __devlink_net_set(struct devlink *devlink, struct net *net)
 {
 	write_pnet(&devlink->_net, net);
 }
 
+void devlink_net_set(struct devlink *devlink, struct net *net)
+{
+	if (WARN_ON(devlink->registered))
+		return;
+	__devlink_net_set(devlink, net);
+}
+EXPORT_SYMBOL_GPL(devlink_net_set);
+
 static struct devlink *devlink_get_from_attrs(struct net *net,
 					      struct nlattr **attrs)
 {
@@ -690,7 +699,7 @@ static void devlink_netns_change(struct devlink *devlink, struct net *net)
 	if (net_eq(devlink_net(devlink), net))
 		return;
 	devlink_all_del_notify(devlink);
-	devlink_net_set(devlink, net);
+	__devlink_net_set(devlink, net);
 	devlink_all_add_notify(devlink);
 }
 
@@ -5606,7 +5615,7 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
 	if (!devlink)
 		return NULL;
 	devlink->ops = ops;
-	devlink_net_set(devlink, &init_net);
+	__devlink_net_set(devlink, &init_net);
 	INIT_LIST_HEAD(&devlink->port_list);
 	INIT_LIST_HEAD(&devlink->sb_list);
 	INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
@@ -5630,6 +5639,7 @@ int devlink_register(struct devlink *devlink, struct device *dev)
 {
 	mutex_lock(&devlink_mutex);
 	devlink->dev = dev;
+	devlink->registered = true;
 	list_add_tail(&devlink->list, &devlink_list);
 	devlink_notify(devlink, DEVLINK_CMD_NEW);
 	mutex_unlock(&devlink_mutex);
-- 
2.21.0

[patch net-next v3 3/3] netdevsim: create devlink and netdev instances in namespace

[Jiri Pirko]

From: Jiri Pirko <jiri@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 <jiri@mellanox.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
v1->v2:
- remove net_namespace.h include and forward decralared net struct
- add comment to initial_net pointer
---
 drivers/net/netdevsim/bus.c       |  1 +
 drivers/net/netdevsim/dev.c       | 17 +++++++++++------
 drivers/net/netdevsim/netdev.c    |  4 +++-
 drivers/net/netdevsim/netdevsim.h |  8 +++++++-
 4 files changed, 22 insertions(+), 8 deletions(-)

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)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index e76ea6a3cb60..8485dd805f7c 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -381,7 +381,8 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
 #define NSIM_DEV_TEST1_DEFAULT true
 
 static struct nsim_dev *
-nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
+nsim_dev_create(struct net *net, struct nsim_bus_dev *nsim_bus_dev,
+		unsigned int port_count)
 {
 	struct nsim_dev *nsim_dev;
 	struct devlink *devlink;
@@ -390,6 +391,7 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
 	devlink = devlink_alloc(&nsim_dev_devlink_ops, sizeof(*nsim_dev));
 	if (!devlink)
 		return ERR_PTR(-ENOMEM);
+	devlink_net_set(devlink, 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);
@@ -469,7 +471,7 @@ static void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 	devlink_free(devlink);
 }
 
-static int __nsim_dev_port_add(struct nsim_dev *nsim_dev,
+static int __nsim_dev_port_add(struct net *net, struct nsim_dev *nsim_dev,
 			       unsigned int port_index)
 {
 	struct nsim_dev_port *nsim_dev_port;
@@ -495,7 +497,7 @@ static int __nsim_dev_port_add(struct nsim_dev *nsim_dev,
 	if (err)
 		goto err_dl_port_unregister;
 
-	nsim_dev_port->ns = nsim_create(nsim_dev, nsim_dev_port);
+	nsim_dev_port->ns = nsim_create(net, nsim_dev, nsim_dev_port);
 	if (IS_ERR(nsim_dev_port->ns)) {
 		err = PTR_ERR(nsim_dev_port->ns);
 		goto err_port_debugfs_exit;
@@ -538,17 +540,19 @@ static void nsim_dev_port_del_all(struct nsim_dev *nsim_dev)
 
 int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
 {
+	struct net *initial_net = nsim_bus_dev->initial_net;
 	struct nsim_dev *nsim_dev;
 	int i;
 	int err;
 
-	nsim_dev = nsim_dev_create(nsim_bus_dev, nsim_bus_dev->port_count);
+	nsim_dev = nsim_dev_create(initial_net, nsim_bus_dev,
+				   nsim_bus_dev->port_count);
 	if (IS_ERR(nsim_dev))
 		return PTR_ERR(nsim_dev);
 	dev_set_drvdata(&nsim_bus_dev->dev, nsim_dev);
 
 	for (i = 0; i < nsim_bus_dev->port_count; i++) {
-		err = __nsim_dev_port_add(nsim_dev, i);
+		err = __nsim_dev_port_add(initial_net, nsim_dev, i);
 		if (err)
 			goto err_port_del_all;
 	}
@@ -583,13 +587,14 @@ int nsim_dev_port_add(struct nsim_bus_dev *nsim_bus_dev,
 		      unsigned int port_index)
 {
 	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
+	struct net *net = devlink_net(priv_to_devlink(nsim_dev));
 	int err;
 
 	mutex_lock(&nsim_dev->port_list_lock);
 	if (__nsim_dev_port_lookup(nsim_dev, port_index))
 		err = -EEXIST;
 	else
-		err = __nsim_dev_port_add(nsim_dev, port_index);
+		err = __nsim_dev_port_add(net, nsim_dev, port_index);
 	mutex_unlock(&nsim_dev->port_list_lock);
 	return err;
 }
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 0740940f41b1..25c7de7a4a31 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -280,7 +280,8 @@ static void nsim_setup(struct net_device *dev)
 }
 
 struct netdevsim *
-nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port)
+nsim_create(struct net *net, struct nsim_dev *nsim_dev,
+	    struct nsim_dev_port *nsim_dev_port)
 {
 	struct net_device *dev;
 	struct netdevsim *ns;
@@ -290,6 +291,7 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port)
 	if (!dev)
 		return ERR_PTR(-ENOMEM);
 
+	dev_net_set(dev, net);
 	ns = netdev_priv(dev);
 	ns->netdev = dev;
 	ns->nsim_dev = nsim_dev;
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 4c758c6919f5..521802d429a0 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -74,8 +74,11 @@ struct netdevsim {
 	struct nsim_ipsec ipsec;
 };
 
+struct net;
+
 struct netdevsim *
-nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port);
+nsim_create(struct net *net, struct nsim_dev *nsim_dev,
+	    struct nsim_dev_port *nsim_dev_port);
 void nsim_destroy(struct netdevsim *ns);
 
 #ifdef CONFIG_BPF_SYSCALL
@@ -216,6 +219,9 @@ struct nsim_bus_dev {
 	struct device dev;
 	struct list_head list;
 	unsigned int port_count;
+	struct net *initial_net; /* Purpose of this is to carry net pointer
+				  * during the probe time only.
+				  */
 	unsigned int num_vfs;
 	struct nsim_vf_config *vfconfigs;
 };
-- 
2.21.0

[patch iproute2-next v3 1/2] devlink: introduce cmdline option to switch to a different namespace

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c  | 12 ++++++++++--
 man/man8/devlink.8 |  4 ++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 91c85dc1de73..6bda25e92238 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -31,6 +31,7 @@
 #include "mnlg.h"
 #include "json_writer.h"
 #include "utils.h"
+#include "namespace.h"
 
 #define ESWITCH_MODE_LEGACY "legacy"
 #define ESWITCH_MODE_SWITCHDEV "switchdev"
@@ -6333,7 +6334,7 @@ static int cmd_health(struct dl *dl)
 static void help(void)
 {
 	pr_err("Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }\n"
-	       "       devlink [ -f[orce] ] -b[atch] filename\n"
+	       "       devlink [ -f[orce] ] -b[atch] filename -N[etns] netnsname\n"
 	       "where  OBJECT := { dev | port | sb | monitor | dpipe | resource | region | health }\n"
 	       "       OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] }\n");
 }
@@ -6479,6 +6480,7 @@ int main(int argc, char **argv)
 		{ "json",		no_argument,		NULL, 'j' },
 		{ "pretty",		no_argument,		NULL, 'p' },
 		{ "verbose",		no_argument,		NULL, 'v' },
+		{ "Netns",		required_argument,	NULL, 'N' },
 		{ NULL, 0, NULL, 0 }
 	};
 	const char *batch_file = NULL;
@@ -6494,7 +6496,7 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	while ((opt = getopt_long(argc, argv, "Vfb:njpv",
+	while ((opt = getopt_long(argc, argv, "Vfb:njpvN:",
 				  long_options, NULL)) >= 0) {
 
 		switch (opt) {
@@ -6520,6 +6522,12 @@ int main(int argc, char **argv)
 		case 'v':
 			dl->verbose = true;
 			break;
+		case 'N':
+			if (netns_switch(optarg)) {
+				ret = EXIT_FAILURE;
+				goto dl_free;
+			}
+			break;
 		default:
 			pr_err("Unknown option.\n");
 			help();
diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
index 13d4dcd908b3..9fc9b034eefe 100644
--- a/man/man8/devlink.8
+++ b/man/man8/devlink.8
@@ -51,6 +51,10 @@ When combined with -j generate a pretty JSON output.
 .BR "\-v" , " --verbose"
 Turn on verbose output.
 
+.TP
+.BR "\-N", " \-Netns " <NETNSNAME>
+Switches to the specified network namespace.
+
 .SS
 .I OBJECT
 
-- 
2.21.0

[patch iproute2-next v3 2/2] devlink: add support for network namespace change

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c            | 54 +++++++++++++++++++++++++++++++++++-
 include/uapi/linux/devlink.h |  4 +++
 man/man8/devlink-dev.8       | 12 ++++++++
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 6bda25e92238..42aebb38e67c 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -234,6 +234,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
 #define DL_OPT_HEALTH_REPORTER_NAME	BIT(27)
 #define DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD	BIT(27)
 #define DL_OPT_HEALTH_REPORTER_AUTO_RECOVER	BIT(28)
+#define DL_OPT_NETNS	BIT(29)
 
 struct dl_opts {
 	uint32_t present; /* flags of present items */
@@ -270,6 +271,8 @@ struct dl_opts {
 	const char *reporter_name;
 	uint64_t reporter_graceful_period;
 	bool reporter_auto_recover;
+	bool netns_is_pid;
+	uint32_t netns;
 };
 
 struct dl {
@@ -1330,6 +1333,22 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
 			if (err)
 				return err;
 			o_found |= DL_OPT_HEALTH_REPORTER_AUTO_RECOVER;
+		} else if (dl_argv_match(dl, "netns") &&
+			(o_all & DL_OPT_NETNS)) {
+			const char *netns_str;
+
+			dl_arg_inc(dl);
+			err = dl_argv_str(dl, &netns_str);
+			if (err)
+				return err;
+			opts->netns = netns_get_fd(netns_str);
+			if (opts->netns < 0) {
+				err = dl_argv_uint32_t(dl, &opts->netns);
+				if (err)
+					return err;
+				opts->netns_is_pid = true;
+			}
+			o_found |= DL_OPT_NETNS;
 		} else {
 			pr_err("Unknown option \"%s\"\n", dl_argv(dl));
 			return -EINVAL;
@@ -1443,7 +1462,11 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
 	if (opts->present & DL_OPT_HEALTH_REPORTER_AUTO_RECOVER)
 		mnl_attr_put_u8(nlh, DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER,
 				opts->reporter_auto_recover);
-
+	if (opts->present & DL_OPT_NETNS)
+		mnl_attr_put_u32(nlh,
+				 opts->netns_is_pid ? DEVLINK_ATTR_NETNS_PID :
+						      DEVLINK_ATTR_NETNS_FD,
+				 opts->netns);
 }
 
 static int dl_argv_parse_put(struct nlmsghdr *nlh, struct dl *dl,
@@ -1498,6 +1521,7 @@ static bool dl_dump_filter(struct dl *dl, struct nlattr **tb)
 static void cmd_dev_help(void)
 {
 	pr_err("Usage: devlink dev show [ DEV ]\n");
+	pr_err("       devlink dev set DEV netns { PID | NAME | ID }\n");
 	pr_err("       devlink dev eswitch set DEV [ mode { legacy | switchdev } ]\n");
 	pr_err("                               [ inline-mode { none | link | network | transport } ]\n");
 	pr_err("                               [ encap { disable | enable } ]\n");
@@ -2547,6 +2571,31 @@ static int cmd_dev_show(struct dl *dl)
 	return err;
 }
 
+static void cmd_dev_set_help(void)
+{
+	pr_err("Usage: devlink dev set DEV netns { PID | NAME | ID }\n");
+}
+
+static int cmd_dev_set(struct dl *dl)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	if (dl_argv_match(dl, "help") || dl_no_arg(dl)) {
+		cmd_dev_set_help();
+		return 0;
+	}
+
+	nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_SET,
+			       NLM_F_REQUEST | NLM_F_ACK);
+
+	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, DL_OPT_NETNS);
+	if (err)
+		return err;
+
+	return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
 static void cmd_dev_reload_help(void)
 {
 	pr_err("Usage: devlink dev reload [ DEV ]\n");
@@ -2743,6 +2792,9 @@ static int cmd_dev(struct dl *dl)
 		   dl_argv_match(dl, "list") || dl_no_arg(dl)) {
 		dl_arg_inc(dl);
 		return cmd_dev_show(dl);
+	} else if (dl_argv_match(dl, "set")) {
+		dl_arg_inc(dl);
+		return cmd_dev_set(dl);
 	} else if (dl_argv_match(dl, "eswitch")) {
 		dl_arg_inc(dl);
 		return cmd_dev_eswitch(dl);
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index fc195cbd66f4..bc1869993e20 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -348,6 +348,10 @@ enum devlink_attr {
 	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
 	DEVLINK_ATTR_PORT_PCI_VF_NUMBER,	/* u16 */
 
+	DEVLINK_ATTR_NETNS_FD,			/* u32 */
+	DEVLINK_ATTR_NETNS_PID,			/* u32 */
+	DEVLINK_ATTR_NETNS_ID,			/* u32 */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/man/man8/devlink-dev.8 b/man/man8/devlink-dev.8
index 1804463b2321..0e1a5523fa7b 100644
--- a/man/man8/devlink-dev.8
+++ b/man/man8/devlink-dev.8
@@ -25,6 +25,13 @@ devlink-dev \- devlink device configuration
 .ti -8
 .B devlink dev help
 
+.ti -8
+.BR "devlink dev set"
+.IR DEV
+.RI "[ "
+.BI "netns { " PID " | " NAME " | " ID " }
+.RI "]"
+
 .ti -8
 .BR "devlink dev eswitch set"
 .IR DEV
@@ -92,6 +99,11 @@ Format is:
 .in +2
 BUS_NAME/BUS_ADDRESS
 
+.SS devlink dev set  - sets devlink device attributes
+
+.TP
+.BI "netns { " PID " | " NAME " | " ID " }
+
 .SS devlink dev eswitch show - display devlink device eswitch attributes
 .SS devlink dev eswitch set  - sets devlink device eswitch attributes
 
-- 
2.21.0

Re: [patch net-next v3 0/3] net: devlink: Finish network namespace support

[David Ahern]

On 8/12/19 7:47 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Devlink from the beginning counts with network namespaces, but the
> instances has been fixed to init_net. The first patch allows user
> to move existing devlink instances into namespaces:
> 
> $ devlink dev
> netdevsim/netdevsim1
> $ ip netns add ns1
> $ devlink dev set netdevsim/netdevsim1 netns ns1
> $ devlink -N ns1 dev
> netdevsim/netdevsim1
> 
> The last patch allows user to create new netdevsim instance directly
> inside network namespace of a caller.

The namespace behavior seems odd to me. If devlink instance is created
in a namespace and never moved, it should die with the namespace. With
this patch set, devlink instance and its ports are moved to init_net on
namespace delete.

The fib controller needs an update to return the namespace of the
devlink instance (on top of the patch applied to net):

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 89795071f085..fa7e876f2d3b 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -114,11 +114,6 @@ static void nsim_dev_port_debugfs_exit(struct
nsim_dev_port *nsim_dev_port)
        debugfs_remove_recursive(nsim_dev_port->ddir);
 }

-static struct net *nsim_devlink_net(struct devlink *devlink)
-{
-       return &init_net;
-}
-
 static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
 {
        struct net *net = priv;
@@ -154,7 +149,7 @@ static int nsim_dev_resources_register(struct
devlink *devlink)
                .size_granularity = 1,
                .unit = DEVLINK_RESOURCE_UNIT_ENTRY
        };
-       struct net *net = nsim_devlink_net(devlink);
+       struct net *net = devlink_net(devlink);
        int err;
        u64 n;

@@ -309,7 +304,7 @@ static int nsim_dev_reload(struct devlink *devlink,
                NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
                NSIM_RESOURCE_IPV6_FIB, NSIM_RESOURCE_IPV6_FIB_RULES
        };
-       struct net *net = nsim_devlink_net(devlink);
+       struct net *net = devlink_net(devlink);
        int i;

        for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {

Re: [patch net-next v3 0/3] net: devlink: Finish network namespace support

[Jakub Kicinski]

On Mon, 12 Aug 2019 18:24:41 -0600, David Ahern wrote:
> On 8/12/19 7:47 AM, Jiri Pirko wrote:
> > From: Jiri Pirko <jiri@mellanox.com>
> > 
> > Devlink from the beginning counts with network namespaces, but the
> > instances has been fixed to init_net. The first patch allows user
> > to move existing devlink instances into namespaces:
> > 
> > $ devlink dev
> > netdevsim/netdevsim1
> > $ ip netns add ns1
> > $ devlink dev set netdevsim/netdevsim1 netns ns1
> > $ devlink -N ns1 dev
> > netdevsim/netdevsim1
> > 
> > The last patch allows user to create new netdevsim instance directly
> > inside network namespace of a caller.  
> 
> The namespace behavior seems odd to me. If devlink instance is created
> in a namespace and never moved, it should die with the namespace. With
> this patch set, devlink instance and its ports are moved to init_net on
> namespace delete.

If the devlink instance just disappeared - that'd be a very very strange
thing. Only software objects disappear with the namespace. 
Netdevices without ->rtnl_link_ops go back to init_net.

Re: [patch net-next v3 0/3] net: devlink: Finish network namespace support

[David Ahern]

On 8/12/19 7:11 PM, Jakub Kicinski wrote:
> If the devlink instance just disappeared - that'd be a very very strange
> thing. Only software objects disappear with the namespace. 
> Netdevices without ->rtnl_link_ops go back to init_net.

netdevsim still has rtnl_link_ops:

static struct rtnl_link_ops nsim_link_ops __read_mostly = {
        .kind           = DRV_NAME,
        .validate       = nsim_validate,
};

Re: [patch net-next v3 0/3] net: devlink: Finish network namespace support

[Jakub Kicinski]

On Mon, 12 Aug 2019 19:46:57 -0600, David Ahern wrote:
> On 8/12/19 7:11 PM, Jakub Kicinski wrote:
> > If the devlink instance just disappeared - that'd be a very very strange
> > thing. Only software objects disappear with the namespace. 
> > Netdevices without ->rtnl_link_ops go back to init_net.  
> 
> netdevsim still has rtnl_link_ops:
> 
> static struct rtnl_link_ops nsim_link_ops __read_mostly = {
>         .kind           = DRV_NAME,
>         .validate       = nsim_validate,
> };

The test harness is the only devlink instance which may 
conceivably have link ops. And implementing the behaviour 
you ask for would require core changes.

We are back to the precedent by test harness argument :(

Re: [patch net-next v3 0/3] net: devlink: Finish network namespace support

[Jiri Pirko]

Tue, Aug 13, 2019 at 03:11:00AM CEST, jakub.kicinski@netronome.com wrote:
>On Mon, 12 Aug 2019 18:24:41 -0600, David Ahern wrote:
>> On 8/12/19 7:47 AM, Jiri Pirko wrote:
>> > From: Jiri Pirko <jiri@mellanox.com>
>> > 
>> > Devlink from the beginning counts with network namespaces, but the
>> > instances has been fixed to init_net. The first patch allows user
>> > to move existing devlink instances into namespaces:
>> > 
>> > $ devlink dev
>> > netdevsim/netdevsim1
>> > $ ip netns add ns1
>> > $ devlink dev set netdevsim/netdevsim1 netns ns1
>> > $ devlink -N ns1 dev
>> > netdevsim/netdevsim1
>> > 
>> > The last patch allows user to create new netdevsim instance directly
>> > inside network namespace of a caller.  
>> 
>> The namespace behavior seems odd to me. If devlink instance is created
>> in a namespace and never moved, it should die with the namespace. With
>> this patch set, devlink instance and its ports are moved to init_net on
>> namespace delete.
>
>If the devlink instance just disappeared - that'd be a very very strange
>thing. Only software objects disappear with the namespace. 
>Netdevices without ->rtnl_link_ops go back to init_net.

Agreed. It makes sense to be moved to init_net.

Re: [patch net-next v3 0/3] net: devlink: Finish network namespace support

[Jiri Pirko]

Tue, Aug 13, 2019 at 02:24:41AM CEST, dsahern@gmail.com wrote:
>On 8/12/19 7:47 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Devlink from the beginning counts with network namespaces, but the
>> instances has been fixed to init_net. The first patch allows user
>> to move existing devlink instances into namespaces:
>> 
>> $ devlink dev
>> netdevsim/netdevsim1
>> $ ip netns add ns1
>> $ devlink dev set netdevsim/netdevsim1 netns ns1
>> $ devlink -N ns1 dev
>> netdevsim/netdevsim1
>> 
>> The last patch allows user to create new netdevsim instance directly
>> inside network namespace of a caller.
>
>The namespace behavior seems odd to me. If devlink instance is created
>in a namespace and never moved, it should die with the namespace. With
>this patch set, devlink instance and its ports are moved to init_net on
>namespace delete.
>
>The fib controller needs an update to return the namespace of the
>devlink instance (on top of the patch applied to net):

I have really no clue what your fib abomination should behave. The
devlink controls device config, that's it. No relation to netns it is
in.


>
>diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>index 89795071f085..fa7e876f2d3b 100644
>--- a/drivers/net/netdevsim/dev.c
>+++ b/drivers/net/netdevsim/dev.c
>@@ -114,11 +114,6 @@ static void nsim_dev_port_debugfs_exit(struct
>nsim_dev_port *nsim_dev_port)
>        debugfs_remove_recursive(nsim_dev_port->ddir);
> }
>
>-static struct net *nsim_devlink_net(struct devlink *devlink)
>-{
>-       return &init_net;
>-}
>-
> static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
> {
>        struct net *net = priv;
>@@ -154,7 +149,7 @@ static int nsim_dev_resources_register(struct
>devlink *devlink)
>                .size_granularity = 1,
>                .unit = DEVLINK_RESOURCE_UNIT_ENTRY
>        };
>-       struct net *net = nsim_devlink_net(devlink);
>+       struct net *net = devlink_net(devlink);
>        int err;
>        u64 n;
>
>@@ -309,7 +304,7 @@ static int nsim_dev_reload(struct devlink *devlink,
>                NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
>                NSIM_RESOURCE_IPV6_FIB, NSIM_RESOURCE_IPV6_FIB_RULES
>        };
>-       struct net *net = nsim_devlink_net(devlink);
>+       struct net *net = devlink_net(devlink);
>        int i;
>
>        for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {
>