netdev - Re: [RFC PATCH v2 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: <2122a8d2-9348-53ca-22f0-18f62109f1bb@gmail.com>
Date:   Fri, 14 Apr 2023 21:20:14 +0100
From:   Edward Cree <ecree.xilinx@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Jakub Kicinski <kuba@...nel.org>, edward.cree@....com,
        linux-net-drivers@....com, davem@...emloft.net, pabeni@...hat.com,
        edumazet@...gle.com, netdev@...r.kernel.org,
        habetsm.xilinx@...il.com, sudheer.mogilappagari@...el.com
Subject: Re: [RFC PATCH v2 net-next 6/7] net: ethtool: add a mutex protecting
 RSS contexts

On 13/04/2023 22:58, Andrew Lunn wrote:
>> (Idk, maybe sfc is just uniquely complex and messy.  It wouldn't be
>>  the first time.)
> 
> Hi Ed
> 
> Have you looked at other drivers? It would be bad to design an API
> around a messy driver.

I have; there's really not many that implement custom RSS contexts
 (just Marvell's mvpp2 and octeontx2, and Mellanox's mlx5).  The
 `rss_ctx_max_id` field is designed for those as they all have fixed-
 size arrays currently and idk whether that's a purely software limit
 or whether it reflects the hardware.
I couldn't find anything in any of them that looked like "restore
 stuff after a device reboot"; maybe it's just not something those
 devices expect to experience normally.

I don't know enough about mlx5 hw to really understand their filter
 code, but the rough equivalent of our efx_mcdi_filter_insert_locked()
 in that driver appears to be _mlx5_add_flow_rules(), which seems to
 be doing some kind of hand-over-hand locking.  And no sign (whether
 in comments or in asserts) of whether the function expects callers to
 hold RTNL.  Same goes for their functions operating on TIRs (whatever
 those are) which are called from all over (aRFS, tc, even kTLS!) in
 addition to the ethtool RSS/ntuple paths.

Anyway I'll cc maintainers of those drivers on v3 so they can chime
 in on the API design.  (Should've done that on v1 really, but I
 forgot.  Mea culpa.)

-ed