Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Michal Kubecek <mkubecek@suse.cz>]

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.

> 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