netdev - Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations

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]

Date:   Mon, 23 Oct 2017 10:05:34 -0400
From:   Steve Lin <steven.lin1@...adcom.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Linux Netdev List <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...lanox.com>,
        "David S . Miller" <davem@...emloft.net>,
        Michael Chan <michael.chan@...adcom.com>,
        John Linville <linville@...driver.com>,
        Andy Gospodarek <gospo@...adcom.com>
Subject: Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter
 get/set operations

On Sat, Oct 21, 2017 at 5:24 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> Fri, Oct 20, 2017 at 05:13:39PM CEST, steven.lin1@...adcom.com wrote:
>>On Fri, Oct 20, 2017 at 10:39 AM, Jiri Pirko <jiri@...nulli.us> wrote:
>>> Thu, Oct 19, 2017 at 09:17:05PM CEST, steven.lin1@...adcom.com wrote:
>>>>Add support for permanent config parameter get/set commands. Used
>>>>for parameters held in NVRAM, persistent device configuration.
>>>>
>>>>Signed-off-by: Steve Lin <steven.lin1@...adcom.com>
>>>>Acked-by: Andy Gospodarek <gospo@...adcom.com>
>>>>---
>>>> include/net/devlink.h        |   3 +
>>>> include/uapi/linux/devlink.h |  11 ++
>>>> net/core/devlink.c           | 234 +++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 248 insertions(+)
>>>>
>>>>diff --git a/include/net/devlink.h b/include/net/devlink.h
>>>>index b9654e1..bd64623 100644
>>>>--- a/include/net/devlink.h
>>>>+++ b/include/net/devlink.h
>>>>@@ -270,6 +270,9 @@ struct devlink_ops {
>>>>       int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
>>>>       int (*eswitch_encap_mode_get)(struct devlink *devlink, u8 *p_encap_mode);
>>>>       int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>>>>+      int (*perm_config_get)(struct devlink *devlink, u32 param, u32 *value);
>>>>+      int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
>>>
>>> Please use enum instead of "u32 param". Also, what would happen if the
>>> value is >u32, like string for example? I believe we need to take it into
>>> the consideration for the UAPI sake.
>>>
>>>
>>
>>Using enum instead of u32 param: ok, will do in v3, thanks.
>>
>>Value > u32:  In the RFC and v1 versions of the patch, each parameter
>>was its own attribute, so could have its own type (u32, string,
>>whatever).  In v2, trying to move to nested parameters w/ parameter
>>being an enum, as requested, it seems to mean that the parameter value
>>now must be defined as a specific type, so I went with u32.
>
> Why? I have to be missing something. In the nest all is same as outside
> of the nest.
>
> Also, please see team_nl_cmd_options_set() where something similar is
> done, for multiple option types.
>
>

Okay, I can implement similar to that driver.  In the current
devlink.c/devlink.h, all attributes have specific types, and aren't
dynamic types like the "data" attribute in the team driver, so I had
thought having specific types defined for each attribute was required
in devlink.