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
| ||
|
Message-ID: <ecd752db-ff2a-6948-2ff8-531343f80696@gmail.com> Date: Tue, 4 Apr 2023 13:14:39 +0100 From: Edward Cree <ecree.xilinx@...il.com> To: Jakub Kicinski <kuba@...nel.org>, edward.cree@....com Cc: 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 net-next 3/6] net: ethtool: let the core choose RSS context IDs On 03/04/2023 22:54, Jakub Kicinski wrote: > On Mon, 3 Apr 2023 17:33:00 +0100 edward.cree@....com wrote: >> int (*set_rxfh_context)(struct net_device *, const u32 *indir, >> const u8 *key, const u8 hfunc, >> - u32 *rss_context, bool delete); >> + u32 rss_context, bool delete); > > Would it be easier to pass struct ethtool_rxfh_context instead of > doing it field by field? Otherwise Intel will need to add more > arguments and touch all drivers. Or are you thinking that they should > use a separate callback for the "RR RSS" or whatever their thing is? Initially I tried to just pass in ctx with the new values already filled in. But that breaks if the op fails; we have to leave the old values in ctx. We maybe could create a second, ephemeral struct ethtool_rxfh_context to pass the new values in, but then we have to worry about which one's priv the driver uses. (We can't e.g. just pass in the ephemeral one, and copy its priv across when we update the real ctx after the op returns, because what if the driver stores, say, a list_head in its priv?) And if we did pass a struct wrapping indir, key and hfunc, then any patch adding more fields to it would need existing drivers to check the new fields were unused / set to NO_CHANGE. So I think we just have to accept that new fields will mean changing all drivers. (There's only half a dozen, anyway.) And doing that through the op arguments means the compiler will catch any driver that hasn't been updated, rather than the driver potentially silently ignoring the new field. > And maybe separate op for create / change / delete? Good idea, that would also elide renaming the legacy op. > And an extack on top... :) Sure.
Powered by blists - more mailing lists