Re: [PATCH net-next v11 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology

[Maxime Chevallier]

On 5/21/26 14:10, Maxime Chevallier wrote:
> 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@xxxxxxx>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
> ---
>  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 +
>  4 files changed, 74 insertions(+)
> 
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index fdfafd951905..207128303ca2 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>
> @@ -22,6 +23,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;
> @@ -44,12 +48,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);

Sashiko says :

"
  If this port is moved to a different netdev (which has a separate
  topology), and its old port->id happens to be in use by another port
  in the new topology, will xa_insert() return -EBUSY?

  Since there is no fallback to xa_alloc_cyclic() on -EBUSY and the
  stale ID is never reset to 0 upon removal, does this permanently
  prevent the port from attaching to the new topology?
"

So we can't move a port to another netdev, the closest thing we can
conceivably to is unbind/rebind a net_device while the phy_device
stays alive. Even then, we'll be re-adding this ports to a fresh
topology, so no collision risk here.

We don't re-set the port id to 0 upon removal on purpose, as implied
by the comment we try to reuse the id, as a much more common case is
that we attach/detach the phy_device that controls this port during
.ndo_open/close, and we don't want the port id to change at each
link flap

Maxime