Message-ID: <CAACQVJrX5QrF3QDKZFY40NJkW=d5z-C6TcOJG8Cekd8ykj4Vcg@mail.gmail.com>
Date:   Thu, 24 Jan 2019 15:12:59 +0530
From:   Vasundhara Volam <vasundhara-v.volam@...adcom.com>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        "michael.chan@...adcom.com" <michael.chan@...adcom.com>,
        Jiri Pirko <jiri@...lanox.com>,
        Jakub Kicinski <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 8:03 PM Michal Kubecek <mkubecek@...e.cz> 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.)
Agreed. This can be extended in future for other types of WoL.

>
> > 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.
This already supports as persistent or runtime WoL. All devlink parameters
have this option to be either persistent or runtime which is defined
by the driver
when defining the parameter.

>
> > 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).
Devlink supports both persistent and runtime WoL setting. It depends on
driver's use case.

>
> Michal Kubecek

Powered by blists - more mailing lists