[PATCH net-next 00/11] ethtool: Generic loopback support

[PATCH net-next 00/11] ethtool: Generic loopback support

[Björn Töpel]

Hi!

Background
==========

This series adds a generic ethtool loopback framework with GET/SET
netlink commands, using a component/id/name/direction model that can
(Hopefully! Please refer to "Open questions" below.) represent
loopback points across the network path (MAC, MODULE, PHY, PCS).

This is the v1 proper of the loopback series, reworked based on
feedback from previous RFC v2 [1].

The main change since the RFC v2 is that LOOPBACK_GET no longer
returns an array of entries in a single doit reply. Instead, it uses
the dumpit infrastructure -- each loopback entry is a separate netlink
message in a DUMP response. This follows the same pattern as PHY_GET
and the perphy helpers in net/ethtool/netlink.c, as suggested by
Maxime.

A filtered DUMP (with a dev-index in the header) lists all loopback
entries for that netdev; an unfiltered DUMP iterates over all netdevs.
The doit handler is also available: when the request includes an
ETHTOOL_A_LOOPBACK_ENTRY nest with component + name, it returns that
specific entry.

The loopback model remains the same: LOOPBACK_GET/SET with a generic
component/name/direction model that can represent loopback points
across the data path -- MODULE, PHY, MAC, and PCS. This series wires
up MODULE/CMIS and MAC as the first users; PHY and PCS return
-EOPNOTSUPP for now.

Loopback lookup and enumeration
===============================

Each loopback entry is uniquely identified by the tuple (component,
id, name). The kernel provides two GET paths:

 - Exact lookup (doit): the user specifies component + name (and
   optionally id) in the request. The kernel dispatches to the right
   component handler: for MAC, it calls the driver's
   get_loopback(dev, name, id, entry) ethtool_op; for MODULE, it
   calls ethtool_cmis_get_loopback(dev, name, entry). Returns a
   single entry or -EOPNOTSUPP.

 - Index enumeration (dump): the kernel iterates a flat index space
   across all components on a device. Each component's
   get_loopback_by_index(dev, index, entry) is tried in order (MAC
   first via ethtool_ops, then MODULE/CMIS). The dump stops when all
   components return -EOPNOTSUPP. This integrates with the generic
   dump_one_dev sub-iterator infrastructure added in patch 1.

SET takes one or more entries, each with component + name + direction,
and dispatches to the driver's set_loopback() ethtool_op (MAC) or
ethtool_cmis_set_loopback() (MODULE).

The Common Management Interface Specification (CMIS) defines four
diagnostic loopback types, characterized by location (Host or Media
Side) and signal direction:

 - Host Side Input (Rx->Tx) -- near-end
 - Host Side Output (Tx->Rx) -- far-end
 - Media Side Input (Rx->Tx) -- near-end
 - Media Side Output (Tx->Rx) -- far-end

Support is detected via Page 13h Byte 128, and loopback is controlled
via Page 13h Bytes 180-183 (one byte per type, one bit per lane).

The CMIS helpers work entirely over get/set_module_eeprom_by_page, so
any driver that already has EEPROM page access gets module loopback
without new ethtool_ops or driver changes.

Currently, only mellanox/mlxsw, and broadcom/bnxt support CMIS
operations. I'll follow-up with mlx5 support.

Implementation
==============

Patch 1/11 ethtool: Add dump_one_dev callback for per-device sub-iteration
  Replaces the per-PHY specific dump functions with a generic
  sub-iterator infrastructure driven by a dump_one_dev callback in
  ethnl_request_ops. When ops->dump_one_dev is set,
  ethnl_default_start() saves the target device's ifindex for
  filtered dumps, and ethnl_default_dumpit() delegates per-device
  iteration to the callback. No separate start/dumpit/done functions
  are needed.

Patch 2/11 ethtool: Add loopback netlink UAPI definitions
  Adds the YAML spec and generated UAPI header for the new
  LOOPBACK_GET/SET commands. Each loopback entry carries a component
  type, optional id, name string, supported directions bitmask, and
  current direction.

Patch 3/11 ethtool: Add loopback GET/SET netlink implementation
  Implements GET/SET dispatch in a new loopback.c. GET uses the
  dump_one_dev infrastructure for dump enumeration (by flat index)
  and supports doit exact lookup by (component, id, name) via
  parse_request. SET switches on the component and calls the right
  handler per entry. No components are wired yet.

Patch 4/11 ethtool: Add CMIS loopback helpers for module loopback control
  Adds cmis_loopback.c with the MODULE component handlers and wires
  them into loopback.c's dispatch. GET enumerates entries by index
  (ethtool_cmis_get_loopback_by_index) or looks up by name
  (ethtool_cmis_get_loopback). SET (ethtool_cmis_set_loopback)
  resolves name to control byte indices and enforces mutual
  exclusivity.

Patch 5/11 selftests: drv-net: Add loopback driver test
  Adds loopback_drv.py with generic tests that work on any device
  with module loopback support: enable/disable, direction switching,
  idempotent enable, and rejection while interface is up.

Patch 6/11 ethtool: Add MAC loopback support via ethtool_ops
  Extends struct ethtool_ops with three loopback callbacks for
  driver-level MAC loopback: get_loopback (exact lookup by name/id),
  get_loopback_by_index (dump enumeration), and set_loopback. Wires
  the MAC component into loopback.c's dispatch. For dump enumeration,
  MAC entries are tried first, then MODULE/CMIS entries follow at the
  next index offset.

Patch 7/11 netdevsim: Add MAC loopback simulation
  Implements the three ethtool loopback ops in netdevsim. Exposes a
  single MAC loopback entry ("mac") with both near-end and far-end
  support. State is stored in memory and exposed via debugfs under
  ethtool/mac_lb/{supported,direction}.

Patch 8/11 selftests: drv-net: Add MAC loopback netdevsim test
  Adds loopback_nsim.py with netdevsim-specific tests for MAC
  loopback: entry presence, SET/GET round-trip with debugfs
  verification, and error paths.

Patch 9/11 MAINTAINERS: Add entry for ethtool loopback
  Adds a MAINTAINERS entry for the ethtool loopback subsystem covering
  the core loopback and CMIS loopback netlink implementation, and the
  associated selftests.

Patch 10/11 netdevsim: Add module EEPROM simulation via debugfs
  Adds get/set_module_eeprom_by_page to netdevsim, backed by a
  256-page x 128-byte array exposed via debugfs.

Patch 11/11 selftests: drv-net: Add CMIS loopback netdevsim test
  Extends loopback_nsim.py with netdevsim-specific tests that seed the
  EEPROM via debugfs: capability reporting, EEPROM byte verification,
  combined MAC + MODULE dump, and error paths.

Changes since RFC v2
====================

 - Switched LOOPBACK_GET from doit-with-array to dumpit, where each
   loopback entry is a separate netlink message. Uses the new generic
   dump_one_dev sub-iterator infrastructure instead of duplicating the
   perphy dump pattern. (Maxime)

 - u32 to u8 to represent the enums in the YAML. (Maxime)
   
 - Tried to document the YAML better. (Andrew)

 - Added doit exact lookup by (component, id, name) via
   parse_request, so single-entry GET doesn't need a flat index.

 - Added MAC loopback support via three new ethtool_ops callbacks
   (get_loopback(), get_loopback_by_index(), set_loopback()) with
   netdevsim implementation and tests.

 - Added MAINTAINERS entry.

Limitations
===========

PHY and PCS loopback are defined in the UAPI but not yet implemented.

No per-lane support -- loopback is all-or-nothing (0xff/0x00) across
lanes.

Open questions
==============

 - Is this the right extensibility model? I'd appreciate input from
   other NIC vendors on whether component/name/direction is flexible
   enough for their loopback implementations. Also, from the PHY/port
   folks (Maxime, Russell)! Naveen, please LMK if the MAC side of
   thing, is good enough for Marvell.

 - Are patches 10-11 (netdevsim EEPROM simulation + netdevsim-specific
   tests) worth carrying? They drive the CMIS Page 13h registers from
   debugfs, which gives good coverage without hardware, but it's
   another netdevsim surface to maintain. If the consensus is that the
   generic driver tests (patch 5) are sufficient, I'm happy to drop
   them.

 - Extend mellanox/mlx5 with .set_module_eeprom_by_page() callback. I
   got it to work in [6], but would like feedback from the Mellanox
   folks.

Related work
============

[1] Generic loopback support, RFC v2
  https://lore.kernel.org/netdev/20260219130050.2390226-1-bjorn@kernel.org/
[2] CMIS loopback, RFC v1
  https://lore.kernel.org/netdev/20260219130050.2390226-1-bjorn@kernel.org/
[3] New loopback modes
  https://lore.kernel.org/netdev/20251024044849.1098222-1-hkelam@marvell.com/
[4] PHY loopback
  https://lore.kernel.org/netdev/20240911212713.2178943-1-maxime.chevallier@bootlin.com/
[5] bnxt_en: add .set_module_eeprom_by_page() support
  https://lore.kernel.org/netdev/20250310183129.3154117-8-michael.chan@broadcom.com/
[6] net/mlx5e: Implement set_module_eeprom_by_page ethtool callback
  https://lore.kernel.org/netdev/20260219130050.2390226-5-bjorn@kernel.org/


Björn Töpel (11):
  ethtool: Add dump_one_dev callback for per-device sub-iteration
  ethtool: Add loopback netlink UAPI definitions
  ethtool: Add loopback GET/SET netlink implementation
  ethtool: Add CMIS loopback helpers for module loopback control
  selftests: drv-net: Add loopback driver test
  ethtool: Add MAC loopback support via ethtool_ops
  netdevsim: Add MAC loopback simulation
  selftests: drv-net: Add MAC loopback netdevsim test
  MAINTAINERS: Add entry for ethtool loopback
  netdevsim: Add module EEPROM simulation via debugfs
  selftests: drv-net: Add CMIS loopback netdevsim test

 Documentation/netlink/specs/ethtool.yaml      | 123 ++++++
 MAINTAINERS                                   |   6 +
 drivers/net/netdevsim/ethtool.c               | 147 +++++++
 drivers/net/netdevsim/netdevsim.h             |  15 +
 include/linux/ethtool.h                       |  23 +
 .../uapi/linux/ethtool_netlink_generated.h    |  59 +++
 net/ethtool/Makefile                          |   2 +-
 net/ethtool/cmis_loopback.c                   | 407 ++++++++++++++++++
 net/ethtool/loopback.c                        | 341 +++++++++++++++
 net/ethtool/mse.c                             |   1 +
 net/ethtool/netlink.c                         | 285 ++++--------
 net/ethtool/netlink.h                         |  45 ++
 net/ethtool/phy.c                             |   1 +
 net/ethtool/plca.c                            |   2 +
 net/ethtool/pse-pd.c                          |   1 +
 .../testing/selftests/drivers/net/hw/Makefile |   2 +
 .../selftests/drivers/net/hw/loopback_drv.py  | 226 ++++++++++
 .../selftests/drivers/net/hw/loopback_nsim.py | 340 +++++++++++++++
 18 files changed, 1820 insertions(+), 206 deletions(-)
 create mode 100644 net/ethtool/cmis_loopback.c
 create mode 100644 net/ethtool/loopback.c
 create mode 100755 tools/testing/selftests/drivers/net/hw/loopback_drv.py
 create mode 100755 tools/testing/selftests/drivers/net/hw/loopback_nsim.py


base-commit: 52ede1bce557c66309f41ac29dd190be23ca9129
-- 
2.53.0

[PATCH net-next 01/11] ethtool: Add dump_one_dev callback for per-device sub-iteration

[Björn Töpel]

The per-PHY specific dump functions share a lot functionality of with
the default dumpit infrastructure, but does not share the actual code.
By introducing a new sub-iterator function, the two dumpit variants
can be folded into one set of functions.

Add a new dump_one_dev callback in ethnl_request_ops. When
ops->dump_one_dev is set, ethnl_default_start() saves the target
device's ifindex for filtered dumps, and ethnl_default_dumpit()
delegates per-device iteration to the callback instead of calling
ethnl_default_dump_one() directly. No separate start/dumpit/done
functions are needed.

For the existing per-PHY commands (PSE, PLCA, PHY, MSE), the shared
ethnl_perphy_dump_one_dev helper provides the xa_for_each_start loop
over the device's PHY topology.

This prepares the ethtool infrastructure for other commands that need
similar per-device sub-iteration.

Suggested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 net/ethtool/mse.c     |   1 +
 net/ethtool/netlink.c | 261 ++++++++++--------------------------------
 net/ethtool/netlink.h |  31 +++++
 net/ethtool/phy.c     |   1 +
 net/ethtool/plca.c    |   2 +
 net/ethtool/pse-pd.c  |   1 +
 6 files changed, 94 insertions(+), 203 deletions(-)

diff --git a/net/ethtool/mse.c b/net/ethtool/mse.c
index 8cb3fc5e7be4..0f2709b4f7de 100644
--- a/net/ethtool/mse.c
+++ b/net/ethtool/mse.c
@@ -325,4 +325,5 @@ const struct ethnl_request_ops ethnl_mse_request_ops = {
 	.cleanup_data = mse_cleanup_data,
 	.reply_size = mse_reply_size,
 	.fill_reply = mse_fill_reply,
+	.dump_one_dev = ethnl_perphy_dump_one_dev,
 };
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 6e5f0f4f815a..e740b11a0609 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -345,36 +345,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;
-};
-
-/**
- * struct ethnl_perphy_dump_ctx - context for dumpit() PHY-aware callbacks
- * @ethnl_ctx: generic ethnl context
- * @ifindex: For Filtered DUMP requests, the ifindex of the targeted netdev
- * @pos_phyindex: iterator position for multi-msg DUMP
- */
-struct ethnl_perphy_dump_ctx {
-	struct ethnl_dump_ctx	ethnl_ctx;
-	unsigned int		ifindex;
-	unsigned long		pos_phyindex;
-};
-
 static const struct ethnl_request_ops *
 ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_STRSET_GET]	= &ethnl_strset_request_ops,
@@ -428,12 +398,6 @@ static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
 	return (struct ethnl_dump_ctx *)cb->ctx;
 }
 
-static struct ethnl_perphy_dump_ctx *
-ethnl_perphy_dump_context(struct netlink_callback *cb)
-{
-	return (struct ethnl_perphy_dump_ctx *)cb->ctx;
-}
-
 /**
  * ethnl_default_parse() - Parse request message
  * @req_info:    pointer to structure to put data into
@@ -616,17 +580,41 @@ static int ethnl_default_dumpit(struct sk_buff *skb,
 				struct netlink_callback *cb)
 {
 	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
+	const struct genl_info *info = genl_info_dump(cb);
 	struct net *net = sock_net(skb->sk);
 	netdevice_tracker dev_tracker;
 	struct net_device *dev;
 	int ret = 0;
 
+	if (ctx->ops->dump_one_dev && ctx->ifindex) {
+		dev = netdev_get_by_index(net, ctx->ifindex, &dev_tracker,
+					  GFP_KERNEL);
+		if (!dev)
+			return -ENODEV;
+
+		ctx->req_info->dev = dev;
+		ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub, info);
+
+		if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
+			ret = skb->len;
+
+		netdev_put(dev, &dev_tracker);
+		return ret;
+	}
+
 	rcu_read_lock();
 	for_each_netdev_dump(net, dev, ctx->pos_ifindex) {
 		netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
 		rcu_read_unlock();
 
-		ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb));
+		if (ctx->ops->dump_one_dev) {
+			ctx->req_info->dev = dev;
+			ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub,
+						     info);
+			ctx->req_info->dev = NULL;
+		} else {
+			ret = ethnl_default_dump_one(skb, dev, ctx, info);
+		}
 
 		rcu_read_lock();
 		netdev_put(dev, &dev_tracker);
@@ -673,10 +661,13 @@ static int ethnl_default_start(struct netlink_callback *cb)
 	if (ret < 0)
 		goto free_reply_data;
 	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 (ops->dump_one_dev) {
+			/* Sub-iterator dumps keep track of the dev's ifindex
+			 * so the dumpit handler can grab/release the netdev
+			 * per iteration.
+			 */
+			ctx->ifindex = req_info->dev->ifindex;
+		}
 		netdev_put(req_info->dev, &req_info->dev_tracker);
 		req_info->dev = NULL;
 	}
@@ -696,169 +687,33 @@ static int ethnl_default_start(struct netlink_callback *cb)
 	return ret;
 }
 
-/* per-PHY ->start() handler for GET requests */
-static int ethnl_perphy_start(struct netlink_callback *cb)
+/* Shared dump_one_dev for per-PHY commands (PSE, PLCA, PHY, MSE) */
+int ethnl_perphy_dump_one_dev(struct sk_buff *skb,
+			      struct ethnl_dump_ctx *ctx,
+			      unsigned long *pos_sub,
+			      const struct genl_info *info)
 {
-	struct ethnl_perphy_dump_ctx *phy_ctx = ethnl_perphy_dump_context(cb);
-	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
-	struct ethnl_dump_ctx *ctx = &phy_ctx->ethnl_ctx;
-	struct ethnl_reply_data *reply_data;
-	const struct ethnl_request_ops *ops;
-	struct ethnl_req_info *req_info;
-	struct genlmsghdr *ghdr;
-	int ret;
-
-	BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
-
-	ghdr = nlmsg_data(cb->nlh);
-	ops = ethnl_default_requests[ghdr->cmd];
-	if (WARN_ONCE(!ops, "cmd %u has no ethnl_request_ops\n", ghdr->cmd))
-		return -EOPNOTSUPP;
-	req_info = kzalloc(ops->req_info_size, GFP_KERNEL);
-	if (!req_info)
-		return -ENOMEM;
-	reply_data = kmalloc(ops->reply_data_size, GFP_KERNEL);
-	if (!reply_data) {
-		ret = -ENOMEM;
-		goto free_req_info;
-	}
-
-	/* Unlike per-dev dump, don't ignore dev. The dump handler
-	 * will notice it and dump PHYs from given dev. We only keep track of
-	 * the dev's ifindex, .dumpit() will grab and release the netdev itself.
-	 */
-	ret = ethnl_default_parse(req_info, &info->info, ops, false);
-	if (ret < 0)
-		goto free_reply_data;
-	if (req_info->dev) {
-		phy_ctx->ifindex = req_info->dev->ifindex;
-		netdev_put(req_info->dev, &req_info->dev_tracker);
-		req_info->dev = NULL;
-	}
-
-	ctx->ops = ops;
-	ctx->req_info = req_info;
-	ctx->reply_data = reply_data;
-	ctx->pos_ifindex = 0;
-
-	return 0;
-
-free_reply_data:
-	kfree(reply_data);
-free_req_info:
-	kfree(req_info);
-
-	return ret;
-}
-
-static int ethnl_perphy_dump_one_dev(struct sk_buff *skb,
-				     struct ethnl_perphy_dump_ctx *ctx,
-				     const struct genl_info *info)
-{
-	struct ethnl_dump_ctx *ethnl_ctx = &ctx->ethnl_ctx;
-	struct net_device *dev = ethnl_ctx->req_info->dev;
+	struct net_device *dev = ctx->req_info->dev;
 	struct phy_device_node *pdn;
 	int ret;
 
 	if (!dev->link_topo)
 		return 0;
 
-	xa_for_each_start(&dev->link_topo->phys, ctx->pos_phyindex, pdn,
-			  ctx->pos_phyindex) {
-		ethnl_ctx->req_info->phy_index = ctx->pos_phyindex;
+	xa_for_each_start(&dev->link_topo->phys, *pos_sub, pdn,
+			  *pos_sub) {
+		ctx->req_info->phy_index = *pos_sub;
 
 		/* We can re-use the original dump_one as ->prepare_data in
 		 * commands use ethnl_req_get_phydev(), which gets the PHY from
 		 * the req_info->phy_index
 		 */
-		ret = ethnl_default_dump_one(skb, dev, ethnl_ctx, info);
+		ret = ethnl_default_dump_one(skb, dev, ctx, info);
 		if (ret)
 			return ret;
 	}
 
-	ctx->pos_phyindex = 0;
-
-	return 0;
-}
-
-static int ethnl_perphy_dump_all_dev(struct sk_buff *skb,
-				     struct ethnl_perphy_dump_ctx *ctx,
-				     const struct genl_info *info)
-{
-	struct ethnl_dump_ctx *ethnl_ctx = &ctx->ethnl_ctx;
-	struct net *net = sock_net(skb->sk);
-	netdevice_tracker dev_tracker;
-	struct net_device *dev;
-	int ret = 0;
-
-	rcu_read_lock();
-	for_each_netdev_dump(net, dev, ethnl_ctx->pos_ifindex) {
-		netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
-		rcu_read_unlock();
-
-		/* per-PHY commands use ethnl_req_get_phydev(), which needs the
-		 * net_device in the req_info
-		 */
-		ethnl_ctx->req_info->dev = dev;
-		ret = ethnl_perphy_dump_one_dev(skb, ctx, info);
-
-		rcu_read_lock();
-		netdev_put(dev, &dev_tracker);
-		ethnl_ctx->req_info->dev = NULL;
-
-		if (ret < 0 && ret != -EOPNOTSUPP) {
-			if (likely(skb->len))
-				ret = skb->len;
-			break;
-		}
-		ret = 0;
-	}
-	rcu_read_unlock();
-
-	return ret;
-}
-
-/* per-PHY ->dumpit() handler for GET requests. */
-static int ethnl_perphy_dumpit(struct sk_buff *skb,
-			       struct netlink_callback *cb)
-{
-	struct ethnl_perphy_dump_ctx *ctx = ethnl_perphy_dump_context(cb);
-	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
-	struct ethnl_dump_ctx *ethnl_ctx = &ctx->ethnl_ctx;
-	int ret = 0;
-
-	if (ctx->ifindex) {
-		netdevice_tracker dev_tracker;
-		struct net_device *dev;
-
-		dev = netdev_get_by_index(genl_info_net(&info->info),
-					  ctx->ifindex, &dev_tracker,
-					  GFP_KERNEL);
-		if (!dev)
-			return -ENODEV;
-
-		ethnl_ctx->req_info->dev = dev;
-		ret = ethnl_perphy_dump_one_dev(skb, ctx, genl_info_dump(cb));
-
-		if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
-			ret = skb->len;
-
-		netdev_put(dev, &dev_tracker);
-	} else {
-		ret = ethnl_perphy_dump_all_dev(skb, ctx, genl_info_dump(cb));
-	}
-
-	return ret;
-}
-
-/* per-PHY ->done() handler for GET requests */
-static int ethnl_perphy_done(struct netlink_callback *cb)
-{
-	struct ethnl_perphy_dump_ctx *ctx = ethnl_perphy_dump_context(cb);
-	struct ethnl_dump_ctx *ethnl_ctx = &ctx->ethnl_ctx;
-
-	kfree(ethnl_ctx->reply_data);
-	kfree(ethnl_ctx->req_info);
+	*pos_sub = 0;
 
 	return 0;
 }
@@ -1420,9 +1275,9 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PSE_GET,
 		.doit	= ethnl_default_doit,
-		.start	= ethnl_perphy_start,
-		.dumpit	= ethnl_perphy_dumpit,
-		.done	= ethnl_perphy_done,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
 		.policy = ethnl_pse_get_policy,
 		.maxattr = ARRAY_SIZE(ethnl_pse_get_policy) - 1,
 	},
@@ -1444,9 +1299,9 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PLCA_GET_CFG,
 		.doit	= ethnl_default_doit,
-		.start	= ethnl_perphy_start,
-		.dumpit	= ethnl_perphy_dumpit,
-		.done	= ethnl_perphy_done,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
 		.policy = ethnl_plca_get_cfg_policy,
 		.maxattr = ARRAY_SIZE(ethnl_plca_get_cfg_policy) - 1,
 	},
@@ -1460,9 +1315,9 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PLCA_GET_STATUS,
 		.doit	= ethnl_default_doit,
-		.start	= ethnl_perphy_start,
-		.dumpit	= ethnl_perphy_dumpit,
-		.done	= ethnl_perphy_done,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
 		.policy = ethnl_plca_get_status_policy,
 		.maxattr = ARRAY_SIZE(ethnl_plca_get_status_policy) - 1,
 	},
@@ -1492,9 +1347,9 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PHY_GET,
 		.doit	= ethnl_default_doit,
-		.start	= ethnl_perphy_start,
-		.dumpit	= ethnl_perphy_dumpit,
-		.done	= ethnl_perphy_done,
+		.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,
 	},
@@ -1538,9 +1393,9 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_MSE_GET,
 		.doit	= ethnl_default_doit,
-		.start	= ethnl_perphy_start,
-		.dumpit	= ethnl_perphy_dumpit,
-		.done	= ethnl_perphy_done,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
 		.policy = ethnl_mse_get_policy,
 		.maxattr = ARRAY_SIZE(ethnl_mse_get_policy) - 1,
 	},
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 89010eaa67df..aa8d51903ecc 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -10,6 +10,28 @@
 
 struct ethnl_req_info;
 
+/**
+ * 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
+ * @ifindex:    for filtered dump requests, the ifindex of the targeted netdev
+ * @pos_sub:    iterator position for per-device iteration
+ *
+ * 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;
+	unsigned int			ifindex;
+	unsigned long			pos_sub;
+};
+
 u32 ethnl_bcast_seq_next(void);
 int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
 			       const struct nlattr *nest, struct net *net,
@@ -408,6 +430,11 @@ struct ethnl_request_ops {
 			  const struct ethnl_reply_data *reply_data);
 	void (*cleanup_data)(struct ethnl_reply_data *reply_data);
 
+	int (*dump_one_dev)(struct sk_buff *skb,
+			    struct ethnl_dump_ctx *ctx,
+			    unsigned long *pos_sub,
+			    const struct genl_info *info);
+
 	int (*set_validate)(struct ethnl_req_info *req_info,
 			    struct genl_info *info);
 	int (*set)(struct ethnl_req_info *req_info,
@@ -514,6 +541,10 @@ int ethnl_tsinfo_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 int ethnl_tsinfo_done(struct netlink_callback *cb);
 int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info);
 int ethnl_rss_delete_doit(struct sk_buff *skb, struct genl_info *info);
+int ethnl_perphy_dump_one_dev(struct sk_buff *skb,
+			      struct ethnl_dump_ctx *ctx,
+			      unsigned long *pos_sub,
+			      const struct genl_info *info);
 
 extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN];
 extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN];
diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index 68372bef4b2f..4158c2abb235 100644
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -162,4 +162,5 @@ const struct ethnl_request_ops ethnl_phy_request_ops = {
 	.reply_size		= phy_reply_size,
 	.fill_reply		= phy_fill_reply,
 	.cleanup_data		= phy_cleanup_data,
+	.dump_one_dev		= ethnl_perphy_dump_one_dev,
 };
diff --git a/net/ethtool/plca.c b/net/ethtool/plca.c
index e1f7820a6158..cad0ba476c29 100644
--- a/net/ethtool/plca.c
+++ b/net/ethtool/plca.c
@@ -188,6 +188,7 @@ const struct ethnl_request_ops ethnl_plca_cfg_request_ops = {
 	.prepare_data		= plca_get_cfg_prepare_data,
 	.reply_size		= plca_get_cfg_reply_size,
 	.fill_reply		= plca_get_cfg_fill_reply,
+	.dump_one_dev		= ethnl_perphy_dump_one_dev,
 
 	.set			= ethnl_set_plca,
 	.set_ntf_cmd		= ETHTOOL_MSG_PLCA_NTF,
@@ -268,4 +269,5 @@ 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_one_dev		= ethnl_perphy_dump_one_dev,
 };
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index 24def9c9dd54..1442a59e033f 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -337,6 +337,7 @@ const struct ethnl_request_ops ethnl_pse_request_ops = {
 	.reply_size		= pse_reply_size,
 	.fill_reply		= pse_fill_reply,
 	.cleanup_data		= pse_cleanup_data,
+	.dump_one_dev		= ethnl_perphy_dump_one_dev,
 
 	.set			= ethnl_set_pse,
 	/* PSE has no notification */
-- 
2.53.0

[PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Björn Töpel]

Add the netlink YAML spec and auto-generated UAPI header for a unified
loopback interface covering MAC, PCS, PHY, and pluggable module
components.

Each loopback point is described by a nested entry attribute
containing:

 - component  where in the path (MAC, PCS, PHY, MODULE)
 - name       subsystem label, e.g. "cmis-host" or "cmis-media"
 - id         optional instance selector (e.g. PHY id, port id)
 - supported  bitmask of supported directions
 - direction  NEAR_END, FAR_END, or 0 (disabled)

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 Documentation/netlink/specs/ethtool.yaml      | 123 ++++++++++++++++++
 .../uapi/linux/ethtool_netlink_generated.h    |  59 +++++++++
 2 files changed, 182 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 4707063af3b4..8bd14a3c946a 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -211,6 +211,49 @@ definitions:
         name: discard
         value: 31
 
+  -
+    name: loopback-component
+    type: enum
+    doc: |
+      Loopback component. Identifies where in the network path the
+      loopback is applied.
+    entries:
+      -
+        name: mac
+        doc: MAC loopback. Loops traffic at the MAC block.
+      -
+        name: pcs
+        doc: |
+          PCS loopback. Loops traffic at the PCS sublayer between the
+          MAC and the PHY.
+      -
+        name: phy
+        doc: |
+          Ethernet PHY loopback. This refers to the Ethernet PHY managed
+          by phylib, not generic PHY drivers. A Base-T SFP module
+          containing an Ethernet PHY driven by Linux should report
+          loopback under this component, not module.
+      -
+        name: module
+        doc: |
+          Pluggable module (e.g. CMIS (Q)SFP) loopback. Covers loopback
+          modes controlled via module firmware or EEPROM registers. When
+          Linux drives an Ethernet PHY inside the module via phylib, use
+          the phy component instead.
+  -
+    name: loopback-direction
+    type: flags
+    doc: |
+      Loopback direction flags. Used as a bitmask in supported, and as
+      a single value in direction.
+    entries:
+      -
+        name: near-end
+        doc: Near-end loopback; host-loop-host
+      -
+        name: far-end
+        doc: Far-end loopback; line-loop-line
+
 attribute-sets:
   -
     name: header
@@ -1903,6 +1946,58 @@ attribute-sets:
         name: link
         type: nest
         nested-attributes: mse-snapshot
+  -
+    name: loopback-entry
+    doc: Per-component loopback configuration entry.
+    attr-cnt-name: __ethtool-a-loopback-entry-cnt
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: component
+        type: u32
+        enum: loopback-component
+        doc: Loopback component
+      -
+        name: id
+        type: u32
+        doc: Optional component instance identifier.
+      -
+        name: name
+        type: string
+        doc: |
+          Subsystem-specific name for the loopback point within the
+          component.
+      -
+        name: supported
+        type: u8
+        enum: loopback-direction
+        enum-as-flags: true
+        doc: Bitmask of supported loopback directions
+      -
+        name: direction
+        type: u8
+        enum: loopback-direction
+        doc: Current loopback direction, 0 means disabled
+  -
+    name: loopback
+    attr-cnt-name: __ethtool-a-loopback-cnt
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: header
+        type: nest
+        nested-attributes: header
+      -
+        name: entry
+        type: nest
+        multi-attr: true
+        nested-attributes: loopback-entry
 
 operations:
   enum-model: directional
@@ -2855,6 +2950,34 @@ operations:
             - worst-channel
             - link
       dump: *mse-get-op
+    -
+      name: loopback-get
+      doc: Get loopback configuration and capabilities.
+
+      attribute-set: loopback
+
+      do: &loopback-get-op
+        request:
+          attributes:
+            - header
+        reply:
+          attributes: &loopback
+            - header
+            - entry
+      dump: *loopback-get-op
+    -
+      name: loopback-set
+      doc: Set loopback configuration.
+
+      attribute-set: loopback
+
+      do:
+        request:
+          attributes: *loopback
+    -
+      name: loopback-ntf
+      doc: Notification for change in loopback configuration.
+      notify: loopback-get
 
 mcast-groups:
   list:
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index 114b83017297..d8bff056a4b1 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -78,6 +78,40 @@ enum ethtool_pse_event {
 	ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR = 64,
 };
 
+/**
+ * enum ethtool_loopback_component - Loopback component. Identifies where in
+ *   the network path the loopback is applied.
+ * @ETHTOOL_LOOPBACK_COMPONENT_MAC: MAC loopback. Loops traffic at the MAC
+ *   block.
+ * @ETHTOOL_LOOPBACK_COMPONENT_PCS: PCS loopback. Loops traffic at the PCS
+ *   sublayer between the MAC and the PHY.
+ * @ETHTOOL_LOOPBACK_COMPONENT_PHY: Ethernet PHY loopback. This refers to the
+ *   Ethernet PHY managed by phylib, not generic PHY drivers. A Base-T SFP
+ *   module containing an Ethernet PHY driven by Linux should report loopback
+ *   under this component, not module.
+ * @ETHTOOL_LOOPBACK_COMPONENT_MODULE: Pluggable module (e.g. CMIS (Q)SFP)
+ *   loopback. Covers loopback modes controlled via module firmware or EEPROM
+ *   registers. When Linux drives an Ethernet PHY inside the module via phylib,
+ *   use the phy component instead.
+ */
+enum ethtool_loopback_component {
+	ETHTOOL_LOOPBACK_COMPONENT_MAC,
+	ETHTOOL_LOOPBACK_COMPONENT_PCS,
+	ETHTOOL_LOOPBACK_COMPONENT_PHY,
+	ETHTOOL_LOOPBACK_COMPONENT_MODULE,
+};
+
+/**
+ * enum ethtool_loopback_direction - Loopback direction flags. Used as a
+ *   bitmask in supported, and as a single value in direction.
+ * @ETHTOOL_LOOPBACK_DIRECTION_NEAR_END: Near-end loopback; host-loop-host
+ * @ETHTOOL_LOOPBACK_DIRECTION_FAR_END: Far-end loopback; line-loop-line
+ */
+enum ethtool_loopback_direction {
+	ETHTOOL_LOOPBACK_DIRECTION_NEAR_END = 1,
+	ETHTOOL_LOOPBACK_DIRECTION_FAR_END = 2,
+};
+
 enum {
 	ETHTOOL_A_HEADER_UNSPEC,
 	ETHTOOL_A_HEADER_DEV_INDEX,
@@ -838,6 +872,27 @@ enum {
 	ETHTOOL_A_MSE_MAX = (__ETHTOOL_A_MSE_CNT - 1)
 };
 
+enum {
+	ETHTOOL_A_LOOPBACK_ENTRY_UNSPEC,
+	ETHTOOL_A_LOOPBACK_ENTRY_COMPONENT,
+	ETHTOOL_A_LOOPBACK_ENTRY_ID,
+	ETHTOOL_A_LOOPBACK_ENTRY_NAME,
+	ETHTOOL_A_LOOPBACK_ENTRY_SUPPORTED,
+	ETHTOOL_A_LOOPBACK_ENTRY_DIRECTION,
+
+	__ETHTOOL_A_LOOPBACK_ENTRY_CNT,
+	ETHTOOL_A_LOOPBACK_ENTRY_MAX = (__ETHTOOL_A_LOOPBACK_ENTRY_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_LOOPBACK_UNSPEC,
+	ETHTOOL_A_LOOPBACK_HEADER,
+	ETHTOOL_A_LOOPBACK_ENTRY,
+
+	__ETHTOOL_A_LOOPBACK_CNT,
+	ETHTOOL_A_LOOPBACK_MAX = (__ETHTOOL_A_LOOPBACK_CNT - 1)
+};
+
 enum {
 	ETHTOOL_MSG_USER_NONE = 0,
 	ETHTOOL_MSG_STRSET_GET = 1,
@@ -891,6 +946,8 @@ enum {
 	ETHTOOL_MSG_RSS_CREATE_ACT,
 	ETHTOOL_MSG_RSS_DELETE_ACT,
 	ETHTOOL_MSG_MSE_GET,
+	ETHTOOL_MSG_LOOPBACK_GET,
+	ETHTOOL_MSG_LOOPBACK_SET,
 
 	__ETHTOOL_MSG_USER_CNT,
 	ETHTOOL_MSG_USER_MAX = (__ETHTOOL_MSG_USER_CNT - 1)
@@ -952,6 +1009,8 @@ enum {
 	ETHTOOL_MSG_RSS_CREATE_NTF,
 	ETHTOOL_MSG_RSS_DELETE_NTF,
 	ETHTOOL_MSG_MSE_GET_REPLY,
+	ETHTOOL_MSG_LOOPBACK_GET_REPLY,
+	ETHTOOL_MSG_LOOPBACK_NTF,
 
 	__ETHTOOL_MSG_KERNEL_CNT,
 	ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
-- 
2.53.0

[PATCH net-next 03/11] ethtool: Add loopback GET/SET netlink implementation

[Björn Töpel]

Add the kernel-side ETHTOOL_MSG_LOOPBACK_GET,
ETHTOOL_MSG_LOOPBACK_SET, and ETHTOOL_MSG_LOOPBACK_NTF handlers using
the standard ethnl_request_ops infrastructure.

GET collects loopback entries from per-component helpers via
loopback_get_entries(). SET parses the nested entry attributes,
dispatches each to loopback_set_one(), and only sends a notification
when the state is changed.

No components are wired yet.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 include/linux/ethtool.h |  16 +++
 net/ethtool/Makefile    |   2 +-
 net/ethtool/loopback.c  | 305 ++++++++++++++++++++++++++++++++++++++++
 net/ethtool/netlink.c   |  24 +++-
 net/ethtool/netlink.h   |   6 +
 5 files changed, 350 insertions(+), 3 deletions(-)
 create mode 100644 net/ethtool/loopback.c

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 83c375840835..c9beca11fc40 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -846,6 +846,22 @@ void ethtool_mmsv_set_mm(struct ethtool_mmsv *mmsv, struct ethtool_mm_cfg *cfg);
 void ethtool_mmsv_init(struct ethtool_mmsv *mmsv, struct net_device *dev,
 		       const struct ethtool_mmsv_ops *ops);
 
+/**
+ * struct ethtool_loopback_entry - Per-component loopback configuration
+ * @id: Optional component instance identifier, 0 means not specified
+ * @supported: Bitmask of supported directions
+ * @component: Loopback component
+ * @direction: Current loopback direction, 0 means disabled
+ * @name: Subsystem-specific name for the loopback point
+ */
+struct ethtool_loopback_entry {
+	enum ethtool_loopback_component component;
+	u32 id;
+	u32 supported;
+	u32 direction;
+	char name[ETH_GSTRING_LEN];
+};
+
 /**
  * struct ethtool_rxfh_param - RXFH (RSS) parameters
  * @hfunc: Defines the current RSS hash function used by HW (or to be set to).
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 629c10916670..ef534b55d724 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -9,4 +9,4 @@ ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o rss.o \
 		   channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
 		   tunnels.o fec.o eeprom.o stats.o phc_vclocks.o mm.o \
 		   module.o cmis_fw_update.o cmis_cdb.o pse-pd.o plca.o \
-		   phy.o tsconfig.o mse.o
+		   phy.o tsconfig.o mse.o loopback.o
diff --git a/net/ethtool/loopback.c b/net/ethtool/loopback.c
new file mode 100644
index 000000000000..8907dd147404
--- /dev/null
+++ b/net/ethtool/loopback.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "netlink.h"
+#include "common.h"
+
+struct loopback_req_info {
+	struct ethnl_req_info base;
+	enum ethtool_loopback_component component;
+	u32 id;
+	char name[ETH_GSTRING_LEN];
+	bool lookup_by_name;
+	u32 index;
+};
+
+#define LOOPBACK_REQINFO(__req_base) \
+	container_of(__req_base, struct loopback_req_info, base)
+
+struct loopback_reply_data {
+	struct ethnl_reply_data base;
+	struct ethtool_loopback_entry entry;
+};
+
+#define LOOPBACK_REPDATA(__reply_base) \
+	container_of(__reply_base, struct loopback_reply_data, base)
+
+/* GET */
+
+static const struct nla_policy
+ethnl_loopback_entry_policy[ETHTOOL_A_LOOPBACK_ENTRY_MAX + 1] = {
+	[ETHTOOL_A_LOOPBACK_ENTRY_COMPONENT] =
+		NLA_POLICY_MAX(NLA_U32, ETHTOOL_LOOPBACK_COMPONENT_MODULE),
+	[ETHTOOL_A_LOOPBACK_ENTRY_ID] = NLA_POLICY_MIN(NLA_U32, 1),
+	[ETHTOOL_A_LOOPBACK_ENTRY_NAME] = { .type = NLA_NUL_STRING,
+					    .len = ETH_GSTRING_LEN },
+	[ETHTOOL_A_LOOPBACK_ENTRY_DIRECTION] =
+		NLA_POLICY_MASK(NLA_U8, ETHTOOL_LOOPBACK_DIRECTION_NEAR_END |
+				ETHTOOL_LOOPBACK_DIRECTION_FAR_END),
+};
+
+const struct nla_policy
+ethnl_loopback_get_policy[ETHTOOL_A_LOOPBACK_MAX + 1] = {
+	[ETHTOOL_A_LOOPBACK_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+	[ETHTOOL_A_LOOPBACK_ENTRY] =
+		NLA_POLICY_NESTED(ethnl_loopback_entry_policy),
+};
+
+static int loopback_parse_request(struct ethnl_req_info *req_base,
+				  struct nlattr **tb,
+				  struct netlink_ext_ack *extack)
+{
+	struct loopback_req_info *req_info = LOOPBACK_REQINFO(req_base);
+	struct nlattr *entry_tb[ETHTOOL_A_LOOPBACK_ENTRY_MAX + 1];
+	int ret;
+
+	if (!tb[ETHTOOL_A_LOOPBACK_ENTRY])
+		return 0;
+
+	ret = nla_parse_nested(entry_tb, ETHTOOL_A_LOOPBACK_ENTRY_MAX,
+			       tb[ETHTOOL_A_LOOPBACK_ENTRY],
+			       ethnl_loopback_entry_policy, extack);
+	if (ret < 0)
+		return ret;
+
+	if (!entry_tb[ETHTOOL_A_LOOPBACK_ENTRY_COMPONENT] ||
+	    !entry_tb[ETHTOOL_A_LOOPBACK_ENTRY_NAME]) {
+		NL_SET_ERR_MSG(extack,
+			       "component and name required for loopback lookup");
+		return -EINVAL;
+	}
+
+	req_info->component =
+		nla_get_u32(entry_tb[ETHTOOL_A_LOOPBACK_ENTRY_COMPONENT]);
+	if (entry_tb[ETHTOOL_A_LOOPBACK_ENTRY_ID])
+		req_info->id =
+			nla_get_u32(entry_tb[ETHTOOL_A_LOOPBACK_ENTRY_ID]);
+	nla_strscpy(req_info->name, entry_tb[ETHTOOL_A_LOOPBACK_ENTRY_NAME],
+		    sizeof(req_info->name));
+	req_info->lookup_by_name = true;
+
+	return 0;
+}
+
+static int loopback_get(struct net_device *dev,
+			enum ethtool_loopback_component component, u32 id,
+			const char *name,
+			struct ethtool_loopback_entry *entry)
+{
+	switch (component) {
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int loopback_get_by_index(struct net_device *dev, u32 index,
+				 struct ethtool_loopback_entry *entry)
+{
+	return -EOPNOTSUPP;
+}
+
+static int loopback_prepare_data(const struct ethnl_req_info *req_base,
+				 struct ethnl_reply_data *reply_base,
+				 const struct genl_info *info)
+{
+	const struct loopback_req_info *req_info = LOOPBACK_REQINFO(req_base);
+	struct loopback_reply_data *data = LOOPBACK_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	int ret;
+
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		return ret;
+
+	if (req_info->lookup_by_name)
+		ret = loopback_get(dev, req_info->component, req_info->id,
+				   req_info->name, &data->entry);
+	else
+		ret = loopback_get_by_index(dev, req_info->index, &data->entry);
+
+	ethnl_ops_complete(dev);
+
+	return ret;
+}
+
+static int loopback_reply_size(const struct ethnl_req_info *req_base,
+			       const struct ethnl_reply_data *reply_base)
+{
+	return nla_total_size(0) +			/* nest */
+	       nla_total_size(sizeof(u32)) +		/* component */
+	       nla_total_size(sizeof(u32)) +		/* id */
+	       nla_total_size(sizeof(u8)) +		/* supported */
+	       nla_total_size(sizeof(u8)) +		/* direction */
+	       nla_total_size(ETH_GSTRING_LEN);		/* name */
+}
+
+static int loopback_fill_reply(struct sk_buff *skb,
+			       const struct ethnl_req_info *req_base,
+			       const struct ethnl_reply_data *reply_base)
+{
+	const struct loopback_reply_data *data = LOOPBACK_REPDATA(reply_base);
+	const struct ethtool_loopback_entry *entry = &data->entry;
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, ETHTOOL_A_LOOPBACK_ENTRY);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (nla_put_u32(skb, ETHTOOL_A_LOOPBACK_ENTRY_COMPONENT,
+			entry->component))
+		goto err_cancel;
+
+	if (entry->id &&
+	    nla_put_u32(skb, ETHTOOL_A_LOOPBACK_ENTRY_ID, entry->id))
+		goto err_cancel;
+
+	if (nla_put_u8(skb, ETHTOOL_A_LOOPBACK_ENTRY_SUPPORTED,
+		       entry->supported) ||
+	    nla_put_u8(skb, ETHTOOL_A_LOOPBACK_ENTRY_DIRECTION,
+		       entry->direction) ||
+	    nla_put_string(skb, ETHTOOL_A_LOOPBACK_ENTRY_NAME,
+			   entry->name))
+		goto err_cancel;
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+err_cancel:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
+/* SET */
+
+const struct nla_policy
+ethnl_loopback_set_policy[ETHTOOL_A_LOOPBACK_MAX + 1] = {
+	[ETHTOOL_A_LOOPBACK_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+	[ETHTOOL_A_LOOPBACK_ENTRY] =
+		NLA_POLICY_NESTED(ethnl_loopback_entry_policy),
+};
+
+static int loopback_parse_entry(struct nlattr *attr,
+				struct ethtool_loopback_entry *entry,
+				struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[ETHTOOL_A_LOOPBACK_ENTRY_MAX + 1];
+	int ret;
+
+	ret = nla_parse_nested(tb, ETHTOOL_A_LOOPBACK_ENTRY_MAX, attr,
+			       ethnl_loopback_entry_policy, extack);
+	if (ret < 0)
+		return ret;
+
+	if (!tb[ETHTOOL_A_LOOPBACK_ENTRY_COMPONENT]) {
+		NL_SET_ERR_MSG_ATTR(extack, attr,
+				    "loopback component is required");
+		return -EINVAL;
+	}
+
+	entry->component = nla_get_u32(tb[ETHTOOL_A_LOOPBACK_ENTRY_COMPONENT]);
+
+	if (tb[ETHTOOL_A_LOOPBACK_ENTRY_ID])
+		entry->id = nla_get_u32(tb[ETHTOOL_A_LOOPBACK_ENTRY_ID]);
+
+	if (!tb[ETHTOOL_A_LOOPBACK_ENTRY_NAME]) {
+		NL_SET_ERR_MSG_ATTR(extack, attr, "loopback name is required");
+		return -EINVAL;
+	}
+	nla_strscpy(entry->name, tb[ETHTOOL_A_LOOPBACK_ENTRY_NAME],
+		    sizeof(entry->name));
+
+	if (!tb[ETHTOOL_A_LOOPBACK_ENTRY_DIRECTION]) {
+		NL_SET_ERR_MSG_ATTR(extack, attr,
+				    "loopback direction is required");
+		return -EINVAL;
+	}
+
+	entry->direction = nla_get_u8(tb[ETHTOOL_A_LOOPBACK_ENTRY_DIRECTION]);
+
+	return 0;
+}
+
+static int __loopback_set(struct net_device *dev,
+			  const struct ethtool_loopback_entry *entry,
+			  struct netlink_ext_ack *extack)
+{
+	switch (entry->component) {
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int loopback_set(struct ethnl_req_info *req_info,
+			struct genl_info *info)
+{
+	struct net_device *dev = req_info->dev;
+	struct ethtool_loopback_entry entry;
+	int rem, ret, mod = 0;
+	struct nlattr *attr;
+	bool found = false;
+
+	nla_for_each_attr(attr, genlmsg_data(info->genlhdr),
+			  genlmsg_len(info->genlhdr), rem) {
+		if (nla_type(attr) != ETHTOOL_A_LOOPBACK_ENTRY)
+			continue;
+
+		found = true;
+		memset(&entry, 0, sizeof(entry));
+		ret = loopback_parse_entry(attr, &entry, info->extack);
+		if (ret < 0)
+			return ret;
+
+		ret = __loopback_set(dev, &entry, info->extack);
+		if (ret < 0)
+			return ret;
+		if (ret > 0)
+			mod = 1;
+	}
+
+	if (!found) {
+		NL_SET_ERR_MSG(info->extack, "no loopback entries specified");
+		return -EINVAL;
+	}
+
+	return mod;
+}
+
+static int loopback_dump_one_dev(struct sk_buff *skb,
+				 struct ethnl_dump_ctx *ctx,
+				 unsigned long *pos_sub,
+				 const struct genl_info *info)
+{
+	struct loopback_req_info *req_info =
+		container_of(ctx->req_info, struct loopback_req_info, base);
+	int ret;
+
+	for (;; (*pos_sub)++) {
+		req_info->index = *pos_sub;
+		ret = ethnl_default_dump_one(skb, ctx->req_info->dev, ctx,
+					     info);
+		if (ret == -EOPNOTSUPP)
+			break;
+		if (ret)
+			return ret;
+	}
+
+	*pos_sub = 0;
+
+	return 0;
+}
+
+const struct ethnl_request_ops ethnl_loopback_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_LOOPBACK_GET,
+	.reply_cmd		= ETHTOOL_MSG_LOOPBACK_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_LOOPBACK_HEADER,
+	.req_info_size		= sizeof(struct loopback_req_info),
+	.reply_data_size	= sizeof(struct loopback_reply_data),
+
+	.parse_request		= loopback_parse_request,
+	.prepare_data		= loopback_prepare_data,
+	.reply_size		= loopback_reply_size,
+	.fill_reply		= loopback_fill_reply,
+	.dump_one_dev		= loopback_dump_one_dev,
+
+	.set			= loopback_set,
+	.set_ntf_cmd		= ETHTOOL_MSG_LOOPBACK_NTF,
+};
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index e740b11a0609..25b2fca05bd8 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -391,6 +391,8 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_TSCONFIG_SET]	= &ethnl_tsconfig_request_ops,
 	[ETHTOOL_MSG_PHY_GET]		= &ethnl_phy_request_ops,
 	[ETHTOOL_MSG_MSE_GET]		= &ethnl_mse_request_ops,
+	[ETHTOOL_MSG_LOOPBACK_GET]	= &ethnl_loopback_request_ops,
+	[ETHTOOL_MSG_LOOPBACK_SET]	= &ethnl_loopback_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -537,8 +539,8 @@ 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,
+int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev,
+			   const struct ethnl_dump_ctx *ctx,
 				  const struct genl_info *info)
 {
 	void *ehdr;
@@ -817,6 +819,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
 	[ETHTOOL_MSG_MM_NTF]		= &ethnl_mm_request_ops,
 	[ETHTOOL_MSG_RSS_NTF]		= &ethnl_rss_request_ops,
 	[ETHTOOL_MSG_RSS_CREATE_NTF]	= &ethnl_rss_request_ops,
+	[ETHTOOL_MSG_LOOPBACK_NTF]	= &ethnl_loopback_request_ops,
 };
 
 /* default notification handler */
@@ -925,6 +928,7 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
 	[ETHTOOL_MSG_MM_NTF]		= ethnl_default_notify,
 	[ETHTOOL_MSG_RSS_NTF]		= ethnl_default_notify,
 	[ETHTOOL_MSG_RSS_CREATE_NTF]	= ethnl_default_notify,
+	[ETHTOOL_MSG_LOOPBACK_NTF]	= ethnl_default_notify,
 };
 
 void ethnl_notify(struct net_device *dev, unsigned int cmd,
@@ -1399,6 +1403,22 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.policy = ethnl_mse_get_policy,
 		.maxattr = ARRAY_SIZE(ethnl_mse_get_policy) - 1,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_LOOPBACK_GET,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+		.policy = ethnl_loopback_get_policy,
+		.maxattr = ARRAY_SIZE(ethnl_loopback_get_policy) - 1,
+	},
+	{
+		.cmd	= ETHTOOL_MSG_LOOPBACK_SET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_default_set_doit,
+		.policy = ethnl_loopback_set_policy,
+		.maxattr = ARRAY_SIZE(ethnl_loopback_set_policy) - 1,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index aa8d51903ecc..2320760f931e 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -470,6 +470,7 @@ extern const struct ethnl_request_ops ethnl_mm_request_ops;
 extern const struct ethnl_request_ops ethnl_phy_request_ops;
 extern const struct ethnl_request_ops ethnl_tsconfig_request_ops;
 extern const struct ethnl_request_ops ethnl_mse_request_ops;
+extern const struct ethnl_request_ops ethnl_loopback_request_ops;
 
 extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
 extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
@@ -526,6 +527,8 @@ extern const struct nla_policy ethnl_phy_get_policy[ETHTOOL_A_PHY_HEADER + 1];
 extern const struct nla_policy ethnl_tsconfig_get_policy[ETHTOOL_A_TSCONFIG_HEADER + 1];
 extern const struct nla_policy ethnl_tsconfig_set_policy[ETHTOOL_A_TSCONFIG_MAX + 1];
 extern const struct nla_policy ethnl_mse_get_policy[ETHTOOL_A_MSE_HEADER + 1];
+extern const struct nla_policy ethnl_loopback_get_policy[ETHTOOL_A_LOOPBACK_MAX + 1];
+extern const struct nla_policy ethnl_loopback_set_policy[ETHTOOL_A_LOOPBACK_MAX + 1];
 
 int ethnl_set_features(struct sk_buff *skb, struct genl_info *info);
 int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
@@ -541,6 +544,9 @@ int ethnl_tsinfo_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 int ethnl_tsinfo_done(struct netlink_callback *cb);
 int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info);
 int ethnl_rss_delete_doit(struct sk_buff *skb, struct genl_info *info);
+int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev,
+			   const struct ethnl_dump_ctx *ctx,
+			   const struct genl_info *info);
 int ethnl_perphy_dump_one_dev(struct sk_buff *skb,
 			      struct ethnl_dump_ctx *ctx,
 			      unsigned long *pos_sub,
-- 
2.53.0

[PATCH net-next 04/11] ethtool: Add CMIS loopback helpers for module loopback control

[Björn Töpel]

Add CMIS loopback functions and wire them into loopback.c for the
MODULE component:

 - ethtool_cmis_get_loopback(): reads Page 13h capabilities and
   current state, appends one entry per supported loopback point
   ("cmis-host" and/or "cmis-media").

 - ethtool_cmis_get_loopback_by_index(): used to enumerate what the
   module supports.

 - ethtool_cmis_set_loopback(): resolves name to a pair of control
   byte indices, validates direction, and writes the Page 13h control
   bytes (0xff = all lanes on, 0x00 = off).

Directions are mutually exclusive: switching from near-end to far-end
first disables the active direction in a separate EEPROM write, then
enables the new one. Requesting multiple direction flags is rejected.

CMIS register mapping (Page 13h, Bytes 180-183):

 - MODULE, "cmis-host",  near-end  ->  Host Side Input    (Byte 183)
 - MODULE, "cmis-host",  far-end   ->  Host Side Output   (Byte 182)
 - MODULE, "cmis-media", near-end  ->  Media Side Input   (Byte 181)
 - MODULE, "cmis-media", far-end   ->  Media Side Output  (Byte 180)

The helpers work entirely over get/set_module_eeprom_by_page, so any
driver with EEPROM page access gets module loopback without new
ethtool_ops or driver changes. SET is rejected when firmware flashing
is in progress or the interface is UP.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 net/ethtool/Makefile        |   2 +-
 net/ethtool/cmis_loopback.c | 407 ++++++++++++++++++++++++++++++++++++
 net/ethtool/loopback.c      |   6 +-
 net/ethtool/netlink.h       |   8 +
 4 files changed, 421 insertions(+), 2 deletions(-)
 create mode 100644 net/ethtool/cmis_loopback.c

diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index ef534b55d724..2f821c7875e1 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -9,4 +9,4 @@ ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o rss.o \
 		   channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
 		   tunnels.o fec.o eeprom.o stats.o phc_vclocks.o mm.o \
 		   module.o cmis_fw_update.o cmis_cdb.o pse-pd.o plca.o \
-		   phy.o tsconfig.o mse.o loopback.o
+		   phy.o tsconfig.o mse.o loopback.o cmis_loopback.o
diff --git a/net/ethtool/cmis_loopback.c b/net/ethtool/cmis_loopback.c
new file mode 100644
index 000000000000..0d419c369d64
--- /dev/null
+++ b/net/ethtool/cmis_loopback.c
@@ -0,0 +1,407 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* CMIS loopback helpers for drivers implementing ethtool
+ * get/set_loopback.
+ *
+ * Maps the generic ethtool loopback model to CMIS Page 13h registers
+ * (CMIS 5.3, Table 8-128).
+ *
+ * Capabilities are read from Page 13h Byte 128, with Page 13h
+ * availability checked via Page 01h Byte 142 bit 5.
+ */
+
+#include <linux/ethtool.h>
+#include <linux/sfp.h>
+
+#include "common.h"
+#include "module_fw.h"
+#include "cmis.h"
+
+/* CMIS Page 00h, Byte 0: Physical module identifier */
+#define CMIS_PHYS_ID_PAGE		0x00
+#define CMIS_PHYS_ID_OFFSET		0x00
+
+/* CMIS Page 01h, Byte 142: Diagnostic Pages Support */
+#define CMIS_DIAG_SUPPORT_PAGE		0x01
+#define CMIS_DIAG_SUPPORT_OFFSET	0x8e
+#define CMIS_DIAG_PAGE13_BIT		BIT(5)
+
+/* CMIS Page 13h, Byte 128: Loopback Capability Advertisement */
+#define CMIS_LB_CAPS_PAGE		0x13
+#define CMIS_LB_CAPS_OFFSET		0x80
+#define CMIS_LB_CAP_MEDIA_OUTPUT	BIT(0)
+#define CMIS_LB_CAP_MEDIA_INPUT		BIT(1)
+#define CMIS_LB_CAP_HOST_OUTPUT		BIT(2)
+#define CMIS_LB_CAP_HOST_INPUT		BIT(3)
+
+/* CMIS Page 13h, Bytes 180-183: Per-Lane Loopback Control
+ *   Byte 180 (0xb4): Media Side Output  -> MODULE, "cmis-media", far-end
+ *   Byte 181 (0xb5): Media Side Input   -> MODULE, "cmis-media", near-end
+ *   Byte 182 (0xb6): Host Side Output   -> MODULE, "cmis-host",  far-end
+ *   Byte 183 (0xb7): Host Side Input    -> MODULE, "cmis-host",  near-end
+ */
+#define CMIS_LB_CTRL_PAGE		0x13
+#define CMIS_LB_CTRL_OFFSET		0xb4
+#define CMIS_LB_CTRL_LEN		4
+#define CMIS_LB_CTRL_IDX_MEDIA_OUTPUT	0
+#define CMIS_LB_CTRL_IDX_MEDIA_INPUT	1
+#define CMIS_LB_CTRL_IDX_HOST_OUTPUT	2
+#define CMIS_LB_CTRL_IDX_HOST_INPUT	3
+
+#define CMIS_LB_NAME_HOST		"cmis-host"
+#define CMIS_LB_NAME_MEDIA		"cmis-media"
+
+static bool cmis_is_module(u8 phys_id)
+{
+	switch (phys_id) {
+	case SFF8024_ID_QSFP_DD:
+	case SFF8024_ID_OSFP:
+	case SFF8024_ID_DSFP:
+	case SFF8024_ID_QSFP_PLUS_CMIS:
+	case SFF8024_ID_SFP_DD_CMIS:
+	case SFF8024_ID_SFP_PLUS_CMIS:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/**
+ * cmis_loopback_caps - Read CMIS loopback capability mask
+ * @dev: Network device
+ *
+ * Return: >0 capability bitmask, 0 if not a CMIS module or no Page
+ *         13h, negative errno on failure.
+ */
+static int cmis_loopback_caps(struct net_device *dev)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct ethtool_module_eeprom page = {};
+	int ret;
+	u8 val;
+
+	if (!ops->get_module_eeprom_by_page)
+		return 0;
+
+	/* Read physical identifier */
+	ethtool_cmis_page_init(&page, CMIS_PHYS_ID_PAGE,
+			       CMIS_PHYS_ID_OFFSET, sizeof(val));
+	page.data = &val;
+	ret = ops->get_module_eeprom_by_page(dev, &page, NULL);
+	if (ret < 0)
+		return ret;
+	if (!cmis_is_module(val))
+		return 0;
+
+	/* Check Page 13h availability */
+	ethtool_cmis_page_init(&page, CMIS_DIAG_SUPPORT_PAGE,
+			       CMIS_DIAG_SUPPORT_OFFSET, sizeof(val));
+	page.data = &val;
+	ret = ops->get_module_eeprom_by_page(dev, &page, NULL);
+	if (ret < 0)
+		return ret;
+	if (!(val & CMIS_DIAG_PAGE13_BIT))
+		return 0;
+
+	/* Read capability byte */
+	ethtool_cmis_page_init(&page, CMIS_LB_CAPS_PAGE,
+			       CMIS_LB_CAPS_OFFSET, sizeof(val));
+	page.data = &val;
+	ret = ops->get_module_eeprom_by_page(dev, &page, NULL);
+	if (ret < 0)
+		return ret;
+
+	return val & (CMIS_LB_CAP_MEDIA_OUTPUT | CMIS_LB_CAP_MEDIA_INPUT |
+		      CMIS_LB_CAP_HOST_OUTPUT | CMIS_LB_CAP_HOST_INPUT);
+}
+
+/**
+ * cmis_loopback_read - Read CMIS loopback capabilities and build entries
+ * @dev: Network device with get_module_eeprom_by_page support
+ * @host: Output host loopback entry (populated if host caps exist)
+ * @media: Output media loopback entry (populated if media caps exist)
+ * @has_host: Set to true if host loopback is supported
+ * @has_media: Set to true if media loopback is supported
+ *
+ * Common helper that reads CMIS caps and control bytes, then populates
+ * the host and media entries with current state.
+ *
+ * Return: 0 on success, -EOPNOTSUPP if no CMIS loopback support,
+ *         negative errno on failure.
+ */
+static int cmis_loopback_read(struct net_device *dev,
+			      struct ethtool_loopback_entry *host,
+			      struct ethtool_loopback_entry *media,
+			      bool *has_host, bool *has_media)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct ethtool_module_eeprom page = {};
+	u8 ctrl[CMIS_LB_CTRL_LEN];
+	int caps, ret;
+
+	*has_host = false;
+	*has_media = false;
+
+	if (dev->ethtool->module_fw_flash_in_progress)
+		return -EBUSY;
+
+	caps = cmis_loopback_caps(dev);
+	if (caps <= 0)
+		return caps ? caps : -EOPNOTSUPP;
+
+	ethtool_cmis_page_init(&page, CMIS_LB_CTRL_PAGE,
+			       CMIS_LB_CTRL_OFFSET, sizeof(ctrl));
+	page.data = ctrl;
+	ret = ops->get_module_eeprom_by_page(dev, &page, NULL);
+	if (ret < 0)
+		return ret;
+
+	memset(host, 0, sizeof(*host));
+	host->component = ETHTOOL_LOOPBACK_COMPONENT_MODULE;
+	strscpy(host->name, CMIS_LB_NAME_HOST, sizeof(host->name));
+
+	memset(media, 0, sizeof(*media));
+	media->component = ETHTOOL_LOOPBACK_COMPONENT_MODULE;
+	strscpy(media->name, CMIS_LB_NAME_MEDIA, sizeof(media->name));
+
+	if (caps & CMIS_LB_CAP_HOST_INPUT) {
+		*has_host = true;
+		host->supported |= ETHTOOL_LOOPBACK_DIRECTION_NEAR_END;
+		if (ctrl[CMIS_LB_CTRL_IDX_HOST_INPUT])
+			host->direction |= ETHTOOL_LOOPBACK_DIRECTION_NEAR_END;
+	}
+	if (caps & CMIS_LB_CAP_HOST_OUTPUT) {
+		*has_host = true;
+		host->supported |= ETHTOOL_LOOPBACK_DIRECTION_FAR_END;
+		if (ctrl[CMIS_LB_CTRL_IDX_HOST_OUTPUT])
+			host->direction |= ETHTOOL_LOOPBACK_DIRECTION_FAR_END;
+	}
+	if (caps & CMIS_LB_CAP_MEDIA_INPUT) {
+		*has_media = true;
+		media->supported |= ETHTOOL_LOOPBACK_DIRECTION_NEAR_END;
+		if (ctrl[CMIS_LB_CTRL_IDX_MEDIA_INPUT])
+			media->direction |= ETHTOOL_LOOPBACK_DIRECTION_NEAR_END;
+	}
+	if (caps & CMIS_LB_CAP_MEDIA_OUTPUT) {
+		*has_media = true;
+		media->supported |= ETHTOOL_LOOPBACK_DIRECTION_FAR_END;
+		if (ctrl[CMIS_LB_CTRL_IDX_MEDIA_OUTPUT])
+			media->direction |= ETHTOOL_LOOPBACK_DIRECTION_FAR_END;
+	}
+
+	return 0;
+}
+
+/**
+ * ethtool_cmis_get_loopback_by_index - Enumerate CMIS loopback entry by index
+ * @dev: Network device with get_module_eeprom_by_page support
+ * @index: Zero-based index of the loopback entry to retrieve
+ * @entry: Output loopback entry
+ *
+ * Used by the dump infrastructure to iterate one entry at a time.
+ *
+ * Return: 0 on success, -EOPNOTSUPP if the index is out of range or
+ *         no CMIS loopback support, negative errno on failure.
+ */
+int ethtool_cmis_get_loopback_by_index(struct net_device *dev, u32 index,
+				       struct ethtool_loopback_entry *entry)
+{
+	struct ethtool_loopback_entry host, media;
+	bool has_host, has_media;
+	u32 cur = 0;
+	int ret;
+
+	ret = cmis_loopback_read(dev, &host, &media, &has_host, &has_media);
+	if (ret)
+		return ret;
+
+	if (has_host) {
+		if (cur == index) {
+			memcpy(entry, &host, sizeof(*entry));
+			return 0;
+		}
+		cur++;
+	}
+
+	if (has_media) {
+		if (cur == index) {
+			memcpy(entry, &media, sizeof(*entry));
+			return 0;
+		}
+	}
+
+	return -EOPNOTSUPP;
+}
+
+/**
+ * ethtool_cmis_get_loopback - Look up CMIS loopback entry by name
+ * @dev: Network device with get_module_eeprom_by_page support
+ * @name: Loopback point name ("cmis-host" or "cmis-media")
+ * @entry: Output loopback entry
+ *
+ * Used by doit requests to look up a specific loopback point.
+ *
+ * Return: 0 on success, -EOPNOTSUPP if name doesn't match or no CMIS
+ *         support, negative errno on failure.
+ */
+int ethtool_cmis_get_loopback(struct net_device *dev,
+			      const char *name,
+			      struct ethtool_loopback_entry *entry)
+{
+	struct ethtool_loopback_entry host, media;
+	bool has_host, has_media;
+	int ret;
+
+	ret = cmis_loopback_read(dev, &host, &media, &has_host, &has_media);
+	if (ret)
+		return ret;
+
+	if (has_host && !strcmp(name, CMIS_LB_NAME_HOST)) {
+		memcpy(entry, &host, sizeof(*entry));
+		return 0;
+	}
+
+	if (has_media && !strcmp(name, CMIS_LB_NAME_MEDIA)) {
+		memcpy(entry, &media, sizeof(*entry));
+		return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+/**
+ * ethtool_cmis_set_loopback - Apply one MODULE loopback entry to CMIS
+ * @dev: Network device with get/set_module_eeprom_by_page support
+ * @entry: Loopback entry to apply (must be MODULE component)
+ * @extack: Netlink extended ack for error reporting
+ *
+ * Matches the entry against CMIS loopback points by name and
+ * direction, then reads, modifies, and writes the corresponding Page
+ * 13h control byte (0xff for all-lanes enable, 0x00 for disable).
+ *
+ * When disabling (direction == 0), all loopback points matching the
+ * name are disabled regardless of their direction. When enabling,
+ * only the specific direction is activated.
+ *
+ * Return: 1 if hardware state changed, 0 if already in requested state,
+ *         negative errno on failure.
+ */
+int ethtool_cmis_set_loopback(struct net_device *dev,
+			      const struct ethtool_loopback_entry *entry,
+			      struct netlink_ext_ack *extack)
+{
+	struct ethtool_module_eeprom page = {};
+	u8 ctrl[CMIS_LB_CTRL_LEN];
+	int near_idx, far_idx;
+	u8 near_cap, far_cap;
+	bool mod = false;
+	int caps, ret;
+
+	if (!dev->ethtool_ops->set_module_eeprom_by_page) {
+		NL_SET_ERR_MSG(extack,
+			       "Module EEPROM write access not supported");
+		return -EOPNOTSUPP;
+	}
+
+	if (dev->ethtool->module_fw_flash_in_progress) {
+		NL_SET_ERR_MSG(extack,
+			       "Module firmware flashing is in progress");
+		return -EBUSY;
+	}
+
+	if (dev->flags & IFF_UP) {
+		NL_SET_ERR_MSG(extack,
+			       "Netdevice is up, module loopback change not permitted");
+		return -EBUSY;
+	}
+
+	if (entry->direction && !is_power_of_2(entry->direction)) {
+		NL_SET_ERR_MSG(extack,
+			       "Only one loopback direction may be enabled at a time");
+		return -EINVAL;
+	}
+
+	if (!strcmp(entry->name, CMIS_LB_NAME_HOST)) {
+		near_idx = CMIS_LB_CTRL_IDX_HOST_INPUT;
+		far_idx = CMIS_LB_CTRL_IDX_HOST_OUTPUT;
+		near_cap = CMIS_LB_CAP_HOST_INPUT;
+		far_cap = CMIS_LB_CAP_HOST_OUTPUT;
+	} else if (!strcmp(entry->name, CMIS_LB_NAME_MEDIA)) {
+		near_idx = CMIS_LB_CTRL_IDX_MEDIA_INPUT;
+		far_idx = CMIS_LB_CTRL_IDX_MEDIA_OUTPUT;
+		near_cap = CMIS_LB_CAP_MEDIA_INPUT;
+		far_cap = CMIS_LB_CAP_MEDIA_OUTPUT;
+	} else {
+		NL_SET_ERR_MSG(extack, "Unknown CMIS loopback name");
+		return -EINVAL;
+	}
+
+	caps = cmis_loopback_caps(dev);
+	if (caps < 0)
+		return caps;
+	if (!caps) {
+		NL_SET_ERR_MSG(extack, "Module does not support CMIS loopback");
+		return -EOPNOTSUPP;
+	}
+
+	/* Read current control bytes */
+	ethtool_cmis_page_init(&page, CMIS_LB_CTRL_PAGE,
+			       CMIS_LB_CTRL_OFFSET, sizeof(ctrl));
+	page.data = ctrl;
+	ret = dev->ethtool_ops->get_module_eeprom_by_page(dev, &page, NULL);
+	if (ret < 0)
+		return ret;
+
+	if (!entry->direction) {
+		/* Disable both directions */
+		if (ctrl[near_idx]) {
+			ctrl[near_idx] = 0x00;
+			mod = true;
+		}
+		if (ctrl[far_idx]) {
+			ctrl[far_idx] = 0x00;
+			mod = true;
+		}
+	} else {
+		int enable_idx, disable_idx;
+		u8 enable_cap;
+
+		if (entry->direction & ETHTOOL_LOOPBACK_DIRECTION_NEAR_END) {
+			enable_idx = near_idx;
+			enable_cap = near_cap;
+			disable_idx = far_idx;
+		} else {
+			enable_idx = far_idx;
+			enable_cap = far_cap;
+			disable_idx = near_idx;
+		}
+
+		if (!(caps & enable_cap)) {
+			NL_SET_ERR_MSG(extack,
+				       "Loopback mode not supported by module");
+			return -EOPNOTSUPP;
+		}
+
+		/* Disable opposite direction first (mutual exclusivity) */
+		if (ctrl[disable_idx]) {
+			ctrl[disable_idx] = 0x00;
+			ret = dev->ethtool_ops->set_module_eeprom_by_page(dev,
+				&page, extack);
+			if (ret < 0)
+				return ret;
+			mod = true;
+		}
+
+		if (ctrl[enable_idx] != 0xff) {
+			ctrl[enable_idx] = 0xff;
+			mod = true;
+		}
+	}
+
+	if (!mod)
+		return 0;
+
+	ret = dev->ethtool_ops->set_module_eeprom_by_page(dev, &page, extack);
+
+	return ret < 0 ? ret : 1;
+}
diff --git a/net/ethtool/loopback.c b/net/ethtool/loopback.c
index 8907dd147404..ed184f45322e 100644
--- a/net/ethtool/loopback.c
+++ b/net/ethtool/loopback.c
@@ -86,6 +86,8 @@ static int loopback_get(struct net_device *dev,
 			struct ethtool_loopback_entry *entry)
 {
 	switch (component) {
+	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
+		return ethtool_cmis_get_loopback(dev, name, entry);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -94,7 +96,7 @@ static int loopback_get(struct net_device *dev,
 static int loopback_get_by_index(struct net_device *dev, u32 index,
 				 struct ethtool_loopback_entry *entry)
 {
-	return -EOPNOTSUPP;
+	return ethtool_cmis_get_loopback_by_index(dev, index, entry);
 }
 
 static int loopback_prepare_data(const struct ethnl_req_info *req_base,
@@ -223,6 +225,8 @@ static int __loopback_set(struct net_device *dev,
 			  struct netlink_ext_ack *extack)
 {
 	switch (entry->component) {
+	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
+		return ethtool_cmis_set_loopback(dev, entry, extack);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 2320760f931e..5aec1b73da7a 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -551,6 +551,14 @@ int ethnl_perphy_dump_one_dev(struct sk_buff *skb,
 			      struct ethnl_dump_ctx *ctx,
 			      unsigned long *pos_sub,
 			      const struct genl_info *info);
+int ethtool_cmis_get_loopback_by_index(struct net_device *dev, u32 index,
+				       struct ethtool_loopback_entry *entry);
+int ethtool_cmis_get_loopback(struct net_device *dev,
+			      const char *name,
+			      struct ethtool_loopback_entry *entry);
+int ethtool_cmis_set_loopback(struct net_device *dev,
+			      const struct ethtool_loopback_entry *entry,
+			      struct netlink_ext_ack *extack);
 
 extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN];
 extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN];
-- 
2.53.0

[PATCH net-next 05/11] selftests: drv-net: Add loopback driver test

[Björn Töpel]

Add a selftest for the ethtool loopback UAPI exercising module
loopback via the loopback GET/SET netlink commands.

Works on any device that reports module loopback entries. Tests cover
enable near-end and far-end, disable, direction switching (mutual
exclusivity), idempotent enable, and rejection while interface is up.
Devices without module loopback support are skipped.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 .../selftests/drivers/net/hw/loopback_drv.py  | 226 ++++++++++++++++++
 2 files changed, 227 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/hw/loopback_drv.py

diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index 91df028abfc0..1c341aaa88c6 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -31,6 +31,7 @@ TEST_PROGS = \
 	iou-zcrx.py \
 	irq.py \
 	loopback.sh \
+	loopback_drv.py \
 	nic_timestamp.py \
 	nk_netns.py \
 	pp_alloc_fail.py \
diff --git a/tools/testing/selftests/drivers/net/hw/loopback_drv.py b/tools/testing/selftests/drivers/net/hw/loopback_drv.py
new file mode 100755
index 000000000000..05374db42ae9
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/loopback_drv.py
@@ -0,0 +1,226 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Tests for ethtool loopback GET/SET with CMIS modules.
+
+Works on any device that reports module loopback entries. On devices
+without CMIS loopback support, tests are skipped.
+"""
+
+import errno
+
+from lib.py import ksft_run, ksft_exit, ksft_eq
+from lib.py import KsftSkipEx, KsftFailEx, ksft_disruptive
+from lib.py import EthtoolFamily, NlError
+from lib.py import NetDrvEnv, ip, defer
+
+# Direction flags as YNL returns them (sets of flag name strings)
+DIR_NONE = set()
+DIR_NEAR_END = {'near-end'}
+DIR_FAR_END = {'far-end'}
+
+
+def _get_loopback(cfg):
+    """GET loopback and return the list of entries (via DUMP)."""
+    results = cfg.ethnl.loopback_get({
+        'header': {'dev-index': cfg.ifindex}
+    }, dump=True)
+    entries = []
+    for msg in results:
+        if 'entry' in msg:
+            entries.extend(msg['entry'])
+    return entries
+
+
+def _set_loopback(cfg, component, name, direction):
+    """SET loopback for a single entry."""
+    cfg.ethnl.loopback_set({
+        'header': {'dev-index': cfg.ifindex},
+        'entry': [{
+            'component': component,
+            'name': name,
+            'direction': direction,
+        }]
+    })
+
+
+def _require_module_entries(cfg):
+    """Return module loopback entries, skip if none available."""
+    entries = _get_loopback(cfg)
+    mod_entries = [e for e in entries if e['component'] == 'module']
+    if not mod_entries:
+        raise KsftSkipEx("No module loopback entries")
+    return mod_entries
+
+
+@ksft_disruptive
+def test_set_near_end(cfg):
+    """SET a module entry to near-end and verify via GET."""
+    mod_entries = _require_module_entries(cfg)
+
+    near = [e for e in mod_entries
+            if 'near-end' in e['supported']]
+    if not near:
+        raise KsftSkipEx("No near-end capable module entry")
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    target = near[0]
+    _set_loopback(cfg, 'module', target['name'], 'near-end')
+    defer(_set_loopback, cfg, 'module', target['name'], 0)
+
+    entries = _get_loopback(cfg)
+    updated = [e for e in entries
+               if e['name'] == target['name']
+               and 'near-end' in e['supported']]
+    ksft_eq(len(updated), 1)
+    ksft_eq(updated[0]['direction'], DIR_NEAR_END)
+
+
+@ksft_disruptive
+def test_set_far_end(cfg):
+    """SET a module entry to far-end and verify via GET."""
+    mod_entries = _require_module_entries(cfg)
+
+    far = [e for e in mod_entries
+           if 'far-end' in e['supported']]
+    if not far:
+        raise KsftSkipEx("No far-end capable module entry")
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    target = far[0]
+    _set_loopback(cfg, 'module', target['name'], 'far-end')
+    defer(_set_loopback, cfg, 'module', target['name'], 0)
+
+    entries = _get_loopback(cfg)
+    updated = [e for e in entries
+               if e['name'] == target['name']
+               and 'far-end' in e['supported']]
+    ksft_eq(len(updated), 1)
+    ksft_eq(updated[0]['direction'], DIR_FAR_END)
+
+
+@ksft_disruptive
+def test_set_disable(cfg):
+    """Enable then disable loopback and verify."""
+    mod_entries = _require_module_entries(cfg)
+
+    near = [e for e in mod_entries
+            if 'near-end' in e['supported']]
+    if not near:
+        raise KsftSkipEx("No near-end capable module entry")
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    target = near[0]
+    _set_loopback(cfg, 'module', target['name'], 'near-end')
+    defer(_set_loopback, cfg, 'module', target['name'], 0)
+
+    # Disable
+    _set_loopback(cfg, 'module', target['name'], 0)
+
+    entries = _get_loopback(cfg)
+    updated = [e for e in entries if e['name'] == target['name']]
+    ksft_eq(updated[0]['direction'], DIR_NONE,
+            "Direction should be off after disable")
+
+
+@ksft_disruptive
+def test_set_direction_switch(cfg):
+    """Enable near-end, then switch to far-end. The kernel must disable
+    near-end before enabling far-end (mutual exclusivity).
+    """
+    mod_entries = _require_module_entries(cfg)
+
+    both = [e for e in mod_entries
+            if 'near-end' in e['supported'] and 'far-end' in e['supported']]
+    if not both:
+        raise KsftSkipEx("No entry with both near-end and far-end support")
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    target = both[0]
+    _set_loopback(cfg, 'module', target['name'], 'near-end')
+    defer(_set_loopback, cfg, 'module', target['name'], 0)
+
+    entries = _get_loopback(cfg)
+    updated = [e for e in entries if e['name'] == target['name']]
+    ksft_eq(updated[0]['direction'], DIR_NEAR_END)
+
+    # Switch to far-end
+    _set_loopback(cfg, 'module', target['name'], 'far-end')
+
+    entries = _get_loopback(cfg)
+    updated = [e for e in entries if e['name'] == target['name']]
+    ksft_eq(updated[0]['direction'], DIR_FAR_END,
+            "Should have switched to far-end")
+
+
+@ksft_disruptive
+def test_set_idempotent(cfg):
+    """Enable the same direction twice. Second call should not fail."""
+    mod_entries = _require_module_entries(cfg)
+
+    near = [e for e in mod_entries
+            if 'near-end' in e['supported']]
+    if not near:
+        raise KsftSkipEx("No near-end capable module entry")
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    target = near[0]
+    _set_loopback(cfg, 'module', target['name'], 'near-end')
+    defer(_set_loopback, cfg, 'module', target['name'], 0)
+
+    # Second enable of the same direction should succeed
+    _set_loopback(cfg, 'module', target['name'], 'near-end')
+
+    entries = _get_loopback(cfg)
+    updated = [e for e in entries
+               if e['name'] == target['name']
+               and 'near-end' in e['supported']]
+    ksft_eq(updated[0]['direction'], DIR_NEAR_END,
+            "Direction should still be near-end")
+
+
+@ksft_disruptive
+def test_set_while_up(cfg):
+    """SET while interface is UP should fail."""
+    mod_entries = _require_module_entries(cfg)
+
+    target = mod_entries[0]
+    direction = 'near-end'
+    if direction not in target['supported']:
+        direction = 'far-end'
+
+    try:
+        _set_loopback(cfg, 'module', target['name'], direction)
+        raise KsftFailEx("Should have rejected SET while interface is up")
+    except NlError as e:
+        ksft_eq(e.error, errno.EBUSY,
+                "Expected EBUSY when interface is up")
+
+
+def main() -> None:
+    with NetDrvEnv(__file__, nsim_test=False) as cfg:
+        cfg.ethnl = EthtoolFamily()
+
+        ksft_run([
+            test_set_near_end,
+            test_set_far_end,
+            test_set_disable,
+            test_set_direction_switch,
+            test_set_idempotent,
+            test_set_while_up,
+        ], args=(cfg, ))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.53.0

[PATCH net-next 06/11] ethtool: Add MAC loopback support via ethtool_ops

[Björn Töpel]

Extend struct ethtool_ops with three loopback callbacks that drivers
can implement for MAC-level loopback control:

 - get_loopback(dev, name, id, entry): exact lookup by name and
   instance id, used by doit (single-entry GET) requests.

 - get_loopback_by_index(dev, index, entry): flat enumeration by
   index, used by dumpit (multi-entry GET) requests to iterate all
   loopback points on a device.

 - set_loopback(dev, entry, extack): apply a loopback configuration
   change. Returns 1 if hardware state changed, 0 if no-op.

Wire the MAC component into loopback.c's dispatch functions. For dump
enumeration, MAC entries are tried first via the driver's
get_loopback_by_index() op, then MODULE/CMIS entries follow at the
next index offset.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 include/linux/ethtool.h |  7 ++++++
 net/ethtool/loopback.c  | 56 ++++++++++++++++++++++++++++++++---------
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index c9beca11fc40..8893e732f930 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -1321,6 +1321,13 @@ struct ethtool_ops {
 	int	(*set_mm)(struct net_device *dev, struct ethtool_mm_cfg *cfg,
 			  struct netlink_ext_ack *extack);
 	void	(*get_mm_stats)(struct net_device *dev, struct ethtool_mm_stats *stats);
+	int	(*get_loopback)(struct net_device *dev, const char *name,
+				u32 id, struct ethtool_loopback_entry *entry);
+	int	(*get_loopback_by_index)(struct net_device *dev, u32 index,
+					 struct ethtool_loopback_entry *entry);
+	int	(*set_loopback)(struct net_device *dev,
+				const struct ethtool_loopback_entry *entry,
+				struct netlink_ext_ack *extack);
 };
 
 int ethtool_check_ops(const struct ethtool_ops *ops);
diff --git a/net/ethtool/loopback.c b/net/ethtool/loopback.c
index ed184f45322e..9c5834be743f 100644
--- a/net/ethtool/loopback.c
+++ b/net/ethtool/loopback.c
@@ -86,6 +86,10 @@ static int loopback_get(struct net_device *dev,
 			struct ethtool_loopback_entry *entry)
 {
 	switch (component) {
+	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
+		if (!dev->ethtool_ops->get_loopback)
+			return -EOPNOTSUPP;
+		return dev->ethtool_ops->get_loopback(dev, name, id, entry);
 	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
 		return ethtool_cmis_get_loopback(dev, name, entry);
 	default:
@@ -93,10 +97,22 @@ static int loopback_get(struct net_device *dev,
 	}
 }
 
-static int loopback_get_by_index(struct net_device *dev, u32 index,
+static int loopback_get_by_index(struct net_device *dev,
+				 enum ethtool_loopback_component component,
+				 u32 index,
 				 struct ethtool_loopback_entry *entry)
 {
-	return ethtool_cmis_get_loopback_by_index(dev, index, entry);
+	switch (component) {
+	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
+		if (!dev->ethtool_ops->get_loopback_by_index)
+			return -EOPNOTSUPP;
+		return dev->ethtool_ops->get_loopback_by_index(dev, index,
+							       entry);
+	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
+		return ethtool_cmis_get_loopback_by_index(dev, index, entry);
+	default:
+		return -EOPNOTSUPP;
+	}
 }
 
 static int loopback_prepare_data(const struct ethnl_req_info *req_base,
@@ -116,7 +132,8 @@ static int loopback_prepare_data(const struct ethnl_req_info *req_base,
 		ret = loopback_get(dev, req_info->component, req_info->id,
 				   req_info->name, &data->entry);
 	else
-		ret = loopback_get_by_index(dev, req_info->index, &data->entry);
+		ret = loopback_get_by_index(dev, req_info->component,
+					    req_info->index, &data->entry);
 
 	ethnl_ops_complete(dev);
 
@@ -225,6 +242,10 @@ static int __loopback_set(struct net_device *dev,
 			  struct netlink_ext_ack *extack)
 {
 	switch (entry->component) {
+	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
+		if (!dev->ethtool_ops->set_loopback)
+			return -EOPNOTSUPP;
+		return dev->ethtool_ops->set_loopback(dev, entry, extack);
 	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
 		return ethtool_cmis_set_loopback(dev, entry, extack);
 	default:
@@ -274,20 +295,31 @@ static int loopback_dump_one_dev(struct sk_buff *skb,
 {
 	struct loopback_req_info *req_info =
 		container_of(ctx->req_info, struct loopback_req_info, base);
+	/* pos_sub encodes: upper 16 bits = component phase, lower 16 = index
+	 * within that component. dump_one_dev is called repeatedly with
+	 * increasing pos_sub until all components are exhausted.
+	 */
+	enum ethtool_loopback_component phase = *pos_sub >> 16;
+	u32 idx = *pos_sub & 0xffff;
 	int ret;
 
-	for (;; (*pos_sub)++) {
-		req_info->index = *pos_sub;
-		ret = ethnl_default_dump_one(skb, ctx->req_info->dev, ctx,
-					     info);
-		if (ret == -EOPNOTSUPP)
-			break;
-		if (ret)
-			return ret;
+	for (; phase <= ETHTOOL_LOOPBACK_COMPONENT_MODULE; phase++) {
+		for (;; idx++) {
+			req_info->component = phase;
+			req_info->index = idx;
+			ret = ethnl_default_dump_one(skb, ctx->req_info->dev,
+						     ctx, info);
+			if (ret == -EOPNOTSUPP)
+				break;
+			if (ret) {
+				*pos_sub = ((unsigned long)phase << 16) | idx;
+				return ret;
+			}
+		}
+		idx = 0;
 	}
 
 	*pos_sub = 0;
-
 	return 0;
 }
 
-- 
2.53.0

[PATCH net-next 07/11] netdevsim: Add MAC loopback simulation

[Björn Töpel]

Implement the three ethtool loopback ops for MAC-level loopback
simulation:

 - get_loopback(): exact lookup by name ("mac")
 - get_loopback_by()_index: enumerate (single entry at index 0)
 - set_loopback(): update direction, return 1 if changed

The MAC loopback entry announces support for both near-end and far-end
support by default. State is stored in nsim_ethtool.mac_lb and exposed
via debugfs under ethtool/mac_lb/{supported,direction} for test
inspection and control.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 drivers/net/netdevsim/ethtool.c   | 64 +++++++++++++++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h |  4 ++
 2 files changed, 68 insertions(+)

diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 36a201533aae..84bc025885f7 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -195,6 +195,58 @@ nsim_get_fec_stats(struct net_device *dev, struct ethtool_fec_stats *fec_stats,
 	values[2].per_lane[3] = 0;
 }
 
+static void nsim_fill_mac_lb_entry(struct netdevsim *ns,
+				   struct ethtool_loopback_entry *entry)
+{
+	memset(entry, 0, sizeof(*entry));
+	entry->component = ETHTOOL_LOOPBACK_COMPONENT_MAC;
+	strscpy(entry->name, "mac", sizeof(entry->name));
+	entry->supported = ns->ethtool.mac_lb.supported;
+	entry->direction = ns->ethtool.mac_lb.direction;
+}
+
+static int nsim_get_loopback(struct net_device *dev, const char *name,
+			     u32 id, struct ethtool_loopback_entry *entry)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	if (strcmp(name, "mac"))
+		return -EOPNOTSUPP;
+
+	nsim_fill_mac_lb_entry(ns, entry);
+	return 0;
+}
+
+static int nsim_get_loopback_by_index(struct net_device *dev, u32 index,
+				      struct ethtool_loopback_entry *entry)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	if (index > 0)
+		return -EOPNOTSUPP;
+
+	nsim_fill_mac_lb_entry(ns, entry);
+	return 0;
+}
+
+static int nsim_set_loopback(struct net_device *dev,
+			     const struct ethtool_loopback_entry *entry,
+			     struct netlink_ext_ack *extack)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	if (strcmp(entry->name, "mac")) {
+		NL_SET_ERR_MSG(extack, "Unknown MAC loopback name");
+		return -EOPNOTSUPP;
+	}
+
+	if (ns->ethtool.mac_lb.direction == entry->direction)
+		return 0;
+
+	ns->ethtool.mac_lb.direction = entry->direction;
+	return 1;
+}
+
 static int nsim_get_ts_info(struct net_device *dev,
 			    struct kernel_ethtool_ts_info *info)
 {
@@ -222,6 +274,9 @@ static const struct ethtool_ops nsim_ethtool_ops = {
 	.set_fecparam			= nsim_set_fecparam,
 	.get_fec_stats			= nsim_get_fec_stats,
 	.get_ts_info			= nsim_get_ts_info,
+	.get_loopback			= nsim_get_loopback,
+	.get_loopback_by_index		= nsim_get_loopback_by_index,
+	.set_loopback			= nsim_set_loopback,
 };
 
 static void nsim_ethtool_ring_init(struct netdevsim *ns)
@@ -270,4 +325,13 @@ void nsim_ethtool_init(struct netdevsim *ns)
 			   &ns->ethtool.ring.rx_mini_max_pending);
 	debugfs_create_u32("tx_max_pending", 0600, dir,
 			   &ns->ethtool.ring.tx_max_pending);
+
+	ns->ethtool.mac_lb.supported = ETHTOOL_LOOPBACK_DIRECTION_NEAR_END |
+				       ETHTOOL_LOOPBACK_DIRECTION_FAR_END;
+
+	dir = debugfs_create_dir("mac_lb", ethtool);
+	debugfs_create_u32("supported", 0600, dir,
+			   &ns->ethtool.mac_lb.supported);
+	debugfs_create_u32("direction", 0600, dir,
+			   &ns->ethtool.mac_lb.direction);
 }
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index f767fc8a7505..2e322b9410d2 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -90,6 +90,10 @@ struct nsim_ethtool {
 	struct ethtool_coalesce coalesce;
 	struct ethtool_ringparam ring;
 	struct ethtool_fecparam fec;
+	struct {
+		u32 supported;
+		u32 direction;
+	} mac_lb;
 };
 
 struct nsim_rq {
-- 
2.53.0

[PATCH net-next 08/11] selftests: drv-net: Add MAC loopback netdevsim test

[Björn Töpel]

Add loopback_nsim.py with netdevsim-specific tests for MAC
loopback via the new ethtool_ops loopback callbacks:

 - test_get_mac_entry: verify MAC entry appears in dump with correct
   component, name, and supported directions
 - test_set_mac_near_end: SET near-end, verify via GET and debugfs
 - test_set_mac_disable: enable then disable
 - test_set_mac_unknown_name: SET with wrong name, expect EOPNOTSUPP

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 .../selftests/drivers/net/hw/loopback_nsim.py | 135 ++++++++++++++++++
 2 files changed, 136 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/hw/loopback_nsim.py

diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index 1c341aaa88c6..d7041b9d7461 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -32,6 +32,7 @@ TEST_PROGS = \
 	irq.py \
 	loopback.sh \
 	loopback_drv.py \
+	loopback_nsim.py \
 	nic_timestamp.py \
 	nk_netns.py \
 	pp_alloc_fail.py \
diff --git a/tools/testing/selftests/drivers/net/hw/loopback_nsim.py b/tools/testing/selftests/drivers/net/hw/loopback_nsim.py
new file mode 100755
index 000000000000..5edc999d920b
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/loopback_nsim.py
@@ -0,0 +1,135 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Netdevsim-specific tests for MAC loopback via ethtool_ops.
+
+Verifies that MAC loopback entries appear in dumps, that SET
+operations update state correctly (both via GET and debugfs).
+"""
+
+import errno
+import os
+
+from lib.py import ksft_run, ksft_exit, ksft_eq
+from lib.py import KsftSkipEx, KsftFailEx, ksft_disruptive
+from lib.py import EthtoolFamily, NlError
+from lib.py import NetDrvEnv, ip, defer
+
+# Direction flags as YNL returns them
+DIR_NONE = set()
+DIR_NEAR_END = {'near-end'}
+DIR_FAR_END = {'far-end'}
+
+
+def _nsim_dfs_path(cfg):
+    return cfg._ns.nsims[0].dfs_dir
+
+
+def _dfs_read_u32(cfg, path):
+    with open(os.path.join(_nsim_dfs_path(cfg), path)) as f:
+        return int(f.read().strip())
+
+
+def _dfs_write_u32(cfg, path, val):
+    with open(os.path.join(_nsim_dfs_path(cfg), path), "w") as f:
+        f.write(str(val))
+
+
+def _get_loopback(cfg):
+    results = cfg.ethnl.loopback_get({
+        'header': {'dev-index': cfg.ifindex}
+    }, dump=True)
+    entries = []
+    for msg in results:
+        if 'entry' in msg:
+            entries.extend(msg['entry'])
+    return entries
+
+
+def _set_loopback(cfg, component, name, direction):
+    cfg.ethnl.loopback_set({
+        'header': {'dev-index': cfg.ifindex},
+        'entry': [{
+            'component': component,
+            'name': name,
+            'direction': direction,
+        }]
+    })
+
+
+def test_get_mac_entry(cfg):
+    """GET should return the MAC loopback entry with correct attributes."""
+    entries = _get_loopback(cfg)
+    mac_entries = [e for e in entries if e['component'] == 'mac']
+
+    ksft_eq(len(mac_entries), 1, "Expected 1 MAC loopback entry")
+    ksft_eq(mac_entries[0]['name'], 'mac')
+    ksft_eq(mac_entries[0]['supported'], DIR_NEAR_END | DIR_FAR_END)
+    ksft_eq(mac_entries[0]['direction'], DIR_NONE)
+
+
+@ksft_disruptive
+def test_set_mac_near_end(cfg):
+    """SET MAC near-end and verify via GET and debugfs."""
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    _set_loopback(cfg, 'mac', 'mac', 'near-end')
+    defer(_set_loopback, cfg, 'mac', 'mac', 0)
+
+    entries = _get_loopback(cfg)
+    mac = [e for e in entries if e['component'] == 'mac']
+    ksft_eq(mac[0]['direction'], DIR_NEAR_END)
+
+    dfs_dir = _dfs_read_u32(cfg, "ethtool/mac_lb/direction")
+    ksft_eq(dfs_dir, 1, "debugfs direction should be 1 (NEAR_END)")
+
+
+@ksft_disruptive
+def test_set_mac_disable(cfg):
+    """Enable then disable MAC loopback."""
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    _set_loopback(cfg, 'mac', 'mac', 'near-end')
+    defer(_set_loopback, cfg, 'mac', 'mac', 0)
+
+    _set_loopback(cfg, 'mac', 'mac', 0)
+
+    entries = _get_loopback(cfg)
+    mac = [e for e in entries if e['component'] == 'mac']
+    ksft_eq(mac[0]['direction'], DIR_NONE, "Direction should be off")
+
+    dfs_dir = _dfs_read_u32(cfg, "ethtool/mac_lb/direction")
+    ksft_eq(dfs_dir, 0, "debugfs direction should be 0")
+
+
+@ksft_disruptive
+def test_set_mac_unknown_name(cfg):
+    """SET with unknown name should fail with EOPNOTSUPP."""
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    try:
+        _set_loopback(cfg, 'mac', 'bogus', 'near-end')
+        raise KsftFailEx("Should have rejected unknown name")
+    except NlError as e:
+        ksft_eq(e.error, errno.EOPNOTSUPP,
+                "Expected EOPNOTSUPP for unknown name")
+
+
+def main() -> None:
+    with NetDrvEnv(__file__) as cfg:
+        cfg.ethnl = EthtoolFamily()
+
+        ksft_run([
+            test_get_mac_entry,
+            test_set_mac_near_end,
+            test_set_mac_disable,
+            test_set_mac_unknown_name,
+        ], args=(cfg, ))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.53.0

[PATCH net-next 09/11] MAINTAINERS: Add entry for ethtool loopback

[Björn Töpel]

Add a MAINTAINERS entry for the ethtool loopback subsystem covering
the core loopback and CMIS loopback netlink implementation, and the
associated selftests.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a38fe0ed7144..37b134feffe9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18392,6 +18392,12 @@ F:	net/ethtool/cabletest.c
 F:	tools/testing/selftests/drivers/net/*/ethtool*
 K:	cable_test
 
+NETWORKING [ETHTOOL LOOPBACK]
+M:	Björn Töpel <bjorn@kernel.org>
+F:	net/ethtool/cmis_loopback.c
+F:	net/ethtool/loopback.c
+F:	tools/testing/selftests/drivers/net/hw/loopback*
+
 NETWORKING [ETHTOOL MAC MERGE]
 M:	Vladimir Oltean <vladimir.oltean@nxp.com>
 F:	net/ethtool/mm.c
-- 
2.53.0

[PATCH net-next 10/11] netdevsim: Add module EEPROM simulation via debugfs

[Björn Töpel]

Add get/set_module_eeprom_by_page ethtool ops to netdevsim, enabling
testing of kernel features that depend on module EEPROM access (e.g.
CMIS loopback) without real hardware.

The EEPROM is backed by a 256-page x 128-byte array exposed as binary
debugfs files under ports/<N>/ethtool/module/pages/{0..255}. Offsets
0-127 map to page 0 (lower memory), 128-255 to the requested page's
upper memory, following the CMIS layout. Error injection via get_err
and set_err follows the existing netdevsim pattern.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 drivers/net/netdevsim/ethtool.c   | 83 +++++++++++++++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h | 11 ++++
 2 files changed, 94 insertions(+)

diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 84bc025885f7..7ef96a747643 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -247,6 +247,68 @@ static int nsim_set_loopback(struct net_device *dev,
 	return 1;
 }
 
+static u8 *nsim_module_eeprom_ptr(struct netdevsim *ns,
+				  const struct ethtool_module_eeprom *page_data,
+				  u32 *len)
+{
+	u32 offset;
+	u8 page;
+
+	if (page_data->offset < NSIM_MODULE_EEPROM_PAGE_LEN) {
+		page = 0;
+		offset = page_data->offset;
+	} else {
+		page = page_data->page;
+		offset = page_data->offset - NSIM_MODULE_EEPROM_PAGE_LEN;
+	}
+
+	*len = min_t(u32, page_data->length,
+		     NSIM_MODULE_EEPROM_PAGE_LEN - offset);
+	return ns->ethtool.module.pages[page] + offset;
+}
+
+static int
+nsim_get_module_eeprom_by_page(struct net_device *dev,
+			       const struct ethtool_module_eeprom *page_data,
+			       struct netlink_ext_ack *extack)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+	u32 len;
+	u8 *ptr;
+
+	if (ns->ethtool.module.get_err)
+		return -ns->ethtool.module.get_err;
+
+	ptr = nsim_module_eeprom_ptr(ns, page_data, &len);
+	if (!ptr)
+		return -EINVAL;
+
+	memcpy(page_data->data, ptr, len);
+
+	return len;
+}
+
+static int
+nsim_set_module_eeprom_by_page(struct net_device *dev,
+			       const struct ethtool_module_eeprom *page_data,
+			       struct netlink_ext_ack *extack)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+	u32 len;
+	u8 *ptr;
+
+	if (ns->ethtool.module.set_err)
+		return -ns->ethtool.module.set_err;
+
+	ptr = nsim_module_eeprom_ptr(ns, page_data, &len);
+	if (!ptr)
+		return -EINVAL;
+
+	memcpy(ptr, page_data->data, len);
+
+	return 0;
+}
+
 static int nsim_get_ts_info(struct net_device *dev,
 			    struct kernel_ethtool_ts_info *info)
 {
@@ -274,6 +336,8 @@ static const struct ethtool_ops nsim_ethtool_ops = {
 	.set_fecparam			= nsim_set_fecparam,
 	.get_fec_stats			= nsim_get_fec_stats,
 	.get_ts_info			= nsim_get_ts_info,
+	.get_module_eeprom_by_page	= nsim_get_module_eeprom_by_page,
+	.set_module_eeprom_by_page	= nsim_set_module_eeprom_by_page,
 	.get_loopback			= nsim_get_loopback,
 	.get_loopback_by_index		= nsim_get_loopback_by_index,
 	.set_loopback			= nsim_set_loopback,
@@ -292,6 +356,7 @@ static void nsim_ethtool_ring_init(struct netdevsim *ns)
 void nsim_ethtool_init(struct netdevsim *ns)
 {
 	struct dentry *ethtool, *dir;
+	int i;
 
 	ns->netdev->ethtool_ops = &nsim_ethtool_ops;
 
@@ -326,6 +391,24 @@ void nsim_ethtool_init(struct netdevsim *ns)
 	debugfs_create_u32("tx_max_pending", 0600, dir,
 			   &ns->ethtool.ring.tx_max_pending);
 
+	dir = debugfs_create_dir("module", ethtool);
+	debugfs_create_u32("get_err", 0600, dir, &ns->ethtool.module.get_err);
+	debugfs_create_u32("set_err", 0600, dir, &ns->ethtool.module.set_err);
+
+	dir = debugfs_create_dir("pages", dir);
+	for (i = 0; i < NSIM_MODULE_EEPROM_PAGES; i++) {
+		char name[8];
+
+		ns->ethtool.module.page_blobs[i].data =
+			ns->ethtool.module.pages[i];
+		ns->ethtool.module.page_blobs[i].size =
+			NSIM_MODULE_EEPROM_PAGE_LEN;
+
+		snprintf(name, sizeof(name), "%u", i);
+		debugfs_create_blob(name, 0600, dir,
+				    &ns->ethtool.module.page_blobs[i]);
+	}
+
 	ns->ethtool.mac_lb.supported = ETHTOOL_LOOPBACK_DIRECTION_NEAR_END |
 				       ETHTOOL_LOOPBACK_DIRECTION_FAR_END;
 
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 2e322b9410d2..f6b2063d41c9 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -82,6 +82,16 @@ struct nsim_ethtool_pauseparam {
 	bool report_stats_tx;
 };
 
+#define NSIM_MODULE_EEPROM_PAGES	256
+#define NSIM_MODULE_EEPROM_PAGE_LEN	128
+
+struct nsim_ethtool_module {
+	u32 get_err;
+	u32 set_err;
+	u8 pages[NSIM_MODULE_EEPROM_PAGES][NSIM_MODULE_EEPROM_PAGE_LEN];
+	struct debugfs_blob_wrapper page_blobs[NSIM_MODULE_EEPROM_PAGES];
+};
+
 struct nsim_ethtool {
 	u32 get_err;
 	u32 set_err;
@@ -94,6 +104,7 @@ struct nsim_ethtool {
 		u32 supported;
 		u32 direction;
 	} mac_lb;
+	struct nsim_ethtool_module module;
 };
 
 struct nsim_rq {
-- 
2.53.0

[PATCH net-next 11/11] selftests: drv-net: Add CMIS loopback netdevsim test

[Björn Töpel]

Extend loopback_nsim.py with netdevsim-specific tests for CMIS module
loopback. These tests seed the EEPROM via debugfs and verify
register-level behavior.

Tests cover: no-module GET, all/partial capability reporting, EEPROM
byte verification for enable/disable and direction switching,
rejection of unsupported directions, rejection without CMIS
support, and combined MAC + MODULE dump.

Signed-off-by: Björn Töpel <bjorn@kernel.org>
---
 .../selftests/drivers/net/hw/loopback_nsim.py | 207 +++++++++++++++++-
 1 file changed, 206 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/hw/loopback_nsim.py b/tools/testing/selftests/drivers/net/hw/loopback_nsim.py
index 5edc999d920b..26e74718098a 100755
--- a/tools/testing/selftests/drivers/net/hw/loopback_nsim.py
+++ b/tools/testing/selftests/drivers/net/hw/loopback_nsim.py
@@ -1,10 +1,13 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
-"""Netdevsim-specific tests for MAC loopback via ethtool_ops.
+"""Netdevsim-specific tests for ethtool loopback.
 
 Verifies that MAC loopback entries appear in dumps, that SET
 operations update state correctly (both via GET and debugfs).
+
+Seeds the CMIS EEPROM via debugfs and verifies register-level
+behavior that can only be checked with controlled EEPROM contents.
 """
 
 import errno
@@ -15,6 +18,20 @@ from lib.py import KsftSkipEx, KsftFailEx, ksft_disruptive
 from lib.py import EthtoolFamily, NlError
 from lib.py import NetDrvEnv, ip, defer
 
+# CMIS register constants matching net/ethtool/cmis_loopback.c
+SFF8024_ID_QSFP_DD = 0x18
+
+# Page 01h, Byte 142: bit 5 = Page 13h supported
+CMIS_DIAG_PAGE13_BIT = 1 << 5
+
+# Page 13h, Byte 128: loopback capability bits
+CMIS_LB_CAP_MEDIA_OUTPUT = 1 << 0
+CMIS_LB_CAP_MEDIA_INPUT = 1 << 1
+CMIS_LB_CAP_HOST_OUTPUT = 1 << 2
+CMIS_LB_CAP_HOST_INPUT = 1 << 3
+CMIS_LB_CAP_ALL = (CMIS_LB_CAP_MEDIA_OUTPUT | CMIS_LB_CAP_MEDIA_INPUT |
+                    CMIS_LB_CAP_HOST_OUTPUT | CMIS_LB_CAP_HOST_INPUT)
+
 # Direction flags as YNL returns them
 DIR_NONE = set()
 DIR_NEAR_END = {'near-end'}
@@ -35,6 +52,54 @@ def _dfs_write_u32(cfg, path, val):
         f.write(str(val))
 
 
+def _nsim_write_page_byte(cfg, page, offset, value):
+    """Write a single byte to a netdevsim EEPROM page via debugfs."""
+    if offset < 128:
+        page_file = os.path.join(_nsim_dfs_path(cfg),
+                                 "ethtool/module/pages/0")
+        file_offset = offset
+    else:
+        page_file = os.path.join(_nsim_dfs_path(cfg),
+                                 f"ethtool/module/pages/{page}")
+        file_offset = offset - 128
+
+    with open(page_file, "r+b") as f:
+        f.seek(file_offset)
+        f.write(bytes([value]))
+
+
+def _nsim_read_page_byte(cfg, page, offset):
+    """Read a single byte from a netdevsim EEPROM page via debugfs."""
+    if offset < 128:
+        page_file = os.path.join(_nsim_dfs_path(cfg),
+                                 "ethtool/module/pages/0")
+        file_offset = offset
+    else:
+        page_file = os.path.join(_nsim_dfs_path(cfg),
+                                 f"ethtool/module/pages/{page}")
+        file_offset = offset - 128
+
+    with open(page_file, "rb") as f:
+        f.seek(file_offset)
+        return f.read(1)[0]
+
+
+def _nsim_seed_cmis(cfg, caps=CMIS_LB_CAP_ALL):
+    """Seed the netdevsim EEPROM with CMIS module identity and
+    loopback capabilities.
+    """
+    _nsim_write_page_byte(cfg, 0x00, 0, SFF8024_ID_QSFP_DD)
+    _nsim_write_page_byte(cfg, 0x01, 0x8e, CMIS_DIAG_PAGE13_BIT)
+    _nsim_write_page_byte(cfg, 0x13, 0x80, caps)
+
+
+def _nsim_clear_cmis(cfg):
+    """Clear CMIS identity bytes left by previous tests."""
+    _nsim_write_page_byte(cfg, 0x00, 0, 0)
+    _nsim_write_page_byte(cfg, 0x01, 0x8e, 0)
+    _nsim_write_page_byte(cfg, 0x13, 0x80, 0)
+
+
 def _get_loopback(cfg):
     results = cfg.ethnl.loopback_get({
         'header': {'dev-index': cfg.ifindex}
@@ -118,6 +183,138 @@ def test_set_mac_unknown_name(cfg):
                 "Expected EOPNOTSUPP for unknown name")
 
 
+def test_get_no_module(cfg):
+    """GET on a device with no CMIS module returns no module entries."""
+    _nsim_clear_cmis(cfg)
+
+    entries = _get_loopback(cfg)
+    mod_entries = [e for e in entries if e['component'] == 'module']
+    ksft_eq(len(mod_entries), 0, "Expected no module entries without CMIS module")
+
+
+def test_get_all_caps(cfg):
+    """GET with all four CMIS loopback capabilities seeded."""
+    _nsim_seed_cmis(cfg, CMIS_LB_CAP_ALL)
+
+    entries = _get_loopback(cfg)
+    mod_entries = [e for e in entries if e['component'] == 'module']
+
+    # Expect 2 entries (one per name), each with both directions
+    ksft_eq(len(mod_entries), 2, "Expected 2 module loopback entries")
+
+    host = [e for e in mod_entries if e['name'] == 'cmis-host']
+    media = [e for e in mod_entries if e['name'] == 'cmis-media']
+    ksft_eq(len(host), 1, "Expected 1 cmis-host entry")
+    ksft_eq(len(media), 1, "Expected 1 cmis-media entry")
+
+    ksft_eq(host[0]['supported'], DIR_NEAR_END | DIR_FAR_END)
+    ksft_eq(media[0]['supported'], DIR_NEAR_END | DIR_FAR_END)
+
+    for e in mod_entries:
+        ksft_eq(e['direction'], DIR_NONE,
+                f"Expected direction=off for {e['name']}")
+
+
+def test_get_partial_caps(cfg):
+    """GET with only host-input capability advertised."""
+    _nsim_seed_cmis(cfg, CMIS_LB_CAP_HOST_INPUT)
+
+    entries = _get_loopback(cfg)
+    mod_entries = [e for e in entries if e['component'] == 'module']
+    ksft_eq(len(mod_entries), 1, "Expected 1 module loopback entry")
+    ksft_eq(mod_entries[0]['name'], 'cmis-host')
+    ksft_eq(mod_entries[0]['supported'], DIR_NEAR_END)
+
+
+@ksft_disruptive
+def test_set_verify_eeprom(cfg):
+    """SET near-end and verify the EEPROM control byte directly."""
+    _nsim_seed_cmis(cfg, CMIS_LB_CAP_ALL)
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    _set_loopback(cfg, 'module', 'cmis-host', 'near-end')
+    defer(_set_loopback, cfg, 'module', 'cmis-host', 0)
+
+    # Host Side Input = Page 13h, Byte 183
+    val = _nsim_read_page_byte(cfg, 0x13, 183)
+    ksft_eq(val, 0xff, "Host Side Input control byte should be 0xff")
+
+    # Disable and verify
+    _set_loopback(cfg, 'module', 'cmis-host', 0)
+    val = _nsim_read_page_byte(cfg, 0x13, 183)
+    ksft_eq(val, 0x00, "Host Side Input should be 0x00 after disable")
+
+
+@ksft_disruptive
+def test_set_direction_switch_eeprom(cfg):
+    """Switch directions and verify both EEPROM bytes."""
+    _nsim_seed_cmis(cfg, CMIS_LB_CAP_ALL)
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    _set_loopback(cfg, 'module', 'cmis-host', 'near-end')
+    defer(_set_loopback, cfg, 'module', 'cmis-host', 0)
+
+    # Switch to far-end
+    _set_loopback(cfg, 'module', 'cmis-host', 'far-end')
+
+    # Near-end (Host Input, Byte 183) should be disabled
+    val = _nsim_read_page_byte(cfg, 0x13, 183)
+    ksft_eq(val, 0x00, "Near-end should be disabled after switch")
+    # Far-end (Host Output, Byte 182) should be enabled
+    val = _nsim_read_page_byte(cfg, 0x13, 182)
+    ksft_eq(val, 0xff, "Far-end should be enabled after switch")
+
+
+@ksft_disruptive
+def test_set_unsupported_direction(cfg):
+    """SET with unsupported direction should fail."""
+    _nsim_seed_cmis(cfg, CMIS_LB_CAP_HOST_INPUT)  # only near-end
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    try:
+        _set_loopback(cfg, 'module', 'cmis-host', 'far-end')
+        raise KsftFailEx("Should have rejected unsupported direction")
+    except NlError as e:
+        ksft_eq(e.error, errno.EOPNOTSUPP,
+                "Expected EOPNOTSUPP for unsupported direction")
+
+
+@ksft_disruptive
+def test_set_no_cmis(cfg):
+    """SET on a device without CMIS loopback support should fail."""
+    _nsim_clear_cmis(cfg)
+
+    ip(f"link set dev {cfg.ifname} down")
+    defer(ip, f"link set dev {cfg.ifname} up")
+
+    try:
+        _set_loopback(cfg, 'module', 'cmis-host', 'near-end')
+        raise KsftFailEx("Should have rejected SET without CMIS support")
+    except NlError as e:
+        ksft_eq(e.error, errno.EOPNOTSUPP,
+                "Expected EOPNOTSUPP without CMIS support")
+
+
+def test_combined_dump(cfg):
+    """Dump should return both MAC and MODULE entries."""
+    _nsim_seed_cmis(cfg, CMIS_LB_CAP_ALL)
+    defer(_nsim_clear_cmis, cfg)
+
+    entries = _get_loopback(cfg)
+    mac_entries = [e for e in entries if e['component'] == 'mac']
+    mod_entries = [e for e in entries if e['component'] == 'module']
+
+    ksft_eq(len(mac_entries), 1, "Expected 1 MAC entry")
+    ksft_eq(len(mod_entries), 2, "Expected 2 MODULE entries")
+    ksft_eq(mac_entries[0]['name'], 'mac')
+
+
 def main() -> None:
     with NetDrvEnv(__file__) as cfg:
         cfg.ethnl = EthtoolFamily()
@@ -127,6 +324,14 @@ def main() -> None:
             test_set_mac_near_end,
             test_set_mac_disable,
             test_set_mac_unknown_name,
+            test_get_no_module,
+            test_get_all_caps,
+            test_get_partial_caps,
+            test_set_verify_eeprom,
+            test_set_direction_switch_eeprom,
+            test_set_unsupported_direction,
+            test_set_no_cmis,
+            test_combined_dump,
         ], args=(cfg, ))
     ksft_exit()
 
-- 
2.53.0

Re: [PATCH 6/11] ethtool: Add MAC loopback support via ethtool_ops

[Naveen Mamindlapalli]

On 2026-03-10 at 16:17:36, Björn Töpel (bjorn@kernel.org) wrote:
> Extend struct ethtool_ops with three loopback callbacks that drivers
> can implement for MAC-level loopback control:
> 
>  - get_loopback(dev, name, id, entry): exact lookup by name and
>    instance id, used by doit (single-entry GET) requests.
> 
>  - get_loopback_by_index(dev, index, entry): flat enumeration by
>    index, used by dumpit (multi-entry GET) requests to iterate all
>    loopback points on a device.
> 
>  - set_loopback(dev, entry, extack): apply a loopback configuration
>    change. Returns 1 if hardware state changed, 0 if no-op.
> 
> Wire the MAC component into loopback.c's dispatch functions. For dump
> enumeration, MAC entries are tried first via the driver's
> get_loopback_by_index() op, then MODULE/CMIS entries follow at the
> next index offset.
> 
> Signed-off-by: Björn Töpel <bjorn@kernel.org>
> ---
>  include/linux/ethtool.h |  7 ++++++
>  net/ethtool/loopback.c  | 56 ++++++++++++++++++++++++++++++++---------
>  2 files changed, 51 insertions(+), 12 deletions(-)
>

The MAC loopback callbacks look good.
Reviewed-by: Naveen Mamindlapalli <naveenm@marvell.com>
 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index c9beca11fc40..8893e732f930 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -1321,6 +1321,13 @@ struct ethtool_ops {
>  	int	(*set_mm)(struct net_device *dev, struct ethtool_mm_cfg *cfg,
>  			  struct netlink_ext_ack *extack);
>  	void	(*get_mm_stats)(struct net_device *dev, struct ethtool_mm_stats *stats);
> +	int	(*get_loopback)(struct net_device *dev, const char *name,
> +				u32 id, struct ethtool_loopback_entry *entry);
> +	int	(*get_loopback_by_index)(struct net_device *dev, u32 index,
> +					 struct ethtool_loopback_entry *entry);
> +	int	(*set_loopback)(struct net_device *dev,
> +				const struct ethtool_loopback_entry *entry,
> +				struct netlink_ext_ack *extack);
>  };
>  
>  int ethtool_check_ops(const struct ethtool_ops *ops);
> diff --git a/net/ethtool/loopback.c b/net/ethtool/loopback.c
> index ed184f45322e..9c5834be743f 100644
> --- a/net/ethtool/loopback.c
> +++ b/net/ethtool/loopback.c
> @@ -86,6 +86,10 @@ static int loopback_get(struct net_device *dev,
>  			struct ethtool_loopback_entry *entry)
>  {
>  	switch (component) {
> +	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
> +		if (!dev->ethtool_ops->get_loopback)
> +			return -EOPNOTSUPP;
> +		return dev->ethtool_ops->get_loopback(dev, name, id, entry);
>  	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
>  		return ethtool_cmis_get_loopback(dev, name, entry);
>  	default:
> @@ -93,10 +97,22 @@ static int loopback_get(struct net_device *dev,
>  	}
>  }
>  
> -static int loopback_get_by_index(struct net_device *dev, u32 index,
> +static int loopback_get_by_index(struct net_device *dev,
> +				 enum ethtool_loopback_component component,
> +				 u32 index,
>  				 struct ethtool_loopback_entry *entry)
>  {
> -	return ethtool_cmis_get_loopback_by_index(dev, index, entry);
> +	switch (component) {
> +	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
> +		if (!dev->ethtool_ops->get_loopback_by_index)
> +			return -EOPNOTSUPP;
> +		return dev->ethtool_ops->get_loopback_by_index(dev, index,
> +							       entry);
> +	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
> +		return ethtool_cmis_get_loopback_by_index(dev, index, entry);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
>  }
>  
>  static int loopback_prepare_data(const struct ethnl_req_info *req_base,
> @@ -116,7 +132,8 @@ static int loopback_prepare_data(const struct ethnl_req_info *req_base,
>  		ret = loopback_get(dev, req_info->component, req_info->id,
>  				   req_info->name, &data->entry);
>  	else
> -		ret = loopback_get_by_index(dev, req_info->index, &data->entry);
> +		ret = loopback_get_by_index(dev, req_info->component,
> +					    req_info->index, &data->entry);
>  
>  	ethnl_ops_complete(dev);
>  
> @@ -225,6 +242,10 @@ static int __loopback_set(struct net_device *dev,
>  			  struct netlink_ext_ack *extack)
>  {
>  	switch (entry->component) {
> +	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
> +		if (!dev->ethtool_ops->set_loopback)
> +			return -EOPNOTSUPP;
> +		return dev->ethtool_ops->set_loopback(dev, entry, extack);
>  	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
>  		return ethtool_cmis_set_loopback(dev, entry, extack);
>  	default:
> @@ -274,20 +295,31 @@ static int loopback_dump_one_dev(struct sk_buff *skb,
>  {
>  	struct loopback_req_info *req_info =
>  		container_of(ctx->req_info, struct loopback_req_info, base);
> +	/* pos_sub encodes: upper 16 bits = component phase, lower 16 = index
> +	 * within that component. dump_one_dev is called repeatedly with
> +	 * increasing pos_sub until all components are exhausted.
> +	 */
> +	enum ethtool_loopback_component phase = *pos_sub >> 16;
> +	u32 idx = *pos_sub & 0xffff;
>  	int ret;
>  
> -	for (;; (*pos_sub)++) {
> -		req_info->index = *pos_sub;
> -		ret = ethnl_default_dump_one(skb, ctx->req_info->dev, ctx,
> -					     info);
> -		if (ret == -EOPNOTSUPP)
> -			break;
> -		if (ret)
> -			return ret;
> +	for (; phase <= ETHTOOL_LOOPBACK_COMPONENT_MODULE; phase++) {
> +		for (;; idx++) {
> +			req_info->component = phase;
> +			req_info->index = idx;
> +			ret = ethnl_default_dump_one(skb, ctx->req_info->dev,
> +						     ctx, info);
> +			if (ret == -EOPNOTSUPP)
> +				break;
> +			if (ret) {
> +				*pos_sub = ((unsigned long)phase << 16) | idx;
> +				return ret;
> +			}
> +		}
> +		idx = 0;
>  	}
>  
>  	*pos_sub = 0;
> -
>  	return 0;
>  }
>  
> 
> -- 
> 2.53.0
> 

Re: [PATCH net-next 00/11] ethtool: Generic loopback support

[Naveen Mamindlapalli]

On 2026-03-10 at 16:17:30, Björn Töpel (bjorn@kernel.org) wrote:
> Hi!
> 
> Background
> ==========
> 
> This series adds a generic ethtool loopback framework with GET/SET
> netlink commands, using a component/id/name/direction model that can
> (Hopefully! Please refer to "Open questions" below.) represent
> loopback points across the network path (MAC, MODULE, PHY, PCS).
> 
> This is the v1 proper of the loopback series, reworked based on
> feedback from previous RFC v2 [1].
> 
> The main change since the RFC v2 is that LOOPBACK_GET no longer
> returns an array of entries in a single doit reply. Instead, it uses
> the dumpit infrastructure -- each loopback entry is a separate netlink
> message in a DUMP response. This follows the same pattern as PHY_GET
> and the perphy helpers in net/ethtool/netlink.c, as suggested by
> Maxime.
> 
> A filtered DUMP (with a dev-index in the header) lists all loopback
> entries for that netdev; an unfiltered DUMP iterates over all netdevs.
> The doit handler is also available: when the request includes an
> ETHTOOL_A_LOOPBACK_ENTRY nest with component + name, it returns that
> specific entry.
> 
> The loopback model remains the same: LOOPBACK_GET/SET with a generic
> component/name/direction model that can represent loopback points
> across the data path -- MODULE, PHY, MAC, and PCS. This series wires
> up MODULE/CMIS and MAC as the first users; PHY and PCS return
> -EOPNOTSUPP for now.
> 
> Loopback lookup and enumeration
> ===============================
> 
> Each loopback entry is uniquely identified by the tuple (component,
> id, name). The kernel provides two GET paths:
> 
>  - Exact lookup (doit): the user specifies component + name (and
>    optionally id) in the request. The kernel dispatches to the right
>    component handler: for MAC, it calls the driver's
>    get_loopback(dev, name, id, entry) ethtool_op; for MODULE, it
>    calls ethtool_cmis_get_loopback(dev, name, entry). Returns a
>    single entry or -EOPNOTSUPP.
> 
>  - Index enumeration (dump): the kernel iterates a flat index space
>    across all components on a device. Each component's
>    get_loopback_by_index(dev, index, entry) is tried in order (MAC
>    first via ethtool_ops, then MODULE/CMIS). The dump stops when all
>    components return -EOPNOTSUPP. This integrates with the generic
>    dump_one_dev sub-iterator infrastructure added in patch 1.
> 
> SET takes one or more entries, each with component + name + direction,
> and dispatches to the driver's set_loopback() ethtool_op (MAC) or
> ethtool_cmis_set_loopback() (MODULE).
> 
> The Common Management Interface Specification (CMIS) defines four
> diagnostic loopback types, characterized by location (Host or Media
> Side) and signal direction:
> 
>  - Host Side Input (Rx->Tx) -- near-end
>  - Host Side Output (Tx->Rx) -- far-end
>  - Media Side Input (Rx->Tx) -- near-end
>  - Media Side Output (Tx->Rx) -- far-end
> 
> Support is detected via Page 13h Byte 128, and loopback is controlled
> via Page 13h Bytes 180-183 (one byte per type, one bit per lane).
> 
> The CMIS helpers work entirely over get/set_module_eeprom_by_page, so
> any driver that already has EEPROM page access gets module loopback
> without new ethtool_ops or driver changes.
> 
> Currently, only mellanox/mlxsw, and broadcom/bnxt support CMIS
> operations. I'll follow-up with mlx5 support.
> 
> Implementation
> ==============
> 
> Patch 1/11 ethtool: Add dump_one_dev callback for per-device sub-iteration
>   Replaces the per-PHY specific dump functions with a generic
>   sub-iterator infrastructure driven by a dump_one_dev callback in
>   ethnl_request_ops. When ops->dump_one_dev is set,
>   ethnl_default_start() saves the target device's ifindex for
>   filtered dumps, and ethnl_default_dumpit() delegates per-device
>   iteration to the callback. No separate start/dumpit/done functions
>   are needed.
> 
> Patch 2/11 ethtool: Add loopback netlink UAPI definitions
>   Adds the YAML spec and generated UAPI header for the new
>   LOOPBACK_GET/SET commands. Each loopback entry carries a component
>   type, optional id, name string, supported directions bitmask, and
>   current direction.
> 
> Patch 3/11 ethtool: Add loopback GET/SET netlink implementation
>   Implements GET/SET dispatch in a new loopback.c. GET uses the
>   dump_one_dev infrastructure for dump enumeration (by flat index)
>   and supports doit exact lookup by (component, id, name) via
>   parse_request. SET switches on the component and calls the right
>   handler per entry. No components are wired yet.
> 
> Patch 4/11 ethtool: Add CMIS loopback helpers for module loopback control
>   Adds cmis_loopback.c with the MODULE component handlers and wires
>   them into loopback.c's dispatch. GET enumerates entries by index
>   (ethtool_cmis_get_loopback_by_index) or looks up by name
>   (ethtool_cmis_get_loopback). SET (ethtool_cmis_set_loopback)
>   resolves name to control byte indices and enforces mutual
>   exclusivity.
> 
> Patch 5/11 selftests: drv-net: Add loopback driver test
>   Adds loopback_drv.py with generic tests that work on any device
>   with module loopback support: enable/disable, direction switching,
>   idempotent enable, and rejection while interface is up.
> 
> Patch 6/11 ethtool: Add MAC loopback support via ethtool_ops
>   Extends struct ethtool_ops with three loopback callbacks for
>   driver-level MAC loopback: get_loopback (exact lookup by name/id),
>   get_loopback_by_index (dump enumeration), and set_loopback. Wires
>   the MAC component into loopback.c's dispatch. For dump enumeration,
>   MAC entries are tried first, then MODULE/CMIS entries follow at the
>   next index offset.
> 
> Patch 7/11 netdevsim: Add MAC loopback simulation
>   Implements the three ethtool loopback ops in netdevsim. Exposes a
>   single MAC loopback entry ("mac") with both near-end and far-end
>   support. State is stored in memory and exposed via debugfs under
>   ethtool/mac_lb/{supported,direction}.
> 
> Patch 8/11 selftests: drv-net: Add MAC loopback netdevsim test
>   Adds loopback_nsim.py with netdevsim-specific tests for MAC
>   loopback: entry presence, SET/GET round-trip with debugfs
>   verification, and error paths.
> 
> Patch 9/11 MAINTAINERS: Add entry for ethtool loopback
>   Adds a MAINTAINERS entry for the ethtool loopback subsystem covering
>   the core loopback and CMIS loopback netlink implementation, and the
>   associated selftests.
> 
> Patch 10/11 netdevsim: Add module EEPROM simulation via debugfs
>   Adds get/set_module_eeprom_by_page to netdevsim, backed by a
>   256-page x 128-byte array exposed via debugfs.
> 
> Patch 11/11 selftests: drv-net: Add CMIS loopback netdevsim test
>   Extends loopback_nsim.py with netdevsim-specific tests that seed the
>   EEPROM via debugfs: capability reporting, EEPROM byte verification,
>   combined MAC + MODULE dump, and error paths.
> 
> Changes since RFC v2
> ====================
> 
>  - Switched LOOPBACK_GET from doit-with-array to dumpit, where each
>    loopback entry is a separate netlink message. Uses the new generic
>    dump_one_dev sub-iterator infrastructure instead of duplicating the
>    perphy dump pattern. (Maxime)
> 
>  - u32 to u8 to represent the enums in the YAML. (Maxime)
>    
>  - Tried to document the YAML better. (Andrew)
> 
>  - Added doit exact lookup by (component, id, name) via
>    parse_request, so single-entry GET doesn't need a flat index.
> 
>  - Added MAC loopback support via three new ethtool_ops callbacks
>    (get_loopback(), get_loopback_by_index(), set_loopback()) with
>    netdevsim implementation and tests.
> 
>  - Added MAINTAINERS entry.
> 
> Limitations
> ===========
> 
> PHY and PCS loopback are defined in the UAPI but not yet implemented.
> 
> No per-lane support -- loopback is all-or-nothing (0xff/0x00) across
> lanes.
> 
> Open questions
> ==============
> 
>  - Is this the right extensibility model? I'd appreciate input from
>    other NIC vendors on whether component/name/direction is flexible
>    enough for their loopback implementations. Also, from the PHY/port
>    folks (Maxime, Russell)! Naveen, please LMK if the MAC side of
>    thing, is good enough for Marvell.

Hi Bjorn,

Is a SERDES component as Maxime suggested something you'd consider
for follow-up patches? It would be a natural fit for SoCs with a
separate SerDes hardware block.

Thanks,
Naveen

> 
>  - Are patches 10-11 (netdevsim EEPROM simulation + netdevsim-specific
>    tests) worth carrying? They drive the CMIS Page 13h registers from
>    debugfs, which gives good coverage without hardware, but it's
>    another netdevsim surface to maintain. If the consensus is that the
>    generic driver tests (patch 5) are sufficient, I'm happy to drop
>    them.
> 
>  - Extend mellanox/mlx5 with .set_module_eeprom_by_page() callback. I
>    got it to work in [6], but would like feedback from the Mellanox
>    folks.
> 
> Related work
> ============
> 
> [1] Generic loopback support, RFC v2
>   https://lore.kernel.org/netdev/20260219130050.2390226-1-bjorn@kernel.org/
> [2] CMIS loopback, RFC v1
>   https://lore.kernel.org/netdev/20260219130050.2390226-1-bjorn@kernel.org/
> [3] New loopback modes
>   https://lore.kernel.org/netdev/20251024044849.1098222-1-hkelam@marvell.com/
> [4] PHY loopback
>   https://lore.kernel.org/netdev/20240911212713.2178943-1-maxime.chevallier@bootlin.com/
> [5] bnxt_en: add .set_module_eeprom_by_page() support
>   https://lore.kernel.org/netdev/20250310183129.3154117-8-michael.chan@broadcom.com/
> [6] net/mlx5e: Implement set_module_eeprom_by_page ethtool callback
>   https://lore.kernel.org/netdev/20260219130050.2390226-5-bjorn@kernel.org/
> 
> 
> Björn Töpel (11):
>   ethtool: Add dump_one_dev callback for per-device sub-iteration
>   ethtool: Add loopback netlink UAPI definitions
>   ethtool: Add loopback GET/SET netlink implementation
>   ethtool: Add CMIS loopback helpers for module loopback control
>   selftests: drv-net: Add loopback driver test
>   ethtool: Add MAC loopback support via ethtool_ops
>   netdevsim: Add MAC loopback simulation
>   selftests: drv-net: Add MAC loopback netdevsim test
>   MAINTAINERS: Add entry for ethtool loopback
>   netdevsim: Add module EEPROM simulation via debugfs
>   selftests: drv-net: Add CMIS loopback netdevsim test
> 
>  Documentation/netlink/specs/ethtool.yaml      | 123 ++++++
>  MAINTAINERS                                   |   6 +
>  drivers/net/netdevsim/ethtool.c               | 147 +++++++
>  drivers/net/netdevsim/netdevsim.h             |  15 +
>  include/linux/ethtool.h                       |  23 +
>  .../uapi/linux/ethtool_netlink_generated.h    |  59 +++
>  net/ethtool/Makefile                          |   2 +-
>  net/ethtool/cmis_loopback.c                   | 407 ++++++++++++++++++
>  net/ethtool/loopback.c                        | 341 +++++++++++++++
>  net/ethtool/mse.c                             |   1 +
>  net/ethtool/netlink.c                         | 285 ++++--------
>  net/ethtool/netlink.h                         |  45 ++
>  net/ethtool/phy.c                             |   1 +
>  net/ethtool/plca.c                            |   2 +
>  net/ethtool/pse-pd.c                          |   1 +
>  .../testing/selftests/drivers/net/hw/Makefile |   2 +
>  .../selftests/drivers/net/hw/loopback_drv.py  | 226 ++++++++++
>  .../selftests/drivers/net/hw/loopback_nsim.py | 340 +++++++++++++++
>  18 files changed, 1820 insertions(+), 206 deletions(-)
>  create mode 100644 net/ethtool/cmis_loopback.c
>  create mode 100644 net/ethtool/loopback.c
>  create mode 100755 tools/testing/selftests/drivers/net/hw/loopback_drv.py
>  create mode 100755 tools/testing/selftests/drivers/net/hw/loopback_nsim.py
> 
> 
> base-commit: 52ede1bce557c66309f41ac29dd190be23ca9129
> -- 
> 2.53.0
> 
> 

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Maxime Chevallier]

Hi again Björn,

First, thanks for iterating so quick !

On 10/03/2026 11:47, Björn Töpel wrote:
> Add the netlink YAML spec and auto-generated UAPI header for a unified
> loopback interface covering MAC, PCS, PHY, and pluggable module
> components.
> 
> Each loopback point is described by a nested entry attribute
> containing:
> 
>  - component  where in the path (MAC, PCS, PHY, MODULE)
>  - name       subsystem label, e.g. "cmis-host" or "cmis-media"
>  - id         optional instance selector (e.g. PHY id, port id)
>  - supported  bitmask of supported directions
>  - direction  NEAR_END, FAR_END, or 0 (disabled)
> 
> Signed-off-by: Björn Töpel <bjorn@kernel.org>
> ---
>  Documentation/netlink/specs/ethtool.yaml      | 123 ++++++++++++++++++
>  .../uapi/linux/ethtool_netlink_generated.h    |  59 +++++++++
>  2 files changed, 182 insertions(+)
> 
> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index 4707063af3b4..8bd14a3c946a 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml
> @@ -211,6 +211,49 @@ definitions:
>          name: discard
>          value: 31
>  
> +  -
> +    name: loopback-component
> +    type: enum
> +    doc: |
> +      Loopback component. Identifies where in the network path the
> +      loopback is applied.
> +    entries:
> +      -
> +        name: mac
> +        doc: MAC loopback. Loops traffic at the MAC block.
> +      -
> +        name: pcs
> +        doc: |
> +          PCS loopback. Loops traffic at the PCS sublayer between the
> +          MAC and the PHY.
> +      -
> +        name: phy
> +        doc: |
> +          Ethernet PHY loopback. This refers to the Ethernet PHY managed
> +          by phylib, not generic PHY drivers. A Base-T SFP module
> +          containing an Ethernet PHY driven by Linux should report
> +          loopback under this component, not module.
> +      -
> +        name: module
> +        doc: |
> +          Pluggable module (e.g. CMIS (Q)SFP) loopback. Covers loopback
> +          modes controlled via module firmware or EEPROM registers. When
> +          Linux drives an Ethernet PHY inside the module via phylib, use
> +          the phy component instead.

So to get back on Andrew's remarks, let's see if we can get something
closer to 802.3.

Here, we have loopback at various locations, which all depends on the
Ethernet standard you use.

It's usually in the PCS, PMA or PMD components. Thing is, we may have
these in multiple places in our link.

If we take an example with a 10G PHY, we may have :

+----SoC-----+
|            |
|  MAC       |- drivers/net/ethernet
|   |        |
| Base-R PCS |- could be in drivers/net/pcs, or directly
|   |        | in the MAC driver
|   |        |
|  SerDes    |- May be in drivers/phy, maybe handled by firmware,
|   |        |  maybe by the MAC driver, maybe by the PCS driver ?
+---|--------+
    |
    | 10GBase-R
    |
+---|-PHY+
|   |    |
| SerDes | \
|   |    | |
|  PCS   | |
|   |    |  > All of that handled by the drivers/net/phy PHY driver
|  PMA   | |
|   |    | |
|  PMD   | /
+---|----+
    |
    v 10GBaseT

So even the "PCS" loopback component is a bit ambiguous, are we talking
about the PHY PCS or the MAC PCS ?

Another thing to consider is that there may be multiple PCSs in the SoC
(e.g. a BaseX and a BaseR PCS like we have in mvpp2), the one in use
depends on the current interface between the MAC and the PHY.

Another open question is, do we deal with loopbacks that may affect
multi-netdev links ? Like the multi-lane modes we discussed with fbnic,
or even for embedded, interfaces such as QSGMII ?

As for the SerDes on the MAC side (say, the comphy on Marvell devices),
can we say it's a PMA for 10GBase-KR ? Or is it something that's simply
out of spec ?

So I'd say, maybe we should not have a PCS loopback component at all,
but instead loopback at the well-defined components on our link, that is:

 - MAC => MAC loopack, PCS on the MAC side, SerDes on the SoC, etc.
 - PHY => Loopbacks on the PCS/PHY/PMA withing the PHY device
 - Module => For non-PHY (Q)SFPs

The important part would therefore to get the "name" part right, making
sure we don't fall into driver specific names.

We can name that 'pcs', 'pma', 'pmd', or maybe even 'mii' ? Let's see :

+----SoC-----+
|            |
|  MAC       |- component = MAC, name = 'mac'
|   |        |
| Base-R PCS |- component = MAC, name = 'pcs'
|   |        |
|   |        |
|  SerDes    |- component = MAC, name = 'mii' ?
|   |        |
+---|--------+
    |
    | 10GBase-R
    |
+---|-PHY+
|   |    |
| SerDes | - component = PHY, name = 'mii' ?
|   |    |
|  PCS   | - component = PHY, name = 'pcs'
|   |    |
|  PMA   | - component = PHY, name = 'pma'
|   |    |
|  PMD   |- component = PHY, name = 'pmd' or 'mdi' ?
+---|----+
    |
    v 10GBaseT

Sorry that's a lot of questions and I don't expect you to have the
answer, but as what you've come-up with is taking a good shape, it's
important to decide on the overall design and draw some lines about
what do we support, and how :(

> +  -
> +    name: loopback-direction
> +    type: flags
> +    doc: |
> +      Loopback direction flags. Used as a bitmask in supported, and as
> +      a single value in direction.
> +    entries:
> +      -
> +        name: near-end
> +        doc: Near-end loopback; host-loop-host
> +      -
> +        name: far-end
> +        doc: Far-end loopback; line-loop-line

I was browsing 802.3, it uses the terminlogy of "local loopback" vs
"remote loopback", I suggest we use those.

Maxime

Re: [PATCH net-next 00/11] ethtool: Generic loopback support

[Björn Töpel]

Naveen Mamindlapalli <naveenm@marvell.com> writes:

>>  - Is this the right extensibility model? I'd appreciate input from
>>    other NIC vendors on whether component/name/direction is flexible
>>    enough for their loopback implementations. Also, from the PHY/port
>>    folks (Maxime, Russell)! Naveen, please LMK if the MAC side of
>>    thing, is good enough for Marvell.
>
> Hi Bjorn,
>
> Is a SERDES component as Maxime suggested something you'd consider
> for follow-up patches? It would be a natural fit for SoCs with a
> separate SerDes hardware block.

I don't think so -- rather, I think I'll follow Maxime's suggestion for
fewer components, where "component" would be "Linux abstraction/object",
and the "name" would be 802.3 vocabolary. Please see my reply to Maxime
(in the writing now ;-)).


Cheers,
Björn

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Björn Töpel]

Maxime Chevallier <maxime.chevallier@bootlin.com> writes:

> Hi again Björn,
>
> First, thanks for iterating so quick !

Thank *you* for helping me navigating the lower levels of the stack! I'm
trying to be like the Iron Maiden tune: Be quick or be... ? :-P

>> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
>> index 4707063af3b4..8bd14a3c946a 100644
>> --- a/Documentation/netlink/specs/ethtool.yaml
>> +++ b/Documentation/netlink/specs/ethtool.yaml
>> @@ -211,6 +211,49 @@ definitions:
>>          name: discard
>>          value: 31
>>  
>> +  -
>> +    name: loopback-component
>> +    type: enum
>> +    doc: |
>> +      Loopback component. Identifies where in the network path the
>> +      loopback is applied.
>> +    entries:
>> +      -
>> +        name: mac
>> +        doc: MAC loopback. Loops traffic at the MAC block.
>> +      -
>> +        name: pcs
>> +        doc: |
>> +          PCS loopback. Loops traffic at the PCS sublayer between the
>> +          MAC and the PHY.
>> +      -
>> +        name: phy
>> +        doc: |
>> +          Ethernet PHY loopback. This refers to the Ethernet PHY managed
>> +          by phylib, not generic PHY drivers. A Base-T SFP module
>> +          containing an Ethernet PHY driven by Linux should report
>> +          loopback under this component, not module.
>> +      -
>> +        name: module
>> +        doc: |
>> +          Pluggable module (e.g. CMIS (Q)SFP) loopback. Covers loopback
>> +          modes controlled via module firmware or EEPROM registers. When
>> +          Linux drives an Ethernet PHY inside the module via phylib, use
>> +          the phy component instead.
>
> So to get back on Andrew's remarks, let's see if we can get something
> closer to 802.3.
>
> Here, we have loopback at various locations, which all depends on the
> Ethernet standard you use.
>
> It's usually in the PCS, PMA or PMD components. Thing is, we may have
> these in multiple places in our link.
>
> If we take an example with a 10G PHY, we may have :
>
> +----SoC-----+
> |            |
> |  MAC       |- drivers/net/ethernet
> |   |        |
> | Base-R PCS |- could be in drivers/net/pcs, or directly
> |   |        | in the MAC driver
> |   |        |
> |  SerDes    |- May be in drivers/phy, maybe handled by firmware,
> |   |        |  maybe by the MAC driver, maybe by the PCS driver ?
> +---|--------+
>     |
>     | 10GBase-R
>     |
> +---|-PHY+
> |   |    |
> | SerDes | \
> |   |    | |
> |  PCS   | |
> |   |    |  > All of that handled by the drivers/net/phy PHY driver
> |  PMA   | |
> |   |    | |
> |  PMD   | /
> +---|----+
>     |
>     v 10GBaseT
>
> So even the "PCS" loopback component is a bit ambiguous, are we talking
> about the PHY PCS or the MAC PCS ?
>
> Another thing to consider is that there may be multiple PCSs in the SoC
> (e.g. a BaseX and a BaseR PCS like we have in mvpp2), the one in use
> depends on the current interface between the MAC and the PHY.
>
> Another open question is, do we deal with loopbacks that may affect
> multi-netdev links ? Like the multi-lane modes we discussed with fbnic,
> or even for embedded, interfaces such as QSGMII ?

Hmm, TBH punt on it for now. The current design is per-netdev, and
drivers should only expose loopbacks they can scope to a single netdev.
Multi-netdev loopbacks can be addressed later if a real use case arises.
That keeps the series focused and avoids designing for hypotheticals.

> As for the SerDes on the MAC side (say, the comphy on Marvell devices),
> can we say it's a PMA for 10GBase-KR ? Or is it something that's simply
> out of spec ?
>
> So I'd say, maybe we should not have a PCS loopback component at all,
> but instead loopback at the well-defined components on our link, that is:
>
>  - MAC => MAC loopack, PCS on the MAC side, SerDes on the SoC, etc.
>  - PHY => Loopbacks on the PCS/PHY/PMA withing the PHY device
>  - Module => For non-PHY (Q)SFPs

Less is more! I like that! So, the component maps to the Linux driver
boundary (who owns the loopback), and the name is the 802.3 sublayer
within that device?

> The important part would therefore to get the "name" part right, making
> sure we don't fall into driver specific names.
>
> We can name that 'pcs', 'pma', 'pmd', or maybe even 'mii' ? Let's see :
>
> +----SoC-----+
> |            |
> |  MAC       |- component = MAC, name = 'mac'
> |   |        |
> | Base-R PCS |- component = MAC, name = 'pcs'
> |   |        |
> |   |        |
> |  SerDes    |- component = MAC, name = 'mii' ?
> |   |        |
> +---|--------+
>     |
>     | 10GBase-R
>     |
> +---|-PHY+
> |   |    |
> | SerDes | - component = PHY, name = 'mii' ?
> |   |    |
> |  PCS   | - component = PHY, name = 'pcs'
> |   |    |
> |  PMA   | - component = PHY, name = 'pma'
> |   |    |
> |  PMD   |- component = PHY, name = 'pmd' or 'mdi' ?
> +---|----+
>     |
>     v 10GBaseT
>
> Sorry that's a lot of questions and I don't expect you to have the
> answer, but as what you've come-up with is taking a good shape, it's
> important to decide on the overall design and draw some lines about
> what do we support, and how :(

(Again, this is why input from folks like you/Andrew/Naveen is
excellent! (Hey, I just wanted the CMIS loopback to start with! ;-)))

I like this. The nice thing is that since "name" is a string, we're not
locked into an enum -- drivers report what they have using 802.3
vocabulary, and we document the recommended names (pcs, pma, pmd, mii)
with references? That way it's unambiguous, but not too constrained.

For the next spin, I'll drop the pcs component entirely and keep only
mac, phy, and module. I'll also expand the component docs to explain
that the sublayer granularity lives in the name attribute using 802.3
terminology. How does that sound?

>> +  -
>> +    name: loopback-direction
>> +    type: flags
>> +    doc: |
>> +      Loopback direction flags. Used as a bitmask in supported, and as
>> +      a single value in direction.
>> +    entries:
>> +      -
>> +        name: near-end
>> +        doc: Near-end loopback; host-loop-host
>> +      -
>> +        name: far-end
>> +        doc: Far-end loopback; line-loop-line
>
> I was browsing 802.3, it uses the terminlogy of "local loopback" vs
> "remote loopback", I suggest we use those.

Sounds good!

Thanks for taking the time to think through the layering -- this is much
cleaner.


Björn

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Oleksij Rempel]

Hi all,

On Wed, Mar 11, 2026 at 08:33:26AM +0100, Maxime Chevallier wrote:
> Hi again Björn,
> 
> First, thanks for iterating so quick !
> 
> On 10/03/2026 11:47, Björn Töpel wrote:
> > Add the netlink YAML spec and auto-generated UAPI header for a unified
> > loopback interface covering MAC, PCS, PHY, and pluggable module
> > components.
> > 
> > Each loopback point is described by a nested entry attribute
> > containing:
> > 
> >  - component  where in the path (MAC, PCS, PHY, MODULE)
> >  - name       subsystem label, e.g. "cmis-host" or "cmis-media"
> >  - id         optional instance selector (e.g. PHY id, port id)
> >  - supported  bitmask of supported directions
> >  - direction  NEAR_END, FAR_END, or 0 (disabled)
> > 
> > Signed-off-by: Björn Töpel <bjorn@kernel.org>
> > ---
> >  Documentation/netlink/specs/ethtool.yaml      | 123 ++++++++++++++++++
> >  .../uapi/linux/ethtool_netlink_generated.h    |  59 +++++++++
> >  2 files changed, 182 insertions(+)
> > 
> > diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> > index 4707063af3b4..8bd14a3c946a 100644
> > --- a/Documentation/netlink/specs/ethtool.yaml
> > +++ b/Documentation/netlink/specs/ethtool.yaml
> > @@ -211,6 +211,49 @@ definitions:
> >          name: discard
> >          value: 31
> >  
> > +  -
> > +    name: loopback-component
> > +    type: enum
> > +    doc: |
> > +      Loopback component. Identifies where in the network path the
> > +      loopback is applied.
> > +    entries:
> > +      -
> > +        name: mac
> > +        doc: MAC loopback. Loops traffic at the MAC block.
> > +      -
> > +        name: pcs
> > +        doc: |
> > +          PCS loopback. Loops traffic at the PCS sublayer between the
> > +          MAC and the PHY.
> > +      -
> > +        name: phy
> > +        doc: |
> > +          Ethernet PHY loopback. This refers to the Ethernet PHY managed
> > +          by phylib, not generic PHY drivers. A Base-T SFP module
> > +          containing an Ethernet PHY driven by Linux should report
> > +          loopback under this component, not module.
> > +      -
> > +        name: module
> > +        doc: |
> > +          Pluggable module (e.g. CMIS (Q)SFP) loopback. Covers loopback
> > +          modes controlled via module firmware or EEPROM registers. When
> > +          Linux drives an Ethernet PHY inside the module via phylib, use
> > +          the phy component instead.
> 
> So to get back on Andrew's remarks, let's see if we can get something
> closer to 802.3.
> 
> Here, we have loopback at various locations, which all depends on the
> Ethernet standard you use.
> 
> It's usually in the PCS, PMA or PMD components. Thing is, we may have
> these in multiple places in our link.
> 
> If we take an example with a 10G PHY, we may have :
> 
> +----SoC-----+
> |            |
> |  MAC       |- drivers/net/ethernet
> |   |        |
> | Base-R PCS |- could be in drivers/net/pcs, or directly
> |   |        | in the MAC driver
> |   |        |
> |  SerDes    |- May be in drivers/phy, maybe handled by firmware,
> |   |        |  maybe by the MAC driver, maybe by the PCS driver ?
> +---|--------+
>     |
>     | 10GBase-R
>     |
> +---|-PHY+
> |   |    |
> | SerDes | \
> |   |    | |
> |  PCS   | |
> |   |    |  > All of that handled by the drivers/net/phy PHY driver
> |  PMA   | |
> |   |    | |
> |  PMD   | /
> +---|----+
>     |
>     v 10GBaseT
> 
> So even the "PCS" loopback component is a bit ambiguous, are we talking
> about the PHY PCS or the MAC PCS ?
> 
> Another thing to consider is that there may be multiple PCSs in the SoC
> (e.g. a BaseX and a BaseR PCS like we have in mvpp2), the one in use
> depends on the current interface between the MAC and the PHY.
> 
> Another open question is, do we deal with loopbacks that may affect
> multi-netdev links ? Like the multi-lane modes we discussed with fbnic,
> or even for embedded, interfaces such as QSGMII ?
> 
> As for the SerDes on the MAC side (say, the comphy on Marvell devices),
> can we say it's a PMA for 10GBase-KR ? Or is it something that's simply
> out of spec ?
> 
> So I'd say, maybe we should not have a PCS loopback component at all,
> but instead loopback at the well-defined components on our link, that is:
> 
>  - MAC => MAC loopack, PCS on the MAC side, SerDes on the SoC, etc.
>  - PHY => Loopbacks on the PCS/PHY/PMA withing the PHY device
>  - Module => For non-PHY (Q)SFPs
> 
> The important part would therefore to get the "name" part right, making
> sure we don't fall into driver specific names.
> 
> We can name that 'pcs', 'pma', 'pmd', or maybe even 'mii' ? Let's see :
> 
> +----SoC-----+
> |            |
> |  MAC       |- component = MAC, name = 'mac'
> |   |        |
> | Base-R PCS |- component = MAC, name = 'pcs'
> |   |        |
> |   |        |
> |  SerDes    |- component = MAC, name = 'mii' ?
> |   |        |
> +---|--------+
>     |
>     | 10GBase-R
>     |
> +---|-PHY+
> |   |    |
> | SerDes | - component = PHY, name = 'mii' ?
> |   |    |
> |  PCS   | - component = PHY, name = 'pcs'
> |   |    |
> |  PMA   | - component = PHY, name = 'pma'
> |   |    |
> |  PMD   |- component = PHY, name = 'pmd' or 'mdi' ?
> +---|----+
>     |
>     v 10GBaseT
> 
> Sorry that's a lot of questions and I don't expect you to have the
> answer, but as what you've come-up with is taking a good shape, it's
> important to decide on the overall design and draw some lines about
> what do we support, and how :(
> 
> > +  -
> > +    name: loopback-direction
> > +    type: flags
> > +    doc: |
> > +      Loopback direction flags. Used as a bitmask in supported, and as
> > +      a single value in direction.
> > +    entries:
> > +      -
> > +        name: near-end
> > +        doc: Near-end loopback; host-loop-host
> > +      -
> > +        name: far-end
> > +        doc: Far-end loopback; line-loop-line
> 
> I was browsing 802.3, it uses the terminlogy of "local loopback" vs
> "remote loopback", I suggest we use those.

I do not want to overload this initial series with complex topology problems,
but we must ensure the proposed UAPI does not block future extensions. I am
currently investigating automated datapath diagnostic, and a flat component +
name model will eventually fail us.

Looking at the current patch:
- component (MAC, PCS, PHY, MODULE)
- name (subsystem label)
- id (local instance selector)
- direction (near-end / far-end): These terms become highly ambiguous in
  branching topologies (like CPU port on DSA switches).

mixed loopbacks across complex interconnects, userspace will eventually need a
Directed Acyclic Graph (DAG) model.

By adopting a DAG topology now, we can reduce the load on the initial
implementation and bypass much of the ongoing naming discussions, as components
are identified by their topological relations rather than arbitrary string
labels.

Can we design the netlink attributes now to ensure we are not blocked from
adding the following fields later:

- node_id: Global system ID. This also allows us to attach more diagnostic
  points (e.g., hardware counters) to exact subcomponents.

- parent_node_id: Upstream pointer for tree reconstruction.

- action: Bitmask of hardware modes (e.g., LOOPBACK, GENERATE) to allow
  simultaneous operations on a single node. See 6.3.1.3.1 Loopback Modes in:
  https://www.ti.com/lit/ds/symlink/dp83tg720s-q1.pdf?ts=1773225830126

- supported_actions: Bitmask of capabilities (e.g., can this node do LOOPBACK
  and GENERATE simultaneously?).

- direction: Towards parent / from parent.

- operational_constraints: MTU limits (e.g., FEC corrupts loopbacks >1522
  bytes), clock injection requirements (e.g., stmmac requires external Rx
  clocks), and required interface modes (e.g. loopback on FEC works only in
  MII mode)

If we hardcode a flat list assumption into the framework now, it will break
when we try to automate tests across datapath forks (e.g., SoC -> DSA Switch ->
PHYs) or handle complex industrial PHYs... :)

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Andrew Lunn]

> If we take an example with a 10G PHY, we may have :
> 
> +----SoC-----+
> |            |
> |  MAC       |- drivers/net/ethernet
> |   |        |
> | Base-R PCS |- could be in drivers/net/pcs, or directly
> |   |        | in the MAC driver
> |   |        |
> |  SerDes    |- May be in drivers/phy, maybe handled by firmware,
> |   |        |  maybe by the MAC driver, maybe by the PCS driver ?
> +---|--------+
>     |
>     | 10GBase-R
>     |
> +---|-PHY+
> |   |    |
> | SerDes | \
> |   |    | |
> |  PCS   | |
> |   |    |  > All of that handled by the drivers/net/phy PHY driver
> |  PMA   | |
> |   |    | |
> |  PMD   | /
> +---|----+
>     |
>     v 10GBaseT

We should also keep in mind this is a "simple" PHY. If you have a PHY
which does rate adaptation it looks more like:

+---|-PHY+
|   |    |
| SerDes |
|   |    |
|  PCS   |
|   |    |
|  MAC   |
|   |    |
| packet |
| buffer |
|   |    |
|  MAC   |
|   |    |
|  PCS   |
|   |    |
|  PMA   |
|   |    |
|  PMD   |
+---|----+
    |
    v 10GBaseT

So there is potentially 5 more loopback points?

Jakub proposal had the concept of 'depth'. Maybe we need that to
handle having the same block repeated a few times as you go towards
the media?

We should also think about when we have a PHY acting as a MII
converter. You see the Marvell PHY placed between the MAC and the SFP
cage. That has a collection of blocks which can do loopback. And then
we could have either a Base-T module/PHY in the cage, with more of the
same blocks, or a fibre modules with loopback.

     Andrew

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Andrew Lunn]

> I like this. The nice thing is that since "name" is a string, we're not
> locked into an enum -- drivers report what they have using 802.3
> vocabulary, and we document the recommended names (pcs, pma, pmd, mii)
> with references? That way it's unambiguous, but not too constrained.

It is both good and bad. I expect some vendors will just ignore the
text and use what their data sheet says, because they don't know
better. An enum forces more consistency.

https://gist.github.com/mjball/9cd028ac793ae8b351df1379f1e721f9

enum gets you around level 9. string around level 3.

	Andrew

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Maxime Chevallier]

On 11/03/2026 16:22, Andrew Lunn wrote:
>> If we take an example with a 10G PHY, we may have :
>>
>> +----SoC-----+
>> |            |
>> |  MAC       |- drivers/net/ethernet
>> |   |        |
>> | Base-R PCS |- could be in drivers/net/pcs, or directly
>> |   |        | in the MAC driver
>> |   |        |
>> |  SerDes    |- May be in drivers/phy, maybe handled by firmware,
>> |   |        |  maybe by the MAC driver, maybe by the PCS driver ?
>> +---|--------+
>>     |
>>     | 10GBase-R
>>     |
>> +---|-PHY+
>> |   |    |
>> | SerDes | \
>> |   |    | |
>> |  PCS   | |
>> |   |    |  > All of that handled by the drivers/net/phy PHY driver
>> |  PMA   | |
>> |   |    | |
>> |  PMD   | /
>> +---|----+
>>     |
>>     v 10GBaseT
> 
> We should also keep in mind this is a "simple" PHY. If you have a PHY
> which does rate adaptation it looks more like:
> 
> +---|-PHY+
> |   |    |
> | SerDes |
> |   |    |
> |  PCS   |
> |   |    |
> |  MAC   |
> |   |    |
> | packet |
> | buffer |
> |   |    |
> |  MAC   |
> |   |    |
> |  PCS   |
> |   |    |
> |  PMA   |
> |   |    |
> |  PMD   |
> +---|----+
>     |
>     v 10GBaseT
> 
> So there is potentially 5 more loopback points?

Good point indeed

> 
> Jakub proposal had the concept of 'depth'. Maybe we need that to
> handle having the same block repeated a few times as you go towards
> the media?

So, the same name + depth ?

 +---|-PHY+
 |   |    |
 | SerDes |
 |   |    |
 |  PCS   | component = PHY, name = "pcs", depth = 0
 |   |    |
 |  MAC   |
 |   |    |
 | packet |
 | buffer |
 |   |    |
 |  MAC   |
 |   |    |
 |  PCS   | component = PHY, name = "pcs", depth = 1
 |   |    |
 |  PMA   |
 |   |    |
 |  PMD   |
 +---|----+
     |
     v 10GBaseT

I think I like this idea of depth + name, as we can consider omitting
the depth information when it's not needed (e.g. simple PHY with 1 PCS),
to keep the API simple.

To continue with your example, with combo-port PHYs we may get multiple
PMA/PMD instances, one per port, that's even more loopback points.

We could potentially associate these with phy_port though ?

> We should also think about when we have a PHY acting as a MII
> converter. You see the Marvell PHY placed between the MAC and the SFP
> cage. That has a collection of blocks which can do loopback. And then
> we could have either a Base-T module/PHY in the cage, with more of the
> same blocks, or a fibre modules with loopback.

For that we have what we need with phy_link_topology, as each PHY has
its index, we should be good to go in that regard hopefully :)

Maxime

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Maxime Chevallier]

On 11/03/2026 16:30, Andrew Lunn wrote:
>> I like this. The nice thing is that since "name" is a string, we're not
>> locked into an enum -- drivers report what they have using 802.3
>> vocabulary, and we document the recommended names (pcs, pma, pmd, mii)
>> with references? That way it's unambiguous, but not too constrained.
> 
> It is both good and bad. I expect some vendors will just ignore the
> text and use what their data sheet says, because they don't know
> better. An enum forces more consistency.
> 
> https://gist.github.com/mjball/9cd028ac793ae8b351df1379f1e721f9
> 
> enum gets you around level 9. string around level 3.
> 
> 	Andrew

Oh didn't know that manifesto.

Givent the current discussions I'm indeed starting to think an enum
value will be enough to cover a good portion of the usecases, so I'm
OK with it :)

Maxime

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Andrew Lunn]

> So, the same name + depth ?
> 
>  +---|-PHY+
>  |   |    |
>  | SerDes |
>  |   |    |
>  |  PCS   | component = PHY, name = "pcs", depth = 0
>  |   |    |
>  |  MAC   |
>  |   |    |
>  | packet |
>  | buffer |
>  |   |    |
>  |  MAC   |
>  |   |    |
>  |  PCS   | component = PHY, name = "pcs", depth = 1
>  |   |    |
>  |  PMA   |
>  |   |    |
>  |  PMD   |
>  +---|----+
>      |
>      v 10GBaseT
> 
> For that we have what we need with phy_link_topology, as each PHY has
> its index, we should be good to go in that regard hopefully :)

So depth would be local to a component? We could have two PHY
components, each with a different index, and depth = 0?

I _think_ Jakub's depth was more at a global level? But then it would
need to be passed down as we do the enumeration.

     Andrew

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Andrew Lunn]

> > https://gist.github.com/mjball/9cd028ac793ae8b351df1379f1e721f9
> > 
> > enum gets you around level 9. string around level 3.
> > 
> > 	Andrew
> 
> Oh didn't know that manifesto.

Rusty has not been involved in kernel work for around 10 years. But it
is much older than that, a keynote from Ottawa Linux Symposium 2003.

https://ozlabs.org/~rusty/ols-2003-keynote/img0.html

      Andrew

Re: [PATCH net-next 01/11] ethtool: Add dump_one_dev callback for per-device sub-iteration

[Jakub Kicinski]

On Tue, 10 Mar 2026 11:47:31 +0100 Björn Töpel wrote:
> The per-PHY specific dump functions share a lot functionality of with
> the default dumpit infrastructure, but does not share the actual code.
> By introducing a new sub-iterator function, the two dumpit variants
> can be folded into one set of functions.
> 
> Add a new dump_one_dev callback in ethnl_request_ops. When
> ops->dump_one_dev is set, ethnl_default_start() saves the target
> device's ifindex for filtered dumps, and ethnl_default_dumpit()
> delegates per-device iteration to the callback instead of calling
> ethnl_default_dump_one() directly. No separate start/dumpit/done
> functions are needed.
> 
> For the existing per-PHY commands (PSE, PLCA, PHY, MSE), the shared
> ethnl_perphy_dump_one_dev helper provides the xa_for_each_start loop
> over the device's PHY topology.
> 
> This prepares the ethtool infrastructure for other commands that need
> similar per-device sub-iteration.

Feels like this could be split into two patches for ease of review.

Warning: net/ethtool/netlink.h:441 struct member 'dump_one_dev' not described in 'ethnl_request_ops'
Warning: net/ethtool/netlink.h:441 struct member 'dump_one_dev' not described in 'ethnl_request_ops'

> @@ -616,17 +580,41 @@ static int ethnl_default_dumpit(struct sk_buff *skb,
>  				struct netlink_callback *cb)
>  {
>  	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
> +	const struct genl_info *info = genl_info_dump(cb);
>  	struct net *net = sock_net(skb->sk);
>  	netdevice_tracker dev_tracker;
>  	struct net_device *dev;
>  	int ret = 0;
>  
> +	if (ctx->ops->dump_one_dev && ctx->ifindex) {
> +		dev = netdev_get_by_index(net, ctx->ifindex, &dev_tracker,
> +					  GFP_KERNEL);
> +		if (!dev)
> +			return -ENODEV;
> +
> +		ctx->req_info->dev = dev;
> +		ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub, info);
> +
> +		if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
> +			ret = skb->len;
> +
> +		netdev_put(dev, &dev_tracker);
> +		return ret;
> +	}
> +
>  	rcu_read_lock();
>  	for_each_netdev_dump(net, dev, ctx->pos_ifindex) {
>  		netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
>  		rcu_read_unlock();
>  
> -		ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb));
> +		if (ctx->ops->dump_one_dev) {
> +			ctx->req_info->dev = dev;
> +			ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub,
> +						     info);
> +			ctx->req_info->dev = NULL;
> +		} else {
> +			ret = ethnl_default_dump_one(skb, dev, ctx, info);
> +		}
>  
>  		rcu_read_lock();
>  		netdev_put(dev, &dev_tracker);

Not sure if it works here but another way to implementing single dump
and a loop dump concisely is to init both ifindex and pos_ifindex when
request is parsed and then add

	if (ctx->ifindex && ctx->ifindex != ctx->pos_ifindex)
		break;

at the start of the loop. That way the body of the loop doesn't have to
be repeated in a separate if 

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Jakub Kicinski]

On Wed, 11 Mar 2026 16:30:03 +0100 Andrew Lunn wrote:
> > I like this. The nice thing is that since "name" is a string, we're not
> > locked into an enum -- drivers report what they have using 802.3
> > vocabulary, and we document the recommended names (pcs, pma, pmd, mii)
> > with references? That way it's unambiguous, but not too constrained.  
> 
> It is both good and bad. I expect some vendors will just ignore the
> text and use what their data sheet says, because they don't know
> better. An enum forces more consistency.

enum only enforces using a value from a fixed set. The uAPI itself
cannot enforce functional equivalence. Worst case this may result
in different vendors interpreting the enum differently. With a string
there's a better chance that even if not matching between the vendors
at least a user with a databook in hand will know exactly what the
Linux API will do.

Could someone please explain to me what the use case for
standardization of the enum values would be? I push hard for
standardization of stats, because with standard stats its easy to build
high level standard tools. I don't understand what value standardization
across vendors what the last step of the MAC is called brings us.
Please explain, and if we have a real use case in mind it should be
possible to write a selftest that verifies that use case is met by any
given device.

Which reminds me -- I was suggesting that we add an order / id to the
stages, not just name. Because AFAIU being able to request the loopback
"very last loopback point of MAC" or "first loopback point of PHY" is
something that should be doable without user having to parse the names
/ enums and understanding them.

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Jakub Kicinski]

On Wed, 11 Mar 2026 11:50:47 +0100 Oleksij Rempel wrote:
> Looking at the current patch:
> - component (MAC, PCS, PHY, MODULE)
> - name (subsystem label)
> - id (local instance selector)
> - direction (near-end / far-end): These terms become highly ambiguous in
>   branching topologies (like CPU port on DSA switches).
> 
> mixed loopbacks across complex interconnects, userspace will eventually need a
> Directed Acyclic Graph (DAG) model.
> 
> By adopting a DAG topology now, we can reduce the load on the initial
> implementation and bypass much of the ongoing naming discussions, as components
> are identified by their topological relations rather than arbitrary string
> labels.

Not sure we need parentage chain or just "stage id" within each
component but FWIW if I interpret what you wrote right - I think 
I agree :) What matters is the topology not the naming of things.

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Jakub Kicinski]

On Wed, 11 Mar 2026 20:26:39 +0100 Andrew Lunn wrote:
> > For that we have what we need with phy_link_topology, as each PHY has
> > its index, we should be good to go in that regard hopefully :)  
> 
> So depth would be local to a component? We could have two PHY
> components, each with a different index, and depth = 0?
> 
> I _think_ Jakub's depth was more at a global level? But then it would
> need to be passed down as we do the enumeration.

Oh, sorry, I responded without reading the whole discussion :)
No, I imagined the depth would be within a single component, 
so under control of a single driver (instance). The ordering
between components should be defined by PHY topology etc so
it's outside of the loopback config.

Re: [net-next,03/11] ethtool: Add loopback GET/SET netlink implementation

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ethtool: Add loopback GET/SET netlink implementation

This commit adds kernel-side handlers for ETHTOOL_MSG_LOOPBACK_GET,
ETHTOOL_MSG_LOOPBACK_SET, and ETHTOOL_MSG_LOOPBACK_NTF messages.

> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 83c375840835..c9beca11fc40 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -846,6 +846,22 @@ void ethtool_mmsv_set_mm(struct ethtool_mmsv *mmsv, struct ethtool_mm_cfg *cfg);
>  void ethtool_mmsv_init(struct ethtool_mmsv *mmsv, struct net_device *dev,
>  		       const struct ethtool_mmsv_ops *ops);
>
> +/**
> + * struct ethtool_loopback_entry - Per-component loopback configuration
> + * @id: Optional component instance identifier, 0 means not specified
> + * @supported: Bitmask of supported directions
> + * @component: Loopback component
> + * @direction: Current loopback direction, 0 means disabled
> + * @name: Subsystem-specific name for the loopback point
> + */
> +struct ethtool_loopback_entry {
> +	enum ethtool_loopback_component component;
> +	u32 id;
> +	u32 supported;
> +	u32 direction;
> +	char name[ETH_GSTRING_LEN];
> +};

The kernel-doc field tags don't match the struct field order. The
documentation lists @id, @supported, @component, @direction, @name, but
the struct declares them as component, id, supported, direction, name.
Kernel-doc requires @field tags to follow the actual declaration order,
and scripts/kernel-doc will emit warnings for this mismatch.

[ ... ]

Re: [net-next,10/11] netdevsim: Add module EEPROM simulation via debugfs

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
netdevsim: Add module EEPROM simulation via debugfs

This patch adds get/set_module_eeprom_by_page ethtool ops to netdevsim
for testing kernel features that depend on module EEPROM access. The
EEPROM is backed by a 256-page x 128-byte array exposed via debugfs.

> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
> index 84bc025885f7..7ef96a747643 100644
> --- a/drivers/net/netdevsim/ethtool.c
> +++ b/drivers/net/netdevsim/ethtool.c
> @@ -247,6 +247,68 @@ static int nsim_set_loopback(struct net_device *dev,
>  	return 1;
>  }
>
> +static u8 *nsim_module_eeprom_ptr(struct netdevsim *ns,
> +				  const struct ethtool_module_eeprom *page_data,
> +				  u32 *len)
> +{
> +	u32 offset;
> +	u8 page;
> +
> +	if (page_data->offset < NSIM_MODULE_EEPROM_PAGE_LEN) {
> +		page = 0;
> +		offset = page_data->offset;
> +	} else {
> +		page = page_data->page;
                     ^^^^

Does this need bounds checking? The page_data->page value comes from userspace
via ethtool and is used directly as an array index at the return statement
below. The module.pages array is defined as:

    u8 pages[NSIM_MODULE_EEPROM_PAGES][NSIM_MODULE_EEPROM_PAGE_LEN]

where NSIM_MODULE_EEPROM_PAGES is 256. A userspace caller could provide a page
number >= 256, leading to an out-of-bounds access:

    return ns->ethtool.module.pages[page] + offset;

This can result in reading or writing (via nsim_set_module_eeprom_by_page)
adjacent kernel memory.

> +		offset = page_data->offset - NSIM_MODULE_EEPROM_PAGE_LEN;
> +	}
> +
> +	*len = min_t(u32, page_data->length,
> +		     NSIM_MODULE_EEPROM_PAGE_LEN - offset);
                                                  ^^^^^^

Can offset exceed NSIM_MODULE_EEPROM_PAGE_LEN here? When page_data->offset >=
128, the code calculates:

    offset = page_data->offset - NSIM_MODULE_EEPROM_PAGE_LEN

If page_data->offset is large (e.g., 300), the resulting offset would be 172.
At the min_t calculation:

    NSIM_MODULE_EEPROM_PAGE_LEN - offset = 128 - 172 = -44

This underflows when cast to u32, becoming a large unsigned value. The return
statement then accesses:

    ns->ethtool.module.pages[page] + offset

where offset can be > 128, reading past the page boundary. Each page is only
NSIM_MODULE_EEPROM_PAGE_LEN (128) bytes, but offset isn't validated to be
within this range.

> +	return ns->ethtool.module.pages[page] + offset;
> +}
> +
> +static int
> +nsim_get_module_eeprom_by_page(struct net_device *dev,
> +			       const struct ethtool_module_eeprom *page_data,
> +			       struct netlink_ext_ack *extack)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +	u32 len;
> +	u8 *ptr;
> +
> +	if (ns->ethtool.module.get_err)
> +		return -ns->ethtool.module.get_err;
> +
> +	ptr = nsim_module_eeprom_ptr(ns, page_data, &len);
> +	if (!ptr)
> +		return -EINVAL;
> +
> +	memcpy(page_data->data, ptr, len);
> +
> +	return len;
> +}
> +
> +static int
> +nsim_set_module_eeprom_by_page(struct net_device *dev,
> +			       const struct ethtool_module_eeprom *page_data,
> +			       struct netlink_ext_ack *extack)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +	u32 len;
> +	u8 *ptr;
> +
> +	if (ns->ethtool.module.set_err)
> +		return -ns->ethtool.module.set_err;
> +
> +	ptr = nsim_module_eeprom_ptr(ns, page_data, &len);
> +	if (!ptr)
> +		return -EINVAL;
> +
> +	memcpy(ptr, page_data->data, len);
> +
> +	return 0;
> +}

[ ... ]

> diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
> index 2e322b9410d2..f6b2063d41c9 100644
> --- a/drivers/net/netdevsim/netdevsim.h
> +++ b/drivers/net/netdevsim/netdevsim.h
> @@ -82,6 +82,16 @@ struct nsim_ethtool_pauseparam {
>  	bool report_stats_tx;
>  };
>
> +#define NSIM_MODULE_EEPROM_PAGES	256
> +#define NSIM_MODULE_EEPROM_PAGE_LEN	128
> +
> +struct nsim_ethtool_module {
> +	u32 get_err;
> +	u32 set_err;
> +	u8 pages[NSIM_MODULE_EEPROM_PAGES][NSIM_MODULE_EEPROM_PAGE_LEN];
> +	struct debugfs_blob_wrapper page_blobs[NSIM_MODULE_EEPROM_PAGES];
> +};

[ ... ]

Re: [PATCH net-next 00/11] ethtool: Generic loopback support

[Jakub Kicinski]

On Tue, 10 Mar 2026 11:47:30 +0100 Björn Töpel wrote:
>  .../selftests/drivers/net/hw/loopback_drv.py  | 226 ++++++++++
>  .../selftests/drivers/net/hw/loopback_nsim.py | 340 +++++++++++++++

pls run
ruff check
and
pylint --disable=R
on Python stuff. Or 
https://github.com/linux-netdev/nipa?tab=readme-ov-file#running-locally

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Oleksij Rempel]

On Wed, Mar 11, 2026 at 07:50:52PM -0700, Jakub Kicinski wrote:
> On Wed, 11 Mar 2026 20:26:39 +0100 Andrew Lunn wrote:
> > > For that we have what we need with phy_link_topology, as each PHY has
> > > its index, we should be good to go in that regard hopefully :)  
> > 
> > So depth would be local to a component? We could have two PHY
> > components, each with a different index, and depth = 0?
> > 
> > I _think_ Jakub's depth was more at a global level? But then it would
> > need to be passed down as we do the enumeration.
> 
> Oh, sorry, I responded without reading the whole discussion :)
> No, I imagined the depth would be within a single component, 
> so under control of a single driver (instance). The ordering
> between components should be defined by PHY topology etc so
> it's outside of the loopback config.

As for me, it is problematic to help the user to understand the datapath
depth on a switch. For example:

CPU -- xMII --- MAC1 [loop] --- fabric --- MAC2 [loop] --- xMII -- PHY
                                    \----- MACx [loop] ---

... each port has two xMII loop configurations: towards the xMII or towards
the fabric. From a driver perspective, a loop towards the xMII is
"remote." However, from a system perspective, a "remote" loop on MAC1 is
a local loop at depth=0, whereas a "local" loop on MAC2 is a local loop
at depth=1.

Other example would be where we have a chain of components which are
attached on the system in a unexpected direction, where the MDI
interface is pointing towards the main CPU, so the remote loopbacks
became to local loop.

One more issue is the test data generator location. The data generator
is not always the CPU. We have HW generators located in components like
PHYs or we may use external source (remote loopback).

This is why i would prefer to have description of topology by not
hard coding the perspective of view.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Maxime Chevallier]

On 12/03/2026 06:04, Oleksij Rempel wrote:
> On Wed, Mar 11, 2026 at 07:50:52PM -0700, Jakub Kicinski wrote:
>> On Wed, 11 Mar 2026 20:26:39 +0100 Andrew Lunn wrote:
>>>> For that we have what we need with phy_link_topology, as each PHY has
>>>> its index, we should be good to go in that regard hopefully :)  
>>>
>>> So depth would be local to a component? We could have two PHY
>>> components, each with a different index, and depth = 0?
>>>
>>> I _think_ Jakub's depth was more at a global level? But then it would
>>> need to be passed down as we do the enumeration.
>>
>> Oh, sorry, I responded without reading the whole discussion :)
>> No, I imagined the depth would be within a single component, 
>> so under control of a single driver (instance). The ordering
>> between components should be defined by PHY topology etc so
>> it's outside of the loopback config.
> 
> As for me, it is problematic to help the user to understand the datapath
> depth on a switch. For example:
> 
> CPU -- xMII --- MAC1 [loop] --- fabric --- MAC2 [loop] --- xMII -- PHY
>                                     \----- MACx [loop] ---
> 
> ... each port has two xMII loop configurations: towards the xMII or towards
> the fabric. From a driver perspective, a loop towards the xMII is
> "remote." However, from a system perspective, a "remote" loop on MAC1 is
> a local loop at depth=0, whereas a "local" loop on MAC2 is a local loop
> at depth=1.

What's important is to specify clearly in the documentation from which
end do we start, where representing the topology. From your scenario
here, each block is already well represented and exposed, and if we use
local depth definitions we should be fine ?

> 
> Other example would be where we have a chain of components which are
> attached on the system in a unexpected direction, where the MDI
> interface is pointing towards the main CPU, so the remote loopbacks
> became to local loop.

I have a few of these types of setup on my desk, where 3 PHY devices are
daisy-chained, we don't support that for now. If we one day add support
for standalone PHYs acting as media converters, I expect we'll be able
to tell which end is pointing where, and let it up to the user to figure
out what "remote" and "local" means in that case.

> 
> One more issue is the test data generator location. The data generator
> is not always the CPU. We have HW generators located in components like
> PHYs or we may use external source (remote loopback).

There were discussions about PRBS, I think the same idea of "pinpointing
which block we want to use" can be applied for both loopback and
generation ?

Maxime

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Oleksij Rempel]

On Thu, Mar 12, 2026 at 08:49:39AM +0100, Maxime Chevallier wrote:
> 
> 
> On 12/03/2026 06:04, Oleksij Rempel wrote:
> > On Wed, Mar 11, 2026 at 07:50:52PM -0700, Jakub Kicinski wrote:
> >> On Wed, 11 Mar 2026 20:26:39 +0100 Andrew Lunn wrote:
> >>>> For that we have what we need with phy_link_topology, as each PHY has
> >>>> its index, we should be good to go in that regard hopefully :)  
> >>>
> >>> So depth would be local to a component? We could have two PHY
> >>> components, each with a different index, and depth = 0?
> >>>
> >>> I _think_ Jakub's depth was more at a global level? But then it would
> >>> need to be passed down as we do the enumeration.
> >>
> >> Oh, sorry, I responded without reading the whole discussion :)
> >> No, I imagined the depth would be within a single component, 
> >> so under control of a single driver (instance). The ordering
> >> between components should be defined by PHY topology etc so
> >> it's outside of the loopback config.
> > 
> > As for me, it is problematic to help the user to understand the datapath
> > depth on a switch. For example:
> > 
> > CPU -- xMII --- MAC1 [loop] --- fabric --- MAC2 [loop] --- xMII -- PHY
> >                                     \----- MACx [loop] ---
> > 
> > ... each port has two xMII loop configurations: towards the xMII or towards
> > the fabric. From a driver perspective, a loop towards the xMII is
> > "remote." However, from a system perspective, a "remote" loop on MAC1 is
> > a local loop at depth=0, whereas a "local" loop on MAC2 is a local loop
> > at depth=1.
> 
> What's important is to specify clearly in the documentation from which
> end do we start, where representing the topology. From your scenario
> here, each block is already well represented and exposed, and if we use
> local depth definitions we should be fine ?

I guess my main problem is to imagine depth representation in two
separate directions for the user. So, the kernel documentation should
describe what is the starting point of view depending on the device
type. For example:
- PHY has typically xMII and MDI end points, so the loop towards the xMII
  is the local loop and towards the MDI is the remote loop.
- a switch/bridge has mutiple, application specific end points. So, we
  have a starting point of view from the fabric. Every loop pointing from
  the fabric towards the outside world of the switch is the remote loop,
  independent on connection type (xMII or MDI).

Correct?

> > Other example would be where we have a chain of components which are
> > attached on the system in a unexpected direction, where the MDI
> > interface is pointing towards the main CPU, so the remote loopbacks
> > became to local loop.
> 
> I have a few of these types of setup on my desk, where 3 PHY devices are
> daisy-chained, we don't support that for now. If we one day add support
> for standalone PHYs acting as media converters, I expect we'll be able
> to tell which end is pointing where, and let it up to the user to figure
> out what "remote" and "local" means in that case.
> 
> > 
> > One more issue is the test data generator location. The data generator
> > is not always the CPU. We have HW generators located in components like
> > PHYs or we may use external source (remote loopback).
> 
> There were discussions about PRBS, I think the same idea of "pinpointing
> which block we want to use" can be applied for both loopback and
> generation ?

Yes, the same apply for the counters. If we represent the data path as
pipe with different components like loopbacks, PRBS, etc on different
stages of the pipe, the same we have with counters. For example
industrial or automotive PHYs have separate counters for xMII and MDI.
A low depth loopback would not triggers some of counters.

Since I do not wont push all of this right now, i suggest to use more
abstract topology representation to make it easily extendable. 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Andrew Lunn]

> As for me, it is problematic to help the user to understand the datapath
> depth on a switch. For example:

Do you mean Ethernet switch? Or MII switch.

> 
> CPU -- xMII --- MAC1 [loop] --- fabric --- MAC2 [loop] --- xMII -- PHY
>                                     \----- MACx [loop] ---

In DSA, MAC1 is the CPU port of the switch. It is not represented by a
netif. Since there is no netif, you cannot use ethtool on it. So it is
impossible to apply loopback here.

This is one of the oddities of DSA. The CPU port and the conduit
interface on the host are just plumbing to make the setup work. In
terms of networking, they are not important. But sometimes you need to
get into the plumbing to find out why it is blocked up, statistics are
useful, and maybe loopback as well. We have discussed it a few times
that MAC1 should have a netif, but the conclusion is that developers
have a hard enough time with the conduit interface, adding yet another
oddball interface with no real purpose other than diagnostics is gone
to make the confusion even worse.

So i don't think depth is relevant here.

> ... each port has two xMII loop configurations: towards the xMII or towards
> the fabric. From a driver perspective, a loop towards the xMII is
> "remote." However, from a system perspective, a "remote" loop on MAC1 is
> a local loop at depth=0, whereas a "local" loop on MAC2 is a local loop
> at depth=1.

If you think about DSA and the Linux representation, the switch fabric
is not seen at all. All you have are user ports, those going to the
outside world. They act the same as interfaces directly connected to
the SoC. So "remote" and "local" must have the same meaning as an
interface directly connected to the host. And this is true for
switchdev in general, DSA is not special in any way.

> One more issue is the test data generator location. The data generator
> is not always the CPU. We have HW generators located in components like
> PHYs or we may use external source (remote loopback).

At the moment, we don't have a Linux model for such generators. There
is interest in them, but nobody has actually stepped up and proposed
anything. I do see there is an intersect, we need to be able to
represent them in the topology, and know which way they are pointing,
but i don't think they have a direct influence on loopback.

	Andrew

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Andrew Lunn]

> Which reminds me -- I was suggesting that we add an order / id to the
> stages, not just name. Because AFAIU being able to request the loopback
> "very last loopback point of MAC" or "first loopback point of PHY"

How do you define where the MAC ends?

I suspect some vendors will include the PCS and the PMA, because the
MAC ends at the MII pins on their SoC. Other vendors are going to say
the MAC ends at the interface to the PCS, especially those who have
licensed the PCS, and are using the shared Linux driver for the
PCS. And the PMA might again be a shared implementation, since it is
also used for USB, PCIe and SATA.

If Linux is driving the hardware, using phylink, phylib, PCS drivers
and generic PHY, we are very likely to have a uniform definition of
all these parts. Are we happy firmware devices will have a much
fuzzier, different interpretation, conglomerating it all together?

	Andrew

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Oleksij Rempel]

On Thu, Mar 12, 2026 at 02:34:40PM +0100, Andrew Lunn wrote:
> > As for me, it is problematic to help the user to understand the datapath
> > depth on a switch. For example:
> 
> Do you mean Ethernet switch? Or MII switch.
> 
> > 
> > CPU -- xMII --- MAC1 [loop] --- fabric --- MAC2 [loop] --- xMII -- PHY
> >                                     \----- MACx [loop] ---
> 
> In DSA, MAC1 is the CPU port of the switch. It is not represented by a
> netif. Since there is no netif, you cannot use ethtool on it. So it is
> impossible to apply loopback here.
> 
> This is one of the oddities of DSA. The CPU port and the conduit
> interface on the host are just plumbing to make the setup work. In
> terms of networking, they are not important. But sometimes you need to
> get into the plumbing to find out why it is blocked up, statistics are
> useful, and maybe loopback as well. We have discussed it a few times
> that MAC1 should have a netif, but the conclusion is that developers
> have a hard enough time with the conduit interface, adding yet another
> oddball interface with no real purpose other than diagnostics is gone
> to make the confusion even worse.
> 
> So i don't think depth is relevant here.
 
I have some projects where we need to configure the egress queue from
the switch to the CPU MAC. Currently my idea is to represent them as
optional HW offloading helpers for the host MAC. For example, this
pipe is already represented on the linux side as one interface:
eth0 - [ SoC MAC0 --- xMII --- Switch MAC1 ]

MAC0 and MAC1 counters are merged to one output.

So, the egress queue configuration of the Switch MAC1 would be an
ingress queue configuration of the MAC0 interface.
The same about remote loopback on MAC1, are additional local loopbacks
of MAC0. If we do it with counters, why not with everything else? 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Maxime Chevallier]

Hi Andrew,

>> One more issue is the test data generator location. The data generator
>> is not always the CPU. We have HW generators located in components like
>> PHYs or we may use external source (remote loopback).
> 
> At the moment, we don't have a Linux model for such generators. There
> is interest in them, but nobody has actually stepped up and proposed
> anything. I do see there is an intersect, we need to be able to
> represent them in the topology, and know which way they are pointing,
> but i don't think they have a direct influence on loopback.

If I'm following Oleksij, the idea would be to have on one side the
ability to "dump" the link topology with a finer granularity so that we
can see all the different blocks (pcs, pma, pmd, etc.), how they are
chained together and who's driving them (MAC, PHY (+ phy_index), module,
etc.), and on another side commands to configure loopback on them, with
the ability to also configure traffic generators in the future, gather
stats, etc.

Another can of worms for sure, and probably too much for what Björn is
trying to achieve. It's hard to say if this is overkill or not, there's
interest in that for sure, but also quite a lot of work to do...

Maxime

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Jakub Kicinski]

On Thu, 12 Mar 2026 14:46:40 +0100 Andrew Lunn wrote:
> > Which reminds me -- I was suggesting that we add an order / id to the
> > stages, not just name. Because AFAIU being able to request the loopback
> > "very last loopback point of MAC" or "first loopback point of PHY"  
> 
> How do you define where the MAC ends?

MAC may not be the greatest of names because I'd include in it
everything past the PHY, up to the DMA blocks.

> I suspect some vendors will include the PCS and the PMA, because the
> MAC ends at the MII pins on their SoC. Other vendors are going to say
> the MAC ends at the interface to the PCS, especially those who have
> licensed the PCS, and are using the shared Linux driver for the
> PCS. And the PMA might again be a shared implementation, since it is
> also used for USB, PCIe and SATA.
> 
> If Linux is driving the hardware, using phylink, phylib, PCS drivers
> and generic PHY, we are very likely to have a uniform definition of
> all these parts. Are we happy firmware devices will have a much
> fuzzier, different interpretation, conglomerating it all together?

As long as the kernel API lets "integrated" devices expose both a MAC
and a PHY node I don't think we should let anyone conglomerate.

I see your point that the enum would work nicely for PHY stages.
But it will be limiting for MAC stages.
These conflicting preferences make having all of loopback config 
in one API tricky. I guess we could have a half-measure to add in
the kernel the "well known" PHY stage name, and WARN_ON_ONCE() if 
some driver exposes PHY stage in something that's not a PHY.
Or uses an unknown name for a PHY stage?

Re: [PATCH net-next 01/11] ethtool: Add dump_one_dev callback for per-device sub-iteration

[Björn Töpel]

Jakub Kicinski <kuba@kernel.org> writes:

> On Tue, 10 Mar 2026 11:47:31 +0100 Björn Töpel wrote:
>> The per-PHY specific dump functions share a lot functionality of with
>> the default dumpit infrastructure, but does not share the actual code.
>> By introducing a new sub-iterator function, the two dumpit variants
>> can be folded into one set of functions.
>> 
>> Add a new dump_one_dev callback in ethnl_request_ops. When
>> ops->dump_one_dev is set, ethnl_default_start() saves the target
>> device's ifindex for filtered dumps, and ethnl_default_dumpit()
>> delegates per-device iteration to the callback instead of calling
>> ethnl_default_dump_one() directly. No separate start/dumpit/done
>> functions are needed.
>> 
>> For the existing per-PHY commands (PSE, PLCA, PHY, MSE), the shared
>> ethnl_perphy_dump_one_dev helper provides the xa_for_each_start loop
>> over the device's PHY topology.
>> 
>> This prepares the ethtool infrastructure for other commands that need
>> similar per-device sub-iteration.
>
> Feels like this could be split into two patches for ease of review.

Hmm, OK! My take was the it was mostly moving stuff.

> Warning: net/ethtool/netlink.h:441 struct member 'dump_one_dev' not described in 'ethnl_request_ops'
> Warning: net/ethtool/netlink.h:441 struct member 'dump_one_dev' not described in 'ethnl_request_ops'

Thanks! (I've looked thru all the pw complaints, and fixed locally!)

>
>> @@ -616,17 +580,41 @@ static int ethnl_default_dumpit(struct sk_buff *skb,
>>  				struct netlink_callback *cb)
>>  {
>>  	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
>> +	const struct genl_info *info = genl_info_dump(cb);
>>  	struct net *net = sock_net(skb->sk);
>>  	netdevice_tracker dev_tracker;
>>  	struct net_device *dev;
>>  	int ret = 0;
>>  
>> +	if (ctx->ops->dump_one_dev && ctx->ifindex) {
>> +		dev = netdev_get_by_index(net, ctx->ifindex, &dev_tracker,
>> +					  GFP_KERNEL);
>> +		if (!dev)
>> +			return -ENODEV;
>> +
>> +		ctx->req_info->dev = dev;
>> +		ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub, info);
>> +
>> +		if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
>> +			ret = skb->len;
>> +
>> +		netdev_put(dev, &dev_tracker);
>> +		return ret;
>> +	}
>> +
>>  	rcu_read_lock();
>>  	for_each_netdev_dump(net, dev, ctx->pos_ifindex) {
>>  		netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
>>  		rcu_read_unlock();
>>  
>> -		ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb));
>> +		if (ctx->ops->dump_one_dev) {
>> +			ctx->req_info->dev = dev;
>> +			ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub,
>> +						     info);
>> +			ctx->req_info->dev = NULL;
>> +		} else {
>> +			ret = ethnl_default_dump_one(skb, dev, ctx, info);
>> +		}
>>  
>>  		rcu_read_lock();
>>  		netdev_put(dev, &dev_tracker);
>
> Not sure if it works here but another way to implementing single dump
> and a loop dump concisely is to init both ifindex and pos_ifindex when
> request is parsed and then add
>
> 	if (ctx->ifindex && ctx->ifindex != ctx->pos_ifindex)
> 		break;
>
> at the start of the loop. That way the body of the loop doesn't have to
> be repeated in a separate if 

Indeed! I think that would work, and that'd be cleaner!

Cheers!

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Björn Töpel]

Folks, thanks for the elaborate discussion (accidental complexity vs
essential complexity comes to mind...)!

Maxime Chevallier <maxime.chevallier@bootlin.com> writes:

> Hi Andrew,
>
>>> One more issue is the test data generator location. The data generator
>>> is not always the CPU. We have HW generators located in components like
>>> PHYs or we may use external source (remote loopback).
>> 
>> At the moment, we don't have a Linux model for such generators. There
>> is interest in them, but nobody has actually stepped up and proposed
>> anything. I do see there is an intersect, we need to be able to
>> represent them in the topology, and know which way they are pointing,
>> but i don't think they have a direct influence on loopback.
>
> If I'm following Oleksij, the idea would be to have on one side the
> ability to "dump" the link topology with a finer granularity so that we
> can see all the different blocks (pcs, pma, pmd, etc.), how they are
> chained together and who's driving them (MAC, PHY (+ phy_index), module,
> etc.), and on another side commands to configure loopback on them, with
> the ability to also configure traffic generators in the future, gather
> stats, etc.
>
> Another can of worms for sure, and probably too much for what Björn is
> trying to achieve. It's hard to say if this is overkill or not, there's
> interest in that for sure, but also quite a lot of work to do...

It's great to have these discussion as input to the first (minimal!)
series, so we can extend/build on it later.

If I try to make sense of the above discussions...

Rough agreement on:

 - Depth/ordering should be local to a component, not global across the
   whole path.
 - Cross-component ordering comes from existing infrastructure (PHY link
   topology, phy_index).
 - The current component set (MAC/PHY/MODULE) is reasonable for a first
   pass.
 - HW traffic generators and full topology dumps are interesting but out
   of scope for now (Please? ;-)).


So, maybe the next steps are:

 1. Keep the current component model (MAC/PHY/MODULE) and the
    NEAR_END/FAR_END direction (naming need to change as Maxime said).
 
 2. Add a depth (or order?) field to ETHTOOL_A_LOOPBACK_ENTRY as Jakub
    suggested, local to each component instance. This addresses the
    "multiple loopback points within one MAC" case without requiring a
    global ordering. I hope it addresses what Oleksij's switch example
    needs (multiple local loops at different depths within one
    component) *insert that screaming emoji*.
 
 3. Document the viewpoint convention clearly.
 
 4. Punt on the grand topology dump. Too much to chew.
 
 5. Don't worry about DSA CPU ports - they don't have a netif, so
    loopback doesn't apply there today. If someone adds netifs for CPU
    ports later, depth handles it.

TL;DR: Add depth, document the viewpoint convention, and ship
it^W^Winterate.

Did I get that right?


Enjoy the w/e!
Björn

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Oleksij Rempel]

Hi Björn,

On Fri, Mar 13, 2026 at 08:11:11PM +0100, Björn Töpel wrote:
> Folks, thanks for the elaborate discussion (accidental complexity vs
> essential complexity comes to mind...)!

Sorry for overthinking :)

> Maxime Chevallier <maxime.chevallier@bootlin.com> writes:
> 
> > Hi Andrew,
> >
> >>> One more issue is the test data generator location. The data generator
> >>> is not always the CPU. We have HW generators located in components like
> >>> PHYs or we may use external source (remote loopback).
> >> 
> >> At the moment, we don't have a Linux model for such generators. There
> >> is interest in them, but nobody has actually stepped up and proposed
> >> anything. I do see there is an intersect, we need to be able to
> >> represent them in the topology, and know which way they are pointing,
> >> but i don't think they have a direct influence on loopback.
> >
> > If I'm following Oleksij, the idea would be to have on one side the
> > ability to "dump" the link topology with a finer granularity so that we
> > can see all the different blocks (pcs, pma, pmd, etc.), how they are
> > chained together and who's driving them (MAC, PHY (+ phy_index), module,
> > etc.), and on another side commands to configure loopback on them, with
> > the ability to also configure traffic generators in the future, gather
> > stats, etc.
> >
> > Another can of worms for sure, and probably too much for what Björn is
> > trying to achieve. It's hard to say if this is overkill or not, there's
> > interest in that for sure, but also quite a lot of work to do...
> 
> It's great to have these discussion as input to the first (minimal!)
> series, so we can extend/build on it later.
> 
> If I try to make sense of the above discussions...
> 
> Rough agreement on:
> 
>  - Depth/ordering should be local to a component, not global across the
>    whole path.

ack

>  - Cross-component ordering comes from existing infrastructure (PHY link
>    topology, phy_index).

ack

>  - The current component set (MAC/PHY/MODULE) is reasonable for a first
>    pass.

I do not have strong opinion here. 

>  - HW traffic generators and full topology dumps are interesting but out
>    of scope for now (Please? ;-)).

It didn't tried to push it here. My point is - image me or may be you,
will need to implement it in the next step. This components will need to
cooperate and user will need to understand the relation and/or topology.

The diagnostic is all about topology.

> So, maybe the next steps are:
> 
>  1. Keep the current component model (MAC/PHY/MODULE) and the
>     NEAR_END/FAR_END direction (naming need to change as Maxime said).

Probably good to document that NEAR_END/FAR_END or local/remote is
related to the viewpoint convention. Otherwise it will get confusing
with components which mount in a unusual direction (embedded worlds is
full of it :))

>  2. Add a depth (or order?) field to ETHTOOL_A_LOOPBACK_ENTRY as Jakub
>     suggested, local to each component instance. This addresses the
>     "multiple loopback points within one MAC" case without requiring a
>     global ordering. I hope it addresses what Oleksij's switch example
>     needs (multiple local loops at different depths within one
>     component) *insert that screaming emoji*.

ack. I guess "depth" fits to the "viewpoint" terminology.

>  3. Document the viewpoint convention clearly.

ack

>  4. Punt on the grand topology dump. Too much to chew.

ack

>  5. Don't worry about DSA CPU ports - they don't have a netif, so
>     loopback doesn't apply there today. If someone adds netifs for CPU
>     ports later, depth handles it.

ack

> TL;DR: Add depth, document the viewpoint convention, and ship
> it^W^Winterate.
> 
> Did I get that right?

I'm ok with it, but maintainers will have the last word.

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Björn Töpel]

On Sun, 15 Mar 2026 at 16:10, Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> Hi Björn,
>
> On Fri, Mar 13, 2026 at 08:11:11PM +0100, Björn Töpel wrote:
> > Folks, thanks for the elaborate discussion (accidental complexity vs
> > essential complexity comes to mind...)!
>
> Sorry for overthinking :)

Haha, don't be! I think it's great that we have these discussions
upfront! If this is overthinking, please continue to do that! :-)

> > Maxime Chevallier <maxime.chevallier@bootlin.com> writes:
> >
> > > Hi Andrew,
> > >
> > >>> One more issue is the test data generator location. The data generator
> > >>> is not always the CPU. We have HW generators located in components like
> > >>> PHYs or we may use external source (remote loopback).
> > >>
> > >> At the moment, we don't have a Linux model for such generators. There
> > >> is interest in them, but nobody has actually stepped up and proposed
> > >> anything. I do see there is an intersect, we need to be able to
> > >> represent them in the topology, and know which way they are pointing,
> > >> but i don't think they have a direct influence on loopback.
> > >
> > > If I'm following Oleksij, the idea would be to have on one side the
> > > ability to "dump" the link topology with a finer granularity so that we
> > > can see all the different blocks (pcs, pma, pmd, etc.), how they are
> > > chained together and who's driving them (MAC, PHY (+ phy_index), module,
> > > etc.), and on another side commands to configure loopback on them, with
> > > the ability to also configure traffic generators in the future, gather
> > > stats, etc.
> > >
> > > Another can of worms for sure, and probably too much for what Björn is
> > > trying to achieve. It's hard to say if this is overkill or not, there's
> > > interest in that for sure, but also quite a lot of work to do...
> >
> > It's great to have these discussion as input to the first (minimal!)
> > series, so we can extend/build on it later.
> >
> > If I try to make sense of the above discussions...
> >
> > Rough agreement on:
> >
> >  - Depth/ordering should be local to a component, not global across the
> >    whole path.
>
> ack
>
> >  - Cross-component ordering comes from existing infrastructure (PHY link
> >    topology, phy_index).
>
> ack
>
> >  - The current component set (MAC/PHY/MODULE) is reasonable for a first
> >    pass.
>
> I do not have strong opinion here.
>
> >  - HW traffic generators and full topology dumps are interesting but out
> >    of scope for now (Please? ;-)).
>
> It didn't tried to push it here. My point is - image me or may be you,
> will need to implement it in the next step. This components will need to
> cooperate and user will need to understand the relation and/or topology.
>
> The diagnostic is all about topology.

I hear you, and I hope this didn't come across as negative. I
definitely think we need something that we all can continue to build
on. ...and if my summary/view isn't, please holler!

> > So, maybe the next steps are:
> >
> >  1. Keep the current component model (MAC/PHY/MODULE) and the
> >     NEAR_END/FAR_END direction (naming need to change as Maxime said).
>
> Probably good to document that NEAR_END/FAR_END or local/remote is
> related to the viewpoint convention. Otherwise it will get confusing
> with components which mount in a unusual direction (embedded worlds is
> full of it :))

ACK!

> >  2. Add a depth (or order?) field to ETHTOOL_A_LOOPBACK_ENTRY as Jakub
> >     suggested, local to each component instance. This addresses the
> >     "multiple loopback points within one MAC" case without requiring a
> >     global ordering. I hope it addresses what Oleksij's switch example
> >     needs (multiple local loops at different depths within one
> >     component) *insert that screaming emoji*.
>
> ack. I guess "depth" fits to the "viewpoint" terminology.
>
> >  3. Document the viewpoint convention clearly.
>
> ack
>
> >  4. Punt on the grand topology dump. Too much to chew.
>
> ack
>
> >  5. Don't worry about DSA CPU ports - they don't have a netif, so
> >     loopback doesn't apply there today. If someone adds netifs for CPU
> >     ports later, depth handles it.
>
> ack
>
> > TL;DR: Add depth, document the viewpoint convention, and ship
> > it^W^Winterate.
> >
> > Did I get that right?
>
> I'm ok with it, but maintainers will have the last word.

Agreed!
Björn

Re: [PATCH net-next 02/11] ethtool: Add loopback netlink UAPI definitions

[Andrew Lunn]

> So, maybe the next steps are:
> 
>  1. Keep the current component model (MAC/PHY/MODULE) and the
>     NEAR_END/FAR_END direction (naming need to change as Maxime said).
>  
>  2. Add a depth (or order?) field to ETHTOOL_A_LOOPBACK_ENTRY as Jakub
>     suggested, local to each component instance. This addresses the
>     "multiple loopback points within one MAC" case without requiring a
>     global ordering. I hope it addresses what Oleksij's switch example
>     needs (multiple local loops at different depths within one
>     component) *insert that screaming emoji*.
>  
>  3. Document the viewpoint convention clearly.
>  
>  4. Punt on the grand topology dump. Too much to chew.
>  
>  5. Don't worry about DSA CPU ports - they don't have a netif, so
>     loopback doesn't apply there today. If someone adds netifs for CPU
>     ports later, depth handles it.
> 
> TL;DR: Add depth, document the viewpoint convention, and ship
> it^W^Winterate.
> 
> Did I get that right?

Sounds reasonable. The first version can be KISS, we just need to keep
in mind reality is more complex and try to avoid adding any roadblocks
for making it more complex to reflect that reality.

    Andrew