On Sat, Oct 21, 2017 at 5:24 AM, Jiri Pirko <j...@resnulli.us> wrote:
> Fri, Oct 20, 2017 at 05:13:39PM CEST, steven.l...@broadcom.com wrote:
>>On Fri, Oct 20, 2017 at 10:39 AM, Jiri Pirko <j...@resnulli.us> wrote:
>>> Thu, Oct 19, 2017 at 09:17:05PM CEST, steven.l...@broadcom.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.l...@broadcom.com>
>>>>Acked-by: Andy Gospodarek <go...@broadcom.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.
