[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181211090225.GA2185@nanopsycho.orion>
Date: Tue, 11 Dec 2018 10:02:25 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Vasundhara Volam <vasundhara-v.volam@...adcom.com>
Cc: davem@...emloft.net, michael.chan@...adcom.com, jiri@...lanox.com,
jakub.kicinski@...ronome.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 RFC 1/8] devlink: Add devlink_param for port
register and unregister
Tue, Dec 11, 2018 at 09:46:41AM CET, vasundhara-v.volam@...adcom.com wrote:
>Add functions to register and unregister for the driver supported
>configuration parameters table per port.
>
>Cc: Jiri Pirko <jiri@...lanox.com>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@...adcom.com>
>---
> 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(¶m_item->list, &devlink->param_list);
>+ list_add_tail(¶m_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(¶m_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
>+
> /**
> * 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
>
Powered by blists - more mailing lists