[PATCH net-next v3 0/3] netdevsim: add support for PHY devices

[PATCH net-next v3 0/3] netdevsim: add support for PHY devices

[Maxime Chevallier]

Hi everyone,

Here's a V3 for the netdevsim PHY support. This V3 includes :
 - A fix for a compiling issue with PHYLIB=n
 - An updated KConfig to only allow PHYLIB=y|n
 - Converted the link setting file to a bool debugfs file, relying on
   link state polling

The idea of this series is to allow attaching virtual PHY devices to
netdevsim, so that we can test PHY-related ethtool commands. This can be
extended in the future for phylib testing as well.

V2: https://lore.kernel.org/netdev/20250708115531.111326-1-maxime.chevallier@bootlin.com/
 - Fix building with PHYLIB=m
 - Use shellcheck on the shell scripts

V1: https://lore.kernel.org/netdev/20250702082806.706973-1-maxime.chevallier@bootlin.com/

Maxime Chevallier (3):
  net: netdevsim: Add PHY support in netdevsim
  selftests: ethtool: Drop the unused old_netdevs variable
  selftests: ethtool: Introduce ethernet PHY selftests on netdevsim

 drivers/net/Kconfig                           |   1 +
 drivers/net/netdevsim/Makefile                |   4 +
 drivers/net/netdevsim/dev.c                   |   2 +
 drivers/net/netdevsim/netdev.c                |   8 +
 drivers/net/netdevsim/netdevsim.h             |  25 ++
 drivers/net/netdevsim/phy.c                   | 375 ++++++++++++++++++
 .../selftests/drivers/net/netdevsim/config    |   1 +
 .../drivers/net/netdevsim/ethtool-common.sh   |  19 +-
 .../drivers/net/netdevsim/ethtool-phy.sh      |  64 +++
 9 files changed, 496 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/netdevsim/phy.c
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/ethtool-phy.sh

-- 
2.49.0

[PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Maxime Chevallier]

With the introduction of phy_link_topology, we have the ability to keep
track of PHY devices that sit behind a net_device. While we still can
only attach one single PHY to a netdev, we can look at all these PHYs
through netlink, with the ETHTOOL_MSG_PHY_GET command.

Moreover, netlink commands that are targeting PHY devices also now
allow specifying which PHY we want to address in a given netlink
command.

That whole process comes with its own complexity, and a few bugs were
dicovered over the months following the introduction of
phy_link_topology.

As devices with multiple PHYs are fairly uncommon, testing the corner
cases of multi-phy setups proves to be difficult.

To that extent, introduce PHY support in netdevsim. The main goal (for
now) is not to be able to test PHYlib, but these phy-specific
netlink interfaces.

These netdevsim PHYs use a custom phy_driver that relies on
re-implementing the phy_driver callbacks. In other words, this is not a
PHY driver that relies on mdio emulation, and will not work with any of
the genphy helpers.

The debugfs API for PHY creation and deletion works as follows :

PHY device creation :

echo $ID > /sys/kernel/debug/netdevsim/netdevsimXXX/ports/YY/phy_add

if $ID is 0, then the PHY parent will be the netdev corresponding to the
port's netdev. The first PHY that is added with the netdev as a parent
will be attached to the netdev.

if $ID > 0, the index must correspond to a previously added PHY. This
allows creating any arbitrary tree of PHYs.

Upon PHY addition, a phyXX directory will be created, XX being the
phyindex of the PHY in the topology:

 [...]/ports/YY/phyXX/

This directory contains a "link" file, allowing to toggle the virtual
PHY's link state.

One can then list the PHYs with "ethtool --show-phys ethX".

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/Kconfig               |   1 +
 drivers/net/netdevsim/Makefile    |   4 +
 drivers/net/netdevsim/dev.c       |   2 +
 drivers/net/netdevsim/netdev.c    |   8 +
 drivers/net/netdevsim/netdevsim.h |  25 ++
 drivers/net/netdevsim/phy.c       | 375 ++++++++++++++++++++++++++++++
 6 files changed, 415 insertions(+)
 create mode 100644 drivers/net/netdevsim/phy.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index b29628d46be9..4f277b04fdea 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -630,6 +630,7 @@ config NETDEVSIM
 	depends on IPV6 || IPV6=n
 	depends on PSAMPLE || PSAMPLE=n
 	depends on PTP_1588_CLOCK_MOCK || PTP_1588_CLOCK_MOCK=n
+	depends on PHYLIB || PHYLIB=n
 	select NET_DEVLINK
 	select PAGE_POOL
 	select NET_SHAPER
diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index f8de93bc5f5b..49f4c515e5e3 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -21,3 +21,7 @@ endif
 ifneq ($(CONFIG_MACSEC),)
 netdevsim-objs += macsec.o
 endif
+
+ifneq ($(CONFIG_PHYLIB),)
+netdevsim-objs += phy.o
+endif
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index b3647691060c..1ebb4f5b3bdd 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1510,6 +1510,7 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
 	devlink = priv_to_devlink(nsim_dev);
 	nsim_dev = devlink_priv(devlink);
 	INIT_LIST_HEAD(&nsim_dev->port_list);
+	INIT_LIST_HEAD(&nsim_dev->phy_list);
 	nsim_dev->fw_update_status = true;
 	nsim_dev->fw_update_overwrite_mask = 0;
 
@@ -1583,6 +1584,7 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
 	nsim_dev->switch_id.id_len = sizeof(nsim_dev->switch_id.id);
 	get_random_bytes(nsim_dev->switch_id.id, nsim_dev->switch_id.id_len);
 	INIT_LIST_HEAD(&nsim_dev->port_list);
+	INIT_LIST_HEAD(&nsim_dev->phy_list);
 	nsim_dev->fw_update_status = true;
 	nsim_dev->fw_update_overwrite_mask = 0;
 	nsim_dev->max_macs = NSIM_DEV_MAX_MACS_DEFAULT;
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index e36d3e846c2d..ff891536f691 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -952,6 +952,7 @@ static int nsim_init_netdevsim(struct netdevsim *ns)
 
 	nsim_macsec_init(ns);
 	nsim_ipsec_init(ns);
+	nsim_phy_init(ns);
 
 	err = register_netdevice(ns->netdev);
 	if (err)
@@ -968,6 +969,7 @@ static int nsim_init_netdevsim(struct netdevsim *ns)
 	return 0;
 
 err_ipsec_teardown:
+	nsim_phy_teardown(ns);
 	nsim_ipsec_teardown(ns);
 	nsim_macsec_teardown(ns);
 	nsim_bpf_uninit(ns);
@@ -1058,6 +1060,7 @@ void nsim_destroy(struct netdevsim *ns)
 	RCU_INIT_POINTER(ns->peer, NULL);
 	unregister_netdevice(dev);
 	if (nsim_dev_port_is_pf(ns->nsim_dev_port)) {
+		nsim_phy_teardown(ns);
 		nsim_macsec_teardown(ns);
 		nsim_ipsec_teardown(ns);
 		nsim_bpf_uninit(ns);
@@ -1098,6 +1101,10 @@ static int __init nsim_module_init(void)
 {
 	int err;
 
+	err = nsim_phy_drv_register();
+	if (err)
+		return err;
+
 	err = nsim_dev_init();
 	if (err)
 		return err;
@@ -1124,6 +1131,7 @@ static void __exit nsim_module_exit(void)
 	rtnl_link_unregister(&nsim_link_ops);
 	nsim_bus_exit();
 	nsim_dev_exit();
+	nsim_phy_drv_unregister();
 }
 
 module_init(nsim_module_init);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 809dd29fc5fe..b7d75c636be9 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -314,6 +314,7 @@ struct nsim_dev {
 	struct list_head bpf_bound_maps;
 	struct netdev_phys_item_id switch_id;
 	struct list_head port_list;
+	struct list_head phy_list;
 	bool fw_update_status;
 	u32 fw_update_overwrite_mask;
 	u32 max_macs;
@@ -419,6 +420,30 @@ static inline void nsim_macsec_teardown(struct netdevsim *ns)
 }
 #endif
 
+#if IS_ENABLED(CONFIG_PHYLIB)
+void nsim_phy_init(struct netdevsim *ns);
+void nsim_phy_teardown(struct netdevsim *dev);
+int nsim_phy_drv_register(void);
+void nsim_phy_drv_unregister(void);
+#else
+static inline void nsim_phy_init(struct netdevsim *ns)
+{
+}
+
+static inline void nsim_phy_teardown(struct netdevsim *ns)
+{
+}
+
+static inline int __init nsim_phy_drv_register(void)
+{
+	return 0;
+}
+
+static inline void __exit nsim_phy_drv_unregister(void)
+{
+}
+#endif
+
 struct nsim_bus_dev {
 	struct device dev;
 	struct list_head list;
diff --git a/drivers/net/netdevsim/phy.c b/drivers/net/netdevsim/phy.c
new file mode 100644
index 000000000000..6ef64e3fdf3c
--- /dev/null
+++ b/drivers/net/netdevsim/phy.c
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2025 Maxime Chevallier <maxime.chevallier@bootlin.com>
+
+#include <linux/debugfs.h>
+#include <linux/list.h>
+#include <linux/netdevice.h>
+#include <linux/slab.h>
+#include <linux/phy.h>
+#include <linux/phy_fixed.h>
+#include <linux/phy_link_topology.h>
+#include <linux/platform_device.h>
+
+#include "netdevsim.h"
+
+static atomic_t bus_num = ATOMIC_INIT(0);
+
+/* Dumb MDIO bus for the virtual PHY to sit on */
+struct nsim_mdiobus {
+	struct platform_device *pdev;
+	struct mii_bus *mii;
+};
+
+static int nsim_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
+{
+	return 0;
+}
+
+static int nsim_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
+			   u16 val)
+{
+	return 0;
+}
+
+struct nsim_phy_device {
+	struct phy_device *phy;
+	struct dentry *phy_dir;
+
+	struct list_head node;
+
+	bool link;
+};
+
+/* Virtual PHY driver for netdevsim */
+static int nsim_match_phy_device(struct phy_device *phydev,
+				 const struct phy_driver *drv)
+{
+	struct mii_bus *mii = phydev->mdio.bus;
+
+	return (mii->read == nsim_mdio_read) &&
+	       (mii->write == nsim_mdio_write);
+}
+
+static int nsim_get_features(struct phy_device *phydev)
+{
+	/* Act like a 1G PHY */
+	linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, phydev->supported);
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, phydev->supported);
+
+	return 0;
+}
+
+static int nsim_config_aneg(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int nsim_read_status(struct phy_device *phydev)
+{
+	struct nsim_phy_device *ns_phy = phydev->priv;
+
+	if (!ns_phy)
+		return 0;
+
+	if (ns_phy->link) {
+		phydev->speed = SPEED_1000;
+		phydev->duplex = DUPLEX_FULL;
+	} else {
+		phydev->speed = SPEED_UNKNOWN;
+		phydev->duplex = DUPLEX_UNKNOWN;
+	}
+
+	phydev->link = ns_phy->link;
+
+	return 0;
+}
+
+static struct phy_driver nsim_virtual_phy_drv[] = {
+	{
+		.name			= "Netdevsim virtual PHY driver",
+		.get_features		= nsim_get_features,
+		.match_phy_device	= nsim_match_phy_device,
+		.config_aneg		= nsim_config_aneg,
+		.read_status		= nsim_read_status,
+	},
+};
+
+static struct nsim_mdiobus *nsim_mdiobus_create(void)
+{
+	struct nsim_mdiobus *mb;
+
+	mb = kzalloc(sizeof(*mb), GFP_KERNEL);
+	if (!mb)
+		return NULL;
+
+	mb->pdev = platform_device_register_simple("nsim MDIO bus",
+						   atomic_read(&bus_num),
+						   NULL, 0);
+	if (IS_ERR(mb->pdev))
+		goto free_mb;
+
+	mb->mii = mdiobus_alloc();
+	if (!mb->mii)
+		goto free_pdev;
+
+	snprintf(mb->mii->id, MII_BUS_ID_SIZE, "nsim-%d", atomic_read(&bus_num));
+	atomic_inc(&bus_num);
+	mb->mii->name = "nsim MDIO Bus";
+	mb->mii->priv = mb;
+	mb->mii->parent = &mb->pdev->dev;
+	mb->mii->read = &nsim_mdio_read;
+	mb->mii->write = &nsim_mdio_write;
+	mb->mii->phy_mask = ~0;
+
+	if (mdiobus_register(mb->mii))
+		goto free_mdiobus;
+
+	return mb;
+
+free_mdiobus:
+	atomic_dec(&bus_num);
+	mdiobus_free(mb->mii);
+free_pdev:
+	platform_device_unregister(mb->pdev);
+free_mb:
+	kfree(mb);
+
+	return NULL;
+}
+
+static void nsim_mdiobus_destroy(struct nsim_mdiobus *mb)
+{
+	mdiobus_unregister(mb->mii);
+	mdiobus_free(mb->mii);
+	atomic_dec(&bus_num);
+	platform_device_unregister(mb->pdev);
+	kfree(mb);
+}
+
+static struct nsim_phy_device *nsim_phy_register(void)
+{
+	struct nsim_phy_device *ns_phy;
+	struct nsim_mdiobus *mb;
+	int err;
+
+	mb = nsim_mdiobus_create();
+	if (IS_ERR(mb))
+		return ERR_CAST(mb);
+
+	ns_phy = kzalloc(sizeof(*ns_phy), GFP_KERNEL);
+	if (!ns_phy) {
+		err = -ENOMEM;
+		goto out_mdio;
+	}
+
+	INIT_LIST_HEAD(&ns_phy->node);
+
+	ns_phy->phy = get_phy_device(mb->mii, 0, false);
+	if (IS_ERR(ns_phy->phy)) {
+		err = PTR_ERR(ns_phy->phy);
+		goto out_phy_free;
+	}
+
+	err = phy_device_register(ns_phy->phy);
+	if (err)
+		goto out_phy;
+
+	ns_phy->phy->priv = ns_phy;
+
+	return ns_phy;
+
+out_phy:
+	phy_device_free(ns_phy->phy);
+out_phy_free:
+	kfree(ns_phy);
+out_mdio:
+	nsim_mdiobus_destroy(mb);
+	return ERR_PTR(err);
+}
+
+static void nsim_phy_destroy(struct nsim_phy_device *ns_phy)
+{
+	struct phy_device *phydev = ns_phy->phy;
+	struct mii_bus *mii = phydev->mdio.bus;
+	struct nsim_mdiobus *mb = mii->priv;
+
+	debugfs_remove_recursive(ns_phy->phy_dir);
+
+	phy_device_remove(phydev);
+	list_del(&ns_phy->node);
+	kfree(ns_phy);
+
+	nsim_mdiobus_destroy(mb);
+}
+
+static void nsim_phy_debugfs_create(struct nsim_dev_port *port,
+				    struct nsim_phy_device *ns_phy)
+{
+	char phy_dir_name[sizeof("phy") + 10];
+
+	sprintf(phy_dir_name, "phy%u", ns_phy->phy->phyindex);
+
+	/* create debugfs stuff */
+	ns_phy->phy_dir = debugfs_create_dir(phy_dir_name, port->ddir);
+
+	debugfs_create_bool("link", 0600, ns_phy->phy_dir, &ns_phy->link);
+}
+
+static void nsim_adjust_link(struct net_device *dev)
+{
+	phy_print_status(dev->phydev);
+}
+
+static ssize_t
+nsim_phy_add_write(struct file *file, const char __user *data,
+		   size_t count, loff_t *ppos)
+{
+	struct net_device *dev = file->private_data;
+	struct netdevsim *ns = netdev_priv(dev);
+	struct nsim_phy_device *ns_phy;
+	struct phy_device *pphy;
+	u32 parent_id;
+	char buf[10];
+	ssize_t ret;
+	int err;
+
+	if (*ppos != 0)
+		return 0;
+
+	if (count >= sizeof(buf))
+		return -ENOSPC;
+
+	ret = copy_from_user(buf, data, count);
+	if (ret)
+		return -EFAULT;
+	buf[count] = '\0';
+
+	ret = kstrtouint(buf, 10, &parent_id);
+	if (ret)
+		return -EINVAL;
+
+	ns_phy = nsim_phy_register();
+	if (IS_ERR(ns_phy))
+		return PTR_ERR(ns_phy);
+
+	if (!parent_id) {
+		if (!dev->phydev) {
+			err = phy_connect_direct(dev, ns_phy->phy, nsim_adjust_link,
+						 PHY_INTERFACE_MODE_NA);
+			if (err)
+				return err;
+
+			phy_attached_info(ns_phy->phy);
+
+			phy_start(ns_phy->phy);
+		} else {
+			phy_link_topo_add_phy(dev, ns_phy->phy, PHY_UPSTREAM_MAC, dev);
+		}
+	} else {
+		pphy = phy_link_topo_get_phy(dev, parent_id);
+		if (!pphy)
+			return -EINVAL;
+
+		phy_link_topo_add_phy(dev, ns_phy->phy, PHY_UPSTREAM_PHY, pphy);
+	}
+
+	nsim_phy_debugfs_create(ns->nsim_dev_port, ns_phy);
+
+	list_add(&ns_phy->node, &ns->nsim_dev->phy_list);
+
+	return count;
+}
+
+static const struct file_operations nsim_phy_add_fops = {
+	.open = simple_open,
+	.write = nsim_phy_add_write,
+	.llseek = generic_file_llseek,
+	.owner = THIS_MODULE,
+};
+
+static ssize_t
+nsim_phy_del_write(struct file *file, const char __user *data,
+		   size_t count, loff_t *ppos)
+{
+	struct net_device *dev = file->private_data;
+	struct nsim_phy_device *ns_phy;
+	struct phy_device *phydev;
+	u32 phy_index;
+	char buf[10];
+	ssize_t ret;
+
+	if (*ppos != 0)
+		return 0;
+
+	if (count >= sizeof(buf))
+		return -ENOSPC;
+
+	ret = copy_from_user(buf, data, count);
+	if (ret)
+		return -EFAULT;
+	buf[count] = '\0';
+
+	ret = kstrtouint(buf, 10, &phy_index);
+	if (ret)
+		return -EINVAL;
+
+	phydev = phy_link_topo_get_phy(dev, phy_index);
+	if (!phydev)
+		return -ENODEV;
+
+	ns_phy = phydev->priv;
+
+	if (dev->phydev && dev->phydev == phydev) {
+		phy_stop(phydev);
+		phy_detach(phydev);
+	} else {
+		phy_link_topo_del_phy(dev, phydev);
+	}
+
+	nsim_phy_destroy(ns_phy);
+
+	return count;
+}
+
+static const struct file_operations nsim_phy_del_fops = {
+	.open = simple_open,
+	.write = nsim_phy_del_write,
+	.llseek = generic_file_llseek,
+	.owner = THIS_MODULE,
+};
+
+void nsim_phy_init(struct netdevsim *ns)
+{
+	debugfs_create_file("phy_add", 0200, ns->nsim_dev_port->ddir,
+			    ns->netdev, &nsim_phy_add_fops);
+
+	debugfs_create_file("phy_del", 0200, ns->nsim_dev_port->ddir,
+			    ns->netdev, &nsim_phy_del_fops);
+}
+
+void nsim_phy_teardown(struct netdevsim *ns)
+{
+	struct nsim_phy_device *ns_phy, *pos;
+
+	list_for_each_entry_safe(ns_phy, pos, &ns->nsim_dev->phy_list, node)
+		nsim_phy_destroy(ns_phy);
+}
+
+int __init nsim_phy_drv_register(void)
+{
+	return phy_drivers_register(nsim_virtual_phy_drv,
+				    ARRAY_SIZE(nsim_virtual_phy_drv),
+				    THIS_MODULE);
+}
+
+void __exit nsim_phy_drv_unregister(void)
+{
+	phy_drivers_unregister(nsim_virtual_phy_drv,
+			       ARRAY_SIZE(nsim_virtual_phy_drv));
+}
-- 
2.49.0

Re: [PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Jakub Kicinski]

On Thu, 10 Jul 2025 08:22:45 +0200 Maxime Chevallier wrote:
> @@ -1098,6 +1101,10 @@ static int __init nsim_module_init(void)
>  {
>  	int err;
>  
> +	err = nsim_phy_drv_register();
> +	if (err)
> +		return err;
> +
>  	err = nsim_dev_init();
>  	if (err)
>  		return err;

I think you're missing error handling in this function if something
after drv_register fails.

> @@ -1124,6 +1131,7 @@ static void __exit nsim_module_exit(void)
>  	rtnl_link_unregister(&nsim_link_ops);
>  	nsim_bus_exit();
>  	nsim_dev_exit();
> +	nsim_phy_drv_unregister();
>  }

> +free_mdiobus:
> +	atomic_dec(&bus_num);
> +	mdiobus_free(mb->mii);
> +free_pdev:
> +	platform_device_unregister(mb->pdev);
> +free_mb:

Others have added netdevsim code so the entire code base doesn't follow
what I'm about to say, but if you dont mind indulging my personal coding
style - error handling labels on a path disjoint from the success path
should be prefixed with err_$first-undo-action. If the error handling
shares the path with success the label prefix should be exit_$..
You can look at drivers/net/netdevsim/bpf.c for examples

> +	kfree(mb);
> +
> +	return NULL;
> +}
> +
> +static ssize_t
> +nsim_phy_add_write(struct file *file, const char __user *data,
> +		   size_t count, loff_t *ppos)
> +{
> +	struct net_device *dev = file->private_data;
> +	struct netdevsim *ns = netdev_priv(dev);
> +	struct nsim_phy_device *ns_phy;
> +	struct phy_device *pphy;
> +	u32 parent_id;
> +	char buf[10];
> +	ssize_t ret;
> +	int err;
> +
> +	if (*ppos != 0)
> +		return 0;
> +
> +	if (count >= sizeof(buf))
> +		return -ENOSPC;
> +
> +	ret = copy_from_user(buf, data, count);
> +	if (ret)
> +		return -EFAULT;
> +	buf[count] = '\0';
> +
> +	ret = kstrtouint(buf, 10, &parent_id);
> +	if (ret)
> +		return -EINVAL;
> +
> +	ns_phy = nsim_phy_register();
> +	if (IS_ERR(ns_phy))
> +		return PTR_ERR(ns_phy);
> +
> +	if (!parent_id) {
> +		if (!dev->phydev) {
> +			err = phy_connect_direct(dev, ns_phy->phy, nsim_adjust_link,
> +						 PHY_INTERFACE_MODE_NA);
> +			if (err)
> +				return err;
> +
> +			phy_attached_info(ns_phy->phy);
> +
> +			phy_start(ns_phy->phy);
> +		} else {
> +			phy_link_topo_add_phy(dev, ns_phy->phy, PHY_UPSTREAM_MAC, dev);
> +		}
> +	} else {
> +		pphy = phy_link_topo_get_phy(dev, parent_id);
> +		if (!pphy)
> +			return -EINVAL;
> +
> +		phy_link_topo_add_phy(dev, ns_phy->phy, PHY_UPSTREAM_PHY, pphy);
> +	}
> +
> +	nsim_phy_debugfs_create(ns->nsim_dev_port, ns_phy);
> +
> +	list_add(&ns_phy->node, &ns->nsim_dev->phy_list);

No locks needed.. for any of this.. ?
-- 
pw-bot: cr

Re: [PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Maxime Chevallier]

On Fri, 11 Jul 2025 16:55:41 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu, 10 Jul 2025 08:22:45 +0200 Maxime Chevallier wrote:
> > @@ -1098,6 +1101,10 @@ static int __init nsim_module_init(void)
> >  {
> >  	int err;
> >  
> > +	err = nsim_phy_drv_register();
> > +	if (err)
> > +		return err;
> > +
> >  	err = nsim_dev_init();
> >  	if (err)
> >  		return err;  
> 
> I think you're missing error handling in this function if something
> after drv_register fails.

Ah true... Thanks

> > @@ -1124,6 +1131,7 @@ static void __exit nsim_module_exit(void)
> >  	rtnl_link_unregister(&nsim_link_ops);
> >  	nsim_bus_exit();
> >  	nsim_dev_exit();
> > +	nsim_phy_drv_unregister();
> >  }  
> 
> > +free_mdiobus:
> > +	atomic_dec(&bus_num);
> > +	mdiobus_free(mb->mii);
> > +free_pdev:
> > +	platform_device_unregister(mb->pdev);
> > +free_mb:  
> 
> Others have added netdevsim code so the entire code base doesn't follow
> what I'm about to say, but if you dont mind indulging my personal coding
> style - error handling labels on a path disjoint from the success path
> should be prefixed with err_$first-undo-action. If the error handling
> shares the path with success the label prefix should be exit_$..
> You can look at drivers/net/netdevsim/bpf.c for examples

That's totally fine by me, it makes sense :)

> > +	kfree(mb);
> > +
> > +	return NULL;
> > +}
> > +
> > +static ssize_t
> > +nsim_phy_add_write(struct file *file, const char __user *data,
> > +		   size_t count, loff_t *ppos)
> > +{
> > +	struct net_device *dev = file->private_data;
> > +	struct netdevsim *ns = netdev_priv(dev);
> > +	struct nsim_phy_device *ns_phy;
> > +	struct phy_device *pphy;
> > +	u32 parent_id;
> > +	char buf[10];
> > +	ssize_t ret;
> > +	int err;
> > +
> > +	if (*ppos != 0)
> > +		return 0;
> > +
> > +	if (count >= sizeof(buf))
> > +		return -ENOSPC;
> > +
> > +	ret = copy_from_user(buf, data, count);
> > +	if (ret)
> > +		return -EFAULT;
> > +	buf[count] = '\0';
> > +
> > +	ret = kstrtouint(buf, 10, &parent_id);
> > +	if (ret)
> > +		return -EINVAL;
> > +
> > +	ns_phy = nsim_phy_register();
> > +	if (IS_ERR(ns_phy))
> > +		return PTR_ERR(ns_phy);
> > +
> > +	if (!parent_id) {
> > +		if (!dev->phydev) {
> > +			err = phy_connect_direct(dev, ns_phy->phy, nsim_adjust_link,
> > +						 PHY_INTERFACE_MODE_NA);
> > +			if (err)
> > +				return err;
> > +
> > +			phy_attached_info(ns_phy->phy);
> > +
> > +			phy_start(ns_phy->phy);
> > +		} else {
> > +			phy_link_topo_add_phy(dev, ns_phy->phy, PHY_UPSTREAM_MAC, dev);
> > +		}
> > +	} else {
> > +		pphy = phy_link_topo_get_phy(dev, parent_id);
> > +		if (!pphy)
> > +			return -EINVAL;
> > +
> > +		phy_link_topo_add_phy(dev, ns_phy->phy, PHY_UPSTREAM_PHY, pphy);
> > +	}
> > +
> > +	nsim_phy_debugfs_create(ns->nsim_dev_port, ns_phy);
> > +
> > +	list_add(&ns_phy->node, &ns->nsim_dev->phy_list);  
> 
> No locks needed.. for any of this.. ?

Heh I guess some locking is needed indeed... Let me add that in the
next round...

Maxime

Re: [PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Andrew Lunn]

> +static int nsim_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
> +{
> +	return 0;
> +}
> +
> +static int nsim_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
> +			   u16 val)
> +{
> +	return 0;
> +}

If i'm reading the code correctly, each PHY has its own MDIO bus? And
the PHY is always at address 0?

Maybe for address != 0, these should return -ENODEV?

I'm guessing the PHY core is going to perform reads/writes for things
like EEE? And if the MAC driver has an IOCTL handler, it could also do
reads/writes. So something is needed here, but i do wounder if hard
coded 0 is going to work out O.K? Have you looked at what accesses the
core actually does?

     Andrew

Re: [PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Maxime Chevallier]

On Sat, 12 Jul 2025 18:54:38 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > +static int nsim_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
> > +{
> > +	return 0;
> > +}
> > +
> > +static int nsim_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
> > +			   u16 val)
> > +{
> > +	return 0;
> > +}  
> 
> If i'm reading the code correctly, each PHY has its own MDIO bus? And
> the PHY is always at address 0?

Yes indeed. 

> Maybe for address != 0, these should return -ENODEV?

That could be done yes, but I don't think this will ever happen as this
is only ever going to be used in netdevsim, which also controls the PHY
instantiation. I'm OK to add the ENODEV though :)

For the record, the first draft implementation I had locally used a
single MDIO bus on which we could register up to 32 netdevsim PHYs, but
that wasn't enough. At some point Jakub pointed me to the case of
netlink DUMP requests that would be too large to fit in a single
netlink message (default threshold for that is > 4K messages), so to
test that with the phy_link_topology stuff, I had to add around 150
PHYs...

> I'm guessing the PHY core is going to perform reads/writes for things
> like EEE? And if the MAC driver has an IOCTL handler, it could also do
> reads/writes. So something is needed here, but i do wounder if hard
> coded 0 is going to work out O.K? Have you looked at what accesses the
> core actually does?

I don't see that driver being useful outside of netdevsim, so at
least we have a good idea of what the MAC driver will do.

There'e no ioctl, but we can be on the safe side and stub it a bit more.

From the tests I've been running, I didn't encounter any side-effect
but I only tested very simple cases (set link up, run ethtool, etc.)

I've considered re-using swphy for example, and using an emulated MDIO
bus for netdevsim PHY, but my fear was that this would in the end get
too complex. Let's say we want to add a new netdevsim debugfs
file that allows us to control what is the speed reported by the phy,
if we want to report say 10G speeds, we also need to emulate C45, etc.

I guess at some point, we will run into scenarios where phylib is going
to call some genphy helpers, it really depends on how deep we want to go
with this netdevsim PHY driver.

The use-cases that I saw were :
 - Being able to test the netlink stuff now that we have the ability to
know which PHY handles which command
 - In the future, write some tests for the linkmode reporting (that I
broke once with phy_caps)
 - When PHY Muxes eventually get there, I'd also like to netdevsim
that.

However that's just my view of the thing, we could also consider having
this driver rely purely on MDIO emulation, where the MDIO registers
would behave exactly as specified in C22/C37/C45, but that's quite more
complex :)

I'm open for any improvement or ideas on that driver :)

>      Andrew

Thanks for the review,

Maxime

Re: [PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Andrew Lunn]

On Tue, Jul 15, 2025 at 08:05:32AM +0200, Maxime Chevallier wrote:
> On Sat, 12 Jul 2025 18:54:38 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > > +static int nsim_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static int nsim_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
> > > +			   u16 val)
> > > +{
> > > +	return 0;
> > > +}  
> > 
> > If i'm reading the code correctly, each PHY has its own MDIO bus? And
> > the PHY is always at address 0?
> 
> Yes indeed. 
> 
> > Maybe for address != 0, these should return -ENODEV?
> 
> That could be done yes, but I don't think this will ever happen as this
> is only ever going to be used in netdevsim, which also controls the PHY
> instantiation. I'm OK to add the ENODEV though :)

It makes the simulation more realistic. The other option is to return
0xffff on read, which is what a real MDIO bus would do when you access
an address without a device.

It currently should not happen, but this is the sort of framework
which will get expanded with time. So this low hanging fruit could
avoid issues later.

> For the record, the first draft implementation I had locally used a
> single MDIO bus on which we could register up to 32 netdevsim PHYs, but
> that wasn't enough. At some point Jakub pointed me to the case of
> netlink DUMP requests that would be too large to fit in a single
> netlink message (default threshold for that is > 4K messages), so to
> test that with the phy_link_topology stuff, I had to add around 150
> PHYs...

I'm happy with an MDIO bus per PHY, for the reasons you give.

> > I'm guessing the PHY core is going to perform reads/writes for things
> > like EEE? And if the MAC driver has an IOCTL handler, it could also do
> > reads/writes. So something is needed here, but i do wounder if hard
> > coded 0 is going to work out O.K? Have you looked at what accesses the
> > core actually does?
> 
> I don't see that driver being useful outside of netdevsim, so at
> least we have a good idea of what the MAC driver will do.
>
> There'e no ioctl, but we can be on the safe side and stub it a bit more.
> 
> From the tests I've been running, I didn't encounter any side-effect
> but I only tested very simple cases (set link up, run ethtool, etc.)

It is hard to say where this will lead. We have seen bugs with EEE, so
being able to test that might be interesting to someone. Given that is
all in the core, that would require implementing the C45 over C22 and
the EEE registers.

> I've considered re-using swphy for example, and using an emulated MDIO
> bus for netdevsim PHY, but my fear was that this would in the end get
> too complex.

Yes, i thought about that too. But i agree, that is the wrong way to
go. We would end up adding a lot of features to swphy which never get
used in real system, and potentially had bugs to real systems. An
emulated PHY for netdevsim might start as a copy/paste of swphy, but i
would then expect it to quickly diverge.

	Andrew

Re: [PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Maxime Chevallier]

> > On Sat, 12 Jul 2025 18:54:38 +0200
> > Andrew Lunn <andrew@lunn.ch> wrote:
> >   
> > > > +static int nsim_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
> > > > +{
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int nsim_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
> > > > +			   u16 val)
> > > > +{
> > > > +	return 0;
> > > > +}    
> > > 
> > > If i'm reading the code correctly, each PHY has its own MDIO bus? And
> > > the PHY is always at address 0?  
> > 
> > Yes indeed. 
> >   
> > > Maybe for address != 0, these should return -ENODEV?  
> > 
> > That could be done yes, but I don't think this will ever happen as this
> > is only ever going to be used in netdevsim, which also controls the PHY
> > instantiation. I'm OK to add the ENODEV though :)  
> 
> It makes the simulation more realistic. The other option is to return
> 0xffff on read, which is what a real MDIO bus would do when you access
> an address without a device.
> 
> It currently should not happen, but this is the sort of framework
> which will get expanded with time. So this low hanging fruit could
> avoid issues later.

True, I'm fine with the 0xffff return on address != 0

> > For the record, the first draft implementation I had locally used a
> > single MDIO bus on which we could register up to 32 netdevsim PHYs, but
> > that wasn't enough. At some point Jakub pointed me to the case of
> > netlink DUMP requests that would be too large to fit in a single
> > netlink message (default threshold for that is > 4K messages), so to
> > test that with the phy_link_topology stuff, I had to add around 150
> > PHYs...  
> 
> I'm happy with an MDIO bus per PHY, for the reasons you give.
> 
> > > I'm guessing the PHY core is going to perform reads/writes for things
> > > like EEE? And if the MAC driver has an IOCTL handler, it could also do
> > > reads/writes. So something is needed here, but i do wounder if hard
> > > coded 0 is going to work out O.K? Have you looked at what accesses the
> > > core actually does?  
> > 
> > I don't see that driver being useful outside of netdevsim, so at
> > least we have a good idea of what the MAC driver will do.
> >
> > There'e no ioctl, but we can be on the safe side and stub it a bit more.
> > 
> > From the tests I've been running, I didn't encounter any side-effect
> > but I only tested very simple cases (set link up, run ethtool, etc.)  
> 
> It is hard to say where this will lead. We have seen bugs with EEE, so
> being able to test that might be interesting to someone. Given that is
> all in the core, that would require implementing the C45 over C22 and
> the EEE registers.
> 
> > I've considered re-using swphy for example, and using an emulated MDIO
> > bus for netdevsim PHY, but my fear was that this would in the end get
> > too complex.  
> 
> Yes, i thought about that too. But i agree, that is the wrong way to
> go. We would end up adding a lot of features to swphy which never get
> used in real system, and potentially had bugs to real systems. An
> emulated PHY for netdevsim might start as a copy/paste of swphy, but i
> would then expect it to quickly diverge.

So should we instead move to a netdevsim PHY model where we would
emulate an mdio bus and use a pure genphy driver instead ?

If you see some future for nsim PHY as a testing ground for phylib (and
not only for ethnl), that would make sense :)

Maxime

Re: [PATCH net-next v3 1/3] net: netdevsim: Add PHY support in netdevsim

[Andrew Lunn]

> So should we instead move to a netdevsim PHY model where we would
> emulate an mdio bus and use a pure genphy driver instead ?

I think what you have is a good start. It actually requires somebody
who is interesting in testing to take it further. And i doubt a pure
genphy driver is sufficient, i guess it will need a driver specific
for simulation. There is some things you can test via the PHY driver
API, and other stuff you need to emulate registers. So it will end up
with a hybrid of both.

	Andrew

[PATCH net-next v3 2/3] selftests: ethtool: Drop the unused old_netdevs variable

[Maxime Chevallier]

old_netdevs is unused in ethtool-common.sh. Only the UDP tunnels test
uses that variable, but it maintains it locally.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 .../testing/selftests/drivers/net/netdevsim/ethtool-common.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
index 80160579e0cc..d9c7a3d397a9 100644
--- a/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
@@ -43,13 +43,11 @@ function check {
 }
 
 function make_netdev {
-    # Make a netdevsim
-    old_netdevs=$(ls /sys/class/net)
-
     if ! $(lsmod | grep -q netdevsim); then
 	modprobe netdevsim
     fi
 
+    # Make a netdevsim
     echo $NSIM_ID $@ > /sys/bus/netdevsim/new_device
     udevadm settle
     # get new device name
-- 
2.49.0

[PATCH net-next v3 3/3] selftests: ethtool: Introduce ethernet PHY selftests on netdevsim

[Maxime Chevallier]

Now that netdevsim supports PHY device simulation, we can start writing
some tests to cover a little bit all PHY-related ethtool commands.

So far we only test the basic use of "ethtool --show-phys", with :
 - A simple command to get a PHY we just added
 - A DUMP command listing PHYs on multiple netdevsim instances
 - A Filtered DUMP command listing all PHYs on a netdevsim

Introduce some helpers to create netdevsim PHYs, and a new test file.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 .../selftests/drivers/net/netdevsim/config    |  1 +
 .../drivers/net/netdevsim/ethtool-common.sh   | 15 +++++
 .../drivers/net/netdevsim/ethtool-phy.sh      | 64 +++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/ethtool-phy.sh

diff --git a/tools/testing/selftests/drivers/net/netdevsim/config b/tools/testing/selftests/drivers/net/netdevsim/config
index 5117c78ddf0a..223e82cb7759 100644
--- a/tools/testing/selftests/drivers/net/netdevsim/config
+++ b/tools/testing/selftests/drivers/net/netdevsim/config
@@ -6,6 +6,7 @@ CONFIG_NETDEVSIM=m
 CONFIG_NET_SCH_MQPRIO=y
 CONFIG_NET_SCH_MULTIQ=y
 CONFIG_NET_SCH_PRIO=y
+CONFIG_PHYLIB=m
 CONFIG_PSAMPLE=y
 CONFIG_PTP_1588_CLOCK_MOCK=y
 CONFIG_VXLAN=m
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
index d9c7a3d397a9..1bd0ac5e7bba 100644
--- a/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
@@ -53,3 +53,18 @@ function make_netdev {
     # get new device name
     ls /sys/bus/netdevsim/devices/netdevsim${NSIM_ID}/net/
 }
+
+function make_phydev_on_netdev {
+    local parent_ndev_nsim_id=$1
+    local parent=$2
+
+    local ndev_dfs=/sys/kernel/debug/netdevsim/netdevsim$parent_ndev_nsim_id/ports/0
+
+    old_dev_dfs=$(find $ndev_dfs -type d)
+    echo $parent > $ndev_dfs/phy_add
+    new_dev_dfs=$(find $ndev_dfs -type d)
+
+    # The new phydev name corresponds to the new file that was created. Its
+    # name isn't predictable.
+    echo $old_dev_dfs $new_dev_dfs | xargs -n1 | sort  | uniq -u
+}
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-phy.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-phy.sh
new file mode 100755
index 000000000000..b10440d108b2
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-phy.sh
@@ -0,0 +1,64 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+source ethtool-common.sh
+
+# Bail if ethtool is too old
+if ! ethtool -h | grep show-phys >/dev/null 2>&1; then
+    echo "SKIP: No --show-phys support in ethtool"
+    exit 4
+fi
+
+function make_netdev_from_id {
+    local new_nsim_id="$1"
+    # Make a netdevsim
+    echo "$new_nsim_id" > /sys/bus/netdevsim/new_device
+    udevadm settle
+    # get new device name
+    ls /sys/bus/netdevsim/devices/netdevsim"${new_nsim_id}"/net/
+}
+
+function cleanup_netdev_from_id {
+    local to_del_nsim_id="$1"
+    echo "$to_del_nsim_id" > /sys/bus/netdevsim/del_device
+}
+
+NSIM_NETDEV=$(make_netdev)
+
+set -o pipefail
+
+# Check simple PHY addition and listing
+
+# Parent == 0 means that the PHY's parent is the netdev
+PHY_DFS=$(make_phydev_on_netdev "$NSIM_ID" 0)
+
+# First PHY gets index 1
+index=$(ethtool --show-phys "$NSIM_NETDEV" | grep "PHY index" | cut -d ' ' -f 3)
+check $? "$index" "1"
+
+# Insert a second PHY, same parent. It gets index 2.
+PHY2_DFS=$(make_phydev_on_netdev "$NSIM_ID" 0)
+
+# Create another netdev
+NSIM_ID2=$((RANDOM % 1024))
+NSIM_NETDEV_2=$(make_netdev_from_id "$NSIM_ID2")
+
+PHY3_DFS=$(make_phydev_on_netdev "$NSIM_ID2" 0);
+
+# Check unfiltered PHY Dump
+n_phy=$(ethtool --show-phys '*' | grep -c "PHY index")
+check $? "$n_phy" "3"
+
+# Check filtered Dump
+n_phy=$(ethtool --show-phys "$NSIM_NETDEV" | grep -c "PHY index")
+check $? "$n_phy" "2"
+
+cleanup_netdev_from_id "$NSIM_ID2"
+
+if [ "$num_errors" -eq 0 ]; then
+    echo "PASSED all $((num_passes)) checks"
+    exit 0
+else
+    echo "FAILED $num_errors/$((num_errors+num_passes)) checks"
+    exit 1
+fi
-- 
2.49.0

Re: [PATCH net-next v3 3/3] selftests: ethtool: Introduce ethernet PHY selftests on netdevsim

[Jakub Kicinski]

On Thu, 10 Jul 2025 08:22:47 +0200 Maxime Chevallier wrote:
> +set -o pipefail
> +
> +# Check simple PHY addition and listing
> +
> +# Parent == 0 means that the PHY's parent is the netdev
> +PHY_DFS=$(make_phydev_on_netdev "$NSIM_ID" 0)
> +
> +# First PHY gets index 1
> +index=$(ethtool --show-phys "$NSIM_NETDEV" | grep "PHY index" | cut -d ' ' -f 3)
> +check $? "$index" "1"
> +
> +# Insert a second PHY, same parent. It gets index 2.
> +PHY2_DFS=$(make_phydev_on_netdev "$NSIM_ID" 0)
> +
> +# Create another netdev
> +NSIM_ID2=$((RANDOM % 1024))
> +NSIM_NETDEV_2=$(make_netdev_from_id "$NSIM_ID2")
> +
> +PHY3_DFS=$(make_phydev_on_netdev "$NSIM_ID2" 0);
> +
> +# Check unfiltered PHY Dump
> +n_phy=$(ethtool --show-phys '*' | grep -c "PHY index")
> +check $? "$n_phy" "3"
> +
> +# Check filtered Dump
> +n_phy=$(ethtool --show-phys "$NSIM_NETDEV" | grep -c "PHY index")
> +check $? "$n_phy" "2"

Not a very strong preference, but I wonder if we should wire up the
paths to the Python lib for drivers/net/netdevsim and switch to Python?
It does the setup and cleanup and it gives us direct YNL access.
More convenient for testing new stuff than jugging ethtool builds..
But I guess you could argue that testing the CLI is good in itself.

Re: [PATCH net-next v3 3/3] selftests: ethtool: Introduce ethernet PHY selftests on netdevsim

[Maxime Chevallier]

Hi Jakub,

On Fri, 11 Jul 2025 16:58:13 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu, 10 Jul 2025 08:22:47 +0200 Maxime Chevallier wrote:
> > +set -o pipefail
> > +
> > +# Check simple PHY addition and listing
> > +
> > +# Parent == 0 means that the PHY's parent is the netdev
> > +PHY_DFS=$(make_phydev_on_netdev "$NSIM_ID" 0)
> > +
> > +# First PHY gets index 1
> > +index=$(ethtool --show-phys "$NSIM_NETDEV" | grep "PHY index" | cut -d ' ' -f 3)
> > +check $? "$index" "1"
> > +
> > +# Insert a second PHY, same parent. It gets index 2.
> > +PHY2_DFS=$(make_phydev_on_netdev "$NSIM_ID" 0)
> > +
> > +# Create another netdev
> > +NSIM_ID2=$((RANDOM % 1024))
> > +NSIM_NETDEV_2=$(make_netdev_from_id "$NSIM_ID2")
> > +
> > +PHY3_DFS=$(make_phydev_on_netdev "$NSIM_ID2" 0);
> > +
> > +# Check unfiltered PHY Dump
> > +n_phy=$(ethtool --show-phys '*' | grep -c "PHY index")
> > +check $? "$n_phy" "3"
> > +
> > +# Check filtered Dump
> > +n_phy=$(ethtool --show-phys "$NSIM_NETDEV" | grep -c "PHY index")
> > +check $? "$n_phy" "2"  
> 
> Not a very strong preference, but I wonder if we should wire up the
> paths to the Python lib for drivers/net/netdevsim and switch to Python?
> It does the setup and cleanup and it gives us direct YNL access.
> More convenient for testing new stuff than jugging ethtool builds..
> But I guess you could argue that testing the CLI is good in itself.

That's totally OK for me, looking at selftests/drivers/net/netdevsim/
it was unclear to me what was the proper way forward.

My python skills are not the best, I'm basically writing C using python
syntax, but I can certainly give that a shot.

Thanks,

Maxime