Hello everyone,
This is V11 for the link topology addition, allowing to track all PHYs
that are linked to netdevices.
This V11 addresses the various netlink-related issues that were raised
by Jakub, and fixes some typos in the documentation.
As a remainder, here's what the PHY listings would look like :
- eth0 has a 88x3310 acting as media converter, and an SFP module with
an embedded 88e1111 PHY
- eth2 has a 88e1510 PHY
# ethtool --show-phys *
PHY for eth0:
PHY index: 1
Driver name: mv88x3310
PHY device name: f212a600.mdio-mii:00
Downstream SFP bus name: sfp-eth0
PHY id: 0
Upstream type: MAC
PHY for eth0:
PHY index: 2
Driver name: Marvell 88E1111
PHY device name: i2c:sfp-eth0:16
PHY id: 21040322
Upstream type: PHY
Upstream PHY index: 1
Upstream SFP name: sfp-eth0
PHY for eth2:
PHY index: 1
Driver name: Marvell 88E1510
PHY device name: f212a200.mdio-mii:00
PHY id: 21040593
Upstream type: MAC
Ethtool patches : https://github.com/minimaxwell/ethtool/tree/link-topo-v6
Link to V10: https://lore.kernel.org/netdev/[email protected]/
Link to V9: https://lore.kernel.org/netdev/[email protected]/
Link to V8: https://lore.kernel.org/netdev/[email protected]/
Link to V7: https://lore.kernel.org/netdev/[email protected]/
Link to V6: https://lore.kernel.org/netdev/[email protected]/
Link to V5: https://lore.kernel.org/netdev/[email protected]/
Link to V4: https://lore.kernel.org/netdev/[email protected]/
Link to V3: https://lore.kernel.org/netdev/[email protected]/
Link to V2: https://lore.kernel.org/netdev/[email protected]/
Link to V1: https://lore.kernel.org/netdev/[email protected]/
Maxime Chevallier (13):
net: phy: Introduce ethernet link topology representation
net: sfp: pass the phy_device when disconnecting an sfp module's PHY
net: phy: add helpers to handle sfp phy connect/disconnect
net: sfp: Add helper to return the SFP bus name
net: ethtool: Allow passing a phy index for some commands
netlink: specs: add phy-index as a header parameter
net: ethtool: Introduce a command to list PHYs on an interface
netlink: specs: add ethnl PHY_GET command set
net: ethtool: plca: Target the command to the requested PHY
net: ethtool: pse-pd: Target the command to the requested PHY
net: ethtool: cable-test: Target the command to the requested PHY
net: ethtool: strset: Allow querying phy stats by index
Documentation: networking: document phy_link_topology
Documentation/netlink/specs/ethtool.yaml | 62 ++++
Documentation/networking/ethtool-netlink.rst | 52 +++
Documentation/networking/index.rst | 1 +
.../networking/phy-link-topology.rst | 120 +++++++
MAINTAINERS | 2 +
drivers/net/phy/Makefile | 2 +-
drivers/net/phy/marvell-88x2222.c | 2 +
drivers/net/phy/marvell.c | 2 +
drivers/net/phy/marvell10g.c | 2 +
drivers/net/phy/phy_device.c | 55 ++++
drivers/net/phy/phy_link_topology.c | 105 ++++++
drivers/net/phy/phylink.c | 3 +-
drivers/net/phy/qcom/at803x.c | 2 +
drivers/net/phy/qcom/qca807x.c | 2 +
drivers/net/phy/sfp-bus.c | 15 +-
include/linux/netdevice.h | 4 +-
include/linux/phy.h | 6 +
include/linux/phy_link_topology.h | 72 +++++
include/linux/phy_link_topology_core.h | 25 ++
include/linux/sfp.h | 8 +-
include/uapi/linux/ethtool.h | 16 +
include/uapi/linux/ethtool_netlink.h | 21 ++
net/core/dev.c | 9 +
net/ethtool/Makefile | 2 +-
net/ethtool/cabletest.c | 16 +-
net/ethtool/netlink.c | 57 +++-
net/ethtool/netlink.h | 10 +
net/ethtool/phy.c | 300 ++++++++++++++++++
net/ethtool/plca.c | 19 +-
net/ethtool/pse-pd.c | 13 +-
net/ethtool/strset.c | 17 +-
31 files changed, 979 insertions(+), 43 deletions(-)
create mode 100644 Documentation/networking/phy-link-topology.rst
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
create mode 100644 net/ethtool/phy.c
--
2.44.0
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. The PHY index can be
re-used for PHYs that are persistent.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: - No changes
V10: - No changes
V9: - No changes
V8: - Rebase on net-next and fixed conflicts
V7: - Protected the phy_link_topo helpers/stubs with IS_REACHABLE
V6: - Made link_topo a pointer
- Reworked the init/cleanup sequence
- Added phy_index recycling if possible
V5: - Dropped the ASSERT_RTNL()
- Made the phy_link_topo_get_phy inline
V4: - Moved the phy_link_topo_init() code to an inline header function
- Made the code build without phylib
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 | 7 ++
drivers/net/phy/phy_link_topology.c | 105 +++++++++++++++++++++++++
include/linux/netdevice.h | 4 +-
include/linux/phy.h | 4 +
include/linux/phy_link_topology.h | 72 +++++++++++++++++
include/linux/phy_link_topology_core.h | 25 ++++++
include/uapi/linux/ethtool.h | 16 ++++
net/core/dev.c | 9 +++
10 files changed, 244 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 909c2c531d8e..db0aa3a926ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8016,6 +8016,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 202ed7f450da..1d8be374915f 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 6c6ec9475709..452fc8b3406d 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>
@@ -1511,6 +1512,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
@@ -1938,6 +1944,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..985941c5c558
--- /dev/null
+++ b/drivers/net/phy/phy_link_topology.c
@@ -0,0 +1,105 @@
+// 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_link_topology *phy_link_topo_create(struct net_device *dev)
+{
+ struct phy_link_topology *topo;
+
+ topo = kzalloc(sizeof(*topo), GFP_KERNEL);
+ if (!topo)
+ return ERR_PTR(-ENOMEM);
+
+ xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
+ topo->next_phy_index = 1;
+
+ return topo;
+}
+
+void phy_link_topo_destroy(struct phy_link_topology *topo)
+{
+ if (!topo)
+ return;
+
+ xa_destroy(&topo->phys);
+ kfree(topo);
+}
+
+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;
+
+ 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;
+
+ /* Attempt to re-use a previously allocated phy_index */
+ if (phy->phyindex) {
+ ret = xa_insert(&topo->phys, phy->phyindex, pdn, GFP_KERNEL);
+
+ /* Errors could be either -ENOMEM or -EBUSY. If the phy has an
+ * index, and there's another entry at the same index, this is
+ * unexpected and we still error-out
+ */
+ if (ret)
+ goto err;
+ return 0;
+ }
+
+ 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);
+
+ /* We delete the PHY from the topology, however we don't re-set the
+ * phy->phyindex field. If the PHY isn't gone, we can re-assign it the
+ * same index next time it's added back to the topology
+ */
+
+ kfree(pdn);
+}
+EXPORT_SYMBOL_GPL(phy_link_topo_del_phy);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0c198620ac93..d45f330d083d 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;
@@ -1974,6 +1974,7 @@ enum netdev_reg_state {
* @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.
@@ -2364,6 +2365,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 e6e83304558e..8c848c79b1fd 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -550,6 +550,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.
@@ -650,6 +653,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..6b79feb607e7
--- /dev/null
+++ b/include/linux/phy_link_topology.h
@@ -0,0 +1,72 @@
+/* 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;
+};
+
+struct phy_link_topology {
+ struct xarray phys;
+ u32 next_phy_index;
+};
+
+static inline 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;
+}
+
+#if IS_REACHABLE(CONFIG_PHYLIB)
+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 inline int phy_link_topo_add_phy(struct phy_link_topology *topo,
+ struct phy_device *phy,
+ enum phy_upstream upt, void *upstream)
+{
+ return 0;
+}
+
+static inline 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..0a6479055745
--- /dev/null
+++ b/include/linux/phy_link_topology_core.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PHY_LINK_TOPOLOGY_CORE_H
+#define __PHY_LINK_TOPOLOGY_CORE_H
+
+struct phy_link_topology;
+
+#if IS_REACHABLE(CONFIG_PHYLIB)
+
+struct phy_link_topology *phy_link_topo_create(struct net_device *dev);
+void phy_link_topo_destroy(struct phy_link_topology *topo);
+
+#else
+
+static inline struct phy_link_topology *phy_link_topo_create(struct net_device *dev)
+{
+ return NULL;
+}
+
+static inline void phy_link_topo_destroy(struct phy_link_topology *topo)
+{
+}
+
+#endif
+
+#endif /* __PHY_LINK_TOPOLOGY_CORE_H */
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 11fc18988bc2..95c2f09f0d0a 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -2268,4 +2268,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 9b821d96eff3..928cf377e843 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -158,6 +158,7 @@
#include <net/page_pool/types.h>
#include <net/page_pool/helpers.h>
#include <net/rps.h>
+#include <linux/phy_link_topology_core.h>
#include "dev.h"
#include "net-sysfs.h"
@@ -10962,6 +10963,12 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
#ifdef CONFIG_NET_SCHED
hash_init(dev->qdisc_hash);
#endif
+ dev->link_topo = phy_link_topo_create(dev);
+ if (IS_ERR(dev->link_topo)) {
+ dev->link_topo = NULL;
+ goto free_all;
+ }
+
dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
setup(dev);
@@ -11050,6 +11057,8 @@ void free_netdev(struct net_device *dev)
free_percpu(dev->xdp_bulkq);
dev->xdp_bulkq = NULL;
+ phy_link_topo_destroy(dev->link_topo);
+
/* Compatibility with error handling in drivers */
if (dev->reg_state == NETREG_UNINITIALIZED) {
netdev_freemem(dev);
--
2.44.0
Pass the phy_device as a parameter to the sfp upstream .disconnect_phy
operation. This is preparatory work to help track phy devices across
a net_device's link.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: No changes
V10: No changes
V9: No changes
V8: No changes
V7: No changes
V6: Moved an incorrectly added hunk from the previous series to patch 0003
V5: No changes
V4: No changes
V3: No changes
drivers/net/phy/phylink.c | 3 ++-
drivers/net/phy/sfp-bus.c | 4 ++--
include/linux/sfp.h | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 84a97088dfc6..0e692a3bcf1a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3408,7 +3408,8 @@ static int phylink_sfp_connect_phy(void *upstream, struct phy_device *phy)
return ret;
}
-static void phylink_sfp_disconnect_phy(void *upstream)
+static void phylink_sfp_disconnect_phy(void *upstream,
+ struct phy_device *phydev)
{
phylink_disconnect_phy(upstream);
}
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index db39dec7f247..e05013aeecc3 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -486,7 +486,7 @@ static void sfp_unregister_bus(struct sfp_bus *bus)
bus->socket_ops->stop(bus->sfp);
bus->socket_ops->detach(bus->sfp);
if (bus->phydev && ops && ops->disconnect_phy)
- ops->disconnect_phy(bus->upstream);
+ ops->disconnect_phy(bus->upstream, bus->phydev);
}
bus->registered = false;
}
@@ -742,7 +742,7 @@ void sfp_remove_phy(struct sfp_bus *bus)
const struct sfp_upstream_ops *ops = sfp_get_upstream_ops(bus);
if (ops && ops->disconnect_phy)
- ops->disconnect_phy(bus->upstream);
+ ops->disconnect_phy(bus->upstream, bus->phydev);
bus->phydev = NULL;
}
EXPORT_SYMBOL_GPL(sfp_remove_phy);
diff --git a/include/linux/sfp.h b/include/linux/sfp.h
index 9346cd44814d..0573e53b0c11 100644
--- a/include/linux/sfp.h
+++ b/include/linux/sfp.h
@@ -544,7 +544,7 @@ struct sfp_upstream_ops {
void (*link_down)(void *priv);
void (*link_up)(void *priv);
int (*connect_phy)(void *priv, struct phy_device *);
- void (*disconnect_phy)(void *priv);
+ void (*disconnect_phy)(void *priv, struct phy_device *);
};
#if IS_ENABLED(CONFIG_SFP)
--
2.44.0
There are a few PHY drivers that can handle SFP modules through their
sfp_upstream_ops. Introduce Phylib helpers to keep track of connected
SFP PHYs in a netdevice's namespace, by adding the SFP PHY to the
upstream PHY's netdev's namespace.
By doing so, these SFP PHYs can be enumerated and exposed to users,
which will be able to use their capabilities.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: No changes
V10: No changes
V9: No changes
V8: Took the at803x into account
V7: No changes
V6: Moved phy_get_link_topology() definition to this patch
V5: No Changes
V4: Rebased the at803x part with the newer version on net-next
V3: Renaming
V2: Renaming
drivers/net/phy/marvell-88x2222.c | 2 ++
drivers/net/phy/marvell.c | 2 ++
drivers/net/phy/marvell10g.c | 2 ++
drivers/net/phy/phy_device.c | 48 +++++++++++++++++++++++++++++++
drivers/net/phy/qcom/at803x.c | 2 ++
drivers/net/phy/qcom/qca807x.c | 2 ++
include/linux/phy.h | 2 ++
7 files changed, 60 insertions(+)
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
index b88398e6872b..0b777cdd7078 100644
--- a/drivers/net/phy/marvell-88x2222.c
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -553,6 +553,8 @@ static const struct sfp_upstream_ops sfp_phy_ops = {
.link_down = mv2222_sfp_link_down,
.attach = phy_sfp_attach,
.detach = phy_sfp_detach,
+ .connect_phy = phy_sfp_connect_phy,
+ .disconnect_phy = phy_sfp_disconnect_phy,
};
static int mv2222_probe(struct phy_device *phydev)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 7c00f47e4ded..b87d0f200643 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -3463,6 +3463,8 @@ static const struct sfp_upstream_ops m88e1510_sfp_ops = {
.module_remove = m88e1510_sfp_remove,
.attach = phy_sfp_attach,
.detach = phy_sfp_detach,
+ .connect_phy = phy_sfp_connect_phy,
+ .disconnect_phy = phy_sfp_disconnect_phy,
};
static int m88e1510_probe(struct phy_device *phydev)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index ad43e280930c..6642eb642d4b 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -503,6 +503,8 @@ static int mv3310_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
static const struct sfp_upstream_ops mv3310_sfp_ops = {
.attach = phy_sfp_attach,
.detach = phy_sfp_detach,
+ .connect_phy = phy_sfp_connect_phy,
+ .disconnect_phy = phy_sfp_disconnect_phy,
.module_insert = mv3310_sfp_insert,
};
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 452fc8b3406d..616bd7ba46cb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -277,6 +277,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);
@@ -1370,6 +1378,46 @@ phy_standalone_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(phy_standalone);
+/**
+ * phy_sfp_connect_phy - Connect the SFP module's PHY to the upstream PHY
+ * @upstream: pointer to the upstream phy device
+ * @phy: pointer to the SFP module's phy device
+ *
+ * This helper allows keeping track of PHY devices on the link. It adds the
+ * SFP module's phy to the phy namespace of the upstream phy
+ */
+int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
+{
+ struct phy_device *phydev = upstream;
+ struct phy_link_topology *topo = phy_get_link_topology(phydev);
+
+ if (topo)
+ return phy_link_topo_add_phy(topo, phy, PHY_UPSTREAM_PHY, phydev);
+
+ return 0;
+}
+EXPORT_SYMBOL(phy_sfp_connect_phy);
+
+/**
+ * phy_sfp_disconnect_phy - Disconnect the SFP module's PHY from the upstream PHY
+ * @upstream: pointer to the upstream phy device
+ * @phy: pointer to the SFP module's phy device
+ *
+ * This helper allows keeping track of PHY devices on the link. It removes the
+ * SFP module's phy to the phy namespace of the upstream phy. As the module phy
+ * will be destroyed, re-inserting the same module will add a new phy with a
+ * new index.
+ */
+void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
+{
+ struct phy_device *phydev = upstream;
+ struct phy_link_topology *topo = phy_get_link_topology(phydev);
+
+ if (topo)
+ phy_link_topo_del_phy(topo, phy);
+}
+EXPORT_SYMBOL(phy_sfp_disconnect_phy);
+
/**
* phy_sfp_attach - attach the SFP bus to the PHY upstream network device
* @upstream: pointer to the phy device
diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
index c8f83e5f78ab..105602581a03 100644
--- a/drivers/net/phy/qcom/at803x.c
+++ b/drivers/net/phy/qcom/at803x.c
@@ -770,6 +770,8 @@ static const struct sfp_upstream_ops at8031_sfp_ops = {
.attach = phy_sfp_attach,
.detach = phy_sfp_detach,
.module_insert = at8031_sfp_insert,
+ .connect_phy = phy_sfp_connect_phy,
+ .disconnect_phy = phy_sfp_disconnect_phy,
};
static int at8031_parse_dt(struct phy_device *phydev)
diff --git a/drivers/net/phy/qcom/qca807x.c b/drivers/net/phy/qcom/qca807x.c
index 672c6929119a..5eb0ab1cb70e 100644
--- a/drivers/net/phy/qcom/qca807x.c
+++ b/drivers/net/phy/qcom/qca807x.c
@@ -699,6 +699,8 @@ static const struct sfp_upstream_ops qca807x_sfp_ops = {
.detach = phy_sfp_detach,
.module_insert = qca807x_sfp_insert,
.module_remove = qca807x_sfp_remove,
+ .connect_phy = phy_sfp_connect_phy,
+ .disconnect_phy = phy_sfp_disconnect_phy,
};
static int qca807x_probe(struct phy_device *phydev)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8c848c79b1fd..3ddfe7fe781a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1758,6 +1758,8 @@ int phy_suspend(struct phy_device *phydev);
int phy_resume(struct phy_device *phydev);
int __phy_resume(struct phy_device *phydev);
int phy_loopback(struct phy_device *phydev, bool enable);
+int phy_sfp_connect_phy(void *upstream, struct phy_device *phy);
+void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy);
void phy_sfp_attach(void *upstream, struct sfp_bus *bus);
void phy_sfp_detach(void *upstream, struct sfp_bus *bus);
int phy_sfp_probe(struct phy_device *phydev,
--
2.44.0
Knowing the bus name is helpful when we want to expose the link topology
to userspace, add a helper to return the SFP bus name.
Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
V11: - No changes
V10: - No changes
V9: - No changes
V8: - No changes
V7: - No changes
V6: - No changes
V5: - Added Andrew's R-b
V4: - No changes
V3: - Added RTNL assert
drivers/net/phy/sfp-bus.c | 11 +++++++++++
include/linux/sfp.h | 6 ++++++
2 files changed, 17 insertions(+)
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index e05013aeecc3..413021619afe 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -859,3 +859,14 @@ void sfp_unregister_socket(struct sfp_bus *bus)
sfp_bus_put(bus);
}
EXPORT_SYMBOL_GPL(sfp_unregister_socket);
+
+const char *sfp_get_name(struct sfp_bus *bus)
+{
+ ASSERT_RTNL();
+
+ if (bus->sfp_dev)
+ return dev_name(bus->sfp_dev);
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(sfp_get_name);
diff --git a/include/linux/sfp.h b/include/linux/sfp.h
index 0573e53b0c11..55c0ab17c9e2 100644
--- a/include/linux/sfp.h
+++ b/include/linux/sfp.h
@@ -570,6 +570,7 @@ struct sfp_bus *sfp_bus_find_fwnode(const struct fwnode_handle *fwnode);
int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
const struct sfp_upstream_ops *ops);
void sfp_bus_del_upstream(struct sfp_bus *bus);
+const char *sfp_get_name(struct sfp_bus *bus);
#else
static inline int sfp_parse_port(struct sfp_bus *bus,
const struct sfp_eeprom_id *id,
@@ -648,6 +649,11 @@ static inline int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
static inline void sfp_bus_del_upstream(struct sfp_bus *bus)
{
}
+
+static inline const char *sfp_get_name(struct sfp_bus *bus)
+{
+ return NULL;
+}
#endif
#endif
--
2.44.0
Some netlink commands are target towards ethernet PHYs, to control some
of their features. As there's several such commands, add the ability to
pass a PHY index in the ethnl request, which will populate the generic
ethnl_req_info with the relevant phydev when the command targets a PHY.
Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
V11: - Fixed documentation typos
- Dropped the netlink attr error message
- Added a check when passing a phy index but no netdev
(Thanks Jakub for the review)
V10: - No changes
V9: - No changes
V8: - No changes
V7: - No changes
V6: - Added dedicated policies when passing a PHY index
- Dropped Andrew's R-b as there were changes
V5: - Added Andrew's R-b
- Fix a typo reported by Simon
V4: - No Changes
V3: - Fixed the documentation
V2: - New patch
Documentation/networking/ethtool-netlink.rst | 7 +++
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/netlink.c | 48 +++++++++++++++++++-
net/ethtool/netlink.h | 5 ++
4 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index d583d9abf2f8..fa126452c1c3 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -57,6 +57,7 @@ Structure of this header is
``ETHTOOL_A_HEADER_DEV_INDEX`` u32 device ifindex
``ETHTOOL_A_HEADER_DEV_NAME`` string device name
``ETHTOOL_A_HEADER_FLAGS`` u32 flags common for all requests
+ ``ETHTOOL_A_HEADER_PHY_INDEX`` u32 phy device index
============================== ====== =============================
``ETHTOOL_A_HEADER_DEV_INDEX`` and ``ETHTOOL_A_HEADER_DEV_NAME`` identify the
@@ -81,6 +82,12 @@ the behaviour is backward compatible, i.e. requests from old clients not aware
of the flag should be interpreted the way the client expects. A client must
not set flags it does not understand.
+``ETHTOOL_A_HEADER_PHY_INDEX`` identifies the Ethernet PHY the message relates to.
+As there are numerous commands that are related to PHY configuration, and because
+there may be more than one PHY on the link, the PHY index can be passed in the
+request for the commands that needs it. It is, however, not mandatory, and if it
+is not passed for commands that target a PHY, the net_device.phydev pointer
+is used.
Bit sets
========
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index accbb1a231df..3b07d8013fb6 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -133,6 +133,7 @@ enum {
ETHTOOL_A_HEADER_DEV_INDEX, /* u32 */
ETHTOOL_A_HEADER_DEV_NAME, /* string */
ETHTOOL_A_HEADER_FLAGS, /* u32 - ETHTOOL_FLAG_* */
+ ETHTOOL_A_HEADER_PHY_INDEX, /* u32 */
/* add new constants above here */
__ETHTOOL_A_HEADER_CNT,
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index bd04f28d5cf4..563e94e0cbd8 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -4,6 +4,7 @@
#include <linux/ethtool_netlink.h>
#include <linux/pm_runtime.h>
#include "netlink.h"
+#include <linux/phy_link_topology.h>
static struct genl_family ethtool_genl_family;
@@ -30,6 +31,24 @@ const struct nla_policy ethnl_header_policy_stats[] = {
ETHTOOL_FLAGS_STATS),
};
+const struct nla_policy ethnl_header_policy_phy[] = {
+ [ETHTOOL_A_HEADER_DEV_INDEX] = { .type = NLA_U32 },
+ [ETHTOOL_A_HEADER_DEV_NAME] = { .type = NLA_NUL_STRING,
+ .len = ALTIFNAMSIZ - 1 },
+ [ETHTOOL_A_HEADER_FLAGS] = NLA_POLICY_MASK(NLA_U32,
+ ETHTOOL_FLAGS_BASIC),
+ [ETHTOOL_A_HEADER_PHY_INDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+};
+
+const struct nla_policy ethnl_header_policy_phy_stats[] = {
+ [ETHTOOL_A_HEADER_DEV_INDEX] = { .type = NLA_U32 },
+ [ETHTOOL_A_HEADER_DEV_NAME] = { .type = NLA_NUL_STRING,
+ .len = ALTIFNAMSIZ - 1 },
+ [ETHTOOL_A_HEADER_FLAGS] = NLA_POLICY_MASK(NLA_U32,
+ ETHTOOL_FLAGS_STATS),
+ [ETHTOOL_A_HEADER_PHY_INDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+};
+
int ethnl_ops_begin(struct net_device *dev)
{
int ret;
@@ -89,8 +108,9 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
const struct nlattr *header, struct net *net,
struct netlink_ext_ack *extack, bool require_dev)
{
- struct nlattr *tb[ARRAY_SIZE(ethnl_header_policy)];
+ struct nlattr *tb[ARRAY_SIZE(ethnl_header_policy_phy)];
const struct nlattr *devname_attr;
+ struct phy_device *phydev = NULL;
struct net_device *dev = NULL;
u32 flags = 0;
int ret;
@@ -104,7 +124,7 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
/* No validation here, command policy should have a nested policy set
* for the header, therefore validation should have already been done.
*/
- ret = nla_parse_nested(tb, ARRAY_SIZE(ethnl_header_policy) - 1, header,
+ ret = nla_parse_nested(tb, ARRAY_SIZE(ethnl_header_policy_phy) - 1, header,
NULL, extack);
if (ret < 0)
return ret;
@@ -145,6 +165,30 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
return -EINVAL;
}
+ if (dev) {
+ if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
+ struct nlattr *phy_id;
+
+ phy_id = tb[ETHTOOL_A_HEADER_PHY_INDEX];
+ phydev = phy_link_topo_get_phy(dev->link_topo,
+ nla_get_u32(phy_id));
+ if (!phydev) {
+ NL_SET_BAD_ATTR(extack, phy_id);
+ return -ENODEV;
+ }
+ } else {
+ /* If we need a PHY but no phy index is specified, fallback
+ * to dev->phydev
+ */
+ phydev = dev->phydev;
+ }
+ } else if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
+ NL_SET_ERR_MSG_ATTR(extack, header,
+ "can't target a PHY without a netdev");
+ return -EINVAL;
+ }
+
+ req_info->phydev = phydev;
req_info->dev = dev;
req_info->flags = flags;
return 0;
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 9a333a8d04c1..d57a890b5d9e 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -250,6 +250,7 @@ static inline unsigned int ethnl_reply_header_size(void)
* @dev: network device the request is for (may be null)
* @dev_tracker: refcount tracker for @dev reference
* @flags: request flags common for all request types
+ * @phydev: phy_device connected to @dev this request is for (may be null)
*
* This is a common base for request specific structures holding data from
* parsed userspace request. These always embed struct ethnl_req_info at
@@ -259,6 +260,7 @@ struct ethnl_req_info {
struct net_device *dev;
netdevice_tracker dev_tracker;
u32 flags;
+ struct phy_device *phydev;
};
static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info)
@@ -395,9 +397,12 @@ extern const struct ethnl_request_ops ethnl_rss_request_ops;
extern const struct ethnl_request_ops ethnl_plca_cfg_request_ops;
extern const struct ethnl_request_ops ethnl_plca_status_request_ops;
extern const struct ethnl_request_ops ethnl_mm_request_ops;
+extern const struct ethnl_request_ops ethnl_phy_request_ops;
extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
+extern const struct nla_policy ethnl_header_policy_phy[ETHTOOL_A_HEADER_PHY_INDEX + 1];
+extern const struct nla_policy ethnl_header_policy_phy_stats[ETHTOOL_A_HEADER_PHY_INDEX + 1];
extern const struct nla_policy ethnl_strset_get_policy[ETHTOOL_A_STRSET_COUNTS_ONLY + 1];
extern const struct nla_policy ethnl_linkinfo_get_policy[ETHTOOL_A_LINKINFO_HEADER + 1];
extern const struct nla_policy ethnl_linkinfo_set_policy[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL + 1];
--
2.44.0
Update the spec to take the newly introduced phy-index as a generic
request parameter.
Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
V11: No changes
V10: No changes
V9: No changes
V8: No changes
V7: No changes
V6: No changes
V5: Added Andrew's R-b
V4: Ditch the ethtool-user generated code
V3: New patch
Documentation/netlink/specs/ethtool.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 197208f419dc..bb6e1dc6d1c5 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -30,6 +30,9 @@ attribute-sets:
-
name: flags
type: u32
+ -
+ name: phy-index
+ type: u32
-
name: bitset-bit
--
2.44.0
The PHY_GET command, supporting both DUMP and GET operations, is used to
retrieve the list of PHYs connected to a netdevice, and get topology
information to know where exactly it sits on the physical link.
Add the netlink specs corresponding to that command.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: No changes
V10: rename upstream-phy-index in the phy-get-op definition
V9: rename upstream-phy-index to upstream-index to align with the
ethtool_netlink.h definition
V8: No changes
V7: No changes
V6: Updated the spec according to the new attributes
V5: No changes
V4: Remove the ethtool-user generated code
V3: New patch
Documentation/netlink/specs/ethtool.yaml | 59 ++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index bb6e1dc6d1c5..58a38a8cace6 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -16,6 +16,11 @@ definitions:
name: stringset
type: enum
entries: []
+ -
+ name: phy-upstream-type
+ enum-name:
+ type: enum
+ entries: [ mac, phy ]
attribute-sets:
-
@@ -945,6 +950,38 @@ attribute-sets:
-
name: burst-tmr
type: u32
+ -
+ name: phy
+ attributes:
+ -
+ name: header
+ type: nest
+ nested-attributes: header
+ -
+ name: index
+ type: u32
+ -
+ name: drvname
+ type: string
+ -
+ name: name
+ type: string
+ -
+ name: upstream-type
+ type: u32
+ enum: phy-upstream-type
+ -
+ name: upstream-index
+ type: u32
+ -
+ name: upstream-sfp-name
+ type: string
+ -
+ name: downstream-sfp-name
+ type: string
+ -
+ name: id
+ type: u32
operations:
enum-model: directional
@@ -1696,3 +1733,25 @@ operations:
name: mm-ntf
doc: Notification for change in MAC Merge configuration.
notify: mm-get
+ -
+ name: phy-get
+ doc: Get PHY devices attached to an interface
+
+ attribute-set: phy
+
+ do: &phy-get-op
+ request:
+ attributes:
+ - header
+ reply:
+ attributes:
+ - header
+ - index
+ - drvname
+ - name
+ - upstream-type
+ - upstream-index
+ - upstream-sfp-name
+ - downstream-sfp-name
+ - id
+ dump: *phy-get-op
--
2.44.0
As we have the ability to track the PHYs connected to a net_device
through the link_topology, we can expose this list to userspace. This
allows userspace to use these identifiers for phy-specific commands and
take the decision of which PHY to target by knowing the link topology.
Add PHY_GET and PHY_DUMP, which can be a filtered DUMP operation to list
devices on only one interface.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: - Fixed typos
- Fixed a leak for the dump context in case of errors
- Split long lines, and joined short lines
- Removed unnecessary length checks
V10: - No changes
V9: - No Changes
V8: - No Changes
V7: - Moved the netdev release into the .done() call for filtered DUMP
requests
V6: - Removed un-needed nest
- Fixed the DUMP code
- Fixed the documentation formatting
V5: - Fixed xmas tree
- Fixed uninitialized return variable (Simon)
V4: - Fixed errors when not having SFP enabled, resulting in null names
being passed as parameters to strlen.
V3: - Fixed the documentation
- Fixed the DUMP implementation
V2: New patch
Documentation/networking/ethtool-netlink.rst | 42 +++
include/uapi/linux/ethtool_netlink.h | 20 ++
net/ethtool/Makefile | 2 +-
net/ethtool/netlink.c | 9 +
net/ethtool/netlink.h | 5 +
net/ethtool/phy.c | 300 +++++++++++++++++++
6 files changed, 377 insertions(+), 1 deletion(-)
create mode 100644 net/ethtool/phy.c
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index fa126452c1c3..2871a6a772db 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -2011,6 +2011,47 @@ The attributes are propagated to the driver through the following structure:
.. kernel-doc:: include/linux/ethtool.h
:identifiers: ethtool_mm_cfg
+PHY_GET
+=======
+
+Retrieve information about a given Ethernet PHY sitting on the link. The DO
+operation returns all available information about dev->phydev. User can also
+specify a PHY_INDEX, in which case the DO request returns information about that
+specific PHY.
+As there can be more than one PHY, the DUMP operation can be used to list the PHYs
+present on a given interface, by passing an interface index or name in
+the dump request.
+
+Request contents:
+
+ ==================================== ====== ==========================
+ ``ETHTOOL_A_PHY_HEADER`` nested request header
+ ==================================== ====== ==========================
+
+Kernel response contents:
+
+ ===================================== ====== ===============================
+ ``ETHTOOL_A_PHY_HEADER`` nested request header
+ ``ETHTOOL_A_PHY_INDEX`` u32 the phy's unique index, that can
+ be used for phy-specific
+ requests
+ ``ETHTOOL_A_PHY_DRVNAME`` string the phy driver name
+ ``ETHTOOL_A_PHY_NAME`` string the phy device name
+ ``ETHTOOL_A_PHY_UPSTREAM_TYPE`` u32 the type of device this phy is
+ connected to
+ ``ETHTOOL_A_PHY_UPSTREAM_INDEX`` u32 the PHY index of the upstream
+ PHY
+ ``ETHTOOL_A_PHY_UPSTREAM_SFP_NAME`` string if this PHY is connected to
+ it's parent PHY through an SFP
+ bus, the name of this sfp bus
+ ``ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME`` string if the phy controls an sfp bus,
+ the name of the sfp bus
+ ``ETHTOOL_A_PHY_ID`` u32 the phy id if the phy is C22
+ ===================================== ====== ===============================
+
+When ``ETHTOOL_A_PHY_UPSTREAM_TYPE`` is PHY_UPSTREAM_PHY, the PHY's parent is
+another PHY.
+
Request translation
===================
@@ -2117,4 +2158,5 @@ are netlink only.
n/a ``ETHTOOL_MSG_PLCA_GET_STATUS``
n/a ``ETHTOOL_MSG_MM_GET``
n/a ``ETHTOOL_MSG_MM_SET``
+ n/a ``ETHTOOL_MSG_PHY_GET``
=================================== =====================================
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 3b07d8013fb6..1d23174211ed 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -57,6 +57,7 @@ enum {
ETHTOOL_MSG_PLCA_GET_STATUS,
ETHTOOL_MSG_MM_GET,
ETHTOOL_MSG_MM_SET,
+ ETHTOOL_MSG_PHY_GET,
/* add new constants above here */
__ETHTOOL_MSG_USER_CNT,
@@ -109,6 +110,8 @@ enum {
ETHTOOL_MSG_PLCA_NTF,
ETHTOOL_MSG_MM_GET_REPLY,
ETHTOOL_MSG_MM_NTF,
+ ETHTOOL_MSG_PHY_GET_REPLY,
+ ETHTOOL_MSG_PHY_NTF,
/* add new constants above here */
__ETHTOOL_MSG_KERNEL_CNT,
@@ -981,6 +984,23 @@ enum {
ETHTOOL_A_MM_MAX = (__ETHTOOL_A_MM_CNT - 1)
};
+enum {
+ ETHTOOL_A_PHY_UNSPEC,
+ ETHTOOL_A_PHY_HEADER, /* nest - _A_HEADER_* */
+ ETHTOOL_A_PHY_INDEX, /* u32 */
+ ETHTOOL_A_PHY_DRVNAME, /* string */
+ ETHTOOL_A_PHY_NAME, /* string */
+ ETHTOOL_A_PHY_UPSTREAM_TYPE, /* u32 */
+ ETHTOOL_A_PHY_UPSTREAM_INDEX, /* u32 */
+ ETHTOOL_A_PHY_UPSTREAM_SFP_NAME, /* string */
+ ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME, /* string */
+ ETHTOOL_A_PHY_ID, /* u32 */
+
+ /* add new constants above here */
+ __ETHTOOL_A_PHY_CNT,
+ ETHTOOL_A_PHY_MAX = (__ETHTOOL_A_PHY_CNT - 1)
+};
+
/* generic netlink info */
#define ETHTOOL_GENL_NAME "ethtool"
#define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 504f954a1b28..0ccd0e9afd3f 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -8,4 +8,4 @@ ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o rss.o \
linkstate.o debug.o wol.o features.o privflags.o rings.o \
channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
tunnels.o fec.o eeprom.o stats.o phc_vclocks.o mm.o \
- module.o pse-pd.o plca.o mm.o
+ module.o pse-pd.o plca.o mm.o phy.o
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 563e94e0cbd8..9847d2ee8402 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -1169,6 +1169,15 @@ static const struct genl_ops ethtool_genl_ops[] = {
.policy = ethnl_mm_set_policy,
.maxattr = ARRAY_SIZE(ethnl_mm_set_policy) - 1,
},
+ {
+ .cmd = ETHTOOL_MSG_PHY_GET,
+ .doit = ethnl_phy_doit,
+ .start = ethnl_phy_start,
+ .dumpit = ethnl_phy_dumpit,
+ .done = ethnl_phy_done,
+ .policy = ethnl_phy_get_policy,
+ .maxattr = ARRAY_SIZE(ethnl_phy_get_policy) - 1,
+ },
};
static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index d57a890b5d9e..0e71b53bdb1c 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -446,6 +446,7 @@ extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1]
extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
extern const struct nla_policy ethnl_mm_get_policy[ETHTOOL_A_MM_HEADER + 1];
extern const struct nla_policy ethnl_mm_set_policy[ETHTOOL_A_MM_MAX + 1];
+extern const struct nla_policy ethnl_phy_get_policy[ETHTOOL_A_PHY_HEADER + 1];
int ethnl_set_features(struct sk_buff *skb, struct genl_info *info);
int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
@@ -453,6 +454,10 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info);
int ethnl_tunnel_info_doit(struct sk_buff *skb, struct genl_info *info);
int ethnl_tunnel_info_start(struct netlink_callback *cb);
int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int ethnl_phy_start(struct netlink_callback *cb);
+int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info);
+int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int ethnl_phy_done(struct netlink_callback *cb);
extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN];
extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN];
diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
new file mode 100644
index 000000000000..0f6cb297dce2
--- /dev/null
+++ b/net/ethtool/phy.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2023 Bootlin
+ *
+ */
+#include "common.h"
+#include "netlink.h"
+
+#include <linux/phy.h>
+#include <linux/phy_link_topology.h>
+#include <linux/sfp.h>
+
+struct phy_req_info {
+ struct ethnl_req_info base;
+ struct phy_device_node pdn;
+};
+
+#define PHY_REQINFO(__req_base) \
+ container_of(__req_base, struct phy_req_info, base)
+
+const struct nla_policy ethnl_phy_get_policy[ETHTOOL_A_PHY_HEADER + 1] = {
+ [ETHTOOL_A_PHY_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+};
+
+/* Caller holds rtnl */
+static ssize_t
+ethnl_phy_reply_size(const struct ethnl_req_info *req_base,
+ struct netlink_ext_ack *extack)
+{
+ struct phy_req_info *req_info = PHY_REQINFO(req_base);
+ struct phy_device_node *pdn = &req_info->pdn;
+ struct phy_device *phydev = pdn->phy;
+ size_t size = 0;
+
+ ASSERT_RTNL();
+
+ /* ETHTOOL_A_PHY_INDEX */
+ size += nla_total_size(sizeof(u32));
+
+ /* ETHTOOL_A_DRVNAME */
+ if (phydev->drv)
+ size += nla_total_size(strlen(phydev->drv->name) + 1);
+
+ /* ETHTOOL_A_NAME */
+ size += nla_total_size(strlen(dev_name(&phydev->mdio.dev)) + 1);
+
+ /* ETHTOOL_A_PHY_UPSTREAM_TYPE */
+ size += nla_total_size(sizeof(u32));
+
+ /* ETHTOOL_A_PHY_ID */
+ size += nla_total_size(sizeof(u32));
+
+ if (phy_on_sfp(phydev)) {
+ const char *upstream_sfp_name = sfp_get_name(pdn->parent_sfp_bus);
+
+ /* ETHTOOL_A_PHY_UPSTREAM_SFP_NAME */
+ if (upstream_sfp_name)
+ size += nla_total_size(strlen(upstream_sfp_name) + 1);
+
+ /* ETHTOOL_A_PHY_UPSTREAM_INDEX */
+ size += nla_total_size(sizeof(u32));
+ }
+
+ /* ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME */
+ if (phydev->sfp_bus) {
+ const char *sfp_name = sfp_get_name(phydev->sfp_bus);
+
+ if (sfp_name)
+ size += nla_total_size(strlen(sfp_name) + 1);
+ }
+
+ return size;
+}
+
+static int
+ethnl_phy_fill_reply(const struct ethnl_req_info *req_base, struct sk_buff *skb)
+{
+ struct phy_req_info *req_info = PHY_REQINFO(req_base);
+ struct phy_device_node *pdn = &req_info->pdn;
+ struct phy_device *phydev = pdn->phy;
+ enum phy_upstream ptype;
+
+ ptype = pdn->upstream_type;
+
+ if (nla_put_u32(skb, ETHTOOL_A_PHY_INDEX, phydev->phyindex) ||
+ nla_put_string(skb, ETHTOOL_A_PHY_NAME, dev_name(&phydev->mdio.dev)) ||
+ nla_put_u32(skb, ETHTOOL_A_PHY_UPSTREAM_TYPE, ptype) ||
+ nla_put_u32(skb, ETHTOOL_A_PHY_ID, phydev->phy_id))
+ return -EMSGSIZE;
+
+ if (phydev->drv &&
+ nla_put_string(skb, ETHTOOL_A_PHY_DRVNAME, phydev->drv->name))
+ return -EMSGSIZE;
+
+ if (ptype == PHY_UPSTREAM_PHY) {
+ struct phy_device *upstream = pdn->upstream.phydev;
+ const char *sfp_upstream_name;
+
+ /* Parent index */
+ if (nla_put_u32(skb, ETHTOOL_A_PHY_UPSTREAM_INDEX, upstream->phyindex))
+ return -EMSGSIZE;
+
+ if (pdn->parent_sfp_bus) {
+ sfp_upstream_name = sfp_get_name(pdn->parent_sfp_bus);
+ if (sfp_upstream_name &&
+ nla_put_string(skb, ETHTOOL_A_PHY_UPSTREAM_SFP_NAME,
+ sfp_upstream_name))
+ return -EMSGSIZE;
+ }
+ }
+
+ if (phydev->sfp_bus) {
+ const char *sfp_name = sfp_get_name(phydev->sfp_bus);
+
+ if (sfp_name &&
+ nla_put_string(skb, ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME,
+ sfp_name))
+ return -EMSGSIZE;
+ }
+
+ return 0;
+}
+
+static int ethnl_phy_parse_request(struct ethnl_req_info *req_base,
+ struct nlattr **tb)
+{
+ struct phy_link_topology *topo = req_base->dev->link_topo;
+ struct phy_req_info *req_info = PHY_REQINFO(req_base);
+ struct phy_device_node *pdn;
+
+ if (!req_base->phydev)
+ return 0;
+
+ pdn = xa_load(&topo->phys, req_base->phydev->phyindex);
+ memcpy(&req_info->pdn, pdn, sizeof(*pdn));
+
+ return 0;
+}
+
+int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct phy_req_info req_info = {};
+ struct nlattr **tb = info->attrs;
+ struct sk_buff *rskb;
+ void *reply_payload;
+ int reply_len;
+ int ret;
+
+ ret = ethnl_parse_header_dev_get(&req_info.base,
+ tb[ETHTOOL_A_PHY_HEADER],
+ genl_info_net(info), info->extack,
+ true);
+ if (ret < 0)
+ return ret;
+
+ rtnl_lock();
+
+ ret = ethnl_phy_parse_request(&req_info.base, tb);
+ if (ret < 0)
+ goto err_unlock_rtnl;
+
+ /* No PHY, return early */
+ if (!req_info.pdn.phy)
+ goto err_unlock_rtnl;
+
+ ret = ethnl_phy_reply_size(&req_info.base, info->extack);
+ if (ret < 0)
+ goto err_unlock_rtnl;
+ reply_len = ret + ethnl_reply_header_size();
+
+ rskb = ethnl_reply_init(reply_len, req_info.base.dev,
+ ETHTOOL_MSG_PHY_GET_REPLY,
+ ETHTOOL_A_PHY_HEADER,
+ info, &reply_payload);
+ if (!rskb) {
+ ret = -ENOMEM;
+ goto err_unlock_rtnl;
+ }
+
+ ret = ethnl_phy_fill_reply(&req_info.base, rskb);
+ if (ret)
+ goto err_free_msg;
+
+ rtnl_unlock();
+ ethnl_parse_header_dev_put(&req_info.base);
+ genlmsg_end(rskb, reply_payload);
+
+ return genlmsg_reply(rskb, info);
+
+err_free_msg:
+ nlmsg_free(rskb);
+err_unlock_rtnl:
+ rtnl_unlock();
+ ethnl_parse_header_dev_put(&req_info.base);
+ return ret;
+}
+
+struct ethnl_phy_dump_ctx {
+ struct phy_req_info *phy_req_info;
+ unsigned long ifindex;
+ unsigned long phy_index;
+};
+
+int ethnl_phy_start(struct netlink_callback *cb)
+{
+ const struct genl_info *info = genl_info_dump(cb);
+ struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
+ int ret;
+
+ BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
+
+ ctx->phy_req_info = kzalloc(sizeof(*ctx->phy_req_info), GFP_KERNEL);
+ if (!ctx->phy_req_info)
+ return -ENOMEM;
+
+ ret = ethnl_parse_header_dev_get(&ctx->phy_req_info->base,
+ info->attrs[ETHTOOL_A_PHY_HEADER],
+ sock_net(cb->skb->sk), cb->extack,
+ false);
+ ctx->ifindex = 0;
+ ctx->phy_index = 0;
+
+ if (ret)
+ kfree(ctx->phy_req_info);
+
+ return ret;
+}
+
+int ethnl_phy_done(struct netlink_callback *cb)
+{
+ struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
+
+ ethnl_parse_header_dev_put(&ctx->phy_req_info->base);
+ kfree(ctx->phy_req_info);
+
+ return 0;
+}
+
+static int ethnl_phy_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
+ struct netlink_callback *cb)
+{
+ struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
+ struct phy_req_info *pri = ctx->phy_req_info;
+ struct phy_device_node *pdn;
+ int ret = 0;
+ void *ehdr;
+
+ pri->base.dev = dev;
+
+ xa_for_each_start(&dev->link_topo->phys, ctx->phy_index, pdn, ctx->phy_index) {
+ ehdr = ethnl_dump_put(skb, cb, ETHTOOL_MSG_PHY_GET_REPLY);
+ if (!ehdr) {
+ ret = -EMSGSIZE;
+ break;
+ }
+
+ ret = ethnl_fill_reply_header(skb, dev, ETHTOOL_A_PHY_HEADER);
+ if (ret < 0) {
+ genlmsg_cancel(skb, ehdr);
+ break;
+ }
+
+ memcpy(&pri->pdn, pdn, sizeof(*pdn));
+ ret = ethnl_phy_fill_reply(&pri->base, skb);
+ if (ret < 0) {
+ genlmsg_cancel(skb, ehdr);
+ break;
+ }
+
+ genlmsg_end(skb, ehdr);
+ }
+
+ return ret;
+}
+
+int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
+ struct net *net = sock_net(skb->sk);
+ struct net_device *dev;
+ int ret = 0;
+
+ rtnl_lock();
+
+ if (ctx->phy_req_info->base.dev) {
+ ret = ethnl_phy_dump_one_dev(skb, ctx->phy_req_info->base.dev, cb);
+ } else {
+ for_each_netdev_dump(net, dev, ctx->ifindex) {
+ ret = ethnl_phy_dump_one_dev(skb, dev, cb);
+ if (ret)
+ break;
+
+ ctx->phy_index = 0;
+ }
+ }
+ rtnl_unlock();
+
+ return ret;
+}
+
--
2.44.0
PLCA is a PHY-specific command. Instead of targeting the command
towards dev->phydev, use the request to pick the targeted PHY.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: No changes
V10: No changes
V9: No changes
V8: No changes
V7: No changes
V6: Use dedicated policy
V5: Added Andrew's R-b
V4: No changes
V3: No changes
V2: New patch
net/ethtool/plca.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/net/ethtool/plca.c b/net/ethtool/plca.c
index b1e2e3b5027f..43b31a4a164e 100644
--- a/net/ethtool/plca.c
+++ b/net/ethtool/plca.c
@@ -25,7 +25,7 @@ struct plca_reply_data {
const struct nla_policy ethnl_plca_get_cfg_policy[] = {
[ETHTOOL_A_PLCA_HEADER] =
- NLA_POLICY_NESTED(ethnl_header_policy),
+ NLA_POLICY_NESTED(ethnl_header_policy_phy),
};
static void plca_update_sint(int *dst, struct nlattr **tb, u32 attrid,
@@ -61,7 +61,7 @@ static int plca_get_cfg_prepare_data(const struct ethnl_req_info *req_base,
int ret;
// check that the PHY device is available and connected
- if (!dev->phydev) {
+ if (!req_base->phydev) {
ret = -EOPNOTSUPP;
goto out;
}
@@ -80,7 +80,7 @@ static int plca_get_cfg_prepare_data(const struct ethnl_req_info *req_base,
memset(&data->plca_cfg, 0xff,
sizeof_field(struct plca_reply_data, plca_cfg));
- ret = ops->get_plca_cfg(dev->phydev, &data->plca_cfg);
+ ret = ops->get_plca_cfg(req_base->phydev, &data->plca_cfg);
ethnl_ops_complete(dev);
out:
@@ -129,7 +129,7 @@ static int plca_get_cfg_fill_reply(struct sk_buff *skb,
const struct nla_policy ethnl_plca_set_cfg_policy[] = {
[ETHTOOL_A_PLCA_HEADER] =
- NLA_POLICY_NESTED(ethnl_header_policy),
+ NLA_POLICY_NESTED(ethnl_header_policy_phy),
[ETHTOOL_A_PLCA_ENABLED] = NLA_POLICY_MAX(NLA_U8, 1),
[ETHTOOL_A_PLCA_NODE_ID] = NLA_POLICY_MAX(NLA_U32, 255),
[ETHTOOL_A_PLCA_NODE_CNT] = NLA_POLICY_RANGE(NLA_U32, 1, 255),
@@ -141,7 +141,6 @@ const struct nla_policy ethnl_plca_set_cfg_policy[] = {
static int
ethnl_set_plca(struct ethnl_req_info *req_info, struct genl_info *info)
{
- struct net_device *dev = req_info->dev;
const struct ethtool_phy_ops *ops;
struct nlattr **tb = info->attrs;
struct phy_plca_cfg plca_cfg;
@@ -149,7 +148,7 @@ ethnl_set_plca(struct ethnl_req_info *req_info, struct genl_info *info)
int ret;
// check that the PHY device is available and connected
- if (!dev->phydev)
+ if (!req_info->phydev)
return -EOPNOTSUPP;
ops = ethtool_phy_ops;
@@ -168,7 +167,7 @@ ethnl_set_plca(struct ethnl_req_info *req_info, struct genl_info *info)
if (!mod)
return 0;
- ret = ops->set_plca_cfg(dev->phydev, &plca_cfg, info->extack);
+ ret = ops->set_plca_cfg(req_info->phydev, &plca_cfg, info->extack);
return ret < 0 ? ret : 1;
}
@@ -191,7 +190,7 @@ const struct ethnl_request_ops ethnl_plca_cfg_request_ops = {
const struct nla_policy ethnl_plca_get_status_policy[] = {
[ETHTOOL_A_PLCA_HEADER] =
- NLA_POLICY_NESTED(ethnl_header_policy),
+ NLA_POLICY_NESTED(ethnl_header_policy_phy),
};
static int plca_get_status_prepare_data(const struct ethnl_req_info *req_base,
@@ -204,7 +203,7 @@ static int plca_get_status_prepare_data(const struct ethnl_req_info *req_base,
int ret;
// check that the PHY device is available and connected
- if (!dev->phydev) {
+ if (!req_base->phydev) {
ret = -EOPNOTSUPP;
goto out;
}
@@ -223,7 +222,7 @@ static int plca_get_status_prepare_data(const struct ethnl_req_info *req_base,
memset(&data->plca_st, 0xff,
sizeof_field(struct plca_reply_data, plca_st));
- ret = ops->get_plca_status(dev->phydev, &data->plca_st);
+ ret = ops->get_plca_status(req_base->phydev, &data->plca_st);
ethnl_ops_complete(dev);
out:
return ret;
--
2.44.0
PSE and PD configuration is a PHY-specific command. Instead of targeting
the command towards dev->phydev, use the request to pick the targeted
PHY device.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: No changes
V10: No changes
V9: No changes
V8: No changes
V7: No changes
V6: Use dedicated policy
V5: Added-back an incorrectly removed check
V4: No changes
V3: No changes
V2: New patch
net/ethtool/pse-pd.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index cc478af77111..be50d79122c4 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -28,15 +28,13 @@ struct pse_reply_data {
/* PSE_GET */
const struct nla_policy ethnl_pse_get_policy[ETHTOOL_A_PSE_HEADER + 1] = {
- [ETHTOOL_A_PSE_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+ [ETHTOOL_A_PSE_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy_phy),
};
-static int pse_get_pse_attributes(struct net_device *dev,
+static int pse_get_pse_attributes(struct phy_device *phydev,
struct netlink_ext_ack *extack,
struct pse_reply_data *data)
{
- struct phy_device *phydev = dev->phydev;
-
if (!phydev) {
NL_SET_ERR_MSG(extack, "No PHY is attached");
return -EOPNOTSUPP;
@@ -64,7 +62,7 @@ static int pse_prepare_data(const struct ethnl_req_info *req_base,
if (ret < 0)
return ret;
- ret = pse_get_pse_attributes(dev, info->extack, data);
+ ret = pse_get_pse_attributes(req_base->phydev, info->extack, data);
ethnl_ops_complete(dev);
@@ -109,7 +107,7 @@ static int pse_fill_reply(struct sk_buff *skb,
/* PSE_SET */
const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
- [ETHTOOL_A_PSE_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+ [ETHTOOL_A_PSE_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy_phy),
[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] =
NLA_POLICY_RANGE(NLA_U32, ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED,
ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED),
@@ -124,7 +122,6 @@ ethnl_set_pse_validate(struct ethnl_req_info *req_info, struct genl_info *info)
static int
ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
{
- struct net_device *dev = req_info->dev;
struct pse_control_config config = {};
struct nlattr **tb = info->attrs;
struct phy_device *phydev;
@@ -132,7 +129,7 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
/* this values are already validated by the ethnl_pse_set_policy */
config.admin_cotrol = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
- phydev = dev->phydev;
+ phydev = req_info->phydev;
if (!phydev) {
NL_SET_ERR_MSG(info->extack, "No PHY is attached");
return -EOPNOTSUPP;
--
2.44.0
The ETH_SS_PHY_STATS command gets PHY statistics. Use the phydev pointer
from the ethnl request to allow query phy stats from each PHY on the
link.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: No changes
V10: No changes
V9: No changes
V8: No changes
V7: No changes
V6: Use dedicated policy
V5: Added Andrew's R-b
V4: No changes
V3: No changes
V2: New patch
net/ethtool/strset.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index c678b484a079..edc826407564 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -126,7 +126,7 @@ struct strset_reply_data {
const struct nla_policy ethnl_strset_get_policy[] = {
[ETHTOOL_A_STRSET_HEADER] =
- NLA_POLICY_NESTED(ethnl_header_policy),
+ NLA_POLICY_NESTED(ethnl_header_policy_phy),
[ETHTOOL_A_STRSET_STRINGSETS] = { .type = NLA_NESTED },
[ETHTOOL_A_STRSET_COUNTS_ONLY] = { .type = NLA_FLAG },
};
@@ -233,17 +233,18 @@ static void strset_cleanup_data(struct ethnl_reply_data *reply_base)
}
static int strset_prepare_set(struct strset_info *info, struct net_device *dev,
- unsigned int id, bool counts_only)
+ struct phy_device *phydev, unsigned int id,
+ bool counts_only)
{
const struct ethtool_phy_ops *phy_ops = ethtool_phy_ops;
const struct ethtool_ops *ops = dev->ethtool_ops;
void *strings;
int count, ret;
- if (id == ETH_SS_PHY_STATS && dev->phydev &&
+ if (id == ETH_SS_PHY_STATS && phydev &&
!ops->get_ethtool_phy_stats && phy_ops &&
phy_ops->get_sset_count)
- ret = phy_ops->get_sset_count(dev->phydev);
+ ret = phy_ops->get_sset_count(phydev);
else if (ops->get_sset_count && ops->get_strings)
ret = ops->get_sset_count(dev, id);
else
@@ -258,10 +259,10 @@ static int strset_prepare_set(struct strset_info *info, struct net_device *dev,
strings = kcalloc(count, ETH_GSTRING_LEN, GFP_KERNEL);
if (!strings)
return -ENOMEM;
- if (id == ETH_SS_PHY_STATS && dev->phydev &&
+ if (id == ETH_SS_PHY_STATS && phydev &&
!ops->get_ethtool_phy_stats && phy_ops &&
phy_ops->get_strings)
- phy_ops->get_strings(dev->phydev, strings);
+ phy_ops->get_strings(phydev, strings);
else
ops->get_strings(dev, id, strings);
info->strings = strings;
@@ -305,8 +306,8 @@ static int strset_prepare_data(const struct ethnl_req_info *req_base,
!data->sets[i].per_dev)
continue;
- ret = strset_prepare_set(&data->sets[i], dev, i,
- req_info->counts_only);
+ ret = strset_prepare_set(&data->sets[i], dev, req_base->phydev,
+ i, req_info->counts_only);
if (ret < 0)
goto err_ops;
}
--
2.44.0
Cable testing is a PHY-specific command. Instead of targeting the command
towards dev->phydev, use the request to pick the targeted PHY.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: No changes
V10: No changes
V9: No changes
V8: No changes
V7: No changes
V6: Use dedicated policy
V5: Added Andrew's R-b
V4: No changes
V3: No changes
V2: New patch
net/ethtool/cabletest.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/ethtool/cabletest.c b/net/ethtool/cabletest.c
index 06a151165c31..536800bbc379 100644
--- a/net/ethtool/cabletest.c
+++ b/net/ethtool/cabletest.c
@@ -13,7 +13,7 @@
const struct nla_policy ethnl_cable_test_act_policy[] = {
[ETHTOOL_A_CABLE_TEST_HEADER] =
- NLA_POLICY_NESTED(ethnl_header_policy),
+ NLA_POLICY_NESTED(ethnl_header_policy_phy),
};
static int ethnl_cable_test_started(struct phy_device *phydev, u8 cmd)
@@ -69,7 +69,7 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info)
return ret;
dev = req_info.dev;
- if (!dev->phydev) {
+ if (!req_info.phydev) {
ret = -EOPNOTSUPP;
goto out_dev_put;
}
@@ -85,12 +85,12 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info)
if (ret < 0)
goto out_rtnl;
- ret = ops->start_cable_test(dev->phydev, info->extack);
+ ret = ops->start_cable_test(req_info.phydev, info->extack);
ethnl_ops_complete(dev);
if (!ret)
- ethnl_cable_test_started(dev->phydev,
+ ethnl_cable_test_started(req_info.phydev,
ETHTOOL_MSG_CABLE_TEST_NTF);
out_rtnl:
@@ -220,7 +220,7 @@ static const struct nla_policy cable_test_tdr_act_cfg_policy[] = {
const struct nla_policy ethnl_cable_test_tdr_act_policy[] = {
[ETHTOOL_A_CABLE_TEST_TDR_HEADER] =
- NLA_POLICY_NESTED(ethnl_header_policy),
+ NLA_POLICY_NESTED(ethnl_header_policy_phy),
[ETHTOOL_A_CABLE_TEST_TDR_CFG] = { .type = NLA_NESTED },
};
@@ -321,7 +321,7 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info)
return ret;
dev = req_info.dev;
- if (!dev->phydev) {
+ if (!req_info.phydev) {
ret = -EOPNOTSUPP;
goto out_dev_put;
}
@@ -342,12 +342,12 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info)
if (ret < 0)
goto out_rtnl;
- ret = ops->start_cable_test_tdr(dev->phydev, info->extack, &cfg);
+ ret = ops->start_cable_test_tdr(req_info.phydev, info->extack, &cfg);
ethnl_ops_complete(dev);
if (!ret)
- ethnl_cable_test_started(dev->phydev,
+ ethnl_cable_test_started(req_info.phydev,
ETHTOOL_MSG_CABLE_TEST_TDR_NTF);
out_rtnl:
--
2.44.0
The newly introduced phy_link_topology tracks all ethernet PHYs that are
attached to a netdevice. Document the base principle, internal and
external APIs. As the phy_link_topology is expected to be extended, this
documentation will hold any further improvements and additions made
relative to topology handling.
Signed-off-by: Maxime Chevallier <[email protected]>
---
V11: - Fixed some typos and phrasing
- Link to that page from the ethtool-netlink documentation
V10: No changes
V9: No changes
V8: No changes
V7: No changes
V6: No changes
V5: Fixed a lot of typos
V4: No changes
V3: New patch
Documentation/networking/ethtool-netlink.rst | 3 +
Documentation/networking/index.rst | 1 +
.../networking/phy-link-topology.rst | 120 ++++++++++++++++++
3 files changed, 124 insertions(+)
create mode 100644 Documentation/networking/phy-link-topology.rst
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 2871a6a772db..ef225d3acfc6 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -2018,10 +2018,13 @@ Retrieve information about a given Ethernet PHY sitting on the link. The DO
operation returns all available information about dev->phydev. User can also
specify a PHY_INDEX, in which case the DO request returns information about that
specific PHY.
+
As there can be more than one PHY, the DUMP operation can be used to list the PHYs
present on a given interface, by passing an interface index or name in
the dump request.
+For more information, refer to :ref:`Documentation/networking/phy-link-topology.rst`
+
Request contents:
==================================== ====== ==========================
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 473d72c36d61..c29bc5179d8a 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -89,6 +89,7 @@ Contents:
operstates
packet_mmap
phonet
+ phy-link-topology
pktgen
plip
ppp_generic
diff --git a/Documentation/networking/phy-link-topology.rst b/Documentation/networking/phy-link-topology.rst
new file mode 100644
index 000000000000..1f021419ae8c
--- /dev/null
+++ b/Documentation/networking/phy-link-topology.rst
@@ -0,0 +1,120 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+PHY link topology
+=================
+
+Overview
+========
+
+The PHY link topology representation in the networking stack aims at representing
+the hardware layout for any given Ethernet link.
+
+An Ethernet interface from userspace's point of view is nothing but a
+:c:type:`struct net_device <net_device>`, which exposes configuration options
+through the legacy ioctls and the ethtool netlink commands. The base assumption
+when designing these configuration APIs were that the link looks something like ::
+
+ +-----------------------+ +----------+ +--------------+
+ | Ethernet Controller / | | Ethernet | | Connector / |
+ | MAC | ------ | PHY | ---- | Port | ---... to LP
+ +-----------------------+ +----------+ +--------------+
+ struct net_device struct phy_device
+
+Commands that needs to configure the PHY will go through the net_device.phydev
+field to reach the PHY and perform the relevant configuration.
+
+This assumption falls apart in more complex topologies that can arise when,
+for example, using SFP transceivers (although that's not the only specific case).
+
+Here, we have 2 basic scenarios. Either the MAC is able to output a serialized
+interface, that can directly be fed to an SFP cage, such as SGMII, 1000BaseX,
+10GBaseR, etc.
+
+The link topology then looks like this (when an SFP module is inserted) ::
+
+ +-----+ SGMII +------------+
+ | MAC | ------- | SFP Module |
+ +-----+ +------------+
+
+Knowing that some modules embed a PHY, the actual link is more like ::
+
+ +-----+ SGMII +--------------+
+ | MAC | -------- | PHY (on SFP) |
+ +-----+ +--------------+
+
+In this case, the SFP PHY is handled by phylib, and registered by phylink through
+its SFP upstream ops.
+
+Now some Ethernet controllers aren't able to output a serialized interface, so
+we can't directly connect them to an SFP cage. However, some PHYs can be used
+as media-converters, to translate the non-serialized MAC MII interface to a
+serialized MII interface fed to the SFP ::
+
+ +-----+ RGMII +-----------------------+ SGMII +--------------+
+ | MAC | ------- | PHY (media converter) | ------- | PHY (on SFP) |
+ +-----+ +-----------------------+ +--------------+
+
+This is where the model of having a single net_device.phydev pointer shows its
+limitations, as we now have 2 PHYs on the link.
+
+The phy_link topology framework aims at providing a way to keep track of every
+PHY on the link, for use by both kernel drivers and subsystems, but also to
+report the topology to userspace, allowing to target individual PHYs in configuration
+commands.
+
+API
+===
+
+The :c:type:`struct phy_link_topology <phy_link_topology>` is a per-netdevice
+resource, that gets initialized at netdevice creation. Once it's initialized,
+it is then possible to register PHYs to the topology through :
+
+:c:func:`phy_link_topo_add_phy`
+
+Besides registering the PHY to the topology, this call will also assign a unique
+index to the PHY, which can then be reported to userspace to refer to this PHY
+(akin to the ifindex). This index is a u32, ranging from 1 to U32_MAX. The value
+0 is reserved to indicate the PHY doesn't belong to any topology yet.
+
+The PHY can then be removed from the topology through
+
+:c:func:`phy_link_topo_del_phy`
+
+These function are already hooked into the phylib subsystem, so all PHYs that
+are linked to a net_device through :c:func:`phy_attach_direct` will automatically
+join the netdev's topology.
+
+PHYs that are on a SFP module will also be automatically registered IF the SFP
+upstream is phylink (so, no media-converter).
+
+PHY drivers that can be used as SFP upstream need to call :c:func:`phy_sfp_attach_phy`
+and :c:func:`phy_sfp_detach_phy`, which can be used as a
+.attach_phy / .detach_phy implementation for the
+:c:type:`struct sfp_upstream_ops <sfp_upstream_ops>`.
+
+UAPI
+====
+
+There exist a set of netlink commands to query the link topology from userspace,
+see ``Documentation/networking/ethtool-netlink.rst``.
+
+The whole point of having a topology representation is to assign the phyindex
+field in :c:type:`struct phy_device <phy_device>`. This index is reported to
+userspace using the ``ETHTOOL_MSG_PHY_GET`` ethtnl command. Performing a DUMP operation
+will result in all PHYs from all net_device being listed. The DUMP command
+accepts either a ``ETHTOOL_A_HEADER_DEV_INDEX`` or ``ETHTOOL_A_HEADER_DEV_NAME``
+to be passed in the request to filter the DUMP to a single net_device.
+
+The retrieved index can then be passed as a request parameter using the
+``ETHTOOL_A_HEADER_PHY_INDEX`` field in the following ethnl commands :
+
+* ``ETHTOOL_MSG_STRSET_GET`` to get the stats string set from a given PHY
+* ``ETHTOOL_MSG_CABLE_TEST_ACT`` and ``ETHTOOL_MSG_CABLE_TEST_ACT``, to perform
+ cable testing on a given PHY on the link (most likely the outermost PHY)
+* ``ETHTOOL_MSG_PSE_SET`` and ``ETHTOOL_MSG_PSE_GET`` for PHY-controlled PoE and PSE settings
+* ``ETHTOOL_MSG_PLCA_GET_CFG``, ``ETHTOOL_MSG_PLCA_SET_CFG`` and ``ETHTOOL_MSG_PLCA_GET_STATUS``
+ to set the PLCA (Physical Layer Collision Avoidance) parameters
+
+Note that the PHY index can be passed to other requests, which will silently
+ignore it if present and irrelevant.
--
2.44.0
On Thu, Apr 04, 2024 at 11:29:51AM +0200, 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. The PHY index can be
> re-used for PHYs that are persistent.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:29:52AM +0200, Maxime Chevallier wrote:
> Pass the phy_device as a parameter to the sfp upstream .disconnect_phy
> operation. This is preparatory work to help track phy devices across
> a net_device's link.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:29:53AM +0200, Maxime Chevallier wrote:
> There are a few PHY drivers that can handle SFP modules through their
> sfp_upstream_ops. Introduce Phylib helpers to keep track of connected
> SFP PHYs in a netdevice's namespace, by adding the SFP PHY to the
> upstream PHY's netdev's namespace.
>
> By doing so, these SFP PHYs can be enumerated and exposed to users,
> which will be able to use their capabilities.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:29:57AM +0200, Maxime Chevallier wrote:
> As we have the ability to track the PHYs connected to a net_device
> through the link_topology, we can expose this list to userspace. This
> allows userspace to use these identifiers for phy-specific commands and
> take the decision of which PHY to target by knowing the link topology.
>
> Add PHY_GET and PHY_DUMP, which can be a filtered DUMP operation to list
> devices on only one interface.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
It would be good if Jakub reviewed this as well, since i don't know
netlink too well. But:
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:29:59AM +0200, Maxime Chevallier wrote:
> PLCA is a PHY-specific command. Instead of targeting the command
> towards dev->phydev, use the request to pick the targeted PHY.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:30:00AM +0200, Maxime Chevallier wrote:
> PSE and PD configuration is a PHY-specific command. Instead of targeting
> the command towards dev->phydev, use the request to pick the targeted
> PHY device.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:30:01AM +0200, Maxime Chevallier wrote:
> Cable testing is a PHY-specific command. Instead of targeting the command
> towards dev->phydev, use the request to pick the targeted PHY.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:30:02AM +0200, Maxime Chevallier wrote:
> The ETH_SS_PHY_STATS command gets PHY statistics. Use the phydev pointer
> from the ethnl request to allow query phy stats from each PHY on the
> link.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:30:03AM +0200, Maxime Chevallier wrote:
> The newly introduced phy_link_topology tracks all ethernet PHYs that are
> attached to a netdevice. Document the base principle, internal and
> external APIs. As the phy_link_topology is expected to be extended, this
> documentation will hold any further improvements and additions made
> relative to topology handling.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 04, 2024 at 11:29:58AM +0200, Maxime Chevallier wrote:
> The PHY_GET command, supporting both DUMP and GET operations, is used to
> retrieve the list of PHYs connected to a netdevice, and get topology
> information to know where exactly it sits on the physical link.
>
> Add the netlink specs corresponding to that command.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
This i don't feel qualified to review this, other than i don't see any
obvious spelling mistakes etc.
Is there a tool to sanity check/validate these spec files? If so,
could it be added to NIPA? Added to Simon's list of checks?
Andrew
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
On Thu, 4 Apr 2024 11:29:50 +0200 you wrote:
> Hello everyone,
>
> This is V11 for the link topology addition, allowing to track all PHYs
> that are linked to netdevices.
>
> This V11 addresses the various netlink-related issues that were raised
> by Jakub, and fixes some typos in the documentation.
>
> [...]
Here is the summary with links:
- [net-next,v11,01/13] net: phy: Introduce ethernet link topology representation
https://git.kernel.org/netdev/net-next/c/6916e461e793
- [net-next,v11,02/13] net: sfp: pass the phy_device when disconnecting an sfp module's PHY
https://git.kernel.org/netdev/net-next/c/0ec5ed6c130e
- [net-next,v11,03/13] net: phy: add helpers to handle sfp phy connect/disconnect
https://git.kernel.org/netdev/net-next/c/e75e4e074c44
- [net-next,v11,04/13] net: sfp: Add helper to return the SFP bus name
https://git.kernel.org/netdev/net-next/c/fdd353965b52
- [net-next,v11,05/13] net: ethtool: Allow passing a phy index for some commands
https://git.kernel.org/netdev/net-next/c/841942bc6212
- [net-next,v11,06/13] netlink: specs: add phy-index as a header parameter
(no matching commit)
- [net-next,v11,07/13] net: ethtool: Introduce a command to list PHYs on an interface
(no matching commit)
- [net-next,v11,08/13] netlink: specs: add ethnl PHY_GET command set
(no matching commit)
- [net-next,v11,09/13] net: ethtool: plca: Target the command to the requested PHY
(no matching commit)
- [net-next,v11,10/13] net: ethtool: pse-pd: Target the command to the requested PHY
(no matching commit)
- [net-next,v11,11/13] net: ethtool: cable-test: Target the command to the requested PHY
(no matching commit)
- [net-next,v11,12/13] net: ethtool: strset: Allow querying phy stats by index
(no matching commit)
- [net-next,v11,13/13] Documentation: networking: document phy_link_topology
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Hi,
On Sat, 06 Apr 2024 17:30:29 +0000
[email protected] wrote:
> Hello:
>
> This series was applied to netdev/net-next.git (main)
> by David S. Miller <[email protected]>:
>
> On Thu, 4 Apr 2024 11:29:50 +0200 you wrote:
> > Hello everyone,
> >
> > This is V11 for the link topology addition, allowing to track all PHYs
> > that are linked to netdevices.
> >
> > This V11 addresses the various netlink-related issues that were raised
> > by Jakub, and fixes some typos in the documentation.
> >
> > [...]
>
> Here is the summary with links:
> - [net-next,v11,01/13] net: phy: Introduce ethernet link topology representation
> https://git.kernel.org/netdev/net-next/c/6916e461e793
> - [net-next,v11,02/13] net: sfp: pass the phy_device when disconnecting an sfp module's PHY
> https://git.kernel.org/netdev/net-next/c/0ec5ed6c130e
> - [net-next,v11,03/13] net: phy: add helpers to handle sfp phy connect/disconnect
> https://git.kernel.org/netdev/net-next/c/e75e4e074c44
> - [net-next,v11,04/13] net: sfp: Add helper to return the SFP bus name
> https://git.kernel.org/netdev/net-next/c/fdd353965b52
> - [net-next,v11,05/13] net: ethtool: Allow passing a phy index for some commands
> https://git.kernel.org/netdev/net-next/c/841942bc6212
> - [net-next,v11,06/13] netlink: specs: add phy-index as a header parameter
> (no matching commit)
> - [net-next,v11,07/13] net: ethtool: Introduce a command to list PHYs on an interface
> (no matching commit)
> - [net-next,v11,08/13] netlink: specs: add ethnl PHY_GET command set
> (no matching commit)
> - [net-next,v11,09/13] net: ethtool: plca: Target the command to the requested PHY
> (no matching commit)
> - [net-next,v11,10/13] net: ethtool: pse-pd: Target the command to the requested PHY
> (no matching commit)
> - [net-next,v11,11/13] net: ethtool: cable-test: Target the command to the requested PHY
> (no matching commit)
> - [net-next,v11,12/13] net: ethtool: strset: Allow querying phy stats by index
> (no matching commit)
> - [net-next,v11,13/13] Documentation: networking: document phy_link_topology
> (no matching commit)
It looks like commits 6 to 13 didn't make it upstream with (the "no
matching commit" messages above). Is that expected ?
Thanks,
Maxime
> > Here is the summary with links:
> > - [net-next,v11,01/13] net: phy: Introduce ethernet link topology representation
> > https://git.kernel.org/netdev/net-next/c/6916e461e793
> > - [net-next,v11,02/13] net: sfp: pass the phy_device when disconnecting an sfp module's PHY
> > https://git.kernel.org/netdev/net-next/c/0ec5ed6c130e
> > - [net-next,v11,03/13] net: phy: add helpers to handle sfp phy connect/disconnect
> > https://git.kernel.org/netdev/net-next/c/e75e4e074c44
> > - [net-next,v11,04/13] net: sfp: Add helper to return the SFP bus name
> > https://git.kernel.org/netdev/net-next/c/fdd353965b52
> > - [net-next,v11,05/13] net: ethtool: Allow passing a phy index for some commands
> > https://git.kernel.org/netdev/net-next/c/841942bc6212
> > - [net-next,v11,06/13] netlink: specs: add phy-index as a header parameter
> > (no matching commit)
> > - [net-next,v11,07/13] net: ethtool: Introduce a command to list PHYs on an interface
> > (no matching commit)
> > - [net-next,v11,08/13] netlink: specs: add ethnl PHY_GET command set
> > (no matching commit)
> > - [net-next,v11,09/13] net: ethtool: plca: Target the command to the requested PHY
> > (no matching commit)
> > - [net-next,v11,10/13] net: ethtool: pse-pd: Target the command to the requested PHY
> > (no matching commit)
> > - [net-next,v11,11/13] net: ethtool: cable-test: Target the command to the requested PHY
> > (no matching commit)
> > - [net-next,v11,12/13] net: ethtool: strset: Allow querying phy stats by index
> > (no matching commit)
> > - [net-next,v11,13/13] Documentation: networking: document phy_link_topology
> > (no matching commit)
>
> It looks like commits 6 to 13 didn't make it upstream with (the "no
> matching commit" messages above). Is that expected ?
They are not in net-next, unlike 1-5.
You probably need to repost them.
Andrew
On Tue, 9 Apr 2024 01:50:54 +0200
Andrew Lunn <[email protected]> wrote:
> > > Here is the summary with links:
> > > - [net-next,v11,01/13] net: phy: Introduce ethernet link topology representation
> > > https://git.kernel.org/netdev/net-next/c/6916e461e793
> > > - [net-next,v11,02/13] net: sfp: pass the phy_device when disconnecting an sfp module's PHY
> > > https://git.kernel.org/netdev/net-next/c/0ec5ed6c130e
> > > - [net-next,v11,03/13] net: phy: add helpers to handle sfp phy connect/disconnect
> > > https://git.kernel.org/netdev/net-next/c/e75e4e074c44
> > > - [net-next,v11,04/13] net: sfp: Add helper to return the SFP bus name
> > > https://git.kernel.org/netdev/net-next/c/fdd353965b52
> > > - [net-next,v11,05/13] net: ethtool: Allow passing a phy index for some commands
> > > https://git.kernel.org/netdev/net-next/c/841942bc6212
> > > - [net-next,v11,06/13] netlink: specs: add phy-index as a header parameter
> > > (no matching commit)
> > > - [net-next,v11,07/13] net: ethtool: Introduce a command to list PHYs on an interface
> > > (no matching commit)
> > > - [net-next,v11,08/13] netlink: specs: add ethnl PHY_GET command set
> > > (no matching commit)
> > > - [net-next,v11,09/13] net: ethtool: plca: Target the command to the requested PHY
> > > (no matching commit)
> > > - [net-next,v11,10/13] net: ethtool: pse-pd: Target the command to the requested PHY
> > > (no matching commit)
> > > - [net-next,v11,11/13] net: ethtool: cable-test: Target the command to the requested PHY
> > > (no matching commit)
> > > - [net-next,v11,12/13] net: ethtool: strset: Allow querying phy stats by index
> > > (no matching commit)
> > > - [net-next,v11,13/13] Documentation: networking: document phy_link_topology
> > > (no matching commit)
> >
> > It looks like commits 6 to 13 didn't make it upstream with (the "no
> > matching commit" messages above). Is that expected ?
>
> They are not in net-next, unlike 1-5.
>
> You probably need to repost them.
I'll repost indeed.
Thanks Andrew BTW for all the reviews !
Maxime
Hi Maxime,
On Thu, Apr 04, 2024 at 11:29:51AM +0200, 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. The PHY index can be
> re-used for PHYs that are persistent.
>
> Signed-off-by: Maxime Chevallier <[email protected]>
> ---
> V11: - No changes
> V10: - No changes
> V9: - No changes
> V8: - Rebase on net-next and fixed conflicts
> V7: - Protected the phy_link_topo helpers/stubs with IS_REACHABLE
> V6: - Made link_topo a pointer
> - Reworked the init/cleanup sequence
> - Added phy_index recycling if possible
> V5: - Dropped the ASSERT_RTNL()
> - Made the phy_link_topo_get_phy inline
> V4: - Moved the phy_link_topo_init() code to an inline header function
> - Made the code build without phylib
> 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 | 7 ++
> drivers/net/phy/phy_link_topology.c | 105 +++++++++++++++++++++++++
> include/linux/netdevice.h | 4 +-
> include/linux/phy.h | 4 +
> include/linux/phy_link_topology.h | 72 +++++++++++++++++
> include/linux/phy_link_topology_core.h | 25 ++++++
> include/uapi/linux/ethtool.h | 16 ++++
> net/core/dev.c | 9 +++
> 10 files changed, 244 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 909c2c531d8e..db0aa3a926ae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8016,6 +8016,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 202ed7f450da..1d8be374915f 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 6c6ec9475709..452fc8b3406d 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>
> @@ -1511,6 +1512,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
> @@ -1938,6 +1944,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..985941c5c558
> --- /dev/null
> +++ b/drivers/net/phy/phy_link_topology.c
> @@ -0,0 +1,105 @@
> +// 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_link_topology *phy_link_topo_create(struct net_device *dev)
> +{
> + struct phy_link_topology *topo;
> +
> + topo = kzalloc(sizeof(*topo), GFP_KERNEL);
> + if (!topo)
> + return ERR_PTR(-ENOMEM);
> +
> + xa_init_flags(&topo->phys, XA_FLAGS_ALLOC1);
> + topo->next_phy_index = 1;
> +
> + return topo;
> +}
> +
> +void phy_link_topo_destroy(struct phy_link_topology *topo)
> +{
> + if (!topo)
> + return;
> +
> + xa_destroy(&topo->phys);
> + kfree(topo);
> +}
> +
> +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;
> +
> + 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;
> +
> + /* Attempt to re-use a previously allocated phy_index */
> + if (phy->phyindex) {
> + ret = xa_insert(&topo->phys, phy->phyindex, pdn, GFP_KERNEL);
> +
> + /* Errors could be either -ENOMEM or -EBUSY. If the phy has an
> + * index, and there's another entry at the same index, this is
> + * unexpected and we still error-out
> + */
> + if (ret)
> + goto err;
> + return 0;
> + }
> +
> + 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);
> +
> + /* We delete the PHY from the topology, however we don't re-set the
> + * phy->phyindex field. If the PHY isn't gone, we can re-assign it the
> + * same index next time it's added back to the topology
> + */
> +
> + kfree(pdn);
> +}
> +EXPORT_SYMBOL_GPL(phy_link_topo_del_phy);
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 0c198620ac93..d45f330d083d 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;
> @@ -1974,6 +1974,7 @@ enum netdev_reg_state {
> * @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.
> @@ -2364,6 +2365,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 e6e83304558e..8c848c79b1fd 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -550,6 +550,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.
> @@ -650,6 +653,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..6b79feb607e7
> --- /dev/null
> +++ b/include/linux/phy_link_topology.h
> @@ -0,0 +1,72 @@
> +/* 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;
> +};
> +
> +struct phy_link_topology {
> + struct xarray phys;
> + u32 next_phy_index;
> +};
> +
> +static inline 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;
> +}
> +
> +#if IS_REACHABLE(CONFIG_PHYLIB)
> +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 inline int phy_link_topo_add_phy(struct phy_link_topology *topo,
> + struct phy_device *phy,
> + enum phy_upstream upt, void *upstream)
> +{
> + return 0;
> +}
> +
> +static inline 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..0a6479055745
> --- /dev/null
> +++ b/include/linux/phy_link_topology_core.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PHY_LINK_TOPOLOGY_CORE_H
> +#define __PHY_LINK_TOPOLOGY_CORE_H
> +
> +struct phy_link_topology;
> +
> +#if IS_REACHABLE(CONFIG_PHYLIB)
> +
> +struct phy_link_topology *phy_link_topo_create(struct net_device *dev);
> +void phy_link_topo_destroy(struct phy_link_topology *topo);
> +
> +#else
> +
> +static inline struct phy_link_topology *phy_link_topo_create(struct net_device *dev)
> +{
> + return NULL;
> +}
> +
> +static inline void phy_link_topo_destroy(struct phy_link_topology *topo)
> +{
> +}
> +
> +#endif
> +
> +#endif /* __PHY_LINK_TOPOLOGY_CORE_H */
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 11fc18988bc2..95c2f09f0d0a 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -2268,4 +2268,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 9b821d96eff3..928cf377e843 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -158,6 +158,7 @@
> #include <net/page_pool/types.h>
> #include <net/page_pool/helpers.h>
> #include <net/rps.h>
> +#include <linux/phy_link_topology_core.h>
>
> #include "dev.h"
> #include "net-sysfs.h"
> @@ -10962,6 +10963,12 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> #ifdef CONFIG_NET_SCHED
> hash_init(dev->qdisc_hash);
> #endif
> + dev->link_topo = phy_link_topo_create(dev);
> + if (IS_ERR(dev->link_topo)) {
> + dev->link_topo = NULL;
> + goto free_all;
> + }
> +
> dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
> setup(dev);
>
> @@ -11050,6 +11057,8 @@ void free_netdev(struct net_device *dev)
> free_percpu(dev->xdp_bulkq);
> dev->xdp_bulkq = NULL;
>
> + phy_link_topo_destroy(dev->link_topo);
> +
> /* Compatibility with error handling in drivers */
> if (dev->reg_state == NETREG_UNINITIALIZED) {
> netdev_freemem(dev);
> --
> 2.44.0
>
I bisected a crash that I see on one of my test devices to this change
in -next as commit 6916e461e793 ("net: phy: Introduce ethernet link
topology representation"). Here is the stack trace passed through
scripts/decode_stacktrace.sh:
[ 0.000000] Linux version 6.9.0-rc2-debug-00664-g6916e461e793 ([email protected]) (x86_64-linux-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP PREEMPT_DYNAMIC Tue Apr 9 12:13:50 MST 2024
...
[ 5.616112] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 5.616148] #PF: supervisor write access in kernel mode
[ 5.616168] #PF: error_code(0x0002) - not-present page
[ 5.616185] PGD 0 P4D 0
[ 5.616199] Oops: 0002 [#1] PREEMPT SMP PTI
[ 5.616216] CPU: 1 PID: 263 Comm: (udev-worker) Not tainted 6.9.0-rc2-debug-00664-g6916e461e793 #1 70fb9ac6019045b1a6f31076c518c9320a4bc47a
[ 5.616255] Hardware name: ASUSTeK COMPUTER INC. Q302LA/Q302LA, BIOS Q302LA.203 05/15/2014
[ 5.616279] RIP: 0010:_raw_spin_lock (arch/x86/include/asm/atomic.h:115 (discriminator 4) include/linux/atomic/atomic-arch-fallback.h:2170 (discriminator 4) include/linux/atomic/atomic-instrumented.h:1302 (discriminator 4) include/asm-generic/qspinlock.h:111 (discriminator 4) include/linux/spinlock.h:187 (discriminator 4) include/linux/spinlock_api_smp.h:134 (discriminator 4) kernel/locking/spinlock.c:154 (discriminator 4))
[ 5.616301] Code: 00 66 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 65 ff 05 e8 51 30 4c 31 c0 ba 01 00 00 00 <f0> 0f b1 17 75 05 c3 cc cc cc cc 89 c6 e8 97 01 00 00 90 c3 cc cc
All code
========
0: 00 66 90 add %ah,-0x70(%rsi)
3: 90 nop
4: 90 nop
5: 90 nop
6: 90 nop
7: 90 nop
8: 90 nop
9: 90 nop
a: 90 nop
b: 90 nop
c: 90 nop
d: 90 nop
e: 90 nop
f: 90 nop
10: 90 nop
11: 90 nop
12: 90 nop
13: f3 0f 1e fa endbr64
17: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
1c: 65 ff 05 e8 51 30 4c incl %gs:0x4c3051e8(%rip) # 0x4c30520b
23: 31 c0 xor %eax,%eax
25: ba 01 00 00 00 mov $0x1,%edx
2a:* f0 0f b1 17 lock cmpxchg %edx,(%rdi) <-- trapping instruction
2e: 75 05 jne 0x35
30: c3 ret
31: cc int3
32: cc int3
33: cc int3
34: cc int3
35: 89 c6 mov %eax,%esi
37: e8 97 01 00 00 call 0x1d3
3c: 90 nop
3d: c3 ret
3e: cc int3
3f: cc int3
Code starting with the faulting instruction
===========================================
0: f0 0f b1 17 lock cmpxchg %edx,(%rdi)
4: 75 05 jne 0xb
6: c3 ret
7: cc int3
8: cc int3
9: cc int3
a: cc int3
b: 89 c6 mov %eax,%esi
d: e8 97 01 00 00 call 0x1a9
12: 90 nop
13: c3 ret
14: cc int3
15: cc int3
[ 5.616355] RSP: 0000:ffffc3aec04bba38 EFLAGS: 00010246
[ 5.617083] RAX: 0000000000000000 RBX: ffffa032d3db1c40 RCX: 0000000000000000
[ 5.617785] RDX: 0000000000000001 RSI: ffffffffc10385e3 RDI: 0000000000000000
[ 5.618502] RBP: ffffa032cdb2f800 R08: 0000000000000020 R09: 0000000000000000
[ 5.619224] R10: ffffc3aec04bba40 R11: 0000000000000001 R12: 0000000000000000
[ 5.619970] R13: 0000000000000000 R14: ffffa032c5ea3000 R15: ffffa032c11f80a8
[ 5.620722] FS: 00007f67d520b540(0000) GS:ffffa033d6e80000(0000) knlGS:0000000000000000
[ 5.621393] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 5.622152] CR2: 0000000000000000 CR3: 0000000103eaa004 CR4: 00000000001706f0
[ 5.622942] Call Trace:
[ 5.623653] <TASK>
[ 5.624329] ? __die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434)
[ 5.625025] ? page_fault_oops (arch/x86/mm/fault.c:713 (discriminator 1))
[ 5.625791] ? exc_page_fault (arch/x86/include/asm/paravirt.h:693 arch/x86/mm/fault.c:1513 arch/x86/mm/fault.c:1563)
[ 5.626525] ? asm_exc_page_fault (arch/x86/include/asm/idtentry.h:623)
[ 5.626535] ? phy_link_topo_add_phy (drivers/net/phy/phy_link_topology.c:46) libphy
[ 5.627954] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:115 (discriminator 4) include/linux/atomic/atomic-arch-fallback.h:2170 (discriminator 4) include/linux/atomic/atomic-instrumented.h:1302 (discriminator 4) include/asm-generic/qspinlock.h:111 (discriminator 4) include/linux/spinlock.h:187 (discriminator 4) include/linux/spinlock_api_smp.h:134 (discriminator 4) kernel/locking/spinlock.c:154 (discriminator 4))
[ 5.627963] phy_link_topo_add_phy (include/linux/xarray.h:977 drivers/net/phy/phy_link_topology.c:80) libphy
[ 5.629462] phy_attach_direct (drivers/net/phy/phy_device.c:1516) libphy
[ 5.629504] phylink_connect_phy (drivers/net/phy/phylink.c:1983) phylink
[ 5.631030] ax88772_bind (drivers/net/usb/asix_devices.c:710 drivers/net/usb/asix_devices.c:919) asix
[ 5.631049] usbnet_probe (drivers/net/usb/usbnet.c:1745) usbnet
[ 5.631062] ? __pfx_read_tsc (arch/x86/kernel/tsc.c:1129)
[ 5.631068] ? ktime_get_mono_fast_ns (kernel/time/timekeeping.c:438 kernel/time/timekeeping.c:452 kernel/time/timekeeping.c:492)
[ 5.631075] usb_probe_interface (drivers/usb/core/driver.c:400)
[ 5.631082] really_probe (drivers/base/dd.c:578 drivers/base/dd.c:656)
[ 5.631087] ? __pfx___driver_attach (drivers/base/dd.c:1155)
[ 5.631091] __driver_probe_device (drivers/base/dd.c:798)
[ 5.631094] driver_probe_device (drivers/base/dd.c:828)
[ 5.631098] __driver_attach (drivers/base/dd.c:1215)
[ 5.631102] bus_for_each_dev (drivers/base/bus.c:368)
[ 5.631108] bus_add_driver (drivers/base/bus.c:673)
[ 5.631112] driver_register (drivers/base/driver.c:246)
[ 5.631117] usb_register_driver (drivers/usb/core/driver.c:1068)
[ 5.631120] ? __pfx_asix_driver_init (drivers/net/usb/asix_devices.c:745) asix
[ 5.631134] do_one_initcall (init/main.c:1238)
[ 5.631141] do_init_module (kernel/module/main.c:2538)
[ 5.631147] init_module_from_file (kernel/module/main.c:3169)
[ 5.631153] idempotent_init_module (include/linux/spinlock.h:351 kernel/module/main.c:3131 kernel/module/main.c:3185)
[ 5.631158] __x64_sys_finit_module (include/linux/file.h:47 kernel/module/main.c:3207 kernel/module/main.c:3189 kernel/module/main.c:3189)
[ 5.631163] do_syscall_64 (arch/x86/entry/common.c:52 (discriminator 1) arch/x86/entry/common.c:83 (discriminator 1))
[ 5.631168] ? do_syscall_64 (arch/x86/include/asm/cpufeature.h:173 arch/x86/entry/common.c:98)
[ 5.631171] ? do_syscall_64 (arch/x86/include/asm/cpufeature.h:173 arch/x86/entry/common.c:98)
[ 5.631174] ? exc_page_fault (arch/x86/include/asm/paravirt.h:693 arch/x86/mm/fault.c:1513 arch/x86/mm/fault.c:1563)
[ 5.631179] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:129)
[ 5.631187] RIP: 0033:0x7f67d4d2488d
[ 5.631217] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 73 14 0d 00 f7 d8 64 89 01 48
All code
========
0: ff c3 inc %ebx
2: 66 2e 0f 1f 84 00 00 cs nopw 0x0(%rax,%rax,1)
9: 00 00 00
c: 90 nop
d: f3 0f 1e fa endbr64
11: 48 89 f8 mov %rdi,%rax
14: 48 89 f7 mov %rsi,%rdi
17: 48 89 d6 mov %rdx,%rsi
1a: 48 89 ca mov %rcx,%rdx
1d: 4d 89 c2 mov %r8,%r10
20: 4d 89 c8 mov %r9,%r8
23: 4c 8b 4c 24 08 mov 0x8(%rsp),%r9
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 ret
33: 48 8b 0d 73 14 0d 00 mov 0xd1473(%rip),%rcx # 0xd14ad
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W
Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 73 01 jae 0x9
8: c3 ret
9: 48 8b 0d 73 14 0d 00 mov 0xd1473(%rip),%rcx # 0xd1483
10: f7 d8 neg %eax
12: 64 89 01 mov %eax,%fs:(%rcx)
15: 48 rex.W
[ 5.631219] RSP: 002b:00007fff5a3fea18 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 5.631223] RAX: ffffffffffffffda RBX: 000055883cfb16b0 RCX: 00007f67d4d2488d
[ 5.631226] RDX: 0000000000000004 RSI: 00007f67d52ea376 RDI: 0000000000000019
[ 5.631229] RBP: 00007f67d52ea376 R08: 0000000000000001 R09: fffffffffffffe88
[ 5.631231] R10: 0000000000000050 R11: 0000000000000246 R12: 0000000000020000
[ 5.631233] R13: 000055883cfaf050 R14: 0000000000000000 R15: 000055883cfb6ea0
[ 5.631238] </TASK>
[ 5.631239] Modules linked in: sha512_ssse3 uvcvideo btusb(+) sha1_ssse3 ptp aesni_intel btrtl pps_core videobuf2_vmalloc ax88796b snd_hda_codec_hdmi asix(+) videobuf2_memops btintel mac80211 crypto_simd phylink cryptd uvc btbcm videobuf2_v4l2 selftests spi_nor mtd btmtk rapl videodev asus_nb_wmi libarc4 usbnet joydev vfat ak8975 iTCO_wdt asus_wmi mii iwlwifi videobuf2_common hid_multitouch intel_cstate at24 intel_pmc_bxt spi_intel_platform snd_hda_codec_realtek sparse_keymap fat mousedev hid_generic mei_pxp iTCO_vendor_support intel_rapl_msr spi_intel i915 mei_hdcp platform_profile libphy mc bluetooth intel_uncore snd_hda_codec_generic i2c_i801 snd_hda_scodec_component psmouse crc16 cfg80211 pcspkr processor_thermal_device_pci_legacy snd_hda_intel usbhid ecdh_generic i2c_smbus snd_intel_dspcfg intel_soc_dts_iosf processor_thermal_device lpc_ich rfkill i2c_algo_bit drm_buddy inv_mpu6050_i2c snd_hda_codec mei_me i2c_mux processor_thermal_wt_hint ttm snd_hwdep processor_thermal_rfim mei processor_thermal_rapl intel_gtt
[ 5.631322] inv_mpu6050 video snd_hda_core drm_display_helper intel_rapl_common snd_pcm inv_sensors_timestamp snd_timer processor_thermal_wt_req drm_kms_helper dell_smo8800 acpi_als int3400_thermal pinctrl_lynxpoint industrialio_triggered_buffer processor_thermal_power_floor snd processor_thermal_mbox cec wmi kfifo_buf int340x_thermal_zone acpi_thermal_rel industrialio soundcore soc_button_array asus_wireless mac_hid pkcs8_key_parser crypto_user drm fuse dm_mod loop nfnetlink zram ip_tables x_tables serio_raw atkbd libps2 vivaldi_fmap sha256_ssse3 xhci_pci xhci_pci_renesas i8042 serio btrfs blake2b_generic libcrc32c crc32c_generic crc32c_intel xor raid6_pq
[ 5.631374] CR2: 0000000000000000
[ 5.631377] ---[ end trace 0000000000000000 ]---
If there is any additional information I can provide or patches I can
test, I am more than happy to do so.
Cheers,
Nathan
# bad: [11cb68ad52ac78c81e33b806b531f097e68edfa2] Add linux-next specific files for 20240408
# good: [fec50db7033ea478773b159e0e2efb135270e3b7] Linux 6.9-rc3
git bisect start '11cb68ad52ac78c81e33b806b531f097e68edfa2' 'fec50db7033ea478773b159e0e2efb135270e3b7'
# bad: [b5ae7d7ff5458d024c8d8be346e405dd53bfe573] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git
git bisect bad b5ae7d7ff5458d024c8d8be346e405dd53bfe573
# good: [54f6c30962ec3738fa235457efe3a304f180e335] Merge branch 'renesas-clk' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
git bisect good 54f6c30962ec3738fa235457efe3a304f180e335
# bad: [267e31750ae89f845cfe7df8f577b19482d9ef9b] Merge branch 'phy-listing-link_topology-tracking'
git bisect bad 267e31750ae89f845cfe7df8f577b19482d9ef9b
# good: [992c287d87780abd184c67a303dec3361b7cb408] dt-bindings: net: snps,dwmac: Align 'snps,priority' type definition
git bisect good 992c287d87780abd184c67a303dec3361b7cb408
# good: [0ccf50df61f98a9f98d46524be4baa00c88c499d] Merge tag 'ath-next-20240402' of git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath
git bisect good 0ccf50df61f98a9f98d46524be4baa00c88c499d
# good: [0cd1453b7e55064d06b49eebe34ffb43748ba12e] mlxsw: pci: Remove mlxsw_pci_sdq_count()
git bisect good 0cd1453b7e55064d06b49eebe34ffb43748ba12e
# good: [da48a65f3ff4155364fb9e3efe0bfba58291da6b] bnxt_en: Fix PTP firmware timeout parameter
git bisect good da48a65f3ff4155364fb9e3efe0bfba58291da6b
# good: [ff8877b04ef282b2bdb16c9dccc2e42216a34f62] netlink: specs: ethtool: define header-flags as an enum
git bisect good ff8877b04ef282b2bdb16c9dccc2e42216a34f62
# good: [9f06f87fef689d28588cde8c7ebb00a67da34026] net: skbuff: generalize the skb->decrypted bit
git bisect good 9f06f87fef689d28588cde8c7ebb00a67da34026
# bad: [0ec5ed6c130e3906ba4ec82d740444a21504fbbf] net: sfp: pass the phy_device when disconnecting an sfp module's PHY
git bisect bad 0ec5ed6c130e3906ba4ec82d740444a21504fbbf
# good: [d133ef1ee2a256ba5a589493cd28dccfede6af11] net: phy: marvell: implement cable test for 88E1111
git bisect good d133ef1ee2a256ba5a589493cd28dccfede6af11
# bad: [6916e461e7933d3d003441291c543938f2ccb371] net: phy: Introduce ethernet link topology representation
git bisect bad 6916e461e7933d3d003441291c543938f2ccb371
# first bad commit: [6916e461e7933d3d003441291c543938f2ccb371] net: phy: Introduce ethernet link topology representation
Hello Nathan,
On Tue, 9 Apr 2024 13:15:53 -0700
Nathan Chancellor <[email protected]> wrote:
> Hi Maxime,
>
> On Thu, Apr 04, 2024 at 11:29:51AM +0200, Maxime Chevallier wrote:
[...]
> I bisected a crash that I see on one of my test devices to this change
> in -next as commit 6916e461e793 ("net: phy: Introduce ethernet link
> topology representation"). Here is the stack trace passed through
> scripts/decode_stacktrace.sh:
[...]
> [ 5.626535] ? phy_link_topo_add_phy (drivers/net/phy/phy_link_topology.c:46) libphy
> [ 5.627954] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:115 (discriminator 4) include/linux/atomic/atomic-arch-fallback.h:2170 (discriminator 4) include/linux/atomic/atomic-instrumented.h:1302 (discriminator 4) include/asm-generic/qspinlock.h:111 (discriminator 4) include/linux/spinlock.h:187 (discriminator 4) include/linux/spinlock_api_smp.h:134 (discriminator 4) kernel/locking/spinlock.c:154 (discriminator 4))
> [ 5.627963] phy_link_topo_add_phy (include/linux/xarray.h:977 drivers/net/phy/phy_link_topology.c:80) libphy
> [ 5.629462] phy_attach_direct (drivers/net/phy/phy_device.c:1516) libphy
> [ 5.629504] phylink_connect_phy (drivers/net/phy/phylink.c:1983) phylink
> [ 5.631030] ax88772_bind (drivers/net/usb/asix_devices.c:710 drivers/net/usb/asix_devices.c:919) asix
> [ 5.631049] usbnet_probe (drivers/net/usb/usbnet.c:1745) usbnet
I've run some tests on an arm64 board using an USB to Ethernet adapter
that uses the same driver (It also goes through ax88772_bind() ) but I
don't reproduce the error.
> If there is any additional information I can provide or patches I can
> test, I am more than happy to do so.
The next step for me would be to try on an x86_64 box to get closer to
the config you used, however could you give me the .config that was used
when the bug was triggered ? I'd like to make sure I didn't miss
anything related to some of the parts being build as modules for
example.
Thanks a lot,
Maxime
> Cheers,
> Nathan
On Wed, Apr 10, 2024 at 10:16:27AM +0200, Maxime Chevallier wrote:
> On Tue, 9 Apr 2024 13:15:53 -0700
> Nathan Chancellor <[email protected]> wrote:
> > On Thu, Apr 04, 2024 at 11:29:51AM +0200, Maxime Chevallier wrote:
>
> [...]
>
> > I bisected a crash that I see on one of my test devices to this change
> > in -next as commit 6916e461e793 ("net: phy: Introduce ethernet link
> > topology representation"). Here is the stack trace passed through
> > scripts/decode_stacktrace.sh:
>
> [...]
>
> > [ 5.626535] ? phy_link_topo_add_phy (drivers/net/phy/phy_link_topology.c:46) libphy
> > [ 5.627954] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:115 (discriminator 4) include/linux/atomic/atomic-arch-fallback.h:2170 (discriminator 4) include/linux/atomic/atomic-instrumented.h:1302 (discriminator 4) include/asm-generic/qspinlock.h:111 (discriminator 4) include/linux/spinlock.h:187 (discriminator 4) include/linux/spinlock_api_smp.h:134 (discriminator 4) kernel/locking/spinlock.c:154 (discriminator 4))
> > [ 5.627963] phy_link_topo_add_phy (include/linux/xarray.h:977 drivers/net/phy/phy_link_topology.c:80) libphy
> > [ 5.629462] phy_attach_direct (drivers/net/phy/phy_device.c:1516) libphy
> > [ 5.629504] phylink_connect_phy (drivers/net/phy/phylink.c:1983) phylink
> > [ 5.631030] ax88772_bind (drivers/net/usb/asix_devices.c:710 drivers/net/usb/asix_devices.c:919) asix
> > [ 5.631049] usbnet_probe (drivers/net/usb/usbnet.c:1745) usbnet
>
> I've run some tests on an arm64 board using an USB to Ethernet adapter
> that uses the same driver (It also goes through ax88772_bind() ) but I
> don't reproduce the error.
>
> > If there is any additional information I can provide or patches I can
> > test, I am more than happy to do so.
>
> The next step for me would be to try on an x86_64 box to get closer to
> the config you used, however could you give me the .config that was used
> when the bug was triggered ? I'd like to make sure I didn't miss
> anything related to some of the parts being build as modules for
> example.
Sure thing! It should be the one I have attached, it is basically just
Arch Linux's configuration:
https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config
Cheers,
Nathan
Hi,
On Wed, 10 Apr 2024 07:53:58 -0700
Nathan Chancellor <[email protected]> wrote:
> On Wed, Apr 10, 2024 at 10:16:27AM +0200, Maxime Chevallier wrote:
> > On Tue, 9 Apr 2024 13:15:53 -0700
> > Nathan Chancellor <[email protected]> wrote:
> > > On Thu, Apr 04, 2024 at 11:29:51AM +0200, Maxime Chevallier wrote:
> >
> > [...]
> >
> > > I bisected a crash that I see on one of my test devices to this change
> > > in -next as commit 6916e461e793 ("net: phy: Introduce ethernet link
> > > topology representation"). Here is the stack trace passed through
> > > scripts/decode_stacktrace.sh:
> >
> > [...]
> >
> > > [ 5.626535] ? phy_link_topo_add_phy (drivers/net/phy/phy_link_topology.c:46) libphy
> > > [ 5.627954] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:115 (discriminator 4) include/linux/atomic/atomic-arch-fallback.h:2170 (discriminator 4) include/linux/atomic/atomic-instrumented.h:1302 (discriminator 4) include/asm-generic/qspinlock.h:111 (discriminator 4) include/linux/spinlock.h:187 (discriminator 4) include/linux/spinlock_api_smp.h:134 (discriminator 4) kernel/locking/spinlock.c:154 (discriminator 4))
> > > [ 5.627963] phy_link_topo_add_phy (include/linux/xarray.h:977 drivers/net/phy/phy_link_topology.c:80) libphy
> > > [ 5.629462] phy_attach_direct (drivers/net/phy/phy_device.c:1516) libphy
> > > [ 5.629504] phylink_connect_phy (drivers/net/phy/phylink.c:1983) phylink
> > > [ 5.631030] ax88772_bind (drivers/net/usb/asix_devices.c:710 drivers/net/usb/asix_devices.c:919) asix
> > > [ 5.631049] usbnet_probe (drivers/net/usb/usbnet.c:1745) usbnet
> >
> > I've run some tests on an arm64 board using an USB to Ethernet adapter
> > that uses the same driver (It also goes through ax88772_bind() ) but I
> > don't reproduce the error.
> >
> > > If there is any additional information I can provide or patches I can
> > > test, I am more than happy to do so.
> >
> > The next step for me would be to try on an x86_64 box to get closer to
> > the config you used, however could you give me the .config that was used
> > when the bug was triggered ? I'd like to make sure I didn't miss
> > anything related to some of the parts being build as modules for
> > example.
>
> Sure thing! It should be the one I have attached, it is basically just
> Arch Linux's configuration:
>
> https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/raw/main/config
Thanks a lot. Let me try with that config and see if I can reproduce :)
Maxime
>
> Cheers,
> Nathan