Re: [PATCH net-next v2 RFC 7/8] devlink: Add a generic port parameter

[Jiri Pirko <jiri@resnulli.us>]

Wed, Dec 12, 2018 at 01:30:23PM CET, vasundhara-v.volam@broadcom.com wrote:
>On Wed, Dec 12, 2018 at 2:45 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>>
>> On Tue, Dec 11, 2018 at 02:16:47PM +0530, Vasundhara Volam wrote:
>> > wake-on-lan - Enables Wake on Lan for this port. If enabled,
>> > the controller asserts a wake pin based on the wake-on-lan type.
>> >
>> > Cc: Jiri Pirko <jiri@mellanox.com>
>> > Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>> > ---
>> >  include/net/devlink.h | 16 ++++++++++++++++
>> >  net/core/devlink.c    |  5 +++++
>> >  2 files changed, 21 insertions(+)
>> >
>> > diff --git a/include/net/devlink.h b/include/net/devlink.h
>> > index db413d3..93a1030 100644
>> > --- a/include/net/devlink.h
>> > +++ b/include/net/devlink.h
>> > @@ -367,12 +367,25 @@ enum devlink_param_generic_id {
>> >       DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
>> >       DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
>> >       DEVLINK_PARAM_GENERIC_ID_FW_LOAD_POLICY,
>> > +     DEVLINK_PARAM_GENERIC_ID_WOL,
>> >
>> >       /* add new param generic ids above here*/
>> >       __DEVLINK_PARAM_GENERIC_ID_MAX,
>> >       DEVLINK_PARAM_GENERIC_ID_MAX = __DEVLINK_PARAM_GENERIC_ID_MAX - 1,
>> >  };
>> >
>> > +enum devlink_param_wol_types {
>> > +     DEVLINK_PARAM_WOL_DISABLE = 0,
>> > +     DEVLINK_PARAM_WOL_MAGIC_PKT,
>> > +     DEVLINK_PARAM_WOL_PHY,
>> > +     DEVLINK_PARAM_WOL_UNICAST,
>> > +     DEVLINK_PARAM_WOL_MULTICAST,
>> > +     DEVLINK_PARAM_WOL_BROADCAST,
>> > +     DEVLINK_PARAM_WOL_ARP,
>> > +     DEVLINK_PARAM_WOL_SECURE,
>> > +     DEVLINK_PARAM_WOL_FILTERS,
>> > +};
>> > +
>> >  #define DEVLINK_PARAM_GENERIC_INT_ERR_RESET_NAME "internal_error_reset"
>> >  #define DEVLINK_PARAM_GENERIC_INT_ERR_RESET_TYPE DEVLINK_PARAM_TYPE_BOOL
>> >
>> > @@ -397,6 +410,9 @@ enum devlink_param_generic_id {
>> >  #define DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME "fw_load_policy"
>> >  #define DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE DEVLINK_PARAM_TYPE_U8
>> >
>> > +#define DEVLINK_PARAM_GENERIC_WOL_NAME "wake-on-lan"
>>
>> Nitpick: the other two parameter names use underscores, it would be more
>> consistent to use them as well.
>Sure, I will fix this in next version. Thanks.
>>
>> > +#define DEVLINK_PARAM_GENERIC_WOL_TYPE DEVLINK_PARAM_TYPE_U8
>>
>> Can you be sure it will be always sufficient to enable only one WoL type
>> unlike in ethtool where bitfield is used so that you can enable multiple
>> types?
>Since this is a persistent setting, NIC may not support multiple types
>of WOL upon power on.
>If there is any requirement in future to add multiple types, data type
>can be modified,

It can't. It will become a UAPI. You need to prepare now.


>as other WOL types are also not added now.
>>
>> Michal Kubecek