net: ethtool: make dev->hwprov ops-protected

dev->hwprov tracks the active hwtstamp provider for the device.
Make it ops protected (instance lock if the netdev driver opts
into holding instance lock around callbacks, otherwise rtnl_lock).

hwprov is written and read in:
 - drivers/net/phy/phy_device.c
    phydev and ops protection don't currently mix, add a comment
 - net/ethtool/
    as of now holds both rtnl lock and ops lock, this one will
    soon only hold one lock or the other

read in:
 - net/core/dev_ioctl.c
    holds both rtnl lock and ops lock
 - net/core/timestamping.c
    RCU reader

The new netdev_ops_lock_dereference() helper does not have
"compat" in the name. The name would be quite long and I think
in this case it should be obvious that we need _a_ lock.
netdev_lock_dereference() already exists and means dev->lock
is always expected.

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-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2026-06-04 17:29:03 -07:00
parent ded86da4bb
commit 97f51bf91b
6 changed files with 28 additions and 7 deletions

View File

@ -1935,6 +1935,9 @@ void phy_detach(struct phy_device *phydev)
if (dev) {
struct hwtstamp_provider *hwprov;
/* hwprov may technically be protected by ops lock but
* not for devices with a phydev, see phy_link_topo_add_phy()
*/
hwprov = rtnl_dereference(dev->hwprov);
/* Disable timestamp if it is the one selected */
if (hwprov && hwprov->phydev == phydev) {

View File

@ -38,7 +38,9 @@ int phy_link_topo_add_phy(struct net_device *dev,
/* 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().
* the two, flag if someone tries. See also:
* - ethnl_req_get_phydev()
* - phy_detach()
*/
if (WARN_ON_ONCE(netdev_need_ops_lock(dev)))
return -EOPNOTSUPP;

View File

@ -2583,6 +2583,9 @@ struct net_device {
* Double protects:
* @up, @moving_ns, @nd_net, @xdp_features
*
* Ops protects:
* @hwprov
*
* Double ops protects:
* @real_num_rx_queues, @real_num_tx_queues
*

View File

@ -102,6 +102,14 @@ static inline void netdev_unlock_ops_compat(struct net_device *dev)
rtnl_unlock();
}
/* Matching "ops protected" category from netdevice.h */
static inline int netdev_is_locked_ops_compat(const struct net_device *dev)
{
if (netdev_need_ops_lock(dev))
return lockdep_is_held(&dev->lock);
return lockdep_rtnl_is_held();
}
static inline int netdev_lock_cmp_fn(const struct lockdep_map *a,
const struct lockdep_map *b)
{
@ -138,6 +146,9 @@ static inline int netdev_lock_cmp_fn(const struct lockdep_map *a,
#define netdev_lock_dereference(p, dev) \
rcu_dereference_protected(p, lockdep_is_held(&(dev)->lock))
#define netdev_ops_lock_dereference(p, dev) \
rcu_dereference_protected(p, netdev_is_locked_ops_compat(dev))
int netdev_debug_event(struct notifier_block *nb, unsigned long event,
void *ptr);

View File

@ -260,7 +260,7 @@ int dev_get_hwtstamp_phylib(struct net_device *dev,
{
struct hwtstamp_provider *hwprov;
hwprov = rtnl_dereference(dev->hwprov);
hwprov = netdev_ops_lock_dereference(dev->hwprov, dev);
if (hwprov) {
cfg->qualifier = hwprov->desc.qualifier;
if (hwprov->source == HWTSTAMP_SOURCE_PHYLIB &&
@ -337,7 +337,7 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
bool phy_ts;
int err;
hwprov = rtnl_dereference(dev->hwprov);
hwprov = netdev_ops_lock_dereference(dev->hwprov, dev);
if (hwprov) {
if (hwprov->source == HWTSTAMP_SOURCE_PHYLIB &&
hwprov->phydev) {

View File

@ -2,6 +2,7 @@
#include <linux/net_tstamp.h>
#include <linux/ptp_clock_kernel.h>
#include <net/netdev_lock.h>
#include "bitset.h"
#include "common.h"
@ -57,7 +58,7 @@ static int tsconfig_prepare_data(const struct ethnl_req_info *req_base,
data->hwtst_config.flags = cfg.flags;
data->hwprov_desc.index = -1;
hwprov = rtnl_dereference(dev->hwprov);
hwprov = netdev_ops_lock_dereference(dev->hwprov, dev);
if (hwprov) {
data->hwprov_desc.index = hwprov->desc.index;
data->hwprov_desc.qualifier = hwprov->desc.qualifier;
@ -213,7 +214,7 @@ static int tsconfig_send_reply(struct net_device *dev, struct genl_info *info)
return -ENOMEM;
}
ASSERT_RTNL();
netdev_assert_locked_ops_compat(dev);
reply_data->base.dev = dev;
ret = tsconfig_prepare_data(&req_info->base, &reply_data->base, info);
if (ret < 0)
@ -316,7 +317,7 @@ static int ethnl_set_tsconfig(struct ethnl_req_info *req_base,
struct hwtstamp_provider_desc __hwprov_desc = {.index = -1};
struct hwtstamp_provider *__hwprov;
__hwprov = rtnl_dereference(dev->hwprov);
__hwprov = netdev_ops_lock_dereference(dev->hwprov, dev);
if (__hwprov) {
__hwprov_desc.index = __hwprov->desc.index;
__hwprov_desc.qualifier = __hwprov->desc.qualifier;
@ -414,7 +415,8 @@ static int ethnl_set_tsconfig(struct ethnl_req_info *req_base,
goto err_free_hwprov;
/* Change the selected hwtstamp source */
__hwprov = rcu_replace_pointer_rtnl(dev->hwprov, hwprov);
__hwprov = rcu_replace_pointer(dev->hwprov, hwprov,
netdev_is_locked_ops_compat(dev));
if (__hwprov)
kfree_rcu(__hwprov, rcu_head);
}