Netdev List
 help / color / mirror / Atom feed
From: Kory Maincent <kory.maincent@bootlin.com>
To: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: davem@davemloft.net, Andrew Lunn <andrew@lunn.ch>,
	Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	thomas.petazzoni@bootlin.com,
	linux-arm-kernel@lists.infradead.org,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Herve Codina <herve.codina@bootlin.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Oleksij Rempel <o.rempel@pengutronix.de>,
	Simon Horman <horms@kernel.org>,
	Romain Gantois <romain.gantois@bootlin.com>,
	Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
Subject: Re: [PATCH net-next v4 2/8] net: ethtool: netlink: Allow per-netdevice DUMP operations
Date: Tue, 25 Mar 2025 12:27:06 +0100	[thread overview]
Message-ID: <20250325122706.5287774d@kmaincent-XPS-13-7390> (raw)
In-Reply-To: <20250324104012.367366-3-maxime.chevallier@bootlin.com>

On Mon, 24 Mar 2025 11:40:04 +0100
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> We have a number of netlink commands in the ethnl family that may have
> multiple objects to dump even for a single net_device, including :
> 
>  - PLCA, PSE-PD, phy: one message per PHY device
>  - tsinfo: one message per timestamp source (netdev + phys)
>  - rss: One per RSS context
> 
> To get this behaviour, these netlink commands need to roll a custom
> ->dumpit().  
> 
> To prepare making per-netdev DUMP more generic in ethnl, introduce a
> member in the ethnl ops to indicate if a given command may allow
> pernetdev DUMPs (also referred to as filtered DUMPs).

...

> +
>  /* generic ->start() handler for GET requests */
>  static int ethnl_default_start(struct netlink_callback *cb)
>  {
> @@ -636,10 +659,10 @@ static int ethnl_default_start(struct netlink_callback
> *cb) }
>  
>  	ret = ethnl_default_parse(req_info, &info->info, ops, false);
> -	if (req_info->dev) {
> -		/* We ignore device specification in dump requests but as the
> -		 * same parser as for non-dump (doit) requests is used, it
> -		 * would take reference to the device if it finds one
> +	if (req_info->dev && !ops->allow_pernetdev_dump) {
> +		/* We ignore device specification in unfiltered dump requests
> +		 * but as the same parser as for non-dump (doit) requests is
> +		 * used, it would take reference to the device if it finds

This means the dump will have a different behavior in case of filtered dump
(allow_pernetdev_dump) or standard dump.
The standard dump will drop the interface device so it will dump all interfaces
even if one is specified.
The filtered dump will dump only the specified interface. 
Maybe it would be nice to have the same behavior for the dump for all the
ethtool command.
Even if this change modify the behavior of the dump for all the ethtool commands
it won't be an issue as the filtered dump did not exist before, so I suppose it
won't break anything. IMHO it is safer to do it now than later, if existing
ethtool command adds support for filtered dump.
We should find another way to know the parser is called from dump or doit.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

  reply	other threads:[~2025-03-25 11:27 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-24 10:40 [PATCH net-next v4 0/8] net: ethtool: Introduce ethnl dump helpers Maxime Chevallier
2025-03-24 10:40 ` [PATCH net-next v4 1/8] net: ethtool: Set the req_info->dev on DUMP requests for each dev Maxime Chevallier
2025-03-25 11:03   ` Kory Maincent
2025-03-24 10:40 ` [PATCH net-next v4 2/8] net: ethtool: netlink: Allow per-netdevice DUMP operations Maxime Chevallier
2025-03-25 11:27   ` Kory Maincent [this message]
2025-03-25 21:15     ` Jakub Kicinski
2025-03-25 21:22       ` Jakub Kicinski
2025-03-26  7:59         ` Maxime Chevallier
2025-03-26 10:29           ` Kory Maincent
2025-03-26 11:26             ` Maxime Chevallier
2025-03-24 10:40 ` [PATCH net-next v4 3/8] net: ethtool: netlink: Rename ethnl_default_dump_one Maxime Chevallier
2025-03-25 13:08   ` Kory Maincent
2025-03-25 21:16   ` Jakub Kicinski
2025-03-26  7:54     ` Maxime Chevallier
2025-03-24 10:40 ` [PATCH net-next v4 4/8] net: ethtool: netlink: Introduce command-specific dump_one_dev Maxime Chevallier
2025-03-25 13:23   ` Kory Maincent
2025-03-24 10:40 ` [PATCH net-next v4 5/8] net: ethtool: netlink: Introduce per-phy DUMP helpers Maxime Chevallier
2025-03-25 13:34   ` Kory Maincent
2025-03-24 10:40 ` [PATCH net-next v4 6/8] net: ethtool: phy: Convert the PHY_GET command to generic phy dump Maxime Chevallier
2025-03-24 10:40 ` [PATCH net-next v4 7/8] net: ethtool: plca: Use per-PHY DUMP operations Maxime Chevallier
2025-03-24 10:40 ` [PATCH net-next v4 8/8] net: ethtool: pse-pd: " Maxime Chevallier
2025-03-25 13:35   ` Kory Maincent
2025-03-25 21:31 ` [PATCH net-next v4 0/8] net: ethtool: Introduce ethnl dump helpers Jakub Kicinski
2025-03-26  7:52   ` Maxime Chevallier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250325122706.5287774d@kmaincent-XPS-13-7390 \
    --to=kory.maincent@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=christophe.leroy@csgroup.eu \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=herve.codina@bootlin.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    --cc=piergiorgio.beruto@gmail.com \
    --cc=romain.gantois@bootlin.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox