On 6/8/2026 3:31 PM, Jacob Keller wrote:

On 6/4/2026 5:29 PM, Jakub Kicinski wrote:

quoted

With the ethtool_get_link_ksettings() situation hopefully ironed out
the previous series (commit 6a5d837f0ce2) let's return to the main
part of the series.

We have been slowly moving towards removing the rtnl_lock dependency
in driver ops since the concept of "ops-locked" drivers have been
introduced last year. Since last year will take the netdev instance
lock before invoking any ndo or ethtool op of "ops-locked" drivers.

We dipped our toes into rtnl_lock-less ops with the queue binding API.
Queue stats, NAPI, and other netdev-netlink objects are also queried
without holding rtnl_lock already. It's time to take the next logical
step and lift the requirement from ethtool ops.

The direct motivation for this patchset is that ethtool ops often
involve communicating with device FW, and may take a long time
to complete. Aggressive polling of device state on machines
with 10+ NICs have been shown to significantly increase rtnl_lock
pressure.

There's a handful of areas which still need rtnl_lock (see below).
I decided to convert everything to rtnl_lock-less by default, and
add a set of flags which let the drivers request rtnl_lock to still
be taken. I don't love this, but I'm worried that opt-in would be
even more confusing.

Agreed. It might be nicer to opt-out first, and then opt-in the drivers
that don't do the update_features.. That would make it easier to prevent
buggy drivers slipping in as easily... But that would result in the
following messy situation:

ethtool ops for ops-locked drivers don't hold RTNL lock, *except* some
which do because they might call update_features.. *except* those which
opt-out of RTNL because they know their driver implementation doesn't
call update_features...

Yea. I think that is too messy. This approach requires slightly more
vigilance on the part of reviewers to ensure that ops-locked drivers
don't make a mistake here. However, the checks to see if features
changed while the RTNL wasn't held should be enough to help catch most
cases. Ok.

quoted

Known issues / exclusions:
 - qdiscs - qdisc configuration currently assumes rtnl_lock, this
   is mostly impacting set_channels callback. qdisc config is probably
   the easiest one of the exclusions to tackle, it's fairly self-contained.
 - features - even tho feature changes are (correctly) plumbed to
   the driver thru ndos they are part of ethtool uAPI. ethtool itself
   calls netdev_features_change() if it has spotted device feature change
   before vs after to the callback. Some drivers also call
   netdev_features_change() directly in response to various changes,
   e.g. setting priv flags.
   Since features have to propagate to upper and lower devices anything
   that touches features is quite hard to move from under rtnl_lock.
 - phylink - phylink and SFP depend on rtnl_lock today, I suspect
   that this is purely for historic reasons. I started poking at
   it and don't really see a need for a global lock. But accessing
   the netdev instance lock from the SFP entry points will require
   some attention from the phylink folks.
 - phydev - similar to phylink, looks quite doable. But no ops-locked
   driver currently has a phydev (fbnic only uses phylink) so phydev
   related paths retain a ASSERT_RTNL() for now.

Makes sense. Taking some steps towards the end goal is better than
trying to wait until every piece here is done.

Whole series looks good to me. I had one minor nit about one patch
adding a bit flag at BIT(1) and then later changing that to BIT(5) but
its ultimately harmless and should not force a respin of this important
work.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

quoted

Tested on mlx5, bnxt and fbnic.

I think currently only iAVF is ops-locked for Intel. Tony, would it make
sense to see if our virtualization validation folks have cycles to give
this a quick pass on iAVF? (Regardless of whether it merges before or
after, it would be beneficial to make sure we don't have any lingering
issues there).

Just to clarify, I'm not suggesting a delay on merging and am 100% fine
with merging this before such testing completes. I just want to make
sure we (Intel) do double check the iavf code so that development team
can try to fix any lurking bugs within the kernel cycle before they
become wide spread.