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

[Naveen Mamindlapalli <naveenm@marvell.com>]

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