netdev - Re: [PATCH net-next] net: ethtool: Don't check if RSS context exists in case of context 0

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: <20250227085608.5a3e32d7@kernel.org>
Date: Thu, 27 Feb 2025 08:56:08 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Edward Cree <ecree.xilinx@...il.com>
Cc: Gal Pressman <gal@...dia.com>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Andrew
 Lunn <andrew+netdev@...n.ch>, netdev@...r.kernel.org, Andrew Lunn
 <andrew@...n.ch>, Simon Horman <horms@...nel.org>, Joe Damato
 <jdamato@...tly.com>, Tariq Toukan <tariqt@...dia.com>
Subject: Re: [PATCH net-next] net: ethtool: Don't check if RSS context
 exists in case of context 0

On Thu, 27 Feb 2025 16:24:52 +0000 Edward Cree wrote:
> > I never uttered the thought that lead me to opposing. 
> > ctx 0 is a poor man's pass / accept. If someone needs a pass we should
> > add an explicit "action pass".  
> 
> To me 'pass' is just a shorthand for whatever specific behaviour
>  happens to match the default.  I.e. if you can already express
>  that behaviour directly, then 'pass' is a strictly nonorthogonal
>  addition and therefore bad interface design.

I presume sfc matches only one rule. For devices which terminate on
first hit and rules are ordered by ntuple ID, the following rules*:

 src-ip 192.168.0.2                 context 0
 src-ip 192.0.0.0   m 0.255.255.255 action -1

implement allowing only 192.168.0.2 out of the 192.0.0.0/8 subnet. 
The device may not even support RSS contexts.

I agree that pass is not very clean, but context 0 _is_ a pass.
It's not asking for context 0, it's asking for a nop rule which
prevents a more general rule from matching. To me at least.

* coin toss on the mask polarity

> But then, I'm the guy who thinks ed(1) is a good UI, so...
> 
> > Using ctx 0 as implicit pass is a very easy thing to miss
> > for driver developers.  
> 
> I don't see why drivers need anything special to handle "0 is
>  pass".  They need to handle "0 isn't in the xarray" (so map it
>  to the default HW context), which I suppose we ought to call out
>  explicitly in the struct ethtool_rxnfc kdoc if we're going to
>  continue allowing it, but it's not the fact that it's "an
>  implicit pass" that makes this necessary.  If there's something
>  I'm missing here please educate me :)

Nothing super specific, but it does produce corner cases.
We may find out if someone got it wrong once we have the test.