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

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

[Maxime Chevallier]

There exist numerous ethtool commands that gets their information from
an interface's PHY. This series allows creating virtual PHY devices
attached to netdevsim, so that we can start testing these commands.

This first series adds a minimal support. The PHY that we add are only
capable of saying if the link is up or down, based on a debugfs file.

When accepted, this interface can be extended to allow testing further
commands, and in greater details.

This series also add some selftests for the "ethtool --show-phys"
command. This is a first step towards having better testability for the
ethtool netlink PHY commands, but this could also potentially be a
stepping stone for some basic phylib tests ?

Thanks,

Maxime

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/netdevsim/Makefile                |   4 +
 drivers/net/netdevsim/dev.c                   |   2 +
 drivers/net/netdevsim/netdev.c                |   3 +
 drivers/net/netdevsim/netdevsim.h             |  14 +
 drivers/net/netdevsim/phy.c                   | 387 ++++++++++++++++++
 .../selftests/drivers/net/netdevsim/config    |   1 +
 .../drivers/net/netdevsim/ethtool-common.sh   |  19 +-
 .../drivers/net/netdevsim/ethtool-phy.sh      |  64 +++
 8 files changed, 491 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 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/netdevsim/Makefile    |   4 +
 drivers/net/netdevsim/dev.c       |   2 +
 drivers/net/netdevsim/netdev.c    |   3 +
 drivers/net/netdevsim/netdevsim.h |  14 ++
 drivers/net/netdevsim/phy.c       | 387 ++++++++++++++++++++++++++++++
 5 files changed, 410 insertions(+)
 create mode 100644 drivers/net/netdevsim/phy.c

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 3e0b61202f0c..56209c5cc740 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1467,6 +1467,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;
 
@@ -1540,6 +1541,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..34bd48fc5b56 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);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 4a0c48c7a384..b4ead16137d7 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -313,6 +313,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;
@@ -418,6 +419,19 @@ 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);
+#else
+static inline void nsim_phy_init(struct netdevsim *ns)
+{
+}
+
+static inline void nsim_phy_teardown(struct netdevsim *ns);
+{
+}
+#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..aee97167e4c2
--- /dev/null
+++ b/drivers/net/netdevsim/phy.c
@@ -0,0 +1,387 @@
+// 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,
+	},
+};
+
+module_phy_driver(nsim_virtual_phy_drv);
+
+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 int nsim_phy_state_link_set(void *data, u64 val)
+{
+	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
+
+	ns_phy->link = !!val;
+
+	phy_trigger_machine(ns_phy->phy);
+
+	return 0;
+}
+
+static int nsim_phy_state_link_get(void *data, u64 *val)
+{
+	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
+
+	*val = ns_phy->link;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(nsim_phy_state_link_fops, nsim_phy_state_link_get,
+			 nsim_phy_state_link_set, "%llu\n");
+
+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_file("link", 0600, ns_phy->phy_dir, ns_phy, &nsim_phy_state_link_fops);
+}
+
+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);
+}
-- 
2.49.0

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

[kernel test robot]

Hi Maxime,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Maxime-Chevallier/net-netdevsim-Add-PHY-support-in-netdevsim/20250702-163058
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250702082806.706973-2-maxime.chevallier%40bootlin.com
patch subject: [PATCH net-next 1/3] net: netdevsim: Add PHY support in netdevsim
config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20250704/202507041906.JXjAtVqe-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250704/202507041906.JXjAtVqe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507041906.JXjAtVqe-lkp@intel.com/

All errors (new ones prefixed by >>):

   microblaze-linux-ld: drivers/net/netdevsim/phy.o: in function `phy_module_exit':
>> phy.o:(.exit.text+0x0): multiple definition of `cleanup_module'; drivers/net/netdevsim/netdev.o:netdev.o:(.exit.text+0x0): first defined here
   microblaze-linux-ld: drivers/net/netdevsim/phy.o: in function `phy_module_init':
>> phy.o:(.init.text+0x0): multiple definition of `init_module'; drivers/net/netdevsim/netdev.o:netdev.o:(.init.text+0x0): first defined here

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Simon Horman]

On Wed, Jul 02, 2025 at 10:28:03AM +0200, Maxime Chevallier wrote:
> 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

Hi Maxime,

As it seems like there will be a v2 anyway: discovered

> phy_link_topology.

...

> +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,
> +	},
> +};
> +
> +module_phy_driver(nsim_virtual_phy_drv);

I see that this has been flagged by Kernel Test Robot,
but as I had already written most of this it seems worth sending anyway.

I am somewhat guessing at the why here, but
I see build failures with this patch applied:

ld: drivers/net/netdevsim/phy.o: in function `phy_module_init':
phy.c:(.init.text+0x0): multiple definition of `init_module'; drivers/net/netdevsim/netdev.o:netdev.c:(.init.text+0x0): first defined here
ld: drivers/net/netdevsim/phy.o: in function `phy_module_exit':
phy.c:(.exit.text+0x0): multiple definition of `cleanup_module'; drivers/net/netdevsim/netdev.o:netdev.c:(.exit.text+0x0): first defined here

I am guessing that this is because above module_phy_driver() will define
init_module and phy_module_exit functions.  But the following lines near
the end of drivers/net/netdevsim/netdev.c also define functions with those
names.

module_init(nsim_module_init);
module_exit(nsim_module_exit);

...

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

[Maxime Chevallier]

Hi Simon,

On Fri, 4 Jul 2025 13:43:36 +0100
Simon Horman <horms@kernel.org> wrote:

> On Wed, Jul 02, 2025 at 10:28:03AM +0200, Maxime Chevallier wrote:
> > 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  
> 
> Hi Maxime,
> 
> As it seems like there will be a v2 anyway: discovered

Thanks :)

> > phy_link_topology.  
> 
> ...
> 
> > +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,
> > +	},
> > +};
> > +
> > +module_phy_driver(nsim_virtual_phy_drv);  
> 
> I see that this has been flagged by Kernel Test Robot,
> but as I had already written most of this it seems worth sending anyway.
> 
> I am somewhat guessing at the why here, but
> I see build failures with this patch applied:
> 
> ld: drivers/net/netdevsim/phy.o: in function `phy_module_init':
> phy.c:(.init.text+0x0): multiple definition of `init_module'; drivers/net/netdevsim/netdev.o:netdev.c:(.init.text+0x0): first defined here
> ld: drivers/net/netdevsim/phy.o: in function `phy_module_exit':
> phy.c:(.exit.text+0x0): multiple definition of `cleanup_module'; drivers/net/netdevsim/netdev.o:netdev.c:(.exit.text+0x0): first defined here
> 
> I am guessing that this is because above module_phy_driver() will define
> init_module and phy_module_exit functions.  But the following lines near
> the end of drivers/net/netdevsim/netdev.c also define functions with those
> names.
> 
> module_init(nsim_module_init);
> module_exit(nsim_module_exit);
> 
> ...

I just received the kernel test robot report indeed :( Thanks for the
investigation ! I'll rework that part, sorry about that :/

Maxime

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

[Andrew Lunn]

> +static int nsim_phy_state_link_set(void *data, u64 val)
> +{
> +	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
> +
> +	ns_phy->link = !!val;
> +
> +	phy_trigger_machine(ns_phy->phy);
> +
> +	return 0;
> +}
> +
> +static int nsim_phy_state_link_get(void *data, u64 *val)
> +{
> +	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
> +
> +	*val = ns_phy->link;
> +
> +	return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(nsim_phy_state_link_fops, nsim_phy_state_link_get,
> +			 nsim_phy_state_link_set, "%llu\n");
> +
> +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_file("link", 0600, ns_phy->phy_dir, ns_phy, &nsim_phy_state_link_fops);

Maybe this can be converted into:

debugfs_create_bool("link", 0600, ns_phy->phy_dir, &ns_phy->link);

You loose the phy_trigger_machine(), but that might actually be
good. PHYs are async to any operation you take on them. It can take up
to 1 second due to the polling before any change is reported. So any
user space tools which expect an immediate state change are broken. So
leaving the PHY state machine to poll the PHY to notice the link has
changed is a better simulation.

	Andrew

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

[Maxime Chevallier]

Hi Andrew,

On Tue, 8 Jul 2025 16:01:06 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > +static int nsim_phy_state_link_set(void *data, u64 val)
> > +{
> > +	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
> > +
> > +	ns_phy->link = !!val;
> > +
> > +	phy_trigger_machine(ns_phy->phy);
> > +
> > +	return 0;
> > +}
> > +
> > +static int nsim_phy_state_link_get(void *data, u64 *val)
> > +{
> > +	struct nsim_phy_device *ns_phy = (struct nsim_phy_device *)data;
> > +
> > +	*val = ns_phy->link;
> > +
> > +	return 0;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(nsim_phy_state_link_fops, nsim_phy_state_link_get,
> > +			 nsim_phy_state_link_set, "%llu\n");
> > +
> > +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_file("link", 0600, ns_phy->phy_dir, ns_phy, &nsim_phy_state_link_fops);  
> 
> Maybe this can be converted into:
> 
> debugfs_create_bool("link", 0600, ns_phy->phy_dir, &ns_phy->link);
> 
> You loose the phy_trigger_machine(), but that might actually be
> good. PHYs are async to any operation you take on them. It can take up
> to 1 second due to the polling before any change is reported. So any
> user space tools which expect an immediate state change are broken. So
> leaving the PHY state machine to poll the PHY to notice the link has
> changed is a better simulation.

That's true indeed... Simpler and more accurate, I'll add that in V3.

Thanks for the feedback :)

Maxime

[PATCH net-next 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

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

[Vladimir Oltean]

On Wed, Jul 02, 2025 at 10:28:04AM +0200, Maxime Chevallier wrote:
> 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>
> ---

The story seems to be told by commits
fbb8531e58bd ("selftests: extract common functions in ethtool-common.sh") and
9e48ee80ac4e ("selftests: refactor get_netdev_name function").

make_netdev() returns the name of the newly created netdevsim interface.
It used to be implemented by comparing the "before" and "after" of
"ls /sys/class/net". But the name can instead be retrieved from /sys/bus/netdevsim,
and "ls /sys/class/net" is a left-over that was never fully cleaned up.

Meanwhile, udp_tunnel_nic.sh still uses the old and racy method of
finding the netdevsim name, based on the before/after comparison.
It seems like a case of not converging the entire netdevsim test suite
towards the same solution: ethtool-common.sh modified the logic in Nov
2020, udp_tunnel_nic.sh had the logic since Jul 2020.

[PATCH net-next 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..7b740a3fda1d
--- /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 2>&1 >/dev/null; 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 "PHY index" | wc -l)
+check $? "$n_phy" "3"
+
+# Check filtered Dump
+n_phy=$(ethtool --show-phys $NSIM_NETDEV | grep "PHY index" | wc -l)
+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 3/3] selftests: ethtool: Introduce ethernet PHY selftests on netdevsim

[Simon Horman]

On Wed, Jul 02, 2025 at 10:28:05AM +0200, Maxime Chevallier wrote:
> 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>

Hi Maxime,

We have recently started running shellcheck as part of our CI (NIPA).
Could you do so for the scripts added and modified by this patch?

> ---
>  .../selftests/drivers/net/netdevsim/config    |  1 +
>  .../drivers/net/netdevsim/ethtool-common.sh   | 15 +++++
>  .../drivers/net/netdevsim/ethtool-phy.sh      | 64 +++++++++++++++++++

Should ethtool-phy.sh be added to TEST_PROGS the Makefile in
the same directory?

...

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

[Maxime Chevallier]

On Fri, 4 Jul 2025 14:20:19 +0100
Simon Horman <horms@kernel.org> wrote:

> On Wed, Jul 02, 2025 at 10:28:05AM +0200, Maxime Chevallier wrote:
> > 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>  
> 
> Hi Maxime,
> 
> We have recently started running shellcheck as part of our CI (NIPA).
> Could you do so for the scripts added and modified by this patch?

Sure thing, I'll do that :)

> 
> > ---
> >  .../selftests/drivers/net/netdevsim/config    |  1 +
> >  .../drivers/net/netdevsim/ethtool-common.sh   | 15 +++++
> >  .../drivers/net/netdevsim/ethtool-phy.sh      | 64 +++++++++++++++++++  
> 
> Should ethtool-phy.sh be added to TEST_PROGS the Makefile in
> the same directory?

Ah yes I forgot that. So any file in that TEST_PROGS list will end-up
being run in NAPI tests ?

Maxime

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

[Simon Horman]

On Fri, Jul 04, 2025 at 03:32:50PM +0200, Maxime Chevallier wrote:
> On Fri, 4 Jul 2025 14:20:19 +0100
> Simon Horman <horms@kernel.org> wrote:
> 
> > On Wed, Jul 02, 2025 at 10:28:05AM +0200, Maxime Chevallier wrote:
> > > 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>  
> > 
> > Hi Maxime,
> > 
> > We have recently started running shellcheck as part of our CI (NIPA).
> > Could you do so for the scripts added and modified by this patch?
> 
> Sure thing, I'll do that :)

Thanks.

> > > ---
> > >  .../selftests/drivers/net/netdevsim/config    |  1 +
> > >  .../drivers/net/netdevsim/ethtool-common.sh   | 15 +++++
> > >  .../drivers/net/netdevsim/ethtool-phy.sh      | 64 +++++++++++++++++++  
> > 
> > Should ethtool-phy.sh be added to TEST_PROGS the Makefile in
> > the same directory?
> 
> Ah yes I forgot that. So any file in that TEST_PROGS list will end-up
> being run in NAPI tests ?

I assume so.