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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190118143319.GG26670@unicorn.suse.cz>
Date:   Fri, 18 Jan 2019 15:33:19 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     netdev@...r.kernel.org
Cc:     Vasundhara Volam <vasundhara-v.volam@...adcom.com>,
        davem@...emloft.net, michael.chan@...adcom.com, jiri@...lanox.com,
        jakub.kicinski@...ronome.com
Subject: Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters
 support for devlink_port

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