Skip to content

Commit ded86da

Browse files
committed
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>
1 parent 8845484 commit ded86da

5 files changed

Lines changed: 21 additions & 6 deletions

File tree

drivers/net/phy/phy_link_topology.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <linux/phy.h>
1111
#include <linux/rtnetlink.h>
1212
#include <linux/xarray.h>
13+
#include <net/netdev_lock.h>
1314

1415
static int netdev_alloc_phy_link_topology(struct net_device *dev)
1516
{
@@ -35,6 +36,13 @@ int phy_link_topo_add_phy(struct net_device *dev,
3536
struct phy_device_node *pdn;
3637
int ret;
3738

39+
/* ethtool ops may run without rtnl_lock, and rtnl_lock is what
40+
* currently protects the PHY topology. No driver currently mixes
41+
* the two, flag if someone tries. See also ethnl_req_get_phydev().
42+
*/
43+
if (WARN_ON_ONCE(netdev_need_ops_lock(dev)))
44+
return -EOPNOTSUPP;
45+
3846
if (!topo) {
3947
ret = netdev_alloc_phy_link_topology(dev);
4048
if (ret)

include/linux/phy_link_topology.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ struct phy_device_node {
3636
struct phy_device *phy;
3737
};
3838

39+
static inline bool phy_link_topo_empty(struct net_device *dev)
40+
{
41+
return !dev->link_topo;
42+
}
43+
3944
#if IS_ENABLED(CONFIG_PHYLIB)
4045
int phy_link_topo_add_phy(struct net_device *dev,
4146
struct phy_device *phy,

net/ethtool/netlink.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,13 @@ struct phy_device *ethnl_req_get_phydev(const struct ethnl_req_info *req_info,
226226
{
227227
struct phy_device *phydev;
228228

229-
ASSERT_RTNL();
230-
231229
if (!req_info->dev)
232230
return NULL;
233231

232+
/* If there is no PHY in sight there's no need for assert locking */
233+
if (!phy_link_topo_empty(req_info->dev))
234+
ASSERT_RTNL();
235+
234236
if (!req_info->phy_index)
235237
return req_info->dev->phydev;
236238

net/ethtool/netlink.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,15 @@ static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info)
275275

276276
/**
277277
* ethnl_req_get_phydev() - Gets the phy_device targeted by this request,
278-
* if any. Must be called under rntl_lock().
278+
* if any.
279279
* @req_info: The ethnl request to get the phy from.
280280
* @tb: The netlink attributes array, for error reporting.
281281
* @header: The netlink header index, used for error reporting.
282282
* @extack: The netlink extended ACK, for error reporting.
283283
*
284-
* The caller must hold RTNL, until it's done interacting with the returned
285-
* phy_device.
284+
* If a phy_device is returned the caller must hold rtnl_lock when calling
285+
* this function, and until it's done interacting with the returned phy_device.
286+
* IOW caller must hold rtnl_lock unless they know netdev has no phy_device.
286287
*
287288
* Return: A phy_device pointer corresponding either to the passed phy_index
288289
* if one is provided. If not, the phy_device attached to the

net/ethtool/phy.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
7878
struct phy_device *phydev;
7979
int ret;
8080

81-
/* RTNL is held by the caller */
8281
phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PHY_HEADER,
8382
info->extack);
8483
if (IS_ERR_OR_NULL(phydev))

0 commit comments

Comments
 (0)