Patchwork [net-next,v2,08/16] netdevsim: add bus attributes to add new and delete devices

login
register
mail settings
Submitter Jiri Pirko
Date April 20, 2019, 10:29 a.m.
Message ID <20190420102922.6001-9-jiri@resnulli.us>
Download mbox | patch
Permalink /patch/777621/
State New
Headers show

Comments

Jiri Pirko - April 20, 2019, 10:29 a.m.
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(-)
Jakub Kicinski - April 22, 2019, 7:23 p.m.
On Sat, 20 Apr 2019 12:29:14 +0200, Jiri Pirko wrote:
> +static ssize_t del_device_store(struct bus_type *bus, const char *buf,
> +				size_t count)

Please use the existing format of breaking  multiline function
declaration, if retype is longer than 3 chars it should be on a
separate line:

type
name(arg, arg, ..
     arg, arg)

Please fix everywhere.
Jakub Kicinski - April 22, 2019, 7:23 p.m.
On Sat, 20 Apr 2019 12:29:14 +0200, Jiri Pirko wrote:
> -	err = ida_alloc(&nsim_bus_dev_ids, GFP_KERNEL);
> +	err = ida_alloc_range(&nsim_bus_dev_ids,
> +			      id == ~0 ? 0 : id, id, GFP_KERNEL);

#define for ~0 ?
Jiri Pirko - April 23, 2019, 6:29 a.m.
Mon, Apr 22, 2019 at 09:23:02PM CEST, jakub.kicinski@netronome.com wrote:
>On Sat, 20 Apr 2019 12:29:14 +0200, Jiri Pirko wrote:
>> +static ssize_t del_device_store(struct bus_type *bus, const char *buf,
>> +				size_t count)
>
>Please use the existing format of breaking  multiline function
>declaration, if retype is longer than 3 chars it should be on a
>separate line:

Where is it written down? First time I hear about such rule.


>
>type
>name(arg, arg, ..
>     arg, arg)
>
>Please fix everywhere.
Jiri Pirko - April 23, 2019, 6:31 a.m.
Mon, Apr 22, 2019 at 09:23:38PM CEST, jakub.kicinski@netronome.com wrote:
>On Sat, 20 Apr 2019 12:29:14 +0200, Jiri Pirko wrote:
>> -	err = ida_alloc(&nsim_bus_dev_ids, GFP_KERNEL);
>> +	err = ida_alloc_range(&nsim_bus_dev_ids,
>> +			      id == ~0 ? 0 : id, id, GFP_KERNEL);
>
>#define for ~0 ?

Sure, but this is remove in patch 15. Is it necessary?
Jakub Kicinski - April 23, 2019, 5:06 p.m.
On Tue, 23 Apr 2019 08:29:23 +0200, Jiri Pirko wrote:
> Mon, Apr 22, 2019 at 09:23:02PM CEST, jakub.kicinski@netronome.com wrote:
> >On Sat, 20 Apr 2019 12:29:14 +0200, Jiri Pirko wrote:  
> >> +static ssize_t del_device_store(struct bus_type *bus, const char *buf,
> >> +				size_t count)  
> >
> >Please use the existing format of breaking  multiline function
> >declaration, if retype is longer than 3 chars it should be on a
> >separate line:  
> 
> Where is it written down? First time I hear about such rule.

That's sort of my rule, for code I maintain.  The general rule of 
the thumb is follow the formatting of the existing code..
Jakub Kicinski - April 23, 2019, 5:08 p.m.
On Tue, 23 Apr 2019 08:31:08 +0200, Jiri Pirko wrote:
> Mon, Apr 22, 2019 at 09:23:38PM CEST, jakub.kicinski@netronome.com wrote:
> >On Sat, 20 Apr 2019 12:29:14 +0200, Jiri Pirko wrote:  
> >> -	err = ida_alloc(&nsim_bus_dev_ids, GFP_KERNEL);
> >> +	err = ida_alloc_range(&nsim_bus_dev_ids,
> >> +			      id == ~0 ? 0 : id, id, GFP_KERNEL);  
> >
> >#define for ~0 ?  
> 
> Sure, but this is remove in patch 15. Is it necessary?

Oh, I missed that.
Jiri Pirko - April 23, 2019, 7:09 p.m.
Tue, Apr 23, 2019 at 07:06:42PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 23 Apr 2019 08:29:23 +0200, Jiri Pirko wrote:
>> Mon, Apr 22, 2019 at 09:23:02PM CEST, jakub.kicinski@netronome.com wrote:
>> >On Sat, 20 Apr 2019 12:29:14 +0200, Jiri Pirko wrote:  
>> >> +static ssize_t del_device_store(struct bus_type *bus, const char *buf,
>> >> +				size_t count)  
>> >
>> >Please use the existing format of breaking  multiline function
>> >declaration, if retype is longer than 3 chars it should be on a
>> >separate line:  
>> 
>> Where is it written down? First time I hear about such rule.
>
>That's sort of my rule, for code I maintain.  The general rule of 
>the thumb is follow the formatting of the existing code..

Okay. Will do :)

Patch

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);