netdev - Re: [PATCH net-next 1/2] ethtool: Introduce WAKE

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: <fa09c534-95f4-4ff3-973f-33914f4e4ee6@broadcom.com>
Date: Thu, 12 Oct 2023 14:02:06 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Florian Fainelli <f.fainelli@...il.com>, Michal Kubecek
 <mkubecek@...e.cz>, netdev@...r.kernel.org,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Jonathan Corbet <corbet@....net>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>,
 Heiner Kallweit <hkallweit1@...il.com>, Russell King
 <linux@...linux.org.uk>, opendmb@...il.com, justin.chen@...adcom.com
Subject: Re: [PATCH net-next 1/2] ethtool: Introduce WAKE_MDA

On 10/12/23 13:24, Andrew Lunn wrote:
>>> My previous concern was discoverability of the feature. Its not part
>>> of ethtool -s eth0 wol. At minimum, i would suggest something in the
>>> --help text in the wol section and man page pointing to the
>>> alternative way to configure wol. And maybe report via the standard
>>> wol flags that the hardware has the capability to use flow-type WoL?
>>
>> WAKE_FILTER is supposed to be set by the driver if it supports waking-up
>> from a network filter. That is how you would know that the device supports
>> waking-up from a network filter, and then you need to configure the filters
>> with ethtool -N (rxnfc).
>>
>> Where this API is a good fit is that you can specify a filter location and
>> the action (-2 = RX_CLS_FLOW_WAKE) to indicate where to install the filter
>> and what it should do. Where it may not be such a great fit is that it is a
>> two step process, where you need to make sure you install filter(s) plus
>> enable WAKE_FILTER from the .set_wol() call.
>>
>> At the time it was proposed it felt like a reasonable way to program,
>> without having "ethtool -s eth0 wol" gain a form of packet matching parser.
>> Also, it does not seem to me like we need the operations to be necessarily
>> atomic in a single call to the kernel but if we feel like this is too
>> difficult to use, we could consider a .set_wol() call that supports being
>> passed network filter(s).
>   
> I think two step is fine. I would say anybody using rxnfc is a pretty
> advanced user.
> 
> But we should clearly define what we expect in terms of ordering and
> maybe try to enforce it in the core. Can we make the rxnfc call return
> -EBUSY or something and an extack message if WAKE_FILTER has not been
> enabled first?

It might make sense to do it the other way around, that is you must 
install filters first and if none are installed by the time we enable 
WAKE_FILTER in .set_wol(), we error out with -EINVAL?
-- 
Florian

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)