linux-kernel - Re: [PATCH net-next v4 2/8] net: ethtool: netlink: Allow per-netdevice DUMP operations

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] [day] [month] [year] [list]

Message-ID: <20250326122627.600bec6b@fedora.home>
Date: Wed, 26 Mar 2025 12:26:27 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net, Andrew Lunn
 <andrew@...n.ch>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
 <pabeni@...hat.com>, Heiner Kallweit <hkallweit1@...il.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 thomas.petazzoni@...tlin.com, linux-arm-kernel@...ts.infradead.org,
 Christophe Leroy <christophe.leroy@...roup.eu>, Herve Codina
 <herve.codina@...tlin.com>, Florian Fainelli <f.fainelli@...il.com>,
 Russell King <linux@...linux.org.uk>, Vladimir Oltean
 <vladimir.oltean@....com>, Oleksij Rempel <o.rempel@...gutronix.de>, Simon
 Horman <horms@...nel.org>, Romain Gantois <romain.gantois@...tlin.com>,
 Piergiorgio Beruto <piergiorgio.beruto@...il.com>
Subject: Re: [PATCH net-next v4 2/8] net: ethtool: netlink: Allow
 per-netdevice DUMP operations

On Wed, 26 Mar 2025 11:29:36 +0100
Kory Maincent <kory.maincent@...tlin.com> wrote:

> On Wed, 26 Mar 2025 08:59:06 +0100
> Maxime Chevallier <maxime.chevallier@...tlin.com> wrote:
> 
> > On Tue, 25 Mar 2025 14:22:02 -0700
> > Jakub Kicinski <kuba@...nel.org> wrote:
> >   
> > > On Tue, 25 Mar 2025 14:15:07 -0700 Jakub Kicinski wrote:    
> >  [...]    
> > > > 
> > > > Let's try. We can probably make required_dev attr of
> > > > ethnl_parse_header_dev_get() a three state one: require, allow, reject?
> > > >    
> > > 
> > > Ah, don't think this is going to work. You're not converting all 
> > > the dumps, just the PHY ones. It's fine either way, then.    
> > 
> > Yeah I noticed that when implementing, but I actually forgot to mention
> > in in my cover, which I definitely should have :(
> > 
> > What we can also do is properly support multi-phy dump but not filtered
> > dump on all the existing phy commands (plca, pse-pd, etc.) so that be
> > behaviour is unchanged for these. Only PHY_GET and any future per-phy
> > commands would support it.  
> 
> Couldn't we remove the existence check of ctx->req_info->dev in
> ethnl_default_start and add the netdev_put() in the ethnl_default_dumpit()?
> Would this work?

It would work, but it seems unnecessary to hold a refcount on a
specific netdev while we iterate on all netdevs in the namespace
afterwards. But I'd say that's an implementation detail :)

> Or we could keep your change and let the userspace adapt to the new support of
> filtered dump. In fact you are modifying all the ethtool commands that are
> already related to PHY, if you don't they surely will one day or another so it
> is good to keep it.

So the question is, is it OK to stop ignoring the ifindex/ifname header
attribute for ethnl dump requests, and if so, should we do that for all
dump commands or only the ones for which is makes sense to do so.

Jakub says "t's fine either way, then.", but don't fully get if this is
an answer to the above question :)

This will only change the behaviour of filtered dumps, that aren't
really used with ethnl (except for PHY_GET but we won't change its
behaviour either way)

Maxime