On Wed, Jan 23, 2019 at 3:48 AM Jakub Kicinski <jakub.kicin...@netronome.com> wrote: > > 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. Jakub, I will add another two bitmask parameters for WoL named as wake_on_lan_runtime and wake_on_lan_persistent. This will give information about what types are runtime and what types are persistent, does the device support for any given WoL types.
Does that sound good? > > 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
