2023-12-01 16:37:23

by Maxime Chevallier

[permalink] [raw]
Subject: [RFC PATCH net-next v3 01/13] net: phy: Introduce ethernet link topology representation

Link topologies containing multiple network PHYs attached to the same
net_device can be found when using a PHY as a media converter for use
with an SFP connector, on which an SFP transceiver containing a PHY can
be used.

With the current model, the transceiver's PHY can't be used for
operations such as cable testing, timestamping, macsec offload, etc.

The reason being that most of the logic for these configuration, coming
from either ethtool netlink or ioctls tend to use netdev->phydev, which
in multi-phy systems will reference the PHY closest to the MAC.

Introduce a numbering scheme allowing to enumerate PHY devices that
belong to any netdev, which can in turn allow userspace to take more
precise decisions with regard to each PHY's configuration.

The numbering is maintained per-netdev, in a phy_device_list.
The numbering works similarly to a netdevice's ifindex, with
identifiers that are only recycled once INT_MAX has been reached.

This prevents races that could occur between PHY listing and SFP
transceiver removal/insertion.

The identifiers are assigned at phy_attach time, as the numbering
depends on the netdevice the phy is attached to.

Signed-off-by: Maxime Chevallier <[email protected]>
---
V3: - Renamed to phy_link_topology
- Added assertions for RTNL
- Various cleanups of leftover, unused test code
- Made the PHY index u32

MAINTAINERS | 2 +
drivers/net/phy/Makefile | 2 +-
drivers/net/phy/phy_device.c | 15 +++++
drivers/net/phy/phy_link_topology.c | 85 ++++++++++++++++++++++++++
include/linux/netdevice.h | 4 +-
include/linux/phy.h | 4 ++
include/linux/phy_link_topology.h | 64 +++++++++++++++++++
include/linux/phy_link_topology_core.h | 15 +++++
include/uapi/linux/ethtool.h | 16 +++++
net/core/dev.c | 3 +
10 files changed, 208 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/phy/phy_link_topology.c
create mode 100644 include/linux/phy_link_topology.h
create mode 100644 include/linux/phy_link_topology_core.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 509281e9e169..54d50510c299 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7922,6 +7922,8 @@ F: include/linux/mii.h
F: include/linux/of_net.h
F: include/linux/phy.h
F: include/linux/phy_fixed.h
+F: include/linux/phy_link_topology.h
+F: include/linux/phy_link_topology_core.h
F: include/linux/phylib_stubs.h
F: include/linux/platform_data/mdio-bcm-unimac.h
F: include/linux/platform_data/mdio-gpio.h
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index f65e85c91fc1..3cf7774df57e 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,7 +2,7 @@
# Makefile for Linux PHY drivers

libphy-y := phy.o phy-c45.o phy-core.o phy_device.o \
- linkmode.o
+ linkmode.o phy_link_topology.o
mdio-bus-y += mdio_bus.o mdio_device.o

ifdef CONFIG_MDIO_DEVICE
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 478126f6b5bc..b921a091d636 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -29,6 +29,7 @@
#include <linux/phy.h>
#include <linux/phylib_stubs.h>
#include <linux/phy_led_triggers.h>
+#include <linux/phy_link_topology.h>
#include <linux/pse-pd/pse.h>
#include <linux/property.h>
#include <linux/rtnetlink.h>
@@ -265,6 +266,14 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)

static struct phy_driver genphy_driver;

+static struct phy_link_topology *phy_get_link_topology(struct phy_device *phydev)
+{
+ if (phydev->attached_dev)
+ return &phydev->attached_dev->link_topo;
+
+ return NULL;
+}
+
static LIST_HEAD(phy_fixup_list);
static DEFINE_MUTEX(phy_fixup_lock);

@@ -1488,6 +1497,11 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,

if (phydev->sfp_bus_attached)
dev->sfp_bus = phydev->sfp_bus;
+
+ err = phy_link_topo_add_phy(&dev->link_topo, phydev,
+ PHY_UPSTREAM_MAC, dev);
+ if (err)
+ goto error;
}

/* Some Ethernet drivers try to connect to a PHY device before
@@ -1813,6 +1827,7 @@ void phy_detach(struct phy_device *phydev)
if (dev) {
phydev->attached_dev->phydev = NULL;
phydev->attached_dev = NULL;
+ phy_link_topo_del_phy(&dev->link_topo, phydev);
}
phydev->phylink = NULL;

diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
new file mode 100644
index 000000000000..4bb8b1eb7df6
--- /dev/null
+++ b/drivers/net/phy/phy_link_topology.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Infrastructure to handle all PHY devices connected to a given netdev,
+ * either directly or indirectly attached.
+ *
+ * Copyright (c) 2023 Maxime Chevallier<[email protected]>
+ */
+
+#include <linux/phy_link_topology.h>
+#include <linux/netdevice.h>
+#include <linux/phy.h>
+#include <linux/rtnetlink.h>
+#include <linux/xarray.h>
+
+struct phy_device *phy_link_topo_get_phy(struct phy_link_topology *topo,
+ u32 phyindex)
+{
+ struct phy_device_node *pdn = xa_load(&topo->phys, phyindex);
+
+ if (pdn)
+ return pdn->phy;
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(phy_link_topo_get_phy);
+
+int phy_link_topo_add_phy(struct phy_link_topology *topo,
+ struct phy_device *phy,
+ enum phy_upstream upt, void *upstream)
+{
+ struct phy_device_node *pdn;
+ int ret;
+
+ /* Protects phy and upstream */
+ ASSERT_RTNL();
+
+ pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
+ if (!pdn)
+ return -ENOMEM;
+
+ pdn->phy = phy;
+ switch (upt) {
+ case PHY_UPSTREAM_MAC:
+ pdn->upstream.netdev = (struct net_device *)upstream;
+ if (phy_on_sfp(phy))
+ pdn->parent_sfp_bus = pdn->upstream.netdev->sfp_bus;
+ break;
+ case PHY_UPSTREAM_PHY:
+ pdn->upstream.phydev = (struct phy_device *)upstream;
+ if (phy_on_sfp(phy))
+ pdn->parent_sfp_bus = pdn->upstream.phydev->sfp_bus;
+ break;
+ default:
+ ret = -EINVAL;
+ goto err;
+ }
+ pdn->upstream_type = upt;
+
+ ret = xa_alloc_cyclic(&topo->phys, &phy->phyindex, pdn, xa_limit_32b,
+ &topo->next_phy_index, GFP_KERNEL);
+ if (ret)
+ goto err;
+
+ return 0;
+
+err:
+ kfree(pdn);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_link_topo_add_phy);
+
+void phy_link_topo_del_phy(struct phy_link_topology *topo,
+ struct phy_device *phy)
+{
+ struct phy_device_node *pdn = xa_erase(&topo->phys, phy->phyindex);
+
+ kfree(pdn);
+}
+EXPORT_SYMBOL_GPL(phy_link_topo_del_phy);
+
+void phy_link_topo_init(struct phy_link_topology *topo)
+{
+ xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
+ topo->next_phy_index = 1;
+}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e87caa81f70c..6ebb37375d39 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -40,7 +40,6 @@
#include <net/dcbnl.h>
#endif
#include <net/netprio_cgroup.h>
-
#include <linux/netdev_features.h>
#include <linux/neighbour.h>
#include <uapi/linux/netdevice.h>
@@ -52,6 +51,7 @@
#include <net/net_trackers.h>
#include <net/net_debug.h>
#include <net/dropreason-core.h>
+#include <linux/phy_link_topology_core.h>

struct netpoll_info;
struct device;
@@ -2035,6 +2035,7 @@ enum netdev_stat_type {
* @fcoe_ddp_xid: Max exchange id for FCoE LRO by ddp
*
* @priomap: XXX: need comments on this one
+ * @link_topo: Physical link topology tracking attached PHYs
* @phydev: Physical device may attach itself
* for hardware timestamping
* @sfp_bus: attached &struct sfp_bus structure.
@@ -2417,6 +2418,7 @@ struct net_device {
#if IS_ENABLED(CONFIG_CGROUP_NET_PRIO)
struct netprio_map __rcu *priomap;
#endif
+ struct phy_link_topology link_topo;
struct phy_device *phydev;
struct sfp_bus *sfp_bus;
struct lock_class_key *qdisc_tx_busylock;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e5f1f41e399c..4cee9c05183f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -543,6 +543,9 @@ struct macsec_ops;
* @drv: Pointer to the driver for this PHY instance
* @devlink: Create a link between phy dev and mac dev, if the external phy
* used by current mac interface is managed by another mac interface.
+ * @phyindex: Unique id across the phy's parent tree of phys to address the PHY
+ * from userspace, similar to ifindex. A zero index means the PHY
+ * wasn't assigned an id yet.
* @phy_id: UID for this device found during discovery
* @c45_ids: 802.3-c45 Device Identifiers if is_c45.
* @is_c45: Set to true if this PHY uses clause 45 addressing.
@@ -640,6 +643,7 @@ struct phy_device {

struct device_link *devlink;

+ u32 phyindex;
u32 phy_id;

struct phy_c45_device_ids c45_ids;
diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
new file mode 100644
index 000000000000..44013a3cff50
--- /dev/null
+++ b/include/linux/phy_link_topology.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PHY device list allow maintaining a list of PHY devices that are
+ * part of a netdevice's link topology. PHYs can for example be chained,
+ * as is the case when using a PHY that exposes an SFP module, on which an
+ * SFP transceiver that embeds a PHY is connected.
+ *
+ * This list can then be used by userspace to leverage individual PHY
+ * capabilities.
+ */
+#ifndef __PHY_LINK_TOPOLOGY_H
+#define __PHY_LINK_TOPOLOGY_H
+
+#include <linux/ethtool.h>
+#include <linux/phy_link_topology_core.h>
+
+struct xarray;
+struct phy_device;
+struct net_device;
+struct sfp_bus;
+
+struct phy_device_node {
+ enum phy_upstream upstream_type;
+
+ union {
+ struct net_device *netdev;
+ struct phy_device *phydev;
+ } upstream;
+
+ struct sfp_bus *parent_sfp_bus;
+
+ struct phy_device *phy;
+};
+
+#if IS_ENABLED(CONFIG_PHYLIB)
+struct phy_device *phy_link_topo_get_phy(struct phy_link_topology *topo,
+ u32 phyindex);
+int phy_link_topo_add_phy(struct phy_link_topology *topo,
+ struct phy_device *phy,
+ enum phy_upstream upt, void *upstream);
+
+void phy_link_topo_del_phy(struct phy_link_topology *lt, struct phy_device *phy);
+
+#else
+static struct phy_device *phy_link_topo_get_phy(struct phy_link_topology *topo,
+ u32 phyindex)
+{
+ return NULL;
+}
+
+static int phy_link_topo_add_phy(struct phy_link_topology *topo,
+ struct phy_device *phy,
+ enum phy_upstream upt, void *upstream)
+{
+ return 0;
+}
+
+static void phy_link_topo_del_phy(struct phy_link_topology *topo,
+ struct phy_device *phy)
+{
+}
+#endif
+
+#endif /* __PHY_LINK_TOPOLOGY_H */
diff --git a/include/linux/phy_link_topology_core.h b/include/linux/phy_link_topology_core.h
new file mode 100644
index 000000000000..7aee9718d305
--- /dev/null
+++ b/include/linux/phy_link_topology_core.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PHY_LINK_TOPOLOGY_CORE_H
+#define __PHY_LINK_TOPOLOGY_CORE_H
+
+struct xarray;
+
+struct phy_link_topology {
+ struct xarray phys;
+
+ u32 next_phy_index;
+};
+
+void phy_link_topo_init(struct phy_link_topology *topo);
+
+#endif /* __PHY_LINK_TOPOLOGY_CORE_H */
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index f7fba0dc87e5..f3a8b582b8d4 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -2205,4 +2205,20 @@ struct ethtool_link_settings {
* __u32 map_lp_advertising[link_mode_masks_nwords];
*/
};
+
+/**
+ * enum phy_upstream - Represents the upstream component a given PHY device
+ * is connected to, as in what is on the other end of the MII bus. Most PHYs
+ * will be attached to an Ethernet MAC controller, but in some cases, there's
+ * an intermediate PHY used as a media-converter, which will driver another
+ * MII interface as its output.
+ * @PHY_UPSTREAM_MAC: Upstream component is a MAC (a switch port,
+ * or ethernet controller)
+ * @PHY_UPSTREAM_PHY: Upstream component is a PHY (likely a media converter)
+ */
+enum phy_upstream {
+ PHY_UPSTREAM_MAC,
+ PHY_UPSTREAM_PHY,
+};
+
#endif /* _UAPI_LINUX_ETHTOOL_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index 3950ced396b5..64b0fda64033 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -153,6 +153,7 @@
#include <linux/prandom.h>
#include <linux/once_lite.h>
#include <net/netdev_rx_queue.h>
+#include <linux/phy_link_topology_core.h>

#include "dev.h"
#include "net-sysfs.h"
@@ -10832,6 +10833,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
#ifdef CONFIG_NET_SCHED
hash_init(dev->qdisc_hash);
#endif
+ phy_link_topo_init(&dev->link_topo);
+
dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
setup(dev);

--
2.42.0


2023-12-09 17:03:02

by Simon Horman

[permalink] [raw]
Subject: Re: [RFC PATCH net-next v3 01/13] net: phy: Introduce ethernet link topology representation

On Fri, Dec 01, 2023 at 05:36:51PM +0100, Maxime Chevallier wrote:
> Link topologies containing multiple network PHYs attached to the same
> net_device can be found when using a PHY as a media converter for use
> with an SFP connector, on which an SFP transceiver containing a PHY can
> be used.
>
> With the current model, the transceiver's PHY can't be used for
> operations such as cable testing, timestamping, macsec offload, etc.
>
> The reason being that most of the logic for these configuration, coming
> from either ethtool netlink or ioctls tend to use netdev->phydev, which
> in multi-phy systems will reference the PHY closest to the MAC.
>
> Introduce a numbering scheme allowing to enumerate PHY devices that
> belong to any netdev, which can in turn allow userspace to take more
> precise decisions with regard to each PHY's configuration.
>
> The numbering is maintained per-netdev, in a phy_device_list.
> The numbering works similarly to a netdevice's ifindex, with
> identifiers that are only recycled once INT_MAX has been reached.
>
> This prevents races that could occur between PHY listing and SFP
> transceiver removal/insertion.
>
> The identifiers are assigned at phy_attach time, as the numbering
> depends on the netdevice the phy is attached to.
>
> Signed-off-by: Maxime Chevallier <[email protected]>

Hi Maxime,

some minor feedback from my side.

...

> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index f65e85c91fc1..3cf7774df57e 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -2,7 +2,7 @@
> # Makefile for Linux PHY drivers
>
> libphy-y := phy.o phy-c45.o phy-core.o phy_device.o \
> - linkmode.o
> + linkmode.o phy_link_topology.o
> mdio-bus-y += mdio_bus.o mdio_device.o
>
> ifdef CONFIG_MDIO_DEVICE
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c

...

> @@ -265,6 +266,14 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
>
> static struct phy_driver genphy_driver;
>
> +static struct phy_link_topology *phy_get_link_topology(struct phy_device *phydev)
> +{
> + if (phydev->attached_dev)
> + return &phydev->attached_dev->link_topo;
> +
> + return NULL;
> +}
> +

This function is declared static but is unused, which causes
allmodconfig W=1 builds to fail. Perhaps it could be introduced
in a latter patch where it is used?

...

> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c

...

> +void phy_link_topo_init(struct phy_link_topology *topo)
> +{
> + xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
> + topo->next_phy_index = 1;
> +}

...

> diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h

...

> +#else
> +static struct phy_device *phy_link_topo_get_phy(struct phy_link_topology *topo,
> + u32 phyindex)
> +{
> + return NULL;
> +}
> +
> +static int phy_link_topo_add_phy(struct phy_link_topology *topo,
> + struct phy_device *phy,
> + enum phy_upstream upt, void *upstream)
> +{
> + return 0;
> +}
> +
> +static void phy_link_topo_del_phy(struct phy_link_topology *topo,
> + struct phy_device *phy)
> +{
> +}
> +#endif

nit: functions in .h should be declared static inline

...

> diff --git a/net/core/dev.c b/net/core/dev.c

...

> @@ -10832,6 +10833,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> #ifdef CONFIG_NET_SCHED
> hash_init(dev->qdisc_hash);
> #endif
> + phy_link_topo_init(&dev->link_topo);
> +

I don't think this can work unless PHYLIB is compiled as a built-in.

> dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
> setup(dev);
>
> --
> 2.42.0
>

2023-12-11 11:06:43

by Maxime Chevallier

[permalink] [raw]
Subject: Re: [RFC PATCH net-next v3 01/13] net: phy: Introduce ethernet link topology representation

Hi Simon,

On Sat, 9 Dec 2023 17:02:41 +0000
Simon Horman <[email protected]> wrote:

> On Fri, Dec 01, 2023 at 05:36:51PM +0100, Maxime Chevallier wrote:
> > Link topologies containing multiple network PHYs attached to the same
> > net_device can be found when using a PHY as a media converter for use
> > with an SFP connector, on which an SFP transceiver containing a PHY can
> > be used.
> >
> > With the current model, the transceiver's PHY can't be used for
> > operations such as cable testing, timestamping, macsec offload, etc.
> >
> > The reason being that most of the logic for these configuration, coming
> > from either ethtool netlink or ioctls tend to use netdev->phydev, which
> > in multi-phy systems will reference the PHY closest to the MAC.
> >
> > Introduce a numbering scheme allowing to enumerate PHY devices that
> > belong to any netdev, which can in turn allow userspace to take more
> > precise decisions with regard to each PHY's configuration.
> >
> > The numbering is maintained per-netdev, in a phy_device_list.
> > The numbering works similarly to a netdevice's ifindex, with
> > identifiers that are only recycled once INT_MAX has been reached.
> >
> > This prevents races that could occur between PHY listing and SFP
> > transceiver removal/insertion.
> >
> > The identifiers are assigned at phy_attach time, as the numbering
> > depends on the netdevice the phy is attached to.
> >
> > Signed-off-by: Maxime Chevallier <[email protected]>
>
> Hi Maxime,
>
> some minor feedback from my side.
>
> ...
>
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index f65e85c91fc1..3cf7774df57e 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -2,7 +2,7 @@
> > # Makefile for Linux PHY drivers
> >
> > libphy-y := phy.o phy-c45.o phy-core.o phy_device.o \
> > - linkmode.o
> > + linkmode.o phy_link_topology.o
> > mdio-bus-y += mdio_bus.o mdio_device.o
> >
> > ifdef CONFIG_MDIO_DEVICE
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>
> ...
>
> > @@ -265,6 +266,14 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
> >
> > static struct phy_driver genphy_driver;
> >
> > +static struct phy_link_topology *phy_get_link_topology(struct phy_device *phydev)
> > +{
> > + if (phydev->attached_dev)
> > + return &phydev->attached_dev->link_topo;
> > +
> > + return NULL;
> > +}
> > +
>
> This function is declared static but is unused, which causes
> allmodconfig W=1 builds to fail. Perhaps it could be introduced
> in a latter patch where it is used?

Ah true, thanks for spotting this !

> ...
>
> > diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
>
> ...
>
> > +void phy_link_topo_init(struct phy_link_topology *topo)
> > +{
> > + xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
> > + topo->next_phy_index = 1;
> > +}
>
> ...
>
> > diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
>
> ...
>
> > +#else
> > +static struct phy_device *phy_link_topo_get_phy(struct phy_link_topology *topo,
> > + u32 phyindex)
> > +{
> > + return NULL;
> > +}
> > +
> > +static int phy_link_topo_add_phy(struct phy_link_topology *topo,
> > + struct phy_device *phy,
> > + enum phy_upstream upt, void *upstream)
> > +{
> > + return 0;
> > +}
> > +
> > +static void phy_link_topo_del_phy(struct phy_link_topology *topo,
> > + struct phy_device *phy)
> > +{
> > +}
> > +#endif
>
> nit: functions in .h should be declared static inline

Will do in next version

> ...
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
>
> ...
>
> > @@ -10832,6 +10833,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> > #ifdef CONFIG_NET_SCHED
> > hash_init(dev->qdisc_hash);
> > #endif
> > + phy_link_topo_init(&dev->link_topo);
> > +
>
> I don't think this can work unless PHYLIB is compiled as a built-in.

Inded, I need to better clarify and document the dependency with
PHYLIB.

Thanks a lot for the review,

Maxime

2023-12-11 14:22:12

by Andrew Lunn

[permalink] [raw]
Subject: Re: [RFC PATCH net-next v3 01/13] net: phy: Introduce ethernet link topology representation

> > > @@ -10832,6 +10833,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> > > #ifdef CONFIG_NET_SCHED
> > > hash_init(dev->qdisc_hash);
> > > #endif
> > > + phy_link_topo_init(&dev->link_topo);
> > > +
> >
> > I don't think this can work unless PHYLIB is compiled as a built-in.
>
> Inded, I need to better clarify and document the dependency with
> PHYLIB.

It is getting harder and harder to make the phylib core a module :-(

How much work does phy_link_topo_init() do? Could it be an inline
function? Are there other dependencies?

Also look at ethtool_phy_ops and e.g. how plca_get_cfg_prepare_data()
uses it.

Andrew

2023-12-12 13:10:49

by Maxime Chevallier

[permalink] [raw]
Subject: Re: [RFC PATCH net-next v3 01/13] net: phy: Introduce ethernet link topology representation

On Mon, 11 Dec 2023 15:09:09 +0100
Andrew Lunn <[email protected]> wrote:

> > > > @@ -10832,6 +10833,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> > > > #ifdef CONFIG_NET_SCHED
> > > > hash_init(dev->qdisc_hash);
> > > > #endif
> > > > + phy_link_topo_init(&dev->link_topo);
> > > > +
> > >
> > > I don't think this can work unless PHYLIB is compiled as a built-in.
> >
> > Inded, I need to better clarify and document the dependency with
> > PHYLIB.
>
> It is getting harder and harder to make the phylib core a module :-(
>
> How much work does phy_link_topo_init() do? Could it be an inline
> function? Are there other dependencies?

Sorry about that, I'll make sure it works with phylib entirely disabled
for next version. I try to keep the integration with net_device minimal
and avoid any dependency bloat, we don't need much besides xarray stuff
(hence the fact there are 2 headers, the phy_link_topology_core.h
containing the bare minimum), but I did miss that.

> Also look at ethtool_phy_ops and e.g. how plca_get_cfg_prepare_data()
> uses it.

Thanks, indeed that's a good example. I'll also address that in the
netlink part as well.

Thanks,

Maxime