Thread (27 messages) 27 messages, 5 authors, 2019-02-06

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

From: Michal Kubecek <hidden>
Date: 2019-01-18 14:33:22

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help