net: ethtool: relax ethnl_req_get_phydev() locking assertion

phydev <> netdev linking and lifecycle depends on rtnl_lock.
We want to switch to instance locks for most ethtool ops.
Let's add an assert that ops locked devices don't use phydev
today. If one does we can either opt the phy ops out of
being purely ops locked, or do deeper surgery to make phy
locking ops-compatible. I don't think there's any fundamental
challenge to make that work.

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-3-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
index 1f1eb5d59b382bf9d3fce5aeb9e4fa962c8e9b67..aed3b26c1674f04abcefe87ec3d0cddd982edae0 100644 (file)
--- a/drivers/net/phy/phy_link_topology.c
+++ b/drivers/net/phy/phy_link_topology.c
@@ -10,6 +10,7 @@
 #include <linux/phy.h>
 #include <linux/rtnetlink.h>
 #include <linux/xarray.h>
+#include <net/netdev_lock.h>
 
 static int netdev_alloc_phy_link_topology(struct net_device *dev)
 {
@@ -35,6 +36,13 @@ int phy_link_topo_add_phy(struct net_device *dev,
        struct phy_device_node *pdn;
        int ret;
 
+       /* ethtool ops may run without rtnl_lock, and rtnl_lock is what
+        * currently protects the PHY topology. No driver currently mixes
+        * the two, flag if someone tries. See also ethnl_req_get_phydev().
+        */
+       if (WARN_ON_ONCE(netdev_need_ops_lock(dev)))
+               return -EOPNOTSUPP;
+
        if (!topo) {
                ret = netdev_alloc_phy_link_topology(dev);
                if (ret)

diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
index 68a59e25821c3d79ccd8e7e7204ee5a4464247d8..95575f68d5bc710aaad89f4d46fd1233fb6e6463 100644 (file)
--- a/include/linux/phy_link_topology.h
+++ b/include/linux/phy_link_topology.h
@@ -36,6 +36,11 @@ struct phy_device_node {
        struct phy_device *phy;
 };
 
+static inline bool phy_link_topo_empty(struct net_device *dev)
+{
+       return !dev->link_topo;
+}
+
 #if IS_ENABLED(CONFIG_PHYLIB)
 int phy_link_topo_add_phy(struct net_device *dev,
                          struct phy_device *phy,

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index c4054a9795ffbbd39d3e8fbcdf9211b7d6265d51..afafed738584121c9d2bdf398622645553c1d23c 100644 (file)
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -226,11 +226,13 @@ struct phy_device *ethnl_req_get_phydev(const struct ethnl_req_info *req_info,
 {
        struct phy_device *phydev;
 
-       ASSERT_RTNL();
-
        if (!req_info->dev)
                return NULL;
 
+       /* If there is no PHY in sight there's no need for assert locking */
+       if (!phy_link_topo_empty(req_info->dev))
+               ASSERT_RTNL();
+
        if (!req_info->phy_index)
                return req_info->dev->phydev;
 

diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index f94aaa66379cd28ae049535fb59753efea4e364f..4ca2eca2e94b1458720a715953690f1bbfbfd342 100644 (file)
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -275,14 +275,15 @@ static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info)
 
 /**
  * ethnl_req_get_phydev() - Gets the phy_device targeted by this request,
- *                         if any. Must be called under rntl_lock().
+ *                         if any.
  * @req_info:  The ethnl request to get the phy from.
  * @tb:                The netlink attributes array, for error reporting.
  * @header:    The netlink header index, used for error reporting.
  * @extack:    The netlink extended ACK, for error reporting.
  *
- * The caller must hold RTNL, until it's done interacting with the returned
- * phy_device.
+ * If a phy_device is returned the caller must hold rtnl_lock when calling
+ * this function, and until it's done interacting with the returned phy_device.
+ * IOW caller must hold rtnl_lock unless they know netdev has no phy_device.
  *
  * Return: A phy_device pointer corresponding either to the passed phy_index
  *        if one is provided. If not, the phy_device attached to the

diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index ddc6eab701ed65ec87fd7a286c45c3431fd46e4d..018b0412be862afaa93209bc5721c7d0b6499c26 100644 (file)
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -78,7 +78,6 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
        struct phy_device *phydev;
        int ret;
 
-       /* RTNL is held by the caller */
        phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PHY_HEADER,
                                      info->extack);
        if (IS_ERR_OR_NULL(phydev))