diff mbox series

[net-next,v7,1/8] devlink: Add devlink_param for port register and unregister

Message ID 1547795385-12354-2-git-send-email-vasundhara-v.volam@broadcom.com
State Changes Requested
Delegated to: David Miller
Headers show
Series devlink: Add configuration parameters support for devlink_port | expand

Commit Message

Vasundhara Volam Jan. 18, 2019, 7:09 a.m. UTC 
Add functions to register and unregister for the driver supported
configuration parameters table per port.

v2->v3:
- Add a helper __devlink_params_register() with common code used by
  both devlink_params_register() and devlink_port_params_register().

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h |  22 +++++++++
 net/core/devlink.c    | 133 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 117 insertions(+), 38 deletions(-)

Comments

Jiri Pirko Jan. 23, 2019, 8:25 a.m. UTC | #1 
Fri, Jan 18, 2019 at 08:09:38AM CET, vasundhara-v.volam@broadcom.com wrote:
>Add functions to register and unregister for the driver supported
>configuration parameters table per port.
>
>v2->v3:
>- Add a helper __devlink_params_register() with common code used by
>  both devlink_params_register() and devlink_port_params_register().
>

[...]	
	
	
>+
>+static int __devlink_params_register(struct devlink *devlink,
>+				     struct list_head *param_list,
>+				     const struct devlink_param *params,
>+				     size_t params_count)
> {
> 	const struct devlink_param *param = params;
> 	int i;
>@@ -4490,20 +4493,11 @@ int devlink_params_register(struct devlink *devlink,
> 
> 	mutex_lock(&devlink->lock);
> 	for (i = 0; i < params_count; i++, param++) {
>-		if (!param || !param->name || !param->supported_cmodes) {
>-			err = -EINVAL;
>+		err = devlink_param_verify(param);
>+		if (err)
> 			goto rollback;
>-		}
>-		if (param->generic) {
>-			err = devlink_param_generic_verify(param);
>-			if (err)
>-				goto rollback;
>-		} else {
>-			err = devlink_param_driver_verify(param);
>-			if (err)
>-				goto rollback;
>-		}
>-		err = devlink_param_register_one(devlink, param);
>+
>+		err = devlink_param_register_one(devlink, param_list, param);
> 		if (err)
> 			goto rollback;
> 	}
>@@ -4515,31 +4509,57 @@ int devlink_params_register(struct devlink *devlink,
> 	if (!i)
> 		goto unlock;
> 	for (param--; i > 0; i--, param--)
>-		devlink_param_unregister_one(devlink, param);
>+		devlink_param_unregister_one(devlink, param_list, param);
> unlock:
> 	mutex_unlock(&devlink->lock);
> 	return err;
> }
>-EXPORT_SYMBOL_GPL(devlink_params_register);
> 
> /**
>- *	devlink_params_unregister - unregister configuration parameters
>+ *	devlink_params_register - register configuration parameters
>+ *
>  *	@devlink: devlink
>- *	@params: configuration parameters to unregister
>+ *	@params: configuration parameters array
>  *	@params_count: number of parameters provided
>+ *
>+ *	Register the configuration parameters supported by the driver.
>  */
>-void devlink_params_unregister(struct devlink *devlink,
>-			       const struct devlink_param *params,
>-			       size_t params_count)
>+int devlink_params_register(struct devlink *devlink,

The order of the functions should be:
__devlink_params_register
__devlink_params_unregister
devlink_params_register
devlink_params_unregister
devlink_port_params_register
devlink_port_params_unregister

Your order is a bit confusing.

Other than that:
Acked-by: Jiri Pirko <jiri@mellanox.com>


[...]
Vasundhara Volam Jan. 23, 2019, 9:13 a.m. UTC | #2 
On Wed, Jan 23, 2019 at 2:04 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Fri, Jan 18, 2019 at 08:09:38AM CET, vasundhara-v.volam@broadcom.com wrote:
> >Add functions to register and unregister for the driver supported
> >configuration parameters table per port.
> >
> >v2->v3:
> >- Add a helper __devlink_params_register() with common code used by
> >  both devlink_params_register() and devlink_port_params_register().
> >
>
> [...]
>
>
> >+
> >+static int __devlink_params_register(struct devlink *devlink,
> >+                                   struct list_head *param_list,
> >+                                   const struct devlink_param *params,
> >+                                   size_t params_count)
> > {
> >       const struct devlink_param *param = params;
> >       int i;
> >@@ -4490,20 +4493,11 @@ int devlink_params_register(struct devlink *devlink,
> >
> >       mutex_lock(&devlink->lock);
> >       for (i = 0; i < params_count; i++, param++) {
> >-              if (!param || !param->name || !param->supported_cmodes) {
> >-                      err = -EINVAL;
> >+              err = devlink_param_verify(param);
> >+              if (err)
> >                       goto rollback;
> >-              }
> >-              if (param->generic) {
> >-                      err = devlink_param_generic_verify(param);
> >-                      if (err)
> >-                              goto rollback;
> >-              } else {
> >-                      err = devlink_param_driver_verify(param);
> >-                      if (err)
> >-                              goto rollback;
> >-              }
> >-              err = devlink_param_register_one(devlink, param);
> >+
> >+              err = devlink_param_register_one(devlink, param_list, param);
> >               if (err)
> >                       goto rollback;
> >       }
> >@@ -4515,31 +4509,57 @@ int devlink_params_register(struct devlink *devlink,
> >       if (!i)
> >               goto unlock;
> >       for (param--; i > 0; i--, param--)
> >-              devlink_param_unregister_one(devlink, param);
> >+              devlink_param_unregister_one(devlink, param_list, param);
> > unlock:
> >       mutex_unlock(&devlink->lock);
> >       return err;
> > }
> >-EXPORT_SYMBOL_GPL(devlink_params_register);
> >
> > /**
> >- *    devlink_params_unregister - unregister configuration parameters
> >+ *    devlink_params_register - register configuration parameters
> >+ *
> >  *    @devlink: devlink
> >- *    @params: configuration parameters to unregister
> >+ *    @params: configuration parameters array
> >  *    @params_count: number of parameters provided
> >+ *
> >+ *    Register the configuration parameters supported by the driver.
> >  */
> >-void devlink_params_unregister(struct devlink *devlink,
> >-                             const struct devlink_param *params,
> >-                             size_t params_count)
> >+int devlink_params_register(struct devlink *devlink,
>
> The order of the functions should be:
> __devlink_params_register
> __devlink_params_unregister
> devlink_params_register
> devlink_params_unregister
> devlink_port_params_register
> devlink_port_params_unregister
>
> Your order is a bit confusing.
>
> Other than that:
> Acked-by: Jiri Pirko <jiri@mellanox.com>
Thank you Jiri. I will update the order in the next version.
>
>
> [...]
diff mbox series

Patch

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 67f4293..98b8a66 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -48,6 +48,7 @@  struct devlink_port_attrs {
 
 struct devlink_port {
 	struct list_head list;
+	struct list_head param_list;
 	struct devlink *devlink;
 	unsigned index;
 	bool registered;
@@ -574,6 +575,12 @@  int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 void devlink_param_value_changed(struct devlink *devlink, u32 param_id);
 void devlink_param_value_str_fill(union devlink_param_value *dst_val,
 				  const char *src);
+int devlink_port_params_register(struct devlink_port *devlink_port,
+				 const struct devlink_param *params,
+				 size_t params_count);
+void devlink_port_params_unregister(struct devlink_port *devlink_port,
+				    const struct devlink_param *params,
+				    size_t params_count);
 struct devlink_region *devlink_region_create(struct devlink *devlink,
 					     const char *region_name,
 					     u32 region_max_snapshots,
@@ -816,6 +823,21 @@  static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
 {
 }
 
+static inline int
+devlink_port_params_register(struct devlink_port *devlink_port,
+			     const struct devlink_param *params,
+			     size_t params_count)
+{
+	return 0;
+}
+
+static inline void
+devlink_port_params_unregister(struct devlink_port *devlink_port,
+			       const struct devlink_param *params,
+			       size_t params_count)
+{
+}
+
 static inline struct devlink_region *
 devlink_region_create(struct devlink *devlink,
 		      const char *region_name,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index abb0da9..41b60f6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3147,12 +3147,12 @@  static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
 }
 
 static int devlink_param_register_one(struct devlink *devlink,
+				      struct list_head *param_list,
 				      const struct devlink_param *param)
 {
 	struct devlink_param_item *param_item;
 
-	if (devlink_param_find_by_name(&devlink->param_list,
-				       param->name))
+	if (devlink_param_find_by_name(param_list, param->name))
 		return -EEXIST;
 
 	if (param->supported_cmodes == BIT(DEVLINK_PARAM_CMODE_DRIVERINIT))
@@ -3165,18 +3165,18 @@  static int devlink_param_register_one(struct devlink *devlink,
 		return -ENOMEM;
 	param_item->param = param;
 
-	list_add_tail(&param_item->list, &devlink->param_list);
+	list_add_tail(&param_item->list, param_list);
 	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_NEW);
 	return 0;
 }
 
 static void devlink_param_unregister_one(struct devlink *devlink,
+					 struct list_head *param_list,
 					 const struct devlink_param *param)
 {
 	struct devlink_param_item *param_item;
 
-	param_item = devlink_param_find_by_name(&devlink->param_list,
-						param->name);
+	param_item = devlink_param_find_by_name(param_list, param->name);
 	WARN_ON(!param_item);
 	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_DEL);
 	list_del(&param_item->list);
@@ -3954,6 +3954,7 @@  int devlink_port_register(struct devlink *devlink,
 	devlink_port->index = port_index;
 	devlink_port->registered = true;
 	list_add_tail(&devlink_port->list, &devlink->port_list);
+	INIT_LIST_HEAD(&devlink_port->param_list);
 	mutex_unlock(&devlink->lock);
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
 	return 0;
@@ -4471,18 +4472,20 @@  void devlink_resource_occ_get_unregister(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
 
-/**
- *	devlink_params_register - register configuration parameters
- *
- *	@devlink: devlink
- *	@params: configuration parameters array
- *	@params_count: number of parameters provided
- *
- *	Register the configuration parameters supported by the driver.
- */
-int devlink_params_register(struct devlink *devlink,
-			    const struct devlink_param *params,
-			    size_t params_count)
+static int devlink_param_verify(const struct devlink_param *param)
+{
+	if (!param || !param->name || !param->supported_cmodes)
+		return -EINVAL;
+	if (param->generic)
+		return devlink_param_generic_verify(param);
+	else
+		return devlink_param_driver_verify(param);
+}
+
+static int __devlink_params_register(struct devlink *devlink,
+				     struct list_head *param_list,
+				     const struct devlink_param *params,
+				     size_t params_count)
 {
 	const struct devlink_param *param = params;
 	int i;
@@ -4490,20 +4493,11 @@  int devlink_params_register(struct devlink *devlink,
 
 	mutex_lock(&devlink->lock);
 	for (i = 0; i < params_count; i++, param++) {
-		if (!param || !param->name || !param->supported_cmodes) {
-			err = -EINVAL;
+		err = devlink_param_verify(param);
+		if (err)
 			goto rollback;
-		}
-		if (param->generic) {
-			err = devlink_param_generic_verify(param);
-			if (err)
-				goto rollback;
-		} else {
-			err = devlink_param_driver_verify(param);
-			if (err)
-				goto rollback;
-		}
-		err = devlink_param_register_one(devlink, param);
+
+		err = devlink_param_register_one(devlink, param_list, param);
 		if (err)
 			goto rollback;
 	}
@@ -4515,31 +4509,57 @@  int devlink_params_register(struct devlink *devlink,
 	if (!i)
 		goto unlock;
 	for (param--; i > 0; i--, param--)
-		devlink_param_unregister_one(devlink, param);
+		devlink_param_unregister_one(devlink, param_list, param);
 unlock:
 	mutex_unlock(&devlink->lock);
 	return err;
 }
-EXPORT_SYMBOL_GPL(devlink_params_register);
 
 /**
- *	devlink_params_unregister - unregister configuration parameters
+ *	devlink_params_register - register configuration parameters
+ *
  *	@devlink: devlink
- *	@params: configuration parameters to unregister
+ *	@params: configuration parameters array
  *	@params_count: number of parameters provided
+ *
+ *	Register the configuration parameters supported by the driver.
  */
-void devlink_params_unregister(struct devlink *devlink,
-			       const struct devlink_param *params,
-			       size_t params_count)
+int devlink_params_register(struct devlink *devlink,
+			    const struct devlink_param *params,
+			    size_t params_count)
+{
+	return __devlink_params_register(devlink, &devlink->param_list, params,
+					 params_count);
+}
+EXPORT_SYMBOL_GPL(devlink_params_register);
+
+static void __devlink_params_unregister(struct devlink *devlink,
+					struct list_head *param_list,
+					const struct devlink_param *params,
+					size_t params_count)
 {
 	const struct devlink_param *param = params;
 	int i;
 
 	mutex_lock(&devlink->lock);
 	for (i = 0; i < params_count; i++, param++)
-		devlink_param_unregister_one(devlink, param);
+		devlink_param_unregister_one(devlink, param_list, param);
 	mutex_unlock(&devlink->lock);
 }
+
+/**
+ *	devlink_params_unregister - unregister configuration parameters
+ *	@devlink: devlink
+ *	@params: configuration parameters to unregister
+ *	@params_count: number of parameters provided
+ */
+void devlink_params_unregister(struct devlink *devlink,
+			       const struct devlink_param *params,
+			       size_t params_count)
+{
+	return __devlink_params_unregister(devlink, &devlink->param_list,
+					   params, params_count);
+}
 EXPORT_SYMBOL_GPL(devlink_params_unregister);
 
 /**
@@ -4657,6 +4677,43 @@  void devlink_param_value_str_fill(union devlink_param_value *dst_val,
 EXPORT_SYMBOL_GPL(devlink_param_value_str_fill);
 
 /**
+ *	devlink_port_params_register - register port configuration parameters
+ *
+ *	@devlink_port: devlink port
+ *	@params: configuration parameters array
+ *	@params_count: number of parameters provided
+ *
+ *	Register the configuration parameters supported by the port.
+ */
+int devlink_port_params_register(struct devlink_port *devlink_port,
+				 const struct devlink_param *params,
+				 size_t params_count)
+{
+	return __devlink_params_register(devlink_port->devlink,
+					 &devlink_port->param_list, params,
+					 params_count);
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_register);
+
+/**
+ *	devlink_port_params_unregister - unregister port configuration
+ *	parameters
+ *
+ *	@devlink_port: devlink port
+ *	@params: configuration parameters array
+ *	@params_count: number of parameters provided
+ */
+void devlink_port_params_unregister(struct devlink_port *devlink_port,
+				    const struct devlink_param *params,
+				    size_t params_count)
+{
+	return __devlink_params_unregister(devlink_port->devlink,
+					   &devlink_port->param_list,
+					   params, params_count);
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_unregister);
+
+/**
  *	devlink_region_create - create a new address region
  *
  *	@devlink: devlink