[patch net-next rfc 00/15] netdevsim: impement proper device model

[patch net-next rfc 00/15] netdevsim: impement proper device model

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Currently the model of netdevsim is a bit odd in multiple ways.
1) devlink instance is not in any way related with actual netdevsim
   netdevices. Instead, it is created per-namespace.
2) multi-port netdevsim device is done using "link" attribute.
3) netdevsim bus is there only to have something to bind the netdev to,
   it really does not act as a bus.
4) netdevsim instances are created by "ip link add" which is great for
   soft devices with no hw backend. The rtnl core allocates netdev and
   calls into driver holding rtnl mutex. For hw-backed devices, this
   flow is wrong as it breaks order in which things are done.

This patchset adjust netdevsim to fix all above.

In order to support proper devlink and devlink port instances and to be
able to emulate real devices, there is need to implement bus probe and
instantiate everything from there. User can specify device id and port
count to be instantianted. For example:

echo "10 4" > /sys/bus/netdevsim/new_device

Then devlink shows this:

$ devlink dev
netdevsim/netdevsim10

$ devlink port
netdevsim/netdevsim10/0: type eth netdev netdevsim10p1 flavour physical
netdevsim/netdevsim10/1: type eth netdev netdevsim10p2 flavour physical
netdevsim/netdevsim10/2: type eth netdev netdevsim10p3 flavour physical
netdevsim/netdevsim10/3: type eth netdev netdevsim10p4 flavour physical

Debugfs topology is also adjusted a bit. The rest stays the same as
before.

TODO:
- teach udev to rename netdevsim netdevices similarly to pci netdevices
- fix tools/testing/selftests/bpf/test_offload.py to work with new iface

Jiri Pirko (15):
  netdevsim: move device registration on bus to be done earlier in init
  netdevsim: create devlink instance per netdevsim instance
  netdevsim: rename devlink.c to dev.c to contain per-dev(asic) items
  netdevsim: put netdevsim bus code into separate file
  netdevsim: move device registration and related code to bus.c
  netdevsim: add stub netdevsim driver implementation
  netdevsim: use ida for bus device ids
  netdevsim: add bus attributes to add new and delete devices
  netdevsim: rename dev_init/exit() functions and make them independent
    on ns
  netdevsim: merge sdev into dev
  netdevsim: generate random switch id instead of using dev id
  netdevsim: change debugfs tree topology
  netdevsim: implement dev probe/remove skeleton with port
    initialization
  netdevsim: move netdev creation/destruction to dev probe
  netdevsim: implement ndo_get_devlink_port

 drivers/net/netdevsim/Makefile              |   2 +-
 drivers/net/netdevsim/bpf.c                 |  92 ++---
 drivers/net/netdevsim/bus.c                 | 305 +++++++++++++++
 drivers/net/netdevsim/dev.c                 | 385 +++++++++++++++++++
 drivers/net/netdevsim/devlink.c             | 295 ---------------
 drivers/net/netdevsim/fib.c                 | 103 +++--
 drivers/net/netdevsim/ipsec.c               |   3 +-
 drivers/net/netdevsim/netdev.c              | 396 +++++---------------
 drivers/net/netdevsim/netdevsim.h           | 124 +++---
 tools/testing/selftests/bpf/test_offload.py |  12 +-
 tools/testing/selftests/net/rtnetlink.sh    |   8 +-
 11 files changed, 964 insertions(+), 761 deletions(-)
 create mode 100644 drivers/net/netdevsim/bus.c
 create mode 100644 drivers/net/netdevsim/dev.c
 delete mode 100644 drivers/net/netdevsim/devlink.c

-- 
2.17.2

[patch net-next rfc 01/15] netdevsim: move device registration on bus to be done earlier in init

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

As a dependency of the subsequent patch, mode device registration to be
done earlier, directly in nsim_newlink().

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/netdev.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 7805fa840383..6ac5447bca02 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -174,25 +174,14 @@ static int nsim_init(struct net_device *dev)
 	if (err)
 		goto err_debugfs_destroy;
 
-	ns->dev.id = nsim_dev_id++;
-	ns->dev.bus = &nsim_bus;
-	ns->dev.type = &nsim_dev_type;
-	err = device_register(&ns->dev);
-	if (err)
-		goto err_bpf_uninit;
-
-	SET_NETDEV_DEV(dev, &ns->dev);
-
 	err = nsim_devlink_setup(ns);
 	if (err)
-		goto err_unreg_dev;
+		goto err_bpf_uninit;
 
 	nsim_ipsec_init(ns);
 
 	return 0;
 
-err_unreg_dev:
-	device_unregister(&ns->dev);
 err_bpf_uninit:
 	nsim_bpf_uninit(ns);
 err_debugfs_destroy:
@@ -514,11 +503,22 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 	if (IS_ERR(ns->sdev))
 		return PTR_ERR(ns->sdev);
 
-	err = register_netdevice(dev);
+	ns->dev.id = nsim_dev_id++;
+	ns->dev.bus = &nsim_bus;
+	ns->dev.type = &nsim_dev_type;
+	err = device_register(&ns->dev);
 	if (err)
 		goto err_sdev_put;
+
+	SET_NETDEV_DEV(dev, &ns->dev);
+
+	err = register_netdevice(dev);
+	if (err)
+		goto err_unreg_dev;
 	return 0;
 
+err_unreg_dev:
+	device_unregister(&ns->dev);
 err_sdev_put:
 	nsim_sdev_put(ns->sdev);
 	return err;
-- 
2.17.2

[patch net-next rfc 02/15] netdevsim: create devlink instance per netdevsim instance

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Currently there is one devlink instance created per network namespace.
That is quite odd considering the fact that devlink instance should
represent an ASIC. The following patches are going to move the devlink
instance even more down to a bus device, but until then, have one
devlink instance per netdevsim instance. Struct nsim_devlink is
introduced to hold fib setting.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/devlink.c   | 179 +++++++++++-------------------
 drivers/net/netdevsim/fib.c       | 103 +++++++----------
 drivers/net/netdevsim/netdev.c    |  27 ++---
 drivers/net/netdevsim/netdevsim.h |  18 +--
 4 files changed, 125 insertions(+), 202 deletions(-)

diff --git a/drivers/net/netdevsim/devlink.c b/drivers/net/netdevsim/devlink.c
index 5135fc371f01..f718912fa52d 100644
--- a/drivers/net/netdevsim/devlink.c
+++ b/drivers/net/netdevsim/devlink.c
@@ -15,59 +15,59 @@
  */
 
 #include <linux/device.h>
+#include <linux/rtnetlink.h>
 #include <net/devlink.h>
-#include <net/netns/generic.h>
 
 #include "netdevsim.h"
 
-static unsigned int nsim_devlink_id;
-
-/* place holder until devlink and namespaces is sorted out */
-static struct net *nsim_devlink_net(struct devlink *devlink)
-{
-	return &init_net;
-}
+struct nsim_devlink {
+	struct nsim_fib_data *fib_data;
+};
 
 /* IPv4
  */
 static u64 nsim_ipv4_fib_resource_occ_get(void *priv)
 {
-	struct net *net = priv;
+	struct nsim_devlink *nsim_devlink = priv;
 
-	return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, false);
+	return nsim_fib_get_val(nsim_devlink->fib_data,
+				NSIM_RESOURCE_IPV4_FIB, false);
 }
 
 static u64 nsim_ipv4_fib_rules_res_occ_get(void *priv)
 {
-	struct net *net = priv;
+	struct nsim_devlink *nsim_devlink = priv;
 
-	return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, false);
+	return nsim_fib_get_val(nsim_devlink->fib_data,
+				NSIM_RESOURCE_IPV4_FIB_RULES, false);
 }
 
 /* IPv6
  */
 static u64 nsim_ipv6_fib_resource_occ_get(void *priv)
 {
-	struct net *net = priv;
+	struct nsim_devlink *nsim_devlink = priv;
 
-	return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, false);
+	return nsim_fib_get_val(nsim_devlink->fib_data,
+				NSIM_RESOURCE_IPV6_FIB, false);
 }
 
 static u64 nsim_ipv6_fib_rules_res_occ_get(void *priv)
 {
-	struct net *net = priv;
+	struct nsim_devlink *nsim_devlink = priv;
 
-	return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, false);
+	return nsim_fib_get_val(nsim_devlink->fib_data,
+				NSIM_RESOURCE_IPV6_FIB_RULES, false);
 }
 
 static int devlink_resources_register(struct devlink *devlink)
 {
+	struct nsim_devlink *nsim_devlink = devlink_priv(devlink);
 	struct devlink_resource_size_params params = {
 		.size_max = (u64)-1,
 		.size_granularity = 1,
 		.unit = DEVLINK_RESOURCE_UNIT_ENTRY
 	};
-	struct net *net = nsim_devlink_net(devlink);
 	int err;
 	u64 n;
 
@@ -81,7 +81,8 @@ static int devlink_resources_register(struct devlink *devlink)
 		goto out;
 	}
 
-	n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, true);
+	n = nsim_fib_get_val(nsim_devlink->fib_data,
+			     NSIM_RESOURCE_IPV4_FIB, true);
 	err = devlink_resource_register(devlink, "fib", n,
 					NSIM_RESOURCE_IPV4_FIB,
 					NSIM_RESOURCE_IPV4, &params);
@@ -90,7 +91,8 @@ static int devlink_resources_register(struct devlink *devlink)
 		return err;
 	}
 
-	n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, true);
+	n = nsim_fib_get_val(nsim_devlink->fib_data,
+			     NSIM_RESOURCE_IPV4_FIB_RULES, true);
 	err = devlink_resource_register(devlink, "fib-rules", n,
 					NSIM_RESOURCE_IPV4_FIB_RULES,
 					NSIM_RESOURCE_IPV4, &params);
@@ -109,7 +111,8 @@ static int devlink_resources_register(struct devlink *devlink)
 		goto out;
 	}
 
-	n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, true);
+	n = nsim_fib_get_val(nsim_devlink->fib_data,
+			     NSIM_RESOURCE_IPV6_FIB, true);
 	err = devlink_resource_register(devlink, "fib", n,
 					NSIM_RESOURCE_IPV6_FIB,
 					NSIM_RESOURCE_IPV6, &params);
@@ -118,7 +121,8 @@ static int devlink_resources_register(struct devlink *devlink)
 		return err;
 	}
 
-	n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, true);
+	n = nsim_fib_get_val(nsim_devlink->fib_data,
+			     NSIM_RESOURCE_IPV6_FIB_RULES, true);
 	err = devlink_resource_register(devlink, "fib-rules", n,
 					NSIM_RESOURCE_IPV6_FIB_RULES,
 					NSIM_RESOURCE_IPV6, &params);
@@ -130,19 +134,19 @@ static int devlink_resources_register(struct devlink *devlink)
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV4_FIB,
 					  nsim_ipv4_fib_resource_occ_get,
-					  net);
+					  nsim_devlink);
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV4_FIB_RULES,
 					  nsim_ipv4_fib_rules_res_occ_get,
-					  net);
+					  nsim_devlink);
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV6_FIB,
 					  nsim_ipv6_fib_resource_occ_get,
-					  net);
+					  nsim_devlink);
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV6_FIB_RULES,
 					  nsim_ipv6_fib_rules_res_occ_get,
-					  net);
+					  nsim_devlink);
 out:
 	return err;
 }
@@ -150,11 +154,11 @@ static int devlink_resources_register(struct devlink *devlink)
 static int nsim_devlink_reload(struct devlink *devlink,
 			       struct netlink_ext_ack *extack)
 {
+	struct nsim_devlink *nsim_devlink = devlink_priv(devlink);
 	enum nsim_resource_id res_ids[] = {
 		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);
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {
@@ -163,7 +167,8 @@ static int nsim_devlink_reload(struct devlink *devlink,
 
 		err = devlink_resource_size_get(devlink, res_ids[i], &val);
 		if (!err) {
-			err = nsim_fib_set_max(net, res_ids[i], val, extack);
+			err = nsim_fib_set_max(nsim_devlink->fib_data,
+					       res_ids[i], val, extack);
 			if (err)
 				return err;
 		}
@@ -172,124 +177,68 @@ static int nsim_devlink_reload(struct devlink *devlink,
 	return 0;
 }
 
-static void nsim_devlink_net_reset(struct net *net)
-{
-	enum nsim_resource_id res_ids[] = {
-		NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
-		NSIM_RESOURCE_IPV6_FIB, NSIM_RESOURCE_IPV6_FIB_RULES
-	};
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {
-		if (nsim_fib_set_max(net, res_ids[i], (u64)-1, NULL)) {
-			pr_err("Failed to reset limit for resource %u\n",
-			       res_ids[i]);
-		}
-	}
-}
-
 static const struct devlink_ops nsim_devlink_ops = {
 	.reload = nsim_devlink_reload,
 };
 
-/* once devlink / namespace issues are sorted out
- * this needs to be net in which a devlink instance
- * is to be created. e.g., dev_net(ns->netdev)
- */
-static struct net *nsim_to_net(struct netdevsim *ns)
+static int __nsim_devlink_init(struct netdevsim *ns)
 {
-	return &init_net;
-}
-
-void nsim_devlink_teardown(struct netdevsim *ns)
-{
-	if (ns->devlink) {
-		struct net *net = nsim_to_net(ns);
-		bool *reg_devlink = net_generic(net, nsim_devlink_id);
-
-		devlink_resources_unregister(ns->devlink, NULL);
-		devlink_unregister(ns->devlink);
-		devlink_free(ns->devlink);
-		ns->devlink = NULL;
-
-		nsim_devlink_net_reset(net);
-		*reg_devlink = true;
-	}
-}
-
-int nsim_devlink_setup(struct netdevsim *ns)
-{
-	struct net *net = nsim_to_net(ns);
-	bool *reg_devlink = net_generic(net, nsim_devlink_id);
+	struct nsim_devlink *nsim_devlink;
 	struct devlink *devlink;
 	int err;
 
-	/* only one device per namespace controls devlink */
-	if (!*reg_devlink) {
-		ns->devlink = NULL;
-		return 0;
-	}
-
-	devlink = devlink_alloc(&nsim_devlink_ops, 0);
+	devlink = devlink_alloc(&nsim_devlink_ops, sizeof(*nsim_devlink));
 	if (!devlink)
 		return -ENOMEM;
+	nsim_devlink = devlink_priv(devlink);
 
-	err = devlink_register(devlink, &ns->dev);
-	if (err)
+	nsim_devlink->fib_data = nsim_fib_create();
+	if (IS_ERR(nsim_devlink->fib_data)) {
+		err = PTR_ERR(nsim_devlink->fib_data);
 		goto err_devlink_free;
+	}
 
 	err = devlink_resources_register(devlink);
 	if (err)
-		goto err_dl_unregister;
+		goto err_fib_destroy;
 
-	ns->devlink = devlink;
+	err = devlink_register(devlink, &ns->dev);
+	if (err)
+		goto err_resources_unregister;
 
-	*reg_devlink = false;
+	ns->devlink = devlink;
 
 	return 0;
 
-err_dl_unregister:
-	devlink_unregister(devlink);
+err_resources_unregister:
+	devlink_resources_unregister(devlink, NULL);
+err_fib_destroy:
+	nsim_fib_destroy(nsim_devlink->fib_data);
 err_devlink_free:
 	devlink_free(devlink);
 
 	return err;
 }
 
-/* Initialize per network namespace state */
-static int __net_init nsim_devlink_netns_init(struct net *net)
+int nsim_devlink_init(struct netdevsim *ns)
 {
-	bool *reg_devlink = net_generic(net, nsim_devlink_id);
-
-	*reg_devlink = true;
-
-	return 0;
-}
-
-static struct pernet_operations nsim_devlink_net_ops = {
-	.init = nsim_devlink_netns_init,
-	.id   = &nsim_devlink_id,
-	.size = sizeof(bool),
-};
+	int err;
 
-void nsim_devlink_exit(void)
-{
-	unregister_pernet_subsys(&nsim_devlink_net_ops);
-	nsim_fib_exit();
+	dev_hold(ns->netdev);
+	rtnl_unlock();
+	err = __nsim_devlink_init(ns);
+	rtnl_lock();
+	dev_put(ns->netdev);
+	return err;
 }
 
-int nsim_devlink_init(void)
+void nsim_devlink_exit(struct netdevsim *ns)
 {
-	int err;
+	struct devlink *devlink = ns->devlink;
+	struct nsim_devlink *nsim_devlink = devlink_priv(devlink);
 
-	err = nsim_fib_init();
-	if (err)
-		goto err_out;
-
-	err = register_pernet_subsys(&nsim_devlink_net_ops);
-	if (err)
-		nsim_fib_exit();
-
-err_out:
-	return err;
+	devlink_unregister(devlink);
+	devlink_resources_unregister(devlink, NULL);
+	nsim_fib_destroy(nsim_devlink->fib_data);
+	devlink_free(devlink);
 }
diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
index f61d094746c0..4387ba05cd8d 100644
--- a/drivers/net/netdevsim/fib.c
+++ b/drivers/net/netdevsim/fib.c
@@ -18,7 +18,6 @@
 #include <net/ip_fib.h>
 #include <net/ip6_fib.h>
 #include <net/fib_rules.h>
-#include <net/netns/generic.h>
 
 #include "netdevsim.h"
 
@@ -33,15 +32,14 @@ struct nsim_per_fib_data {
 };
 
 struct nsim_fib_data {
+	struct notifier_block fib_nb;
 	struct nsim_per_fib_data ipv4;
 	struct nsim_per_fib_data ipv6;
 };
 
-static unsigned int nsim_fib_net_id;
-
-u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max)
+u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
+		     enum nsim_resource_id res_id, bool max)
 {
-	struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
 	struct nsim_fib_entry *entry;
 
 	switch (res_id) {
@@ -64,10 +62,10 @@ u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max)
 	return max ? entry->max : entry->num;
 }
 
-int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
+int nsim_fib_set_max(struct nsim_fib_data *fib_data,
+		     enum nsim_resource_id res_id, u64 val,
 		     struct netlink_ext_ack *extack)
 {
-	struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
 	struct nsim_fib_entry *entry;
 	int err = 0;
 
@@ -120,9 +118,9 @@ static int nsim_fib_rule_account(struct nsim_fib_entry *entry, bool add,
 	return err;
 }
 
-static int nsim_fib_rule_event(struct fib_notifier_info *info, bool add)
+static int nsim_fib_rule_event(struct nsim_fib_data *data,
+			       struct fib_notifier_info *info, bool add)
 {
-	struct nsim_fib_data *data = net_generic(info->net, nsim_fib_net_id);
 	struct netlink_ext_ack *extack = info->extack;
 	int err = 0;
 
@@ -157,9 +155,9 @@ static int nsim_fib_account(struct nsim_fib_entry *entry, bool add,
 	return err;
 }
 
-static int nsim_fib_event(struct fib_notifier_info *info, bool add)
+static int nsim_fib_event(struct nsim_fib_data *data,
+			  struct fib_notifier_info *info, bool add)
 {
-	struct nsim_fib_data *data = net_generic(info->net, nsim_fib_net_id);
 	struct netlink_ext_ack *extack = info->extack;
 	int err = 0;
 
@@ -178,18 +176,22 @@ static int nsim_fib_event(struct fib_notifier_info *info, bool add)
 static int nsim_fib_event_nb(struct notifier_block *nb, unsigned long event,
 			     void *ptr)
 {
+	struct nsim_fib_data *data = container_of(nb, struct nsim_fib_data,
+						  fib_nb);
 	struct fib_notifier_info *info = ptr;
 	int err = 0;
 
 	switch (event) {
 	case FIB_EVENT_RULE_ADD: /* fall through */
 	case FIB_EVENT_RULE_DEL:
-		err = nsim_fib_rule_event(info, event == FIB_EVENT_RULE_ADD);
+		err = nsim_fib_rule_event(data, info,
+					  event == FIB_EVENT_RULE_ADD);
 		break;
 
 	case FIB_EVENT_ENTRY_ADD:  /* fall through */
 	case FIB_EVENT_ENTRY_DEL:
-		err = nsim_fib_event(info, event == FIB_EVENT_ENTRY_ADD);
+		err = nsim_fib_event(data, info,
+				     event == FIB_EVENT_ENTRY_ADD);
 		break;
 	}
 
@@ -199,30 +201,23 @@ static int nsim_fib_event_nb(struct notifier_block *nb, unsigned long event,
 /* inconsistent dump, trying again */
 static void nsim_fib_dump_inconsistent(struct notifier_block *nb)
 {
-	struct nsim_fib_data *data;
-	struct net *net;
-
-	rcu_read_lock();
-	for_each_net_rcu(net) {
-		data = net_generic(net, nsim_fib_net_id);
-
-		data->ipv4.fib.num = 0ULL;
-		data->ipv4.rules.num = 0ULL;
+	struct nsim_fib_data *data = container_of(nb, struct nsim_fib_data,
+						  fib_nb);
 
-		data->ipv6.fib.num = 0ULL;
-		data->ipv6.rules.num = 0ULL;
-	}
-	rcu_read_unlock();
+	data->ipv4.fib.num = 0ULL;
+	data->ipv4.rules.num = 0ULL;
+	data->ipv6.fib.num = 0ULL;
+	data->ipv6.rules.num = 0ULL;
 }
 
-static struct notifier_block nsim_fib_nb = {
-	.notifier_call = nsim_fib_event_nb,
-};
-
-/* Initialize per network namespace state */
-static int __net_init nsim_fib_netns_init(struct net *net)
+struct nsim_fib_data *nsim_fib_create(void)
 {
-	struct nsim_fib_data *data = net_generic(net, nsim_fib_net_id);
+	struct nsim_fib_data *data;
+	int err;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return ERR_PTR(-ENOMEM);
 
 	data->ipv4.fib.max = (u64)-1;
 	data->ipv4.rules.max = (u64)-1;
@@ -230,37 +225,23 @@ static int __net_init nsim_fib_netns_init(struct net *net)
 	data->ipv6.fib.max = (u64)-1;
 	data->ipv6.rules.max = (u64)-1;
 
-	return 0;
-}
-
-static struct pernet_operations nsim_fib_net_ops = {
-	.init = nsim_fib_netns_init,
-	.id   = &nsim_fib_net_id,
-	.size = sizeof(struct nsim_fib_data),
-};
-
-void nsim_fib_exit(void)
-{
-	unregister_pernet_subsys(&nsim_fib_net_ops);
-	unregister_fib_notifier(&nsim_fib_nb);
-}
-
-int nsim_fib_init(void)
-{
-	int err;
-
-	err = register_pernet_subsys(&nsim_fib_net_ops);
-	if (err < 0) {
-		pr_err("Failed to register pernet subsystem\n");
-		goto err_out;
-	}
-
-	err = register_fib_notifier(&nsim_fib_nb, nsim_fib_dump_inconsistent);
-	if (err < 0) {
+	data->fib_nb.notifier_call = nsim_fib_event_nb;
+	err = register_fib_notifier(&data->fib_nb, nsim_fib_dump_inconsistent);
+	if (err) {
 		pr_err("Failed to register fib notifier\n");
 		goto err_out;
 	}
 
+	return data;
+
 err_out:
-	return err;
+	kfree(data);
+	return ERR_PTR(err);
 }
+
+void nsim_fib_destroy(struct nsim_fib_data *data)
+{
+	unregister_fib_notifier(&data->fib_nb);
+	kfree(data);
+}
+
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 6ac5447bca02..04aa084dc34c 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -161,7 +161,6 @@ static int nsim_init(struct net_device *dev)
 	char sdev_link_name[32];
 	int err;
 
-	ns->netdev = dev;
 	ns->ddir = debugfs_create_dir(netdev_name(dev), nsim_ddir);
 	if (IS_ERR_OR_NULL(ns->ddir))
 		return -ENOMEM;
@@ -174,16 +173,10 @@ static int nsim_init(struct net_device *dev)
 	if (err)
 		goto err_debugfs_destroy;
 
-	err = nsim_devlink_setup(ns);
-	if (err)
-		goto err_bpf_uninit;
-
 	nsim_ipsec_init(ns);
 
 	return 0;
 
-err_bpf_uninit:
-	nsim_bpf_uninit(ns);
 err_debugfs_destroy:
 	debugfs_remove_recursive(ns->ddir);
 	return err;
@@ -194,7 +187,6 @@ static void nsim_uninit(struct net_device *dev)
 	struct netdevsim *ns = netdev_priv(dev);
 
 	nsim_ipsec_teardown(ns);
-	nsim_devlink_teardown(ns);
 	debugfs_remove_recursive(ns->ddir);
 	nsim_bpf_uninit(ns);
 }
@@ -203,6 +195,7 @@ static void nsim_free(struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
 
+	nsim_devlink_exit(ns);
 	device_unregister(&ns->dev);
 	/* netdev and vf state will be freed out of device_release() */
 	nsim_sdev_put(ns->sdev);
@@ -511,12 +504,19 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 		goto err_sdev_put;
 
 	SET_NETDEV_DEV(dev, &ns->dev);
+	ns->netdev = dev;
 
-	err = register_netdevice(dev);
+	err = nsim_devlink_init(ns);
 	if (err)
 		goto err_unreg_dev;
+
+	err = register_netdevice(dev);
+	if (err)
+		goto err_devlink_exit;
 	return 0;
 
+err_devlink_exit:
+	nsim_devlink_exit(ns);
 err_unreg_dev:
 	device_unregister(&ns->dev);
 err_sdev_put:
@@ -548,18 +548,12 @@ static int __init nsim_module_init(void)
 	if (err)
 		goto err_sdev_exit;
 
-	err = nsim_devlink_init();
-	if (err)
-		goto err_unreg_bus;
-
 	err = rtnl_link_register(&nsim_link_ops);
 	if (err)
-		goto err_dl_fini;
+		goto err_unreg_bus;
 
 	return 0;
 
-err_dl_fini:
-	nsim_devlink_exit();
 err_unreg_bus:
 	bus_unregister(&nsim_bus);
 err_sdev_exit:
@@ -572,7 +566,6 @@ static int __init nsim_module_init(void)
 static void __exit nsim_module_exit(void)
 {
 	rtnl_link_unregister(&nsim_link_ops);
-	nsim_devlink_exit();
 	bus_unregister(&nsim_bus);
 	nsim_sdev_exit();
 	debugfs_remove_recursive(nsim_ddir);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 2667f9b0e1f9..df50eb19715d 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -30,6 +30,7 @@ struct bpf_prog;
 struct bpf_offload_dev;
 struct dentry;
 struct nsim_vf_config;
+struct nsim_fib_data;
 
 struct netdevsim_shared_dev {
 	unsigned int refcnt;
@@ -153,16 +154,15 @@ enum nsim_resource_id {
 	NSIM_RESOURCE_IPV6_FIB_RULES,
 };
 
-int nsim_devlink_setup(struct netdevsim *ns);
-void nsim_devlink_teardown(struct netdevsim *ns);
+int nsim_devlink_init(struct netdevsim *ns);
+void nsim_devlink_exit(struct netdevsim *ns);
 
-int nsim_devlink_init(void);
-void nsim_devlink_exit(void);
-
-int nsim_fib_init(void);
-void nsim_fib_exit(void);
-u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max);
-int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
+struct nsim_fib_data *nsim_fib_create(void);
+void nsim_fib_destroy(struct nsim_fib_data *fib_data);
+u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
+		     enum nsim_resource_id res_id, bool max);
+int nsim_fib_set_max(struct nsim_fib_data *fib_data,
+		     enum nsim_resource_id res_id, u64 val,
 		     struct netlink_ext_ack *extack);
 
 #if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
-- 
2.17.2

Re: [patch net-next rfc 02/15] netdevsim: create devlink instance per netdevsim instance

[David Ahern]

On 4/13/19 10:20 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Currently there is one devlink instance created per network namespace.
> That is quite odd considering the fact that devlink instance should
> represent an ASIC. The following patches are going to move the devlink
> instance even more down to a bus device, but until then, have one
> devlink instance per netdevsim instance. Struct nsim_devlink is
> introduced to hold fib setting.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  drivers/net/netdevsim/devlink.c   | 179 +++++++++++-------------------
>  drivers/net/netdevsim/fib.c       | 103 +++++++----------
>  drivers/net/netdevsim/netdev.c    |  27 ++---
>  drivers/net/netdevsim/netdevsim.h |  18 +--
>  4 files changed, 125 insertions(+), 202 deletions(-)

you are making a lot of changes to how fib events are handled and not
explaining what you are doing.

Re: [patch net-next rfc 02/15] netdevsim: create devlink instance per netdevsim instance

[Jiri Pirko]

Mon, Apr 15, 2019 at 04:24:24AM CEST, dsahern@gmail.com wrote:
>On 4/13/19 10:20 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Currently there is one devlink instance created per network namespace.
>> That is quite odd considering the fact that devlink instance should
>> represent an ASIC. The following patches are going to move the devlink
>> instance even more down to a bus device, but until then, have one
>> devlink instance per netdevsim instance. Struct nsim_devlink is
>> introduced to hold fib setting.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>  drivers/net/netdevsim/devlink.c   | 179 +++++++++++-------------------
>>  drivers/net/netdevsim/fib.c       | 103 +++++++----------
>>  drivers/net/netdevsim/netdev.c    |  27 ++---
>>  drivers/net/netdevsim/netdevsim.h |  18 +--
>>  4 files changed, 125 insertions(+), 202 deletions(-)
>
>you are making a lot of changes to how fib events are handled and not
>explaining what you are doing.

They are handled in the exact same way. This is just about devlink
instance. Will add that to description.

Re: [patch net-next rfc 02/15] netdevsim: create devlink instance per netdevsim instance

[David Ahern]

On 4/14/19 11:41 PM, Jiri Pirko wrote:
> Mon, Apr 15, 2019 at 04:24:24AM CEST, dsahern@gmail.com wrote:
>> On 4/13/19 10:20 AM, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>>
>>> Currently there is one devlink instance created per network namespace.
>>> That is quite odd considering the fact that devlink instance should
>>> represent an ASIC. The following patches are going to move the devlink
>>> instance even more down to a bus device, but until then, have one
>>> devlink instance per netdevsim instance. Struct nsim_devlink is
>>> introduced to hold fib setting.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>> ---
>>>  drivers/net/netdevsim/devlink.c   | 179 +++++++++++-------------------
>>>  drivers/net/netdevsim/fib.c       | 103 +++++++----------
>>>  drivers/net/netdevsim/netdev.c    |  27 ++---
>>>  drivers/net/netdevsim/netdevsim.h |  18 +--
>>>  4 files changed, 125 insertions(+), 202 deletions(-)
>>
>> you are making a lot of changes to how fib events are handled and not
>> explaining what you are doing.
> 
> They are handled in the exact same way. This is just about devlink
> instance. Will add that to description.
> 


Data is currently stored as a net_generic. You are changing that. The
changelog should state that.

Re: [patch net-next rfc 02/15] netdevsim: create devlink instance per netdevsim instance

[Jiri Pirko]

Mon, Apr 15, 2019 at 05:07:04PM CEST, dsahern@gmail.com wrote:
>On 4/14/19 11:41 PM, Jiri Pirko wrote:
>> Mon, Apr 15, 2019 at 04:24:24AM CEST, dsahern@gmail.com wrote:
>>> On 4/13/19 10:20 AM, Jiri Pirko wrote:
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> Currently there is one devlink instance created per network namespace.
>>>> That is quite odd considering the fact that devlink instance should
>>>> represent an ASIC. The following patches are going to move the devlink
>>>> instance even more down to a bus device, but until then, have one
>>>> devlink instance per netdevsim instance. Struct nsim_devlink is
>>>> introduced to hold fib setting.
>>>>
>>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>>> ---
>>>>  drivers/net/netdevsim/devlink.c   | 179 +++++++++++-------------------
>>>>  drivers/net/netdevsim/fib.c       | 103 +++++++----------
>>>>  drivers/net/netdevsim/netdev.c    |  27 ++---
>>>>  drivers/net/netdevsim/netdevsim.h |  18 +--
>>>>  4 files changed, 125 insertions(+), 202 deletions(-)
>>>
>>> you are making a lot of changes to how fib events are handled and not
>>> explaining what you are doing.
>> 
>> They are handled in the exact same way. This is just about devlink
>> instance. Will add that to description.
>> 
>
>
>Data is currently stored as a net_generic. You are changing that. The
>changelog should state that.

Added that into description. Thanks.

[patch net-next rfc 03/15] netdevsim: rename devlink.c to dev.c to contain per-dev(asic) items

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

The existing devlink.c code is going to be extended to represent asic
device on a bus. As this is about more than just devlink,
rename the file. Do appropriate prefix renaming alongside with that.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/Makefile             |  2 +-
 drivers/net/netdevsim/{devlink.c => dev.c} | 99 +++++++++++-----------
 drivers/net/netdevsim/netdev.c             | 10 +--
 drivers/net/netdevsim/netdevsim.h          |  4 +-
 4 files changed, 56 insertions(+), 59 deletions(-)
 rename drivers/net/netdevsim/{devlink.c => dev.c} (66%)

diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index cdf8611d2811..d9486e05f4ea 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -3,7 +3,7 @@
 obj-$(CONFIG_NETDEVSIM) += netdevsim.o
 
 netdevsim-objs := \
-	netdev.o devlink.o fib.o sdev.o \
+	netdev.o dev.o fib.o sdev.o \
 
 ifeq ($(CONFIG_BPF_SYSCALL),y)
 netdevsim-objs += \
diff --git a/drivers/net/netdevsim/devlink.c b/drivers/net/netdevsim/dev.c
similarity index 66%
rename from drivers/net/netdevsim/devlink.c
rename to drivers/net/netdevsim/dev.c
index f718912fa52d..c87f0abfaff7 100644
--- a/drivers/net/netdevsim/devlink.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2018 Cumulus Networks. All rights reserved.
  * Copyright (c) 2018 David Ahern <dsa@cumulusnetworks.com>
+ * Copyright (c) 2019 Mellanox Technologies. All rights reserved.
  *
  * This software is licensed under the GNU General License Version 2,
  * June 1991 as shown in the file COPYING in the top-level directory of this
@@ -20,49 +21,45 @@
 
 #include "netdevsim.h"
 
-struct nsim_devlink {
+struct nsim_dev {
 	struct nsim_fib_data *fib_data;
 };
 
-/* IPv4
- */
-static u64 nsim_ipv4_fib_resource_occ_get(void *priv)
+static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
 {
-	struct nsim_devlink *nsim_devlink = priv;
+	struct nsim_dev *nsim_dev = priv;
 
-	return nsim_fib_get_val(nsim_devlink->fib_data,
+	return nsim_fib_get_val(nsim_dev->fib_data,
 				NSIM_RESOURCE_IPV4_FIB, false);
 }
 
-static u64 nsim_ipv4_fib_rules_res_occ_get(void *priv)
+static u64 nsim_dev_ipv4_fib_rules_res_occ_get(void *priv)
 {
-	struct nsim_devlink *nsim_devlink = priv;
+	struct nsim_dev *nsim_dev = priv;
 
-	return nsim_fib_get_val(nsim_devlink->fib_data,
+	return nsim_fib_get_val(nsim_dev->fib_data,
 				NSIM_RESOURCE_IPV4_FIB_RULES, false);
 }
 
-/* IPv6
- */
-static u64 nsim_ipv6_fib_resource_occ_get(void *priv)
+static u64 nsim_dev_ipv6_fib_resource_occ_get(void *priv)
 {
-	struct nsim_devlink *nsim_devlink = priv;
+	struct nsim_dev *nsim_dev = priv;
 
-	return nsim_fib_get_val(nsim_devlink->fib_data,
+	return nsim_fib_get_val(nsim_dev->fib_data,
 				NSIM_RESOURCE_IPV6_FIB, false);
 }
 
-static u64 nsim_ipv6_fib_rules_res_occ_get(void *priv)
+static u64 nsim_dev_ipv6_fib_rules_res_occ_get(void *priv)
 {
-	struct nsim_devlink *nsim_devlink = priv;
+	struct nsim_dev *nsim_dev = priv;
 
-	return nsim_fib_get_val(nsim_devlink->fib_data,
+	return nsim_fib_get_val(nsim_dev->fib_data,
 				NSIM_RESOURCE_IPV6_FIB_RULES, false);
 }
 
-static int devlink_resources_register(struct devlink *devlink)
+static int nsim_dev_resources_register(struct devlink *devlink)
 {
-	struct nsim_devlink *nsim_devlink = devlink_priv(devlink);
+	struct nsim_dev *nsim_dev = devlink_priv(devlink);
 	struct devlink_resource_size_params params = {
 		.size_max = (u64)-1,
 		.size_granularity = 1,
@@ -81,7 +78,7 @@ static int devlink_resources_register(struct devlink *devlink)
 		goto out;
 	}
 
-	n = nsim_fib_get_val(nsim_devlink->fib_data,
+	n = nsim_fib_get_val(nsim_dev->fib_data,
 			     NSIM_RESOURCE_IPV4_FIB, true);
 	err = devlink_resource_register(devlink, "fib", n,
 					NSIM_RESOURCE_IPV4_FIB,
@@ -91,7 +88,7 @@ static int devlink_resources_register(struct devlink *devlink)
 		return err;
 	}
 
-	n = nsim_fib_get_val(nsim_devlink->fib_data,
+	n = nsim_fib_get_val(nsim_dev->fib_data,
 			     NSIM_RESOURCE_IPV4_FIB_RULES, true);
 	err = devlink_resource_register(devlink, "fib-rules", n,
 					NSIM_RESOURCE_IPV4_FIB_RULES,
@@ -111,7 +108,7 @@ static int devlink_resources_register(struct devlink *devlink)
 		goto out;
 	}
 
-	n = nsim_fib_get_val(nsim_devlink->fib_data,
+	n = nsim_fib_get_val(nsim_dev->fib_data,
 			     NSIM_RESOURCE_IPV6_FIB, true);
 	err = devlink_resource_register(devlink, "fib", n,
 					NSIM_RESOURCE_IPV6_FIB,
@@ -121,7 +118,7 @@ static int devlink_resources_register(struct devlink *devlink)
 		return err;
 	}
 
-	n = nsim_fib_get_val(nsim_devlink->fib_data,
+	n = nsim_fib_get_val(nsim_dev->fib_data,
 			     NSIM_RESOURCE_IPV6_FIB_RULES, true);
 	err = devlink_resource_register(devlink, "fib-rules", n,
 					NSIM_RESOURCE_IPV6_FIB_RULES,
@@ -133,28 +130,28 @@ static int devlink_resources_register(struct devlink *devlink)
 
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV4_FIB,
-					  nsim_ipv4_fib_resource_occ_get,
-					  nsim_devlink);
+					  nsim_dev_ipv4_fib_resource_occ_get,
+					  nsim_dev);
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV4_FIB_RULES,
-					  nsim_ipv4_fib_rules_res_occ_get,
-					  nsim_devlink);
+					  nsim_dev_ipv4_fib_rules_res_occ_get,
+					  nsim_dev);
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV6_FIB,
-					  nsim_ipv6_fib_resource_occ_get,
-					  nsim_devlink);
+					  nsim_dev_ipv6_fib_resource_occ_get,
+					  nsim_dev);
 	devlink_resource_occ_get_register(devlink,
 					  NSIM_RESOURCE_IPV6_FIB_RULES,
-					  nsim_ipv6_fib_rules_res_occ_get,
-					  nsim_devlink);
+					  nsim_dev_ipv6_fib_rules_res_occ_get,
+					  nsim_dev);
 out:
 	return err;
 }
 
-static int nsim_devlink_reload(struct devlink *devlink,
-			       struct netlink_ext_ack *extack)
+static int nsim_dev_reload(struct devlink *devlink,
+			   struct netlink_ext_ack *extack)
 {
-	struct nsim_devlink *nsim_devlink = devlink_priv(devlink);
+	struct nsim_dev *nsim_dev = devlink_priv(devlink);
 	enum nsim_resource_id res_ids[] = {
 		NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
 		NSIM_RESOURCE_IPV6_FIB, NSIM_RESOURCE_IPV6_FIB_RULES
@@ -167,7 +164,7 @@ static int nsim_devlink_reload(struct devlink *devlink,
 
 		err = devlink_resource_size_get(devlink, res_ids[i], &val);
 		if (!err) {
-			err = nsim_fib_set_max(nsim_devlink->fib_data,
+			err = nsim_fib_set_max(nsim_dev->fib_data,
 					       res_ids[i], val, extack);
 			if (err)
 				return err;
@@ -177,28 +174,28 @@ static int nsim_devlink_reload(struct devlink *devlink,
 	return 0;
 }
 
-static const struct devlink_ops nsim_devlink_ops = {
-	.reload = nsim_devlink_reload,
+static const struct devlink_ops nsim_dev_devlink_ops = {
+	.reload = nsim_dev_reload,
 };
 
-static int __nsim_devlink_init(struct netdevsim *ns)
+static int __nsim_dev_init(struct netdevsim *ns)
 {
-	struct nsim_devlink *nsim_devlink;
+	struct nsim_dev *nsim_dev;
 	struct devlink *devlink;
 	int err;
 
-	devlink = devlink_alloc(&nsim_devlink_ops, sizeof(*nsim_devlink));
+	devlink = devlink_alloc(&nsim_dev_devlink_ops, sizeof(*nsim_dev));
 	if (!devlink)
 		return -ENOMEM;
-	nsim_devlink = devlink_priv(devlink);
+	nsim_dev = devlink_priv(devlink);
 
-	nsim_devlink->fib_data = nsim_fib_create();
-	if (IS_ERR(nsim_devlink->fib_data)) {
-		err = PTR_ERR(nsim_devlink->fib_data);
+	nsim_dev->fib_data = nsim_fib_create();
+	if (IS_ERR(nsim_dev->fib_data)) {
+		err = PTR_ERR(nsim_dev->fib_data);
 		goto err_devlink_free;
 	}
 
-	err = devlink_resources_register(devlink);
+	err = nsim_dev_resources_register(devlink);
 	if (err)
 		goto err_fib_destroy;
 
@@ -213,32 +210,32 @@ static int __nsim_devlink_init(struct netdevsim *ns)
 err_resources_unregister:
 	devlink_resources_unregister(devlink, NULL);
 err_fib_destroy:
-	nsim_fib_destroy(nsim_devlink->fib_data);
+	nsim_fib_destroy(nsim_dev->fib_data);
 err_devlink_free:
 	devlink_free(devlink);
 
 	return err;
 }
 
-int nsim_devlink_init(struct netdevsim *ns)
+int nsim_dev_init(struct netdevsim *ns)
 {
 	int err;
 
 	dev_hold(ns->netdev);
 	rtnl_unlock();
-	err = __nsim_devlink_init(ns);
+	err = __nsim_dev_init(ns);
 	rtnl_lock();
 	dev_put(ns->netdev);
 	return err;
 }
 
-void nsim_devlink_exit(struct netdevsim *ns)
+void nsim_dev_exit(struct netdevsim *ns)
 {
 	struct devlink *devlink = ns->devlink;
-	struct nsim_devlink *nsim_devlink = devlink_priv(devlink);
+	struct nsim_dev *nsim_dev = devlink_priv(devlink);
 
 	devlink_unregister(devlink);
 	devlink_resources_unregister(devlink, NULL);
-	nsim_fib_destroy(nsim_devlink->fib_data);
+	nsim_fib_destroy(nsim_dev->fib_data);
 	devlink_free(devlink);
 }
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 04aa084dc34c..31fc6564d181 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -195,7 +195,7 @@ static void nsim_free(struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
 
-	nsim_devlink_exit(ns);
+	nsim_dev_exit(ns);
 	device_unregister(&ns->dev);
 	/* netdev and vf state will be freed out of device_release() */
 	nsim_sdev_put(ns->sdev);
@@ -506,17 +506,17 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 	SET_NETDEV_DEV(dev, &ns->dev);
 	ns->netdev = dev;
 
-	err = nsim_devlink_init(ns);
+	err = nsim_dev_init(ns);
 	if (err)
 		goto err_unreg_dev;
 
 	err = register_netdevice(dev);
 	if (err)
-		goto err_devlink_exit;
+		goto err_dev_exit;
 	return 0;
 
-err_devlink_exit:
-	nsim_devlink_exit(ns);
+err_dev_exit:
+	nsim_dev_exit(ns);
 err_unreg_dev:
 	device_unregister(&ns->dev);
 err_sdev_put:
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index df50eb19715d..23d19b461873 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -154,8 +154,8 @@ enum nsim_resource_id {
 	NSIM_RESOURCE_IPV6_FIB_RULES,
 };
 
-int nsim_devlink_init(struct netdevsim *ns);
-void nsim_devlink_exit(struct netdevsim *ns);
+int nsim_dev_init(struct netdevsim *ns);
+void nsim_dev_exit(struct netdevsim *ns);
 
 struct nsim_fib_data *nsim_fib_create(void);
 void nsim_fib_destroy(struct nsim_fib_data *fib_data);
-- 
2.17.2

[patch net-next rfc 04/15] netdevsim: put netdevsim bus code into separate file

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

As the code related to netdevsim bus is going to get bigger, move the
existing code to a separate file.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/Makefile    |  2 +-
 drivers/net/netdevsim/bus.c       | 24 ++++++++++++++++++++++++
 drivers/net/netdevsim/netdev.c    | 18 ++++++------------
 drivers/net/netdevsim/netdevsim.h |  7 +++++++
 4 files changed, 38 insertions(+), 13 deletions(-)
 create mode 100644 drivers/net/netdevsim/bus.c

diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index d9486e05f4ea..eea6162f3886 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -3,7 +3,7 @@
 obj-$(CONFIG_NETDEVSIM) += netdevsim.o
 
 netdevsim-objs := \
-	netdev.o dev.o fib.o sdev.o \
+	netdev.o dev.o fib.o sdev.o bus.o \
 
 ifeq ($(CONFIG_BPF_SYSCALL),y)
 netdevsim-objs += \
diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
new file mode 100644
index 000000000000..26b866b72afc
--- /dev/null
+++ b/drivers/net/netdevsim/bus.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2017 Netronome Systems, Inc.
+ * Copyright (C) 2019 Mellanox Technologies. All rights reserved
+ */
+
+#include <linux/device.h>
+
+#include "netdevsim.h"
+
+struct bus_type nsim_bus = {
+	.name		= DRV_NAME,
+	.dev_name	= DRV_NAME,
+	.num_vf		= nsim_num_vf,
+};
+
+int nsim_bus_init(void)
+{
+	return bus_register(&nsim_bus);
+}
+
+void nsim_bus_exit(void)
+{
+	bus_unregister(&nsim_bus);
+}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 31fc6564d181..7bc0da8cc10d 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -42,19 +42,13 @@ struct nsim_vf_config {
 
 static struct dentry *nsim_ddir;
 
-static int nsim_num_vf(struct device *dev)
+int nsim_num_vf(struct device *dev)
 {
 	struct netdevsim *ns = to_nsim(dev);
 
 	return ns->num_vfs;
 }
 
-static struct bus_type nsim_bus = {
-	.name		= DRV_NAME,
-	.dev_name	= DRV_NAME,
-	.num_vf		= nsim_num_vf,
-};
-
 static int nsim_vfs_enable(struct netdevsim *ns, unsigned int num_vfs)
 {
 	ns->vfconfigs = kcalloc(num_vfs, sizeof(struct nsim_vf_config),
@@ -544,18 +538,18 @@ static int __init nsim_module_init(void)
 	if (err)
 		goto err_debugfs_destroy;
 
-	err = bus_register(&nsim_bus);
+	err = nsim_bus_init();
 	if (err)
 		goto err_sdev_exit;
 
 	err = rtnl_link_register(&nsim_link_ops);
 	if (err)
-		goto err_unreg_bus;
+		goto err_bus_exit;
 
 	return 0;
 
-err_unreg_bus:
-	bus_unregister(&nsim_bus);
+err_bus_exit:
+	nsim_bus_exit();
 err_sdev_exit:
 	nsim_sdev_exit();
 err_debugfs_destroy:
@@ -566,7 +560,7 @@ static int __init nsim_module_init(void)
 static void __exit nsim_module_exit(void)
 {
 	rtnl_link_unregister(&nsim_link_ops);
-	bus_unregister(&nsim_bus);
+	nsim_bus_exit();
 	nsim_sdev_exit();
 	debugfs_remove_recursive(nsim_ddir);
 }
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 23d19b461873..7a144aa7965a 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -188,3 +188,10 @@ static inline struct netdevsim *to_nsim(struct device *ptr)
 {
 	return container_of(ptr, struct netdevsim, dev);
 }
+
+int nsim_num_vf(struct device *dev);
+
+extern struct bus_type nsim_bus;
+
+int nsim_bus_init(void);
+void nsim_bus_exit(void);
-- 
2.17.2

Re: [patch net-next rfc 04/15] netdevsim: put netdevsim bus code into separate file

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Sat, 13 Apr 2019 18:21:01 +0200

> @@ -3,7 +3,7 @@
>  obj-$(CONFIG_NETDEVSIM) += netdevsim.o
>  
>  netdevsim-objs := \
> -	netdev.o dev.o fib.o sdev.o \
> +	netdev.o dev.o fib.o sdev.o bus.o \
>  

I know this is unrelated to what you are trying to do, but could you please
also get rid of that trailing backslash?

Thank you.

Re: [patch net-next rfc 04/15] netdevsim: put netdevsim bus code into separate file

[Jiri Pirko]

Sun, Apr 14, 2019 at 10:27:56PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Sat, 13 Apr 2019 18:21:01 +0200
>
>> @@ -3,7 +3,7 @@
>>  obj-$(CONFIG_NETDEVSIM) += netdevsim.o
>>  
>>  netdevsim-objs := \
>> -	netdev.o dev.o fib.o sdev.o \
>> +	netdev.o dev.o fib.o sdev.o bus.o \
>>  
>
>I know this is unrelated to what you are trying to do, but could you please
>also get rid of that trailing backslash?

ok

>
>Thank you.

[patch net-next rfc 05/15] netdevsim: move device registration and related code to bus.c

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Move netdevsim device registration into bus.c and alongside with that
the related sysfs attributes. Introduce new struct nsim_bus_dev to
represent a netdevsim device on netdevsim bus.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/bus.c       | 142 +++++++++++++++++++++-
 drivers/net/netdevsim/dev.c       |   2 +-
 drivers/net/netdevsim/netdev.c    | 195 +++++++-----------------------
 drivers/net/netdevsim/netdevsim.h |  31 +++--
 4 files changed, 207 insertions(+), 163 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 26b866b72afc..7e83a2e856d3 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -4,15 +4,155 @@
  */
 
 #include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/rtnetlink.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
 
 #include "netdevsim.h"
 
-struct bus_type nsim_bus = {
+static u32 nsim_bus_dev_id;
+
+static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
+{
+	return container_of(dev, struct nsim_bus_dev, dev);
+}
+
+static int nsim_bus_dev_vfs_enable(struct nsim_bus_dev *nsim_bus_dev,
+				   unsigned int num_vfs)
+{
+	nsim_bus_dev->vfconfigs = kcalloc(num_vfs,
+					  sizeof(struct nsim_vf_config),
+					  GFP_KERNEL);
+	if (!nsim_bus_dev->vfconfigs)
+		return -ENOMEM;
+	nsim_bus_dev->num_vfs = num_vfs;
+
+	return 0;
+}
+
+static void nsim_bus_dev_vfs_disable(struct nsim_bus_dev *nsim_bus_dev)
+{
+	kfree(nsim_bus_dev->vfconfigs);
+	nsim_bus_dev->vfconfigs = NULL;
+	nsim_bus_dev->num_vfs = 0;
+}
+
+static ssize_t
+nsim_bus_dev_numvfs_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
+	unsigned int num_vfs;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &num_vfs);
+	if (ret)
+		return ret;
+
+	rtnl_lock();
+	if (nsim_bus_dev->num_vfs == num_vfs)
+		goto exit_good;
+	if (nsim_bus_dev->num_vfs && num_vfs) {
+		ret = -EBUSY;
+		goto exit_unlock;
+	}
+
+	if (num_vfs) {
+		ret = nsim_bus_dev_vfs_enable(nsim_bus_dev, num_vfs);
+		if (ret)
+			goto exit_unlock;
+	} else {
+		nsim_bus_dev_vfs_disable(nsim_bus_dev);
+	}
+exit_good:
+	ret = count;
+exit_unlock:
+	rtnl_unlock();
+
+	return ret;
+}
+
+static ssize_t
+nsim_bus_dev_numvfs_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
+
+	return sprintf(buf, "%u\n", nsim_bus_dev->num_vfs);
+}
+
+static struct device_attribute nsim_bus_dev_numvfs_attr =
+	__ATTR(sriov_numvfs, 0664, nsim_bus_dev_numvfs_show,
+	       nsim_bus_dev_numvfs_store);
+
+static struct attribute *nsim_bus_dev_attrs[] = {
+	&nsim_bus_dev_numvfs_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group nsim_bus_dev_attr_group = {
+	.attrs = nsim_bus_dev_attrs,
+};
+
+static const struct attribute_group *nsim_bus_dev_attr_groups[] = {
+	&nsim_bus_dev_attr_group,
+	NULL,
+};
+
+static void nsim_bus_dev_release(struct device *dev)
+{
+	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
+
+	nsim_bus_dev_vfs_disable(nsim_bus_dev);
+}
+
+static struct device_type nsim_bus_dev_type = {
+	.groups = nsim_bus_dev_attr_groups,
+	.release = nsim_bus_dev_release,
+};
+
+int nsim_num_vf(struct device *dev)
+{
+	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
+
+	return nsim_bus_dev->num_vfs;
+}
+
+static struct bus_type nsim_bus = {
 	.name		= DRV_NAME,
 	.dev_name	= DRV_NAME,
 	.num_vf		= nsim_num_vf,
 };
 
+struct nsim_bus_dev *nsim_bus_dev_new(void)
+{
+	struct nsim_bus_dev *nsim_bus_dev;
+	int err;
+
+	nsim_bus_dev = kzalloc(sizeof(*nsim_bus_dev), GFP_KERNEL);
+	if (!nsim_bus_dev)
+		return ERR_PTR(-ENOMEM);
+
+	nsim_bus_dev->dev.id = nsim_bus_dev_id++;
+	nsim_bus_dev->dev.bus = &nsim_bus;
+	nsim_bus_dev->dev.type = &nsim_bus_dev_type;
+	err = device_register(&nsim_bus_dev->dev);
+	if (err)
+		goto err_nsim_bus_dev_free;
+	return nsim_bus_dev;
+
+err_nsim_bus_dev_free:
+	kfree(nsim_bus_dev);
+	return ERR_PTR(err);
+}
+
+void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
+{
+	device_unregister(&nsim_bus_dev->dev);
+	kfree(nsim_bus_dev);
+}
+
 int nsim_bus_init(void)
 {
 	return bus_register(&nsim_bus);
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index c87f0abfaff7..c83c08edf94f 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -199,7 +199,7 @@ static int __nsim_dev_init(struct netdevsim *ns)
 	if (err)
 		goto err_fib_destroy;
 
-	err = devlink_register(devlink, &ns->dev);
+	err = devlink_register(devlink, &ns->nsim_bus_dev->dev);
 	if (err)
 		goto err_resources_unregister;
 
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 7bc0da8cc10d..3407b009929e 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -25,120 +25,8 @@
 
 #include "netdevsim.h"
 
-static u32 nsim_dev_id;
-
-struct nsim_vf_config {
-	int link_state;
-	u16 min_tx_rate;
-	u16 max_tx_rate;
-	u16 vlan;
-	__be16 vlan_proto;
-	u16 qos;
-	u8 vf_mac[ETH_ALEN];
-	bool spoofchk_enabled;
-	bool trusted;
-	bool rss_query_enabled;
-};
-
 static struct dentry *nsim_ddir;
 
-int nsim_num_vf(struct device *dev)
-{
-	struct netdevsim *ns = to_nsim(dev);
-
-	return ns->num_vfs;
-}
-
-static int nsim_vfs_enable(struct netdevsim *ns, unsigned int num_vfs)
-{
-	ns->vfconfigs = kcalloc(num_vfs, sizeof(struct nsim_vf_config),
-				GFP_KERNEL);
-	if (!ns->vfconfigs)
-		return -ENOMEM;
-	ns->num_vfs = num_vfs;
-
-	return 0;
-}
-
-static void nsim_vfs_disable(struct netdevsim *ns)
-{
-	kfree(ns->vfconfigs);
-	ns->vfconfigs = NULL;
-	ns->num_vfs = 0;
-}
-
-static ssize_t
-nsim_numvfs_store(struct device *dev, struct device_attribute *attr,
-		  const char *buf, size_t count)
-{
-	struct netdevsim *ns = to_nsim(dev);
-	unsigned int num_vfs;
-	int ret;
-
-	ret = kstrtouint(buf, 0, &num_vfs);
-	if (ret)
-		return ret;
-
-	rtnl_lock();
-	if (ns->num_vfs == num_vfs)
-		goto exit_good;
-	if (ns->num_vfs && num_vfs) {
-		ret = -EBUSY;
-		goto exit_unlock;
-	}
-
-	if (num_vfs) {
-		ret = nsim_vfs_enable(ns, num_vfs);
-		if (ret)
-			goto exit_unlock;
-	} else {
-		nsim_vfs_disable(ns);
-	}
-exit_good:
-	ret = count;
-exit_unlock:
-	rtnl_unlock();
-
-	return ret;
-}
-
-static ssize_t
-nsim_numvfs_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct netdevsim *ns = to_nsim(dev);
-
-	return sprintf(buf, "%u\n", ns->num_vfs);
-}
-
-static struct device_attribute nsim_numvfs_attr =
-	__ATTR(sriov_numvfs, 0664, nsim_numvfs_show, nsim_numvfs_store);
-
-static struct attribute *nsim_dev_attrs[] = {
-	&nsim_numvfs_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group nsim_dev_attr_group = {
-	.attrs = nsim_dev_attrs,
-};
-
-static const struct attribute_group *nsim_dev_attr_groups[] = {
-	&nsim_dev_attr_group,
-	NULL,
-};
-
-static void nsim_dev_release(struct device *dev)
-{
-	struct netdevsim *ns = to_nsim(dev);
-
-	nsim_vfs_disable(ns);
-}
-
-static struct device_type nsim_dev_type = {
-	.groups = nsim_dev_attr_groups,
-	.release = nsim_dev_release,
-};
-
 static int nsim_get_port_parent_id(struct net_device *dev,
 				   struct netdev_phys_item_id *ppid)
 {
@@ -190,7 +78,7 @@ static void nsim_free(struct net_device *dev)
 	struct netdevsim *ns = netdev_priv(dev);
 
 	nsim_dev_exit(ns);
-	device_unregister(&ns->dev);
+	nsim_bus_dev_del(ns->nsim_bus_dev);
 	/* netdev and vf state will be freed out of device_release() */
 	nsim_sdev_put(ns->sdev);
 }
@@ -271,11 +159,12 @@ nsim_setup_tc_block(struct net_device *dev, struct tc_block_offload *f)
 static int nsim_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
 	/* Only refuse multicast addresses, zero address can mean unset/any. */
-	if (vf >= ns->num_vfs || is_multicast_ether_addr(mac))
+	if (vf >= nsim_bus_dev->num_vfs || is_multicast_ether_addr(mac))
 		return -EINVAL;
-	memcpy(ns->vfconfigs[vf].vf_mac, mac, ETH_ALEN);
+	memcpy(nsim_bus_dev->vfconfigs[vf].vf_mac, mac, ETH_ALEN);
 
 	return 0;
 }
@@ -284,13 +173,14 @@ static int nsim_set_vf_vlan(struct net_device *dev, int vf,
 			    u16 vlan, u8 qos, __be16 vlan_proto)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
-	if (vf >= ns->num_vfs || vlan > 4095 || qos > 7)
+	if (vf >= nsim_bus_dev->num_vfs || vlan > 4095 || qos > 7)
 		return -EINVAL;
 
-	ns->vfconfigs[vf].vlan = vlan;
-	ns->vfconfigs[vf].qos = qos;
-	ns->vfconfigs[vf].vlan_proto = vlan_proto;
+	nsim_bus_dev->vfconfigs[vf].vlan = vlan;
+	nsim_bus_dev->vfconfigs[vf].qos = qos;
+	nsim_bus_dev->vfconfigs[vf].vlan_proto = vlan_proto;
 
 	return 0;
 }
@@ -298,12 +188,13 @@ static int nsim_set_vf_vlan(struct net_device *dev, int vf,
 static int nsim_set_vf_rate(struct net_device *dev, int vf, int min, int max)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
-	if (vf >= ns->num_vfs)
+	if (vf >= nsim_bus_dev->num_vfs)
 		return -EINVAL;
 
-	ns->vfconfigs[vf].min_tx_rate = min;
-	ns->vfconfigs[vf].max_tx_rate = max;
+	nsim_bus_dev->vfconfigs[vf].min_tx_rate = min;
+	nsim_bus_dev->vfconfigs[vf].max_tx_rate = max;
 
 	return 0;
 }
@@ -311,10 +202,11 @@ static int nsim_set_vf_rate(struct net_device *dev, int vf, int min, int max)
 static int nsim_set_vf_spoofchk(struct net_device *dev, int vf, bool val)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
-	if (vf >= ns->num_vfs)
+	if (vf >= nsim_bus_dev->num_vfs)
 		return -EINVAL;
-	ns->vfconfigs[vf].spoofchk_enabled = val;
+	nsim_bus_dev->vfconfigs[vf].spoofchk_enabled = val;
 
 	return 0;
 }
@@ -322,10 +214,11 @@ static int nsim_set_vf_spoofchk(struct net_device *dev, int vf, bool val)
 static int nsim_set_vf_rss_query_en(struct net_device *dev, int vf, bool val)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
-	if (vf >= ns->num_vfs)
+	if (vf >= nsim_bus_dev->num_vfs)
 		return -EINVAL;
-	ns->vfconfigs[vf].rss_query_enabled = val;
+	nsim_bus_dev->vfconfigs[vf].rss_query_enabled = val;
 
 	return 0;
 }
@@ -333,10 +226,11 @@ static int nsim_set_vf_rss_query_en(struct net_device *dev, int vf, bool val)
 static int nsim_set_vf_trust(struct net_device *dev, int vf, bool val)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
-	if (vf >= ns->num_vfs)
+	if (vf >= nsim_bus_dev->num_vfs)
 		return -EINVAL;
-	ns->vfconfigs[vf].trusted = val;
+	nsim_bus_dev->vfconfigs[vf].trusted = val;
 
 	return 0;
 }
@@ -345,21 +239,22 @@ static int
 nsim_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
-	if (vf >= ns->num_vfs)
+	if (vf >= nsim_bus_dev->num_vfs)
 		return -EINVAL;
 
 	ivi->vf = vf;
-	ivi->linkstate = ns->vfconfigs[vf].link_state;
-	ivi->min_tx_rate = ns->vfconfigs[vf].min_tx_rate;
-	ivi->max_tx_rate = ns->vfconfigs[vf].max_tx_rate;
-	ivi->vlan = ns->vfconfigs[vf].vlan;
-	ivi->vlan_proto = ns->vfconfigs[vf].vlan_proto;
-	ivi->qos = ns->vfconfigs[vf].qos;
-	memcpy(&ivi->mac, ns->vfconfigs[vf].vf_mac, ETH_ALEN);
-	ivi->spoofchk = ns->vfconfigs[vf].spoofchk_enabled;
-	ivi->trusted = ns->vfconfigs[vf].trusted;
-	ivi->rss_query_en = ns->vfconfigs[vf].rss_query_enabled;
+	ivi->linkstate = nsim_bus_dev->vfconfigs[vf].link_state;
+	ivi->min_tx_rate = nsim_bus_dev->vfconfigs[vf].min_tx_rate;
+	ivi->max_tx_rate = nsim_bus_dev->vfconfigs[vf].max_tx_rate;
+	ivi->vlan = nsim_bus_dev->vfconfigs[vf].vlan;
+	ivi->vlan_proto = nsim_bus_dev->vfconfigs[vf].vlan_proto;
+	ivi->qos = nsim_bus_dev->vfconfigs[vf].qos;
+	memcpy(&ivi->mac, nsim_bus_dev->vfconfigs[vf].vf_mac, ETH_ALEN);
+	ivi->spoofchk = nsim_bus_dev->vfconfigs[vf].spoofchk_enabled;
+	ivi->trusted = nsim_bus_dev->vfconfigs[vf].trusted;
+	ivi->rss_query_en = nsim_bus_dev->vfconfigs[vf].rss_query_enabled;
 
 	return 0;
 }
@@ -367,8 +262,9 @@ nsim_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi)
 static int nsim_set_vf_link_state(struct net_device *dev, int vf, int state)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
 
-	if (vf >= ns->num_vfs)
+	if (vf >= nsim_bus_dev->num_vfs)
 		return -EINVAL;
 
 	switch (state) {
@@ -380,7 +276,7 @@ static int nsim_set_vf_link_state(struct net_device *dev, int vf, int state)
 		return -EINVAL;
 	}
 
-	ns->vfconfigs[vf].link_state = state;
+	nsim_bus_dev->vfconfigs[vf].link_state = state;
 
 	return 0;
 }
@@ -490,19 +386,18 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 	if (IS_ERR(ns->sdev))
 		return PTR_ERR(ns->sdev);
 
-	ns->dev.id = nsim_dev_id++;
-	ns->dev.bus = &nsim_bus;
-	ns->dev.type = &nsim_dev_type;
-	err = device_register(&ns->dev);
-	if (err)
+	ns->nsim_bus_dev = nsim_bus_dev_new();
+	if (IS_ERR(ns->nsim_bus_dev)) {
+		err = PTR_ERR(ns->nsim_bus_dev);
 		goto err_sdev_put;
+	}
 
-	SET_NETDEV_DEV(dev, &ns->dev);
+	SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev);
 	ns->netdev = dev;
 
 	err = nsim_dev_init(ns);
 	if (err)
-		goto err_unreg_dev;
+		goto err_dev_del;
 
 	err = register_netdevice(dev);
 	if (err)
@@ -511,8 +406,8 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 
 err_dev_exit:
 	nsim_dev_exit(ns);
-err_unreg_dev:
-	device_unregister(&ns->dev);
+err_dev_del:
+	nsim_bus_dev_del(ns->nsim_bus_dev);
 err_sdev_put:
 	nsim_sdev_put(ns->sdev);
 	return err;
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 7a144aa7965a..528d1e7d3e6b 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -85,14 +85,11 @@ struct netdevsim {
 	u64 tx_bytes;
 	struct u64_stats_sync syncp;
 
-	struct device dev;
+	struct nsim_bus_dev *nsim_bus_dev;
 	struct netdevsim_shared_dev *sdev;
 
 	struct dentry *ddir;
 
-	unsigned int num_vfs;
-	struct nsim_vf_config *vfconfigs;
-
 	struct bpf_prog	*bpf_offloaded;
 	u32 bpf_offloaded_id;
 
@@ -184,14 +181,26 @@ static inline bool nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb)
 }
 #endif
 
-static inline struct netdevsim *to_nsim(struct device *ptr)
-{
-	return container_of(ptr, struct netdevsim, dev);
-}
-
-int nsim_num_vf(struct device *dev);
+struct nsim_vf_config {
+	int link_state;
+	u16 min_tx_rate;
+	u16 max_tx_rate;
+	u16 vlan;
+	__be16 vlan_proto;
+	u16 qos;
+	u8 vf_mac[ETH_ALEN];
+	bool spoofchk_enabled;
+	bool trusted;
+	bool rss_query_enabled;
+};
 
-extern struct bus_type nsim_bus;
+struct nsim_bus_dev {
+	struct device dev;
+	unsigned int num_vfs;
+	struct nsim_vf_config *vfconfigs;
+};
 
+struct nsim_bus_dev *nsim_bus_dev_new(void);
+void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev);
 int nsim_bus_init(void);
 void nsim_bus_exit(void);
-- 
2.17.2

[patch net-next rfc 06/15] netdevsim: add stub netdevsim driver implementation

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

In order to bus probing to work correctly, register a simple  netdevsim
driver implementation.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/bus.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 7e83a2e856d3..5b5a9b0831a9 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -153,12 +153,31 @@ void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
 	kfree(nsim_bus_dev);
 }
 
+static struct device_driver nsim_driver = {
+	.name		= DRV_NAME,
+	.bus		= &nsim_bus,
+	.owner		= THIS_MODULE,
+};
+
 int nsim_bus_init(void)
 {
-	return bus_register(&nsim_bus);
+	int err;
+
+	err = bus_register(&nsim_bus);
+	if (err)
+		return err;
+	err = driver_register(&nsim_driver);
+	if (err)
+		goto err_bus_unregister;
+	return 0;
+
+err_bus_unregister:
+	bus_unregister(&nsim_bus);
+	return err;
 }
 
 void nsim_bus_exit(void)
 {
+	driver_unregister(&nsim_driver);
 	bus_unregister(&nsim_bus);
 }
-- 
2.17.2

[patch net-next rfc 07/15] netdevsim: use ida for bus device ids

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Instead of increments of u32 value, use ida to manage bus device ids.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/bus.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 5b5a9b0831a9..c50c5ea90555 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/device.h>
+#include <linux/idr.h>
 #include <linux/kernel.h>
 #include <linux/rtnetlink.h>
 #include <linux/slab.h>
@@ -11,7 +12,7 @@
 
 #include "netdevsim.h"
 
-static u32 nsim_bus_dev_id;
+static DEFINE_IDA(nsim_bus_dev_ids);
 
 static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
 {
@@ -134,14 +135,19 @@ struct nsim_bus_dev *nsim_bus_dev_new(void)
 	if (!nsim_bus_dev)
 		return ERR_PTR(-ENOMEM);
 
-	nsim_bus_dev->dev.id = nsim_bus_dev_id++;
+	err = ida_alloc(&nsim_bus_dev_ids, GFP_KERNEL);
+	if (err < 0)
+		goto err_nsim_bus_dev_free;
+	nsim_bus_dev->dev.id = err;
 	nsim_bus_dev->dev.bus = &nsim_bus;
 	nsim_bus_dev->dev.type = &nsim_bus_dev_type;
 	err = device_register(&nsim_bus_dev->dev);
 	if (err)
-		goto err_nsim_bus_dev_free;
+		goto err_nsim_bus_dev_id_free;
 	return nsim_bus_dev;
 
+err_nsim_bus_dev_id_free:
+	ida_free(&nsim_bus_dev_ids, nsim_bus_dev->dev.id);
 err_nsim_bus_dev_free:
 	kfree(nsim_bus_dev);
 	return ERR_PTR(err);
@@ -150,6 +156,7 @@ struct nsim_bus_dev *nsim_bus_dev_new(void)
 void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
 {
 	device_unregister(&nsim_bus_dev->dev);
+	ida_free(&nsim_bus_dev_ids, nsim_bus_dev->dev.id);
 	kfree(nsim_bus_dev);
 }
 
-- 
2.17.2

[patch net-next rfc 08/15] netdevsim: add bus attributes to add new and delete devices

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Add a way to add new netdevsim device on netdevsim bus and also to
delete existing netdevsim device from the bus. Track the bus devices
in using a list.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/bus.c       | 97 ++++++++++++++++++++++++++++++-
 drivers/net/netdevsim/netdev.c    |  2 +-
 drivers/net/netdevsim/netdevsim.h |  4 +-
 3 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index c50c5ea90555..4471f5db50ee 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -6,6 +6,8 @@
 #include <linux/device.h>
 #include <linux/idr.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
 #include <linux/rtnetlink.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
@@ -13,6 +15,8 @@
 #include "netdevsim.h"
 
 static DEFINE_IDA(nsim_bus_dev_ids);
+static LIST_HEAD(nsim_bus_dev_list);
+static DEFINE_MUTEX(nsim_bus_dev_list_lock);
 
 static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
 {
@@ -113,6 +117,83 @@ static struct device_type nsim_bus_dev_type = {
 	.release = nsim_bus_dev_release,
 };
 
+static ssize_t new_device_store(struct bus_type *bus, const char *buf,
+				size_t count)
+{
+	struct nsim_bus_dev *nsim_bus_dev;
+	unsigned int port_count;
+	unsigned int id;
+	int err;
+
+	err = sscanf(buf, "%u %u", &id, &port_count);
+	switch (err) {
+	case 1:
+		port_count = 1;
+		/* pass through */
+	case 2:
+		if (id > INT_MAX) {
+			pr_err("Value of \"id\" is too big.\n");
+			return -EINVAL;
+		}
+		break;
+	default:
+		pr_err("Format for adding new device is \"id port_count\" (uint uint).\n");
+		return -EINVAL;
+	}
+	nsim_bus_dev = nsim_bus_dev_new(id, port_count);
+	if (IS_ERR(nsim_bus_dev))
+		return PTR_ERR(nsim_bus_dev);
+
+	mutex_lock(&nsim_bus_dev_list_lock);
+	list_add_tail(&nsim_bus_dev->list, &nsim_bus_dev_list);
+	mutex_unlock(&nsim_bus_dev_list_lock);
+
+	return count;
+}
+static BUS_ATTR_WO(new_device);
+
+static ssize_t del_device_store(struct bus_type *bus, const char *buf,
+				size_t count)
+{
+	struct nsim_bus_dev *nsim_bus_dev, *tmp;
+	unsigned int id;
+	int err;
+
+	err = sscanf(buf, "%u", &id);
+	switch (err) {
+	case 1:
+		if (id > INT_MAX) {
+			pr_err("Value of \"id\" is too big.\n");
+			return -EINVAL;
+		}
+		break;
+	default:
+		pr_err("Format for deleting device is \"id\" (uint).\n");
+		return -EINVAL;
+	}
+
+	err = -ENOENT;
+	mutex_lock(&nsim_bus_dev_list_lock);
+	list_for_each_entry_safe(nsim_bus_dev, tmp, &nsim_bus_dev_list, list) {
+		if (nsim_bus_dev->dev.id != id)
+			continue;
+		list_del(&nsim_bus_dev->list);
+		nsim_bus_dev_del(nsim_bus_dev);
+		err = 0;
+		break;
+	}
+	mutex_unlock(&nsim_bus_dev_list_lock);
+	return !err ? count : err;
+}
+static BUS_ATTR_WO(del_device);
+
+static struct attribute *nsim_bus_attrs[] = {
+	&bus_attr_new_device.attr,
+	&bus_attr_del_device.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(nsim_bus);
+
 int nsim_num_vf(struct device *dev)
 {
 	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
@@ -123,10 +204,11 @@ int nsim_num_vf(struct device *dev)
 static struct bus_type nsim_bus = {
 	.name		= DRV_NAME,
 	.dev_name	= DRV_NAME,
+	.bus_groups	= nsim_bus_groups,
 	.num_vf		= nsim_num_vf,
 };
 
-struct nsim_bus_dev *nsim_bus_dev_new(void)
+struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count)
 {
 	struct nsim_bus_dev *nsim_bus_dev;
 	int err;
@@ -135,12 +217,15 @@ struct nsim_bus_dev *nsim_bus_dev_new(void)
 	if (!nsim_bus_dev)
 		return ERR_PTR(-ENOMEM);
 
-	err = ida_alloc(&nsim_bus_dev_ids, GFP_KERNEL);
+	err = ida_alloc_range(&nsim_bus_dev_ids,
+			      id == ~0 ? 0 : id, id, GFP_KERNEL);
 	if (err < 0)
 		goto err_nsim_bus_dev_free;
 	nsim_bus_dev->dev.id = err;
 	nsim_bus_dev->dev.bus = &nsim_bus;
 	nsim_bus_dev->dev.type = &nsim_bus_dev_type;
+	nsim_bus_dev->port_count = port_count;
+
 	err = device_register(&nsim_bus_dev->dev);
 	if (err)
 		goto err_nsim_bus_dev_id_free;
@@ -185,6 +270,14 @@ int nsim_bus_init(void)
 
 void nsim_bus_exit(void)
 {
+	struct nsim_bus_dev *nsim_bus_dev, *tmp;
+
+	mutex_lock(&nsim_bus_dev_list_lock);
+	list_for_each_entry_safe(nsim_bus_dev, tmp, &nsim_bus_dev_list, list) {
+		list_del(&nsim_bus_dev->list);
+		nsim_bus_dev_del(nsim_bus_dev);
+	}
+	mutex_unlock(&nsim_bus_dev_list_lock);
 	driver_unregister(&nsim_driver);
 	bus_unregister(&nsim_bus);
 }
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 3407b009929e..37a442ffcb8b 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -386,7 +386,7 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 	if (IS_ERR(ns->sdev))
 		return PTR_ERR(ns->sdev);
 
-	ns->nsim_bus_dev = nsim_bus_dev_new();
+	ns->nsim_bus_dev = nsim_bus_dev_new(~0, 0);
 	if (IS_ERR(ns->nsim_bus_dev)) {
 		err = PTR_ERR(ns->nsim_bus_dev);
 		goto err_sdev_put;
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 528d1e7d3e6b..8d61fcb55f39 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -196,11 +196,13 @@ struct nsim_vf_config {
 
 struct nsim_bus_dev {
 	struct device dev;
+	struct list_head list;
+	unsigned int port_count;
 	unsigned int num_vfs;
 	struct nsim_vf_config *vfconfigs;
 };
 
-struct nsim_bus_dev *nsim_bus_dev_new(void);
+struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count);
 void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev);
 int nsim_bus_init(void);
 void nsim_bus_exit(void);
-- 
2.17.2

[patch net-next rfc 09/15] netdevsim: rename dev_init/exit() functions and make them independent on ns

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

These functions are going to be called from bus probe/release(),
therefore make them independent on ns struct and rename accordingly.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/dev.c       | 31 ++++++++++++-------------------
 drivers/net/netdevsim/netdev.c    | 14 ++++++++------
 drivers/net/netdevsim/netdevsim.h | 11 ++++++++---
 3 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index c83c08edf94f..d0bec47cf8e5 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -21,10 +21,6 @@
 
 #include "netdevsim.h"
 
-struct nsim_dev {
-	struct nsim_fib_data *fib_data;
-};
-
 static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
 {
 	struct nsim_dev *nsim_dev = priv;
@@ -178,7 +174,7 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
 	.reload = nsim_dev_reload,
 };
 
-static int __nsim_dev_init(struct netdevsim *ns)
+static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev)
 {
 	struct nsim_dev *nsim_dev;
 	struct devlink *devlink;
@@ -186,7 +182,7 @@ static int __nsim_dev_init(struct netdevsim *ns)
 
 	devlink = devlink_alloc(&nsim_dev_devlink_ops, sizeof(*nsim_dev));
 	if (!devlink)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 	nsim_dev = devlink_priv(devlink);
 
 	nsim_dev->fib_data = nsim_fib_create();
@@ -199,13 +195,11 @@ static int __nsim_dev_init(struct netdevsim *ns)
 	if (err)
 		goto err_fib_destroy;
 
-	err = devlink_register(devlink, &ns->nsim_bus_dev->dev);
+	err = devlink_register(devlink, &nsim_bus_dev->dev);
 	if (err)
 		goto err_resources_unregister;
 
-	ns->devlink = devlink;
-
-	return 0;
+	return nsim_dev;
 
 err_resources_unregister:
 	devlink_resources_unregister(devlink, NULL);
@@ -213,26 +207,25 @@ static int __nsim_dev_init(struct netdevsim *ns)
 	nsim_fib_destroy(nsim_dev->fib_data);
 err_devlink_free:
 	devlink_free(devlink);
-
-	return err;
+	return ERR_PTR(err);
 }
 
-int nsim_dev_init(struct netdevsim *ns)
+struct nsim_dev *nsim_dev_create_with_ns(struct nsim_bus_dev *nsim_bus_dev,
+					 struct netdevsim *ns)
 {
-	int err;
+	struct nsim_dev *nsim_dev;
 
 	dev_hold(ns->netdev);
 	rtnl_unlock();
-	err = __nsim_dev_init(ns);
+	nsim_dev = nsim_dev_create(nsim_bus_dev);
 	rtnl_lock();
 	dev_put(ns->netdev);
-	return err;
+	return nsim_dev;
 }
 
-void nsim_dev_exit(struct netdevsim *ns)
+void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 {
-	struct devlink *devlink = ns->devlink;
-	struct nsim_dev *nsim_dev = devlink_priv(devlink);
+	struct devlink *devlink = priv_to_devlink(nsim_dev);
 
 	devlink_unregister(devlink);
 	devlink_resources_unregister(devlink, NULL);
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 37a442ffcb8b..28231bfbc989 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -77,7 +77,7 @@ static void nsim_free(struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
 
-	nsim_dev_exit(ns);
+	nsim_dev_destroy(ns->nsim_dev);
 	nsim_bus_dev_del(ns->nsim_bus_dev);
 	/* netdev and vf state will be freed out of device_release() */
 	nsim_sdev_put(ns->sdev);
@@ -395,17 +395,19 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 	SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev);
 	ns->netdev = dev;
 
-	err = nsim_dev_init(ns);
-	if (err)
+	ns->nsim_dev = nsim_dev_create_with_ns(ns->nsim_bus_dev, ns);
+	if (IS_ERR(ns->nsim_dev)) {
+		err = PTR_ERR(ns->nsim_dev);
 		goto err_dev_del;
+	}
 
 	err = register_netdevice(dev);
 	if (err)
-		goto err_dev_exit;
+		goto err_dev_destroy;
 	return 0;
 
-err_dev_exit:
-	nsim_dev_exit(ns);
+err_dev_destroy:
+	nsim_dev_destroy(ns->nsim_dev);
 err_dev_del:
 	nsim_bus_dev_del(ns->nsim_bus_dev);
 err_sdev_put:
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 8d61fcb55f39..cfcd801cecc2 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -80,6 +80,7 @@ struct nsim_ipsec {
 
 struct netdevsim {
 	struct net_device *netdev;
+	struct nsim_dev *nsim_dev;
 
 	u64 tx_packets;
 	u64 tx_bytes;
@@ -102,7 +103,6 @@ struct netdevsim {
 	bool bpf_xdpoffload_accept;
 
 	bool bpf_map_accept;
-	struct devlink *devlink;
 	struct nsim_ipsec ipsec;
 };
 
@@ -151,8 +151,13 @@ enum nsim_resource_id {
 	NSIM_RESOURCE_IPV6_FIB_RULES,
 };
 
-int nsim_dev_init(struct netdevsim *ns);
-void nsim_dev_exit(struct netdevsim *ns);
+struct nsim_dev {
+	struct nsim_fib_data *fib_data;
+};
+
+struct nsim_dev *nsim_dev_create_with_ns(struct nsim_bus_dev *nsim_bus_dev,
+					 struct netdevsim *ns);
+void nsim_dev_destroy(struct nsim_dev *nsim_dev);
 
 struct nsim_fib_data *nsim_fib_create(void);
 void nsim_fib_destroy(struct nsim_fib_data *fib_data);
-- 
2.17.2

[patch net-next rfc 10/15] netdevsim: merge sdev into dev

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

As previously introduce dev which is mapped 1:1 to a bus device covers
the purpose of the original shared device, merge the sdev code into dev.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/Makefile              |  2 +-
 drivers/net/netdevsim/bpf.c                 | 79 +++++++++------------
 drivers/net/netdevsim/dev.c                 | 47 ++++++++++++
 drivers/net/netdevsim/netdev.c              | 49 ++++---------
 drivers/net/netdevsim/netdevsim.h           | 54 ++++++--------
 tools/testing/selftests/bpf/test_offload.py |  8 +--
 6 files changed, 120 insertions(+), 119 deletions(-)

diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index eea6162f3886..c8d8acac68c0 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -3,7 +3,7 @@
 obj-$(CONFIG_NETDEVSIM) += netdevsim.o
 
 netdevsim-objs := \
-	netdev.o dev.o fib.o sdev.o bus.o \
+	netdev.o dev.o fib.o bus.o \
 
 ifeq ($(CONFIG_BPF_SYSCALL),y)
 netdevsim-objs += \
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index a93aafe87db3..89980b223adc 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -27,7 +27,7 @@
 	bpf_verifier_log_write(env, "[netdevsim] " fmt, ##__VA_ARGS__)
 
 struct nsim_bpf_bound_prog {
-	struct netdevsim_shared_dev *sdev;
+	struct nsim_dev *nsim_dev;
 	struct bpf_prog *prog;
 	struct dentry *ddir;
 	const char *state;
@@ -65,8 +65,8 @@ nsim_bpf_verify_insn(struct bpf_verifier_env *env, int insn_idx, int prev_insn)
 	struct nsim_bpf_bound_prog *state;
 
 	state = env->prog->aux->offload->dev_priv;
-	if (state->sdev->bpf_bind_verifier_delay && !insn_idx)
-		msleep(state->sdev->bpf_bind_verifier_delay);
+	if (state->nsim_dev->bpf_bind_verifier_delay && !insn_idx)
+		msleep(state->nsim_dev->bpf_bind_verifier_delay);
 
 	if (insn_idx == env->prog->len - 1)
 		pr_vlog(env, "Hello from netdevsim!\n");
@@ -213,7 +213,7 @@ nsim_xdp_set_prog(struct netdevsim *ns, struct netdev_bpf *bpf,
 	return 0;
 }
 
-static int nsim_bpf_create_prog(struct netdevsim_shared_dev *sdev,
+static int nsim_bpf_create_prog(struct nsim_dev *nsim_dev,
 				struct bpf_prog *prog)
 {
 	struct nsim_bpf_bound_prog *state;
@@ -223,13 +223,13 @@ static int nsim_bpf_create_prog(struct netdevsim_shared_dev *sdev,
 	if (!state)
 		return -ENOMEM;
 
-	state->sdev = sdev;
+	state->nsim_dev = nsim_dev;
 	state->prog = prog;
 	state->state = "verify";
 
 	/* Program id is not populated yet when we create the state. */
-	sprintf(name, "%u", sdev->prog_id_gen++);
-	state->ddir = debugfs_create_dir(name, sdev->ddir_bpf_bound_progs);
+	sprintf(name, "%u", nsim_dev->prog_id_gen++);
+	state->ddir = debugfs_create_dir(name, nsim_dev->ddir_bpf_bound_progs);
 	if (IS_ERR_OR_NULL(state->ddir)) {
 		kfree(state);
 		return -ENOMEM;
@@ -240,7 +240,7 @@ static int nsim_bpf_create_prog(struct netdevsim_shared_dev *sdev,
 			    &state->state, &nsim_bpf_string_fops);
 	debugfs_create_bool("loaded", 0400, state->ddir, &state->is_loaded);
 
-	list_add_tail(&state->l, &sdev->bpf_bound_progs);
+	list_add_tail(&state->l, &nsim_dev->bpf_bound_progs);
 
 	prog->aux->offload->dev_priv = state;
 
@@ -249,13 +249,13 @@ static int nsim_bpf_create_prog(struct netdevsim_shared_dev *sdev,
 
 static int nsim_bpf_verifier_prep(struct bpf_prog *prog)
 {
-	struct netdevsim_shared_dev *sdev =
+	struct nsim_dev *nsim_dev =
 			bpf_offload_dev_priv(prog->aux->offload->offdev);
 
-	if (!sdev->bpf_bind_accept)
+	if (!nsim_dev->bpf_bind_accept)
 		return -EOPNOTSUPP;
 
-	return nsim_bpf_create_prog(sdev, prog);
+	return nsim_bpf_create_prog(nsim_dev, prog);
 }
 
 static int nsim_bpf_translate(struct bpf_prog *prog)
@@ -514,7 +514,7 @@ nsim_bpf_map_alloc(struct netdevsim *ns, struct bpf_offloaded_map *offmap)
 	}
 
 	offmap->dev_ops = &nsim_bpf_map_ops;
-	list_add_tail(&nmap->l, &ns->sdev->bpf_bound_maps);
+	list_add_tail(&nmap->l, &ns->nsim_dev->bpf_bound_maps);
 
 	return 0;
 
@@ -578,51 +578,46 @@ int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	}
 }
 
-static int nsim_bpf_sdev_init(struct netdevsim_shared_dev *sdev)
+int nsim_bpf_dev_init(struct nsim_dev *nsim_dev)
 {
 	int err;
 
-	INIT_LIST_HEAD(&sdev->bpf_bound_progs);
-	INIT_LIST_HEAD(&sdev->bpf_bound_maps);
+	INIT_LIST_HEAD(&nsim_dev->bpf_bound_progs);
+	INIT_LIST_HEAD(&nsim_dev->bpf_bound_maps);
 
-	sdev->ddir_bpf_bound_progs =
-		debugfs_create_dir("bpf_bound_progs", sdev->ddir);
-	if (IS_ERR_OR_NULL(sdev->ddir_bpf_bound_progs))
+	nsim_dev->ddir_bpf_bound_progs = debugfs_create_dir("bpf_bound_progs",
+							    nsim_dev->ddir);
+	if (IS_ERR_OR_NULL(nsim_dev->ddir_bpf_bound_progs))
 		return -ENOMEM;
 
-	sdev->bpf_dev = bpf_offload_dev_create(&nsim_bpf_dev_ops, sdev);
-	err = PTR_ERR_OR_ZERO(sdev->bpf_dev);
+	nsim_dev->bpf_dev = bpf_offload_dev_create(&nsim_bpf_dev_ops, nsim_dev);
+	err = PTR_ERR_OR_ZERO(nsim_dev->bpf_dev);
 	if (err)
 		return err;
 
-	sdev->bpf_bind_accept = true;
-	debugfs_create_bool("bpf_bind_accept", 0600, sdev->ddir,
-			    &sdev->bpf_bind_accept);
-	debugfs_create_u32("bpf_bind_verifier_delay", 0600, sdev->ddir,
-			   &sdev->bpf_bind_verifier_delay);
+	nsim_dev->bpf_bind_accept = true;
+	debugfs_create_bool("bpf_bind_accept", 0600, nsim_dev->ddir,
+			    &nsim_dev->bpf_bind_accept);
+	debugfs_create_u32("bpf_bind_verifier_delay", 0600, nsim_dev->ddir,
+			   &nsim_dev->bpf_bind_verifier_delay);
 	return 0;
 }
 
-static void nsim_bpf_sdev_uninit(struct netdevsim_shared_dev *sdev)
+void nsim_bpf_dev_exit(struct nsim_dev *nsim_dev)
 {
-	WARN_ON(!list_empty(&sdev->bpf_bound_progs));
-	WARN_ON(!list_empty(&sdev->bpf_bound_maps));
-	bpf_offload_dev_destroy(sdev->bpf_dev);
+	WARN_ON(!list_empty(&nsim_dev->bpf_bound_progs));
+	WARN_ON(!list_empty(&nsim_dev->bpf_bound_maps));
+	bpf_offload_dev_destroy(nsim_dev->bpf_dev);
 }
 
 int nsim_bpf_init(struct netdevsim *ns)
 {
 	int err;
 
-	if (ns->sdev->refcnt == 1) {
-		err = nsim_bpf_sdev_init(ns->sdev);
-		if (err)
-			return err;
-	}
-
-	err = bpf_offload_dev_netdev_register(ns->sdev->bpf_dev, ns->netdev);
+	err = bpf_offload_dev_netdev_register(ns->nsim_dev->bpf_dev,
+					      ns->netdev);
 	if (err)
-		goto err_bpf_sdev_uninit;
+		return err;
 
 	debugfs_create_u32("bpf_offloaded_id", 0400, ns->ddir,
 			   &ns->bpf_offloaded_id);
@@ -644,11 +639,6 @@ int nsim_bpf_init(struct netdevsim *ns)
 			    &ns->bpf_map_accept);
 
 	return 0;
-
-err_bpf_sdev_uninit:
-	if (ns->sdev->refcnt == 1)
-		nsim_bpf_sdev_uninit(ns->sdev);
-	return err;
 }
 
 void nsim_bpf_uninit(struct netdevsim *ns)
@@ -656,8 +646,5 @@ void nsim_bpf_uninit(struct netdevsim *ns)
 	WARN_ON(ns->xdp.prog);
 	WARN_ON(ns->xdp_hw.prog);
 	WARN_ON(ns->bpf_offloaded);
-	bpf_offload_dev_netdev_unregister(ns->sdev->bpf_dev, ns->netdev);
-
-	if (ns->sdev->refcnt == 1)
-		nsim_bpf_sdev_uninit(ns->sdev);
+	bpf_offload_dev_netdev_unregister(ns->nsim_dev->bpf_dev, ns->netdev);
 }
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index d0bec47cf8e5..ced32c85dd73 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -15,12 +15,31 @@
  * THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
  */
 
+#include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/rtnetlink.h>
 #include <net/devlink.h>
 
 #include "netdevsim.h"
 
+static struct dentry *nsim_dev_ddir;
+
+static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
+{
+	char dev_ddir_name[10];
+
+	sprintf(dev_ddir_name, "%u", nsim_dev->nsim_bus_dev->dev.id);
+	nsim_dev->ddir = debugfs_create_dir(dev_ddir_name, nsim_dev_ddir);
+	if (IS_ERR_OR_NULL(nsim_dev->ddir))
+		return PTR_ERR_OR_ZERO(nsim_dev->ddir) ?: -EINVAL;
+	return 0;
+}
+
+static void nsim_dev_debugfs_exit(struct nsim_dev *nsim_dev)
+{
+	debugfs_remove_recursive(nsim_dev->ddir);
+}
+
 static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
 {
 	struct nsim_dev *nsim_dev = priv;
@@ -184,6 +203,7 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev)
 	if (!devlink)
 		return ERR_PTR(-ENOMEM);
 	nsim_dev = devlink_priv(devlink);
+	nsim_dev->nsim_bus_dev = nsim_bus_dev;
 
 	nsim_dev->fib_data = nsim_fib_create();
 	if (IS_ERR(nsim_dev->fib_data)) {
@@ -199,8 +219,20 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev)
 	if (err)
 		goto err_resources_unregister;
 
+	err = nsim_dev_debugfs_init(nsim_dev);
+	if (err)
+		goto err_dl_unregister;
+
+	err = nsim_bpf_dev_init(nsim_dev);
+	if (err)
+		goto err_debugfs_exit;
+
 	return nsim_dev;
 
+err_debugfs_exit:
+	nsim_dev_debugfs_exit(nsim_dev);
+err_dl_unregister:
+	devlink_unregister(devlink);
 err_resources_unregister:
 	devlink_resources_unregister(devlink, NULL);
 err_fib_destroy:
@@ -227,8 +259,23 @@ void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 {
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
 
+	nsim_bpf_dev_exit(nsim_dev);
+	nsim_dev_debugfs_exit(nsim_dev);
 	devlink_unregister(devlink);
 	devlink_resources_unregister(devlink, NULL);
 	nsim_fib_destroy(nsim_dev->fib_data);
 	devlink_free(devlink);
 }
+
+int nsim_dev_init(void)
+{
+	nsim_dev_ddir = debugfs_create_dir(DRV_NAME "_dev", NULL);
+	if (IS_ERR_OR_NULL(nsim_dev_ddir))
+		return -ENOMEM;
+	return 0;
+}
+
+void nsim_dev_exit(void)
+{
+	debugfs_remove_recursive(nsim_dev_ddir);
+}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 28231bfbc989..c5f4bbb9716f 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -32,24 +32,24 @@ static int nsim_get_port_parent_id(struct net_device *dev,
 {
 	struct netdevsim *ns = netdev_priv(dev);
 
-	ppid->id_len = sizeof(ns->sdev->switch_id);
-	memcpy(&ppid->id, &ns->sdev->switch_id, ppid->id_len);
+	ppid->id_len = sizeof(ns->nsim_dev->nsim_bus_dev->dev.id);
+	memcpy(&ppid->id, &ns->nsim_dev->nsim_bus_dev->dev.id, ppid->id_len);
 	return 0;
 }
 
 static int nsim_init(struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	char sdev_link_name[32];
+	char dev_link_name[32];
 	int err;
 
 	ns->ddir = debugfs_create_dir(netdev_name(dev), nsim_ddir);
 	if (IS_ERR_OR_NULL(ns->ddir))
 		return -ENOMEM;
 
-	sprintf(sdev_link_name, "../../" DRV_NAME "_sdev/%u",
-		ns->sdev->switch_id);
-	debugfs_create_symlink("sdev", ns->ddir, sdev_link_name);
+	sprintf(dev_link_name, "../../" DRV_NAME "_dev/%u",
+		ns->nsim_dev->nsim_bus_dev->dev.id);
+	debugfs_create_symlink("dev", ns->ddir, dev_link_name);
 
 	err = nsim_bpf_init(ns);
 	if (err)
@@ -80,7 +80,6 @@ static void nsim_free(struct net_device *dev)
 	nsim_dev_destroy(ns->nsim_dev);
 	nsim_bus_dev_del(ns->nsim_bus_dev);
 	/* netdev and vf state will be freed out of device_release() */
-	nsim_sdev_put(ns->sdev);
 }
 
 static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -366,31 +365,11 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 			struct netlink_ext_ack *extack)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct netdevsim *joinns = NULL;
 	int err;
 
-	if (tb[IFLA_LINK]) {
-		struct net_device *joindev;
-
-		joindev = __dev_get_by_index(src_net,
-					     nla_get_u32(tb[IFLA_LINK]));
-		if (!joindev)
-			return -ENODEV;
-		if (joindev->netdev_ops != &nsim_netdev_ops)
-			return -EINVAL;
-
-		joinns = netdev_priv(joindev);
-	}
-
-	ns->sdev = nsim_sdev_get(joinns);
-	if (IS_ERR(ns->sdev))
-		return PTR_ERR(ns->sdev);
-
 	ns->nsim_bus_dev = nsim_bus_dev_new(~0, 0);
-	if (IS_ERR(ns->nsim_bus_dev)) {
-		err = PTR_ERR(ns->nsim_bus_dev);
-		goto err_sdev_put;
-	}
+	if (IS_ERR(ns->nsim_bus_dev))
+		return PTR_ERR(ns->nsim_bus_dev);
 
 	SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev);
 	ns->netdev = dev;
@@ -410,8 +389,6 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 	nsim_dev_destroy(ns->nsim_dev);
 err_dev_del:
 	nsim_bus_dev_del(ns->nsim_bus_dev);
-err_sdev_put:
-	nsim_sdev_put(ns->sdev);
 	return err;
 }
 
@@ -431,13 +408,13 @@ static int __init nsim_module_init(void)
 	if (IS_ERR_OR_NULL(nsim_ddir))
 		return -ENOMEM;
 
-	err = nsim_sdev_init();
+	err = nsim_dev_init();
 	if (err)
 		goto err_debugfs_destroy;
 
 	err = nsim_bus_init();
 	if (err)
-		goto err_sdev_exit;
+		goto err_dev_exit;
 
 	err = rtnl_link_register(&nsim_link_ops);
 	if (err)
@@ -447,8 +424,8 @@ static int __init nsim_module_init(void)
 
 err_bus_exit:
 	nsim_bus_exit();
-err_sdev_exit:
-	nsim_sdev_exit();
+err_dev_exit:
+	nsim_dev_exit();
 err_debugfs_destroy:
 	debugfs_remove_recursive(nsim_ddir);
 	return err;
@@ -458,7 +435,7 @@ static void __exit nsim_module_exit(void)
 {
 	rtnl_link_unregister(&nsim_link_ops);
 	nsim_bus_exit();
-	nsim_sdev_exit();
+	nsim_dev_exit();
 	debugfs_remove_recursive(nsim_ddir);
 }
 
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index cfcd801cecc2..5c03d0f6d285 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -26,37 +26,6 @@
 
 #define NSIM_EA(extack, msg)	NL_SET_ERR_MSG_MOD((extack), msg)
 
-struct bpf_prog;
-struct bpf_offload_dev;
-struct dentry;
-struct nsim_vf_config;
-struct nsim_fib_data;
-
-struct netdevsim_shared_dev {
-	unsigned int refcnt;
-	u32 switch_id;
-
-	struct dentry *ddir;
-
-	struct bpf_offload_dev *bpf_dev;
-
-	bool bpf_bind_accept;
-	u32 bpf_bind_verifier_delay;
-
-	struct dentry *ddir_bpf_bound_progs;
-	u32 prog_id_gen;
-
-	struct list_head bpf_bound_progs;
-	struct list_head bpf_bound_maps;
-};
-
-struct netdevsim;
-
-struct netdevsim_shared_dev *nsim_sdev_get(struct netdevsim *joinns);
-void nsim_sdev_put(struct netdevsim_shared_dev *sdev);
-int nsim_sdev_init(void);
-void nsim_sdev_exit(void);
-
 #define NSIM_IPSEC_MAX_SA_COUNT		33
 #define NSIM_IPSEC_VALID		BIT(31)
 
@@ -87,7 +56,6 @@ struct netdevsim {
 	struct u64_stats_sync syncp;
 
 	struct nsim_bus_dev *nsim_bus_dev;
-	struct netdevsim_shared_dev *sdev;
 
 	struct dentry *ddir;
 
@@ -107,6 +75,8 @@ struct netdevsim {
 };
 
 #ifdef CONFIG_BPF_SYSCALL
+int nsim_bpf_dev_init(struct nsim_dev *nsim_dev);
+void nsim_bpf_dev_exit(struct nsim_dev *nsim_dev);
 int nsim_bpf_init(struct netdevsim *ns);
 void nsim_bpf_uninit(struct netdevsim *ns);
 int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf);
@@ -114,6 +84,15 @@ int nsim_bpf_disable_tc(struct netdevsim *ns);
 int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
 			       void *type_data, void *cb_priv);
 #else
+
+static inline int nsim_bpf_dev_init(struct nsim_dev *nsim_dev)
+{
+	return 0;
+}
+
+static inline void nsim_bpf_dev_exit(struct nsim_dev *nsim_dev)
+{
+}
 static inline int nsim_bpf_init(struct netdevsim *ns)
 {
 	return 0;
@@ -152,12 +131,23 @@ enum nsim_resource_id {
 };
 
 struct nsim_dev {
+	struct nsim_bus_dev *nsim_bus_dev;
 	struct nsim_fib_data *fib_data;
+	struct dentry *ddir;
+	struct bpf_offload_dev *bpf_dev;
+	bool bpf_bind_accept;
+	u32 bpf_bind_verifier_delay;
+	struct dentry *ddir_bpf_bound_progs;
+	u32 prog_id_gen;
+	struct list_head bpf_bound_progs;
+	struct list_head bpf_bound_maps;
 };
 
 struct nsim_dev *nsim_dev_create_with_ns(struct nsim_bus_dev *nsim_bus_dev,
 					 struct netdevsim *ns);
 void nsim_dev_destroy(struct nsim_dev *nsim_dev);
+int nsim_dev_init(void);
+void nsim_dev_exit(void);
 
 struct nsim_fib_data *nsim_fib_create(void);
 void nsim_fib_destroy(struct nsim_fib_data *fib_data);
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index a7f95106119f..91dfe876a707 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -335,7 +335,7 @@ class NetdevSim:
         self.ns = ""
 
         self.dfs_dir = '/sys/kernel/debug/netdevsim/%s' % (self.dev['ifname'])
-        self.sdev_dir = self.dfs_dir + '/sdev/'
+        self.dev_dir = self.dfs_dir + '/dev/'
         self.dfs_refresh()
 
     def __getitem__(self, key):
@@ -368,12 +368,12 @@ class NetdevSim:
         return data.strip()
 
     def dfs_num_bound_progs(self):
-        path = os.path.join(self.sdev_dir, "bpf_bound_progs")
+        path = os.path.join(self.dev_dir, "bpf_bound_progs")
         _, progs = cmd('ls %s' % (path))
         return len(progs.split())
 
     def dfs_get_bound_progs(self, expected):
-        progs = DebugfsDir(os.path.join(self.sdev_dir, "bpf_bound_progs"))
+        progs = DebugfsDir(os.path.join(self.dev_dir, "bpf_bound_progs"))
         if expected is not None:
             if len(progs) != expected:
                 fail(True, "%d BPF programs bound, expected %d" %
@@ -1055,7 +1055,7 @@ try:
 
     start_test("Test if netdev removal waits for translation...")
     delay_msec = 500
-    sim.dfs["sdev/bpf_bind_verifier_delay"] = delay_msec
+    sim.dfs["dev/bpf_bind_verifier_delay"] = delay_msec
     start = time.time()
     cmd_line = "tc filter add dev %s ingress bpf %s da skip_sw" % \
                (sim['ifname'], obj)
-- 
2.17.2

Re: [patch net-next rfc 10/15] netdevsim: merge sdev into dev

[Jakub Kicinski]

On Sat, 13 Apr 2019 18:21:07 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> As previously introduce dev which is mapped 1:1 to a bus device covers
> the purpose of the original shared device, merge the sdev code into dev.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

One ASIC may have multiple bus devices.  sdev is ASIC.

Re: [patch net-next rfc 10/15] netdevsim: merge sdev into dev

[Jiri Pirko]

Mon, Apr 15, 2019 at 10:49:46PM CEST, jakub.kicinski@netronome.com wrote:
>On Sat, 13 Apr 2019 18:21:07 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> As previously introduce dev which is mapped 1:1 to a bus device covers
>> the purpose of the original shared device, merge the sdev code into dev.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>One ASIC may have multiple bus devices.  sdev is ASIC.

Yeah, that is a generic problem. That would be job for "devlink aliases".

[patch net-next rfc 11/15] netdevsim: generate random switch id instead of using dev id

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Current implementation of parent_id/switch_id does not follow the
original idea of being unique. The values are "0", "1", etc. Instead of
that, generate 32 random bytes.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/dev.c       | 3 +++
 drivers/net/netdevsim/netdev.c    | 3 +--
 drivers/net/netdevsim/netdevsim.h | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index ced32c85dd73..cad96c44f885 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -17,6 +17,7 @@
 
 #include <linux/debugfs.h>
 #include <linux/device.h>
+#include <linux/random.h>
 #include <linux/rtnetlink.h>
 #include <net/devlink.h>
 
@@ -204,6 +205,8 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev)
 		return ERR_PTR(-ENOMEM);
 	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);
+	get_random_bytes(nsim_dev->switch_id.id, nsim_dev->switch_id.id_len);
 
 	nsim_dev->fib_data = nsim_fib_create();
 	if (IS_ERR(nsim_dev->fib_data)) {
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index c5f4bbb9716f..9b4310e20129 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -32,8 +32,7 @@ static int nsim_get_port_parent_id(struct net_device *dev,
 {
 	struct netdevsim *ns = netdev_priv(dev);
 
-	ppid->id_len = sizeof(ns->nsim_dev->nsim_bus_dev->dev.id);
-	memcpy(&ppid->id, &ns->nsim_dev->nsim_bus_dev->dev.id, ppid->id_len);
+	memcpy(ppid, &ns->nsim_dev->switch_id, sizeof(*ppid));
 	return 0;
 }
 
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 5c03d0f6d285..47c596d85519 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -141,6 +141,7 @@ struct nsim_dev {
 	u32 prog_id_gen;
 	struct list_head bpf_bound_progs;
 	struct list_head bpf_bound_maps;
+	struct netdev_phys_item_id switch_id;
 };
 
 struct nsim_dev *nsim_dev_create_with_ns(struct nsim_bus_dev *nsim_bus_dev,
-- 
2.17.2

[patch net-next rfc 12/15] netdevsim: change debugfs tree topology

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

With the model where dev is represented by devlink and ports are
represented by devlink ports, make debugfs file names independent
on netdev names. Change the topology to the one illustrated
by the following example:

$ ls /sys/kernel/debug/netdevsim/
netdevsim1
$ ls /sys/kernel/debug/netdevsim/netdevsim1/
bpf_bind_accept  bpf_bind_verifier_delay  bpf_bound_progs  ports
$ ls /sys/kernel/debug/netdevsim/netdevsim1/ports/
0  1
$ ls /sys/kernel/debug/netdevsim/netdevsim1/ports/0/
bpf_map_accept  bpf_offloaded_id  bpf_tc_accept  bpf_tc_non_bound_accept  bpf_xdpdrv_accept  bpf_xdpoffload_accept  dev  ipsec
$ ls /sys/kernel/debug/netdevsim/netdevsim1/ports/0/dev -l
lrwxrwxrwx 1 root root 0 Apr 13 15:58 /sys/kernel/debug/netdevsim/netdevsim1/ports/0/dev -> ../../../netdevsim1

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/dev.c                 | 10 +++++++---
 drivers/net/netdevsim/netdev.c              | 15 +++------------
 drivers/net/netdevsim/netdevsim.h           |  1 +
 tools/testing/selftests/bpf/test_offload.py |  4 +++-
 tools/testing/selftests/net/rtnetlink.sh    |  2 +-
 5 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index cad96c44f885..ef518d59afba 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -27,17 +27,21 @@ static struct dentry *nsim_dev_ddir;
 
 static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
 {
-	char dev_ddir_name[10];
+	char dev_ddir_name[16];
 
-	sprintf(dev_ddir_name, "%u", nsim_dev->nsim_bus_dev->dev.id);
+	sprintf(dev_ddir_name, DRV_NAME "%u", nsim_dev->nsim_bus_dev->dev.id);
 	nsim_dev->ddir = debugfs_create_dir(dev_ddir_name, nsim_dev_ddir);
 	if (IS_ERR_OR_NULL(nsim_dev->ddir))
 		return PTR_ERR_OR_ZERO(nsim_dev->ddir) ?: -EINVAL;
+	nsim_dev->ports_ddir = debugfs_create_dir("ports", nsim_dev->ddir);
+	if (IS_ERR_OR_NULL(nsim_dev->ports_ddir))
+		return PTR_ERR_OR_ZERO(nsim_dev->ports_ddir) ?: -EINVAL;
 	return 0;
 }
 
 static void nsim_dev_debugfs_exit(struct nsim_dev *nsim_dev)
 {
+	debugfs_remove_recursive(nsim_dev->ports_ddir);
 	debugfs_remove_recursive(nsim_dev->ddir);
 }
 
@@ -272,7 +276,7 @@ void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 
 int nsim_dev_init(void)
 {
-	nsim_dev_ddir = debugfs_create_dir(DRV_NAME "_dev", NULL);
+	nsim_dev_ddir = debugfs_create_dir(DRV_NAME, NULL);
 	if (IS_ERR_OR_NULL(nsim_dev_ddir))
 		return -ENOMEM;
 	return 0;
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 9b4310e20129..eb823bd0dc39 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -25,8 +25,6 @@
 
 #include "netdevsim.h"
 
-static struct dentry *nsim_ddir;
-
 static int nsim_get_port_parent_id(struct net_device *dev,
 				   struct netdev_phys_item_id *ppid)
 {
@@ -42,11 +40,11 @@ static int nsim_init(struct net_device *dev)
 	char dev_link_name[32];
 	int err;
 
-	ns->ddir = debugfs_create_dir(netdev_name(dev), nsim_ddir);
+	ns->ddir = debugfs_create_dir("0", ns->nsim_dev->ports_ddir);
 	if (IS_ERR_OR_NULL(ns->ddir))
 		return -ENOMEM;
 
-	sprintf(dev_link_name, "../../" DRV_NAME "_dev/%u",
+	sprintf(dev_link_name, "../../../" DRV_NAME "%u",
 		ns->nsim_dev->nsim_bus_dev->dev.id);
 	debugfs_create_symlink("dev", ns->ddir, dev_link_name);
 
@@ -403,13 +401,9 @@ static int __init nsim_module_init(void)
 {
 	int err;
 
-	nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
-	if (IS_ERR_OR_NULL(nsim_ddir))
-		return -ENOMEM;
-
 	err = nsim_dev_init();
 	if (err)
-		goto err_debugfs_destroy;
+		return err;
 
 	err = nsim_bus_init();
 	if (err)
@@ -425,8 +419,6 @@ static int __init nsim_module_init(void)
 	nsim_bus_exit();
 err_dev_exit:
 	nsim_dev_exit();
-err_debugfs_destroy:
-	debugfs_remove_recursive(nsim_ddir);
 	return err;
 }
 
@@ -435,7 +427,6 @@ static void __exit nsim_module_exit(void)
 	rtnl_link_unregister(&nsim_link_ops);
 	nsim_bus_exit();
 	nsim_dev_exit();
-	debugfs_remove_recursive(nsim_ddir);
 }
 
 module_init(nsim_module_init);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 47c596d85519..ec678371865f 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -134,6 +134,7 @@ struct nsim_dev {
 	struct nsim_bus_dev *nsim_bus_dev;
 	struct nsim_fib_data *fib_data;
 	struct dentry *ddir;
+	struct dentry *ports_ddir;
 	struct bpf_offload_dev *bpf_dev;
 	bool bpf_bind_accept;
 	u32 bpf_bind_verifier_delay;
diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 91dfe876a707..5f2e4f9e70e4 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -306,6 +306,8 @@ class DebugfsDir:
 
         _, out = cmd('ls ' + path)
         for f in out.split():
+            if f == "ports":
+                continue
             p = os.path.join(path, f)
             if os.path.isfile(p):
                 _, out = cmd('cat %s/%s' % (path, f))
@@ -334,7 +336,7 @@ class NetdevSim:
 
         self.ns = ""
 
-        self.dfs_dir = '/sys/kernel/debug/netdevsim/%s' % (self.dev['ifname'])
+        self.dfs_dir = '/sys/kernel/debug/netdevsim/netdevsim0/ports/0/'
         self.dev_dir = self.dfs_dir + '/dev/'
         self.dfs_refresh()
 
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index b447803f3f8a..311f82bcbe8d 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -697,7 +697,7 @@ kci_test_ipsec_offload()
 	srcip=192.168.123.3
 	dstip=192.168.123.4
 	dev=simx1
-	sysfsd=/sys/kernel/debug/netdevsim/$dev
+	sysfsd=/sys/kernel/debug/netdevsim/netdevsim0/ports/0/
 	sysfsf=$sysfsd/ipsec
 
 	# setup netdevsim since dummydev doesn't have offload support
-- 
2.17.2

[patch net-next rfc 13/15] netdevsim: implement dev probe/remove skeleton with port initialization

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Implement netdevsim bus probing of netdevsim devices. For every probed
device create a devlink instance. According to the user-passed value,
create a number of ports represented by devlink port instances.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/bus.c       |  29 ++++++++
 drivers/net/netdevsim/dev.c       | 115 ++++++++++++++++++++++++++----
 drivers/net/netdevsim/netdev.c    |  15 ++--
 drivers/net/netdevsim/netdevsim.h |  14 +++-
 4 files changed, 144 insertions(+), 29 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 4471f5db50ee..46108a926890 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -194,6 +194,21 @@ static struct attribute *nsim_bus_attrs[] = {
 };
 ATTRIBUTE_GROUPS(nsim_bus);
 
+static int nsim_bus_probe(struct device *dev)
+{
+	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
+
+	return nsim_dev_probe(nsim_bus_dev);
+}
+
+static int nsim_bus_remove(struct device *dev)
+{
+	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
+
+	nsim_dev_remove(nsim_bus_dev);
+	return 0;
+}
+
 int nsim_num_vf(struct device *dev)
 {
 	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
@@ -205,6 +220,8 @@ static struct bus_type nsim_bus = {
 	.name		= DRV_NAME,
 	.dev_name	= DRV_NAME,
 	.bus_groups	= nsim_bus_groups,
+	.probe		= nsim_bus_probe,
+	.remove		= nsim_bus_remove,
 	.num_vf		= nsim_num_vf,
 };
 
@@ -238,6 +255,18 @@ struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count)
 	return ERR_PTR(err);
 }
 
+struct nsim_bus_dev *nsim_bus_dev_new_with_ns(struct netdevsim *ns)
+{
+	struct nsim_bus_dev *nsim_bus_dev;
+
+	dev_hold(ns->netdev);
+	rtnl_unlock();
+	nsim_bus_dev = nsim_bus_dev_new(~0, 0);
+	rtnl_lock();
+	dev_put(ns->netdev);
+	return nsim_bus_dev;
+}
+
 void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
 {
 	device_unregister(&nsim_bus_dev->dev);
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index ef518d59afba..43d87c457ad9 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -45,6 +45,30 @@ static void nsim_dev_debugfs_exit(struct nsim_dev *nsim_dev)
 	debugfs_remove_recursive(nsim_dev->ddir);
 }
 
+static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev,
+				      struct nsim_dev_port *nsim_dev_port)
+{
+	char port_ddir_name[16];
+	char dev_link_name[32];
+
+	sprintf(port_ddir_name, "%u", nsim_dev_port->port_index);
+	nsim_dev_port->ddir = debugfs_create_dir(port_ddir_name,
+						 nsim_dev->ports_ddir);
+	if (IS_ERR_OR_NULL(nsim_dev_port->ddir))
+		return -ENOMEM;
+
+	sprintf(dev_link_name, "../../../" DRV_NAME "%u",
+		nsim_dev->nsim_bus_dev->dev.id);
+	debugfs_create_symlink("dev", nsim_dev_port->ddir, dev_link_name);
+
+	return 0;
+}
+
+static void nsim_dev_port_debugfs_exit(struct nsim_dev_port *nsim_dev_port)
+{
+	debugfs_remove_recursive(nsim_dev_port->ddir);
+}
+
 static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
 {
 	struct nsim_dev *nsim_dev = priv;
@@ -198,13 +222,15 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
 	.reload = nsim_dev_reload,
 };
 
-static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev)
+static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev,
+					unsigned int port_count)
 {
 	struct nsim_dev *nsim_dev;
 	struct devlink *devlink;
 	int err;
 
-	devlink = devlink_alloc(&nsim_dev_devlink_ops, sizeof(*nsim_dev));
+	devlink = devlink_alloc(&nsim_dev_devlink_ops,
+				struct_size(nsim_dev, ports, port_count));
 	if (!devlink)
 		return ERR_PTR(-ENOMEM);
 	nsim_dev = devlink_priv(devlink);
@@ -249,19 +275,6 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev)
 	return ERR_PTR(err);
 }
 
-struct nsim_dev *nsim_dev_create_with_ns(struct nsim_bus_dev *nsim_bus_dev,
-					 struct netdevsim *ns)
-{
-	struct nsim_dev *nsim_dev;
-
-	dev_hold(ns->netdev);
-	rtnl_unlock();
-	nsim_dev = nsim_dev_create(nsim_bus_dev);
-	rtnl_lock();
-	dev_put(ns->netdev);
-	return nsim_dev;
-}
-
 void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 {
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
@@ -274,6 +287,78 @@ void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 	devlink_free(devlink);
 }
 
+static int nsim_dev_port_init(struct nsim_dev *nsim_dev,
+			      unsigned int port_index)
+{
+	struct nsim_dev_port *nsim_dev_port = &nsim_dev->ports[port_index];
+	struct devlink_port *devlink_port = &nsim_dev_port->devlink_port;
+	int err;
+
+	nsim_dev_port->port_index = port_index;
+	devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
+			       port_index + 1, 0, 0,
+			       nsim_dev->switch_id.id,
+			       nsim_dev->switch_id.id_len);
+	err = devlink_port_register(priv_to_devlink(nsim_dev), devlink_port,
+				    port_index);
+	if (err)
+		return err;
+
+	err = nsim_dev_port_debugfs_init(nsim_dev, nsim_dev_port);
+	if (err)
+		goto err_dl_port_unregister;
+	return 0;
+
+err_dl_port_unregister:
+	devlink_port_unregister(devlink_port);
+	return err;
+}
+
+static void nsim_dev_port_exit(struct nsim_dev *nsim_dev,
+			       unsigned int port_index)
+{
+	struct nsim_dev_port *nsim_dev_port = &nsim_dev->ports[port_index];
+	struct devlink_port *devlink_port = &nsim_dev_port->devlink_port;
+
+	nsim_dev_port_debugfs_exit(nsim_dev_port);
+	devlink_port_unregister(devlink_port);
+}
+
+int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
+{
+	struct nsim_dev *nsim_dev;
+	int i;
+	int err;
+
+	nsim_dev = nsim_dev_create(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_init(nsim_dev, i);
+		if (err)
+			goto port_rollback;
+	}
+	return 0;
+
+port_rollback:
+	for (i--; i >= 0; i--)
+		nsim_dev_port_exit(nsim_dev, i);
+	nsim_dev_destroy(nsim_dev);
+	return err;
+}
+
+void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev)
+{
+	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
+	int i;
+
+	for (i = nsim_bus_dev->port_count - 1; i >= 0; i--)
+		nsim_dev_port_exit(nsim_dev, i);
+	nsim_dev_destroy(nsim_dev);
+}
+
 int nsim_dev_init(void)
 {
 	nsim_dev_ddir = debugfs_create_dir(DRV_NAME, NULL);
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index eb823bd0dc39..99169fe521f2 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -74,7 +74,6 @@ static void nsim_free(struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
 
-	nsim_dev_destroy(ns->nsim_dev);
 	nsim_bus_dev_del(ns->nsim_bus_dev);
 	/* netdev and vf state will be freed out of device_release() */
 }
@@ -364,26 +363,20 @@ static int nsim_newlink(struct net *src_net, struct net_device *dev,
 	struct netdevsim *ns = netdev_priv(dev);
 	int err;
 
-	ns->nsim_bus_dev = nsim_bus_dev_new(~0, 0);
+	ns->netdev = dev;
+	ns->nsim_bus_dev = nsim_bus_dev_new_with_ns(ns);
 	if (IS_ERR(ns->nsim_bus_dev))
 		return PTR_ERR(ns->nsim_bus_dev);
 
 	SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev);
-	ns->netdev = dev;
 
-	ns->nsim_dev = nsim_dev_create_with_ns(ns->nsim_bus_dev, ns);
-	if (IS_ERR(ns->nsim_dev)) {
-		err = PTR_ERR(ns->nsim_dev);
-		goto err_dev_del;
-	}
+	ns->nsim_dev = dev_get_drvdata(&ns->nsim_bus_dev->dev);
 
 	err = register_netdevice(dev);
 	if (err)
-		goto err_dev_destroy;
+		goto err_dev_del;
 	return 0;
 
-err_dev_destroy:
-	nsim_dev_destroy(ns->nsim_dev);
 err_dev_del:
 	nsim_bus_dev_del(ns->nsim_bus_dev);
 	return err;
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index ec678371865f..c0a08af5e724 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -18,6 +18,7 @@
 #include <linux/list.h>
 #include <linux/netdevice.h>
 #include <linux/u64_stats_sync.h>
+#include <net/devlink.h>
 #include <net/xdp.h>
 
 #define DRV_NAME	"netdevsim"
@@ -130,6 +131,12 @@ enum nsim_resource_id {
 	NSIM_RESOURCE_IPV6_FIB_RULES,
 };
 
+struct nsim_dev_port {
+	struct devlink_port devlink_port;
+	unsigned int port_index;
+	struct dentry *ddir;
+};
+
 struct nsim_dev {
 	struct nsim_bus_dev *nsim_bus_dev;
 	struct nsim_fib_data *fib_data;
@@ -143,13 +150,13 @@ struct nsim_dev {
 	struct list_head bpf_bound_progs;
 	struct list_head bpf_bound_maps;
 	struct netdev_phys_item_id switch_id;
+	struct nsim_dev_port ports[0];
 };
 
-struct nsim_dev *nsim_dev_create_with_ns(struct nsim_bus_dev *nsim_bus_dev,
-					 struct netdevsim *ns);
-void nsim_dev_destroy(struct nsim_dev *nsim_dev);
 int nsim_dev_init(void);
 void nsim_dev_exit(void);
+int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev);
+void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev);
 
 struct nsim_fib_data *nsim_fib_create(void);
 void nsim_fib_destroy(struct nsim_fib_data *fib_data);
@@ -200,6 +207,7 @@ struct nsim_bus_dev {
 };
 
 struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count);
+struct nsim_bus_dev *nsim_bus_dev_new_with_ns(struct netdevsim *ns);
 void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev);
 int nsim_bus_init(void);
 void nsim_bus_exit(void);
-- 
2.17.2

[patch net-next rfc 14/15] netdevsim: move netdev creation/destruction to dev probe

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Remove the existing way to create netdevsim over rtnetlink and move the
netdev creation/destruction to dev probe, so for every probed port,
a netdevsim-netdev instance is created.

Adjust selftests to work with new interface.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/bpf.c              |  13 ++-
 drivers/net/netdevsim/bus.c              |  25 ++--
 drivers/net/netdevsim/dev.c              |  14 ++-
 drivers/net/netdevsim/ipsec.c            |   3 +-
 drivers/net/netdevsim/netdev.c           | 138 ++++++++---------------
 drivers/net/netdevsim/netdevsim.h        |  11 +-
 tools/testing/selftests/net/rtnetlink.sh |   6 +-
 7 files changed, 89 insertions(+), 121 deletions(-)

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 89980b223adc..2b74425822ab 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -612,6 +612,7 @@ void nsim_bpf_dev_exit(struct nsim_dev *nsim_dev)
 
 int nsim_bpf_init(struct netdevsim *ns)
 {
+	struct dentry *ddir = ns->nsim_dev_port->ddir;
 	int err;
 
 	err = bpf_offload_dev_netdev_register(ns->nsim_dev->bpf_dev,
@@ -619,23 +620,23 @@ int nsim_bpf_init(struct netdevsim *ns)
 	if (err)
 		return err;
 
-	debugfs_create_u32("bpf_offloaded_id", 0400, ns->ddir,
+	debugfs_create_u32("bpf_offloaded_id", 0400, ddir,
 			   &ns->bpf_offloaded_id);
 
 	ns->bpf_tc_accept = true;
-	debugfs_create_bool("bpf_tc_accept", 0600, ns->ddir,
+	debugfs_create_bool("bpf_tc_accept", 0600, ddir,
 			    &ns->bpf_tc_accept);
-	debugfs_create_bool("bpf_tc_non_bound_accept", 0600, ns->ddir,
+	debugfs_create_bool("bpf_tc_non_bound_accept", 0600, ddir,
 			    &ns->bpf_tc_non_bound_accept);
 	ns->bpf_xdpdrv_accept = true;
-	debugfs_create_bool("bpf_xdpdrv_accept", 0600, ns->ddir,
+	debugfs_create_bool("bpf_xdpdrv_accept", 0600, ddir,
 			    &ns->bpf_xdpdrv_accept);
 	ns->bpf_xdpoffload_accept = true;
-	debugfs_create_bool("bpf_xdpoffload_accept", 0600, ns->ddir,
+	debugfs_create_bool("bpf_xdpoffload_accept", 0600, ddir,
 			    &ns->bpf_xdpoffload_accept);
 
 	ns->bpf_map_accept = true;
-	debugfs_create_bool("bpf_map_accept", 0600, ns->ddir,
+	debugfs_create_bool("bpf_map_accept", 0600, ddir,
 			    &ns->bpf_map_accept);
 
 	return 0;
diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 46108a926890..a8e0b1f60a6f 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -117,6 +117,9 @@ static struct device_type nsim_bus_dev_type = {
 	.release = nsim_bus_dev_release,
 };
 
+static struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id,
+					     unsigned int port_count);
+
 static ssize_t new_device_store(struct bus_type *bus, const char *buf,
 				size_t count)
 {
@@ -152,6 +155,8 @@ static ssize_t new_device_store(struct bus_type *bus, const char *buf,
 }
 static BUS_ATTR_WO(new_device);
 
+static void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev);
+
 static ssize_t del_device_store(struct bus_type *bus, const char *buf,
 				size_t count)
 {
@@ -225,7 +230,8 @@ static struct bus_type nsim_bus = {
 	.num_vf		= nsim_num_vf,
 };
 
-struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count)
+static struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id,
+					     unsigned int port_count)
 {
 	struct nsim_bus_dev *nsim_bus_dev;
 	int err;
@@ -234,8 +240,7 @@ struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count)
 	if (!nsim_bus_dev)
 		return ERR_PTR(-ENOMEM);
 
-	err = ida_alloc_range(&nsim_bus_dev_ids,
-			      id == ~0 ? 0 : id, id, GFP_KERNEL);
+	err = ida_alloc_range(&nsim_bus_dev_ids, id, id, GFP_KERNEL);
 	if (err < 0)
 		goto err_nsim_bus_dev_free;
 	nsim_bus_dev->dev.id = err;
@@ -255,19 +260,7 @@ struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count)
 	return ERR_PTR(err);
 }
 
-struct nsim_bus_dev *nsim_bus_dev_new_with_ns(struct netdevsim *ns)
-{
-	struct nsim_bus_dev *nsim_bus_dev;
-
-	dev_hold(ns->netdev);
-	rtnl_unlock();
-	nsim_bus_dev = nsim_bus_dev_new(~0, 0);
-	rtnl_lock();
-	dev_put(ns->netdev);
-	return nsim_bus_dev;
-}
-
-void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
+static void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
 {
 	device_unregister(&nsim_bus_dev->dev);
 	ida_free(&nsim_bus_dev_ids, nsim_bus_dev->dev.id);
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 43d87c457ad9..7bd33989f484 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -275,7 +275,7 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev,
 	return ERR_PTR(err);
 }
 
-void nsim_dev_destroy(struct nsim_dev *nsim_dev)
+static void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 {
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
 
@@ -307,8 +307,18 @@ static int nsim_dev_port_init(struct nsim_dev *nsim_dev,
 	err = nsim_dev_port_debugfs_init(nsim_dev, nsim_dev_port);
 	if (err)
 		goto err_dl_port_unregister;
+
+	nsim_dev_port->ns = nsim_create(nsim_dev, nsim_dev_port);
+	if (IS_ERR(nsim_dev_port->ns)) {
+		err = PTR_ERR(nsim_dev_port->ns);
+		goto err_port_debugfs_exit;
+	}
+
+	devlink_port_type_eth_set(devlink_port, nsim_dev_port->ns->netdev);
 	return 0;
 
+err_port_debugfs_exit:
+	nsim_dev_port_debugfs_exit(nsim_dev_port);
 err_dl_port_unregister:
 	devlink_port_unregister(devlink_port);
 	return err;
@@ -320,6 +330,8 @@ static void nsim_dev_port_exit(struct nsim_dev *nsim_dev,
 	struct nsim_dev_port *nsim_dev_port = &nsim_dev->ports[port_index];
 	struct devlink_port *devlink_port = &nsim_dev_port->devlink_port;
 
+	devlink_port_type_clear(devlink_port);
+	nsim_destroy(nsim_dev_port->ns);
 	nsim_dev_port_debugfs_exit(nsim_dev_port);
 	devlink_port_unregister(devlink_port);
 }
diff --git a/drivers/net/netdevsim/ipsec.c b/drivers/net/netdevsim/ipsec.c
index 76e11d889bb6..e27fc1a4516d 100644
--- a/drivers/net/netdevsim/ipsec.c
+++ b/drivers/net/netdevsim/ipsec.c
@@ -283,7 +283,8 @@ void nsim_ipsec_init(struct netdevsim *ns)
 	ns->netdev->features |= NSIM_ESP_FEATURES;
 	ns->netdev->hw_enc_features |= NSIM_ESP_FEATURES;
 
-	ns->ipsec.pfile = debugfs_create_file("ipsec", 0400, ns->ddir, ns,
+	ns->ipsec.pfile = debugfs_create_file("ipsec", 0400,
+					      ns->nsim_dev_port->ddir, ns,
 					      &ipsec_dbg_fops);
 }
 
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 99169fe521f2..38ebf846ed49 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -25,59 +25,6 @@
 
 #include "netdevsim.h"
 
-static int nsim_get_port_parent_id(struct net_device *dev,
-				   struct netdev_phys_item_id *ppid)
-{
-	struct netdevsim *ns = netdev_priv(dev);
-
-	memcpy(ppid, &ns->nsim_dev->switch_id, sizeof(*ppid));
-	return 0;
-}
-
-static int nsim_init(struct net_device *dev)
-{
-	struct netdevsim *ns = netdev_priv(dev);
-	char dev_link_name[32];
-	int err;
-
-	ns->ddir = debugfs_create_dir("0", ns->nsim_dev->ports_ddir);
-	if (IS_ERR_OR_NULL(ns->ddir))
-		return -ENOMEM;
-
-	sprintf(dev_link_name, "../../../" DRV_NAME "%u",
-		ns->nsim_dev->nsim_bus_dev->dev.id);
-	debugfs_create_symlink("dev", ns->ddir, dev_link_name);
-
-	err = nsim_bpf_init(ns);
-	if (err)
-		goto err_debugfs_destroy;
-
-	nsim_ipsec_init(ns);
-
-	return 0;
-
-err_debugfs_destroy:
-	debugfs_remove_recursive(ns->ddir);
-	return err;
-}
-
-static void nsim_uninit(struct net_device *dev)
-{
-	struct netdevsim *ns = netdev_priv(dev);
-
-	nsim_ipsec_teardown(ns);
-	debugfs_remove_recursive(ns->ddir);
-	nsim_bpf_uninit(ns);
-}
-
-static void nsim_free(struct net_device *dev)
-{
-	struct netdevsim *ns = netdev_priv(dev);
-
-	nsim_bus_dev_del(ns->nsim_bus_dev);
-	/* netdev and vf state will be freed out of device_release() */
-}
-
 static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
@@ -299,8 +246,6 @@ nsim_set_features(struct net_device *dev, netdev_features_t features)
 }
 
 static const struct net_device_ops nsim_netdev_ops = {
-	.ndo_init		= nsim_init,
-	.ndo_uninit		= nsim_uninit,
 	.ndo_start_xmit		= nsim_start_xmit,
 	.ndo_set_rx_mode	= nsim_set_rx_mode,
 	.ndo_set_mac_address	= eth_mac_addr,
@@ -318,7 +263,6 @@ static const struct net_device_ops nsim_netdev_ops = {
 	.ndo_setup_tc		= nsim_setup_tc,
 	.ndo_set_features	= nsim_set_features,
 	.ndo_bpf		= nsim_bpf,
-	.ndo_get_port_parent_id	= nsim_get_port_parent_id,
 };
 
 static void nsim_setup(struct net_device *dev)
@@ -326,10 +270,6 @@ static void nsim_setup(struct net_device *dev)
 	ether_setup(dev);
 	eth_hw_addr_random(dev);
 
-	dev->netdev_ops = &nsim_netdev_ops;
-	dev->needs_free_netdev = true;
-	dev->priv_destructor = nsim_free;
-
 	dev->tx_queue_len = 0;
 	dev->flags |= IFF_NOARP;
 	dev->flags &= ~IFF_MULTICAST;
@@ -344,50 +284,70 @@ static void nsim_setup(struct net_device *dev)
 	dev->max_mtu = ETH_MAX_MTU;
 }
 
-static int nsim_validate(struct nlattr *tb[], struct nlattr *data[],
-			 struct netlink_ext_ack *extack)
-{
-	if (tb[IFLA_ADDRESS]) {
-		if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
-			return -EINVAL;
-		if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
-			return -EADDRNOTAVAIL;
-	}
-	return 0;
-}
-
-static int nsim_newlink(struct net *src_net, struct net_device *dev,
-			struct nlattr *tb[], struct nlattr *data[],
-			struct netlink_ext_ack *extack)
+struct netdevsim *nsim_create(struct nsim_dev *nsim_dev,
+			      struct nsim_dev_port *nsim_dev_port)
 {
-	struct netdevsim *ns = netdev_priv(dev);
+	struct net_device *dev;
+	struct netdevsim *ns;
 	int err;
 
-	ns->netdev = dev;
-	ns->nsim_bus_dev = nsim_bus_dev_new_with_ns(ns);
-	if (IS_ERR(ns->nsim_bus_dev))
-		return PTR_ERR(ns->nsim_bus_dev);
+	dev = alloc_netdev(sizeof(*ns), "eth%d", NET_NAME_UNKNOWN, nsim_setup);
+	if (!dev)
+		return ERR_PTR(-ENOMEM);
 
+	ns = netdev_priv(dev);
+	ns->netdev = dev;
+	ns->nsim_dev = nsim_dev;
+	ns->nsim_dev_port = nsim_dev_port;
+	ns->nsim_bus_dev = nsim_dev->nsim_bus_dev;
 	SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev);
+	dev->netdev_ops = &nsim_netdev_ops;
 
-	ns->nsim_dev = dev_get_drvdata(&ns->nsim_bus_dev->dev);
+	rtnl_lock();
+	err = nsim_bpf_init(ns);
+	if (err)
+		goto err_free_netdev;
+
+	nsim_ipsec_init(ns);
 
 	err = register_netdevice(dev);
 	if (err)
-		goto err_dev_del;
-	return 0;
+		goto err_ipsec_teardown;
+	rtnl_unlock();
 
-err_dev_del:
-	nsim_bus_dev_del(ns->nsim_bus_dev);
-	return err;
+	return ns;
+
+err_ipsec_teardown:
+	nsim_ipsec_teardown(ns);
+	nsim_bpf_uninit(ns);
+	rtnl_unlock();
+err_free_netdev:
+	free_netdev(dev);
+	return ERR_PTR(err);
+}
+
+void nsim_destroy(struct netdevsim *ns)
+{
+	struct net_device *dev = ns->netdev;
+
+	rtnl_lock();
+	unregister_netdevice(dev);
+	nsim_ipsec_teardown(ns);
+	nsim_bpf_uninit(ns);
+	rtnl_unlock();
+	free_netdev(dev);
+}
+
+static int nsim_validate(struct nlattr *tb[], struct nlattr *data[],
+			 struct netlink_ext_ack *extack)
+{
+	NL_SET_ERR_MSG_MOD(extack, "Please use: echo \"[ID] [PORT_COUNT]\" > /sys/bus/netdevsim/new_device");
+	return -EOPNOTSUPP;
 }
 
 static struct rtnl_link_ops nsim_link_ops __read_mostly = {
 	.kind		= DRV_NAME,
-	.priv_size	= sizeof(struct netdevsim),
-	.setup		= nsim_setup,
 	.validate	= nsim_validate,
-	.newlink	= nsim_newlink,
 };
 
 static int __init nsim_module_init(void)
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index c0a08af5e724..a91e2118dd0e 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -51,6 +51,7 @@ struct nsim_ipsec {
 struct netdevsim {
 	struct net_device *netdev;
 	struct nsim_dev *nsim_dev;
+	struct nsim_dev_port *nsim_dev_port;
 
 	u64 tx_packets;
 	u64 tx_bytes;
@@ -58,8 +59,6 @@ struct netdevsim {
 
 	struct nsim_bus_dev *nsim_bus_dev;
 
-	struct dentry *ddir;
-
 	struct bpf_prog	*bpf_offloaded;
 	u32 bpf_offloaded_id;
 
@@ -75,6 +74,10 @@ struct netdevsim {
 	struct nsim_ipsec ipsec;
 };
 
+struct netdevsim *nsim_create(struct nsim_dev *nsim_dev,
+			      struct nsim_dev_port *nsim_dev_port);
+void nsim_destroy(struct netdevsim *ns);
+
 #ifdef CONFIG_BPF_SYSCALL
 int nsim_bpf_dev_init(struct nsim_dev *nsim_dev);
 void nsim_bpf_dev_exit(struct nsim_dev *nsim_dev);
@@ -135,6 +138,7 @@ struct nsim_dev_port {
 	struct devlink_port devlink_port;
 	unsigned int port_index;
 	struct dentry *ddir;
+	struct netdevsim *ns;
 };
 
 struct nsim_dev {
@@ -206,8 +210,5 @@ struct nsim_bus_dev {
 	struct nsim_vf_config *vfconfigs;
 };
 
-struct nsim_bus_dev *nsim_bus_dev_new(unsigned int id, unsigned int port_count);
-struct nsim_bus_dev *nsim_bus_dev_new_with_ns(struct netdevsim *ns);
-void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev);
 int nsim_bus_init(void);
 void nsim_bus_exit(void);
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 311f82bcbe8d..710d73ceb9c3 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -696,7 +696,7 @@ kci_test_ipsec_offload()
 	algo="aead rfc4106(gcm(aes)) 0x3132333435363738393031323334353664636261 128"
 	srcip=192.168.123.3
 	dstip=192.168.123.4
-	dev=simx1
+	dev=netdevsim0p1
 	sysfsd=/sys/kernel/debug/netdevsim/netdevsim0/ports/0/
 	sysfsf=$sysfsd/ipsec
 
@@ -708,7 +708,8 @@ kci_test_ipsec_offload()
 		return 1
 	fi
 
-	ip link add $dev type netdevsim
+	echo "0" > /sys/bus/netdevsim/new_device
+	sleep 0.2
 	ip addr add $srcip dev $dev
 	ip link set $dev up
 	if [ ! -d $sysfsd ] ; then
@@ -781,7 +782,6 @@ EOF
 	fi
 
 	# clean up any leftovers
-	ip link del $dev
 	rmmod netdevsim
 
 	if [ $ret -ne 0 ]; then
-- 
2.17.2

[patch net-next rfc 15/15] netdevsim: implement ndo_get_devlink_port

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Implement ndo_get_devlink_port and allow switch_id and port_name to be
handled by devlink.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/netdev.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 38ebf846ed49..9723b14363f1 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -245,6 +245,13 @@ nsim_set_features(struct net_device *dev, netdev_features_t features)
 	return 0;
 }
 
+static struct devlink_port *nsim_get_devlink_port(struct net_device *dev)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	return &ns->nsim_dev_port->devlink_port;
+}
+
 static const struct net_device_ops nsim_netdev_ops = {
 	.ndo_start_xmit		= nsim_start_xmit,
 	.ndo_set_rx_mode	= nsim_set_rx_mode,
@@ -263,6 +270,7 @@ static const struct net_device_ops nsim_netdev_ops = {
 	.ndo_setup_tc		= nsim_setup_tc,
 	.ndo_set_features	= nsim_set_features,
 	.ndo_bpf		= nsim_bpf,
+	.ndo_get_devlink_port	= nsim_get_devlink_port,
 };
 
 static void nsim_setup(struct net_device *dev)
-- 
2.17.2

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jakub Kicinski]

On Sat, 13 Apr 2019 18:20:57 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Currently the model of netdevsim is a bit odd in multiple ways.
> 1) devlink instance is not in any way related with actual netdevsim
>    netdevices. Instead, it is created per-namespace.
> 2) multi-port netdevsim device is done using "link" attribute.
> 3) netdevsim bus is there only to have something to bind the netdev to,
>    it really does not act as a bus.

Nope, it's there to expose SR-IOV ops :)

> 4) netdevsim instances are created by "ip link add" which is great for
>    soft devices with no hw backend. The rtnl core allocates netdev and
>    calls into driver holding rtnl mutex. For hw-backed devices, this
>    flow is wrong as it breaks order in which things are done.
> 
> This patchset adjust netdevsim to fix all above.
> 
> In order to support proper devlink and devlink port instances and to be
> able to emulate real devices, there is need to implement bus probe and
> instantiate everything from there. User can specify device id and port
> count to be instantianted. For example:
> 
> echo "10 4" > /sys/bus/netdevsim/new_device

I really don't like the design where ID has to be allocated by user
space.  It's a step back.

I also dislike declaring ports from the start.  In real drivers ports
are never "atomically" registered, they are crated and destroyed one 
by one, and a lot of races/UAFs/bugs lie in those small periods of
time where one netdev got unregistered, but other are still around...

> Then devlink shows this:
> 
> $ devlink dev
> netdevsim/netdevsim10
> 
> $ devlink port
> netdevsim/netdevsim10/0: type eth netdev netdevsim10p1 flavour physical
> netdevsim/netdevsim10/1: type eth netdev netdevsim10p2 flavour physical
> netdevsim/netdevsim10/2: type eth netdev netdevsim10p3 flavour physical
> netdevsim/netdevsim10/3: type eth netdev netdevsim10p4 flavour physical
> 
> Debugfs topology is also adjusted a bit. The rest stays the same as
> before.
> 
> TODO:
> - teach udev to rename netdevsim netdevices similarly to pci netdevices

So we can test udev as well?

> - fix tools/testing/selftests/bpf/test_offload.py to work with new iface

That'd step 0 :)

BTW are you testing all this with the various sysfs/kobject debug
checks?  I don't remember all the deets now, but there were certainly
ordering considerations coming from there.

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jiri Pirko]

Mon, Apr 15, 2019 at 09:27:09PM CEST, jakub.kicinski@netronome.com wrote:
>On Sat, 13 Apr 2019 18:20:57 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Currently the model of netdevsim is a bit odd in multiple ways.
>> 1) devlink instance is not in any way related with actual netdevsim
>>    netdevices. Instead, it is created per-namespace.
>> 2) multi-port netdevsim device is done using "link" attribute.
>> 3) netdevsim bus is there only to have something to bind the netdev to,
>>    it really does not act as a bus.
>
>Nope, it's there to expose SR-IOV ops :)

They are not on the bus, they are on the device.


>
>> 4) netdevsim instances are created by "ip link add" which is great for
>>    soft devices with no hw backend. The rtnl core allocates netdev and
>>    calls into driver holding rtnl mutex. For hw-backed devices, this
>>    flow is wrong as it breaks order in which things are done.
>> 
>> This patchset adjust netdevsim to fix all above.
>> 
>> In order to support proper devlink and devlink port instances and to be
>> able to emulate real devices, there is need to implement bus probe and
>> instantiate everything from there. User can specify device id and port
>> count to be instantianted. For example:
>> 
>> echo "10 4" > /sys/bus/netdevsim/new_device
>
>I really don't like the design where ID has to be allocated by user
>space.  It's a step back.

The reason is the then the address, devlink handles and netdev names are
predictable. If it would be auto-generated, how could you know what did
you just created?


>
>I also dislike declaring ports from the start.  In real drivers ports
>are never "atomically" registered, they are crated and destroyed one 
>by one, and a lot of races/UAFs/bugs lie in those small periods of
>time where one netdev got unregistered, but other are still around...

So what do you suggest?


>
>> Then devlink shows this:
>> 
>> $ devlink dev
>> netdevsim/netdevsim10
>> 
>> $ devlink port
>> netdevsim/netdevsim10/0: type eth netdev netdevsim10p1 flavour physical
>> netdevsim/netdevsim10/1: type eth netdev netdevsim10p2 flavour physical
>> netdevsim/netdevsim10/2: type eth netdev netdevsim10p3 flavour physical
>> netdevsim/netdevsim10/3: type eth netdev netdevsim10p4 flavour physical
>> 
>> Debugfs topology is also adjusted a bit. The rest stays the same as
>> before.
>> 
>> TODO:
>> - teach udev to rename netdevsim netdevices similarly to pci netdevices
>
>So we can test udev as well?

https://github.com/systemd/systemd/pull/12328


>
>> - fix tools/testing/selftests/bpf/test_offload.py to work with new iface
>
>That'd step 0 :)
>
>BTW are you testing all this with the various sysfs/kobject debug
>checks?  I don't remember all the deets now, but there were certainly
>ordering considerations coming from there.

Yes.

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jakub Kicinski]

On Tue, 16 Apr 2019 10:53:12 +0200, Jiri Pirko wrote:
> Mon, Apr 15, 2019 at 09:27:09PM CEST, jakub.kicinski@netronome.com wrote:
> >On Sat, 13 Apr 2019 18:20:57 +0200, Jiri Pirko wrote:  
> >> From: Jiri Pirko <jiri@mellanox.com>
> >> 
> >> Currently the model of netdevsim is a bit odd in multiple ways.
> >> 1) devlink instance is not in any way related with actual netdevsim
> >>    netdevices. Instead, it is created per-namespace.
> >> 2) multi-port netdevsim device is done using "link" attribute.
> >> 3) netdevsim bus is there only to have something to bind the netdev to,
> >>    it really does not act as a bus.  
> >
> >Nope, it's there to expose SR-IOV ops :)  
> 
> They are not on the bus, they are on the device.

Right, I'm just explaining why there is a device in the first place.

> >> 4) netdevsim instances are created by "ip link add" which is great for
> >>    soft devices with no hw backend. The rtnl core allocates netdev and
> >>    calls into driver holding rtnl mutex. For hw-backed devices, this
> >>    flow is wrong as it breaks order in which things are done.
> >> 
> >> This patchset adjust netdevsim to fix all above.
> >> 
> >> In order to support proper devlink and devlink port instances and to be
> >> able to emulate real devices, there is need to implement bus probe and
> >> instantiate everything from there. User can specify device id and port
> >> count to be instantianted. For example:
> >> 
> >> echo "10 4" > /sys/bus/netdevsim/new_device  
> >
> >I really don't like the design where ID has to be allocated by user
> >space.  It's a step back.  
> 
> The reason is the then the address, devlink handles and netdev names are
> predictable. If it would be auto-generated, how could you know what did
> you just created?
> 
> >I also dislike declaring ports from the start.  In real drivers ports
> >are never "atomically" registered, they are crated and destroyed one 
> >by one, and a lot of races/UAFs/bugs lie in those small periods of
> >time where one netdev got unregistered, but other are still around...  
> 
> So what do you suggest?

I haven't looked at this in detail.  One quick idea is perhaps to
remember what  ID was used after the write on the file descriptor, 
and then next read would return the ID?

> >> Then devlink shows this:
> >> 
> >> $ devlink dev
> >> netdevsim/netdevsim10
> >> 
> >> $ devlink port
> >> netdevsim/netdevsim10/0: type eth netdev netdevsim10p1 flavour physical
> >> netdevsim/netdevsim10/1: type eth netdev netdevsim10p2 flavour physical
> >> netdevsim/netdevsim10/2: type eth netdev netdevsim10p3 flavour physical
> >> netdevsim/netdevsim10/3: type eth netdev netdevsim10p4 flavour physical
> >> 
> >> Debugfs topology is also adjusted a bit. The rest stays the same as
> >> before.
> >> 
> >> TODO:
> >> - teach udev to rename netdevsim netdevices similarly to pci netdevices  
> >
> >So we can test udev as well?  
> 
> https://github.com/systemd/systemd/pull/12328
> 
> >> - fix tools/testing/selftests/bpf/test_offload.py to work with new iface  
> >
> >That'd step 0 :)
> >
> >BTW are you testing all this with the various sysfs/kobject debug
> >checks?  I don't remember all the deets now, but there were certainly
> >ordering considerations coming from there.  
> 
> Yes.

Cool :)

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jiri Pirko]

Mon, Apr 15, 2019 at 09:27:09PM CEST, jakub.kicinski@netronome.com wrote:
>On Sat, 13 Apr 2019 18:20:57 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Currently the model of netdevsim is a bit odd in multiple ways.
>> 1) devlink instance is not in any way related with actual netdevsim
>>    netdevices. Instead, it is created per-namespace.
>> 2) multi-port netdevsim device is done using "link" attribute.
>> 3) netdevsim bus is there only to have something to bind the netdev to,
>>    it really does not act as a bus.
>
>Nope, it's there to expose SR-IOV ops :)
>
>> 4) netdevsim instances are created by "ip link add" which is great for
>>    soft devices with no hw backend. The rtnl core allocates netdev and
>>    calls into driver holding rtnl mutex. For hw-backed devices, this
>>    flow is wrong as it breaks order in which things are done.
>> 
>> This patchset adjust netdevsim to fix all above.
>> 
>> In order to support proper devlink and devlink port instances and to be
>> able to emulate real devices, there is need to implement bus probe and
>> instantiate everything from there. User can specify device id and port
>> count to be instantianted. For example:
>> 
>> echo "10 4" > /sys/bus/netdevsim/new_device
>
>I really don't like the design where ID has to be allocated by user
>space.  It's a step back.
>
>I also dislike declaring ports from the start.  In real drivers ports
>are never "atomically" registered, they are crated and destroyed one 

Care to define "atomically" here? It is done in a very similar way
to how it is done in mlxsw for example. Same flows.


>by one, and a lot of races/UAFs/bugs lie in those small periods of
>time where one netdev got unregistered, but other are still around...

Same here. Not sure where do you see the differences.

Also, I plan to implement port splitting in follow-up patchset. All
flows are there as well.


>
>> Then devlink shows this:
>> 
>> $ devlink dev
>> netdevsim/netdevsim10
>> 
>> $ devlink port
>> netdevsim/netdevsim10/0: type eth netdev netdevsim10p1 flavour physical
>> netdevsim/netdevsim10/1: type eth netdev netdevsim10p2 flavour physical
>> netdevsim/netdevsim10/2: type eth netdev netdevsim10p3 flavour physical
>> netdevsim/netdevsim10/3: type eth netdev netdevsim10p4 flavour physical
>> 
>> Debugfs topology is also adjusted a bit. The rest stays the same as
>> before.
>> 
>> TODO:
>> - teach udev to rename netdevsim netdevices similarly to pci netdevices
>
>So we can test udev as well?
>
>> - fix tools/testing/selftests/bpf/test_offload.py to work with new iface
>
>That'd step 0 :)
>
>BTW are you testing all this with the various sysfs/kobject debug
>checks?  I don't remember all the deets now, but there were certainly
>ordering considerations coming from there.

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jakub Kicinski]

On Tue, 16 Apr 2019 10:59:37 +0200, Jiri Pirko wrote:
> >> 4) netdevsim instances are created by "ip link add" which is great for
> >>    soft devices with no hw backend. The rtnl core allocates netdev and
> >>    calls into driver holding rtnl mutex. For hw-backed devices, this
> >>    flow is wrong as it breaks order in which things are done.
> >> 
> >> This patchset adjust netdevsim to fix all above.
> >> 
> >> In order to support proper devlink and devlink port instances and to be
> >> able to emulate real devices, there is need to implement bus probe and
> >> instantiate everything from there. User can specify device id and port
> >> count to be instantianted. For example:
> >> 
> >> echo "10 4" > /sys/bus/netdevsim/new_device  
> >
> >I really don't like the design where ID has to be allocated by user
> >space.  It's a step back.
> >
> >I also dislike declaring ports from the start.  In real drivers ports
> >are never "atomically" registered, they are crated and destroyed one   
> 
> Care to define "atomically" here? It is done in a very similar way
> to how it is done in mlxsw for example. Same flows.
> 
> 
> >by one, and a lot of races/UAFs/bugs lie in those small periods of
> >time where one netdev got unregistered, but other are still around...  
> 
> Same here. Not sure where do you see the differences.

The difference is that today I can do this:

create a netdevsim1 with shared dev 1
create some state associated with shared dev 1
create a netdevsim2 with shared dev 1
check if all the shared dev 1 state created for netdevsim1 is visible
	via netdevsim2
destroy netdevsim1
check the shared dev 1 state again

If I say "give me 2 ports" from the start, that makes the testing
(which is the whole point of this code) harder.

> Also, I plan to implement port splitting in follow-up patchset. All
> flows are there as well.

Sure, let's just be clear that we won't be merging an ABI that has just
a netdevsim implementation, right?  I have some reservations about the
"port splitting" or device slicing, which should be discussed over real
code, not netdevsim.

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jiri Pirko]

Tue, Apr 16, 2019 at 08:04:59PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 16 Apr 2019 10:59:37 +0200, Jiri Pirko wrote:
>> >> 4) netdevsim instances are created by "ip link add" which is great for
>> >>    soft devices with no hw backend. The rtnl core allocates netdev and
>> >>    calls into driver holding rtnl mutex. For hw-backed devices, this
>> >>    flow is wrong as it breaks order in which things are done.
>> >> 
>> >> This patchset adjust netdevsim to fix all above.
>> >> 
>> >> In order to support proper devlink and devlink port instances and to be
>> >> able to emulate real devices, there is need to implement bus probe and
>> >> instantiate everything from there. User can specify device id and port
>> >> count to be instantianted. For example:
>> >> 
>> >> echo "10 4" > /sys/bus/netdevsim/new_device  
>> >
>> >I really don't like the design where ID has to be allocated by user
>> >space.  It's a step back.
>> >
>> >I also dislike declaring ports from the start.  In real drivers ports
>> >are never "atomically" registered, they are crated and destroyed one   
>> 
>> Care to define "atomically" here? It is done in a very similar way
>> to how it is done in mlxsw for example. Same flows.
>> 
>> 
>> >by one, and a lot of races/UAFs/bugs lie in those small periods of
>> >time where one netdev got unregistered, but other are still around...  
>> 
>> Same here. Not sure where do you see the differences.
>
>The difference is that today I can do this:
>
>create a netdevsim1 with shared dev 1
>create some state associated with shared dev 1
>create a netdevsim2 with shared dev 1
>check if all the shared dev 1 state created for netdevsim1 is visible
>	via netdevsim2

Hmm, you are testing netdevsim implementation then, not the kernel
interfaces. What is the point of testing netdevsim?


>destroy netdevsim1
>check the shared dev 1 state again
>
>If I say "give me 2 ports" from the start, that makes the testing
>(which is the whole point of this code) harder.
>
>> Also, I plan to implement port splitting in follow-up patchset. All
>> flows are there as well.
>
>Sure, let's just be clear that we won't be merging an ABI that has just
>a netdevsim implementation, right?  I have some reservations about the

So what do you suggest? Allow to somehow add and remove ports during
test? You can already do that with VFs. Do you want to do that with
netdevsim "physical" ports? If yes, how? I can imagine to extend devlink
port api with something like:

$ sudo devlink dev
netdevsim/netdevsim0
$ sudo devlink port
netdevsim/netdevsim0/0: type eth netdev eth0 flavour physical

$ sudo devlink dev port add netdevsim/netdevsim0 index 22
$ sudo devlink port
netdevsim/netdevsim0/0: type eth netdev eni0p1 flavour physical
netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical

$ sudo devlink port del netdevsim/netdevsim0/0
$ sudo devlink port
netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical

But I see only usecase for this extension for netdevsim, not for real
devices..


>"port splitting" or device slicing, which should be discussed over real
>code, not netdevsim.

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jakub Kicinski]

On Thu, 18 Apr 2019 09:22:56 +0200, Jiri Pirko wrote:
> Tue, Apr 16, 2019 at 08:04:59PM CEST, jakub.kicinski@netronome.com wrote:
> >On Tue, 16 Apr 2019 10:59:37 +0200, Jiri Pirko wrote:  
> >> >> 4) netdevsim instances are created by "ip link add" which is great for
> >> >>    soft devices with no hw backend. The rtnl core allocates netdev and
> >> >>    calls into driver holding rtnl mutex. For hw-backed devices, this
> >> >>    flow is wrong as it breaks order in which things are done.
> >> >> 
> >> >> This patchset adjust netdevsim to fix all above.
> >> >> 
> >> >> In order to support proper devlink and devlink port instances and to be
> >> >> able to emulate real devices, there is need to implement bus probe and
> >> >> instantiate everything from there. User can specify device id and port
> >> >> count to be instantianted. For example:
> >> >> 
> >> >> echo "10 4" > /sys/bus/netdevsim/new_device    
> >> >
> >> >I really don't like the design where ID has to be allocated by user
> >> >space.  It's a step back.
> >> >
> >> >I also dislike declaring ports from the start.  In real drivers ports
> >> >are never "atomically" registered, they are crated and destroyed one     
> >> 
> >> Care to define "atomically" here? It is done in a very similar way
> >> to how it is done in mlxsw for example. Same flows.
> >> 
> >>   
> >> >by one, and a lot of races/UAFs/bugs lie in those small periods of
> >> >time where one netdev got unregistered, but other are still around...    
> >> 
> >> Same here. Not sure where do you see the differences.  
> >
> >The difference is that today I can do this:
> >
> >create a netdevsim1 with shared dev 1
> >create some state associated with shared dev 1
> >create a netdevsim2 with shared dev 1
> >check if all the shared dev 1 state created for netdevsim1 is visible
> >	via netdevsim2  
> 
> Hmm, you are testing netdevsim implementation then, not the kernel
> interfaces. What is the point of testing netdevsim?

BPF offload tries to leave as much code as possible in the core, and
make the drivers simple.  I'm testing whether the core reacts correctly,
netdevsim just calls register/unregister.

> >destroy netdevsim1
> >check the shared dev 1 state again
> >
> >If I say "give me 2 ports" from the start, that makes the testing
> >(which is the whole point of this code) harder.
> >  
> >> Also, I plan to implement port splitting in follow-up patchset. All
> >> flows are there as well.  
> >
> >Sure, let's just be clear that we won't be merging an ABI that has just
> >a netdevsim implementation, right?  I have some reservations about the  
> 
> So what do you suggest? Allow to somehow add and remove ports during
> test? You can already do that with VFs. Do you want to do that with
> netdevsim "physical" ports? If yes, how? I can imagine to extend devlink
> port api with something like:
> 
> $ sudo devlink dev
> netdevsim/netdevsim0
> $ sudo devlink port
> netdevsim/netdevsim0/0: type eth netdev eth0 flavour physical
> 
> $ sudo devlink dev port add netdevsim/netdevsim0 index 22
> $ sudo devlink port
> netdevsim/netdevsim0/0: type eth netdev eni0p1 flavour physical
> netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical
> 
> $ sudo devlink port del netdevsim/netdevsim0/0
> $ sudo devlink port
> netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical
> 
> But I see only usecase for this extension for netdevsim, not for real
> devices..

Hm.. I'm getting lost, sorry, I'm probably confusing myself here..

Netdevsim is supposed to test real, existing kernel interfaces and core
code.  What we do today with linking based on netdevs is quite simple
and works very well for the BPF offload tests.

If you want to test some devlink code, that's also real, perfect.

For BPF tests we want the ability to add and remove netdevs to a sdev
during tests, yes.  That ability cannot be lost.

> >"port splitting" or device slicing, which should be discussed over real
> >code, not netdevsim.  

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jiri Pirko]

Thu, Apr 18, 2019 at 07:07:48PM CEST, jakub.kicinski@netronome.com wrote:
>On Thu, 18 Apr 2019 09:22:56 +0200, Jiri Pirko wrote:
>> Tue, Apr 16, 2019 at 08:04:59PM CEST, jakub.kicinski@netronome.com wrote:
>> >On Tue, 16 Apr 2019 10:59:37 +0200, Jiri Pirko wrote:  
>> >> >> 4) netdevsim instances are created by "ip link add" which is great for
>> >> >>    soft devices with no hw backend. The rtnl core allocates netdev and
>> >> >>    calls into driver holding rtnl mutex. For hw-backed devices, this
>> >> >>    flow is wrong as it breaks order in which things are done.
>> >> >> 
>> >> >> This patchset adjust netdevsim to fix all above.
>> >> >> 
>> >> >> In order to support proper devlink and devlink port instances and to be
>> >> >> able to emulate real devices, there is need to implement bus probe and
>> >> >> instantiate everything from there. User can specify device id and port
>> >> >> count to be instantianted. For example:
>> >> >> 
>> >> >> echo "10 4" > /sys/bus/netdevsim/new_device    
>> >> >
>> >> >I really don't like the design where ID has to be allocated by user
>> >> >space.  It's a step back.
>> >> >
>> >> >I also dislike declaring ports from the start.  In real drivers ports
>> >> >are never "atomically" registered, they are crated and destroyed one     
>> >> 
>> >> Care to define "atomically" here? It is done in a very similar way
>> >> to how it is done in mlxsw for example. Same flows.
>> >> 
>> >>   
>> >> >by one, and a lot of races/UAFs/bugs lie in those small periods of
>> >> >time where one netdev got unregistered, but other are still around...    
>> >> 
>> >> Same here. Not sure where do you see the differences.  
>> >
>> >The difference is that today I can do this:
>> >
>> >create a netdevsim1 with shared dev 1
>> >create some state associated with shared dev 1
>> >create a netdevsim2 with shared dev 1
>> >check if all the shared dev 1 state created for netdevsim1 is visible
>> >	via netdevsim2  
>> 
>> Hmm, you are testing netdevsim implementation then, not the kernel
>> interfaces. What is the point of testing netdevsim?
>
>BPF offload tries to leave as much code as possible in the core, and
>make the drivers simple.  I'm testing whether the core reacts correctly,
>netdevsim just calls register/unregister.
>
>> >destroy netdevsim1
>> >check the shared dev 1 state again
>> >
>> >If I say "give me 2 ports" from the start, that makes the testing
>> >(which is the whole point of this code) harder.
>> >  
>> >> Also, I plan to implement port splitting in follow-up patchset. All
>> >> flows are there as well.  
>> >
>> >Sure, let's just be clear that we won't be merging an ABI that has just
>> >a netdevsim implementation, right?  I have some reservations about the  
>> 
>> So what do you suggest? Allow to somehow add and remove ports during
>> test? You can already do that with VFs. Do you want to do that with
>> netdevsim "physical" ports? If yes, how? I can imagine to extend devlink
>> port api with something like:
>> 
>> $ sudo devlink dev
>> netdevsim/netdevsim0
>> $ sudo devlink port
>> netdevsim/netdevsim0/0: type eth netdev eth0 flavour physical
>> 
>> $ sudo devlink dev port add netdevsim/netdevsim0 index 22
>> $ sudo devlink port
>> netdevsim/netdevsim0/0: type eth netdev eni0p1 flavour physical
>> netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical
>> 
>> $ sudo devlink port del netdevsim/netdevsim0/0
>> $ sudo devlink port
>> netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical
>> 
>> But I see only usecase for this extension for netdevsim, not for real
>> devices..
>
>Hm.. I'm getting lost, sorry, I'm probably confusing myself here..
>
>Netdevsim is supposed to test real, existing kernel interfaces and core
>code.  What we do today with linking based on netdevs is quite simple
>and works very well for the BPF offload tests.
>
>If you want to test some devlink code, that's also real, perfect.
>
>For BPF tests we want the ability to add and remove netdevs to a sdev
>during tests, yes.  That ability cannot be lost.

Yeah, but since the api is changing, the original approach of using
IFLA_LINK cannot be used anymore. Not to mention is is abuse of the API
from the very beginning. You want to add/del ports during lifetime, I'm
trying to find interface. Basically whe have two options:
1) devlink extension I suggested above
2) sysfs files to add/del ports. Similar to "new_device" and
   "del_device".

I like the idea of having 2) better, the reason is this is very speficic
to netdevsim and not really applicable on real devices.


>
>
>> >"port splitting" or device slicing, which should be discussed over real
>> >code, not netdevsim.  

Re: [patch net-next rfc 00/15] netdevsim: impement proper device model

[Jakub Kicinski]

On Fri, 19 Apr 2019 07:25:01 +0200, Jiri Pirko wrote:
> >Hm.. I'm getting lost, sorry, I'm probably confusing myself here..
> >
> >Netdevsim is supposed to test real, existing kernel interfaces and core
> >code.  What we do today with linking based on netdevs is quite simple
> >and works very well for the BPF offload tests.
> >
> >If you want to test some devlink code, that's also real, perfect.
> >
> >For BPF tests we want the ability to add and remove netdevs to a sdev
> >during tests, yes.  That ability cannot be lost.  
> 
> Yeah, but since the api is changing, the original approach of using
> IFLA_LINK cannot be used anymore. Not to mention is is abuse of the API
> from the very beginning. You want to add/del ports during lifetime, I'm
> trying to find interface. Basically whe have two options:
> 1) devlink extension I suggested above
> 2) sysfs files to add/del ports. Similar to "new_device" and
>    "del_device".
> 
> I like the idea of having 2) better, the reason is this is very speficic
> to netdevsim and not really applicable on real devices.

Sure, if custom sysfs API is any better than the use of IFLA_LINK in a
test mock up, go for it.

Creating a custom devlink ABI for a test mockup netdev seems like a bad
idea.