* [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing
@ 2026-09-27 13:36 Maxime Chevallier
2026-09-27 13:36 ` [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc Maxime Chevallier
` (10 more replies)
0 siblings, 11 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
Hello everyone,
Here's V18 for the phy_port improved SFP support and netlink interface.
V18 aggregates Andrew's reviews (thanks !) and Aleksei Tested-by tags
(thank you as well, this is nice). It also contains a few tweaks
following sashiko and Aleksei's report after testing, the ops-locked
check also checks for NULL netdev ops. I also added a missing rtnl()
around port deletion in phylink, and changed a bitfield to a bool in
phy.h following sashiko and Aleksei's reviews.
This work extends on the recent addition of phy_port representation to enable
listing the front-facing ports of an interface. For now, we don't control
these ports, we merely list their presence and their capabilities.
As the most common use-case of multi-port interfaces is combo-ports that
provide both RJ45 and SFP connectors on a single MAC, there's a lot of
SFP stuff in this series.
This series is in 2 main parts. The first one aims at representing the
SFP cages and modules using phy_port, as combo-ports with RJ45 + SFP are
by far the most common cases for multi-connector setups.
The second part is the netlink interface to list those ports, now that
most use-cases are covered.
Let's see what we can do with some examples of the new ethtool API :
- Get MII interfaces supported by an empty SFP cage :
# ethtool --show-ports eth3
Port for eth3:
Port id: 1
Supported MII interfaces : sgmii, 1000base-x, 2500base-x
Port type: sfp
- Get Combo-ports supported modes, on each port :
# ethtool --show-ports eth1
Port for eth1:
Port id: 1
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
10000baseT/Full
2500baseT/Full
5000baseT/Full
Port type: mdi
Port for eth1:
Port id: 2
Supported MII interfaces : 10gbase-r
Port type: sfp
- Get Achievable linkmodes on a SFP module (combo port with a DAC in the
SFP cage)
# ethtool --show-ports eth1
Port for eth1:
Port id: 1
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
10000baseT/Full
2500baseT/Full
5000baseT/Full
Port type: mdi
Port for eth1:
Port id: 2
Supported MII interfaces : 10gbase-r
Port type: sfp
Port for eth1:
Port id: 3
Upstream id: 2
Supported link modes: 10000baseCR/Full
Port type: mdi
Note that here, we have 3 ports :
- The Copper port
- The SFP Cage itself,
- The SFP module
This series builds on top of phy_port and phy_link_topology to allow
tracking the ports of an interface. We maintain a list of supported
linkmodes/interfaces on each port, which allows for fine-grained
reporting of each port's capability.
What this series doesn't do :
- We don't support selecting which port is active. This is the next step.
- We only support PHY-driven combo ports. The end-goal of this whole
journey that started with phy_link_topology is to get support for MII
muxes, such as the one we have on the Turris Omnia. This will eventually
be upstreamed as well.
If you want to play around with it, here's [1] the patched ethtool that I've
been using to produce the outputs above.
Thanks !
Maxime
[1] : https://github.com/minimaxwell/ethtool/tree/mc/ethtool_port
Changelog :
Changes in v18:
- Add Review/Tested tags
- add missing rtnl() in phylink when removing the cage port
- add a missing NULL check on netdev ops
- Added missing items in the netlink docs
Changes in v17:
https://lore.kernel.org/netdev/20260910170103.1029108-1-maxime.chevallier@bootlin.com/
- Handle port index wraparound
- Regen the ethtool headers, now with the comment :)
Changes in v16:
https://lore.kernel.org/r/20260902212135.1805948-1-maxime.chevallier@bootlin.com
- unregister phy_ports at phy_detach
- typos in patch 1 (of -> or)
- Fix the netlink specs
Changes in v15:
https://lore.kernel.org/netdev/20260801074332.98755-1-maxime.chevallier@bootlin.com/
- Fail on port access for ops-locked devices
- Use a direct phy_port pointer for upstream tracking
- Addressed the kdoc issues
Changes in v14:
https://lore.kernel.org/netdev/20260720174808.139573-1-maxime.chevallier@bootlin.com/
- Added a big comment on the first patch for the topo alloc potential
races
- Fixed the multi-buff DUMP on last patch with a separate iterator for
dumps
- Fixed some typos in the documentation
Changes in V13:
V13: https://lore.kernel.org/r/20260701110427.143945-1-maxime.chevallier@bootlin.com
- Rebase on net-next
- Fix the SFP bus cleanup path in patch 5
Changes in V12:
V12: https://lore.kernel.org/r/20260615153907.862987-1-maxime.chevallier@bootlin.com
- Rebased on net-next, including fixes on the phy probing and cleanup
paths
- Rebased on Jakub's netdev_ops_locked changes in phy_link_topology
- Fixed some typos reported by Andrew and sashiko in the documentation
Changes in v11:
V11:https://lore.kernel.org/r/20260521121040.1199622-1-maxime.chevallier@bootlin.com
- Aggregated Andrew's reviews :)
- Removed the "vacant" field, replaced it with "upstream_port"
Changes in V10:
V10: https://lore.kernel.org/r/20260513130521.1064094-1-maxime.chevallier@bootlin.com
- Rebase on net-next
- Rename phylink/phy_device sfp_bus_port to sfp_cage_port
- Sashiko's reviews were mostly unrealistic or wrong :(
Changes in V9:
V9: https://lore.kernel.org/r/20260403123755.175742-1-maxime.chevallier@bootlin.com
- Added missing netlink doc updates for u8->u32 conversion
- Removed dead code with a condition that can never be true in
phylink's mod_port code
- Fixed the error path in phy_sfp_connect_phy
Changes in v8:
V8: https://lore.kernel.org/r/20260325081937.571115-1-maxime.chevallier@bootlin.com
- Set the new phydev.has_sfp_mod_phy field when we're sure that no
errors occured
- Fix formatting of the copyright info in ethnl port
- Use a policy to validate the range of port_id
- Use GENL_REQ_ATTR_CHECK
- alpha-sort headers
- use u32 in netlink messages
- return better error codes
- don't check the skb len, the core does that
Changes in V7:
V7: https://lore.kernel.org/all/20260309152747.702373-1-maxime.chevallier@bootlin.com/
- Changed the port cleanup path to use list_for_each_entry_continue_reverse
- Adjusted the cleanup path in phylink for the port vacant state
- Pass the right cmd for the netlink dump message
Changes in V6:
V6: https://lore.kernel.org/r/20260304145444.442334-1-maxime.chevallier@bootlin.com
- Added some comments in th mod_port cleanup
- changed some kmalloc to kmalloc_obj
- Removed some phy_link_topo_del_port that wasn't needed
Changes in V5:
V5: https://lore.kernel.org/r/20260205092317.755906-1-maxime.chevallier@bootlin.com
- Fixed a check on a potentially un-initialized pointer, reported by
Simon
- Fixed a documentation formatting issue
- Remove a stray pr_info
- Rebased on net-next
Changes in V4:
V4 : https://lore.kernel.org/netdev/20260203172839.548524-1-maxime.chevallier@bootlin.com/
- Add a cleanup patch for the of port parsing
- Added a match to sync the port's linkmodes with the PHY's for OF
ports
- Added RTNL assert in the port_get topo helper
- nullify the bus port for phylink support
- Fix some typos
Changes in V3:
V3: https://lore.kernel.org/netdev/20260201151249.642015-1-maxime.chevallier@bootlin.com/
- Remove the sfp bus ops for nophy, and use .module_start() as
suggested by Russell
- Added missing cleanup for the topology, as per AI review
- Fixed a few typos as per Romain's review
- Changed "occupied" to "vacant" as per Romain's review
- Added missing checks for null ports, per AI review
Changes in V2:
V2: https://lore.kernel.org/netdev/20260128204526.170927-1-maxime.chevallier@bootlin.com/
- Fix the cleanup path of phy_link_topo_add_phy, as per AI review
- Fix the cleanup path of phy_sfp_probe, as per AI review
- Fix the call-site of the disconnect_nophy sfp bus ops, per AI review
- Fix the netdev-less case uin phylink, per AI review
- Fix the prototype of phy_link_topo_get_port for the stubs
- Dropped patch 11. It ended-up breaking 'allnoconfig', so instead we
built a phy_interface_names array in net/ethtool/netlink.c
- Fix an ethool-netlink spec discrepancy with the type of an attribute
- Fix the size computation in the netlink port API
- Fix the cleanup path in the netlink port API
V1: https://lore.kernel.org/netdev/20260127134202.8208-1-maxime.chevallier@bootlin.com/
Maxime Chevallier (10):
net: phy: phy_link_topology: Add a helper for opportunistic alloc
net: phy: phy_link_topology: Track ports in phy_link_topology
net: phylink: Register a phy_port for MAC-driven SFP cages
net: phy: Create SFP phy_port before registering upstream
net: phy: Represent PHY-less SFP modules with phy_port
net: phy: phy_port: Store information about a port's upstream
net: phy: phy_link_topology: Add a helper to retrieve ports
netlink: specs: Add ethernet port listing with ethtool
net: ethtool: Introduce ethtool command to list ports
Documentation: networking: Update the phy_port infrastructure
description
Documentation/netlink/specs/ethtool.yaml | 55 +++
Documentation/networking/ethtool-netlink.rst | 36 ++
Documentation/networking/phy-port.rst | 26 +-
MAINTAINERS | 1 +
drivers/net/phy/phy-caps.h | 2 +
drivers/net/phy/phy_caps.c | 26 ++
drivers/net/phy/phy_device.c | 186 +++++++--
drivers/net/phy/phy_link_topology.c | 101 ++++-
drivers/net/phy/phylink.c | 132 +++++-
include/linux/phy.h | 10 +
include/linux/phy_link_topology.h | 43 ++
include/linux/phy_port.h | 5 +
.../uapi/linux/ethtool_netlink_generated.h | 22 +
net/core/dev.c | 1 +
net/ethtool/Makefile | 2 +-
net/ethtool/netlink.c | 25 ++
net/ethtool/netlink.h | 8 +
net/ethtool/port.c | 382 ++++++++++++++++++
18 files changed, 1025 insertions(+), 38 deletions(-)
create mode 100644 net/ethtool/port.c
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:36 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology Maxime Chevallier
` (9 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
The phy_link_topology structure stores information about the PHY-related
components connected to a net_device. It is opportunistically allocated,
when we add the first item to the topology, as this is not relevant for
all kinds of net_devices.
In preparation for the addition of phy_port tracking in the topology,
let's make a dedicated helper for that allocation sequence.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/phy/phy_link_topology.c | 40 +++++++++++++++++++++++------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
index 4134de7ae313..0462283c8020 100644
--- a/drivers/net/phy/phy_link_topology.c
+++ b/drivers/net/phy/phy_link_topology.c
@@ -28,11 +28,39 @@ static int netdev_alloc_phy_link_topology(struct net_device *dev)
return 0;
}
+static struct phy_link_topology *phy_link_topo_get_or_alloc(struct net_device *dev)
+{
+ int ret;
+
+ if (dev->link_topo)
+ return dev->link_topo;
+
+ /* The topology is allocated the first time we add an object to it.
+ * It is freed alongside the netdev. It can be called on multiple
+ * contexts:
+ * - It can be called from .probe() : No rtnl, no netdev_lock
+ * - .ndo_open() : rtnl and possibly netdev_lock
+ * - SFP state machine : rtnl held or not
+ *
+ * However, we can't really have races :
+ * - If we have a PHY, phy_link_topo_add_phy() will always run first
+ * and trigger the alloc. Only then the ports can be added through
+ * phylib or sfp.
+ * - If we don't, the SFP port for the cage is registered first, and
+ * only then other ports/PHYs can be registered.
+ */
+ ret = netdev_alloc_phy_link_topology(dev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return dev->link_topo;
+}
+
int phy_link_topo_add_phy(struct net_device *dev,
struct phy_device *phy,
enum phy_upstream upt, void *upstream)
{
- struct phy_link_topology *topo = dev->link_topo;
+ struct phy_link_topology *topo;
struct phy_device_node *pdn;
int ret;
@@ -45,13 +73,9 @@ int phy_link_topo_add_phy(struct net_device *dev,
if (WARN_ON_ONCE(netdev_need_ops_lock(dev)))
return -EOPNOTSUPP;
- if (!topo) {
- ret = netdev_alloc_phy_link_topology(dev);
- if (ret)
- return ret;
-
- topo = dev->link_topo;
- }
+ topo = phy_link_topo_get_or_alloc(dev);
+ if (IS_ERR(topo))
+ return PTR_ERR(topo);
pdn = kzalloc_obj(*pdn);
if (!pdn)
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
2026-09-27 13:36 ` [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:37 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages Maxime Chevallier
` (8 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
phy_port is aimed at representing the various physical interfaces of a
net_device. They can be controlled by various components in the link,
such as the Ethernet PHY, the Ethernet MAC, and SFP module, etc.
Let's therefore make so we keep track of all the ports connected to a
netdev in phy_link_topology. The only ports added for now are phy-driven
ports.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/phy/phy_device.c | 3 ++
drivers/net/phy/phy_link_topology.c | 53 +++++++++++++++++++++++++++++
include/linux/phy_link_topology.h | 18 ++++++++++
include/linux/phy_port.h | 2 ++
net/core/dev.c | 1 +
5 files changed, 77 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5b13a74e2fa9..c4d2c03b8282 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1580,6 +1580,9 @@ static void phy_del_port(struct phy_device *phydev, struct phy_port *port)
list_del(&port->head);
+ if (phydev->attached_dev)
+ phy_link_topo_del_port(phydev->attached_dev, port);
+
phydev->n_ports--;
}
diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
index 0462283c8020..28c5bf3a8849 100644
--- a/drivers/net/phy/phy_link_topology.c
+++ b/drivers/net/phy/phy_link_topology.c
@@ -7,6 +7,7 @@
*/
#include <linux/phy_link_topology.h>
+#include <linux/phy_port.h>
#include <linux/phy.h>
#include <linux/rtnetlink.h>
#include <linux/xarray.h>
@@ -23,6 +24,9 @@ static int netdev_alloc_phy_link_topology(struct net_device *dev)
xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
topo->next_phy_index = 1;
+ xa_init_flags(&topo->ports, XA_FLAGS_ALLOC1);
+ topo->next_port_index = 1;
+
dev->link_topo = topo;
return 0;
@@ -56,12 +60,45 @@ static struct phy_link_topology *phy_link_topo_get_or_alloc(struct net_device *d
return dev->link_topo;
}
+int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port)
+{
+ struct phy_link_topology *topo;
+ int ret;
+
+ topo = phy_link_topo_get_or_alloc(dev);
+ if (IS_ERR(topo))
+ return PTR_ERR(topo);
+
+ /* Attempt to re-use a previously allocated port_id */
+ if (port->id)
+ ret = xa_insert(&topo->ports, port->id, port, GFP_KERNEL);
+ else
+ ret = xa_alloc_cyclic(&topo->ports, &port->id, port,
+ xa_limit_32b, &topo->next_port_index,
+ GFP_KERNEL);
+
+ return ret < 0 ? ret : 0;
+}
+EXPORT_SYMBOL_GPL(phy_link_topo_add_port);
+
+void phy_link_topo_del_port(struct net_device *dev, struct phy_port *port)
+{
+ struct phy_link_topology *topo = dev->link_topo;
+
+ if (!topo)
+ return;
+
+ xa_erase(&topo->ports, port->id);
+}
+EXPORT_SYMBOL_GPL(phy_link_topo_del_port);
+
int phy_link_topo_add_phy(struct net_device *dev,
struct phy_device *phy,
enum phy_upstream upt, void *upstream)
{
struct phy_link_topology *topo;
struct phy_device_node *pdn;
+ struct phy_port *port;
int ret;
/* ethtool ops may run without rtnl_lock, and rtnl_lock is what
@@ -110,8 +147,20 @@ int phy_link_topo_add_phy(struct net_device *dev,
if (ret < 0)
goto err;
+ /* Add all the PHY's ports to the topology */
+ list_for_each_entry(port, &phy->ports, head) {
+ ret = phy_link_topo_add_port(dev, port);
+ if (ret)
+ goto del_ports;
+ }
+
return 0;
+del_ports:
+ list_for_each_entry_continue_reverse(port, &phy->ports, head)
+ phy_link_topo_del_port(dev, port);
+
+ xa_erase(&topo->phys, phy->phyindex);
err:
kfree(pdn);
return ret;
@@ -123,10 +172,14 @@ void phy_link_topo_del_phy(struct net_device *dev,
{
struct phy_link_topology *topo = dev->link_topo;
struct phy_device_node *pdn;
+ struct phy_port *port;
if (!topo)
return;
+ list_for_each_entry(port, &phy->ports, head)
+ phy_link_topo_del_port(dev, port);
+
pdn = xa_erase(&topo->phys, phy->phyindex);
/* We delete the PHY from the topology, however we don't re-set the
diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
index 95575f68d5bc..296ee514ba46 100644
--- a/include/linux/phy_link_topology.h
+++ b/include/linux/phy_link_topology.h
@@ -16,11 +16,15 @@
struct xarray;
struct phy_device;
+struct phy_port;
struct sfp_bus;
struct phy_link_topology {
struct xarray phys;
u32 next_phy_index;
+
+ struct xarray ports;
+ u32 next_port_index;
};
struct phy_device_node {
@@ -48,6 +52,9 @@ int phy_link_topo_add_phy(struct net_device *dev,
void phy_link_topo_del_phy(struct net_device *dev, struct phy_device *phy);
+int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port);
+void phy_link_topo_del_port(struct net_device *dev, struct phy_port *port);
+
static inline struct phy_device *
phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
{
@@ -77,6 +84,17 @@ static inline void phy_link_topo_del_phy(struct net_device *dev,
{
}
+static inline int phy_link_topo_add_port(struct net_device *dev,
+ struct phy_port *port)
+{
+ return 0;
+}
+
+static inline void phy_link_topo_del_port(struct net_device *dev,
+ struct phy_port *port)
+{
+}
+
static inline struct phy_device *
phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
{
diff --git a/include/linux/phy_port.h b/include/linux/phy_port.h
index 0ef0f5ce4709..4e2a3fdd2f2e 100644
--- a/include/linux/phy_port.h
+++ b/include/linux/phy_port.h
@@ -36,6 +36,7 @@ struct phy_port_ops {
/**
* struct phy_port - A representation of a network device physical interface
*
+ * @id: Unique identifier for the port within the topology
* @head: Used by the port's parent to list ports
* @parent_type: The type of device this port is directly connected to
* @phy: If the parent is PHY_PORT_PHYDEV, the PHY controlling that port
@@ -52,6 +53,7 @@ struct phy_port_ops {
* @is_sfp: Indicates if this port drives an SFP cage.
*/
struct phy_port {
+ u32 id;
struct list_head head;
enum phy_port_parent parent_type;
union {
diff --git a/net/core/dev.c b/net/core/dev.c
index f660fccfc0db..97caa6846333 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11378,6 +11378,7 @@ static void netdev_free_phy_link_topology(struct net_device *dev)
if (IS_ENABLED(CONFIG_PHYLIB) && topo) {
xa_destroy(&topo->phys);
+ xa_destroy(&topo->ports);
kfree(topo);
dev->link_topo = NULL;
}
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
2026-09-27 13:36 ` [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc Maxime Chevallier
2026-09-27 13:36 ` [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:38 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream Maxime Chevallier
` (7 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
phy_port tracks the interfaces that a netdevice feeds into. SFP cages are
such ports, but so far we are only tracking the ones that are driven by
PHYs acting as media-converters.
Let's populate a phy_port for MAC driven SFP cages, handled by phylink.
This phy_port represents the SFP cage itself, and not the module that
may be plugged into it. It's therefore not an MDI interface, so only the
'interfaces' field is relevant here.
The phy_port is only populated for 'NETDEV' phylink instances, as
otherwise we don't have any topology to attach the port to.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/phy/phylink.c | 57 +++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a7d086cdc9b2..19090916af75 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -14,6 +14,8 @@
#include <linux/of_mdio.h>
#include <linux/phy.h>
#include <linux/phy_fixed.h>
+#include <linux/phy_link_topology.h>
+#include <linux/phy_port.h>
#include <linux/phylink.h>
#include <linux/rtnetlink.h>
#include <linux/spinlock.h>
@@ -93,6 +95,7 @@ struct phylink {
DECLARE_PHY_INTERFACE_MASK(sfp_interfaces);
__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
u8 sfp_port;
+ struct phy_port *sfp_cage_port;
struct eee_config eee_cfg;
@@ -1767,6 +1770,50 @@ static void phylink_fixed_poll(struct timer_list *t)
static const struct sfp_upstream_ops sfp_phylink_ops;
+static int phylink_create_sfp_cage_port(struct phylink *pl)
+{
+ struct phy_port *port;
+ int ret = 0;
+
+ if (!pl->netdev || !pl->sfp_bus)
+ return 0;
+
+ port = phy_port_alloc();
+ if (!port)
+ return -ENOMEM;
+
+ port->is_sfp = true;
+ port->is_mii = true;
+ port->active = true;
+
+ phy_interface_and(port->interfaces, pl->config->supported_interfaces,
+ phylink_sfp_interfaces);
+ phy_port_update_supported(port);
+
+ ret = phy_link_topo_add_port(pl->netdev, port);
+ if (ret)
+ phy_port_destroy(port);
+ else
+ pl->sfp_cage_port = port;
+
+ return ret;
+}
+
+static void phylink_destroy_sfp_cage_port(struct phylink *pl)
+{
+ if (pl->netdev && pl->sfp_cage_port) {
+ /* The port may still be accessed under RTNL */
+ rtnl_lock();
+ phy_link_topo_del_port(pl->netdev, pl->sfp_cage_port);
+ rtnl_unlock();
+ }
+
+ if (pl->sfp_cage_port)
+ phy_port_destroy(pl->sfp_cage_port);
+
+ pl->sfp_cage_port = NULL;
+}
+
static int phylink_register_sfp(struct phylink *pl,
const struct fwnode_handle *fwnode)
{
@@ -1784,9 +1831,18 @@ static int phylink_register_sfp(struct phylink *pl,
pl->sfp_bus = bus;
+ ret = phylink_create_sfp_cage_port(pl);
+ if (ret) {
+ sfp_bus_put(bus);
+ return ret;
+ }
+
ret = sfp_bus_add_upstream(bus, pl, &sfp_phylink_ops);
sfp_bus_put(bus);
+ if (ret)
+ phylink_destroy_sfp_cage_port(pl);
+
return ret;
}
@@ -2069,6 +2125,7 @@ EXPORT_SYMBOL_GPL(phylink_create);
void phylink_destroy(struct phylink *pl)
{
sfp_bus_del_upstream(pl->sfp_bus);
+ phylink_destroy_sfp_cage_port(pl);
if (pl->link_gpio)
gpiod_put(pl->link_gpio);
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (2 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:40 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port Maxime Chevallier
` (6 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
When dealing with PHY-driven SFP, we create a phy_port representing the
SFP bus when we know we have such a bus.
We can move the port creation before registering the sfp upstream ops,
as long as we know the SFP bus is there. This will allow passing the
phy_port along with the upstream information to the SFP bus.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/phy/phy_device.c | 58 +++++++++++++++++++++++-------------
1 file changed, 38 insertions(+), 20 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index c4d2c03b8282..3c20d4cc1a79 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1586,13 +1586,13 @@ static void phy_del_port(struct phy_device *phydev, struct phy_port *port)
phydev->n_ports--;
}
-static int phy_setup_sfp_port(struct phy_device *phydev)
+static struct phy_port *phy_setup_sfp_port(struct phy_device *phydev)
{
struct phy_port *port = phy_port_alloc();
int ret;
if (!port)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
port->parent_type = PHY_PORT_PHY;
port->phy = phydev;
@@ -1607,10 +1607,12 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
* when attaching the port to the phydev.
*/
ret = phy_add_port(phydev, port);
- if (ret)
+ if (ret) {
phy_port_destroy(port);
+ return ERR_PTR(ret);
+ }
- return ret;
+ return port;
}
/**
@@ -1619,30 +1621,46 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
*/
static int phy_sfp_probe(struct phy_device *phydev)
{
+ struct phy_port *port = NULL;
struct sfp_bus *bus;
- int ret = 0;
+ int ret;
- if (phydev->mdio.dev.fwnode) {
- bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
- if (IS_ERR(bus))
- return PTR_ERR(bus);
+ if (!phydev->mdio.dev.fwnode)
+ return 0;
- phydev->sfp_bus = bus;
+ bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
+ if (IS_ERR(bus))
+ return PTR_ERR(bus);
- ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
- sfp_bus_put(bus);
+ phydev->sfp_bus = bus;
- if (ret)
- phydev->sfp_bus = NULL;
+ if (bus) {
+ port = phy_setup_sfp_port(phydev);
+ if (IS_ERR(port)) {
+ ret = PTR_ERR(port);
+ goto out_sfp;
+ }
}
- if (!ret && phydev->sfp_bus) {
- ret = phy_setup_sfp_port(phydev);
- if (ret) {
- sfp_bus_del_upstream(phydev->sfp_bus);
- phydev->sfp_bus = NULL;
- }
+ ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
+ if (ret)
+ goto out_port;
+
+ /* sfp_bus_add_upstream() grabs a ref to the sfp bus on success, it's
+ * safe to release it now.
+ */
+ sfp_bus_put(bus);
+
+ return ret;
+
+out_port:
+ if (port) {
+ phy_del_port(phydev, port);
+ phy_port_destroy(port);
}
+out_sfp:
+ sfp_bus_put(bus);
+ phydev->sfp_bus = NULL;
return ret;
}
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (3 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:42 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream Maxime Chevallier
` (5 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
Now that the SFP bus infrastructure notifies when PHY-less modules are
connected, we can create a phy_port to represent it. Instead of letting
the SFP subsystem handle that, the Bus' upstream is in charge of
maintaining that phy_port and register it to the topology, as the
upstream (in this case a phy device) is directly interacting with the
underlying net_device.
Add a phy_caps helper to get the achievable modes on this module based
on what the phy_port representing the bus supports.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/phy/phy-caps.h | 2 +
drivers/net/phy/phy_caps.c | 26 +++++++++
drivers/net/phy/phy_device.c | 100 +++++++++++++++++++++++++++++++++--
drivers/net/phy/phylink.c | 76 ++++++++++++++++++++++++--
include/linux/phy.h | 6 +++
5 files changed, 203 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
index 421088e6f6e8..ec3d39a0ae06 100644
--- a/drivers/net/phy/phy-caps.h
+++ b/drivers/net/phy/phy-caps.h
@@ -66,5 +66,7 @@ void phy_caps_medium_get_supported(unsigned long *supported,
enum ethtool_link_medium medium,
int lanes);
u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes);
+void phy_caps_linkmode_filter_ifaces(unsigned long *to, const unsigned long *from,
+ const unsigned long *interfaces);
#endif /* __PHY_CAPS_H */
diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
index 942d43191561..558e4df4d63c 100644
--- a/drivers/net/phy/phy_caps.c
+++ b/drivers/net/phy/phy_caps.c
@@ -445,3 +445,29 @@ u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes)
return mediums;
}
EXPORT_SYMBOL_GPL(phy_caps_mediums_from_linkmodes);
+
+/**
+ * phy_caps_linkmode_filter_ifaces() - Filter linkmodes with an interface list
+ * @to: Stores the filtered linkmodes
+ * @from: Linkmodes to filter
+ * @interfaces: Bitfield of phy_interface_t that we use for filtering
+ *
+ * Filter the provided linkmodes, only to keep the ones we can possibly achieve
+ * when using any of the provided MII interfaces.
+ */
+void phy_caps_linkmode_filter_ifaces(unsigned long *to,
+ const unsigned long *from,
+ const unsigned long *interfaces)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(ifaces_supported) = {};
+ unsigned int ifaces_caps = 0;
+ phy_interface_t interface;
+
+ for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
+ ifaces_caps |= phy_caps_from_interface(interface);
+
+ phy_caps_linkmodes(ifaces_caps, ifaces_supported);
+
+ linkmode_and(to, from, ifaces_supported);
+}
+EXPORT_SYMBOL_GPL(phy_caps_linkmode_filter_ifaces);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 3c20d4cc1a79..ab9631b161bc 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1400,11 +1400,21 @@ static int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
{
struct phy_device *phydev = upstream;
struct net_device *dev = phydev->attached_dev;
+ int ret;
- if (dev)
- return phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev);
+ phydev->has_sfp_mod_phy = true;
- return 0;
+ /* If we aren't attached to a netdev, we can't add the SFP PHY to its
+ * topology.
+ */
+ if (!dev)
+ return 0;
+
+ ret = phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev);
+ if (ret)
+ phydev->has_sfp_mod_phy = false;
+
+ return ret;
}
/**
@@ -1422,6 +1432,8 @@ static void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
struct phy_device *phydev = upstream;
struct net_device *dev = phydev->attached_dev;
+ phydev->has_sfp_mod_phy = false;
+
if (dev)
phy_link_topo_del_phy(dev, phy);
}
@@ -1527,6 +1539,75 @@ static void phy_sfp_link_down(void *upstream)
port->ops->link_down(port);
}
+static int phy_add_sfp_mod_port(struct phy_device *phydev)
+{
+ const struct sfp_module_caps *caps;
+ struct phy_port *port;
+ int ret = 0;
+
+ /* Create mod port */
+ port = phy_port_alloc();
+ if (!port)
+ return -ENOMEM;
+
+ port->active = true;
+
+ caps = sfp_get_module_caps(phydev->sfp_bus);
+
+ phy_caps_linkmode_filter_ifaces(port->supported, caps->link_modes,
+ phydev->sfp_cage_port->interfaces);
+
+ if (phydev->attached_dev) {
+ ret = phy_link_topo_add_port(phydev->attached_dev, port);
+ if (ret) {
+ phy_port_destroy(port);
+ return ret;
+ }
+ }
+
+ /* we don't use phy_add_port() here as the module port isn't a direct
+ * interface from the PHY, but rather an extension to the sfp-bus, that
+ * is already represented by its own phy_port
+ */
+ phydev->mod_port = port;
+
+ return 0;
+}
+
+static void phy_del_sfp_mod_port(struct phy_device *phydev)
+{
+ if (!phydev->mod_port)
+ return;
+
+ if (phydev->attached_dev)
+ phy_link_topo_del_port(phydev->attached_dev, phydev->mod_port);
+
+ phy_port_destroy(phydev->mod_port);
+ phydev->mod_port = NULL;
+}
+
+static int phy_sfp_module_start(void *upstream)
+{
+ struct phy_device *phydev = upstream;
+
+ /* If there's a downstream SFP module, and it doesn't contain a PHY
+ * device, let's create a phy_port to represent that module.
+ */
+ if (!phydev->has_sfp_mod_phy)
+ return phy_add_sfp_mod_port(phydev);
+
+ return 0;
+}
+
+static void phy_sfp_module_stop(void *upstream)
+{
+ struct phy_device *phydev = upstream;
+
+ /* Called upon module removal or upstream removal */
+ if (!phydev->has_sfp_mod_phy)
+ phy_del_sfp_mod_port(phydev);
+}
+
static const struct sfp_upstream_ops sfp_phydev_ops = {
.attach = phy_sfp_attach,
.detach = phy_sfp_detach,
@@ -1536,6 +1617,8 @@ static const struct sfp_upstream_ops sfp_phydev_ops = {
.link_down = phy_sfp_link_down,
.connect_phy = phy_sfp_connect_phy,
.disconnect_phy = phy_sfp_disconnect_phy,
+ .module_start = phy_sfp_module_start,
+ .module_stop = phy_sfp_module_stop,
};
static int phy_add_port(struct phy_device *phydev, struct phy_port *port)
@@ -1638,6 +1721,7 @@ static int phy_sfp_probe(struct phy_device *phydev)
port = phy_setup_sfp_port(phydev);
if (IS_ERR(port)) {
ret = PTR_ERR(port);
+ port = NULL;
goto out_sfp;
}
}
@@ -1651,6 +1735,8 @@ static int phy_sfp_probe(struct phy_device *phydev)
*/
sfp_bus_put(bus);
+ phydev->sfp_cage_port = port;
+
return ret;
out_port:
@@ -1744,6 +1830,8 @@ static void phy_detach_internal(struct phy_device *phydev, bool notify_bus)
phydev->attached_dev->phydev = NULL;
phydev->attached_dev = NULL;
phy_link_topo_del_phy(dev, phydev);
+ if (phydev->mod_port)
+ phy_link_topo_del_port(dev, phydev->mod_port);
}
phydev->phy_link_change = NULL;
@@ -1873,6 +1961,12 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
err = phy_link_topo_add_phy(dev, phydev, PHY_UPSTREAM_MAC, dev);
if (err)
goto error;
+
+ if (phydev->mod_port) {
+ err = phy_link_topo_add_port(dev, phydev->mod_port);
+ if (err)
+ goto error;
+ }
}
/* Some Ethernet drivers try to connect to a PHY device before
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 19090916af75..db05d2dda429 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -96,6 +96,7 @@ struct phylink {
__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
u8 sfp_port;
struct phy_port *sfp_cage_port;
+ struct phy_port *mod_port;
struct eee_config eee_cfg;
@@ -1792,10 +1793,15 @@ static int phylink_create_sfp_cage_port(struct phylink *pl)
ret = phy_link_topo_add_port(pl->netdev, port);
if (ret)
- phy_port_destroy(port);
- else
- pl->sfp_cage_port = port;
+ goto out_destroy_port;
+
+ pl->sfp_cage_port = port;
+
+ return 0;
+out_destroy_port:
+ phy_port_destroy(port);
+ pl->sfp_cage_port = NULL;
return ret;
}
@@ -4012,14 +4018,65 @@ static void phylink_sfp_module_remove(void *upstream)
phy_interface_zero(pl->sfp_interfaces);
}
+static int phylink_add_sfp_mod_port(struct phylink *pl)
+{
+ const struct sfp_module_caps *caps;
+ struct phy_port *port;
+ int ret = 0;
+
+ if (!pl->sfp_cage_port)
+ return 0;
+
+ /* Create mod port */
+ port = phy_port_alloc();
+ if (!port)
+ return -ENOMEM;
+
+ port->active = true;
+
+ caps = sfp_get_module_caps(pl->sfp_bus);
+
+ phy_caps_linkmode_filter_ifaces(port->supported, caps->link_modes,
+ pl->sfp_cage_port->interfaces);
+
+ if (pl->netdev) {
+ ret = phy_link_topo_add_port(pl->netdev, port);
+ if (ret) {
+ phy_port_destroy(port);
+ return ret;
+ }
+ }
+
+ pl->mod_port = port;
+
+ return 0;
+}
+
+static void phylink_del_sfp_mod_port(struct phylink *pl)
+{
+ if (!pl->mod_port)
+ return;
+
+ if (pl->netdev)
+ phy_link_topo_del_port(pl->netdev, pl->mod_port);
+
+ phy_port_destroy(pl->mod_port);
+ pl->mod_port = NULL;
+}
+
static int phylink_sfp_module_start(void *upstream)
{
struct phylink *pl = upstream;
+ int ret;
/* If this SFP module has a PHY, start the PHY now. */
if (pl->phydev) {
phy_start(pl->phydev);
return 0;
+ } else {
+ ret = phylink_add_sfp_mod_port(pl);
+ if (ret)
+ return ret;
}
/* If the module may have a PHY but we didn't detect one we
@@ -4028,7 +4085,16 @@ static int phylink_sfp_module_start(void *upstream)
if (!pl->sfp_may_have_phy)
return 0;
- return phylink_sfp_config_optical(pl);
+ ret = phylink_sfp_config_optical(pl);
+ if (ret)
+ goto del_mod_port;
+
+ return 0;
+
+del_mod_port:
+ phylink_del_sfp_mod_port(pl);
+
+ return ret;
}
static void phylink_sfp_module_stop(void *upstream)
@@ -4038,6 +4104,8 @@ static void phylink_sfp_module_stop(void *upstream)
/* If this SFP module has a PHY, stop it. */
if (pl->phydev)
phy_stop(pl->phydev);
+ else
+ phylink_del_sfp_mod_port(pl);
}
static void phylink_sfp_link_down(void *upstream)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c5098a0dd6c..dbd6ed5864f2 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -611,7 +611,10 @@ struct phy_oatc14_sqi_capability {
* @irq: IRQ number of the PHY's interrupt (-1 if none)
* @phylink: Pointer to phylink instance for this PHY
* @sfp_bus_attached: Flag indicating whether the SFP bus has been attached
+ * @has_sfp_mod_phy: Set true if downstream SFP bus's module contains a PHY
* @sfp_bus: SFP bus attached to this PHY's fiber port
+ * @sfp_cage_port: The phy_port connected to the downstream SFP cage
+ * @mod_port: phy_port representing the SFP module, if it is phy-less
* @attached_dev: The attached enet driver's device instance ptr
* @adjust_link: Callback for the enet controller to respond to changes: in the
* link state.
@@ -802,7 +805,10 @@ struct phy_device {
/* This may be modified under the rtnl lock */
bool sfp_bus_attached;
+ bool has_sfp_mod_phy;
struct sfp_bus *sfp_bus;
+ struct phy_port *sfp_cage_port;
+ struct phy_port *mod_port;
struct phylink *phylink;
struct net_device *attached_dev;
struct mii_timestamper *mii_ts;
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (4 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports Maxime Chevallier
` (4 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
MII phy_ports are not meant to be connected directly to a link partner.
They are meant to feed into some media converter devices that will
expose an MDI phy_port, so far we only support SFP modules for that.
In the case an MDI phy_port is backed by an MII port (e.g. a SFP
module's port, backed by the SFP cage port), let's keep track of the
port id of the MII port backing it.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/phy/phy_device.c | 29 +++++++++++++++++++++++++++--
drivers/net/phy/phylink.c | 5 +++++
include/linux/phy.h | 4 ++++
include/linux/phy_port.h | 3 +++
4 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ab9631b161bc..2bb4abd0d392 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1403,6 +1403,7 @@ static int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
int ret;
phydev->has_sfp_mod_phy = true;
+ phy_set_upstream_port(phy, phydev->sfp_cage_port);
/* If we aren't attached to a netdev, we can't add the SFP PHY to its
* topology.
@@ -1436,6 +1437,8 @@ static void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
if (dev)
phy_link_topo_del_phy(dev, phy);
+
+ phy_set_upstream_port(phy, NULL);
}
/**
@@ -1571,6 +1574,8 @@ static int phy_add_sfp_mod_port(struct phy_device *phydev)
*/
phydev->mod_port = port;
+ port->upstream_port = phydev->sfp_cage_port;
+
return 0;
}
@@ -1726,6 +1731,8 @@ static int phy_sfp_probe(struct phy_device *phydev)
}
}
+ phydev->sfp_cage_port = port;
+
ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
if (ret)
goto out_port;
@@ -1735,14 +1742,13 @@ static int phy_sfp_probe(struct phy_device *phydev)
*/
sfp_bus_put(bus);
- phydev->sfp_cage_port = port;
-
return ret;
out_port:
if (port) {
phy_del_port(phydev, port);
phy_port_destroy(port);
+ phydev->sfp_cage_port = NULL;
}
out_sfp:
sfp_bus_put(bus);
@@ -3687,6 +3693,25 @@ struct phy_port *phy_get_sfp_port(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(phy_get_sfp_port);
+/**
+ * phy_set_upstream_port() - Sets the phy_port controlling the MII this PHY is
+ * attached to.
+ * @phydev: pointer to the PHY device we set the upstream of.
+ * @port: The phy_port upstream of this PHY, can be NULL.
+ */
+void phy_set_upstream_port(struct phy_device *phydev, struct phy_port *port)
+{
+ struct phy_port *local_port;
+
+ ASSERT_RTNL();
+
+ phydev->upstream_port = port;
+
+ phy_for_each_port(phydev, local_port)
+ local_port->upstream_port = port;
+}
+EXPORT_SYMBOL_GPL(phy_set_upstream_port);
+
/**
* fwnode_mdio_find_device - Given a fwnode, find the mdio_device
* @fwnode: pointer to the mdio_device's fwnode
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index db05d2dda429..2583e913cff6 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -4047,6 +4047,8 @@ static int phylink_add_sfp_mod_port(struct phylink *pl)
}
}
+ port->upstream_port = pl->sfp_cage_port;
+
pl->mod_port = port;
return 0;
@@ -4150,6 +4152,8 @@ static int phylink_sfp_connect_phy(void *upstream, struct phy_device *phy)
phy_interface_and(phy->host_interfaces, phylink_sfp_interfaces,
pl->config->supported_interfaces);
+ phy_set_upstream_port(phy, pl->sfp_cage_port);
+
/* Do the initial configuration */
return phylink_sfp_config_phy(pl, phy);
}
@@ -4158,6 +4162,7 @@ static void phylink_sfp_disconnect_phy(void *upstream,
struct phy_device *phydev)
{
phylink_disconnect_phy(upstream);
+ phy_set_upstream_port(phydev, NULL);
}
static const struct sfp_upstream_ops sfp_phylink_ops = {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index dbd6ed5864f2..1fb87d12367b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -615,6 +615,7 @@ struct phy_oatc14_sqi_capability {
* @sfp_bus: SFP bus attached to this PHY's fiber port
* @sfp_cage_port: The phy_port connected to the downstream SFP cage
* @mod_port: phy_port representing the SFP module, if it is phy-less
+ * @upstream_port: phy_port this PHY's MII attaches to, if any
* @attached_dev: The attached enet driver's device instance ptr
* @adjust_link: Callback for the enet controller to respond to changes: in the
* link state.
@@ -809,6 +810,7 @@ struct phy_device {
struct sfp_bus *sfp_bus;
struct phy_port *sfp_cage_port;
struct phy_port *mod_port;
+ struct phy_port *upstream_port;
struct phylink *phylink;
struct net_device *attached_dev;
struct mii_timestamper *mii_ts;
@@ -2493,6 +2495,8 @@ int __phy_hwtstamp_set(struct phy_device *phydev,
struct phy_port *phy_get_sfp_port(struct phy_device *phydev);
+void phy_set_upstream_port(struct phy_device *phydev, struct phy_port *port);
+
/**
* phy_module_driver() - Helper macro for registering PHY drivers
* @__phy_drivers: array of PHY drivers to register
diff --git a/include/linux/phy_port.h b/include/linux/phy_port.h
index 4e2a3fdd2f2e..8f45c031cbc1 100644
--- a/include/linux/phy_port.h
+++ b/include/linux/phy_port.h
@@ -40,6 +40,8 @@ struct phy_port_ops {
* @head: Used by the port's parent to list ports
* @parent_type: The type of device this port is directly connected to
* @phy: If the parent is PHY_PORT_PHYDEV, the PHY controlling that port
+ * @upstream_port: Indicates the MII port that feeds this port, if any,
+ * e.g. the SFP cage port for a SFP module port.
* @ops: Callback ops implemented by the port controller
* @pairs: The number of pairs this port has, 0 if not applicable
* @mediums: Bitmask of the physical mediums this port provides access to
@@ -59,6 +61,7 @@ struct phy_port {
union {
struct phy_device *phy;
};
+ struct phy_port *upstream_port;
const struct phy_port_ops *ops;
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (5 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool Maxime Chevallier
` (3 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
In order to allow netlink access to phy_ports, let's add a helper to
retrieve them. When handling a port coming from phy_link_topology, the
caller must hold rtnl until it's done with it.
We don't really support PHY and phy_port operation on ops-locked devices
yet, warn when we try to do so on such an interface.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/phy/phy_link_topology.c | 8 ++++++++
include/linux/phy_link_topology.h | 25 +++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
index 28c5bf3a8849..214ccce23cf6 100644
--- a/drivers/net/phy/phy_link_topology.c
+++ b/drivers/net/phy/phy_link_topology.c
@@ -65,6 +65,14 @@ int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port)
struct phy_link_topology *topo;
int ret;
+ /* Ports can now be queried without rtnl for ops-locked devices, which
+ * we don't support now as port retrieval is done under rtnl.
+ * We don't have phy_port enabled devices yet, let's make sure
+ * we are loudly warned about that when it happens.
+ */
+ if (WARN_ON_ONCE(dev->netdev_ops && netdev_need_ops_lock(dev)))
+ return -EOPNOTSUPP;
+
topo = phy_link_topo_get_or_alloc(dev);
if (IS_ERR(topo))
return PTR_ERR(topo);
diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
index 296ee514ba46..f2e493c433ff 100644
--- a/include/linux/phy_link_topology.h
+++ b/include/linux/phy_link_topology.h
@@ -13,6 +13,7 @@
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
struct xarray;
struct phy_device;
@@ -71,6 +72,24 @@ phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
return NULL;
}
+static inline struct phy_port *
+phy_link_topo_get_port(struct net_device *dev, u32 port_id)
+{
+ struct phy_link_topology *topo = dev->link_topo;
+
+ if (!topo)
+ return NULL;
+
+ /* ports in the topo are RTNL protected, but may be accessed under
+ * netdev_lock for ops-locked devices. For now keep them under rtnl
+ * protection, as no ops-locked devices have phy_port capabilities
+ * yet.
+ */
+ ASSERT_RTNL();
+
+ return xa_load(&topo->ports, port_id);
+}
+
#else
static inline int phy_link_topo_add_phy(struct net_device *dev,
struct phy_device *phy,
@@ -100,6 +119,12 @@ phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
{
return NULL;
}
+
+static inline struct phy_port *
+phy_link_topo_get_port(struct net_device *dev, u32 port_id)
+{
+ return NULL;
+}
#endif
#endif /* __PHY_LINK_TOPOLOGY_H */
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (6 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:47 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports Maxime Chevallier
` (2 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
Ethernet network interfaces may have more than one front-facing port.
The phy_port infrastructure was introduced to keep track of
these ports, and allow userspace to know about the presence and
capability of these ports. Add a ethnl netlink message to report this
information.
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
Documentation/netlink/specs/ethtool.yaml | 55 +++++++++++++++++++
Documentation/networking/ethtool-netlink.rst | 36 ++++++++++++
.../uapi/linux/ethtool_netlink_generated.h | 22 ++++++++
3 files changed, 113 insertions(+)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 5dd4d1b5d94b..35e028c0be29 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -210,6 +210,11 @@ definitions:
-
name: discard
value: 31
+ -
+ name: port-type
+ doc: type of physical connector
+ type: enum
+ entries: [mdi, sfp]
attribute-sets:
-
@@ -1905,6 +1910,32 @@ attribute-sets:
name: link
type: nest
nested-attributes: mse-snapshot
+ -
+ name: port
+ attr-cnt-name: --ethtool-a-port-cnt
+ attributes:
+ -
+ name: header
+ type: nest
+ nested-attributes: header
+ -
+ name: id
+ type: u32
+ -
+ name: supported-modes
+ type: nest
+ nested-attributes: bitset
+ -
+ name: supported-interfaces
+ type: nest
+ nested-attributes: bitset
+ -
+ name: type
+ type: u32
+ enum: port-type
+ -
+ name: upstream-port
+ type: u32
operations:
enum-model: directional
@@ -2859,6 +2890,30 @@ operations:
- worst-channel
- link
dump: *mse-get-op
+ -
+ name: port-get
+ doc: Get ports attached to an interface
+
+ attribute-set: port
+
+ do:
+ request:
+ attributes:
+ - header
+ - id
+ reply: &port-get-op-reply
+ attributes:
+ - header
+ - id
+ - supported-modes
+ - supported-interfaces
+ - type
+ - upstream-port
+ dump:
+ request:
+ attributes:
+ - header
+ reply: *port-get-op-reply
mcast-groups:
list:
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 393f2b6d3863..eba226c79ec3 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -247,6 +247,7 @@ Userspace to kernel:
``ETHTOOL_MSG_RSS_CREATE_ACT`` create an additional RSS context
``ETHTOOL_MSG_RSS_DELETE_ACT`` delete an additional RSS context
``ETHTOOL_MSG_MSE_GET`` get MSE diagnostic data
+ ``ETHTOOL_MSG_PORT_GET`` get port information
===================================== =================================
Kernel to userspace:
@@ -305,6 +306,7 @@ Kernel to userspace:
``ETHTOOL_MSG_RSS_CREATE_NTF`` additional RSS context created
``ETHTOOL_MSG_RSS_DELETE_NTF`` additional RSS context deleted
``ETHTOOL_MSG_MSE_GET_REPLY`` MSE diagnostic data
+ ``ETHTOOL_MSG_PORT_GET_REPLY`` port information
======================================== =================================
``GET`` requests are sent by userspace applications to retrieve device
@@ -2537,6 +2539,39 @@ Within each channel nest, only the metrics supported by the PHY will be present.
See ``struct phy_mse_snapshot`` kernel documentation in
``include/linux/phy.h``.
+PORT_GET
+========
+
+Retrieve information about the physical connection points of a network device,
+referred to as "ports". User needs to specify a PORT_ID for the DO operation,
+in which case the DO request returns information about that specific port.
+
+As there can be more than one port, the DUMP operation can be used to list the
+ports present on a given interface, by passing an interface index or name in
+the dump request.
+
+Request contents:
+
+ ===================================== ====== ===============================
+ ``ETHTOOL_A_PORT_HEADER`` nested request header
+ ``ETHTOOL_A_PORT_ID`` u32 port id
+ ===================================== ====== ===============================
+
+Kernel response contents:
+
+ ======================================= ====== =============================
+ ``ETHTOOL_A_PORT_HEADER`` nested request header
+ ``ETHTOOL_A_PORT_ID`` u32 the port's unique identifier,
+ per netdevice.
+ ``ETHTOOL_A_PORT_SUPPORTED_MODES`` bitset bitset of supported linkmodes
+ ``ETHTOOL_A_PORT_SUPPORTED_INTERFACES`` bitset bitset of supported MII
+ interfaces
+ ``ETHTOOL_A_PORT_TYPE`` u32 the port type
+ ``ETHTOOL_A_PORT_UPSTREAM_PORT`` u32 If any, the index of the MII
+ port that feeds into this
+ port.
+ ======================================= ====== =============================
+
Request translation
===================
@@ -2647,4 +2682,5 @@ are netlink only.
n/a ``ETHTOOL_MSG_PHY_GET``
``SIOCGHWTSTAMP`` ``ETHTOOL_MSG_TSCONFIG_GET``
``SIOCSHWTSTAMP`` ``ETHTOOL_MSG_TSCONFIG_SET``
+ n/a ``ETHTOOL_MSG_PORT_GET``
=================================== =====================================
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index 8134baf7860f..660e0c08a766 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -78,6 +78,14 @@ enum ethtool_pse_event {
ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR = 64,
};
+/*
+ * type of physical connector
+ */
+enum ethtool_port_type {
+ ETHTOOL_PORT_TYPE_MDI,
+ ETHTOOL_PORT_TYPE_SFP,
+};
+
enum {
ETHTOOL_A_HEADER_UNSPEC,
ETHTOOL_A_HEADER_DEV_INDEX,
@@ -840,6 +848,18 @@ enum {
ETHTOOL_A_MSE_MAX = (__ETHTOOL_A_MSE_CNT - 1)
};
+enum {
+ ETHTOOL_A_PORT_HEADER = 1,
+ ETHTOOL_A_PORT_ID,
+ ETHTOOL_A_PORT_SUPPORTED_MODES,
+ ETHTOOL_A_PORT_SUPPORTED_INTERFACES,
+ ETHTOOL_A_PORT_TYPE,
+ ETHTOOL_A_PORT_UPSTREAM_PORT,
+
+ __ETHTOOL_A_PORT_CNT,
+ ETHTOOL_A_PORT_MAX = (__ETHTOOL_A_PORT_CNT - 1)
+};
+
enum {
ETHTOOL_MSG_USER_NONE = 0,
ETHTOOL_MSG_STRSET_GET = 1,
@@ -893,6 +913,7 @@ enum {
ETHTOOL_MSG_RSS_CREATE_ACT,
ETHTOOL_MSG_RSS_DELETE_ACT,
ETHTOOL_MSG_MSE_GET,
+ ETHTOOL_MSG_PORT_GET,
__ETHTOOL_MSG_USER_CNT,
ETHTOOL_MSG_USER_MAX = (__ETHTOOL_MSG_USER_CNT - 1)
@@ -954,6 +975,7 @@ enum {
ETHTOOL_MSG_RSS_CREATE_NTF,
ETHTOOL_MSG_RSS_DELETE_NTF,
ETHTOOL_MSG_MSE_GET_REPLY,
+ ETHTOOL_MSG_PORT_GET_REPLY,
__ETHTOOL_MSG_KERNEL_CNT,
ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (7 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 9:48 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 10/10] Documentation: networking: Update the phy_port infrastructure description Maxime Chevallier
2026-09-28 17:31 ` [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Christophe Leroy (CS GROUP)
10 siblings, 2 replies; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
Expose the phy_port information to userspace, so that we can know how
many ports are available on a given interface, as well as their
capabilities. For MDI ports, we report the list of supported linkmodes
based on what the PHY that drives this port says.
For MII ports, i.e. empty SFP cages, we report the MII linkmodes that we
can output on this port.
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
MAINTAINERS | 1 +
net/ethtool/Makefile | 2 +-
net/ethtool/netlink.c | 25 +++
net/ethtool/netlink.h | 8 +
net/ethtool/port.c | 382 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 417 insertions(+), 1 deletion(-)
create mode 100644 net/ethtool/port.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6de1ff058db6..877f3316b5ea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19010,6 +19010,7 @@ F: Documentation/devicetree/bindings/net/ethernet-connector.yaml
F: Documentation/networking/phy-port.rst
F: drivers/net/phy/phy_port.c
F: include/linux/phy_port.h
+F: net/ethtool/port.c
K: struct\s+phy_port|phy_port_
NETWORKING [GENERAL]
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 629c10916670..9b5b09670008 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 port.o
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 1af395b54330..c076c07d0a08 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -26,6 +26,8 @@ static u32 ethnl_bcast_seq;
ETHTOOL_FLAG_OMIT_REPLY)
#define ETHTOOL_FLAGS_STATS (ETHTOOL_FLAGS_BASIC | ETHTOOL_FLAG_STATS)
+char phy_interface_names[PHY_INTERFACE_MODE_MAX][ETH_GSTRING_LEN] __ro_after_init;
+
const struct nla_policy ethnl_header_policy[] = {
[ETHTOOL_A_HEADER_DEV_INDEX] = { .type = NLA_U32 },
[ETHTOOL_A_HEADER_DEV_NAME] = { .type = NLA_NUL_STRING,
@@ -431,6 +433,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
[ETHTOOL_MSG_TSCONFIG_SET] = ðnl_tsconfig_request_ops,
[ETHTOOL_MSG_PHY_GET] = ðnl_phy_request_ops,
[ETHTOOL_MSG_MSE_GET] = ðnl_mse_request_ops,
+ [ETHTOOL_MSG_PORT_GET] = ðnl_port_request_ops,
};
static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -1572,6 +1575,15 @@ static const struct genl_ops ethtool_genl_ops[] = {
.policy = ethnl_mse_get_policy,
.maxattr = ARRAY_SIZE(ethnl_mse_get_policy) - 1,
},
+ {
+ .cmd = ETHTOOL_MSG_PORT_GET,
+ .doit = ethnl_default_doit,
+ .start = ethnl_port_dump_start,
+ .dumpit = ethnl_port_dumpit,
+ .done = ethnl_port_dump_done,
+ .policy = ethnl_port_get_policy,
+ .maxattr = ARRAY_SIZE(ethnl_port_get_policy) - 1,
+ },
};
static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
@@ -1594,10 +1606,23 @@ static struct genl_family ethtool_genl_family __ro_after_init = {
/* module setup */
+static void __init ethnl_phy_names_populate(void)
+{
+ const char *name;
+ int i;
+
+ for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) {
+ name = phy_modes(i);
+ strscpy(phy_interface_names[i], name, ETH_GSTRING_LEN);
+ }
+}
+
static int __init ethnl_init(void)
{
int ret;
+ ethnl_phy_names_populate();
+
ret = genl_register_family(ðtool_genl_family);
if (WARN(ret < 0, "ethtool: genetlink family registration failed"))
return ret;
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 3e969a070f9f..dc7211f67c4d 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -5,11 +5,14 @@
#include <linux/ethtool_netlink.h>
#include <linux/netdevice.h>
+#include <linux/phy.h>
#include <net/genetlink.h>
#include <net/sock.h>
struct ethnl_req_info;
+extern char phy_interface_names[PHY_INTERFACE_MODE_MAX][ETH_GSTRING_LEN];
+
int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
const struct nlattr *nest, struct net *net,
struct netlink_ext_ack *extack,
@@ -474,6 +477,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_port_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];
@@ -530,6 +534,7 @@ 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_port_get_policy[ETHTOOL_A_PORT_ID + 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);
@@ -545,6 +550,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_port_dump_start(struct netlink_callback *cb);
+int ethnl_port_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int ethnl_port_dump_done(struct netlink_callback *cb);
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/port.c b/net/ethtool/port.c
new file mode 100644
index 000000000000..0d6150e5f66a
--- /dev/null
+++ b/net/ethtool/port.c
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2026 Bootlin
+ */
+#include <linux/phy.h>
+#include <linux/phy_link_topology.h>
+#include <linux/phy_port.h>
+#include <net/netdev_lock.h>
+
+#include "bitset.h"
+#include "common.h"
+#include "netlink.h"
+
+struct port_req_info {
+ struct ethnl_req_info base;
+ u32 port_id;
+};
+
+struct port_reply_data {
+ struct ethnl_reply_data base;
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+ DECLARE_PHY_INTERFACE_MASK(interfaces);
+ u32 port_id;
+ bool mii;
+ bool sfp;
+ u32 upstream_port;
+};
+
+#define PORT_REQINFO(__req_base) \
+ container_of(__req_base, struct port_req_info, base)
+
+#define PORT_REPDATA(__reply_base) \
+ container_of(__reply_base, struct port_reply_data, base)
+
+const struct nla_policy ethnl_port_get_policy[ETHTOOL_A_PORT_ID + 1] = {
+ [ETHTOOL_A_PORT_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+ [ETHTOOL_A_PORT_ID] = NLA_POLICY_MIN(NLA_U32, 1),
+};
+
+static int port_parse_request(struct ethnl_req_info *req_info,
+ const struct genl_info *info,
+ struct nlattr **tb,
+ struct netlink_ext_ack *extack)
+{
+ struct port_req_info *request = PORT_REQINFO(req_info);
+
+ if (GENL_REQ_ATTR_CHECK(info, ETHTOOL_A_PORT_ID))
+ return -EINVAL;
+
+ request->port_id = nla_get_u32(tb[ETHTOOL_A_PORT_ID]);
+
+ return 0;
+}
+
+static int port_prepare_data(const struct ethnl_req_info *req_info,
+ struct ethnl_reply_data *reply_data,
+ const struct genl_info *info)
+{
+ struct port_reply_data *reply = PORT_REPDATA(reply_data);
+ struct port_req_info *request = PORT_REQINFO(req_info);
+ struct phy_port *port;
+
+ /* RTNL must be held while holding a ref to the phy_port. Here, caller
+ * holds RTNL.
+ */
+ port = phy_link_topo_get_port(req_info->dev, request->port_id);
+ if (!port)
+ return -ENODEV;
+
+ linkmode_copy(reply->supported, port->supported);
+ phy_interface_copy(reply->interfaces, port->interfaces);
+ reply->port_id = port->id;
+ reply->mii = port->is_mii;
+ reply->sfp = port->is_sfp;
+ reply->upstream_port = port->upstream_port ? port->upstream_port->id : 0;
+
+ return 0;
+}
+
+static int port_reply_size(const struct ethnl_req_info *req_info,
+ const struct ethnl_reply_data *reply_data)
+{
+ bool compact = req_info->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+ struct port_reply_data *reply = PORT_REPDATA(reply_data);
+ size_t size = 0;
+ int ret;
+
+ /* ETHTOOL_A_PORT_ID */
+ size += nla_total_size(sizeof(u32));
+
+ if (!reply->mii) {
+ /* ETHTOOL_A_PORT_SUPPORTED_MODES */
+ ret = ethnl_bitset_size(reply->supported, NULL,
+ __ETHTOOL_LINK_MODE_MASK_NBITS,
+ link_mode_names, compact);
+ if (ret < 0)
+ return ret;
+
+ size += ret;
+ } else {
+ /* ETHTOOL_A_PORT_SUPPORTED_INTERFACES */
+ ret = ethnl_bitset_size(reply->interfaces, NULL,
+ PHY_INTERFACE_MODE_MAX,
+ phy_interface_names, compact);
+ if (ret < 0)
+ return ret;
+
+ size += ret;
+ }
+
+ /* ETHTOOL_A_PORT_TYPE */
+ size += nla_total_size(sizeof(u32));
+
+ /* ETHTOOL_A_PORT_UPSTREAM_PORT */
+ if (reply->upstream_port)
+ size += nla_total_size(sizeof(u32));
+
+ return size;
+}
+
+static int port_fill_reply(struct sk_buff *skb,
+ const struct ethnl_req_info *req_info,
+ const struct ethnl_reply_data *reply_data)
+{
+ bool compact = req_info->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+ struct port_reply_data *reply = PORT_REPDATA(reply_data);
+ int ret, port_type = ETHTOOL_PORT_TYPE_MDI;
+
+ if (nla_put_u32(skb, ETHTOOL_A_PORT_ID, reply->port_id))
+ return -EMSGSIZE;
+
+ if (!reply->mii) {
+ ret = ethnl_put_bitset(skb, ETHTOOL_A_PORT_SUPPORTED_MODES,
+ reply->supported, NULL,
+ __ETHTOOL_LINK_MODE_MASK_NBITS,
+ link_mode_names, compact);
+ if (ret < 0)
+ return ret;
+ } else {
+ ret = ethnl_put_bitset(skb, ETHTOOL_A_PORT_SUPPORTED_INTERFACES,
+ reply->interfaces, NULL,
+ PHY_INTERFACE_MODE_MAX,
+ phy_interface_names, compact);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (reply->mii || reply->sfp)
+ port_type = ETHTOOL_PORT_TYPE_SFP;
+
+ if (nla_put_u32(skb, ETHTOOL_A_PORT_TYPE, port_type))
+ return -EMSGSIZE;
+
+ if (reply->upstream_port &&
+ nla_put_u32(skb, ETHTOOL_A_PORT_UPSTREAM_PORT,
+ reply->upstream_port))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
+struct port_dump_ctx {
+ struct port_req_info *req_info;
+ struct port_reply_data *reply_data;
+ unsigned long ifindex;
+ unsigned long pos_ifindex;
+ unsigned long pos_portid;
+};
+
+static struct port_dump_ctx *
+port_dump_ctx_get(struct netlink_callback *cb)
+{
+ return (struct port_dump_ctx *)cb->ctx;
+}
+
+int ethnl_port_dump_start(struct netlink_callback *cb)
+{
+ const struct genl_dumpit_info *info = genl_dumpit_info(cb);
+ struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
+ struct nlattr **tb = info->info.attrs;
+ struct port_reply_data *reply_data;
+ struct port_req_info *req_info;
+ int ret;
+
+ BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
+
+ req_info = kzalloc_obj(*req_info);
+ if (!req_info)
+ return -ENOMEM;
+
+ reply_data = kmalloc_obj(*reply_data);
+ if (!reply_data) {
+ ret = -ENOMEM;
+ goto free_req_info;
+ }
+
+ ret = ethnl_parse_header_dev_get(&req_info->base, tb[ETHTOOL_A_PORT_HEADER],
+ genl_info_net(&info->info),
+ info->info.extack, false);
+ if (ret < 0)
+ goto free_rep_data;
+
+ ctx->ifindex = 0;
+ ctx->pos_ifindex = 0;
+
+ /* For filtered DUMP requests, let's just store the ifindex. We'll check
+ * again if the netdev is still there when looping over the netdev list
+ * in the DUMP loop.
+ */
+ if (req_info->base.dev) {
+ ctx->ifindex = req_info->base.dev->ifindex;
+ netdev_put(req_info->base.dev, &req_info->base.dev_tracker);
+ req_info->base.dev = NULL;
+ }
+
+ ctx->req_info = req_info;
+ ctx->reply_data = reply_data;
+
+ return 0;
+
+free_rep_data:
+ kfree(reply_data);
+free_req_info:
+ kfree(req_info);
+
+ return ret;
+}
+
+static int port_dump_one(struct sk_buff *skb, struct net_device *dev,
+ struct netlink_callback *cb)
+{
+ struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
+ void *ehdr;
+ int ret;
+
+ ehdr = ethnl_dump_put(skb, cb, ETHTOOL_MSG_PORT_GET_REPLY);
+ if (!ehdr)
+ return -EMSGSIZE;
+
+ memset(ctx->reply_data, 0, sizeof(struct port_reply_data));
+ ctx->reply_data->base.dev = dev;
+
+ rtnl_lock();
+ netdev_lock_ops(dev);
+
+ ret = port_prepare_data(&ctx->req_info->base, &ctx->reply_data->base,
+ genl_info_dump(cb));
+
+ netdev_unlock_ops(dev);
+ rtnl_unlock();
+
+ if (ret < 0)
+ goto out;
+
+ ret = ethnl_fill_reply_header(skb, dev, ETHTOOL_A_PORT_HEADER);
+ if (ret < 0)
+ goto out;
+
+ ret = port_fill_reply(skb, &ctx->req_info->base, &ctx->reply_data->base);
+
+out:
+ ctx->reply_data->base.dev = NULL;
+ if (ret < 0)
+ genlmsg_cancel(skb, ehdr);
+ else
+ genlmsg_end(skb, ehdr);
+
+ return ret;
+}
+
+static int port_dump_one_dev(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
+ struct net_device *dev;
+ struct phy_port *port;
+ int ret;
+
+ dev = ctx->req_info->base.dev;
+
+ if (!dev->link_topo)
+ return 0;
+
+ xa_for_each_start(&dev->link_topo->ports, ctx->pos_portid, port,
+ ctx->pos_portid) {
+ ctx->req_info->port_id = ctx->pos_portid;
+
+ ret = port_dump_one(skb, dev, cb);
+ if (ret == -ENODEV)
+ continue;
+ if (ret)
+ return ret;
+ }
+
+ ctx->pos_portid = 0;
+
+ return 0;
+}
+
+static int port_dump_all_dev(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
+ 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, ctx->pos_ifindex) {
+ netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
+ rcu_read_unlock();
+
+ ctx->req_info->base.dev = dev;
+ ret = port_dump_one_dev(skb, cb);
+
+ rcu_read_lock();
+ netdev_put(dev, &dev_tracker);
+ ctx->req_info->base.dev = NULL;
+
+ if (ret < 0 && ret != -EOPNOTSUPP) {
+ if (likely(skb->len))
+ ret = skb->len;
+ break;
+ }
+
+ ret = 0;
+ }
+ rcu_read_unlock();
+
+ return ret;
+}
+
+int ethnl_port_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ const struct genl_dumpit_info *info = genl_dumpit_info(cb);
+ struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
+ 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;
+
+ ctx->req_info->base.dev = dev;
+ ret = port_dump_one_dev(skb, cb);
+ if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
+ ret = skb->len;
+
+ netdev_put(dev, &dev_tracker);
+ } else {
+ ret = port_dump_all_dev(skb, cb);
+ }
+
+ return ret;
+}
+
+int ethnl_port_dump_done(struct netlink_callback *cb)
+{
+ struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
+
+ kfree(ctx->req_info);
+ kfree(ctx->reply_data);
+
+ return 0;
+}
+
+const struct ethnl_request_ops ethnl_port_request_ops = {
+ .request_cmd = ETHTOOL_MSG_PORT_GET,
+ .reply_cmd = ETHTOOL_MSG_PORT_GET_REPLY,
+ .hdr_attr = ETHTOOL_A_PORT_HEADER,
+ .req_info_size = sizeof(struct port_req_info),
+ .reply_data_size = sizeof(struct port_reply_data),
+
+ .parse_request = port_parse_request,
+ .prepare_data = port_prepare_data,
+ .reply_size = port_reply_size,
+ .fill_reply = port_fill_reply,
+};
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next v18 10/10] Documentation: networking: Update the phy_port infrastructure description
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (8 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports Maxime Chevallier
@ 2026-09-27 13:36 ` Maxime Chevallier
2026-09-28 10:07 ` Christophe Leroy (CS GROUP)
2026-09-28 17:31 ` [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Christophe Leroy (CS GROUP)
10 siblings, 1 reply; 31+ messages in thread
From: Maxime Chevallier @ 2026-09-27 13:36 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Christophe Leroy, Herve Codina, Florian Fainelli,
Vladimir Oltean, Köry Maincent, Marek Behún,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas,
Romain Gantois, Daniel Golle, Dimitri Fedrau, Frank Wunderlich,
Pietro Ameruoso, Aleksei Sviridkin
With SFP now properly supported with phy_port, add some details in the
documentation. Fix a typo along the way (driver -> driven).
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Aleksei Sviridkin <f@lex.la>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
Documentation/networking/phy-port.rst | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/Documentation/networking/phy-port.rst b/Documentation/networking/phy-port.rst
index 6e28d9094bce..2a9b1ec93b70 100644
--- a/Documentation/networking/phy-port.rst
+++ b/Documentation/networking/phy-port.rst
@@ -99,13 +99,29 @@ will eventually be able to report its own ksettings::
(_____)-----| Port |
+------+
+SFP ports
+=========
+
+SFP interfaces involve 2 distinct components, each represented by
+a :c:type:`struct phy_port <phy_port>` instance :
+
+ - The SFP cage itself is a :c:type:`struct phy_port <phy_port>`. It's special
+ in that it's not an MDI interface, but rather a hot-pluggable MII.
+ The :c:type:`struct phy_port <phy_port>` associated to it lists the different
+ MII interfaces we can use on the cage.
+
+ - The SFP module, when inserted, will also be associated to a
+ :c:type:`struct phy_port <phy_port>`, that represents the various linkmodes
+ that it gives access to. The module's :c:type:`struct phy_port <phy_port>`
+ doesn't supersede the cage's port, it references it through
+ the :c:type:`struct phy_port <phy_port>` :c:member:`upstream_port` field.
+
Next steps
==========
-As of writing this documentation, only ports controlled by PHY devices are
-supported. The next steps will be to add the Netlink API to expose these
-to userspace and add support for raw ports (controlled by some firmware, and directly
-managed by the NIC driver).
+As of writing this documentation, the port's presence and information can only
+be queried, and it's not possible to change any of the port's settings or select
+which one should be used.
Another parallel task is the introduction of a MII muxing framework to allow the
-control of non-PHY driver multi-port setups.
+control of non-PHY driven multi-port setups.
--
2.55.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc
2026-09-27 13:36 ` [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc Maxime Chevallier
@ 2026-09-28 9:36 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:36 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> The phy_link_topology structure stores information about the PHY-related
> components connected to a net_device. It is opportunistically allocated,
> when we add the first item to the topology, as this is not relevant for
> all kinds of net_devices.
>
> In preparation for the addition of phy_port tracking in the topology,
> let's make a dedicated helper for that allocation sequence.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> drivers/net/phy/phy_link_topology.c | 40 +++++++++++++++++++++++------
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 4134de7ae313..0462283c8020 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c
> @@ -28,11 +28,39 @@ static int netdev_alloc_phy_link_topology(struct net_device *dev)
> return 0;
> }
>
> +static struct phy_link_topology *phy_link_topo_get_or_alloc(struct net_device *dev)
> +{
> + int ret;
> +
> + if (dev->link_topo)
> + return dev->link_topo;
> +
> + /* The topology is allocated the first time we add an object to it.
> + * It is freed alongside the netdev. It can be called on multiple
> + * contexts:
> + * - It can be called from .probe() : No rtnl, no netdev_lock
> + * - .ndo_open() : rtnl and possibly netdev_lock
> + * - SFP state machine : rtnl held or not
> + *
> + * However, we can't really have races :
> + * - If we have a PHY, phy_link_topo_add_phy() will always run first
> + * and trigger the alloc. Only then the ports can be added through
> + * phylib or sfp.
> + * - If we don't, the SFP port for the cage is registered first, and
> + * only then other ports/PHYs can be registered.
> + */
> + ret = netdev_alloc_phy_link_topology(dev);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return dev->link_topo;
> +}
> +
> int phy_link_topo_add_phy(struct net_device *dev,
> struct phy_device *phy,
> enum phy_upstream upt, void *upstream)
> {
> - struct phy_link_topology *topo = dev->link_topo;
> + struct phy_link_topology *topo;
> struct phy_device_node *pdn;
> int ret;
>
> @@ -45,13 +73,9 @@ int phy_link_topo_add_phy(struct net_device *dev,
> if (WARN_ON_ONCE(netdev_need_ops_lock(dev)))
> return -EOPNOTSUPP;
>
> - if (!topo) {
> - ret = netdev_alloc_phy_link_topology(dev);
> - if (ret)
> - return ret;
> -
> - topo = dev->link_topo;
> - }
> + topo = phy_link_topo_get_or_alloc(dev);
> + if (IS_ERR(topo))
> + return PTR_ERR(topo);
>
> pdn = kzalloc_obj(*pdn);
> if (!pdn)
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology
2026-09-27 13:36 ` [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology Maxime Chevallier
@ 2026-09-28 9:37 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:37 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> phy_port is aimed at representing the various physical interfaces of a
> net_device. They can be controlled by various components in the link,
> such as the Ethernet PHY, the Ethernet MAC, and SFP module, etc.
>
> Let's therefore make so we keep track of all the ports connected to a
> netdev in phy_link_topology. The only ports added for now are phy-driven
> ports.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> drivers/net/phy/phy_device.c | 3 ++
> drivers/net/phy/phy_link_topology.c | 53 +++++++++++++++++++++++++++++
> include/linux/phy_link_topology.h | 18 ++++++++++
> include/linux/phy_port.h | 2 ++
> net/core/dev.c | 1 +
> 5 files changed, 77 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 5b13a74e2fa9..c4d2c03b8282 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1580,6 +1580,9 @@ static void phy_del_port(struct phy_device *phydev, struct phy_port *port)
>
> list_del(&port->head);
>
> + if (phydev->attached_dev)
> + phy_link_topo_del_port(phydev->attached_dev, port);
> +
> phydev->n_ports--;
> }
>
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 0462283c8020..28c5bf3a8849 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/phy_link_topology.h>
> +#include <linux/phy_port.h>
> #include <linux/phy.h>
> #include <linux/rtnetlink.h>
> #include <linux/xarray.h>
> @@ -23,6 +24,9 @@ static int netdev_alloc_phy_link_topology(struct net_device *dev)
> xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
> topo->next_phy_index = 1;
>
> + xa_init_flags(&topo->ports, XA_FLAGS_ALLOC1);
> + topo->next_port_index = 1;
> +
> dev->link_topo = topo;
>
> return 0;
> @@ -56,12 +60,45 @@ static struct phy_link_topology *phy_link_topo_get_or_alloc(struct net_device *d
> return dev->link_topo;
> }
>
> +int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port)
> +{
> + struct phy_link_topology *topo;
> + int ret;
> +
> + topo = phy_link_topo_get_or_alloc(dev);
> + if (IS_ERR(topo))
> + return PTR_ERR(topo);
> +
> + /* Attempt to re-use a previously allocated port_id */
> + if (port->id)
> + ret = xa_insert(&topo->ports, port->id, port, GFP_KERNEL);
> + else
> + ret = xa_alloc_cyclic(&topo->ports, &port->id, port,
> + xa_limit_32b, &topo->next_port_index,
> + GFP_KERNEL);
> +
> + return ret < 0 ? ret : 0;
> +}
> +EXPORT_SYMBOL_GPL(phy_link_topo_add_port);
> +
> +void phy_link_topo_del_port(struct net_device *dev, struct phy_port *port)
> +{
> + struct phy_link_topology *topo = dev->link_topo;
> +
> + if (!topo)
> + return;
> +
> + xa_erase(&topo->ports, port->id);
> +}
> +EXPORT_SYMBOL_GPL(phy_link_topo_del_port);
> +
> int phy_link_topo_add_phy(struct net_device *dev,
> struct phy_device *phy,
> enum phy_upstream upt, void *upstream)
> {
> struct phy_link_topology *topo;
> struct phy_device_node *pdn;
> + struct phy_port *port;
> int ret;
>
> /* ethtool ops may run without rtnl_lock, and rtnl_lock is what
> @@ -110,8 +147,20 @@ int phy_link_topo_add_phy(struct net_device *dev,
> if (ret < 0)
> goto err;
>
> + /* Add all the PHY's ports to the topology */
> + list_for_each_entry(port, &phy->ports, head) {
> + ret = phy_link_topo_add_port(dev, port);
> + if (ret)
> + goto del_ports;
> + }
> +
> return 0;
>
> +del_ports:
> + list_for_each_entry_continue_reverse(port, &phy->ports, head)
> + phy_link_topo_del_port(dev, port);
> +
> + xa_erase(&topo->phys, phy->phyindex);
> err:
> kfree(pdn);
> return ret;
> @@ -123,10 +172,14 @@ void phy_link_topo_del_phy(struct net_device *dev,
> {
> struct phy_link_topology *topo = dev->link_topo;
> struct phy_device_node *pdn;
> + struct phy_port *port;
>
> if (!topo)
> return;
>
> + list_for_each_entry(port, &phy->ports, head)
> + phy_link_topo_del_port(dev, port);
> +
> pdn = xa_erase(&topo->phys, phy->phyindex);
>
> /* We delete the PHY from the topology, however we don't re-set the
> diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
> index 95575f68d5bc..296ee514ba46 100644
> --- a/include/linux/phy_link_topology.h
> +++ b/include/linux/phy_link_topology.h
> @@ -16,11 +16,15 @@
>
> struct xarray;
> struct phy_device;
> +struct phy_port;
> struct sfp_bus;
>
> struct phy_link_topology {
> struct xarray phys;
> u32 next_phy_index;
> +
> + struct xarray ports;
> + u32 next_port_index;
> };
>
> struct phy_device_node {
> @@ -48,6 +52,9 @@ int phy_link_topo_add_phy(struct net_device *dev,
>
> void phy_link_topo_del_phy(struct net_device *dev, struct phy_device *phy);
>
> +int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port);
> +void phy_link_topo_del_port(struct net_device *dev, struct phy_port *port);
> +
> static inline struct phy_device *
> phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
> {
> @@ -77,6 +84,17 @@ static inline void phy_link_topo_del_phy(struct net_device *dev,
> {
> }
>
> +static inline int phy_link_topo_add_port(struct net_device *dev,
> + struct phy_port *port)
> +{
> + return 0;
> +}
> +
> +static inline void phy_link_topo_del_port(struct net_device *dev,
> + struct phy_port *port)
> +{
> +}
> +
> static inline struct phy_device *
> phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
> {
> diff --git a/include/linux/phy_port.h b/include/linux/phy_port.h
> index 0ef0f5ce4709..4e2a3fdd2f2e 100644
> --- a/include/linux/phy_port.h
> +++ b/include/linux/phy_port.h
> @@ -36,6 +36,7 @@ struct phy_port_ops {
> /**
> * struct phy_port - A representation of a network device physical interface
> *
> + * @id: Unique identifier for the port within the topology
> * @head: Used by the port's parent to list ports
> * @parent_type: The type of device this port is directly connected to
> * @phy: If the parent is PHY_PORT_PHYDEV, the PHY controlling that port
> @@ -52,6 +53,7 @@ struct phy_port_ops {
> * @is_sfp: Indicates if this port drives an SFP cage.
> */
> struct phy_port {
> + u32 id;
> struct list_head head;
> enum phy_port_parent parent_type;
> union {
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0db..97caa6846333 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -11378,6 +11378,7 @@ static void netdev_free_phy_link_topology(struct net_device *dev)
>
> if (IS_ENABLED(CONFIG_PHYLIB) && topo) {
> xa_destroy(&topo->phys);
> + xa_destroy(&topo->ports);
> kfree(topo);
> dev->link_topo = NULL;
> }
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages
2026-09-27 13:36 ` [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages Maxime Chevallier
@ 2026-09-28 9:38 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:38 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> phy_port tracks the interfaces that a netdevice feeds into. SFP cages are
> such ports, but so far we are only tracking the ones that are driven by
> PHYs acting as media-converters.
>
> Let's populate a phy_port for MAC driven SFP cages, handled by phylink.
>
> This phy_port represents the SFP cage itself, and not the module that
> may be plugged into it. It's therefore not an MDI interface, so only the
> 'interfaces' field is relevant here.
>
> The phy_port is only populated for 'NETDEV' phylink instances, as
> otherwise we don't have any topology to attach the port to.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> drivers/net/phy/phylink.c | 57 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index a7d086cdc9b2..19090916af75 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -14,6 +14,8 @@
> #include <linux/of_mdio.h>
> #include <linux/phy.h>
> #include <linux/phy_fixed.h>
> +#include <linux/phy_link_topology.h>
> +#include <linux/phy_port.h>
> #include <linux/phylink.h>
> #include <linux/rtnetlink.h>
> #include <linux/spinlock.h>
> @@ -93,6 +95,7 @@ struct phylink {
> DECLARE_PHY_INTERFACE_MASK(sfp_interfaces);
> __ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
> u8 sfp_port;
> + struct phy_port *sfp_cage_port;
>
> struct eee_config eee_cfg;
>
> @@ -1767,6 +1770,50 @@ static void phylink_fixed_poll(struct timer_list *t)
>
> static const struct sfp_upstream_ops sfp_phylink_ops;
>
> +static int phylink_create_sfp_cage_port(struct phylink *pl)
> +{
> + struct phy_port *port;
> + int ret = 0;
> +
> + if (!pl->netdev || !pl->sfp_bus)
> + return 0;
> +
> + port = phy_port_alloc();
> + if (!port)
> + return -ENOMEM;
> +
> + port->is_sfp = true;
> + port->is_mii = true;
> + port->active = true;
> +
> + phy_interface_and(port->interfaces, pl->config->supported_interfaces,
> + phylink_sfp_interfaces);
> + phy_port_update_supported(port);
> +
> + ret = phy_link_topo_add_port(pl->netdev, port);
> + if (ret)
> + phy_port_destroy(port);
> + else
> + pl->sfp_cage_port = port;
> +
> + return ret;
> +}
> +
> +static void phylink_destroy_sfp_cage_port(struct phylink *pl)
> +{
> + if (pl->netdev && pl->sfp_cage_port) {
> + /* The port may still be accessed under RTNL */
> + rtnl_lock();
> + phy_link_topo_del_port(pl->netdev, pl->sfp_cage_port);
> + rtnl_unlock();
> + }
> +
> + if (pl->sfp_cage_port)
> + phy_port_destroy(pl->sfp_cage_port);
> +
> + pl->sfp_cage_port = NULL;
> +}
> +
> static int phylink_register_sfp(struct phylink *pl,
> const struct fwnode_handle *fwnode)
> {
> @@ -1784,9 +1831,18 @@ static int phylink_register_sfp(struct phylink *pl,
>
> pl->sfp_bus = bus;
>
> + ret = phylink_create_sfp_cage_port(pl);
> + if (ret) {
> + sfp_bus_put(bus);
> + return ret;
> + }
> +
> ret = sfp_bus_add_upstream(bus, pl, &sfp_phylink_ops);
> sfp_bus_put(bus);
>
> + if (ret)
> + phylink_destroy_sfp_cage_port(pl);
> +
> return ret;
> }
>
> @@ -2069,6 +2125,7 @@ EXPORT_SYMBOL_GPL(phylink_create);
> void phylink_destroy(struct phylink *pl)
> {
> sfp_bus_del_upstream(pl->sfp_bus);
> + phylink_destroy_sfp_cage_port(pl);
> if (pl->link_gpio)
> gpiod_put(pl->link_gpio);
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream
2026-09-27 13:36 ` [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream Maxime Chevallier
@ 2026-09-28 9:40 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:40 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> When dealing with PHY-driven SFP, we create a phy_port representing the
> SFP bus when we know we have such a bus.
>
> We can move the port creation before registering the sfp upstream ops,
> as long as we know the SFP bus is there. This will allow passing the
> phy_port along with the upstream information to the SFP bus.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> drivers/net/phy/phy_device.c | 58 +++++++++++++++++++++++-------------
> 1 file changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index c4d2c03b8282..3c20d4cc1a79 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1586,13 +1586,13 @@ static void phy_del_port(struct phy_device *phydev, struct phy_port *port)
> phydev->n_ports--;
> }
>
> -static int phy_setup_sfp_port(struct phy_device *phydev)
> +static struct phy_port *phy_setup_sfp_port(struct phy_device *phydev)
> {
> struct phy_port *port = phy_port_alloc();
> int ret;
>
> if (!port)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
>
> port->parent_type = PHY_PORT_PHY;
> port->phy = phydev;
> @@ -1607,10 +1607,12 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
> * when attaching the port to the phydev.
> */
> ret = phy_add_port(phydev, port);
> - if (ret)
> + if (ret) {
> phy_port_destroy(port);
> + return ERR_PTR(ret);
> + }
>
> - return ret;
> + return port;
> }
>
> /**
> @@ -1619,30 +1621,46 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
> */
> static int phy_sfp_probe(struct phy_device *phydev)
> {
> + struct phy_port *port = NULL;
> struct sfp_bus *bus;
> - int ret = 0;
> + int ret;
>
> - if (phydev->mdio.dev.fwnode) {
> - bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
> - if (IS_ERR(bus))
> - return PTR_ERR(bus);
> + if (!phydev->mdio.dev.fwnode)
> + return 0;
>
> - phydev->sfp_bus = bus;
> + bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
> + if (IS_ERR(bus))
> + return PTR_ERR(bus);
>
> - ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
> - sfp_bus_put(bus);
> + phydev->sfp_bus = bus;
>
> - if (ret)
> - phydev->sfp_bus = NULL;
> + if (bus) {
> + port = phy_setup_sfp_port(phydev);
> + if (IS_ERR(port)) {
> + ret = PTR_ERR(port);
> + goto out_sfp;
> + }
> }
>
> - if (!ret && phydev->sfp_bus) {
> - ret = phy_setup_sfp_port(phydev);
> - if (ret) {
> - sfp_bus_del_upstream(phydev->sfp_bus);
> - phydev->sfp_bus = NULL;
> - }
> + ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
> + if (ret)
> + goto out_port;
> +
> + /* sfp_bus_add_upstream() grabs a ref to the sfp bus on success, it's
> + * safe to release it now.
> + */
> + sfp_bus_put(bus);
> +
> + return ret;
> +
> +out_port:
> + if (port) {
> + phy_del_port(phydev, port);
> + phy_port_destroy(port);
> }
> +out_sfp:
> + sfp_bus_put(bus);
> + phydev->sfp_bus = NULL;
>
> return ret;
> }
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port
2026-09-27 13:36 ` [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port Maxime Chevallier
@ 2026-09-28 9:42 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:42 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> Now that the SFP bus infrastructure notifies when PHY-less modules are
> connected, we can create a phy_port to represent it. Instead of letting
> the SFP subsystem handle that, the Bus' upstream is in charge of
> maintaining that phy_port and register it to the topology, as the
> upstream (in this case a phy device) is directly interacting with the
> underlying net_device.
>
> Add a phy_caps helper to get the achievable modes on this module based
> on what the phy_port representing the bus supports.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> drivers/net/phy/phy-caps.h | 2 +
> drivers/net/phy/phy_caps.c | 26 +++++++++
> drivers/net/phy/phy_device.c | 100 +++++++++++++++++++++++++++++++++--
> drivers/net/phy/phylink.c | 76 ++++++++++++++++++++++++--
> include/linux/phy.h | 6 +++
> 5 files changed, 203 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h
> index 421088e6f6e8..ec3d39a0ae06 100644
> --- a/drivers/net/phy/phy-caps.h
> +++ b/drivers/net/phy/phy-caps.h
> @@ -66,5 +66,7 @@ void phy_caps_medium_get_supported(unsigned long *supported,
> enum ethtool_link_medium medium,
> int lanes);
> u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes);
> +void phy_caps_linkmode_filter_ifaces(unsigned long *to, const unsigned long *from,
> + const unsigned long *interfaces);
>
> #endif /* __PHY_CAPS_H */
> diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
> index 942d43191561..558e4df4d63c 100644
> --- a/drivers/net/phy/phy_caps.c
> +++ b/drivers/net/phy/phy_caps.c
> @@ -445,3 +445,29 @@ u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes)
> return mediums;
> }
> EXPORT_SYMBOL_GPL(phy_caps_mediums_from_linkmodes);
> +
> +/**
> + * phy_caps_linkmode_filter_ifaces() - Filter linkmodes with an interface list
> + * @to: Stores the filtered linkmodes
> + * @from: Linkmodes to filter
> + * @interfaces: Bitfield of phy_interface_t that we use for filtering
> + *
> + * Filter the provided linkmodes, only to keep the ones we can possibly achieve
> + * when using any of the provided MII interfaces.
> + */
> +void phy_caps_linkmode_filter_ifaces(unsigned long *to,
> + const unsigned long *from,
> + const unsigned long *interfaces)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(ifaces_supported) = {};
> + unsigned int ifaces_caps = 0;
> + phy_interface_t interface;
> +
> + for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
> + ifaces_caps |= phy_caps_from_interface(interface);
> +
> + phy_caps_linkmodes(ifaces_caps, ifaces_supported);
> +
> + linkmode_and(to, from, ifaces_supported);
> +}
> +EXPORT_SYMBOL_GPL(phy_caps_linkmode_filter_ifaces);
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 3c20d4cc1a79..ab9631b161bc 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1400,11 +1400,21 @@ static int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
> {
> struct phy_device *phydev = upstream;
> struct net_device *dev = phydev->attached_dev;
> + int ret;
>
> - if (dev)
> - return phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev);
> + phydev->has_sfp_mod_phy = true;
>
> - return 0;
> + /* If we aren't attached to a netdev, we can't add the SFP PHY to its
> + * topology.
> + */
> + if (!dev)
> + return 0;
> +
> + ret = phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev);
> + if (ret)
> + phydev->has_sfp_mod_phy = false;
> +
> + return ret;
> }
>
> /**
> @@ -1422,6 +1432,8 @@ static void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
> struct phy_device *phydev = upstream;
> struct net_device *dev = phydev->attached_dev;
>
> + phydev->has_sfp_mod_phy = false;
> +
> if (dev)
> phy_link_topo_del_phy(dev, phy);
> }
> @@ -1527,6 +1539,75 @@ static void phy_sfp_link_down(void *upstream)
> port->ops->link_down(port);
> }
>
> +static int phy_add_sfp_mod_port(struct phy_device *phydev)
> +{
> + const struct sfp_module_caps *caps;
> + struct phy_port *port;
> + int ret = 0;
> +
> + /* Create mod port */
> + port = phy_port_alloc();
> + if (!port)
> + return -ENOMEM;
> +
> + port->active = true;
> +
> + caps = sfp_get_module_caps(phydev->sfp_bus);
> +
> + phy_caps_linkmode_filter_ifaces(port->supported, caps->link_modes,
> + phydev->sfp_cage_port->interfaces);
> +
> + if (phydev->attached_dev) {
> + ret = phy_link_topo_add_port(phydev->attached_dev, port);
> + if (ret) {
> + phy_port_destroy(port);
> + return ret;
> + }
> + }
> +
> + /* we don't use phy_add_port() here as the module port isn't a direct
> + * interface from the PHY, but rather an extension to the sfp-bus, that
> + * is already represented by its own phy_port
> + */
> + phydev->mod_port = port;
> +
> + return 0;
> +}
> +
> +static void phy_del_sfp_mod_port(struct phy_device *phydev)
> +{
> + if (!phydev->mod_port)
> + return;
> +
> + if (phydev->attached_dev)
> + phy_link_topo_del_port(phydev->attached_dev, phydev->mod_port);
> +
> + phy_port_destroy(phydev->mod_port);
> + phydev->mod_port = NULL;
> +}
> +
> +static int phy_sfp_module_start(void *upstream)
> +{
> + struct phy_device *phydev = upstream;
> +
> + /* If there's a downstream SFP module, and it doesn't contain a PHY
> + * device, let's create a phy_port to represent that module.
> + */
> + if (!phydev->has_sfp_mod_phy)
> + return phy_add_sfp_mod_port(phydev);
> +
> + return 0;
> +}
> +
> +static void phy_sfp_module_stop(void *upstream)
> +{
> + struct phy_device *phydev = upstream;
> +
> + /* Called upon module removal or upstream removal */
> + if (!phydev->has_sfp_mod_phy)
> + phy_del_sfp_mod_port(phydev);
> +}
> +
> static const struct sfp_upstream_ops sfp_phydev_ops = {
> .attach = phy_sfp_attach,
> .detach = phy_sfp_detach,
> @@ -1536,6 +1617,8 @@ static const struct sfp_upstream_ops sfp_phydev_ops = {
> .link_down = phy_sfp_link_down,
> .connect_phy = phy_sfp_connect_phy,
> .disconnect_phy = phy_sfp_disconnect_phy,
> + .module_start = phy_sfp_module_start,
> + .module_stop = phy_sfp_module_stop,
> };
>
> static int phy_add_port(struct phy_device *phydev, struct phy_port *port)
> @@ -1638,6 +1721,7 @@ static int phy_sfp_probe(struct phy_device *phydev)
> port = phy_setup_sfp_port(phydev);
> if (IS_ERR(port)) {
> ret = PTR_ERR(port);
> + port = NULL;
> goto out_sfp;
> }
> }
> @@ -1651,6 +1735,8 @@ static int phy_sfp_probe(struct phy_device *phydev)
> */
> sfp_bus_put(bus);
>
> + phydev->sfp_cage_port = port;
> +
> return ret;
>
> out_port:
> @@ -1744,6 +1830,8 @@ static void phy_detach_internal(struct phy_device *phydev, bool notify_bus)
> phydev->attached_dev->phydev = NULL;
> phydev->attached_dev = NULL;
> phy_link_topo_del_phy(dev, phydev);
> + if (phydev->mod_port)
> + phy_link_topo_del_port(dev, phydev->mod_port);
> }
>
> phydev->phy_link_change = NULL;
> @@ -1873,6 +1961,12 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
> err = phy_link_topo_add_phy(dev, phydev, PHY_UPSTREAM_MAC, dev);
> if (err)
> goto error;
> +
> + if (phydev->mod_port) {
> + err = phy_link_topo_add_port(dev, phydev->mod_port);
> + if (err)
> + goto error;
> + }
> }
>
> /* Some Ethernet drivers try to connect to a PHY device before
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 19090916af75..db05d2dda429 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -96,6 +96,7 @@ struct phylink {
> __ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
> u8 sfp_port;
> struct phy_port *sfp_cage_port;
> + struct phy_port *mod_port;
>
> struct eee_config eee_cfg;
>
> @@ -1792,10 +1793,15 @@ static int phylink_create_sfp_cage_port(struct phylink *pl)
>
> ret = phy_link_topo_add_port(pl->netdev, port);
> if (ret)
> - phy_port_destroy(port);
> - else
> - pl->sfp_cage_port = port;
> + goto out_destroy_port;
> +
> + pl->sfp_cage_port = port;
> +
> + return 0;
>
> +out_destroy_port:
> + phy_port_destroy(port);
> + pl->sfp_cage_port = NULL;
> return ret;
> }
>
> @@ -4012,14 +4018,65 @@ static void phylink_sfp_module_remove(void *upstream)
> phy_interface_zero(pl->sfp_interfaces);
> }
>
> +static int phylink_add_sfp_mod_port(struct phylink *pl)
> +{
> + const struct sfp_module_caps *caps;
> + struct phy_port *port;
> + int ret = 0;
> +
> + if (!pl->sfp_cage_port)
> + return 0;
> +
> + /* Create mod port */
> + port = phy_port_alloc();
> + if (!port)
> + return -ENOMEM;
> +
> + port->active = true;
> +
> + caps = sfp_get_module_caps(pl->sfp_bus);
> +
> + phy_caps_linkmode_filter_ifaces(port->supported, caps->link_modes,
> + pl->sfp_cage_port->interfaces);
> +
> + if (pl->netdev) {
> + ret = phy_link_topo_add_port(pl->netdev, port);
> + if (ret) {
> + phy_port_destroy(port);
> + return ret;
> + }
> + }
> +
> + pl->mod_port = port;
> +
> + return 0;
> +}
> +
> +static void phylink_del_sfp_mod_port(struct phylink *pl)
> +{
> + if (!pl->mod_port)
> + return;
> +
> + if (pl->netdev)
> + phy_link_topo_del_port(pl->netdev, pl->mod_port);
> +
> + phy_port_destroy(pl->mod_port);
> + pl->mod_port = NULL;
> +}
> +
> static int phylink_sfp_module_start(void *upstream)
> {
> struct phylink *pl = upstream;
> + int ret;
>
> /* If this SFP module has a PHY, start the PHY now. */
> if (pl->phydev) {
> phy_start(pl->phydev);
> return 0;
> + } else {
> + ret = phylink_add_sfp_mod_port(pl);
> + if (ret)
> + return ret;
> }
>
> /* If the module may have a PHY but we didn't detect one we
> @@ -4028,7 +4085,16 @@ static int phylink_sfp_module_start(void *upstream)
> if (!pl->sfp_may_have_phy)
> return 0;
>
> - return phylink_sfp_config_optical(pl);
> + ret = phylink_sfp_config_optical(pl);
> + if (ret)
> + goto del_mod_port;
> +
> + return 0;
> +
> +del_mod_port:
> + phylink_del_sfp_mod_port(pl);
> +
> + return ret;
> }
>
> static void phylink_sfp_module_stop(void *upstream)
> @@ -4038,6 +4104,8 @@ static void phylink_sfp_module_stop(void *upstream)
> /* If this SFP module has a PHY, stop it. */
> if (pl->phydev)
> phy_stop(pl->phydev);
> + else
> + phylink_del_sfp_mod_port(pl);
> }
>
> static void phylink_sfp_link_down(void *upstream)
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 7c5098a0dd6c..dbd6ed5864f2 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -611,7 +611,10 @@ struct phy_oatc14_sqi_capability {
> * @irq: IRQ number of the PHY's interrupt (-1 if none)
> * @phylink: Pointer to phylink instance for this PHY
> * @sfp_bus_attached: Flag indicating whether the SFP bus has been attached
> + * @has_sfp_mod_phy: Set true if downstream SFP bus's module contains a PHY
> * @sfp_bus: SFP bus attached to this PHY's fiber port
> + * @sfp_cage_port: The phy_port connected to the downstream SFP cage
> + * @mod_port: phy_port representing the SFP module, if it is phy-less
> * @attached_dev: The attached enet driver's device instance ptr
> * @adjust_link: Callback for the enet controller to respond to changes: in the
> * link state.
> @@ -802,7 +805,10 @@ struct phy_device {
>
> /* This may be modified under the rtnl lock */
> bool sfp_bus_attached;
> + bool has_sfp_mod_phy;
> struct sfp_bus *sfp_bus;
> + struct phy_port *sfp_cage_port;
> + struct phy_port *mod_port;
> struct phylink *phylink;
> struct net_device *attached_dev;
> struct mii_timestamper *mii_ts;
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream
2026-09-27 13:36 ` [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream Maxime Chevallier
@ 2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:43 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> MII phy_ports are not meant to be connected directly to a link partner.
> They are meant to feed into some media converter devices that will
> expose an MDI phy_port, so far we only support SFP modules for that.
>
> In the case an MDI phy_port is backed by an MII port (e.g. a SFP
> module's port, backed by the SFP cage port), let's keep track of the
> port id of the MII port backing it.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> drivers/net/phy/phy_device.c | 29 +++++++++++++++++++++++++++--
> drivers/net/phy/phylink.c | 5 +++++
> include/linux/phy.h | 4 ++++
> include/linux/phy_port.h | 3 +++
> 4 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index ab9631b161bc..2bb4abd0d392 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1403,6 +1403,7 @@ static int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
> int ret;
>
> phydev->has_sfp_mod_phy = true;
> + phy_set_upstream_port(phy, phydev->sfp_cage_port);
>
> /* If we aren't attached to a netdev, we can't add the SFP PHY to its
> * topology.
> @@ -1436,6 +1437,8 @@ static void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
>
> if (dev)
> phy_link_topo_del_phy(dev, phy);
> +
> + phy_set_upstream_port(phy, NULL);
> }
>
> /**
> @@ -1571,6 +1574,8 @@ static int phy_add_sfp_mod_port(struct phy_device *phydev)
> */
> phydev->mod_port = port;
>
> + port->upstream_port = phydev->sfp_cage_port;
> +
> return 0;
> }
>
> @@ -1726,6 +1731,8 @@ static int phy_sfp_probe(struct phy_device *phydev)
> }
> }
>
> + phydev->sfp_cage_port = port;
> +
> ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
> if (ret)
> goto out_port;
> @@ -1735,14 +1742,13 @@ static int phy_sfp_probe(struct phy_device *phydev)
> */
> sfp_bus_put(bus);
>
> - phydev->sfp_cage_port = port;
> -
> return ret;
>
> out_port:
> if (port) {
> phy_del_port(phydev, port);
> phy_port_destroy(port);
> + phydev->sfp_cage_port = NULL;
> }
> out_sfp:
> sfp_bus_put(bus);
> @@ -3687,6 +3693,25 @@ struct phy_port *phy_get_sfp_port(struct phy_device *phydev)
> }
> EXPORT_SYMBOL_GPL(phy_get_sfp_port);
>
> +/**
> + * phy_set_upstream_port() - Sets the phy_port controlling the MII this PHY is
> + * attached to.
> + * @phydev: pointer to the PHY device we set the upstream of.
> + * @port: The phy_port upstream of this PHY, can be NULL.
> + */
> +void phy_set_upstream_port(struct phy_device *phydev, struct phy_port *port)
> +{
> + struct phy_port *local_port;
> +
> + ASSERT_RTNL();
> +
> + phydev->upstream_port = port;
> +
> + phy_for_each_port(phydev, local_port)
> + local_port->upstream_port = port;
> +}
> +EXPORT_SYMBOL_GPL(phy_set_upstream_port);
> +
> /**
> * fwnode_mdio_find_device - Given a fwnode, find the mdio_device
> * @fwnode: pointer to the mdio_device's fwnode
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index db05d2dda429..2583e913cff6 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -4047,6 +4047,8 @@ static int phylink_add_sfp_mod_port(struct phylink *pl)
> }
> }
>
> + port->upstream_port = pl->sfp_cage_port;
> +
> pl->mod_port = port;
>
> return 0;
> @@ -4150,6 +4152,8 @@ static int phylink_sfp_connect_phy(void *upstream, struct phy_device *phy)
> phy_interface_and(phy->host_interfaces, phylink_sfp_interfaces,
> pl->config->supported_interfaces);
>
> + phy_set_upstream_port(phy, pl->sfp_cage_port);
> +
> /* Do the initial configuration */
> return phylink_sfp_config_phy(pl, phy);
> }
> @@ -4158,6 +4162,7 @@ static void phylink_sfp_disconnect_phy(void *upstream,
> struct phy_device *phydev)
> {
> phylink_disconnect_phy(upstream);
> + phy_set_upstream_port(phydev, NULL);
> }
>
> static const struct sfp_upstream_ops sfp_phylink_ops = {
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index dbd6ed5864f2..1fb87d12367b 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -615,6 +615,7 @@ struct phy_oatc14_sqi_capability {
> * @sfp_bus: SFP bus attached to this PHY's fiber port
> * @sfp_cage_port: The phy_port connected to the downstream SFP cage
> * @mod_port: phy_port representing the SFP module, if it is phy-less
> + * @upstream_port: phy_port this PHY's MII attaches to, if any
> * @attached_dev: The attached enet driver's device instance ptr
> * @adjust_link: Callback for the enet controller to respond to changes: in the
> * link state.
> @@ -809,6 +810,7 @@ struct phy_device {
> struct sfp_bus *sfp_bus;
> struct phy_port *sfp_cage_port;
> struct phy_port *mod_port;
> + struct phy_port *upstream_port;
> struct phylink *phylink;
> struct net_device *attached_dev;
> struct mii_timestamper *mii_ts;
> @@ -2493,6 +2495,8 @@ int __phy_hwtstamp_set(struct phy_device *phydev,
>
> struct phy_port *phy_get_sfp_port(struct phy_device *phydev);
>
> +void phy_set_upstream_port(struct phy_device *phydev, struct phy_port *port);
> +
> /**
> * phy_module_driver() - Helper macro for registering PHY drivers
> * @__phy_drivers: array of PHY drivers to register
> diff --git a/include/linux/phy_port.h b/include/linux/phy_port.h
> index 4e2a3fdd2f2e..8f45c031cbc1 100644
> --- a/include/linux/phy_port.h
> +++ b/include/linux/phy_port.h
> @@ -40,6 +40,8 @@ struct phy_port_ops {
> * @head: Used by the port's parent to list ports
> * @parent_type: The type of device this port is directly connected to
> * @phy: If the parent is PHY_PORT_PHYDEV, the PHY controlling that port
> + * @upstream_port: Indicates the MII port that feeds this port, if any,
> + * e.g. the SFP cage port for a SFP module port.
> * @ops: Callback ops implemented by the port controller
> * @pairs: The number of pairs this port has, 0 if not applicable
> * @mediums: Bitmask of the physical mediums this port provides access to
> @@ -59,6 +61,7 @@ struct phy_port {
> union {
> struct phy_device *phy;
> };
> + struct phy_port *upstream_port;
>
> const struct phy_port_ops *ops;
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports
2026-09-27 13:36 ` [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports Maxime Chevallier
@ 2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:43 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> In order to allow netlink access to phy_ports, let's add a helper to
> retrieve them. When handling a port coming from phy_link_topology, the
> caller must hold rtnl until it's done with it.
>
> We don't really support PHY and phy_port operation on ops-locked devices
> yet, warn when we try to do so on such an interface.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> drivers/net/phy/phy_link_topology.c | 8 ++++++++
> include/linux/phy_link_topology.h | 25 +++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 28c5bf3a8849..214ccce23cf6 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c
> @@ -65,6 +65,14 @@ int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port)
> struct phy_link_topology *topo;
> int ret;
>
> + /* Ports can now be queried without rtnl for ops-locked devices, which
> + * we don't support now as port retrieval is done under rtnl.
> + * We don't have phy_port enabled devices yet, let's make sure
> + * we are loudly warned about that when it happens.
> + */
> + if (WARN_ON_ONCE(dev->netdev_ops && netdev_need_ops_lock(dev)))
> + return -EOPNOTSUPP;
> +
> topo = phy_link_topo_get_or_alloc(dev);
> if (IS_ERR(topo))
> return PTR_ERR(topo);
> diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
> index 296ee514ba46..f2e493c433ff 100644
> --- a/include/linux/phy_link_topology.h
> +++ b/include/linux/phy_link_topology.h
> @@ -13,6 +13,7 @@
>
> #include <linux/ethtool.h>
> #include <linux/netdevice.h>
> +#include <linux/rtnetlink.h>
>
> struct xarray;
> struct phy_device;
> @@ -71,6 +72,24 @@ phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
> return NULL;
> }
>
> +static inline struct phy_port *
> +phy_link_topo_get_port(struct net_device *dev, u32 port_id)
> +{
> + struct phy_link_topology *topo = dev->link_topo;
> +
> + if (!topo)
> + return NULL;
> +
> + /* ports in the topo are RTNL protected, but may be accessed under
> + * netdev_lock for ops-locked devices. For now keep them under rtnl
> + * protection, as no ops-locked devices have phy_port capabilities
> + * yet.
> + */
> + ASSERT_RTNL();
> +
> + return xa_load(&topo->ports, port_id);
> +}
> +
> #else
> static inline int phy_link_topo_add_phy(struct net_device *dev,
> struct phy_device *phy,
> @@ -100,6 +119,12 @@ phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
> {
> return NULL;
> }
> +
> +static inline struct phy_port *
> +phy_link_topo_get_port(struct net_device *dev, u32 port_id)
> +{
> + return NULL;
> +}
> #endif
>
> #endif /* __PHY_LINK_TOPOLOGY_H */
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool
2026-09-27 13:36 ` [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool Maxime Chevallier
@ 2026-09-28 9:47 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:47 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> Ethernet network interfaces may have more than one front-facing port.
> The phy_port infrastructure was introduced to keep track of
> these ports, and allow userspace to know about the presence and
> capability of these ports. Add a ethnl netlink message to report this
> information.
>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
I'm not an expert in documentation language, but the content makes sense
and looks consistant.
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> Documentation/netlink/specs/ethtool.yaml | 55 +++++++++++++++++++
> Documentation/networking/ethtool-netlink.rst | 36 ++++++++++++
> .../uapi/linux/ethtool_netlink_generated.h | 22 ++++++++
> 3 files changed, 113 insertions(+)
>
> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index 5dd4d1b5d94b..35e028c0be29 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml
> @@ -210,6 +210,11 @@ definitions:
> -
> name: discard
> value: 31
> + -
> + name: port-type
> + doc: type of physical connector
> + type: enum
> + entries: [mdi, sfp]
>
> attribute-sets:
> -
> @@ -1905,6 +1910,32 @@ attribute-sets:
> name: link
> type: nest
> nested-attributes: mse-snapshot
> + -
> + name: port
> + attr-cnt-name: --ethtool-a-port-cnt
> + attributes:
> + -
> + name: header
> + type: nest
> + nested-attributes: header
> + -
> + name: id
> + type: u32
> + -
> + name: supported-modes
> + type: nest
> + nested-attributes: bitset
> + -
> + name: supported-interfaces
> + type: nest
> + nested-attributes: bitset
> + -
> + name: type
> + type: u32
> + enum: port-type
> + -
> + name: upstream-port
> + type: u32
>
> operations:
> enum-model: directional
> @@ -2859,6 +2890,30 @@ operations:
> - worst-channel
> - link
> dump: *mse-get-op
> + -
> + name: port-get
> + doc: Get ports attached to an interface
> +
> + attribute-set: port
> +
> + do:
> + request:
> + attributes:
> + - header
> + - id
> + reply: &port-get-op-reply
> + attributes:
> + - header
> + - id
> + - supported-modes
> + - supported-interfaces
> + - type
> + - upstream-port
> + dump:
> + request:
> + attributes:
> + - header
> + reply: *port-get-op-reply
>
> mcast-groups:
> list:
> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> index 393f2b6d3863..eba226c79ec3 100644
> --- a/Documentation/networking/ethtool-netlink.rst
> +++ b/Documentation/networking/ethtool-netlink.rst
> @@ -247,6 +247,7 @@ Userspace to kernel:
> ``ETHTOOL_MSG_RSS_CREATE_ACT`` create an additional RSS context
> ``ETHTOOL_MSG_RSS_DELETE_ACT`` delete an additional RSS context
> ``ETHTOOL_MSG_MSE_GET`` get MSE diagnostic data
> + ``ETHTOOL_MSG_PORT_GET`` get port information
> ===================================== =================================
>
> Kernel to userspace:
> @@ -305,6 +306,7 @@ Kernel to userspace:
> ``ETHTOOL_MSG_RSS_CREATE_NTF`` additional RSS context created
> ``ETHTOOL_MSG_RSS_DELETE_NTF`` additional RSS context deleted
> ``ETHTOOL_MSG_MSE_GET_REPLY`` MSE diagnostic data
> + ``ETHTOOL_MSG_PORT_GET_REPLY`` port information
> ======================================== =================================
>
> ``GET`` requests are sent by userspace applications to retrieve device
> @@ -2537,6 +2539,39 @@ Within each channel nest, only the metrics supported by the PHY will be present.
> See ``struct phy_mse_snapshot`` kernel documentation in
> ``include/linux/phy.h``.
>
> +PORT_GET
> +========
> +
> +Retrieve information about the physical connection points of a network device,
> +referred to as "ports". User needs to specify a PORT_ID for the DO operation,
> +in which case the DO request returns information about that specific port.
> +
> +As there can be more than one port, the DUMP operation can be used to list the
> +ports present on a given interface, by passing an interface index or name in
> +the dump request.
> +
> +Request contents:
> +
> + ===================================== ====== ===============================
> + ``ETHTOOL_A_PORT_HEADER`` nested request header
> + ``ETHTOOL_A_PORT_ID`` u32 port id
> + ===================================== ====== ===============================
> +
> +Kernel response contents:
> +
> + ======================================= ====== =============================
> + ``ETHTOOL_A_PORT_HEADER`` nested request header
> + ``ETHTOOL_A_PORT_ID`` u32 the port's unique identifier,
> + per netdevice.
> + ``ETHTOOL_A_PORT_SUPPORTED_MODES`` bitset bitset of supported linkmodes
> + ``ETHTOOL_A_PORT_SUPPORTED_INTERFACES`` bitset bitset of supported MII
> + interfaces
> + ``ETHTOOL_A_PORT_TYPE`` u32 the port type
> + ``ETHTOOL_A_PORT_UPSTREAM_PORT`` u32 If any, the index of the MII
> + port that feeds into this
> + port.
> + ======================================= ====== =============================
> +
> Request translation
> ===================
>
> @@ -2647,4 +2682,5 @@ are netlink only.
> n/a ``ETHTOOL_MSG_PHY_GET``
> ``SIOCGHWTSTAMP`` ``ETHTOOL_MSG_TSCONFIG_GET``
> ``SIOCSHWTSTAMP`` ``ETHTOOL_MSG_TSCONFIG_SET``
> + n/a ``ETHTOOL_MSG_PORT_GET``
> =================================== =====================================
> diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
> index 8134baf7860f..660e0c08a766 100644
> --- a/include/uapi/linux/ethtool_netlink_generated.h
> +++ b/include/uapi/linux/ethtool_netlink_generated.h
> @@ -78,6 +78,14 @@ enum ethtool_pse_event {
> ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR = 64,
> };
>
> +/*
> + * type of physical connector
> + */
> +enum ethtool_port_type {
> + ETHTOOL_PORT_TYPE_MDI,
> + ETHTOOL_PORT_TYPE_SFP,
> +};
> +
> enum {
> ETHTOOL_A_HEADER_UNSPEC,
> ETHTOOL_A_HEADER_DEV_INDEX,
> @@ -840,6 +848,18 @@ enum {
> ETHTOOL_A_MSE_MAX = (__ETHTOOL_A_MSE_CNT - 1)
> };
>
> +enum {
> + ETHTOOL_A_PORT_HEADER = 1,
> + ETHTOOL_A_PORT_ID,
> + ETHTOOL_A_PORT_SUPPORTED_MODES,
> + ETHTOOL_A_PORT_SUPPORTED_INTERFACES,
> + ETHTOOL_A_PORT_TYPE,
> + ETHTOOL_A_PORT_UPSTREAM_PORT,
> +
> + __ETHTOOL_A_PORT_CNT,
> + ETHTOOL_A_PORT_MAX = (__ETHTOOL_A_PORT_CNT - 1)
> +};
> +
> enum {
> ETHTOOL_MSG_USER_NONE = 0,
> ETHTOOL_MSG_STRSET_GET = 1,
> @@ -893,6 +913,7 @@ enum {
> ETHTOOL_MSG_RSS_CREATE_ACT,
> ETHTOOL_MSG_RSS_DELETE_ACT,
> ETHTOOL_MSG_MSE_GET,
> + ETHTOOL_MSG_PORT_GET,
>
> __ETHTOOL_MSG_USER_CNT,
> ETHTOOL_MSG_USER_MAX = (__ETHTOOL_MSG_USER_CNT - 1)
> @@ -954,6 +975,7 @@ enum {
> ETHTOOL_MSG_RSS_CREATE_NTF,
> ETHTOOL_MSG_RSS_DELETE_NTF,
> ETHTOOL_MSG_MSE_GET_REPLY,
> + ETHTOOL_MSG_PORT_GET_REPLY,
>
> __ETHTOOL_MSG_KERNEL_CNT,
> ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports
2026-09-27 13:36 ` [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports Maxime Chevallier
@ 2026-09-28 9:48 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 9:48 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> Expose the phy_port information to userspace, so that we can know how
> many ports are available on a given interface, as well as their
> capabilities. For MDI ports, we report the list of supported linkmodes
> based on what the PHY that drives this port says.
> For MII ports, i.e. empty SFP cages, we report the MII linkmodes that we
> can output on this port.
>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> MAINTAINERS | 1 +
> net/ethtool/Makefile | 2 +-
> net/ethtool/netlink.c | 25 +++
> net/ethtool/netlink.h | 8 +
> net/ethtool/port.c | 382 ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 417 insertions(+), 1 deletion(-)
> create mode 100644 net/ethtool/port.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6de1ff058db6..877f3316b5ea 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19010,6 +19010,7 @@ F: Documentation/devicetree/bindings/net/ethernet-connector.yaml
> F: Documentation/networking/phy-port.rst
> F: drivers/net/phy/phy_port.c
> F: include/linux/phy_port.h
> +F: net/ethtool/port.c
> K: struct\s+phy_port|phy_port_
>
> NETWORKING [GENERAL]
> diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
> index 629c10916670..9b5b09670008 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 port.o
> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
> index 1af395b54330..c076c07d0a08 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -26,6 +26,8 @@ static u32 ethnl_bcast_seq;
> ETHTOOL_FLAG_OMIT_REPLY)
> #define ETHTOOL_FLAGS_STATS (ETHTOOL_FLAGS_BASIC | ETHTOOL_FLAG_STATS)
>
> +char phy_interface_names[PHY_INTERFACE_MODE_MAX][ETH_GSTRING_LEN] __ro_after_init;
> +
> const struct nla_policy ethnl_header_policy[] = {
> [ETHTOOL_A_HEADER_DEV_INDEX] = { .type = NLA_U32 },
> [ETHTOOL_A_HEADER_DEV_NAME] = { .type = NLA_NUL_STRING,
> @@ -431,6 +433,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
> [ETHTOOL_MSG_TSCONFIG_SET] = ðnl_tsconfig_request_ops,
> [ETHTOOL_MSG_PHY_GET] = ðnl_phy_request_ops,
> [ETHTOOL_MSG_MSE_GET] = ðnl_mse_request_ops,
> + [ETHTOOL_MSG_PORT_GET] = ðnl_port_request_ops,
> };
>
> static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
> @@ -1572,6 +1575,15 @@ static const struct genl_ops ethtool_genl_ops[] = {
> .policy = ethnl_mse_get_policy,
> .maxattr = ARRAY_SIZE(ethnl_mse_get_policy) - 1,
> },
> + {
> + .cmd = ETHTOOL_MSG_PORT_GET,
> + .doit = ethnl_default_doit,
> + .start = ethnl_port_dump_start,
> + .dumpit = ethnl_port_dumpit,
> + .done = ethnl_port_dump_done,
> + .policy = ethnl_port_get_policy,
> + .maxattr = ARRAY_SIZE(ethnl_port_get_policy) - 1,
> + },
> };
>
> static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
> @@ -1594,10 +1606,23 @@ static struct genl_family ethtool_genl_family __ro_after_init = {
>
> /* module setup */
>
> +static void __init ethnl_phy_names_populate(void)
> +{
> + const char *name;
> + int i;
> +
> + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) {
> + name = phy_modes(i);
> + strscpy(phy_interface_names[i], name, ETH_GSTRING_LEN);
> + }
> +}
> +
> static int __init ethnl_init(void)
> {
> int ret;
>
> + ethnl_phy_names_populate();
> +
> ret = genl_register_family(ðtool_genl_family);
> if (WARN(ret < 0, "ethtool: genetlink family registration failed"))
> return ret;
> diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
> index 3e969a070f9f..dc7211f67c4d 100644
> --- a/net/ethtool/netlink.h
> +++ b/net/ethtool/netlink.h
> @@ -5,11 +5,14 @@
>
> #include <linux/ethtool_netlink.h>
> #include <linux/netdevice.h>
> +#include <linux/phy.h>
> #include <net/genetlink.h>
> #include <net/sock.h>
>
> struct ethnl_req_info;
>
> +extern char phy_interface_names[PHY_INTERFACE_MODE_MAX][ETH_GSTRING_LEN];
> +
> int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
> const struct nlattr *nest, struct net *net,
> struct netlink_ext_ack *extack,
> @@ -474,6 +477,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_port_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];
> @@ -530,6 +534,7 @@ 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_port_get_policy[ETHTOOL_A_PORT_ID + 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);
> @@ -545,6 +550,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_port_dump_start(struct netlink_callback *cb);
> +int ethnl_port_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
> +int ethnl_port_dump_done(struct netlink_callback *cb);
>
> 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/port.c b/net/ethtool/port.c
> new file mode 100644
> index 000000000000..0d6150e5f66a
> --- /dev/null
> +++ b/net/ethtool/port.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2026 Bootlin
> + */
> +#include <linux/phy.h>
> +#include <linux/phy_link_topology.h>
> +#include <linux/phy_port.h>
> +#include <net/netdev_lock.h>
> +
> +#include "bitset.h"
> +#include "common.h"
> +#include "netlink.h"
> +
> +struct port_req_info {
> + struct ethnl_req_info base;
> + u32 port_id;
> +};
> +
> +struct port_reply_data {
> + struct ethnl_reply_data base;
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
> + DECLARE_PHY_INTERFACE_MASK(interfaces);
> + u32 port_id;
> + bool mii;
> + bool sfp;
> + u32 upstream_port;
> +};
> +
> +#define PORT_REQINFO(__req_base) \
> + container_of(__req_base, struct port_req_info, base)
> +
> +#define PORT_REPDATA(__reply_base) \
> + container_of(__reply_base, struct port_reply_data, base)
> +
> +const struct nla_policy ethnl_port_get_policy[ETHTOOL_A_PORT_ID + 1] = {
> + [ETHTOOL_A_PORT_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
> + [ETHTOOL_A_PORT_ID] = NLA_POLICY_MIN(NLA_U32, 1),
> +};
> +
> +static int port_parse_request(struct ethnl_req_info *req_info,
> + const struct genl_info *info,
> + struct nlattr **tb,
> + struct netlink_ext_ack *extack)
> +{
> + struct port_req_info *request = PORT_REQINFO(req_info);
> +
> + if (GENL_REQ_ATTR_CHECK(info, ETHTOOL_A_PORT_ID))
> + return -EINVAL;
> +
> + request->port_id = nla_get_u32(tb[ETHTOOL_A_PORT_ID]);
> +
> + return 0;
> +}
> +
> +static int port_prepare_data(const struct ethnl_req_info *req_info,
> + struct ethnl_reply_data *reply_data,
> + const struct genl_info *info)
> +{
> + struct port_reply_data *reply = PORT_REPDATA(reply_data);
> + struct port_req_info *request = PORT_REQINFO(req_info);
> + struct phy_port *port;
> +
> + /* RTNL must be held while holding a ref to the phy_port. Here, caller
> + * holds RTNL.
> + */
> + port = phy_link_topo_get_port(req_info->dev, request->port_id);
> + if (!port)
> + return -ENODEV;
> +
> + linkmode_copy(reply->supported, port->supported);
> + phy_interface_copy(reply->interfaces, port->interfaces);
> + reply->port_id = port->id;
> + reply->mii = port->is_mii;
> + reply->sfp = port->is_sfp;
> + reply->upstream_port = port->upstream_port ? port->upstream_port->id : 0;
> +
> + return 0;
> +}
> +
> +static int port_reply_size(const struct ethnl_req_info *req_info,
> + const struct ethnl_reply_data *reply_data)
> +{
> + bool compact = req_info->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
> + struct port_reply_data *reply = PORT_REPDATA(reply_data);
> + size_t size = 0;
> + int ret;
> +
> + /* ETHTOOL_A_PORT_ID */
> + size += nla_total_size(sizeof(u32));
> +
> + if (!reply->mii) {
> + /* ETHTOOL_A_PORT_SUPPORTED_MODES */
> + ret = ethnl_bitset_size(reply->supported, NULL,
> + __ETHTOOL_LINK_MODE_MASK_NBITS,
> + link_mode_names, compact);
> + if (ret < 0)
> + return ret;
> +
> + size += ret;
> + } else {
> + /* ETHTOOL_A_PORT_SUPPORTED_INTERFACES */
> + ret = ethnl_bitset_size(reply->interfaces, NULL,
> + PHY_INTERFACE_MODE_MAX,
> + phy_interface_names, compact);
> + if (ret < 0)
> + return ret;
> +
> + size += ret;
> + }
> +
> + /* ETHTOOL_A_PORT_TYPE */
> + size += nla_total_size(sizeof(u32));
> +
> + /* ETHTOOL_A_PORT_UPSTREAM_PORT */
> + if (reply->upstream_port)
> + size += nla_total_size(sizeof(u32));
> +
> + return size;
> +}
> +
> +static int port_fill_reply(struct sk_buff *skb,
> + const struct ethnl_req_info *req_info,
> + const struct ethnl_reply_data *reply_data)
> +{
> + bool compact = req_info->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
> + struct port_reply_data *reply = PORT_REPDATA(reply_data);
> + int ret, port_type = ETHTOOL_PORT_TYPE_MDI;
> +
> + if (nla_put_u32(skb, ETHTOOL_A_PORT_ID, reply->port_id))
> + return -EMSGSIZE;
> +
> + if (!reply->mii) {
> + ret = ethnl_put_bitset(skb, ETHTOOL_A_PORT_SUPPORTED_MODES,
> + reply->supported, NULL,
> + __ETHTOOL_LINK_MODE_MASK_NBITS,
> + link_mode_names, compact);
> + if (ret < 0)
> + return ret;
> + } else {
> + ret = ethnl_put_bitset(skb, ETHTOOL_A_PORT_SUPPORTED_INTERFACES,
> + reply->interfaces, NULL,
> + PHY_INTERFACE_MODE_MAX,
> + phy_interface_names, compact);
> + if (ret < 0)
> + return ret;
> + }
> +
> + if (reply->mii || reply->sfp)
> + port_type = ETHTOOL_PORT_TYPE_SFP;
> +
> + if (nla_put_u32(skb, ETHTOOL_A_PORT_TYPE, port_type))
> + return -EMSGSIZE;
> +
> + if (reply->upstream_port &&
> + nla_put_u32(skb, ETHTOOL_A_PORT_UPSTREAM_PORT,
> + reply->upstream_port))
> + return -EMSGSIZE;
> +
> + return 0;
> +}
> +
> +struct port_dump_ctx {
> + struct port_req_info *req_info;
> + struct port_reply_data *reply_data;
> + unsigned long ifindex;
> + unsigned long pos_ifindex;
> + unsigned long pos_portid;
> +};
> +
> +static struct port_dump_ctx *
> +port_dump_ctx_get(struct netlink_callback *cb)
> +{
> + return (struct port_dump_ctx *)cb->ctx;
> +}
> +
> +int ethnl_port_dump_start(struct netlink_callback *cb)
> +{
> + const struct genl_dumpit_info *info = genl_dumpit_info(cb);
> + struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
> + struct nlattr **tb = info->info.attrs;
> + struct port_reply_data *reply_data;
> + struct port_req_info *req_info;
> + int ret;
> +
> + BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
> +
> + req_info = kzalloc_obj(*req_info);
> + if (!req_info)
> + return -ENOMEM;
> +
> + reply_data = kmalloc_obj(*reply_data);
> + if (!reply_data) {
> + ret = -ENOMEM;
> + goto free_req_info;
> + }
> +
> + ret = ethnl_parse_header_dev_get(&req_info->base, tb[ETHTOOL_A_PORT_HEADER],
> + genl_info_net(&info->info),
> + info->info.extack, false);
> + if (ret < 0)
> + goto free_rep_data;
> +
> + ctx->ifindex = 0;
> + ctx->pos_ifindex = 0;
> +
> + /* For filtered DUMP requests, let's just store the ifindex. We'll check
> + * again if the netdev is still there when looping over the netdev list
> + * in the DUMP loop.
> + */
> + if (req_info->base.dev) {
> + ctx->ifindex = req_info->base.dev->ifindex;
> + netdev_put(req_info->base.dev, &req_info->base.dev_tracker);
> + req_info->base.dev = NULL;
> + }
> +
> + ctx->req_info = req_info;
> + ctx->reply_data = reply_data;
> +
> + return 0;
> +
> +free_rep_data:
> + kfree(reply_data);
> +free_req_info:
> + kfree(req_info);
> +
> + return ret;
> +}
> +
> +static int port_dump_one(struct sk_buff *skb, struct net_device *dev,
> + struct netlink_callback *cb)
> +{
> + struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
> + void *ehdr;
> + int ret;
> +
> + ehdr = ethnl_dump_put(skb, cb, ETHTOOL_MSG_PORT_GET_REPLY);
> + if (!ehdr)
> + return -EMSGSIZE;
> +
> + memset(ctx->reply_data, 0, sizeof(struct port_reply_data));
> + ctx->reply_data->base.dev = dev;
> +
> + rtnl_lock();
> + netdev_lock_ops(dev);
> +
> + ret = port_prepare_data(&ctx->req_info->base, &ctx->reply_data->base,
> + genl_info_dump(cb));
> +
> + netdev_unlock_ops(dev);
> + rtnl_unlock();
> +
> + if (ret < 0)
> + goto out;
> +
> + ret = ethnl_fill_reply_header(skb, dev, ETHTOOL_A_PORT_HEADER);
> + if (ret < 0)
> + goto out;
> +
> + ret = port_fill_reply(skb, &ctx->req_info->base, &ctx->reply_data->base);
> +
> +out:
> + ctx->reply_data->base.dev = NULL;
> + if (ret < 0)
> + genlmsg_cancel(skb, ehdr);
> + else
> + genlmsg_end(skb, ehdr);
> +
> + return ret;
> +}
> +
> +static int port_dump_one_dev(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
> + struct net_device *dev;
> + struct phy_port *port;
> + int ret;
> +
> + dev = ctx->req_info->base.dev;
> +
> + if (!dev->link_topo)
> + return 0;
> +
> + xa_for_each_start(&dev->link_topo->ports, ctx->pos_portid, port,
> + ctx->pos_portid) {
> + ctx->req_info->port_id = ctx->pos_portid;
> +
> + ret = port_dump_one(skb, dev, cb);
> + if (ret == -ENODEV)
> + continue;
> + if (ret)
> + return ret;
> + }
> +
> + ctx->pos_portid = 0;
> +
> + return 0;
> +}
> +
> +static int port_dump_all_dev(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
> + 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, ctx->pos_ifindex) {
> + netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
> + rcu_read_unlock();
> +
> + ctx->req_info->base.dev = dev;
> + ret = port_dump_one_dev(skb, cb);
> +
> + rcu_read_lock();
> + netdev_put(dev, &dev_tracker);
> + ctx->req_info->base.dev = NULL;
> +
> + if (ret < 0 && ret != -EOPNOTSUPP) {
> + if (likely(skb->len))
> + ret = skb->len;
> + break;
> + }
> +
> + ret = 0;
> + }
> + rcu_read_unlock();
> +
> + return ret;
> +}
> +
> +int ethnl_port_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + const struct genl_dumpit_info *info = genl_dumpit_info(cb);
> + struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
> + 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;
> +
> + ctx->req_info->base.dev = dev;
> + ret = port_dump_one_dev(skb, cb);
> + if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
> + ret = skb->len;
> +
> + netdev_put(dev, &dev_tracker);
> + } else {
> + ret = port_dump_all_dev(skb, cb);
> + }
> +
> + return ret;
> +}
> +
> +int ethnl_port_dump_done(struct netlink_callback *cb)
> +{
> + struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
> +
> + kfree(ctx->req_info);
> + kfree(ctx->reply_data);
> +
> + return 0;
> +}
> +
> +const struct ethnl_request_ops ethnl_port_request_ops = {
> + .request_cmd = ETHTOOL_MSG_PORT_GET,
> + .reply_cmd = ETHTOOL_MSG_PORT_GET_REPLY,
> + .hdr_attr = ETHTOOL_A_PORT_HEADER,
> + .req_info_size = sizeof(struct port_req_info),
> + .reply_data_size = sizeof(struct port_reply_data),
> +
> + .parse_request = port_parse_request,
> + .prepare_data = port_prepare_data,
> + .reply_size = port_reply_size,
> + .fill_reply = port_fill_reply,
> +};
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 10/10] Documentation: networking: Update the phy_port infrastructure description
2026-09-27 13:36 ` [PATCH net-next v18 10/10] Documentation: networking: Update the phy_port infrastructure description Maxime Chevallier
@ 2026-09-28 10:07 ` Christophe Leroy (CS GROUP)
0 siblings, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 10:07 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> With SFP now properly supported with phy_port, add some details in the
> documentation. Fix a typo along the way (driver -> driven).
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> Documentation/networking/phy-port.rst | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/networking/phy-port.rst b/Documentation/networking/phy-port.rst
> index 6e28d9094bce..2a9b1ec93b70 100644
> --- a/Documentation/networking/phy-port.rst
> +++ b/Documentation/networking/phy-port.rst
> @@ -99,13 +99,29 @@ will eventually be able to report its own ksettings::
> (_____)-----| Port |
> +------+
>
> +SFP ports
> +=========
> +
> +SFP interfaces involve 2 distinct components, each represented by
> +a :c:type:`struct phy_port <phy_port>` instance :
> +
> + - The SFP cage itself is a :c:type:`struct phy_port <phy_port>`. It's special
> + in that it's not an MDI interface, but rather a hot-pluggable MII.
> + The :c:type:`struct phy_port <phy_port>` associated to it lists the different
> + MII interfaces we can use on the cage.
> +
> + - The SFP module, when inserted, will also be associated to a
> + :c:type:`struct phy_port <phy_port>`, that represents the various linkmodes
> + that it gives access to. The module's :c:type:`struct phy_port <phy_port>`
> + doesn't supersede the cage's port, it references it through
> + the :c:type:`struct phy_port <phy_port>` :c:member:`upstream_port` field.
> +
> Next steps
> ==========
>
> -As of writing this documentation, only ports controlled by PHY devices are
> -supported. The next steps will be to add the Netlink API to expose these
> -to userspace and add support for raw ports (controlled by some firmware, and directly
> -managed by the NIC driver).
> +As of writing this documentation, the port's presence and information can only
> +be queried, and it's not possible to change any of the port's settings or select
> +which one should be used.
>
> Another parallel task is the introduction of a MII muxing framework to allow the
> -control of non-PHY driver multi-port setups.
> +control of non-PHY driven multi-port setups.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
` (9 preceding siblings ...)
2026-09-27 13:36 ` [PATCH net-next v18 10/10] Documentation: networking: Update the phy_port infrastructure description Maxime Chevallier
@ 2026-09-28 17:31 ` Christophe Leroy (CS GROUP)
10 siblings, 0 replies; 31+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-28 17:31 UTC (permalink / raw)
To: Maxime Chevallier, davem, Andrew Lunn, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, Heiner Kallweit
Cc: netdev, linux-kernel, thomas.petazzoni, Herve Codina,
Florian Fainelli, Vladimir Oltean, Köry Maincent,
Marek Behún, Oleksij Rempel, Nicolò Veronese,
Simon Horman, mwojtas, Romain Gantois, Daniel Golle,
Dimitri Fedrau, Frank Wunderlich, Pietro Ameruoso,
Aleksei Sviridkin
Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> Hello everyone,
>
> Here's V18 for the phy_port improved SFP support and netlink interface.
>
> V18 aggregates Andrew's reviews (thanks !) and Aleksei Tested-by tags
> (thank you as well, this is nice). It also contains a few tweaks
> following sashiko and Aleksei's report after testing, the ops-locked
> check also checks for NULL netdev ops. I also added a missing rtnl()
> around port deletion in phylink, and changed a bitfield to a bool in
> phy.h following sashiko and Aleksei's reviews.
>
> This work extends on the recent addition of phy_port representation to enable
> listing the front-facing ports of an interface. For now, we don't control
> these ports, we merely list their presence and their capabilities.
>
> As the most common use-case of multi-port interfaces is combo-ports that
> provide both RJ45 and SFP connectors on a single MAC, there's a lot of
> SFP stuff in this series.
>
> This series is in 2 main parts. The first one aims at representing the
> SFP cages and modules using phy_port, as combo-ports with RJ45 + SFP are
> by far the most common cases for multi-connector setups.
>
> The second part is the netlink interface to list those ports, now that
> most use-cases are covered.
>
> Let's see what we can do with some examples of the new ethtool API :
>
> - Get MII interfaces supported by an empty SFP cage :
>
> # ethtool --show-ports eth3
>
> Port for eth3:
> Port id: 1
> Supported MII interfaces : sgmii, 1000base-x, 2500base-x
> Port type: sfp
>
> - Get Combo-ports supported modes, on each port :
>
> # ethtool --show-ports eth1
>
> Port for eth1:
> Port id: 1
> Supported link modes: 10baseT/Half 10baseT/Full
> 100baseT/Half 100baseT/Full
> 1000baseT/Full
> 10000baseT/Full
> 2500baseT/Full
> 5000baseT/Full
>
> Port type: mdi
>
> Port for eth1:
> Port id: 2
> Supported MII interfaces : 10gbase-r
> Port type: sfp
>
> - Get Achievable linkmodes on a SFP module (combo port with a DAC in the
> SFP cage)
>
> # ethtool --show-ports eth1
>
> Port for eth1:
> Port id: 1
> Supported link modes: 10baseT/Half 10baseT/Full
> 100baseT/Half 100baseT/Full
> 1000baseT/Full
> 10000baseT/Full
> 2500baseT/Full
> 5000baseT/Full
> Port type: mdi
>
> Port for eth1:
> Port id: 2
> Supported MII interfaces : 10gbase-r
> Port type: sfp
>
> Port for eth1:
> Port id: 3
> Upstream id: 2
> Supported link modes: 10000baseCR/Full
> Port type: mdi
>
> Note that here, we have 3 ports :
> - The Copper port
> - The SFP Cage itself,
> - The SFP module
>
> This series builds on top of phy_port and phy_link_topology to allow
> tracking the ports of an interface. We maintain a list of supported
> linkmodes/interfaces on each port, which allows for fine-grained
> reporting of each port's capability.
>
> What this series doesn't do :
> - We don't support selecting which port is active. This is the next step.
> - We only support PHY-driven combo ports. The end-goal of this whole
> journey that started with phy_link_topology is to get support for MII
> muxes, such as the one we have on the Turris Omnia. This will eventually
> be upstreamed as well.
>
> If you want to play around with it, here's [1] the patched ethtool that I've
> been using to produce the outputs above.
>
> Thanks !
>
Tested on one of my powerpc boards, I get the expected result:
~# ./ethtool --show-ports eth0
Port for eth0:
Port id: 1
Port type: phy
forced : off
Tested-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Looking forward to the support for MII muxes.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc
2026-09-27 13:36 ` [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc Maxime Chevallier
2026-09-28 9:36 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The new comment in phy_link_topo_get_or_alloc()
(drivers/net/phy/phy_link_topology.c) lists the contexts the helper…
Pre-existing issues:
- [Low] Documentation/networking/phy-link-topology.rst (API section) says
struct phy_link_topology "is a per-netdevice resource, that gets…
--
net: phy: phy_link_topology: Add a helper for opportunistic alloc
This commit moves the lazy allocation of dev->link_topo out of
phy_link_topo_add_phy() and into a new helper,
phy_link_topo_get_or_alloc(). The helper returns the existing topology
or allocates a new one. It prepares for tracking phy_port objects in
the topology.
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 4134de7ae313d..0462283c80202 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c
> @@ -28,11 +28,39 @@ static int netdev_alloc_phy_link_topology(struct net_device *dev)
> return 0;
> }
>
> +static struct phy_link_topology *phy_link_topo_get_or_alloc(struct net_device *dev)
> +{
> + int ret;
> +
> + if (dev->link_topo)
> + return dev->link_topo;
> +
> + /* The topology is allocated the first time we add an object to it.
> + * It is freed alongside the netdev. It can be called on multiple
> + * contexts:
> + * - It can be called from .probe() : No rtnl, no netdev_lock
> + * - .ndo_open() : rtnl and possibly netdev_lock
[Severity: Low]
Can phy_link_topo_get_or_alloc() actually be reached with netdev_lock
held?
At this commit, the only caller is phy_link_topo_add_phy(). It returns
early for ops-locked devices before it calls the helper:
phy_link_topo_add_phy() {
...
if (WARN_ON_ONCE(netdev_need_ops_lock(dev)))
return -EOPNOTSUPP;
...
}
Later in the series, phy_link_topo_add_port() rejects the same devices
with WARN_ON_ONCE(dev->netdev_ops && netdev_need_ops_lock(dev)).
On the open path, dev_open() only takes netdev_lock through
netdev_lock_ops():
include/net/netdev_lock.h:netdev_lock_ops() {
if (netdev_need_ops_lock(dev))
netdev_lock(dev);
}
That means netdev_lock is held only for the devices that both callers
reject. Would it be more accurate to drop "possibly netdev_lock" from
the .ndo_open() line?
This comment also mentions ports and the SFP cage port, which don't
exist yet at this commit. struct phy_link_topology only has phys and
next_phy_index. The rest of the series fixes that. "net: phy:
phy_link_topology: Track ports in phy_link_topology" adds the ports
xarray and phy_link_topo_add_port(). Later patches add
phylink_create_sfp_cage_port(), phylink_add_sfp_mod_port() and
phy_add_sfp_mod_port(). By the end of the series, only the netdev_lock
part is still inaccurate.
> + * - SFP state machine : rtnl held or not
> + *
> + * However, we can't really have races :
> + * - If we have a PHY, phy_link_topo_add_phy() will always run first
> + * and trigger the alloc. Only then the ports can be added through
> + * phylib or sfp.
> + * - If we don't, the SFP port for the cage is registered first, and
> + * only then other ports/PHYs can be registered.
> + */
> + ret = netdev_alloc_phy_link_topology(dev);
> + if (ret)
> + return ERR_PTR(ret);
[Severity: Low]
This is a pre-existing issue, not something this patch introduced. The
API section of Documentation/networking/phy-link-topology.rst still
says struct phy_link_topology "is a per-netdevice resource, that gets
initialized at netdevice creation".
The code allocates dev->link_topo only when the first object is added.
phy_link_topo_empty() and phy_link_topo_get_phy() both handle a NULL
dev->link_topo.
The baseline already did this allocation inline in
phy_link_topo_add_phy(). The last patch in the series, "Documentation:
networking: Update the phy_port infrastructure description", leaves
that sentence unchanged.
Could the documentation say that the topology is allocated when the
first object is added?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology
2026-09-27 13:36 ` [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology Maxime Chevallier
2026-09-28 9:37 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 2 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] Ports of SFP module PHYs that are connected via PHY_UPSTREAM_PHY
can be freed while they are still in dev->link_topo->ports, leaving…
- [Medium] phy_link_topo_del_port() erases topo->ports[port->id] without
checking that the stored entry is this port, and port->id is…
- [Medium] The patch makes two paths touch the same data under locks that
never overlap.
- [Low] Adding and removing ports in the topology is asymmetric.
--
net: phy: phy_link_topology: Track ports in phy_link_topology
This commit adds a ports xarray to struct phy_link_topology and an id
field to struct phy_port. PHY-driven ports are added to the netdev
topology when phy_link_topo_add_phy() runs. They are removed from it in
phy_link_topo_del_phy() and phy_del_port().
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 5b13a74e2fa92..c4d2c03b8282b 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1580,6 +1580,9 @@ static void phy_del_port(struct phy_device *phydev, struct phy_port *port)
>
> list_del(&port->head);
>
> + if (phydev->attached_dev)
> + phy_link_topo_del_port(phydev->attached_dev, port);
> +
[Severity: High]
Can this leave freed ports in dev->link_topo->ports for SFP module PHYs?
phy_sfp_connect_phy() adds the module PHY with PHY_UPSTREAM_PHY:
struct net_device *dev = phydev->attached_dev;
if (dev)
return phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev);
The new loop in phy_link_topo_add_phy() then inserts all of the module
PHY's ports into the upstream netdev's topology. The module PHY never
goes through phy_attach_direct(), so its attached_dev stays NULL. Because
of that, the check above never calls phy_link_topo_del_port() for it.
One path is an unbind of the module PHY's driver:
phy_remove(M)
phy_cleanup_ports(M)
phy_del_port() <- M->attached_dev is NULL, no xa_erase()
phy_port_destroy() <- kfree(port)
Later, phy_sfp_disconnect_phy() -> phy_link_topo_del_phy(dev, M) walks
M->ports. That list is empty by then, so the entry is never erased.
A second path involves MAC drivers that connect the PHY in ndo_open and
disconnect it in ndo_stop (for example mvpp2 or mvneta via phylink). With
a module PHY present, taking the link down runs phy_detach_internal(P) ->
phy_link_topo_del_phy(dev, P), which removes only P's own ports.
When the module is then unplugged, sfp_sm_phy_detach() -> sfp_remove_phy()
-> phy_sfp_disconnect_phy() sees P->attached_dev == NULL and skips
phy_link_topo_del_phy(dev, M). phy_device_remove(M) -> phy_remove() ->
phy_cleanup_ports() then frees M's ports without erasing them.
xa_destroy() doesn't dereference the entries, so this commit alone won't
crash. However, a later patch in this series, "net: ethtool: Introduce
ethtool command to list ports", adds an ETHTOOL_MSG_PORT_GET dump with no
admin permission flag. That dump walks xa_for_each(&dev->link_topo->ports).
Its port_prepare_data() copies port->supported and port->interfaces and
dereferences port->upstream_port.
Wouldn't that dump read freed memory? phy_del_port(),
phy_sfp_disconnect_phy() and phy_link_topo_del_phy() are unchanged at the
end of the series.
> phydev->n_ports--;
> }
>
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 0462283c80202..28c5bf3a8849a 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c
[ ... ]
> @@ -56,12 +60,45 @@ static struct phy_link_topology *phy_link_topo_get_or_alloc(struct net_device *d
> return dev->link_topo;
> }
>
> +int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port)
> +{
[ ... ]
> + /* Attempt to re-use a previously allocated port_id */
> + if (port->id)
> + ret = xa_insert(&topo->ports, port->id, port, GFP_KERNEL);
> + else
> + ret = xa_alloc_cyclic(&topo->ports, &port->id, port,
> + xa_limit_32b, &topo->next_port_index,
> + GFP_KERNEL);
> +
> + return ret < 0 ? ret : 0;
> +}
> +EXPORT_SYMBOL_GPL(phy_link_topo_add_port);
> +
> +void phy_link_topo_del_port(struct net_device *dev, struct phy_port *port)
> +{
> + struct phy_link_topology *topo = dev->link_topo;
> +
> + if (!topo)
> + return;
> +
> + xa_erase(&topo->ports, port->id);
[Severity: Medium]
Can this erase an entry that belongs to a different port?
port->id is deliberately kept across detach and attach. When the id is
reused, phy_link_topo_add_port() calls xa_insert(). That returns -EBUSY if
another port in the target topology already owns the index, and there is
no fallback to a fresh allocation.
In that case phy_link_topo_add_phy() rolls back the ports it inserted and
returns the error. phy_attach_direct() then does:
err = phy_link_topo_add_phy(dev, phydev, PHY_UPSTREAM_MAC, dev);
if (err)
goto error;
The error path calls phy_detach_internal() while attached_dev is still
set:
phy_detach_internal()
phy_link_topo_del_phy(dev, phydev)
list_for_each_entry(port, &phy->ports, head)
phy_link_topo_del_port(dev, port)
xa_erase(&topo->ports, port->id)
This walks every port in phy->ports, including the port that failed and
any ports that were never inserted. The failing port's id belongs to
another live port.
Wouldn't that live port be removed from the topology while its owner
still exists? Would checking the stored entry, for example with
xa_cmpxchg(&topo->ports, port->id, port, NULL, 0), avoid this?
The same full teardown runs when phy_link_topo_add_phy() fails earlier,
either on pdn allocation -ENOMEM or on a phyindex collision.
On -ENOMEM from xa_alloc_cyclic(), __xa_alloc() may already have written
an index into port->id that was never stored. The port could then carry an
id it doesn't own.
The PHY attach also fails with -EBUSY in the collision case. The code is
unchanged at the end of the series. Later patches add more pre-registered
ports, such as the phylink SFP cage port allocated at phylink_create()
time, and these could collide with kept ids.
> +}
> +EXPORT_SYMBOL_GPL(phy_link_topo_del_port);
[ ... ]
> @@ -110,8 +147,20 @@ int phy_link_topo_add_phy(struct net_device *dev,
> if (ret < 0)
> goto err;
>
> + /* Add all the PHY's ports to the topology */
> + list_for_each_entry(port, &phy->ports, head) {
> + ret = phy_link_topo_add_port(dev, port);
> + if (ret)
> + goto del_ports;
> + }
[Severity: Low]
Should phy_add_port() also add the port to the topology?
phy_del_port() now calls phy_link_topo_del_port() whenever
phydev->attached_dev is set, but phy_add_port() only does:
list_add(&port->head, &phydev->ports);
phydev->n_ports++;
Ports reach topo->ports only through this loop, at attach or connect time.
Consider an attached PHY whose driver is unbound and rebound through
sysfs. phy_remove() leaves attached_dev and the PHY's topology entry in
place, and phy_cleanup_ports() -> phy_del_port() removes the original
ports from the topology.
The rebind then runs phy_probe() -> phy_setup_ports() -> phy_add_port().
That rebuilds phydev->ports but doesn't add the new ports to the topology.
No phy_attach_direct() runs, so the netdev's topology is missing all of
the PHY's ports until the next detach and attach.
The same gap applies to a module PHY whose driver binds after
phy_sfp_connect_phy() has already walked an empty port list.
The comment in phy_link_topo_get_or_alloc() also says:
* - If we have a PHY, phy_link_topo_add_phy() will always run first
* and trigger the alloc. Only then the ports can be added through
* phylib or sfp.
However, there doesn't seem to be a per-port add path in phylib.
phy_add_port() is unchanged at the end of the series.
> +
> return 0;
>
> +del_ports:
> + list_for_each_entry_continue_reverse(port, &phy->ports, head)
> + phy_link_topo_del_port(dev, port);
> +
> + xa_erase(&topo->phys, phy->phyindex);
> err:
> kfree(pdn);
> return ret;
> @@ -123,10 +172,14 @@ void phy_link_topo_del_phy(struct net_device *dev,
> {
> struct phy_link_topology *topo = dev->link_topo;
> struct phy_device_node *pdn;
> + struct phy_port *port;
>
> if (!topo)
> return;
>
> + list_for_each_entry(port, &phy->ports, head)
> + phy_link_topo_del_port(dev, port);
> +
[Severity: Medium]
Is it safe to walk phy->ports here with only RTNL held?
phy_link_topo_add_phy() and phy_link_topo_del_phy() now walk
phydev->ports under RTNL, from phy_attach_direct() and
phy_detach_internal(). The driver unbind path changes the same list under
device_lock only, without RTNL:
phy_remove()
phy_cleanup_ports()
phy_del_port()
list_del(&port->head)
phy_link_topo_del_port() <- xa_erase() on dev->link_topo->ports
phy_port_destroy() <- kfree(port)
Suppose an attached PHY is unbound through sysfs while "ip link set down"
runs phy_disconnect() -> phy_detach_internal() -> phy_link_topo_del_phy().
Could this walk then follow a port that phy_cleanup_ports() has just
unlinked and freed?
phy_del_port() also reads phydev->attached_dev with no RTNL and no
READ_ONCE(), then uses it to reach dev->link_topo. Can
phy_detach_internal() clear attached_dev between that check and the use?
A later patch in this series, "net: phy: phy_link_topology: Add a helper
to retrieve ports", says that "the caller must hold rtnl until it's done
with it", and phy_link_topo_get_port() does ASSERT_RTNL(). The xa_erase()
and kfree() in phy_remove() run without RTNL.
Could an ethtool port dump that holds RTNL xa_load() a port that is then
freed underneath it?
> pdn = xa_erase(&topo->phys, phy->phyindex);
>
> /* We delete the PHY from the topology, however we don't re-set the
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages
2026-09-27 13:36 ` [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages Maxime Chevallier
2026-09-28 9:38 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] phylink_create_sfp_cage_port() calls phy_link_topo_add_port()
without RTNL.
- [Low] phylink_create_sfp_cage_port() publishes a phy_port into the
netdev's link topology without setting parent_type.
--
net: phylink: Register a phy_port for MAC-driven SFP cages
This commit registers a phy_port for SFP cages that the MAC drives
directly through phylink, and attaches it to the netdev's link
topology. The port is only created for phylink instances that have a
netdev. It is created in phylink_register_sfp() and removed in
phylink_destroy().
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index a7d086cdc9b25..19090916af75f 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
[ ... ]
> @@ -1767,6 +1770,50 @@ static void phylink_fixed_poll(struct timer_list *t)
>
> static const struct sfp_upstream_ops sfp_phylink_ops;
>
> +static int phylink_create_sfp_cage_port(struct phylink *pl)
> +{
> + struct phy_port *port;
> + int ret = 0;
> +
> + if (!pl->netdev || !pl->sfp_bus)
> + return 0;
> +
> + port = phy_port_alloc();
> + if (!port)
> + return -ENOMEM;
> +
> + port->is_sfp = true;
> + port->is_mii = true;
> + port->active = true;
[Severity: Low]
parent_type is never set here. Should it be?
phy_port_alloc() uses kzalloc, so this port ends up with
parent_type == PHY_PORT_PHY and port->phy == NULL. The only parent
type defined in include/linux/phy_port.h is:
enum phy_port_parent {
PHY_PORT_PHY,
};
and its kernel-doc says "Indicates that the port is driven by a PHY
device". This looks like the first phy_port that is driven by the MAC
or phylink instead of a PHY.
The other initializers in phy_device.c, such as phy_setup_sfp_port(),
always set the two fields together:
port->parent_type = PHY_PORT_PHY;
port->phy = phydev;
Nothing reads parent_type today, so this does not cause a NULL
dereference right now. Would it make sense to add a phylink/MAC
parent type to enum phy_port_parent and set it here? That way the
published port would not claim a PHY parent it lacks.
The same applies to phylink_add_sfp_mod_port() later in the series.
> +
> + phy_interface_and(port->interfaces, pl->config->supported_interfaces,
> + phylink_sfp_interfaces);
> + phy_port_update_supported(port);
> +
> + ret = phy_link_topo_add_port(pl->netdev, port);
[Severity: Medium]
This runs from phylink_create(), which is documented as "the rtnl lock
must not be held when calling this function". Can this race with
ethtool readers of dev->link_topo?
If the netdev has no topology yet, phy_link_topo_add_port() allocates
and publishes it with a plain store:
drivers/net/phy/phy_link_topology.c:netdev_alloc_phy_link_topology() {
...
xa_init_flags(&topo->ports, XA_FLAGS_ALLOC1);
topo->next_port_index = 1;
dev->link_topo = topo;
...
}
This store has no RTNL, WRITE_ONCE() or release barrier.
phylink_create() can be called on a netdev that is already registered.
For example, dpaa2 handles a runtime endpoint change like this:
dpni_irq0_handler_thread()
dpaa2_eth_connect_mac()
dpaa2_mac_connect()
phylink_create() <- no RTNL held
phylink_register_sfp()
phylink_create_sfp_cage_port()
phy_link_topo_add_port()
netdev_alloc_phy_link_topology()
Only the later phylink_fwnode_phy_connect() call in dpaa2_mac_connect()
is inside rtnl_lock(). Before this patch, that later call was the one
that first allocated the topology, through
phy_attach_direct()->phy_link_topo_add_phy().
The ethtool netlink readers read dev->link_topo and dereference it
while holding RTNL, not via RCU. For example,
ethnl_tsinfo_dump_one_net_topo() does:
if (!dev->link_topo) {
...
xa_for_each_start(&dev->link_topo->phys, ctx->pos_phyindex, pdn,
phy_link_topo_get_phy(), ethnl_req_get_phydev() and the PHY dump in
net/ethtool/netlink.c follow the same pattern.
On arm64, can such a reader see the new pointer before the
kzalloc/xa_init_flags() stores are visible?
The comment in phy_link_topo_get_or_alloc() explains the ordering
between writers, but it does not seem to cover RTNL-holding readers.
Also, phylink_destroy_sfp_cage_port() takes rtnl_lock() around
phy_link_topo_del_port() with the comment "The port may still be
accessed under RTNL". Should the insertion side use the same locking?
> + if (ret)
> + phy_port_destroy(port);
> + else
> + pl->sfp_cage_port = port;
> +
> + return ret;
> +}
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream
2026-09-27 13:36 ` [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream Maxime Chevallier
2026-09-28 9:40 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] This commit fixes a real, user-visible bug but the commit
message calls it preparation only ("This will allow passing the…
- [Medium] Now that module_insert can succeed synchronously inside
phy_sfp_probe() (see the previous finding), it runs before…
--
net: phy: Create SFP phy_port before registering upstream
This commit moves creation of the SFP phy_port in phy_sfp_probe() so that
it happens before sfp_bus_add_upstream() instead of after it. It also
changes phy_setup_sfp_port() to return the port pointer, and reworks the
error path to delete the port and drop the bus reference on failure.
> We can move the port creation before registering the sfp upstream ops,
> as long as we know the SFP bus is there. This will allow passing the
> phy_port along with the upstream information to the SFP bus.
[Severity: Medium]
The message describes this as preparation only. Does the reordering also
fix an existing bug?
Before this patch, phy_sfp_probe() called sfp_bus_add_upstream() before
phy_setup_sfp_port(). Suppose the SFP cage driver has already probed the
module, so sm_mod_state is SFP_MOD_WAITDEV. Two common ways to get there
are a built-in sfp driver with a modular PHY driver loaded later, and an
unbind/rebind of the PHY driver while a module is plugged in. In that
case, registering the upstream runs module_insert synchronously:
phy_sfp_probe()
sfp_bus_add_upstream()
sfp_register_bus()
bus->registered = true;
bus->socket_ops->attach()
sfp_attach()
sfp_sm_event(SFP_E_DEV_ATTACH)
sfp_sm_module() /* SFP_MOD_WAITDEV */
sfp_module_insert()
phy_sfp_module_insert()
The SFP phy_port did not exist yet at that point, so
phy_sfp_module_insert() returned without printing anything:
port = phy_get_sfp_port(phydev);
if (!port)
return -EINVAL;
sfp_sm_module() then moved the module to SFP_MOD_ERROR. It only leaves
that state on SFP_E_REMOVE or SFP_E_INSERT, so the module stayed unusable
until someone physically re-inserted it.
With the port now created before sfp_bus_add_upstream(), this path
succeeds. Should the commit message describe the fix? Should it also have
a Fixes: tag so that stable kernels with the phy_port SFP infrastructure
pick it up? Possible candidates:
d7c6082f7e77 ("net: phy: Introduce generic SFP handling for PHY drivers")
07f3ca9e092c ("net: phy: Create a phy_port for PHY-driven SFPs")
The first made module_insert depend on phy_get_sfp_port(). The second
put the port setup after upstream registration.
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index c4d2c03b8282b..3c20d4cc1a79d 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
[ ... ]
> @@ -1619,30 +1621,46 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
> */
> static int phy_sfp_probe(struct phy_device *phydev)
> {
> + struct phy_port *port = NULL;
> struct sfp_bus *bus;
> - int ret = 0;
> + int ret;
>
> - if (phydev->mdio.dev.fwnode) {
> - bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
> - if (IS_ERR(bus))
> - return PTR_ERR(bus);
> + if (!phydev->mdio.dev.fwnode)
> + return 0;
>
> - phydev->sfp_bus = bus;
> + bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
> + if (IS_ERR(bus))
> + return PTR_ERR(bus);
>
> - ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
> - sfp_bus_put(bus);
> + phydev->sfp_bus = bus;
>
> - if (ret)
> - phydev->sfp_bus = NULL;
> + if (bus) {
> + port = phy_setup_sfp_port(phydev);
> + if (IS_ERR(port)) {
> + ret = PTR_ERR(port);
> + goto out_sfp;
> + }
> }
>
> - if (!ret && phydev->sfp_bus) {
> - ret = phy_setup_sfp_port(phydev);
> - if (ret) {
> - sfp_bus_del_upstream(phydev->sfp_bus);
> - phydev->sfp_bus = NULL;
> - }
> + ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
[Severity: Medium]
phy_sfp_module_insert() can now succeed from inside phy_sfp_probe(). What
happens to the phydev->port value it sets there?
Take a PHY whose SFP port is its only port (max_n_ports == 1, such as
at8031, 88e1510 or 88x2222). The synchronous insert does this in
phy_sfp_module_insert():
if (phydev->n_ports == 1)
phydev->port = caps->port;
Then phy_sfp_probe() returns, and phy_setup_ports() unconditionally
overwrites the value:
/* For now, the phy->port field is set as the first active port's type */
list_for_each_entry(port, &phydev->ports, head)
if (port->active) {
phydev->port = phy_port_get_type(port);
break;
}
The SFP port has is_mii set and no mediums, so phy_port_get_type()
returns PORT_OTHER for it.
As a result, a module that is already present when the PHY probes reports
PORT_OTHER. The same module hot-plugged after probe keeps caps->port,
for example PORT_FIBRE or PORT_DA.
qca807x in combo mode appears to have the same problem.
qca807x_configure_serdes() sets phydev->port = PORT_FIBRE during the
synchronous insert. phy_setup_ports() then adds the default copper port
and overwrites phydev->port with PORT_TP. The hot-plug path keeps
PORT_FIBRE.
at803x, 88x2222 and qca807x never recompute phydev->port in read_status,
so the port type that ethtool shows stays wrong until the module is
removed. marvell.c and marvell10g recompute it at runtime.
Before this patch, this path failed with -EINVAL, so the overwrite only
becomes reachable with this reordering. Later patches in the series leave
phy_setup_ports() and phy_sfp_module_insert() unchanged, so the behaviour
remains at the end of the series.
Should phy_setup_ports() leave phydev->port alone when an SFP module was
already inserted during phy_sfp_probe()?
> + if (ret)
> + goto out_port;
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port
2026-09-27 13:36 ` [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port Maxime Chevallier
2026-09-28 9:42 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 3 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] At this commit, phy_sfp_probe() assigns phydev->sfp_cage_port
only after sfp_bus_add_upstream() (drivers/net/phy/phy_device.c:1729…
- [Medium] phy_caps_linkmode_filter_ifaces()
(drivers/net/phy/phy_caps.c:458-472) turns each host cage interface…
- [Medium] phy_detach_internal() (drivers/net/phy/phy_device.c:1833-1834)
calls phy_link_topo_del_port(dev, phydev->mod_port) whenever mod_port…
- [Low] The new module ports (phydev->mod_port from
phy_add_sfp_mod_port() and pl->mod_port from…
--
net: phy: Represent PHY-less SFP modules with phy_port
This commit adds a phy_port to represent PHY-less SFP modules. The port
belongs to the SFP bus upstream, which is either a PHY device or phylink.
It is added to the netdev's link topology on module_start and removed on
module_stop. The commit also adds phy_caps_linkmode_filter_ifaces(), which
works out the module port's supported link modes from the cage port's
interfaces.
> diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
> index 942d43191561e..558e4df4d63c0 100644
> --- a/drivers/net/phy/phy_caps.c
> +++ b/drivers/net/phy/phy_caps.c
> @@ -445,3 +445,29 @@ u32 phy_caps_mediums_from_linkmodes(unsigned long *linkmodes)
[ ... ]
> +void phy_caps_linkmode_filter_ifaces(unsigned long *to,
> + const unsigned long *from,
> + const unsigned long *interfaces)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(ifaces_supported) = {};
> + unsigned int ifaces_caps = 0;
> + phy_interface_t interface;
> +
> + for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
> + ifaces_caps |= phy_caps_from_interface(interface);
> +
> + phy_caps_linkmodes(ifaces_caps, ifaces_supported);
> +
> + linkmode_and(to, from, ifaces_supported);
> +}
[Severity: Medium]
Can this report link modes that the host can't run with the inserted
module?
This helper reduces each interface to speed/duplex caps, so the
encoding is lost. It also never looks at the module's own
caps->interfaces. Both phy_add_sfp_mod_port() and
phylink_add_sfp_mod_port() pass the raw EEPROM caps->link_modes through
it.
Take a cage port with interfaces {SGMII, 10GBASE-R} and a dual-rate
module with link_modes {1000baseX_Full, 10000baseSR_Full} and
caps->interfaces {1000BASEX, 10GBASER}. phylink_sfp_config_optical()
narrows the usable set to 10GBASE-R:
phy_interface_and(pl->sfp_interfaces, pl->config->supported_interfaces,
pl->sfp_interfaces);
SGMII maps to LINK_CAPA_1000FD here, so mod_port->supported still
contains 1000baseX_Full.
In the same way, a cage with {USXGMII, 1000BASEX} would keep
10000baseSR_Full, because USXGMII maps to LINK_CAPA_10000FD.
The ETHTOOL_MSG_PORT_GET support added later in the series passes this
mask to userspace unchanged.
Could the cage interfaces be intersected with caps->interfaces first?
For phylink, could the mask come from the validated pl->sfp_support
instead?
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 3c20d4cc1a79d..ab9631b161bc4 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
[ ... ]
> @@ -1527,6 +1539,75 @@ static void phy_sfp_link_down(void *upstream)
> port->ops->link_down(port);
> }
>
> +static int phy_add_sfp_mod_port(struct phy_device *phydev)
> +{
> + const struct sfp_module_caps *caps;
> + struct phy_port *port;
> + int ret = 0;
> +
> + /* Create mod port */
> + port = phy_port_alloc();
> + if (!port)
> + return -ENOMEM;
> +
> + port->active = true;
> +
> + caps = sfp_get_module_caps(phydev->sfp_bus);
> +
> + phy_caps_linkmode_filter_ifaces(port->supported, caps->link_modes,
> + phydev->sfp_cage_port->interfaces);
[Severity: Low]
This has no functional effect today, but should more of the port be
filled in before it goes into dev->link_topo->ports?
Only active and supported are set. phy_port_alloc() uses kzalloc, so
parent_type is 0 (PHY_PORT_PHY), phy and ops are NULL, and mediums is 0.
The struct phy_port kernel-doc says:
* @phy: If the parent is PHY_PORT_PHYDEV, the PHY controlling that port
* @mediums: Bitmask of the physical mediums this port provides access to
So a fibre module port claims a PHY parent but has no PHY. Also,
phy_port_get_type() and phy_port_is_fiber() would return PORT_OTHER and
false for it.
The supported mask is also missing some bits.
phy_caps_linkmode_filter_ifaces() builds its mask from
link_caps[].linkmodes, and phy_caps_init() only fills that in for link
modes with a known speed. As a result, the Autoneg, Pause, Asym_Pause,
FIBRE and TP bits in caps->link_modes don't make it into
mod_port->supported.
That doesn't match the MDI ports that phy_default_setup_single_port()
creates. The ETHTOOL_MSG_PORT_GET support added later in the series
reports this mask unchanged.
phylink_add_sfp_mod_port() has the same issue.
[ ... ]
> @@ -1651,6 +1735,8 @@ static int phy_sfp_probe(struct phy_device *phydev)
> */
> sfp_bus_put(bus);
>
> + phydev->sfp_cage_port = port;
> +
> return ret;
>
> out_port:
[Severity: Medium]
Is sfp_cage_port assigned too late here? By this point
sfp_bus_add_upstream() has already registered sfp_phydev_ops, including
the new .module_start, and has dropped RTNL.
phy_add_sfp_mod_port() dereferences the cage port without a NULL check,
unlike phylink_add_sfp_mod_port():
phy_caps_linkmode_filter_ifaces(port->supported, caps->link_modes,
phydev->sfp_cage_port->interfaces);
sfp_upstream_clear() doesn't clear bus->started. On a rebind,
sfp_register_bus() can therefore restart the state machine:
if (bus->started)
bus->socket_ops->start(bus->sfp);
If module_start runs before this assignment, it would read NULL on the
first bind.
On a rebind it would read a freed port. phy_sfp_release() destroys the
cage port but leaves phydev->sfp_cage_port pointing at it:
list_for_each_entry_safe(port, tmp, &phydev->ports, head) {
if (!port->is_sfp)
continue;
phy_del_port(phydev, port);
phy_port_destroy(port);
}
The window is small. module_start runs from the SFP delayed work at
least module_t_wait (about 50ms) after the upstream is added.
A later patch in this series, "net: phy: phy_port: Store information
about a port's upstream", moves this assignment before
sfp_bus_add_upstream() and clears it on the out_port error path. Would
it make sense to use that ordering in this patch instead?
[ ... ]
> @@ -1744,6 +1830,8 @@ static void phy_detach_internal(struct phy_device *phydev, bool notify_bus)
> phydev->attached_dev->phydev = NULL;
> phydev->attached_dev = NULL;
> phy_link_topo_del_phy(dev, phydev);
> + if (phydev->mod_port)
> + phy_link_topo_del_port(dev, phydev->mod_port);
> }
>
> phydev->phy_link_change = NULL;
> @@ -1873,6 +1961,12 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
> err = phy_link_topo_add_phy(dev, phydev, PHY_UPSTREAM_MAC, dev);
> if (err)
> goto error;
> +
> + if (phydev->mod_port) {
> + err = phy_link_topo_add_port(dev, phydev->mod_port);
> + if (err)
> + goto error;
> + }
> }
[Severity: Medium]
Can this error path remove an unrelated port from the new netdev's
topology?
Both goto error paths end up in phy_detach_internal(). That function
now calls phy_link_topo_del_port(dev, phydev->mod_port) whenever
mod_port is set, and phy_link_topo_del_port() erases by id with no
further check:
xa_erase(&topo->ports, port->id);
The first goto is taken when phy_link_topo_add_phy() fails. At that
point mod_port hasn't been added to this topology at all.
mod_port can also keep a stale non-zero id from an earlier netdev. For
example, the PHY is started with a PHY-less module fitted, and then
phy_detach() is called without phy_stop(). phy_detach() doesn't stop
the PHY; only phy_disconnect() does.
On the next phy_attach_direct(), phy_link_topo_add_port() reuses that
id:
ret = xa_insert(&topo->ports, port->id, port, GFP_KERNEL);
If another port already owns that id in the new topology, this fails
with -EBUSY. The error path would then erase the other port's entry.
phy_link_topo_del_phy() does the same thing for the PHY's own ports.
The mod_port case is new in this patch.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream
2026-09-27 13:36 ` [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream Maxime Chevallier
2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] `phy_set_upstream_port()`
(drivers/net/phy/phy_device.c:3702-3712) walks `phydev->ports` and…
- [Low] `phy_set_upstream_port()`
(drivers/net/phy/phy_device.c:3702-3712) sets `phydev->upstream_port`…
--
net: phy: phy_port: Store information about a port's upstream
This commit adds an upstream_port pointer to struct phy_port and struct
phy_device. With it, a SFP module port or a module PHY can record the SFP
cage MII port that feeds it. A new helper, phy_set_upstream_port(), is
called from the SFP connect/disconnect callbacks in phylib and phylink.
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index ab9631b161bc4..2bb4abd0d3924 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
[ ... ]
> @@ -1436,6 +1437,8 @@ static void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
>
> if (dev)
> phy_link_topo_del_phy(dev, phy);
> +
> + phy_set_upstream_port(phy, NULL);
> }
[Severity: Medium]
Can this walk of phy->ports race with phy_remove() on the module PHY?
phy_set_upstream_port() walks phy->ports and writes through each entry.
Only RTNL is held while it does this.
phy_remove()->phy_cleanup_ports() does list_del() and phy_port_destroy()
on the same entries without RTNL. It holds only the device lock from the
driver core.
For a module PHY, phydev->sfp_bus is NULL, so phy_sfp_release() does not
take RTNL either. PHY drivers do not set suppress_bind_attrs, so root can
unbind the module PHY's driver through sysfs:
CPU0 (rtnl_lock held)
sfp_check_state() / sfp_timeout()
sfp_sm_event()
sfp_sm_phy_detach()
sfp_remove_phy()
phy_sfp_disconnect_phy() / phylink_sfp_disconnect_phy()
phy_set_upstream_port(phy, NULL)
phy_for_each_port(phydev, local_port)
local_port->upstream_port = port;
CPU1 (device lock only)
sysfs unbind
device_release_driver()
phy_remove()
phy_cleanup_ports()
phy_del_port()
phy_port_destroy()
If these overlap, could CPU0 write NULL into a phy_port that CPU1 has
just freed? The same applies to the new call in
phylink_sfp_disconnect_phy().
This disconnect path already had an RTNL-only walk of phy->ports through
phy_link_topo_del_phy(). That walk was added earlier in this series by
"net: phy: phy_link_topology: Track ports in phy_link_topology". This
patch adds a second walker, and this one also writes through the entries.
The underlying gap seems to be that removing entries from phydev->ports
isn't serialized with RTNL.
[ ... ]
> @@ -3687,6 +3693,25 @@ struct phy_port *phy_get_sfp_port(struct phy_device *phydev)
> }
> EXPORT_SYMBOL_GPL(phy_get_sfp_port);
>
> +/**
> + * phy_set_upstream_port() - Sets the phy_port controlling the MII this PHY is
> + * attached to.
> + * @phydev: pointer to the PHY device we set the upstream of.
> + * @port: The phy_port upstream of this PHY, can be NULL.
> + */
> +void phy_set_upstream_port(struct phy_device *phydev, struct phy_port *port)
> +{
> + struct phy_port *local_port;
> +
> + ASSERT_RTNL();
> +
> + phydev->upstream_port = port;
> +
> + phy_for_each_port(phydev, local_port)
> + local_port->upstream_port = port;
> +}
[Severity: Low]
This copies the upstream only into the ports that are on phydev->ports
when it is called. What happens to ports added later?
phy_add_port() doesn't copy phydev->upstream_port into the new port:
drivers/net/phy/phy_device.c:phy_add_port() {
...
phy_port_update_supported(port);
list_add(&port->head, &phydev->ports);
phydev->n_ports++;
...
}
This can happen in two ways.
First, phy_sfp_connect_phy() accepts a module PHY that has no driver
bound. phylink_sfp_connect_phy() rejects this case with its phy->drv
check. If the driver binds later, phy_probe()->phy_setup_ports()->
phy_add_port() creates ports with upstream_port == NULL.
Second, the module PHY's driver can be unbound and rebound through sysfs
while the module stays connected. phy_remove()->phy_cleanup_ports() frees
the ports, and the next probe recreates them without an upstream.
In both cases, phydev->upstream_port points at the SFP cage port while
the phy_port entries have upstream_port == NULL. That doesn't match the
new kernel-doc for phy_port::upstream_port in include/linux/phy_port.h.
At the end of the series this has no visible effect. port_prepare_data()
in net/ethtool/port.c only sees ports in the netdev topology, and these
later ports are never added there. Nothing reads phydev->upstream_port
yet.
Would it make sense for phy_add_port() to inherit phydev->upstream_port
when it adds a port?
> +EXPORT_SYMBOL_GPL(phy_set_upstream_port);
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports
2026-09-27 13:36 ` [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports Maxime Chevallier
2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] phy_link_topo_get_port() (include/linux/phy_link_topology.h)
returns a raw, unreferenced struct phy_port pointer from topo->ports.
--
net: phy: phy_link_topology: Add a helper to retrieve ports
This adds phy_link_topo_get_port(), which returns a phy_port from the
netdev's link topology xarray. Callers must hold RTNL. The commit also
makes phy_link_topo_add_port() warn and return -EOPNOTSUPP for ops-locked
devices.
> diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
> index 296ee514ba463..f2e493c433fff 100644
> --- a/include/linux/phy_link_topology.h
> +++ b/include/linux/phy_link_topology.h
[ ... ]
> @@ -71,6 +72,24 @@ phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
> return NULL;
> }
>
> +static inline struct phy_port *
> +phy_link_topo_get_port(struct net_device *dev, u32 port_id)
> +{
> + struct phy_link_topology *topo = dev->link_topo;
> +
> + if (!topo)
> + return NULL;
> +
> + /* ports in the topo are RTNL protected, but may be accessed under
> + * netdev_lock for ops-locked devices. For now keep them under rtnl
> + * protection, as no ops-locked devices have phy_port capabilities
> + * yet.
> + */
> + ASSERT_RTNL();
> +
> + return xa_load(&topo->ports, port_id);
> +}
[Severity: High]
No reference is taken on the returned port. The only thing keeping it
alive is RTNL, and ASSERT_RTNL() only warns. The commit message states the
rule as "the caller must hold rtnl until it's done with it".
Do all the paths that erase and free ports hold RTNL?
phy_remove() is the phy_driver .remove callback. It runs from sysfs driver
unbind or MDIO bus device removal, and it does not hold RTNL:
phy_remove()
phy_sfp_release() / phy_cleanup_ports()
phy_del_port()
if (phydev->attached_dev)
phy_link_topo_del_port(phydev->attached_dev, port); <- xa_erase()
phy_port_destroy(port); <- kfree()
This path holds no RTNL and waits for no RCU grace period.
In contrast, phylink_destroy_sfp_cage_port() does wrap its delete in RTNL:
/* The port may still be accessed under RTNL */
rtnl_lock();
phy_link_topo_del_port(pl->netdev, pl->sfp_cage_port);
rtnl_unlock();
The teardown paths in phy_device.c do not do the same.
Later in the series, port_prepare_data() in net/ethtool/port.c adds a
reader. It holds RTNL, calls phy_link_topo_get_port(), and then reads
port->supported, port->interfaces, port->id, port->is_mii and port->is_sfp.
It also follows port->upstream_port->id.
The device_link created in phy_attach_direct() is DL_FLAG_STATELESS, so the
MAC is not unbound first and attached_dev stays set. Could an
ETHTOOL_MSG_PORT_GET that races with a PHY driver unbind then read a freed
struct phy_port?
This still looks present at the end of the series. Would it be possible to
take RTNL around the del and destroy steps in phy_sfp_release() and
phy_cleanup_ports()? Another option would be kfree_rcu() or refcounting for
the port.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool
2026-09-27 13:36 ` [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool Maxime Chevallier
2026-09-28 9:47 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] ETHTOOL_A_PORT_SUPPORTED_INTERFACES is added to the spec
(Documentation/netlink/specs/ethtool.yaml, attribute-set 'port',…
- [Low] Baseline rejected this as a false positive.
--
netlink: specs: Add ethernet port listing with ethtool
This commit adds the YAML spec, the generated uAPI header entries and the
documentation for a new ETHTOOL_MSG_PORT_GET netlink command. The command
reports the front-facing ports (phy_port) of a network interface, including
their type, supported link modes and supported MII interfaces.
> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index 5dd4d1b5d94bb..35e028c0be296 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml
[ ... ]
> @@ -1905,6 +1910,32 @@ attribute-sets:
> name: link
> type: nest
> nested-attributes: mse-snapshot
> + -
> + name: port
> + attr-cnt-name: --ethtool-a-port-cnt
> + attributes:
[ ... ]
> + -
> + name: supported-modes
> + type: nest
> + nested-attributes: bitset
> + -
> + name: supported-interfaces
> + type: nest
> + nested-attributes: bitset
[Severity: Medium]
How is userspace expected to decode the bits in this bitset?
Nothing in the uAPI says which bit index maps to which interface. The
implementation added later in the series, port_fill_reply() in
net/ethtool/port.c, emits it as:
ret = ethnl_put_bitset(skb, ETHTOOL_A_PORT_SUPPORTED_INTERFACES,
reply->interfaces, NULL,
PHY_INTERFACE_MODE_MAX,
phy_interface_names, compact);
That means the bit positions are the kernel-internal phy_interface_t values
from include/linux/phy.h. The names come from phy_interface_names[] in
net/ethtool/netlink.c, which is not exposed as an ETH_SS_* string set.
enum ethtool_stringset has ETH_SS_LINK_MODES for the sibling
supported-modes bitset, but it has no entry for PHY interface modes.
If the request header sets ETHTOOL_FLAG_COMPACT_BITSETS, ethnl_put_bitset()
sends only the size and value words, without the per-bit names. Would the
client then get raw phy_interface_t indices that it has no uAPI way to map
back to interface names?
It would also make the ordering of phy_interface_t de facto ABI. Adding or
reordering a mode would then change what compact-bitset users see.
Should this bitset get a matching string set? If not, should the spec and
ethtool-netlink.rst say that only the verbose form can be decoded? Right
now the documentation only says "bitset of supported MII interfaces".
[ ... ]
> @@ -2859,6 +2890,30 @@ operations:
> - worst-channel
> - link
> dump: *mse-get-op
> + -
> + name: port-get
> + doc: Get ports attached to an interface
> +
> + attribute-set: port
[Severity: Low]
This commit adds the port-get operation, the ETHTOOL_MSG_PORT_GET and
ETHTOOL_MSG_PORT_GET_REPLY IDs and the ETHTOOL_A_PORT_* attributes to the
uAPI. There is no kernel handler for them yet at this point in the series.
Would a kernel built at this commit return -EOPNOTSUPP for PORT_GET?
The next patch in the series, "net: ethtool: Introduce ethtool command to
list ports", adds net/ethtool/port.c. It also adds the
ethnl_default_requests[] entry and the ethtool_genl_ops entry for
ETHTOOL_MSG_PORT_GET. By the end of the series the command works, so this
only matters when bisecting to this intermediate commit.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports
2026-09-27 13:36 ` [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports Maxime Chevallier
2026-09-28 9:48 ` Christophe Leroy (CS GROUP)
@ 2026-09-29 0:38 ` netdev-bot+sashiko
1 sibling, 0 replies; 31+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 0:38 UTC (permalink / raw)
To: maxime.chevallier
Cc: davem, andrew, kuba, edumazet, pabeni, linux, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, christophe.leroy, herve.codina,
f.fainelli, vladimir.oltean, kory.maincent, kabel, o.rempel,
nicveronese, horms, mwojtas, romain.gantois, daniel,
dimitri.fedrau, frank.wunderlich, p.ameruoso, f
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 3 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] ETHTOOL_A_PORT_SUPPORTED_INTERFACES is sent as an ethtool
bitset.
- [Medium] In the unfiltered PORT_GET dump, ctx->pos_portid is carried
from one dumpit() call to the next without recording which device it…
- [Medium] port_dump_one_dev() reads dev->link_topo twice (the NULL
check, then &dev->link_topo->ports) and walks the ports xarray with no…
- [Low] The ETHTOOL_MSG_PORT_GET genl_ops entry in net/ethtool/netlink.c
uses ethnl_port_get_policy for both doit and dumpit.
--
net: ethtool: Introduce ethtool command to list ports
This adds a new ETHTOOL_MSG_PORT_GET netlink command that exposes phy_port
information to userspace. MDI ports report their supported linkmodes, and
MII ports (empty SFP cages) report their supported PHY interface modes.
The command has a doit handler and its own dump implementation.
> diff --git a/net/ethtool/port.c b/net/ethtool/port.c
> new file mode 100644
> index 0000000000000..0d6150e5f66a2
> --- /dev/null
> +++ b/net/ethtool/port.c
> @@ -0,0 +1,382 @@
[ ... ]
> +const struct nla_policy ethnl_port_get_policy[ETHTOOL_A_PORT_ID + 1] = {
> + [ETHTOOL_A_PORT_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
> + [ETHTOOL_A_PORT_ID] = NLA_POLICY_MIN(NLA_U32, 1),
> +};
[Severity: Low]
The doit and the dumpit of ETHTOOL_MSG_PORT_GET both use this policy. A
dump request that includes ETHTOOL_A_PORT_ID therefore passes validation.
ethnl_port_dump_start() only reads tb[ETHTOOL_A_PORT_HEADER]. The ID is
silently ignored and the dump returns every port. The port-get dump
request in Documentation/netlink/specs/ethtool.yaml only lists header.
Should the dump use a header-only policy, or reject ETHTOOL_A_PORT_ID?
That would keep "ignore the ID" from becoming the de facto behaviour, and
filtering by ID could still be added later.
[ ... ]
> +static int port_fill_reply(struct sk_buff *skb,
> + const struct ethnl_req_info *req_info,
> + const struct ethnl_reply_data *reply_data)
> +{
> + bool compact = req_info->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
[ ... ]
> + } else {
> + ret = ethnl_put_bitset(skb, ETHTOOL_A_PORT_SUPPORTED_INTERFACES,
> + reply->interfaces, NULL,
> + PHY_INTERFACE_MODE_MAX,
> + phy_interface_names, compact);
[Severity: Medium]
ETHTOOL_A_PORT_SUPPORTED_INTERFACES is sent as an ethtool bitset indexed
by phy_interface_t. The compact flag comes directly from
ETHTOOL_FLAG_COMPACT_BITSETS in the request header, and port_reply_size()
uses the same encoding. In compact mode, userspace only gets raw bit
positions.
How is userspace expected to map those bits to names?
Every other ethtool bitset has a matching string set in info_template in
net/ethtool/strset.c (ETH_SS_LINK_MODES, ETH_SS_FEATURES,
ETH_SS_WOL_MODES, ETH_SS_TS_FLAGS, and so on). That lets userspace decode
compact replies with ETHTOOL_MSG_STRSET_GET.
I couldn't find an ETH_SS_* id in include/uapi/linux/ethtool.h, or a
strset entry, for phy_interface_names.
Without one, a compact client can either request again in verbose mode or
hard-code the phy_interface_t numbering. phy_interface_t is defined in
include/linux/phy.h and is not append-only. PSGMII sits between MOCA and
QSGMII, and 5GBASER sits between 2500BASEX and RXAUI. Inserting a new mode
later would shift what each bit means.
Could phy_interface_names be exported as a new string set? Or could this
bitset always be sent in verbose form? The commit message doesn't say how
userspace is meant to decode it.
A smaller point: ethnl_phy_names_populate() in net/ethtool/netlink.c fills
entry 0 from phy_modes(PHY_INTERFACE_MODE_NA), which returns "":
for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) {
name = phy_modes(i);
strscpy(phy_interface_names[i], name, ETH_GSTRING_LEN);
}
A verbose bitset with that bit set would then carry an empty name. Is that
intended?
[ ... ]
> +static int port_dump_one_dev(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + struct port_dump_ctx *ctx = port_dump_ctx_get(cb);
> + struct net_device *dev;
> + struct phy_port *port;
> + int ret;
> +
> + dev = ctx->req_info->base.dev;
> +
> + if (!dev->link_topo)
> + return 0;
> +
> + xa_for_each_start(&dev->link_topo->ports, ctx->pos_portid, port,
> + ctx->pos_portid) {
[Severity: Medium]
port_dump_one_dev() reads dev->link_topo twice and walks the ports xarray
with no RTNL, no netdev instance lock and no READ_ONCE(). RTNL is only
taken later, once per port, inside port_dump_one().
On the writer side, the topology is allocated lazily and published with a
plain store:
drivers/net/phy/phy_link_topology.c:netdev_alloc_phy_link_topology() {
topo = kzalloc_obj(*topo);
...
xa_init_flags(&topo->ports, XA_FLAGS_ALLOC1);
topo->next_port_index = 1;
dev->link_topo = topo;
}
The comment in phy_link_topo_get_or_alloc() says this can run from
.ndo_open() or from the SFP state machine. At that point the netdev is
already registered and visible to dumps.
Can this race with an unprivileged PORT_GET dump?
CPU0 (ndo_open, RTNL held)
phy_attach_direct()
phy_link_topo_add_phy()
netdev_alloc_phy_link_topology()
dev->link_topo = topo;
CPU1 (PORT_GET dump)
port_dump_all_dev()
rcu_read_unlock();
port_dump_one_dev()
if (!dev->link_topo)
xa_for_each_start(&dev->link_topo->ports, ...)
On weakly ordered architectures such as arm64 or powerpc, could CPU1 see
the new pointer before the xa_init_flags() stores are visible? xa_find()
would then follow a stale xa_head. Even if that never happens in practice,
this looks like a plain load racing a plain store, which KCSAN would
report.
ethnl_perphy_dump_one_dev() reads the topology the same way, but this
patch adds another lockless reader. Would it be better to take RTNL around
the whole per-device walk, or to pair smp_store_release() with
smp_load_acquire() on dev->link_topo?
> + ctx->req_info->port_id = ctx->pos_portid;
> +
> + ret = port_dump_one(skb, dev, cb);
> + if (ret == -ENODEV)
> + continue;
> + if (ret)
> + return ret;
> + }
> +
> + ctx->pos_portid = 0;
> +
> + return 0;
> +}
> +
> +static int port_dump_all_dev(struct sk_buff *skb, struct netlink_callback *cb)
> +{
[ ... ]
> + rcu_read_lock();
> + for_each_netdev_dump(net, dev, ctx->pos_ifindex) {
> + netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
> + rcu_read_unlock();
> +
> + ctx->req_info->base.dev = dev;
> + ret = port_dump_one_dev(skb, cb);
> +
> + rcu_read_lock();
> + netdev_put(dev, &dev_tracker);
> + ctx->req_info->base.dev = NULL;
> +
> + if (ret < 0 && ret != -EOPNOTSUPP) {
> + if (likely(skb->len))
> + ret = skb->len;
> + break;
> + }
[Severity: Medium]
Suppose port_dump_one() returns -EMSGSIZE partway through a device's
ports. port_dump_one_dev() then returns with ctx->pos_portid still set to
that port's ID, and this loop breaks without advancing ctx->pos_ifindex.
pos_portid only goes back to 0 once every port on a device has been
dumped.
On the next dumpit() call, for_each_netdev_dump() resumes with xa_find()
from pos_ifindex. If that device was unregistered in between, xa_find()
returns the next device with a higher ifindex. port_dump_one_dev() then
starts walking that device's ports from the stale pos_portid:
xa_for_each_start(&dev->link_topo->ports, ctx->pos_portid, port,
ctx->pos_portid) {
Would ports with lower IDs on that unrelated device be silently dropped
from the dump, with no NLM_F_DUMP_INTR to tell userspace?
ethnl_perphy_dump_all_dev() has the same pattern with pos_phyindex, but
this code is new. Could the dump context record which ifindex pos_portid
belongs to, and reset pos_portid when the device changes?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927133619.955236-1-maxime.chevallier%40bootlin.com
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2026-09-29 0:38 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-27 13:36 [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Maxime Chevallier
2026-09-27 13:36 ` [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc Maxime Chevallier
2026-09-28 9:36 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology Maxime Chevallier
2026-09-28 9:37 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 03/10] net: phylink: Register a phy_port for MAC-driven SFP cages Maxime Chevallier
2026-09-28 9:38 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 04/10] net: phy: Create SFP phy_port before registering upstream Maxime Chevallier
2026-09-28 9:40 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 05/10] net: phy: Represent PHY-less SFP modules with phy_port Maxime Chevallier
2026-09-28 9:42 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 06/10] net: phy: phy_port: Store information about a port's upstream Maxime Chevallier
2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 07/10] net: phy: phy_link_topology: Add a helper to retrieve ports Maxime Chevallier
2026-09-28 9:43 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 08/10] netlink: specs: Add ethernet port listing with ethtool Maxime Chevallier
2026-09-28 9:47 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 09/10] net: ethtool: Introduce ethtool command to list ports Maxime Chevallier
2026-09-28 9:48 ` Christophe Leroy (CS GROUP)
2026-09-29 0:38 ` netdev-bot+sashiko
2026-09-27 13:36 ` [PATCH net-next v18 10/10] Documentation: networking: Update the phy_port infrastructure description Maxime Chevallier
2026-09-28 10:07 ` Christophe Leroy (CS GROUP)
2026-09-28 17:31 ` [PATCH net-next v18 00/10] net: phy_port: SFP modules representation and phy_port listing Christophe Leroy (CS GROUP)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®