On Tue, Dec 11, 2018 at 2:39 PM Jiri Pirko [off-list ref] wrote:

Tue, Dec 11, 2018 at 09:46:41AM CET, vasundhara-v.volam@broadcom.com wrote:

quoted

Add functions to register and unregister for the driver supported
configuration parameters table per port.

Cc: Jiri Pirko <redacted>
Signed-off-by: Vasundhara Volam <redacted>
---
include/net/devlink.h |  22 +++++++++
net/core/devlink.c    | 128 ++++++++++++++++++++++++++++++++++++++++----------
2 files changed, 124 insertions(+), 26 deletions(-)

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..da7f6bd 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,6 +4472,16 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,

}
EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);

+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);
+}
+
/**
 *    devlink_params_register - register configuration parameters
 *

@@ -4490,20 +4501,12 @@ 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, &devlink->param_list,
+                                               param);
              if (err)
                      goto rollback;
      }

@@ -4515,13 +4518,28 @@ 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, &devlink->param_list,
+                                           param);

unlock:
      mutex_unlock(&devlink->lock);
      return err;
}
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_list, param);
+      mutex_unlock(&devlink->lock);
+}

Please do devlink_params_register in a similar way. Have
__devlink_params_register() with list_head * arg

This cannot be optimised as you see the whole patch-set, unregister
also requires different command in case of failure of
param_register_one and sending 2 types of command as argument will not
simplify this code. It is better this way, as you see the final
function.

quoted

+
/**
 *    devlink_params_unregister - unregister configuration parameters
 *    @devlink: devlink

@@ -4532,13 +4550,8 @@ void devlink_params_unregister(struct devlink *devlink,
                             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);
-      mutex_unlock(&devlink->lock);
+      return __devlink_params_unregister(devlink, &devlink->param_list,
+                                         params, params_count);
}
EXPORT_SYMBOL_GPL(devlink_params_unregister);

@@ -4657,6 +4670,69 @@ 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)
+{
+      struct devlink *devlink = devlink_port->devlink;
+      const struct devlink_param *param = params;
+      int i, err;
+
+      mutex_lock(&devlink->lock);
+      for (i = 0; i < params_count; i++) {
+              err = devlink_param_verify(param);
+              if (err)
+                      goto rollback;
+
+              err = devlink_param_register_one(devlink,
+                                               &devlink_port->param_list,
+                                               param);
+              if (err)
+                      goto rollback;
+      }
+
+      mutex_unlock(&devlink->lock);
+      return 0;
+
+rollback:
+      if (!i)
+              goto unlock;
+      for (param--; i > 0; i--, param--)
+              devlink_param_unregister_one(devlink, &devlink_port->param_list,
+                                           param);
+unlock:
+      mutex_unlock(&devlink->lock);
+      return err;
+}
+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
--
1.8.3.1