Message-ID: <a9027470-f4b4-483b-b0f3-88e17edaa7a1@gmail.com>
Date: Thu, 15 Aug 2024 09:38:48 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Maciej Żenczykowski <maze@...gle.com>,
 Doug Berger <opendmb@...il.com>, Justin Chen <justin.chen@...adcom.com>,
 Maciej Żenczykowski <zenczykowski@...il.com>,
 Linux Network Development Mailing List <netdev@...r.kernel.org>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 "Kory Maincent (Dent Project)" <kory.maincent@...tlin.com>,
 Ahmed Zaki <ahmed.zaki@...el.com>, Edward Cree <ecree.xilinx@...il.com>,
 Yuyang Huang <yuyanghuang@...gle.com>, Lorenzo Colitti <lorenzo@...gle.com>
Subject: Re: [PATCH net-next] ethtool: add tunable api to disable various
 firmware offloads

On 8/15/24 08:45, Jakub Kicinski wrote:
> On Wed, 14 Aug 2024 20:08:05 -0700 Florian Fainelli wrote:
>>> You gotta find an upstream driver which implements this for us to merge.
>>> If Florian doesn't have any quick uses -- I think Intel ethernet drivers
>>> have private flags for enabling/disabling an LLDP agent. That could be
>>> another way..
>>
>> Currently we have both bcmgenet and bcmasp support the WAKE_FILTER
>> Wake-on-LAN specifier. Our configuration is typically done in user-space
>> for mDNS with something like:
>>
>> ethtool -N eth0 flow-type ether dst 33:33:00:00:00:fb action
>> 0xfffffffffffffffe user-def 0x320000 m 0xffffffffff000fff
>> ethtool -N eth0 flow-type ether dst 01:00:5e:00:00:fb action
>> 0xfffffffffffffffe user-def 0x1e0000 m 0xffffffffff000fff
>> ethtool -s eth0 wol f
>>
>> I would offer that we wire up the tunable into bcmgenet and bcmasp and
>> we'd make sure on our side that the respective firmware implementations
>> behave accordingly, but the respective firmware implementations
>> currently look at whether any network filter have been programmed into
>> the hardware, and if so, they are using those for offload. So we do not
>> really need the tunable in a way, but if we were to add it, then we
>> would need to find a way to tell the firmware not to use the network
>> filters. We liked our design because there is no kernel <=> firmware
>> communication.
> 
> Hm, I may be lacking the practical exposure to such systems to say
> anything sensible, but when has that ever stopped me..
> IIUC you're saying that FW enables MLD offload if the wake filter is
> in place. But for ping/arp/igmp offload there's no need to wake, ever,
> so there wouldn't be a filter?

That is right, we only want to wake-up on mDNS in our case. There are 
two cases deployed, at least the first one is, the second one might have 
been more of a "to be added in the future" improvement:

- a simplistic one where we use the hardware filters to trigger a 
wake-up event, and then some piece of firmware will look at the mDNS 
query contents and figure out whether the query was for one of the 
services in the local database (typically _googlecast._tcp.local is of 
particular interest). If that is the case, we trigger a system wake-up 
and we let the Host CPU process the mDNS query and we stay awake for a 
few seconds in case a streaming operation happens

- a more sophisticated one where after the mDNS query wake-up event has 
been identified we wait until we get a 3-way TCP handshake targeting the 
_googlecast._tcp.local service before waking up the Host CPU. This is 
more reflective of an actual intent to use the device that was asleep

Hope this helps.
--
Florian

Powered by blists - more mailing lists