From bfcccfe78b361f5f6ef48554aed5bcd30c72f67f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 5 Nov 2019 13:26:11 -0800 Subject: [PATCH 1/2] netdevsim: drop code duplicated by a merge Looks like the port adding loop makes a re-appearance on net-next after net was merged back into it (even though it doesn't feature in the merge diff). The ports are already added in nsim_dev_create() so when we try to add them again get EEXIST, and see: netdevsim: probe of netdevsim0 failed with error -17 in the logs. When we remove the loop again the nsim_dev_probe() and nsim_dev_remove() become a wrapper of nsim_dev_create() and nsim_dev_destroy(). Remove this layer of indirection. Fixes: d31e95585ca6 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net") Signed-off-by: Jakub Kicinski Acked-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/netdevsim/dev.c | 47 +++++++------------------------------ 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index e59a8826f36de..3da96c7e82655 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -753,7 +753,7 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev, return err; } -static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev) +int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev) { struct nsim_dev *nsim_dev; struct devlink *devlink; @@ -761,7 +761,7 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev) devlink = devlink_alloc(&nsim_dev_devlink_ops, sizeof(*nsim_dev)); if (!devlink) - return ERR_PTR(-ENOMEM); + return -ENOMEM; devlink_net_set(devlink, nsim_bus_dev->initial_net); nsim_dev = devlink_priv(devlink); nsim_dev->nsim_bus_dev = nsim_bus_dev; @@ -773,6 +773,8 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev) nsim_dev->max_macs = NSIM_DEV_MAX_MACS_DEFAULT; nsim_dev->test1 = NSIM_DEV_TEST1_DEFAULT; + dev_set_drvdata(&nsim_bus_dev->dev, nsim_dev); + err = nsim_dev_resources_register(devlink); if (err) goto err_devlink_free; @@ -818,7 +820,7 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev) goto err_bpf_dev_exit; devlink_params_publish(devlink); - return nsim_dev; + return 0; err_bpf_dev_exit: nsim_bpf_dev_exit(nsim_dev); @@ -841,7 +843,7 @@ static struct nsim_dev *nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev) devlink_resources_unregister(devlink, NULL); err_devlink_free: devlink_free(devlink); - return ERR_PTR(err); + return err; } static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev) @@ -858,8 +860,9 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev) nsim_fib_destroy(devlink, nsim_dev->fib_data); } -static void nsim_dev_destroy(struct nsim_dev *nsim_dev) +void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev) { + struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev); struct devlink *devlink = priv_to_devlink(nsim_dev); nsim_dev_reload_destroy(nsim_dev); @@ -873,40 +876,6 @@ static void nsim_dev_destroy(struct nsim_dev *nsim_dev) devlink_free(devlink); } -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); - if (IS_ERR(nsim_dev)) - return PTR_ERR(nsim_dev); - dev_set_drvdata(&nsim_bus_dev->dev, nsim_dev); - - mutex_lock(&nsim_dev->port_list_lock); - for (i = 0; i < nsim_bus_dev->port_count; i++) { - err = __nsim_dev_port_add(nsim_dev, i); - if (err) - goto err_port_del_all; - } - mutex_unlock(&nsim_dev->port_list_lock); - return 0; - -err_port_del_all: - mutex_unlock(&nsim_dev->port_list_lock); - nsim_dev_port_del_all(nsim_dev); - 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); - - nsim_dev_destroy(nsim_dev); -} - static struct nsim_dev_port * __nsim_dev_port_lookup(struct nsim_dev *nsim_dev, unsigned int port_index) { From acceca8d2416bf866c5826240f9907cc91273f95 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 5 Nov 2019 13:26:12 -0800 Subject: [PATCH 2/2] selftests: bpf: log direct file writes Recent changes to netdevsim moved creating and destroying devices from netlink to sysfs. The sysfs writes have been implemented as direct writes, without shelling out. This is faster, but leaves no trace in the logs. Add explicit logs to make debugging possible. Signed-off-by: Jakub Kicinski Acked-by: Daniel Borkmann Signed-off-by: David S. Miller --- tools/testing/selftests/bpf/test_offload.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py index 1afa22c88e42a..8294ae3ffb3cb 100755 --- a/tools/testing/selftests/bpf/test_offload.py +++ b/tools/testing/selftests/bpf/test_offload.py @@ -335,13 +335,22 @@ class NetdevSimDev: """ Class for netdevsim bus device and its attributes. """ + @staticmethod + def ctrl_write(path, val): + fullpath = os.path.join("/sys/bus/netdevsim/", path) + try: + with open(fullpath, "w") as f: + f.write(val) + except OSError as e: + log("WRITE %s: %r" % (fullpath, val), -e.errno) + raise e + log("WRITE %s: %r" % (fullpath, val), 0) def __init__(self, port_count=1): addr = 0 while True: try: - with open("/sys/bus/netdevsim/new_device", "w") as f: - f.write("%u %u" % (addr, port_count)) + self.ctrl_write("new_device", "%u %u" % (addr, port_count)) except OSError as e: if e.errno == errno.ENOSPC: addr += 1 @@ -403,14 +412,13 @@ def dfs_get_bound_progs(self, expected): return progs def remove(self): - with open("/sys/bus/netdevsim/del_device", "w") as f: - f.write("%u" % self.addr) + self.ctrl_write("del_device", "%u" % (self.addr, )) devs.remove(self) def remove_nsim(self, nsim): self.nsims.remove(nsim) - with open("/sys/bus/netdevsim/devices/netdevsim%u/del_port" % self.addr ,"w") as f: - f.write("%u" % nsim.port_index) + self.ctrl_write("devices/netdevsim%u/del_port" % (self.addr, ), + "%u" % (nsim.port_index, )) class NetdevSim: """