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

[patch net-next 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 |   5 +-
 include/net/devlink.h             |   3 +
 include/uapi/linux/devlink.h      |   4 +
 net/core/devlink.c                | 128 ++++++++++++++++++++++++++++--
 7 files changed, 148 insertions(+), 14 deletions(-)

-- 
2.21.0

[patch net-next 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>
---
 include/uapi/linux/devlink.h |   4 ++
 net/core/devlink.c           | 112 ++++++++++++++++++++++++++++++++++-
 2 files changed, 113 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 4f40aeace902..ec024462e7d4 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -439,8 +439,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);
 }
@@ -645,6 +653,70 @@ 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)) ||
+	    (netns_fd_attr && (netns_pid_attr || netns_id_attr)) ||
+	    (netns_id_attr && (netns_pid_attr || netns_fd_attr))) {
+		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);
+	}
+	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_netns_change(struct devlink *devlink, struct net *net)
+{
+	if (net_eq(devlink_net(devlink), net))
+		return;
+	devlink_notify(devlink, DEVLINK_CMD_DEL);
+	devlink_net_set(devlink, net);
+	devlink_notify(devlink, DEVLINK_CMD_NEW);
+}
+
+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 +5256,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 +5270,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,
@@ -6955,9 +7037,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

[patch net-next 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>
---
 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 bc36f942a7d5..98b89eabd73a 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 ec024462e7d4..ad57058ed0d5 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)
 {
@@ -695,7 +704,7 @@ static void devlink_netns_change(struct devlink *devlink, struct net *net)
 	if (net_eq(devlink_net(devlink), net))
 		return;
 	devlink_notify(devlink, DEVLINK_CMD_DEL);
-	devlink_net_set(devlink, net);
+	__devlink_net_set(devlink, net);
 	devlink_notify(devlink, DEVLINK_CMD_NEW);
 }
 
@@ -5602,7 +5611,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);
@@ -5626,6 +5635,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 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>
---
 drivers/net/netdevsim/bus.c       |  1 +
 drivers/net/netdevsim/dev.c       | 17 +++++++++++------
 drivers/net/netdevsim/netdev.c    |  4 +++-
 drivers/net/netdevsim/netdevsim.h |  5 ++++-
 4 files changed, 19 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 c5c417a3c0ce..685dd21f5500 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -268,7 +268,8 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
 };
 
 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;
@@ -277,6 +278,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);
@@ -335,7 +337,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;
@@ -361,7 +363,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;
@@ -404,17 +406,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;
 	}
@@ -449,13 +453,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 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>
 #include <net/xdp.h>
 
 #define DRV_NAME	"netdevsim"
@@ -75,7 +76,8 @@ struct netdevsim {
 };
 
 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
@@ -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;
 };
-- 
2.21.0

[patch iproute2 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 d8197ea3a478..9242cc05ad0c 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -32,6 +32,7 @@
 #include "mnlg.h"
 #include "json_writer.h"
 #include "utils.h"
+#include "namespace.h"
 
 #define ESWITCH_MODE_LEGACY "legacy"
 #define ESWITCH_MODE_SWITCHDEV "switchdev"
@@ -6332,7 +6333,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");
 }
@@ -6478,6 +6479,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;
@@ -6493,7 +6495,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) {
@@ -6519,6 +6521,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 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 9242cc05ad0c..a39bd8771d8b 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -235,6 +235,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 */
@@ -271,6 +272,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 {
@@ -1331,6 +1334,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;
@@ -1444,7 +1463,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,
@@ -1499,6 +1522,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");
@@ -2551,6 +2575,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");
@@ -2747,6 +2796,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 iproute2 1/2] devlink: introduce cmdline option to switch to a different namespace

[Toke Høiland-Jørgensen]

Jiri Pirko <jiri@resnulli.us> writes:

> 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 d8197ea3a478..9242cc05ad0c 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -32,6 +32,7 @@
>  #include "mnlg.h"
>  #include "json_writer.h"
>  #include "utils.h"
> +#include "namespace.h"
>  
>  #define ESWITCH_MODE_LEGACY "legacy"
>  #define ESWITCH_MODE_SWITCHDEV "switchdev"
> @@ -6332,7 +6333,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"

'ip' uses lower-case n for this; why not be consistent?

-Toke

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

[Jiri Pirko]

Sat, Jul 27, 2019 at 12:12:48PM CEST, toke@redhat.com wrote:
>Jiri Pirko <jiri@resnulli.us> writes:
>
>> 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 d8197ea3a478..9242cc05ad0c 100644
>> --- a/devlink/devlink.c
>> +++ b/devlink/devlink.c
>> @@ -32,6 +32,7 @@
>>  #include "mnlg.h"
>>  #include "json_writer.h"
>>  #include "utils.h"
>> +#include "namespace.h"
>>  
>>  #define ESWITCH_MODE_LEGACY "legacy"
>>  #define ESWITCH_MODE_SWITCHDEV "switchdev"
>> @@ -6332,7 +6333,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"
>
>'ip' uses lower-case n for this; why not be consistent?

Because "n" is taken :/


>
>-Toke

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

[Toke Høiland-Jørgensen]

Jiri Pirko <jiri@resnulli.us> writes:

> Sat, Jul 27, 2019 at 12:12:48PM CEST, toke@redhat.com wrote:
>>Jiri Pirko <jiri@resnulli.us> writes:
>>
>>> 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 d8197ea3a478..9242cc05ad0c 100644
>>> --- a/devlink/devlink.c
>>> +++ b/devlink/devlink.c
>>> @@ -32,6 +32,7 @@
>>>  #include "mnlg.h"
>>>  #include "json_writer.h"
>>>  #include "utils.h"
>>> +#include "namespace.h"
>>>  
>>>  #define ESWITCH_MODE_LEGACY "legacy"
>>>  #define ESWITCH_MODE_SWITCHDEV "switchdev"
>>> @@ -6332,7 +6333,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"
>>
>>'ip' uses lower-case n for this; why not be consistent?
>
> Because "n" is taken :/

Ah, right, that was right there on the line below in the patch context.
Oops, by bad (and too bad!)

-Toke

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

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Sat, 27 Jul 2019 11:44:57 +0200

> +	if ((netns_pid_attr && (netns_fd_attr || netns_id_attr)) ||
> +	    (netns_fd_attr && (netns_pid_attr || netns_id_attr)) ||
> +	    (netns_id_attr && (netns_pid_attr || netns_fd_attr))) {
> +		NL_SET_ERR_MSG(info->extack, "multiple netns identifying attributes specified");
> +		return ERR_PTR(-EINVAL);
> +	}

How about:

	if (!!a + !!b + !!c > 1) {
	...

> +
> +	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);
> +	}
> +	if (IS_ERR(net)) {

I think this is going to be one of those cases where a compiler won't be able
to prove that 'net' is guaranteed to be initialized at this spot.  Please
rearrange this code somehow so that is unlikely to happen.

Thanks.

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

[Jakub Kicinski]

On Sat, 27 Jul 2019 11:44:59 +0200, Jiri Pirko wrote:
> 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>

> 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?
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.

> 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.

>  #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.kicinski@netronome.com>

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

[David Ahern]

On 7/27/19 3:44 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:
> 

so you intend for an asic, for example, to have multiple devlink
instances where each instance governs a set of related ports (e.g.,
ports that share a set of hardware resources) and those instances can be
managed from distinct network namespaces?

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

[David Ahern]

subject line should have iproute2-next

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

[David Ahern]

On 7/27/19 4:21 AM, Jiri Pirko wrote:
>>> diff --git a/devlink/devlink.c b/devlink/devlink.c
>>> index d8197ea3a478..9242cc05ad0c 100644
>>> --- a/devlink/devlink.c
>>> +++ b/devlink/devlink.c
>>> @@ -32,6 +32,7 @@
>>>  #include "mnlg.h"
>>>  #include "json_writer.h"
>>>  #include "utils.h"
>>> +#include "namespace.h"
>>>  
>>>  #define ESWITCH_MODE_LEGACY "legacy"
>>>  #define ESWITCH_MODE_SWITCHDEV "switchdev"
>>> @@ -6332,7 +6333,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"
>>
>> 'ip' uses lower-case n for this; why not be consistent?
> 
> Because "n" is taken :/

that's really unfortunate. commands within a package should have similar
syntax and -n/N are backwards between ip/tc and devlink. That's the
stuff that drives users crazy.

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

[Jiri Pirko]

Mon, Jul 29, 2019 at 07:52:16PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Sat, 27 Jul 2019 11:44:57 +0200
>
>> +	if ((netns_pid_attr && (netns_fd_attr || netns_id_attr)) ||
>> +	    (netns_fd_attr && (netns_pid_attr || netns_id_attr)) ||
>> +	    (netns_id_attr && (netns_pid_attr || netns_fd_attr))) {
>> +		NL_SET_ERR_MSG(info->extack, "multiple netns identifying attributes specified");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>
>How about:
>
>	if (!!a + !!b + !!c > 1) {
>	...

I just copied the logic from the existing code. But sure :)


>
>> +
>> +	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);
>> +	}
>> +	if (IS_ERR(net)) {
>
>I think this is going to be one of those cases where a compiler won't be able
>to prove that 'net' is guaranteed to be initialized at this spot.  Please
>rearrange this code somehow so that is unlikely to happen.

It does not complain though. The function cannot be entered unless at
least one is. I'll check again.


>
>Thanks.

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

[Jiri Pirko]

Mon, Jul 29, 2019 at 08:59:06PM CEST, jakub.kicinski@netronome.com wrote:
>On Sat, 27 Jul 2019 11:44:59 +0200, Jiri Pirko wrote:
>> 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>
>
>> 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.kicinski@netronome.com>

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

[Jiri Pirko]

Mon, Jul 29, 2019 at 10:21:11PM CEST, dsahern@gmail.com wrote:
>On 7/27/19 4:21 AM, Jiri Pirko wrote:
>>>> diff --git a/devlink/devlink.c b/devlink/devlink.c
>>>> index d8197ea3a478..9242cc05ad0c 100644
>>>> --- a/devlink/devlink.c
>>>> +++ b/devlink/devlink.c
>>>> @@ -32,6 +32,7 @@
>>>>  #include "mnlg.h"
>>>>  #include "json_writer.h"
>>>>  #include "utils.h"
>>>> +#include "namespace.h"
>>>>  
>>>>  #define ESWITCH_MODE_LEGACY "legacy"
>>>>  #define ESWITCH_MODE_SWITCHDEV "switchdev"
>>>> @@ -6332,7 +6333,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"
>>>
>>> 'ip' uses lower-case n for this; why not be consistent?
>> 
>> Because "n" is taken :/
>
>that's really unfortunate. commands within a package should have similar
>syntax and -n/N are backwards between ip/tc and devlink. That's the
>stuff that drives users crazy.

I agree. But this particular ship has sailed :(

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

[Jiri Pirko]

Mon, Jul 29, 2019 at 10:17:25PM CEST, dsahern@gmail.com wrote:
>On 7/27/19 3:44 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:
>> 
>
>so you intend for an asic, for example, to have multiple devlink
>instances where each instance governs a set of related ports (e.g.,
>ports that share a set of hardware resources) and those instances can be
>managed from distinct network namespaces?

No, no multiple devlink instances for asic intended.

>

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

[Jakub Kicinski]

On Tue, 30 Jul 2019 08:06:55 +0200, Jiri Pirko wrote:
> >> 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?

Less time to compile the kernel after net_namespace.h was touched.
Don't we all spend more time that we would like to recompiling the
kernel? :(  Not a huge deal if you have a strong preference.

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

[Jiri Pirko]

Tue, Jul 30, 2019 at 07:14:11PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 30 Jul 2019 08:06:55 +0200, Jiri Pirko wrote:
>> >> 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?
>
>Less time to compile the kernel after net_namespace.h was touched.
>Don't we all spend more time that we would like to recompiling the
>kernel? :(  Not a huge deal if you have a strong preference.

I removed it in v2. I don't care that much either :)

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

[David Ahern]

On 7/30/19 12:08 AM, Jiri Pirko wrote:
> Mon, Jul 29, 2019 at 10:17:25PM CEST, dsahern@gmail.com wrote:
>> On 7/27/19 3:44 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:
>>>
>>
>> so you intend for an asic, for example, to have multiple devlink
>> instances where each instance governs a set of related ports (e.g.,
>> ports that share a set of hardware resources) and those instances can be
>> managed from distinct network namespaces?
> 
> No, no multiple devlink instances for asic intended.

So it should be allowed for an asic to have resources split across
network namespaces. e.g., something like this:

   namespace 1 |  namespace 2  | ... | namespace N
               |               |     |
  { ports 1 }  |   { ports 2 } | ... | { ports N }
               |               |     |
   devlink 1   |    devlink 2  | ... |  devlink N
  =================================================
                   driver

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

[Jakub Kicinski]

On Wed, 31 Jul 2019 15:50:26 -0600, David Ahern wrote:
> On 7/30/19 12:08 AM, Jiri Pirko wrote:
> > Mon, Jul 29, 2019 at 10:17:25PM CEST, dsahern@gmail.com wrote:  
> >> On 7/27/19 3:44 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:
> >>>  
> >>
> >> so you intend for an asic, for example, to have multiple devlink
> >> instances where each instance governs a set of related ports (e.g.,
> >> ports that share a set of hardware resources) and those instances can be
> >> managed from distinct network namespaces?  
> > 
> > No, no multiple devlink instances for asic intended.  
> 
> So it should be allowed for an asic to have resources split across
> network namespaces. e.g., something like this:
> 
>    namespace 1 |  namespace 2  | ... | namespace N
>                |               |     |
>   { ports 1 }  |   { ports 2 } | ... | { ports N }
>                |               |     |
>    devlink 1   |    devlink 2  | ... |  devlink N
>   =================================================
>                    driver

Can you elaborate further? Ports for most purposes are represented by
netdevices. Devlink port instances expose global topological view of
the ports which is primarily relevant if you can see the entire ASIC.
I think the global configuration and global view of resources is still
the most relevant need, so in your diagram you must account for some
"all-seeing" instance, e.g.:

   namespace 1 |  namespace 2  | ... | namespace N
               |               |     |
  { ports 1 }  |   { ports 2 } | ... | { ports N }
               |               |     |
subdevlink 1   | subdevlink 2  | ... |  subdevlink N
         \______      |              _______/
                 master ASIC devlink
  =================================================
                   driver

No?

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

[David Ahern]

On 7/31/19 4:02 PM, Jakub Kicinski wrote:
> On Wed, 31 Jul 2019 15:50:26 -0600, David Ahern wrote:
>> On 7/30/19 12:08 AM, Jiri Pirko wrote:
>>> Mon, Jul 29, 2019 at 10:17:25PM CEST, dsahern@gmail.com wrote:  
>>>> On 7/27/19 3:44 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:
>>>>>  
>>>>
>>>> so you intend for an asic, for example, to have multiple devlink
>>>> instances where each instance governs a set of related ports (e.g.,
>>>> ports that share a set of hardware resources) and those instances can be
>>>> managed from distinct network namespaces?  
>>>
>>> No, no multiple devlink instances for asic intended.  
>>
>> So it should be allowed for an asic to have resources split across
>> network namespaces. e.g., something like this:
>>
>>    namespace 1 |  namespace 2  | ... | namespace N
>>                |               |     |
>>   { ports 1 }  |   { ports 2 } | ... | { ports N }
>>                |               |     |
>>    devlink 1   |    devlink 2  | ... |  devlink N
>>   =================================================
>>                    driver
> 
> Can you elaborate further? Ports for most purposes are represented by
> netdevices. Devlink port instances expose global topological view of
> the ports which is primarily relevant if you can see the entire ASIC.
> I think the global configuration and global view of resources is still
> the most relevant need, so in your diagram you must account for some
> "all-seeing" instance, e.g.:
> 
>    namespace 1 |  namespace 2  | ... | namespace N
>                |               |     |
>   { ports 1 }  |   { ports 2 } | ... | { ports N }
>                |               |     |
> subdevlink 1   | subdevlink 2  | ... |  subdevlink N
>          \______      |              _______/
>                  master ASIC devlink
>   =================================================
>                    driver
> 
> No?
> 

sure, there could be a master devlink visible to the user if that makes
sense or the driver can account for it behind the scenes as the sum of
the devlink instances.

The goal is to allow ports within an asic [1] to be divided across
network namespace where each namespace sees a subset of the ports. This
allows creating multiple logical switches from a single physical asic.

[1] within constraints imposed by the driver/hardware - for example to
account for resources shared by a set of ports. e.g., front panel ports
1 - 4 have shared resources and must always be in the same devlink instance.

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

[Jakub Kicinski]

On Wed, 31 Jul 2019 16:07:31 -0600, David Ahern wrote:
> On 7/31/19 4:02 PM, Jakub Kicinski wrote:
> > Can you elaborate further? Ports for most purposes are represented by
> > netdevices. Devlink port instances expose global topological view of
> > the ports which is primarily relevant if you can see the entire ASIC.
> > I think the global configuration and global view of resources is still
> > the most relevant need, so in your diagram you must account for some
> > "all-seeing" instance, e.g.:
> > 
> >    namespace 1 |  namespace 2  | ... | namespace N
> >                |               |     |
> >   { ports 1 }  |   { ports 2 } | ... | { ports N }
> >                |               |     |
> > subdevlink 1   | subdevlink 2  | ... |  subdevlink N
> >          \______      |              _______/
> >                  master ASIC devlink
> >   =================================================
> >                    driver
> > 
> > No?
> 
> sure, there could be a master devlink visible to the user if that makes
> sense or the driver can account for it behind the scenes as the sum of
> the devlink instances.
> 
> The goal is to allow ports within an asic [1] to be divided across
> network namespace where each namespace sees a subset of the ports. This
> allows creating multiple logical switches from a single physical asic.
> 
> [1] within constraints imposed by the driver/hardware - for example to
> account for resources shared by a set of ports. e.g., front panel ports
> 1 - 4 have shared resources and must always be in the same devlink instance.

So the ASIC would start out all partitioned? Presumably some would
still like to use it non-partitioned? What follows there must be a top
level instance to decide on partitioning, and moving resources between
sub-instances.

Right now I don't think there is much info in devlink ports which would
be relevant without full view of the ASIC..

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

[David Ahern]

On 7/31/19 4:28 PM, Jakub Kicinski wrote:
> On Wed, 31 Jul 2019 16:07:31 -0600, David Ahern wrote:
>> On 7/31/19 4:02 PM, Jakub Kicinski wrote:
>>> Can you elaborate further? Ports for most purposes are represented by
>>> netdevices. Devlink port instances expose global topological view of
>>> the ports which is primarily relevant if you can see the entire ASIC.
>>> I think the global configuration and global view of resources is still
>>> the most relevant need, so in your diagram you must account for some
>>> "all-seeing" instance, e.g.:
>>>
>>>    namespace 1 |  namespace 2  | ... | namespace N
>>>                |               |     |
>>>   { ports 1 }  |   { ports 2 } | ... | { ports N }
>>>                |               |     |
>>> subdevlink 1   | subdevlink 2  | ... |  subdevlink N
>>>          \______      |              _______/
>>>                  master ASIC devlink
>>>   =================================================
>>>                    driver
>>>
>>> No?
>>
>> sure, there could be a master devlink visible to the user if that makes
>> sense or the driver can account for it behind the scenes as the sum of
>> the devlink instances.
>>
>> The goal is to allow ports within an asic [1] to be divided across
>> network namespace where each namespace sees a subset of the ports. This
>> allows creating multiple logical switches from a single physical asic.
>>
>> [1] within constraints imposed by the driver/hardware - for example to
>> account for resources shared by a set of ports. e.g., front panel ports
>> 1 - 4 have shared resources and must always be in the same devlink instance.
> 
> So the ASIC would start out all partitioned? Presumably some would
> still like to use it non-partitioned? What follows there must be a top
> level instance to decide on partitioning, and moving resources between
> sub-instances.
> 
> Right now I don't think there is much info in devlink ports which would
> be relevant without full view of the ASIC..
> 

not sure how it would play out. really just 'thinking out loud' about
the above use case to make sure devlink with proper namespace support
allows it - or does not prevent it.

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

[Jiri Pirko]

Thu, Aug 01, 2019 at 12:31:52AM CEST, dsahern@gmail.com wrote:
>On 7/31/19 4:28 PM, Jakub Kicinski wrote:
>> On Wed, 31 Jul 2019 16:07:31 -0600, David Ahern wrote:
>>> On 7/31/19 4:02 PM, Jakub Kicinski wrote:
>>>> Can you elaborate further? Ports for most purposes are represented by
>>>> netdevices. Devlink port instances expose global topological view of
>>>> the ports which is primarily relevant if you can see the entire ASIC.
>>>> I think the global configuration and global view of resources is still
>>>> the most relevant need, so in your diagram you must account for some
>>>> "all-seeing" instance, e.g.:
>>>>
>>>>    namespace 1 |  namespace 2  | ... | namespace N
>>>>                |               |     |
>>>>   { ports 1 }  |   { ports 2 } | ... | { ports N }
>>>>                |               |     |
>>>> subdevlink 1   | subdevlink 2  | ... |  subdevlink N
>>>>          \______      |              _______/
>>>>                  master ASIC devlink
>>>>   =================================================
>>>>                    driver
>>>>
>>>> No?
>>>
>>> sure, there could be a master devlink visible to the user if that makes
>>> sense or the driver can account for it behind the scenes as the sum of
>>> the devlink instances.
>>>
>>> The goal is to allow ports within an asic [1] to be divided across
>>> network namespace where each namespace sees a subset of the ports. This
>>> allows creating multiple logical switches from a single physical asic.
>>>
>>> [1] within constraints imposed by the driver/hardware - for example to
>>> account for resources shared by a set of ports. e.g., front panel ports
>>> 1 - 4 have shared resources and must always be in the same devlink instance.
>> 
>> So the ASIC would start out all partitioned? Presumably some would
>> still like to use it non-partitioned? What follows there must be a top
>> level instance to decide on partitioning, and moving resources between
>> sub-instances.
>> 
>> Right now I don't think there is much info in devlink ports which would
>> be relevant without full view of the ASIC..
>> 
>
>not sure how it would play out. really just 'thinking out loud' about
>the above use case to make sure devlink with proper namespace support
>allows it - or does not prevent it.

I Don't see reason or usecase to have ports or other objects of devlink
in separate namespaces. Devlink and it's objects are one big item,
should be always together.