lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAACQVJqBYE47+bTkLLT6G-ax3JEwqrTMui38tJ+=_kXjVL5VsA@mail.gmail.com>
Date:   Tue, 11 Dec 2018 15:30:25 +0530
From:   Vasundhara Volam <vasundhara-v.volam@...adcom.com>
To:     Jiří Pírko <jiri@...nulli.us>
Cc:     David Miller <davem@...emloft.net>,
        "michael.chan@...adcom.com" <michael.chan@...adcom.com>,
        Jiri Pirko <jiri@...lanox.com>, jakub.kicinski@...ronome.com,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 RFC 1/8] devlink: Add devlink_param for port
 register and unregister

On Tue, Dec 11, 2018 at 2:39 PM Jiri Pirko <jiri@...nulli.us> wrote:
>
> 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(&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
Okay, but if you see whole patchset, more arguments(nearly 6) are
needed similar to
__devlink_params_unregister().
>
>
> >+
> > /**
> >  *    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

Powered by Openwall GNU/*/Linux Powered by OpenVZ