RE: [EXTERNAL] Re: [PATCH net] net: mana: do not discard an ethtool-configured RSS table on a queue rebuild
From: Long Li
Date: Thu Sep 10 2026 - 16:02:28 EST
> 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.