Re: [PATCH net v2] net: fix self-deadlock when an upper disables a feature of its lower

[Wang Zhan <wang.zhan@smartx.com>]

Hi,

On Tue, 27 Oct 2026 16:00:49 +0200, Eva Kurchatova wrote:
> There is no way to tell whether the current task owns the lower instance
> lock. Try to take it, and otherwise leave the job to netdev_work, which
> disables the features the uppers lack under the lower's instance lock.

We may not need to tell whether the current task owns the lower instance
lock. Instead, we can check whether the device is already in the current
notification chain. netdev_features_change() is always called with RTNL
held, so we can track it with a global feature-change stack, like this:

  /* RTNL serialized */
  static stack netdev_feature_change_stack;

  netdev_features_change(dev):
          push(netdev_feature_change_stack, dev)
          call_netdevice_notifiers(NETDEV_FEAT_CHANGE, dev)
          pop(netdev_feature_change_stack)

  netdev_sync_lower_features(upper, lower, ...):
          take_ops_lock = !in_stack(netdev_feature_change_stack, lower)

          if (take_ops_lock)
                  netdev_lock_ops(lower)

          ...

          if (take_ops_lock)
                  netdev_unlock_ops(lower)

> In the deadlock case above, the lock holder is the context that sent
> NETDEV_FEAT_CHANGE, so steal the pending work at the end of
> netdev_features_change() and run it inline, like netif_rx_mode_sync()
> does for rx_mode.

The "otherwise" path takes only the instance lock for ops-locked devices:

  netdev_work_proc()
    netdev_lock_ops_compat(dev)

but then reaches:

  netif_sync_upper_disables()
    __netdev_update_features()
      ASSERT_RTNL()

ndo_*_features callbacks also require RTNL, so the deferred path cannot
run feature synchronization.