[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250709133456.46a1908e@fedora>
Date: Wed, 9 Jul 2025 13:34:56 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: davem@...emloft.net
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
thomas.petazzoni@...tlin.com, Andrew Lunn <andrew@...n.ch>, Jakub Kicinski
<kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
<pabeni@...hat.com>, Russell King <linux@...linux.org.uk>, Florian Fainelli
<f.fainelli@...il.com>, Heiner Kallweit <hkallweit1@...il.com>, Vladimir
Oltean <vladimir.oltean@....com>, Köry Maincent
<kory.maincent@...tlin.com>, Oleksij Rempel <o.rempel@...gutronix.de>,
Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>,
linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/3] net: netdevsim: Add PHY support in
netdevsim
On Tue, 8 Jul 2025 13:55:28 +0200
Maxime Chevallier <maxime.chevallier@...tlin.com> 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
> 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@...tlin.com>
> ---
> 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 | 398 ++++++++++++++++++++++++++++++
> 5 files changed, 437 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 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..c73d16e67a3c 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);
Meh... that's why this fails, stray ';' that slept through the cracks
when building netdevsim with CONFIG_PHYLIB=n ...
I'll address that,
Maxime
Powered by blists - more mailing lists