On Fri, 18 Jan 2019 15:33:19 +0100, Michal Kubecek wrote:
> On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> > There is difference of opinion on adding WOL parameter to devlink, between
> > Jakub Kicinski and Michael Chan.
> >
> > Quote from Jakud Kicinski:
> > ********
> > 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.
> >
> > The 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.
> > ********
> >
> > Quote from Michael Chan:
> > ********
> > The devlink's WoL parameter is a persistent WoL parameter stored in the
> > NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> > ways. ethtool WoL is not persistent over AC power cycle and is considered
> > OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> > including n-tuple with IP address in addition to the more basic types
> > (e.g. magic packet). Whereas OS-absent power up WoL should only include
> > magic packet and other simple types.
>
> If I understand correctly, it's that way now. I'm not sure there is a
> technical reason preventing more complex WoL types in the OS-absent case
> in the future. Also, even with traditional ethtool WoL setting, most
> NICs only support some of the types (I'm not sure if there is a NIC
> which would support all of them.)
>
> > The devlink WoL setting does not have to match the ethtool WoL
> > setting.
>
> IMHO this is not really a problem. We can either use an additional flag
> telling kernel/driver if we are setting runtime or persistent WoL mask
> or we can pass (up to) two bitmaps.
I think reusing new netlink ethtool with a special flag would be a nice,
complete solution. We could also address link settings this way (which
are a pre-requisite for WoL).
I have no strong preference on the mechanism, but for ease of setting
as well as dumping would it be workable to use a nesting, like this:
Run time settings:
[ETHTOOL_SETTINGS_BLA]
[ETHTOOL_BLA_VAL_1]
[ETHTOOL_BLA_VAL_2]
...
Persistent:
[ETHTOOL_PERSISTENT]
[ETHTOOL_SETTINGS_BLA]
[ETHTOOL_BLA_VAL_1]
[ETHTOOL_BLA_VAL_2]
IOW encapsulate settings into a "persistent" attribute?
How does that look to you, Michal?
> > The card will autoneg up to the speed supported by Vaux so no special
> > devlink link setting is needed.
> > ********
>
> Like Jakub, I'm not convinced there is a strong technical reason to have
> each of the WoL settings handled through a different interface. I don't
> say, though, that ethtool is necessarily the right one. If there is
> a consensus that it better fits into devlink, I can imagine that both
> could be accessible through devlink (for start, in drivers which choose
> so, e.g. because they want to implement the persistent setting).
>
> Michal Kubecek
