RE: [EXTERNAL] Re: [PATCH net] net: mana: do not discard an ethtool-configured RSS table on a queue rebuild

On Fri,  4 Sep 2026 17:44:01 -0700 Long Li wrote:

quoted

mana_alloc_queues() regenerates the RSS indirection table from the
driver default every time the queues are built, so a table installed
with "ethtool -X" is silently replaced by any operation that rebuilds them:
an MTU change, a ring-size, channel-count or private-flag change, an
XDP attach, TX-timeout reset recovery, or resume.

The driver never clears the core's IFF_RXFH_CONFIGURED, so
netif_is_rxfh_configured() keeps reporting a user table while the
hardware has been reprogrammed with the default one. "ethtool -x" then
shows a table the user did not ask for, with no indication it changed:

  # ethtool -X ens1 equal 2
  # ethtool -x ens1
  RX flow hash indirection table for ens1 with 16 RX ring(s):
      0:      0     1     0     1     0     1     0     1
      8:      0     1     0     1     0     1     0     1
  # ip link set ens1 mtu 1400
  # ethtool -x ens1
  RX flow hash indirection table for ens1 with 16 RX ring(s):
      0:      0     1     2     3     4     5     6     7
      8:      8     9    10    11    12    13    14    15

Keep the table instead, and rebuild it only when it is
driver-generated or cannot be honoured. An entry may not be kept if it
points past the last queue: mana_config_rss() uses these entries to
index apc->rxqs[], which holds apc->num_queues pointers. That is
reachable because
mana_attach() calls mana_init_port(), which lowers apc->num_queues to
the maximum the device reports, so a table configured for more queues
can outlive them.

To be clear this is only acceptable if the number of queues drops due to re-
negotiation of caps with the device, not for example if XDP requires some
queues to be used for other purposes. In the latter case just refuse the config
change.

quoted

A table that cannot be kept is still replaced by the default silently,
without ethtool_rxfh_indir_lost(). That helper sends
ETHTOOL_MSG_RSS_NTF, which requires the netdev instance lock, and
mana_alloc_queues() runs both with that lock held, from ndo_open, and
without it, from mana_attach() on the reset and resume paths. Leaving
the core's view untouched is what the driver did for every table
before this change.

Okay, so you have a problem of not having the lock...

quoted

Opt the RSS ethtool operations into rtnl_lock() while here. Reading
the table in mana_alloc_queues() has to be serialized against
mana_set_rxfh() replacing it, and the two had no lock in common:
mana_set_rxfh() ran under the netdev instance lock alone, while
mana_alloc_queues() reaches this point holding only RTNL, from
ndo_open and from mana_attach() on the reset and resume paths. Taking
the instance lock there instead is not possible, since ndo_open
already runs with it held. The same flag covers the netlink and ioctl entry

points.

.. and yet your fix is not to try to take it but the reverse, to add a different
lock? You need to explain why reset path can't take the instance lock. Of
course you can't take it _inside_ ndo_open, but the caller should be able to.

Okay, will send a patch implementing the locks.