netdev - Re: [PATCH net-next v2 5/5] net: bcmgenet: Interrogate PHY for 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: <6456509b-9df7-47e3-b941-c307594a80d2@intel.com>
Date: Fri, 27 Oct 2023 09:55:04 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Florian Fainelli <florian.fainelli@...adcom.com>, <netdev@...r.kernel.org>
CC: Doug Berger <opendmb@...il.com>, Broadcom internal kernel review list
	<bcm-kernel-feedback-list@...adcom.com>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Andrew Lunn
	<andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, Russell King
	<linux@...linux.org.uk>, Vladimir Oltean <vladimir.oltean@....com>, "Tariq
 Toukan" <tariqt@...dia.com>, Gal Pressman <gal@...dia.com>, Willem de Bruijn
	<willemb@...gle.com>, Daniil Tatianin <d-tatianin@...dex-team.ru>, "Simon
 Horman" <horms@...nel.org>, Justin Chen <justin.chen@...adcom.com>, "Ratheesh
 Kannoth" <rkannoth@...vell.com>, Joe Damato <jdamato@...tly.com>, "Vincent
 Mailhol" <mailhol.vincent@...adoo.fr>, Jiri Pirko <jiri@...nulli.us>, "open
 list" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2 5/5] net: bcmgenet: Interrogate PHY for
 WAKE_FILTER programming



On 10/26/2023 4:52 PM, Florian Fainelli wrote:
> On 10/26/23 16:23, Jacob Keller wrote:
>>
>>
>> On 10/26/2023 3:45 PM, Florian Fainelli wrote:
>>> Determine whether the PHY can support waking up from the user programmed
>>> network filter, and if it can utilize it.
>>>
>>
>> Here, you're passing through to phy_ethtool_set_rxnfc, basically
>> allowing the lower device to program the wakeup filter if its supported. Ok.
>>
>> This almost feels like it would belong generally in the higher level
>> ethtool code rather than in the driver?
> 
> Agreed, as Doug just pointed out to me, there is still an open question 
> about reconciling the PHY and the MAC RXNFC spaces into a single 
> ethtool_rxnfc structure.
> 
> An ideal goal is to have zero modifications to neither the MAC or the 
> PHY drivers such that they can both work in their own spaces as if they 
> were alone, or combined.
> 
> I suppose that if we get the number of supported rules from the MAC 
> first, and then get the supported number of rules from the PHY next, we 
> could do something like this:
> 
> rule index
> | 0|
> | .| -> MAC rules
> |15|
> |16| -> PHY rule
> 
> and each of the MAC or the PHY {get,set}_rxnfc() operate within a base 
> rule number which is relative to their own space. So the MAC driver 
> would continue to care about its (max..first) - base (0) range, and the 
> PHY would care about (max..first) - base (16).
> 
> Though then the issue is discoverability, how do you know which rule 
> location is backed by which hardware block. We could create an 
> intermediate and inert rule at index 16 for instance that acts as a 
> delimiter?
> 
> Or we could create yet another RX_CLS_LOC_* value that is "special" and 
> can denote whether of the MAC or the PHY we should be targeting 
> whichever is supported, but that does not usually lend itself to being 
> logically ORed with the existing RX_CLS_LOC_* values. WDYT?
> 
> pw-bot: cr

Ah, yea there is a lot of complexity to consider here.

I'm not entirely sure what we should do here. What about extending with
another attribute entirely instead of another bit in RX_CLS_LOC?