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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 4 Feb 2019 12:25:13 +0530
From:   Vasundhara Volam <vasundhara-v.volam@...adcom.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Michal Kubecek <mkubecek@...e.cz>, Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        "michael.chan@...adcom.com" <michael.chan@...adcom.com>,
        Jiri Pirko <jiri@...lanox.com>
Subject: Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters
 support for devlink_port

On Wed, Jan 23, 2019 at 3:48 AM Jakub Kicinski
<jakub.kicinski@...ronome.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