tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

net: dsa: remove redundant netdev_lock_ops() from conduit ethtool ops

DSA replaces the conduit (master) device's ethtool_ops with its own
wrappers that aggregate stats from both the conduit and DSA switch
ports. Taking the lock again inside the DSA wrappers causes a deadlock.

Stumbled upon this when booting qemu with fbnic and CONFIG_NET_DSA_LOOP=y
(which looks like some kind of testing device that auto-populates the ports
of eth0). `ethtool -i` is enough to deadlock. This means we have basically zero
coverage for DSA stuff with real ops locked devs.

Remove the redundant netdev_lock_ops()/netdev_unlock_ops() calls from
the DSA conduit ethtool wrappers.

Fixes: 2bcf4772e45a ("net: ethtool: try to protect all callback with netdev instance lock")
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Link: https://patch.msgid.link/20260414231035.1917035-1-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Stanislav Fomichev and committed by
Jakub Kicinski 0f99e0c3 105425b1

+1 -15
+1 -15
net/dsa/conduit.c
··· 27 27 int len; 28 28 29 29 if (ops && ops->get_regs_len) { 30 - netdev_lock_ops(dev); 31 30 len = ops->get_regs_len(dev); 32 - netdev_unlock_ops(dev); 33 31 if (len < 0) 34 32 return len; 35 33 ret += len; ··· 58 60 int len; 59 61 60 62 if (ops && ops->get_regs_len && ops->get_regs) { 61 - netdev_lock_ops(dev); 62 63 len = ops->get_regs_len(dev); 63 - if (len < 0) { 64 - netdev_unlock_ops(dev); 64 + if (len < 0) 65 65 return; 66 - } 67 66 regs->len = len; 68 67 ops->get_regs(dev, regs, data); 69 - netdev_unlock_ops(dev); 70 68 data += regs->len; 71 69 } 72 70 ··· 109 115 int count, mcount = 0; 110 116 111 117 if (ops && ops->get_sset_count && ops->get_ethtool_stats) { 112 - netdev_lock_ops(dev); 113 118 mcount = ops->get_sset_count(dev, ETH_SS_STATS); 114 119 ops->get_ethtool_stats(dev, stats, data); 115 - netdev_unlock_ops(dev); 116 120 } 117 121 118 122 list_for_each_entry(dp, &dst->ports, list) { ··· 141 149 if (count >= 0) 142 150 phy_ethtool_get_stats(dev->phydev, stats, data); 143 151 } else if (ops && ops->get_sset_count && ops->get_ethtool_phy_stats) { 144 - netdev_lock_ops(dev); 145 152 count = ops->get_sset_count(dev, ETH_SS_PHY_STATS); 146 153 ops->get_ethtool_phy_stats(dev, stats, data); 147 - netdev_unlock_ops(dev); 148 154 } 149 155 150 156 if (count < 0) ··· 166 176 struct dsa_switch_tree *dst = cpu_dp->dst; 167 177 int count = 0; 168 178 169 - netdev_lock_ops(dev); 170 179 if (sset == ETH_SS_PHY_STATS && dev->phydev && 171 180 (!ops || !ops->get_ethtool_phy_stats)) 172 181 count = phy_ethtool_get_sset_count(dev->phydev); 173 182 else if (ops && ops->get_sset_count) 174 183 count = ops->get_sset_count(dev, sset); 175 - netdev_unlock_ops(dev); 176 184 177 185 if (count < 0) 178 186 count = 0; ··· 227 239 struct dsa_switch_tree *dst = cpu_dp->dst; 228 240 int count, mcount = 0; 229 241 230 - netdev_lock_ops(dev); 231 242 if (stringset == ETH_SS_PHY_STATS && dev->phydev && 232 243 !ops->get_ethtool_phy_stats) { 233 244 mcount = phy_ethtool_get_sset_count(dev->phydev); ··· 240 253 mcount = 0; 241 254 ops->get_strings(dev, stringset, data); 242 255 } 243 - netdev_unlock_ops(dev); 244 256 245 257 list_for_each_entry(dp, &dst->ports, list) { 246 258 if (!dsa_port_is_dsa(dp) && !dsa_port_is_cpu(dp))