| Message ID | 1544518008-15289-8-git-send-email-vasundhara-v.volam@broadcom.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | devlink: Add configuration parameters support for devlink_port | expand |
Tue, Dec 11, 2018 at 09:46:47AM CET, vasundhara-v.volam@broadcom.com 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, No need to put unused values to enum now. They should be added only when used. >+}; >+ > #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" >+#define DEVLINK_PARAM_GENERIC_WOL_TYPE DEVLINK_PARAM_TYPE_U8 >+ > #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate) \ > { \ > .id = DEVLINK_PARAM_GENERIC_ID_##_id, \ >diff --git a/net/core/devlink.c b/net/core/devlink.c >index 6bd7e13..cbdccb2 100644 >--- a/net/core/devlink.c >+++ b/net/core/devlink.c >@@ -2697,6 +2697,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info) > .name = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME, > .type = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE, > }, >+ { >+ .id = DEVLINK_PARAM_GENERIC_ID_WOL, >+ .name = DEVLINK_PARAM_GENERIC_WOL_NAME, >+ .type = DEVLINK_PARAM_GENERIC_WOL_TYPE, >+ }, > }; > > static int devlink_param_generic_verify(const struct devlink_param *param) >-- >1.8.3.1 >
On Tue, Dec 11, 2018 at 3:58 PM Jiri Pirko <jiri@resnulli.us> wrote: > > Tue, Dec 11, 2018 at 09:46:47AM CET, vasundhara-v.volam@broadcom.com 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, > > No need to put unused values to enum now. They should be added only when > used. Okay, I will remove them in next version. Thanks. > > > >+}; > >+ > > #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" > >+#define DEVLINK_PARAM_GENERIC_WOL_TYPE DEVLINK_PARAM_TYPE_U8 > >+ > > #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate) \ > > { \ > > .id = DEVLINK_PARAM_GENERIC_ID_##_id, \ > >diff --git a/net/core/devlink.c b/net/core/devlink.c > >index 6bd7e13..cbdccb2 100644 > >--- a/net/core/devlink.c > >+++ b/net/core/devlink.c > >@@ -2697,6 +2697,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info) > > .name = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME, > > .type = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE, > > }, > >+ { > >+ .id = DEVLINK_PARAM_GENERIC_ID_WOL, > >+ .name = DEVLINK_PARAM_GENERIC_WOL_NAME, > >+ .type = DEVLINK_PARAM_GENERIC_WOL_TYPE, > >+ }, > > }; > > > > static int devlink_param_generic_verify(const struct devlink_param *param) > >-- > >1.8.3.1 > >
On Tue, 11 Dec 2018 14:16:47 +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> As explained previously I think it's a very bad idea to add existing configuration options to devlink, just because devlink has the ability to persist the setting in NVM. Especially that for WoL you have to get the link up so you potentially have all link config stuff as well. And that n-tuple filters are one of the WoL options, meaning we'd need the ability to persist n-tuple filters via devlink. Your effort would be far better spent helping with migrating ethtool to netlink, and allowing persisting there. I have not heard any reason why devlink is a better fit. I can imagine you're just doing it here because it's less effort for you since ethtool is not yet migrated. So I'm opposed, let's call it a nack.
On Tue, Dec 11, 2018 at 10:18 AM Jakub Kicinski <jakub.kicinski@netronome.com> wrote: > > On Tue, 11 Dec 2018 14:16:47 +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> > > As explained previously I think it's a very bad idea to add existing > configuration options to devlink, just because devlink has the ability > to persist the setting in NVM. Especially that for WoL you have to get > the link up so you potentially have all link config stuff as well. And > that n-tuple filters are one of the WoL options, meaning we'd need the > ability to persist n-tuple filters via devlink. As I said before, firmware will automatically set the link to autoneg up to the speed supported by Vaux if WoL is enabled. No special link setting is needed as I said before. I don't think n-tuple is suitable for the default power-up WoL setting. n-tuple requires ip address. The ip address belongs to the system, not to the card that can move from system to system. The n-tuple WoL packet should be a transient setting set by the OS that won't persist a power down. The default power-up WoL packet types should be the most basic magic packet and other basic types. So I really don't think there is a need to persist n-tuple WoL packet types.
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. > +#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? Michal Kubecek
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, as other WOL types are also not added now. > > Michal Kubecek
On Tue, 11 Dec 2018 23:35:31 -0800, Michael Chan wrote: > On Tue, Dec 11, 2018 at 10:18 AM Jakub Kicinski wrote: > > On Tue, 11 Dec 2018 14:16:47 +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> > > > > As explained previously I think it's a very bad idea to add existing > > configuration options to devlink, just because devlink has the ability > > to persist the setting in NVM. Especially that for WoL you have to get > > the link up so you potentially have all link config stuff as well. And > > that n-tuple filters are one of the WoL options, meaning we'd need the > > ability to persist n-tuple filters via devlink. > > As I said before, firmware will automatically set the link to autoneg > up to the speed supported by Vaux if WoL is enabled. No special link > setting is needed as I said before. I don't think n-tuple is suitable > for the default power-up WoL setting. n-tuple requires ip address. > The ip address belongs to the system, not to the card that can move > from system to system. The n-tuple WoL packet should be a transient > setting set by the OS that won't persist a power down. The default > power-up WoL packet types should be the most basic magic packet and > other basic types. So I really don't think there is a need to persist > n-tuple WoL packet types. Sure your firmware sets the link to autoneg today, and your driver does not allow n-tuple filter WoL. How about we step back and think about the API as a whole rather than exposing ad hoc FW knobs via delink params :/ Also what gives you the idea that n-tuple filters require an ip address?
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
On Thu, Dec 13, 2018 at 6:33 PM Jiri Pirko <jiri@resnulli.us> wrote: > > 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. > > Okay, I will make this definition close to ethtool WOL types. Also rename according to ethtool. > >as other WOL types are also not added now. > >> > >> Michal Kubecek
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" +#define DEVLINK_PARAM_GENERIC_WOL_TYPE DEVLINK_PARAM_TYPE_U8 + #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate) \ { \ .id = DEVLINK_PARAM_GENERIC_ID_##_id, \ diff --git a/net/core/devlink.c b/net/core/devlink.c index 6bd7e13..cbdccb2 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -2697,6 +2697,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info) .name = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME, .type = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE, }, + { + .id = DEVLINK_PARAM_GENERIC_ID_WOL, + .name = DEVLINK_PARAM_GENERIC_WOL_NAME, + .type = DEVLINK_PARAM_GENERIC_WOL_TYPE, + }, }; static int devlink_param_generic_verify(const struct devlink_param *param)
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(+)