> On Fri, 4 Sep 2026 17:44:01 -0700 Long Li wrote: > > 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. > > > 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... > > > 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.
