netdev - Re: [PATCH v4 net-next 6/7] net: ethtool: add a mutex protecting RSS contexts

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2ea45188-5554-8067-820d-378cada735ee@gmail.com>
Date: Thu, 7 Dec 2023 14:15:30 +0000
From: Edward Cree <ecree.xilinx@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.net, edumazet@...gle.com,
 pabeni@...hat.com, netdev@...r.kernel.org, habetsm.xilinx@...il.com,
 sudheer.mogilappagari@...el.com, jdamato@...tly.com, andrew@...n.ch,
 mw@...ihalf.com, linux@...linux.org.uk, sgoutham@...vell.com,
 gakula@...vell.com, sbhatta@...vell.com, hkelam@...vell.com,
 saeedm@...dia.com, leon@...nel.org
Subject: Re: [PATCH v4 net-next 6/7] net: ethtool: add a mutex protecting RSS
 contexts

On 05/10/2023 00:16, Jakub Kicinski wrote:
> On Wed, 27 Sep 2023 19:13:37 +0100 edward.cree@....com wrote:
>> While this is not needed to serialise the ethtool entry points (which
>>  are all under RTNL), drivers may have cause to asynchronously access
>>  dev->ethtool->rss_ctx; taking dev->ethtool->rss_lock allows them to
>>  do this safely without needing to take the RTNL.
> 
> Can we use a replay mechanism, like we do in TC offloads and VxLAN/UDP
> ports? The driver which lost config can ask for the rss contexts to be
> "replayed" and the core will issue a series of ->create calls for all
> existing entries?
So I tried to prototype this, and unfortunately I ran into a problem.
While we can replay the contexts alright, that still leaves the ntuple
 filters which we also want to restore, and which might depend on the
 contexts, so that can't be done until after context restore is done.
So to do this we'd need to *also* have the core replay the filters,
 which would mean adding a filter array to the core similar to this
 context array.  Now that's a thing that might be useful to have,
 enabling netlink dumps and so on, but it would considerably extend
 the scope of this work, in which case who knows if it'll ever be
 ready to merge :S
Moreover, at least in the case of sfc (as usual, no idea about other
 NICs), the filter table on the device contains more than just ntuple
 filters; stuff like the device's unicast address list, PTP filters
 and representor filters, some of which are required for correct
 operation, live in the same table.
When coming up after a reset, currently we:
1) restore RSS contexts
2) restore all filters (both driver-internal and ethtool ntuple) from
   the software shadow filter table into the hardware
3) bring up the NIC datapath.
Instead we would need to:
1) restore all the 'internal' filters (which do not, and after these
   changes could not ever, use custom RSS contexts), and discard all
   ntuple filters from the software shadow filter table in the driver
2) request RSS+ntuple replay
3) bring up the NIC datapath
4) the replay workitem runs, and reinserts RSS contexts and ntuple
   filters.
This would also mean that the default RSS context, which is used by
 the unicast/multicast address and Ethernet broadcast filters, could
 not ever migrate to be tracked in the XArray (otherwise a desirable
 simplification).

tl;dr: none of this is impossible, but it'd be a lot of work just to
 get rid of one mutex, and would paint us into a bit of a corner.
So I propose to stick with the locking scheme for now; I'll post v5
 with the other review comments addressed; and if at some point in
 the future core tracking of ntuple filters gets added, we can
 revisit then whether moving to a replay scheme is viable.  Okay?

-ed