[PATCH net-next v4 0/8] net: ethtool: Introduce ethnl dump helpers

[PATCH net-next v4 0/8] net: ethtool: Introduce ethnl dump helpers

[Maxime Chevallier]

Hi everyone,

This is V4 for the ethnl dump support, allowing better handling of
per-phy dump but also any other dump operation that needs to dump more
than one message per netdev.

Changes in V4:
 - Don't grab rcu_read_lock when we already have a refcounter netdev on
   the filtered dump path (Paolo)
 - Move the dump_all stuff in a dedicated helper (Paolo)
 - Added patch 1 to set the dev in ctx->req_info

Changes in V3:
 - Fixed some typos and xmas tree issues
 - Added a missing check for EOPNOTSUPP in patch 1
 - Added missing kdoc
 - Added missing comments in phy_reply_size

Changes in V2:
 - Rebased on the netdev_lock work by Stanislav and the fixes from Eric
 - Fixed a bissectability issue
 - Fixed kdoc for the new ethnl ops and fields

V1: https://lore.kernel.org/netdev/20250305141938.319282-1-maxime.chevallier@bootlin.com/
V2: https://lore.kernel.org/netdev/20250308155440.267782-1-maxime.chevallier@bootlin.com/
V3: https://lore.kernel.org/netdev/20250313182647.250007-1-maxime.chevallier@bootlin.com/

As of today when using ethnl's default ops, the DUMP requests will
simply perform a GET for each netdev.

That hits limitations for commands that may return multiple messages for
a single netdev, such as :

 - RSS (listing contexts)
 - All PHY-specific commands (PLCA, PSE-PD, phy)
 - tsinfo (one item for the netdev +  one per phy)

 Commands that need a non-default DUMP support have to re-implement
 ->dumpit() themselves, which prevents using most of ethnl's internal
 circuitry.

This series therefore introduces a better support for dump operations in
ethnl.

The patches 1 and 2 introduce the support for filtered DUMPs, where an
ifindex/ifname can be passed in the request header for the DUMP
operation. This is for when we want to dump everything a netdev
supports, but without doing so for every single netdev. ethtool's
"--show-phys ethX" option for example performs a filtered dump.

Patch 3 introduces 3 new ethnl ops : 
 ->dump_start() to initialize a dump context
 ->dump_one_dev(), that can be implemented per-command to dump
 everything on a given netdev
 ->dump_done() to release the context

The default behaviour for dumps remains the same, calling the whole
->doit() path for each netdev.

Patch 4 introduces a set of ->dump_start(), ->dump_one_dev() and
->dump_done() callback implementations that can simply be plugged into
the existing commands that list objects per-phy, making the 
phy-targeting command behaviour more coherent.

Patch 5 uses that new set of helpers to rewrite the phy.c support, which
now uses the regulat ethnl_ops instead of fully custom genl ops. This
one is the hardest to review, sorry about that, I couldn't really manage
to incrementally rework that file :(

Patches 6 and 7 are where the new dump infra shines, adding per-netdev
per-phy dump support for PLCA and PSE-PD.

We could also consider converting tsinfo/tsconfig, rss and tunnels to
these new ->dump_***() operations as well, but that's out of this
series' scope.

Thanks,

Maxime



Maxime Chevallier (8):
  net: ethtool: Set the req_info->dev on DUMP requests for each dev
  net: ethtool: netlink: Allow per-netdevice DUMP operations
  net: ethtool: netlink: Rename ethnl_default_dump_one
  net: ethtool: netlink: Introduce command-specific dump_one_dev
  net: ethtool: netlink: Introduce per-phy DUMP helpers
  net: ethtool: phy: Convert the PHY_GET command to generic phy dump
  net: ethtool: plca: Use per-PHY DUMP operations
  net: ethtool: pse-pd: Use per-PHY DUMP operations

 net/ethtool/netlink.c | 185 ++++++++++++++++++-----
 net/ethtool/netlink.h |  47 +++++-
 net/ethtool/phy.c     | 344 ++++++++++++------------------------------
 net/ethtool/plca.c    |  12 ++
 net/ethtool/pse-pd.c  |   6 +
 5 files changed, 310 insertions(+), 284 deletions(-)

-- 
2.48.1

[PATCH net-next v4 1/8] net: ethtool: Set the req_info->dev on DUMP requests for each dev

[Maxime Chevallier]

There are a few netlink commands that rely on the req_info->dev field
being populated by ethnl in their ->prepare_data() and ->fill_reply().

For a regular GET request, this will be set by ethnl_default_parse(),
which calls ethnl_parse_header_dev_get().

In the case of a DUMP request, the ->prepare_data() and ->fill_reply()
callbacks will be called with the req_info->dev being NULL, which can
cause discrepancies in the behaviour between GET and DUMP results.

The main impact is that ethnl_req_get_phydev() will not find any
phy_device, impacting :
 - plca
 - pse-pd
 - stats

Some other commands rely on req_info->dev, namely :
 - coalesce in ->fill_reply to look for an irq_moder

Although cable_test and tunnels also rely on req_info->dev being set,
that's not a problem for these commands as :
 - cable_test doesn't support DUMP
 - tunnels rolls its own ->dumpit (and sets dev in the req_info).
 - phy also has its own ->dumpit

All other commands use reply_data->dev (probably the correct way of
doing things) and aren't facing this issue.

Simply set the dev in the req_info context when iterating to dump each
dev.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4 : New patch (was sent separaltely once)

 net/ethtool/netlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index a163d40c6431..6b1725795435 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -591,6 +591,7 @@ static int ethnl_default_dumpit(struct sk_buff *skb,
 		dev_hold(dev);
 		rcu_read_unlock();
 
+		ctx->req_info->dev = dev;
 		ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb));
 
 		rcu_read_lock();
-- 
2.48.1

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

[Maxime Chevallier]

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).

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4: - Don't hold RCU in the filtered path
    - Move dump_all into a new separate helper

 net/ethtool/netlink.c | 37 ++++++++++++++++++++++++++++++-------
 net/ethtool/netlink.h |  2 ++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 6b1725795435..31ab89ca0bcc 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -577,9 +577,7 @@ static int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev,
 	return ret;
 }
 
-/* Default ->dumpit() handler for GET requests. */
-static int ethnl_default_dumpit(struct sk_buff *skb,
-				struct netlink_callback *cb)
+static int ethnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
 	struct net *net = sock_net(skb->sk);
@@ -609,6 +607,31 @@ static int ethnl_default_dumpit(struct sk_buff *skb,
 	return ret;
 }
 
+/* Default ->dumpit() handler for GET requests. */
+static int ethnl_default_dumpit(struct sk_buff *skb,
+				struct netlink_callback *cb)
+{
+	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
+	int ret;
+
+	if (ctx->req_info->dev) {
+		/* Filtered DUMP request targeted to a single netdev. We already
+		 * hold a ref to the netdev from ->start()
+		 */
+		ret = ethnl_default_dump_one(skb, ctx->req_info->dev, ctx,
+					     genl_info_dump(cb));
+		netdev_put(ctx->req_info->dev, &ctx->req_info->dev_tracker);
+
+		if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
+			ret = skb->len;
+
+	} else {
+		ret = ethnl_dump_all(skb, cb);
+	}
+
+	return ret;
+}
+
 /* 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 one
 		 */
 		netdev_put(req_info->dev, &req_info->dev_tracker);
 		req_info->dev = NULL;
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index ec6ab5443a6f..4aaa73282d6a 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -331,6 +331,7 @@ int ethnl_sock_priv_set(struct sk_buff *skb, struct net_device *dev, u32 portid,
  * @req_info_size:    size of request info
  * @reply_data_size:  size of reply data
  * @allow_nodev_do:   allow non-dump request with no device identification
+ * @allow_pernetdev_dump: allow filtering dump requests with ifname/ifindex
  * @set_ntf_cmd:      notification to generate on changes (SET)
  * @parse_request:
  *	Parse request except common header (struct ethnl_req_info). Common
@@ -388,6 +389,7 @@ struct ethnl_request_ops {
 	unsigned int		req_info_size;
 	unsigned int		reply_data_size;
 	bool			allow_nodev_do;
+	bool			allow_pernetdev_dump;
 	u8			set_ntf_cmd;
 
 	int (*parse_request)(struct ethnl_req_info *req_info,
-- 
2.48.1

[PATCH net-next v4 3/8] net: ethtool: netlink: Rename ethnl_default_dump_one

[Maxime Chevallier]

As we work on getting more objects out of a per-netdev DUMP, rename
ethnl_default_dump_one() into ethnl_default_dump_one_dev(), making it
explicit that this dumps everything for one netdev.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4 : No changes

 net/ethtool/netlink.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 31ab89ca0bcc..63ede3638708 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -540,9 +540,9 @@ static int ethnl_default_doit(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
-static int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev,
-				  const struct ethnl_dump_ctx *ctx,
-				  const struct genl_info *info)
+static int ethnl_default_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
+				      const struct ethnl_dump_ctx *ctx,
+				      const struct genl_info *info)
 {
 	void *ehdr;
 	int ret;
@@ -590,7 +590,8 @@ static int ethnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 		rcu_read_unlock();
 
 		ctx->req_info->dev = dev;
-		ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb));
+		ret = ethnl_default_dump_one_dev(skb, dev, ctx,
+						 genl_info_dump(cb));
 
 		rcu_read_lock();
 		dev_put(dev);
@@ -618,8 +619,8 @@ static int ethnl_default_dumpit(struct sk_buff *skb,
 		/* Filtered DUMP request targeted to a single netdev. We already
 		 * hold a ref to the netdev from ->start()
 		 */
-		ret = ethnl_default_dump_one(skb, ctx->req_info->dev, ctx,
-					     genl_info_dump(cb));
+		ret = ethnl_default_dump_one_dev(skb, ctx->req_info->dev, ctx,
+						 genl_info_dump(cb));
 		netdev_put(ctx->req_info->dev, &ctx->req_info->dev_tracker);
 
 		if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
-- 
2.48.1

[PATCH net-next v4 4/8] net: ethtool: netlink: Introduce command-specific dump_one_dev

[Maxime Chevallier]

Prepare more generic ethnl DUMP hanldling, by allowing netlink commands
to register their own dump_one_dev() callback. This avoids having to
roll with a fully custom genl ->dumpit callback, allowing the re-use
of some ethnl plumbing.

Fallback to the default dump_one_dev behaviour when no custom callback
is found.

The command dump context is maintained within the ethnl_dump_ctx, that
we move in netlink.h so that command handlers can access it.

This context can be allocated/freed in new ->dump_start() and
->dump_done() callbacks.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4 : No changes

 net/ethtool/netlink.c | 61 +++++++++++++++++++++++++------------------
 net/ethtool/netlink.h | 35 +++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 26 deletions(-)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 63ede3638708..0345bffa0678 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -339,24 +339,6 @@ int ethnl_multicast(struct sk_buff *skb, struct net_device *dev)
 
 /* GET request helpers */
 
-/**
- * struct ethnl_dump_ctx - context structure for generic dumpit() callback
- * @ops:        request ops of currently processed message type
- * @req_info:   parsed request header of processed request
- * @reply_data: data needed to compose the reply
- * @pos_ifindex: saved iteration position - ifindex
- *
- * These parameters are kept in struct netlink_callback as context preserved
- * between iterations. They are initialized by ethnl_default_start() and used
- * in ethnl_default_dumpit() and ethnl_default_done().
- */
-struct ethnl_dump_ctx {
-	const struct ethnl_request_ops	*ops;
-	struct ethnl_req_info		*req_info;
-	struct ethnl_reply_data		*reply_data;
-	unsigned long			pos_ifindex;
-};
-
 static const struct ethnl_request_ops *
 ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_STRSET_GET]	= &ethnl_strset_request_ops,
@@ -540,9 +522,9 @@ static int ethnl_default_doit(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
-static int ethnl_default_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
-				      const struct ethnl_dump_ctx *ctx,
-				      const struct genl_info *info)
+static int ethnl_default_dump_one(struct sk_buff *skb,
+				  const struct ethnl_dump_ctx *ctx,
+				  const struct genl_info *info)
 {
 	void *ehdr;
 	int ret;
@@ -553,15 +535,15 @@ static int ethnl_default_dump_one_dev(struct sk_buff *skb, struct net_device *de
 	if (!ehdr)
 		return -EMSGSIZE;
 
-	ethnl_init_reply_data(ctx->reply_data, ctx->ops, dev);
 	rtnl_lock();
-	netdev_lock_ops(dev);
+	netdev_lock_ops(ctx->reply_data->dev);
 	ret = ctx->ops->prepare_data(ctx->req_info, ctx->reply_data, info);
-	netdev_unlock_ops(dev);
+	netdev_unlock_ops(ctx->reply_data->dev);
 	rtnl_unlock();
 	if (ret < 0)
 		goto out;
-	ret = ethnl_fill_reply_header(skb, dev, ctx->ops->hdr_attr);
+	ret = ethnl_fill_reply_header(skb, ctx->reply_data->dev,
+				      ctx->ops->hdr_attr);
 	if (ret < 0)
 		goto out;
 	ret = ctx->ops->fill_reply(skb, ctx->req_info, ctx->reply_data);
@@ -569,11 +551,29 @@ static int ethnl_default_dump_one_dev(struct sk_buff *skb, struct net_device *de
 out:
 	if (ctx->ops->cleanup_data)
 		ctx->ops->cleanup_data(ctx->reply_data);
-	ctx->reply_data->dev = NULL;
+
 	if (ret < 0)
 		genlmsg_cancel(skb, ehdr);
 	else
 		genlmsg_end(skb, ehdr);
+
+	return ret;
+}
+
+static int ethnl_default_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
+				      struct ethnl_dump_ctx *ctx,
+				      const struct genl_info *info)
+{
+	int ret;
+
+	ethnl_init_reply_data(ctx->reply_data, ctx->ops, dev);
+
+	if (ctx->ops->dump_one_dev)
+		ret = ctx->ops->dump_one_dev(skb, ctx, info);
+	else
+		ret = ethnl_default_dump_one(skb, ctx, info);
+
+	ctx->reply_data->dev = NULL;
 	return ret;
 }
 
@@ -676,6 +676,12 @@ static int ethnl_default_start(struct netlink_callback *cb)
 	ctx->reply_data = reply_data;
 	ctx->pos_ifindex = 0;
 
+	if (ctx->ops->dump_start) {
+		ret = ctx->ops->dump_start(ctx);
+		if (ret)
+			goto free_reply_data;
+	}
+
 	return 0;
 
 free_reply_data:
@@ -691,6 +697,9 @@ static int ethnl_default_done(struct netlink_callback *cb)
 {
 	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
 
+	if (ctx->ops->dump_done)
+		ctx->ops->dump_done(ctx);
+
 	kfree(ctx->reply_data);
 	kfree(ctx->req_info);
 
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 4aaa73282d6a..79fe98190c64 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -307,6 +307,26 @@ struct ethnl_reply_data {
 	struct net_device		*dev;
 };
 
+/**
+ * struct ethnl_dump_ctx - context structure for generic dumpit() callback
+ * @ops:        request ops of currently processed message type
+ * @req_info:   parsed request header of processed request
+ * @reply_data: data needed to compose the reply
+ * @pos_ifindex: saved iteration position - ifindex
+ * @cmd_ctx: command-specific context to maintain across the dump.
+ *
+ * These parameters are kept in struct netlink_callback as context preserved
+ * between iterations. They are initialized by ethnl_default_start() and used
+ * in ethnl_default_dumpit() and ethnl_default_done().
+ */
+struct ethnl_dump_ctx {
+	const struct ethnl_request_ops	*ops;
+	struct ethnl_req_info		*req_info;
+	struct ethnl_reply_data		*reply_data;
+	unsigned long			pos_ifindex;
+	void				*cmd_ctx;
+};
+
 int ethnl_ops_begin(struct net_device *dev);
 void ethnl_ops_complete(struct net_device *dev);
 
@@ -373,6 +393,15 @@ int ethnl_sock_priv_set(struct sk_buff *skb, struct net_device *dev, u32 portid,
  *	 - 0 if no configuration has changed
  *	 - 1 if configuration changed and notification should be generated
  *	 - negative errno on errors
+ * @dump_start:
+ *	Optional callback to prepare a dump operation, should there be a need
+ *	to maintain some context across the dump.
+ * @dump_one_dev:
+ *	Optional callback to generate all messages for a given netdev. This
+ *	is relevant only when a request can produce different results for the
+ *	same netdev depending on command-specific attributes.
+ * @dump_done:
+ *	Optional callback to cleanup any context allocated in ->dump_start()
  *
  * Description of variable parts of GET request handling when using the
  * unified infrastructure. When used, a pointer to an instance of this
@@ -409,6 +438,12 @@ struct ethnl_request_ops {
 			    struct genl_info *info);
 	int (*set)(struct ethnl_req_info *req_info,
 		   struct genl_info *info);
+
+	int (*dump_start)(struct ethnl_dump_ctx *ctx);
+	int (*dump_one_dev)(struct sk_buff *skb,
+			    struct ethnl_dump_ctx *ctx,
+			    const struct genl_info *info);
+	void (*dump_done)(struct ethnl_dump_ctx *ctx);
 };
 
 /* request handlers */
-- 
2.48.1

[PATCH net-next v4 5/8] net: ethtool: netlink: Introduce per-phy DUMP helpers

[Maxime Chevallier]

As there are multiple ethnl commands that report messages based on
phy_device information, let's introduce a set of ethnl generic dump
helpers to allow DUMP support for each PHY on a given netdev.

This logic iterates over the phy_link_topology of each netdev (or a
single netdev for filtered DUMP), and call ethnl_default_dump_one() with
the req_info populated with ifindex + phyindex.

This allows re-using all the existing infra for phy-targetting commands
that already use ethnl generic helpers.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4 : No changes

 net/ethtool/netlink.c | 78 +++++++++++++++++++++++++++++++++++++++++++
 net/ethtool/netlink.h |  6 ++++
 2 files changed, 84 insertions(+)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 0345bffa0678..171290eaf406 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -560,6 +560,84 @@ static int ethnl_default_dump_one(struct sk_buff *skb,
 	return ret;
 }
 
+/* Specific context for phy-targeting command DUMP operatins. We keep in context
+ * the latest phy_index we dumped, in case of an interrupted DUMP.
+ */
+struct ethnl_dump_ctx_perphy {
+	unsigned long phy_index;
+};
+
+/**
+ * ethnl_dump_start_perphy() - Initialise a dump for PHY cmds
+ * @ctx: Generic ethnl dump context whose cmd_ctx will be initialized
+ *
+ * Initializes a dump context for ethnl commands that may return
+ * one message per PHY on a single netdev.
+ *
+ * Returns: 0 for success, a negative value for errors.
+ */
+int ethnl_dump_start_perphy(struct ethnl_dump_ctx *ctx)
+{
+	struct ethnl_dump_ctx_perphy *dump_ctx;
+
+	dump_ctx = kzalloc(sizeof(*dump_ctx), GFP_KERNEL);
+	if (!dump_ctx)
+		return -ENOMEM;
+
+	ctx->cmd_ctx = dump_ctx;
+
+	return 0;
+}
+
+/**
+ * ethnl_dump_done_perphy() - Releases the per-phy dump context
+ * @ctx: Generic ethnl dump context whose cmd_ctx will be released
+ */
+void ethnl_dump_done_perphy(struct ethnl_dump_ctx *ctx)
+{
+	kfree(ctx->cmd_ctx);
+}
+
+/**
+ * ethnl_dump_one_dev_perphy() - Dump all PHY-related messages for one netdev
+ * @skb: skb containing the DUMP result
+ * @ctx: Dump context. Will be kept across the DUMP operation.
+ * @info: Genl receive info
+ *
+ * Some commands are related to PHY devices attached to netdevs. As there may be
+ * multiple PHYs, this DUMP handler will populate the reply with one message per
+ * PHY on a single netdev.
+ *
+ * Returns: 0 for success or when nothing to do, a negative value otherwise.
+ */
+int ethnl_dump_one_dev_perphy(struct sk_buff *skb,
+			      struct ethnl_dump_ctx *ctx,
+			      const struct genl_info *info)
+{
+	struct ethnl_dump_ctx_perphy *dump_ctx = ctx->cmd_ctx;
+	struct net_device *dev = ctx->reply_data->dev;
+	struct phy_device_node *pdn;
+	int ret = 0;
+
+	if (!dev->link_topo)
+		return 0;
+
+	xa_for_each_start(&dev->link_topo->phys, dump_ctx->phy_index,
+			  pdn, dump_ctx->phy_index) {
+		ctx->req_info->phy_index = dump_ctx->phy_index;
+
+		/* We can re-use the original dump_one as ->prepare_data in
+		 * commands use ethnl_req_get_phydev(), which gets the PHY from
+		 * what's in req_info
+		 */
+		ret = ethnl_default_dump_one(skb, ctx, info);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
 static int ethnl_default_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
 				      struct ethnl_dump_ctx *ctx,
 				      const struct genl_info *info)
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 79fe98190c64..530a9b5c8b39 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -327,6 +327,12 @@ struct ethnl_dump_ctx {
 	void				*cmd_ctx;
 };
 
+/* Generic callbacks to be used by PHY targeting commands */
+int ethnl_dump_start_perphy(struct ethnl_dump_ctx *ctx);
+int ethnl_dump_one_dev_perphy(struct sk_buff *skb, struct ethnl_dump_ctx *ctx,
+			      const struct genl_info *info);
+void ethnl_dump_done_perphy(struct ethnl_dump_ctx *ctx);
+
 int ethnl_ops_begin(struct net_device *dev);
 void ethnl_ops_complete(struct net_device *dev);
 
-- 
2.48.1

[PATCH net-next v4 6/8] net: ethtool: phy: Convert the PHY_GET command to generic phy dump

[Maxime Chevallier]

Now that we have an infrastructure in ethnl for perphy DUMPs, we can get
rid of the custom ->doit and ->dumpit to deal with PHY listing commands.

As most of the code was custom, this basically means re-writing how we
deal with PHY listing.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4 : No changes

 net/ethtool/netlink.c |   9 +-
 net/ethtool/netlink.h |   4 -
 net/ethtool/phy.c     | 344 ++++++++++++------------------------------
 3 files changed, 105 insertions(+), 252 deletions(-)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 171290eaf406..217024af68fd 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -382,6 +382,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_MM_SET]		= &ethnl_mm_request_ops,
 	[ETHTOOL_MSG_TSCONFIG_GET]	= &ethnl_tsconfig_request_ops,
 	[ETHTOOL_MSG_TSCONFIG_SET]	= &ethnl_tsconfig_request_ops,
+	[ETHTOOL_MSG_PHY_GET]		= &ethnl_phy_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -1381,10 +1382,10 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	},
 	{
 		.cmd	= ETHTOOL_MSG_PHY_GET,
-		.doit	= ethnl_phy_doit,
-		.start	= ethnl_phy_start,
-		.dumpit	= ethnl_phy_dumpit,
-		.done	= ethnl_phy_done,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
 		.policy = ethnl_phy_get_policy,
 		.maxattr = ARRAY_SIZE(ethnl_phy_get_policy) - 1,
 	},
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 530a9b5c8b39..60f16090640f 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -542,10 +542,6 @@ int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 int ethnl_act_module_fw_flash(struct sk_buff *skb, struct genl_info *info);
 int ethnl_rss_dump_start(struct netlink_callback *cb);
 int ethnl_rss_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
-int ethnl_phy_start(struct netlink_callback *cb);
-int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info);
-int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
-int ethnl_phy_done(struct netlink_callback *cb);
 int ethnl_tsinfo_start(struct netlink_callback *cb);
 int ethnl_tsinfo_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 int ethnl_tsinfo_done(struct netlink_callback *cb);
diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index 1f590e8d75ed..88343fed7366 100644
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -12,304 +12,160 @@
 #include <net/netdev_lock.h>
 
 struct phy_req_info {
-	struct ethnl_req_info		base;
-	struct phy_device_node		*pdn;
+	struct ethnl_req_info base;
 };
 
-#define PHY_REQINFO(__req_base) \
-	container_of(__req_base, struct phy_req_info, base)
+struct phy_reply_data {
+	struct ethnl_reply_data	base;
+	u32 phyindex;
+	char *drvname;
+	char *name;
+	unsigned int upstream_type;
+	char *upstream_sfp_name;
+	unsigned int upstream_index;
+	char *downstream_sfp_name;
+};
+
+#define PHY_REPDATA(__reply_base) \
+	container_of(__reply_base, struct phy_reply_data, base)
 
 const struct nla_policy ethnl_phy_get_policy[ETHTOOL_A_PHY_HEADER + 1] = {
 	[ETHTOOL_A_PHY_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
 };
 
-/* Caller holds rtnl */
-static ssize_t
-ethnl_phy_reply_size(const struct ethnl_req_info *req_base,
-		     struct netlink_ext_ack *extack)
+static int phy_reply_size(const struct ethnl_req_info *req_info,
+			  const struct ethnl_reply_data *reply_data)
 {
-	struct phy_req_info *req_info = PHY_REQINFO(req_base);
-	struct phy_device_node *pdn = req_info->pdn;
-	struct phy_device *phydev = pdn->phy;
+	struct phy_reply_data *rep_data = PHY_REPDATA(reply_data);
 	size_t size = 0;
 
-	ASSERT_RTNL();
-
 	/* ETHTOOL_A_PHY_INDEX */
 	size += nla_total_size(sizeof(u32));
 
 	/* ETHTOOL_A_DRVNAME */
-	if (phydev->drv)
-		size += nla_total_size(strlen(phydev->drv->name) + 1);
+	if (rep_data->drvname)
+		size += nla_total_size(strlen(rep_data->drvname) + 1);
 
 	/* ETHTOOL_A_NAME */
-	size += nla_total_size(strlen(dev_name(&phydev->mdio.dev)) + 1);
+	size += nla_total_size(strlen(rep_data->name) + 1);
 
 	/* ETHTOOL_A_PHY_UPSTREAM_TYPE */
 	size += nla_total_size(sizeof(u32));
 
-	if (phy_on_sfp(phydev)) {
-		const char *upstream_sfp_name = sfp_get_name(pdn->parent_sfp_bus);
-
-		/* ETHTOOL_A_PHY_UPSTREAM_SFP_NAME */
-		if (upstream_sfp_name)
-			size += nla_total_size(strlen(upstream_sfp_name) + 1);
+	/* ETHTOOL_A_PHY_UPSTREAM_SFP_NAME */
+	if (rep_data->upstream_sfp_name)
+		size += nla_total_size(strlen(rep_data->upstream_sfp_name) + 1);
 
-		/* ETHTOOL_A_PHY_UPSTREAM_INDEX */
+	/* ETHTOOL_A_PHY_UPSTREAM_INDEX */
+	if (rep_data->upstream_index)
 		size += nla_total_size(sizeof(u32));
-	}
 
 	/* ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME */
-	if (phydev->sfp_bus) {
-		const char *sfp_name = sfp_get_name(phydev->sfp_bus);
-
-		if (sfp_name)
-			size += nla_total_size(strlen(sfp_name) + 1);
-	}
+	if (rep_data->downstream_sfp_name)
+		size += nla_total_size(strlen(rep_data->downstream_sfp_name) + 1);
 
 	return size;
 }
 
-static int
-ethnl_phy_fill_reply(const struct ethnl_req_info *req_base, struct sk_buff *skb)
+static int phy_prepare_data(const struct ethnl_req_info *req_info,
+			    struct ethnl_reply_data *reply_data,
+			    const struct genl_info *info)
 {
-	struct phy_req_info *req_info = PHY_REQINFO(req_base);
-	struct phy_device_node *pdn = req_info->pdn;
-	struct phy_device *phydev = pdn->phy;
-	enum phy_upstream ptype;
+	struct phy_link_topology *topo = reply_data->dev->link_topo;
+	struct phy_reply_data *rep_data = PHY_REPDATA(reply_data);
+	struct nlattr **tb = info->attrs;
+	struct phy_device_node *pdn;
+	struct phy_device *phydev;
 
-	ptype = pdn->upstream_type;
+	/* RTNL is held by the caller */
+	phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PHY_HEADER,
+				      info->extack);
+	if (IS_ERR_OR_NULL(phydev))
+		return -EOPNOTSUPP;
 
-	if (nla_put_u32(skb, ETHTOOL_A_PHY_INDEX, phydev->phyindex) ||
-	    nla_put_string(skb, ETHTOOL_A_PHY_NAME, dev_name(&phydev->mdio.dev)) ||
-	    nla_put_u32(skb, ETHTOOL_A_PHY_UPSTREAM_TYPE, ptype))
-		return -EMSGSIZE;
+	pdn = xa_load(&topo->phys, phydev->phyindex);
+	if (!pdn)
+		return -EOPNOTSUPP;
 
-	if (phydev->drv &&
-	    nla_put_string(skb, ETHTOOL_A_PHY_DRVNAME, phydev->drv->name))
-		return -EMSGSIZE;
+	rep_data->phyindex = phydev->phyindex;
+	rep_data->name = kstrdup(dev_name(&phydev->mdio.dev), GFP_KERNEL);
+	rep_data->drvname = kstrdup(phydev->drv->name, GFP_KERNEL);
+	rep_data->upstream_type = pdn->upstream_type;
 
-	if (ptype == PHY_UPSTREAM_PHY) {
+	if (pdn->upstream_type == PHY_UPSTREAM_PHY) {
 		struct phy_device *upstream = pdn->upstream.phydev;
-		const char *sfp_upstream_name;
-
-		/* Parent index */
-		if (nla_put_u32(skb, ETHTOOL_A_PHY_UPSTREAM_INDEX, upstream->phyindex))
-			return -EMSGSIZE;
-
-		if (pdn->parent_sfp_bus) {
-			sfp_upstream_name = sfp_get_name(pdn->parent_sfp_bus);
-			if (sfp_upstream_name &&
-			    nla_put_string(skb, ETHTOOL_A_PHY_UPSTREAM_SFP_NAME,
-					   sfp_upstream_name))
-				return -EMSGSIZE;
-		}
+		rep_data->upstream_index = upstream->phyindex;
 	}
 
-	if (phydev->sfp_bus) {
-		const char *sfp_name = sfp_get_name(phydev->sfp_bus);
+	if (pdn->parent_sfp_bus)
+		rep_data->upstream_sfp_name = kstrdup(sfp_get_name(pdn->parent_sfp_bus),
+						      GFP_KERNEL);
 
-		if (sfp_name &&
-		    nla_put_string(skb, ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME,
-				   sfp_name))
-			return -EMSGSIZE;
-	}
+	if (phydev->sfp_bus)
+		rep_data->downstream_sfp_name = kstrdup(sfp_get_name(phydev->sfp_bus),
+							GFP_KERNEL);
 
 	return 0;
 }
 
-static int ethnl_phy_parse_request(struct ethnl_req_info *req_base,
-				   struct nlattr **tb,
-				   struct netlink_ext_ack *extack)
+static int phy_fill_reply(struct sk_buff *skb,
+			  const struct ethnl_req_info *req_info,
+			  const struct ethnl_reply_data *reply_data)
 {
-	struct phy_link_topology *topo = req_base->dev->link_topo;
-	struct phy_req_info *req_info = PHY_REQINFO(req_base);
-	struct phy_device *phydev;
-
-	phydev = ethnl_req_get_phydev(req_base, tb, ETHTOOL_A_PHY_HEADER,
-				      extack);
-	if (!phydev)
-		return 0;
-
-	if (IS_ERR(phydev))
-		return PTR_ERR(phydev);
-
-	if (!topo)
-		return 0;
-
-	req_info->pdn = xa_load(&topo->phys, phydev->phyindex);
-
-	return 0;
-}
-
-int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
-{
-	struct phy_req_info req_info = {};
-	struct nlattr **tb = info->attrs;
-	struct sk_buff *rskb;
-	void *reply_payload;
-	int reply_len;
-	int ret;
-
-	ret = ethnl_parse_header_dev_get(&req_info.base,
-					 tb[ETHTOOL_A_PHY_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-
-	rtnl_lock();
-	netdev_lock_ops(req_info.base.dev);
-
-	ret = ethnl_phy_parse_request(&req_info.base, tb, info->extack);
-	if (ret < 0)
-		goto err_unlock;
-
-	/* No PHY, return early */
-	if (!req_info.pdn)
-		goto err_unlock;
-
-	ret = ethnl_phy_reply_size(&req_info.base, info->extack);
-	if (ret < 0)
-		goto err_unlock;
-	reply_len = ret + ethnl_reply_header_size();
-
-	rskb = ethnl_reply_init(reply_len, req_info.base.dev,
-				ETHTOOL_MSG_PHY_GET_REPLY,
-				ETHTOOL_A_PHY_HEADER,
-				info, &reply_payload);
-	if (!rskb) {
-		ret = -ENOMEM;
-		goto err_unlock;
-	}
+	struct phy_reply_data *rep_data = PHY_REPDATA(reply_data);
 
-	ret = ethnl_phy_fill_reply(&req_info.base, rskb);
-	if (ret)
-		goto err_free_msg;
-
-	netdev_unlock_ops(req_info.base.dev);
-	rtnl_unlock();
-	ethnl_parse_header_dev_put(&req_info.base);
-	genlmsg_end(rskb, reply_payload);
-
-	return genlmsg_reply(rskb, info);
-
-err_free_msg:
-	nlmsg_free(rskb);
-err_unlock:
-	netdev_unlock_ops(req_info.base.dev);
-	rtnl_unlock();
-	ethnl_parse_header_dev_put(&req_info.base);
-	return ret;
-}
-
-struct ethnl_phy_dump_ctx {
-	struct phy_req_info	*phy_req_info;
-	unsigned long ifindex;
-	unsigned long phy_index;
-};
-
-int ethnl_phy_start(struct netlink_callback *cb)
-{
-	const struct genl_info *info = genl_info_dump(cb);
-	struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
-	int ret;
+	if (nla_put_u32(skb, ETHTOOL_A_PHY_INDEX, rep_data->phyindex) ||
+	    nla_put_string(skb, ETHTOOL_A_PHY_NAME, rep_data->name) ||
+	    nla_put_u32(skb, ETHTOOL_A_PHY_UPSTREAM_TYPE, rep_data->upstream_type))
+		return -EMSGSIZE;
 
-	BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
+	if (rep_data->drvname &&
+	    nla_put_string(skb, ETHTOOL_A_PHY_DRVNAME, rep_data->drvname))
+		return -EMSGSIZE;
 
-	ctx->phy_req_info = kzalloc(sizeof(*ctx->phy_req_info), GFP_KERNEL);
-	if (!ctx->phy_req_info)
-		return -ENOMEM;
+	if (rep_data->upstream_index &&
+	    nla_put_u32(skb, ETHTOOL_A_PHY_UPSTREAM_INDEX,
+			rep_data->upstream_index))
+		return -EMSGSIZE;
 
-	ret = ethnl_parse_header_dev_get(&ctx->phy_req_info->base,
-					 info->attrs[ETHTOOL_A_PHY_HEADER],
-					 sock_net(cb->skb->sk), cb->extack,
-					 false);
-	ctx->ifindex = 0;
-	ctx->phy_index = 0;
+	if (rep_data->upstream_sfp_name &&
+	    nla_put_string(skb, ETHTOOL_A_PHY_UPSTREAM_SFP_NAME,
+			   rep_data->upstream_sfp_name))
+		return -EMSGSIZE;
 
-	if (ret)
-		kfree(ctx->phy_req_info);
+	if (rep_data->downstream_sfp_name &&
+	    nla_put_string(skb, ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME,
+			   rep_data->downstream_sfp_name))
+		return -EMSGSIZE;
 
-	return ret;
+	return 0;
 }
 
-int ethnl_phy_done(struct netlink_callback *cb)
+static void phy_cleanup_data(struct ethnl_reply_data *reply_data)
 {
-	struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
-
-	if (ctx->phy_req_info->base.dev)
-		ethnl_parse_header_dev_put(&ctx->phy_req_info->base);
-
-	kfree(ctx->phy_req_info);
+	struct phy_reply_data *rep_data = PHY_REPDATA(reply_data);
 
-	return 0;
+	kfree(rep_data->drvname);
+	kfree(rep_data->name);
+	kfree(rep_data->upstream_sfp_name);
+	kfree(rep_data->downstream_sfp_name);
 }
 
-static int ethnl_phy_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
-				  struct netlink_callback *cb)
-{
-	struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
-	struct phy_req_info *pri = ctx->phy_req_info;
-	struct phy_device_node *pdn;
-	int ret = 0;
-	void *ehdr;
-
-	if (!dev->link_topo)
-		return 0;
-
-	xa_for_each_start(&dev->link_topo->phys, ctx->phy_index, pdn, ctx->phy_index) {
-		ehdr = ethnl_dump_put(skb, cb, ETHTOOL_MSG_PHY_GET_REPLY);
-		if (!ehdr) {
-			ret = -EMSGSIZE;
-			break;
-		}
-
-		ret = ethnl_fill_reply_header(skb, dev, ETHTOOL_A_PHY_HEADER);
-		if (ret < 0) {
-			genlmsg_cancel(skb, ehdr);
-			break;
-		}
-
-		pri->pdn = pdn;
-		ret = ethnl_phy_fill_reply(&pri->base, skb);
-		if (ret < 0) {
-			genlmsg_cancel(skb, ehdr);
-			break;
-		}
-
-		genlmsg_end(skb, ehdr);
-	}
+const struct ethnl_request_ops ethnl_phy_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PHY_GET,
+	.reply_cmd		= ETHTOOL_MSG_PHY_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_PHY_HEADER,
+	.req_info_size		= sizeof(struct phy_req_info),
+	.reply_data_size	= sizeof(struct phy_reply_data),
 
-	return ret;
-}
+	.prepare_data		= phy_prepare_data,
+	.reply_size		= phy_reply_size,
+	.fill_reply		= phy_fill_reply,
+	.cleanup_data		= phy_cleanup_data,
 
-int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
-{
-	struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
-	struct net *net = sock_net(skb->sk);
-	struct net_device *dev;
-	int ret = 0;
-
-	rtnl_lock();
-
-	if (ctx->phy_req_info->base.dev) {
-		dev = ctx->phy_req_info->base.dev;
-		netdev_lock_ops(dev);
-		ret = ethnl_phy_dump_one_dev(skb, dev, cb);
-		netdev_unlock_ops(dev);
-	} else {
-		for_each_netdev_dump(net, dev, ctx->ifindex) {
-			netdev_lock_ops(dev);
-			ret = ethnl_phy_dump_one_dev(skb, dev, cb);
-			netdev_unlock_ops(dev);
-			if (ret)
-				break;
-
-			ctx->phy_index = 0;
-		}
-	}
-	rtnl_unlock();
+	.dump_start		= ethnl_dump_start_perphy,
+	.dump_one_dev		= ethnl_dump_one_dev_perphy,
+	.dump_done		= ethnl_dump_done_perphy,
 
-	return ret;
-}
+	.allow_pernetdev_dump	= true,
+};
-- 
2.48.1

[PATCH net-next v4 7/8] net: ethtool: plca: Use per-PHY DUMP operations

[Maxime Chevallier]

Leverage the per-phy ethnl DUMP helpers in case we have more that one
PLCA-able PHY on the link.

This is done for both PLCA status and PLCA config.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4 : No changes

 net/ethtool/plca.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/ethtool/plca.c b/net/ethtool/plca.c
index e1f7820a6158..2148ff607561 100644
--- a/net/ethtool/plca.c
+++ b/net/ethtool/plca.c
@@ -191,6 +191,12 @@ const struct ethnl_request_ops ethnl_plca_cfg_request_ops = {
 
 	.set			= ethnl_set_plca,
 	.set_ntf_cmd		= ETHTOOL_MSG_PLCA_NTF,
+
+	.dump_start		= ethnl_dump_start_perphy,
+	.dump_one_dev		= ethnl_dump_one_dev_perphy,
+	.dump_done		= ethnl_dump_done_perphy,
+
+	.allow_pernetdev_dump	= true,
 };
 
 // PLCA get status message -------------------------------------------------- //
@@ -268,4 +274,10 @@ const struct ethnl_request_ops ethnl_plca_status_request_ops = {
 	.prepare_data		= plca_get_status_prepare_data,
 	.reply_size		= plca_get_status_reply_size,
 	.fill_reply		= plca_get_status_fill_reply,
+
+	.dump_start		= ethnl_dump_start_perphy,
+	.dump_one_dev		= ethnl_dump_one_dev_perphy,
+	.dump_done		= ethnl_dump_done_perphy,
+
+	.allow_pernetdev_dump	= true,
 };
-- 
2.48.1

[PATCH net-next v4 8/8] net: ethtool: pse-pd: Use per-PHY DUMP operations

[Maxime Chevallier]

Leverage the per-phy ethnl DUMP helpers in case we have more that one
PSE PHY on the link.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V4 : No changes

 net/ethtool/pse-pd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index 4f6b99eab2a6..f3d14be8bdd9 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -314,4 +314,10 @@ const struct ethnl_request_ops ethnl_pse_request_ops = {
 
 	.set			= ethnl_set_pse,
 	/* PSE has no notification */
+
+	.dump_start		= ethnl_dump_start_perphy,
+	.dump_one_dev		= ethnl_dump_one_dev_perphy,
+	.dump_done		= ethnl_dump_done_perphy,
+
+	.allow_pernetdev_dump	= true,
 };
-- 
2.48.1

Re: [PATCH net-next v4 1/8] net: ethtool: Set the req_info->dev on DUMP requests for each dev

[Kory Maincent]

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

> There are a few netlink commands that rely on the req_info->dev field
> being populated by ethnl in their ->prepare_data() and ->fill_reply().
> 
> For a regular GET request, this will be set by ethnl_default_parse(),
> which calls ethnl_parse_header_dev_get().
> 
> In the case of a DUMP request, the ->prepare_data() and ->fill_reply()
> callbacks will be called with the req_info->dev being NULL, which can
> cause discrepancies in the behaviour between GET and DUMP results.
> 
> The main impact is that ethnl_req_get_phydev() will not find any
> phy_device, impacting :
>  - plca
>  - pse-pd
>  - stats
> 
> Some other commands rely on req_info->dev, namely :
>  - coalesce in ->fill_reply to look for an irq_moder
> 
> Although cable_test and tunnels also rely on req_info->dev being set,
> that's not a problem for these commands as :
>  - cable_test doesn't support DUMP
>  - tunnels rolls its own ->dumpit (and sets dev in the req_info).
>  - phy also has its own ->dumpit
> 
> All other commands use reply_data->dev (probably the correct way of
> doing things) and aren't facing this issue.
> 
> Simply set the dev in the req_info context when iterating to dump each
> dev.

Tested-by: Kory Maincent <kory.maincent@bootlin.com>
Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

Thank you, this fixes the PSE dump!

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

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

[Kory Maincent]

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

Re: [PATCH net-next v4 3/8] net: ethtool: netlink: Rename ethnl_default_dump_one

[Kory Maincent]

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

> As we work on getting more objects out of a per-netdev DUMP, rename
> ethnl_default_dump_one() into ethnl_default_dump_one_dev(), making it
> explicit that this dumps everything for one netdev.

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

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

Re: [PATCH net-next v4 4/8] net: ethtool: netlink: Introduce command-specific dump_one_dev

[Kory Maincent]

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

> Prepare more generic ethnl DUMP hanldling, by allowing netlink commands
> to register their own dump_one_dev() callback. This avoids having to
> roll with a fully custom genl ->dumpit callback, allowing the re-use
> of some ethnl plumbing.
> 
> Fallback to the default dump_one_dev behaviour when no custom callback
> is found.
> 
> The command dump context is maintained within the ethnl_dump_ctx, that
> we move in netlink.h so that command handlers can access it.
> 
> This context can be allocated/freed in new ->dump_start() and
> ->dump_done() callbacks.  
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> V4 : No changes
> 
>  net/ethtool/netlink.c | 61 +++++++++++++++++++++++++------------------
>  net/ethtool/netlink.h | 35 +++++++++++++++++++++++++
>  2 files changed, 70 insertions(+), 26 deletions(-)
> 
> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
> index 63ede3638708..0345bffa0678 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -339,24 +339,6 @@ int ethnl_multicast(struct sk_buff *skb, struct
> net_device *dev) 
>  /* GET request helpers */
>  
> -/**
> - * struct ethnl_dump_ctx - context structure for generic dumpit() callback
> - * @ops:        request ops of currently processed message type
> - * @req_info:   parsed request header of processed request
> - * @reply_data: data needed to compose the reply
> - * @pos_ifindex: saved iteration position - ifindex
> - *
> - * These parameters are kept in struct netlink_callback as context preserved
> - * between iterations. They are initialized by ethnl_default_start() and used
> - * in ethnl_default_dumpit() and ethnl_default_done().
> - */
> -struct ethnl_dump_ctx {
> -	const struct ethnl_request_ops	*ops;
> -	struct ethnl_req_info		*req_info;
> -	struct ethnl_reply_data		*reply_data;
> -	unsigned long			pos_ifindex;
> -};
> -
>  static const struct ethnl_request_ops *
>  ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
>  	[ETHTOOL_MSG_STRSET_GET]	= &ethnl_strset_request_ops,
> @@ -540,9 +522,9 @@ static int ethnl_default_doit(struct sk_buff *skb, struct
> genl_info *info) return ret;
>  }
>  
> -static int ethnl_default_dump_one_dev(struct sk_buff *skb, struct net_device
> *dev,
> -				      const struct ethnl_dump_ctx *ctx,
> -				      const struct genl_info *info)
> +static int ethnl_default_dump_one(struct sk_buff *skb,
> +				  const struct ethnl_dump_ctx *ctx,
> +				  const struct genl_info *info)
>  {
>  	void *ehdr;
>  	int ret;
> @@ -553,15 +535,15 @@ static int ethnl_default_dump_one_dev(struct sk_buff
> *skb, struct net_device *de if (!ehdr)
>  		return -EMSGSIZE;
>  
> -	ethnl_init_reply_data(ctx->reply_data, ctx->ops, dev);
>  	rtnl_lock();
> -	netdev_lock_ops(dev);
> +	netdev_lock_ops(ctx->reply_data->dev);
>  	ret = ctx->ops->prepare_data(ctx->req_info, ctx->reply_data, info);
> -	netdev_unlock_ops(dev);
> +	netdev_unlock_ops(ctx->reply_data->dev);
>  	rtnl_unlock();
>  	if (ret < 0)
>  		goto out;
> -	ret = ethnl_fill_reply_header(skb, dev, ctx->ops->hdr_attr);
> +	ret = ethnl_fill_reply_header(skb, ctx->reply_data->dev,
> +				      ctx->ops->hdr_attr);

Instead of modifying all these lines you could simply add this at the beginning:
struct net_device *dev = ctx->reply_data->dev;

With that:
Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

Thank you!

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

Re: [PATCH net-next v4 5/8] net: ethtool: netlink: Introduce per-phy DUMP helpers

[Kory Maincent]

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

> As there are multiple ethnl commands that report messages based on
> phy_device information, let's introduce a set of ethnl generic dump
> helpers to allow DUMP support for each PHY on a given netdev.
> 
> This logic iterates over the phy_link_topology of each netdev (or a
> single netdev for filtered DUMP), and call ethnl_default_dump_one() with
> the req_info populated with ifindex + phyindex.
> 
> This allows re-using all the existing infra for phy-targetting commands
> that already use ethnl generic helpers.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> V4 : No changes
> 
>  net/ethtool/netlink.c | 78 +++++++++++++++++++++++++++++++++++++++++++
>  net/ethtool/netlink.h |  6 ++++
>  2 files changed, 84 insertions(+)
> 
> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
> index 0345bffa0678..171290eaf406 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -560,6 +560,84 @@ static int ethnl_default_dump_one(struct sk_buff *skb,
>  	return ret;
>  }
>  
> +/* Specific context for phy-targeting command DUMP operatins. We keep in
> context
> + * the latest phy_index we dumped, in case of an interrupted DUMP.
> + */
> +struct ethnl_dump_ctx_perphy {
> +	unsigned long phy_index;
> +};
> +
> +/**
> + * ethnl_dump_start_perphy() - Initialise a dump for PHY cmds
> + * @ctx: Generic ethnl dump context whose cmd_ctx will be initialized
> + *
> + * Initializes a dump context for ethnl commands that may return
> + * one message per PHY on a single netdev.
> + *
> + * Returns: 0 for success, a negative value for errors.

Minimal nitpick. In the kernel doc Return does not take a s.

> +/**
> + * ethnl_dump_one_dev_perphy() - Dump all PHY-related messages for one netdev
> + * @skb: skb containing the DUMP result
> + * @ctx: Dump context. Will be kept across the DUMP operation.
> + * @info: Genl receive info
> + *
> + * Some commands are related to PHY devices attached to netdevs. As there
> may be
> + * multiple PHYs, this DUMP handler will populate the reply with one message
> per
> + * PHY on a single netdev.
> + *
> + * Returns: 0 for success or when nothing to do, a negative value otherwise.

Same.

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

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

Re: [PATCH net-next v4 8/8] net: ethtool: pse-pd: Use per-PHY DUMP operations

[Kory Maincent]

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

> Leverage the per-phy ethnl DUMP helpers in case we have more that one
> PSE PHY on the link.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> V4 : No changes
> 
>  net/ethtool/pse-pd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
> index 4f6b99eab2a6..f3d14be8bdd9 100644
> --- a/net/ethtool/pse-pd.c
> +++ b/net/ethtool/pse-pd.c
> @@ -314,4 +314,10 @@ const struct ethnl_request_ops ethnl_pse_request_ops = {
>  
>  	.set			= ethnl_set_pse,
>  	/* PSE has no notification */
> +
> +	.dump_start		= ethnl_dump_start_perphy,
> +	.dump_one_dev		= ethnl_dump_one_dev_perphy,
> +	.dump_done		= ethnl_dump_done_perphy,
> +
> +	.allow_pernetdev_dump	= true,
>  };

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

Thank you!

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

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

[Jakub Kicinski]

On Tue, 25 Mar 2025 12:27:06 +0100 Kory Maincent wrote:
> > @@ -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.

Let's try. We can probably make required_dev attr of
ethnl_parse_header_dev_get() a three state one: require, allow, reject?

Part of the problem is that ethtool is not converted to split ops, so
do and dump share the same parsing policy. But that's too painful to
fix now, I think.
-- 
pw-bot: cr

Re: [PATCH net-next v4 3/8] net: ethtool: netlink: Rename ethnl_default_dump_one

[Jakub Kicinski]

On Mon, 24 Mar 2025 11:40:05 +0100 Maxime Chevallier wrote:
> As we work on getting more objects out of a per-netdev DUMP, rename
> ethnl_default_dump_one() into ethnl_default_dump_one_dev(), making it
> explicit that this dumps everything for one netdev.

Maybe ethnl_default_dump_dev() ?

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

[Jakub Kicinski]

On Tue, 25 Mar 2025 14:15:07 -0700 Jakub Kicinski wrote:
> > 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.  
> 
> 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.

Re: [PATCH net-next v4 0/8] net: ethtool: Introduce ethnl dump helpers

[Jakub Kicinski]

On Mon, 24 Mar 2025 11:40:02 +0100 Maxime Chevallier wrote:
> The patches 1 and 2 introduce the support for filtered DUMPs, where an
> ifindex/ifname can be passed in the request header for the DUMP
> operation. This is for when we want to dump everything a netdev
> supports, but without doing so for every single netdev. ethtool's
> "--show-phys ethX" option for example performs a filtered dump.
> 
> Patch 3 introduces 3 new ethnl ops : 
>  ->dump_start() to initialize a dump context
>  ->dump_one_dev(), that can be implemented per-command to dump  
>  everything on a given netdev
>  ->dump_done() to release the context  

Did you consider ignoring the RSS and focusing purely on PHYs?
The 3 callbacks are a bit generic, but we end up primarily
using them for PHY stuff. And the implementations still need 
to call ethnl_req_get_phydev() AFAICT

Re: [PATCH net-next v4 0/8] net: ethtool: Introduce ethnl dump helpers

[Maxime Chevallier]

Hi Jakub,

On Tue, 25 Mar 2025 14:31:11 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 24 Mar 2025 11:40:02 +0100 Maxime Chevallier wrote:
> > The patches 1 and 2 introduce the support for filtered DUMPs, where an
> > ifindex/ifname can be passed in the request header for the DUMP
> > operation. This is for when we want to dump everything a netdev
> > supports, but without doing so for every single netdev. ethtool's
> > "--show-phys ethX" option for example performs a filtered dump.
> > 
> > Patch 3 introduces 3 new ethnl ops :   
> >  ->dump_start() to initialize a dump context
> >  ->dump_one_dev(), that can be implemented per-command to dump    
> >  everything on a given netdev  
> >  ->dump_done() to release the context    
> 
> Did you consider ignoring the RSS and focusing purely on PHYs?
> The 3 callbacks are a bit generic, but we end up primarily
> using them for PHY stuff. And the implementations still need 
> to call ethnl_req_get_phydev() AFAICT

True, one can even argue that the start() and done() aren't really
useful (allocate/free a ctx, we only really need to know the size of
the context), we'd end-up with just one dedicated helper for PHY dump.

I'll rework this and spin a new version when net-next re-opens, and
I'll clarify the DUMP behaviour for filtering, based on the discussin
with Köry.

Thanks a lot for taking a look then,

Maxime

Re: [PATCH net-next v4 3/8] net: ethtool: netlink: Rename ethnl_default_dump_one

[Maxime Chevallier]

On Tue, 25 Mar 2025 14:16:05 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 24 Mar 2025 11:40:05 +0100 Maxime Chevallier wrote:
> > As we work on getting more objects out of a per-netdev DUMP, rename
> > ethnl_default_dump_one() into ethnl_default_dump_one_dev(), making it
> > explicit that this dumps everything for one netdev.  
> 
> Maybe ethnl_default_dump_dev() ?

Sure, that's better indeed :)

Maxime

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

[Maxime Chevallier]

On Tue, 25 Mar 2025 14:22:02 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Tue, 25 Mar 2025 14:15:07 -0700 Jakub Kicinski wrote:
> > > 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.    
> > 
> > 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.

Maxime

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

[Kory Maincent]

On Wed, 26 Mar 2025 08:59:06 +0100
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> On Tue, 25 Mar 2025 14:22:02 -0700
> Jakub Kicinski <kuba@kernel.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?

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.

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

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

[Maxime Chevallier]

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

> On Wed, 26 Mar 2025 08:59:06 +0100
> Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> 
> > On Tue, 25 Mar 2025 14:22:02 -0700
> > Jakub Kicinski <kuba@kernel.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