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

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

[Long Li]

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.

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.

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.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 37 ++++++++++++++++++-
 .../ethernet/microsoft/mana/mana_ethtool.c    |  3 +-
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 7a1ac853e3abcd28c4a1e5c6987ec631a18ad840..97386e17b9421aedefa25bab6fbf0b7b1f2996d4 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -3306,6 +3306,28 @@ static void mana_rss_table_init(struct mana_port_context *apc)
 			ethtool_rxfh_indir_default(i, apc->num_queues);
 }
 
+/* Whether the current indirection table can be kept for apc->num_queues,
+ * rather than rebuilt from the driver default.
+ *
+ * Only a user table ("ethtool -X") is worth keeping; a driver-generated one
+ * is rebuilt so that it spreads over every queue. An entry pointing past the
+ * last queue cannot be kept: mana_config_rss() uses these entries to index
+ * apc->rxqs[], which holds apc->num_queues pointers.
+ */
+static bool mana_rss_table_keep(struct mana_port_context *apc)
+{
+	u32 i;
+
+	if (!netif_is_rxfh_configured(apc->ndev))
+		return false;
+
+	for (i = 0; i < apc->indir_table_sz; i++)
+		if (apc->indir_table[i] >= apc->num_queues)
+			return false;
+
+	return true;
+}
+
 int mana_disable_vport_rx(struct mana_port_context *apc)
 {
 	return mana_cfg_vport_steering(apc, TRI_STATE_FALSE, false, false,
@@ -3621,7 +3643,20 @@ int mana_alloc_queues(struct net_device *ndev)
 		goto destroy_rxq;
 	}
 
-	mana_rss_table_init(apc);
+	/* Keep a user-configured table across the rebuild: its entries are
+	 * queue indices and stay meaningful while they are all still in range.
+	 * Only a driver-generated table is regenerated here.
+	 *
+	 * A table that cannot be kept is replaced by the default without
+	 * telling the core, which keeps reporting the table as user
+	 * configured. That is what this function did for every table before,
+	 * and reporting it here is not an option: ethtool_rxfh_indir_lost()
+	 * sends ETHTOOL_MSG_RSS_NTF, which requires the netdev instance lock,
+	 * and this runs both with that lock held (ndo_open) and without it
+	 * (mana_attach() from the reset and resume paths).
+	 */
+	if (!mana_rss_table_keep(apc))
+		mana_rss_table_init(apc);
 
 	err = mana_config_rss(apc, TRI_STATE_TRUE, true, true);
 	if (err) {
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index ece7ff9cc409a806b6a6de70a85b44874bfa6dad..e3acaf17efabcd62f7cffabdb8404cf16b917272 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -871,7 +871,8 @@ const struct ethtool_ops mana_ethtool_ops = {
 	.op_needs_rtnl		= ETHTOOL_OP_NEEDS_RTNL_SCHANNELS |
 				  ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM |
 				  ETHTOOL_OP_NEEDS_RTNL_SPFLAGS |
-				  ETHTOOL_OP_NEEDS_RTNL_GLINK,
+				  ETHTOOL_OP_NEEDS_RTNL_GLINK |
+				  ETHTOOL_OP_NEEDS_RTNL_RSS,
 	.get_ethtool_stats	= mana_get_ethtool_stats,
 	.get_sset_count		= mana_get_sset_count,
 	.get_strings		= mana_get_strings,
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] State desynchronization causes newly added queues to be left unutilized after a sequence of channel count changes.

Pre-existing issues:
- [Critical] Pre-existing Use-After-Free in stats collection due to ad-hoc synchronization flag.
--

commit dc8331f7b2434de9035275921f9768699946cdcb
Author: Long Li <longli@microsoft.com>

net: mana: do not discard an ethtool-configured RSS table on a queue rebuild

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.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 7a1ac853e3abc..97386e17b9421 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -3306,6 +3306,28 @@ static void mana_rss_table_init(struct mana_port_context *apc)
>  			ethtool_rxfh_indir_default(i, apc->num_queues);
>  }
>  
> +/* Whether the current indirection table can be kept for apc->num_queues,
> + * rather than rebuilt from the driver default.
> + *
> + * Only a user table ("ethtool -X") is worth keeping; a driver-generated one
> + * is rebuilt so that it spreads over every queue. An entry pointing past the
> + * last queue cannot be kept: mana_config_rss() uses these entries to index
> + * apc->rxqs[], which holds apc->num_queues pointers.
> + */
> +static bool mana_rss_table_keep(struct mana_port_context *apc)
> +{
> +	u32 i;
> +
> +	if (!netif_is_rxfh_configured(apc->ndev))
> +		return false;
> +
> +	for (i = 0; i < apc->indir_table_sz; i++)
> +		if (apc->indir_table[i] >= apc->num_queues)
> +			return false;
> +
> +	return true;
> +}

[Severity: High]
Does this check wrongly trust netif_is_rxfh_configured() without validating
if the table was previously replaced by a driver fallback?

If a user sets a custom RSS table via ethtool -X and then reduces the channel
count, mana_alloc_queues() detects the user table indices are out of bounds
and generates a narrow driver-default table, but intentionally avoids
notifying the core via ethtool_rxfh_indir_lost(). The core's
netif_is_rxfh_configured() remains true.

When the user subsequently increases the channel count, could this function
incorrectly believe the narrow driver-generated fallback table is still a
user-configured table? Since its bounds fit the new queue count, it seems
this might permanently freeze the table at the smaller size.

> +
>  int mana_disable_vport_rx(struct mana_port_context *apc)
>  {
>  	return mana_cfg_vport_steering(apc, TRI_STATE_FALSE, false, false,
> @@ -3621,7 +3643,20 @@ int mana_alloc_queues(struct net_device *ndev)
>  		goto destroy_rxq;
>  	}
>  
> -	mana_rss_table_init(apc);
> +	/* Keep a user-configured table across the rebuild: its entries are
> +	 * queue indices and stay meaningful while they are all still in range.
> +	 * Only a driver-generated table is regenerated here.
> +	 *
> +	 * A table that cannot be kept is replaced by the default without
> +	 * telling the core, which keeps reporting the table as user
> +	 * configured. That is what this function did for every table before,
> +	 * and reporting it here is not an option: ethtool_rxfh_indir_lost()
> +	 * sends ETHTOOL_MSG_RSS_NTF, which requires the netdev instance lock,
> +	 * and this runs both with that lock held (ndo_open) and without it
> +	 * (mana_attach() from the reset and resume paths).
> +	 */
> +	if (!mana_rss_table_keep(apc))
> +		mana_rss_table_init(apc);

[Severity: High]
If mana_rss_table_keep() incorrectly returns true due to the
desynchronization described above, does it skip the table regeneration and
leave newly added queues unutilized?

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but is there a Use-After-Free in the stats
collection due to the ad-hoc port_is_up synchronization flag?

Looking at mana_get_stats64(), the reader path relies on the ad-hoc
port_is_up flag and dereferences the queue structure, but can be preempted
before accessing it:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_get_stats64() {
	if (!apc->port_is_up)
		return;
...
	for (q = 0; q < num_queues; q++) {
		rx_stats = &apc->rxqs[q]->stats;
...
}

Concurrently, a teardown path clears the flag and proceeds to free the
underlying memory structures without waiting for concurrent readers:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_detach() {
	apc->port_st_save = apc->port_is_up;
	apc->port_is_up = false;

	/* Ensure port state updated before txq state */
	smp_wmb();

	netif_tx_disable(ndev);

	if (apc->port_st_save) {
		err = mana_dealloc_queues(ndev);
...
}

The memory is then synchronously freed, guaranteeing a Use-After-Free if a
reader is mid-execution:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_destroy_txq() {
...
		mana_deinit_txq(apc, &apc->tx_qp[i]->txq);

		kvfree(apc->tx_qp[i]);
...
}

Could a preempted reader accessing the freed memory lead to memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905004401.3937066-1-longli@microsoft.com?part=1

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

[Long Li]

> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Saturday, September 5, 2026 5:45 PM
> To: Long Li <longli@microsoft.com>
> Cc: linux-hyperv@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH net] net: mana: do not discard an ethtool-
> configured RSS table on a queue rebuild
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> 
> New issues:
> - [High] State desynchronization causes newly added queues to be left
> unutilized after a sequence of channel count changes.
> 
> Pre-existing issues:
> - [Critical] Pre-existing Use-After-Free in stats collection due to ad-hoc
> synchronization flag.
> --
> 
> commit dc8331f7b2434de9035275921f9768699946cdcb
> Author: Long Li <longli@microsoft.com>
> 
> net: mana: do not discard an ethtool-configured RSS table on a queue
> rebuild
> 
> 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.
> 
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index 7a1ac853e3abc..97386e17b9421 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > @@ -3306,6 +3306,28 @@ static void mana_rss_table_init(struct
> mana_port_context *apc)
> >  			ethtool_rxfh_indir_default(i, apc->num_queues);  }
> >
> > +/* Whether the current indirection table can be kept for
> > +apc->num_queues,
> > + * rather than rebuilt from the driver default.
> > + *
> > + * Only a user table ("ethtool -X") is worth keeping; a
> > +driver-generated one
> > + * is rebuilt so that it spreads over every queue. An entry pointing
> > +past the
> > + * last queue cannot be kept: mana_config_rss() uses these entries to
> > +index
> > + * apc->rxqs[], which holds apc->num_queues pointers.
> > + */
> > +static bool mana_rss_table_keep(struct mana_port_context *apc) {
> > +	u32 i;
> > +
> > +	if (!netif_is_rxfh_configured(apc->ndev))
> > +		return false;
> > +
> > +	for (i = 0; i < apc->indir_table_sz; i++)
> > +		if (apc->indir_table[i] >= apc->num_queues)
> > +			return false;
> > +
> > +	return true;
> > +}
> 
> [Severity: High]
> Does this check wrongly trust netif_is_rxfh_configured() without validating if
> the table was previously replaced by a driver fallback?
> 
> If a user sets a custom RSS table via ethtool -X and then reduces the channel
> count, mana_alloc_queues() detects the user table indices are out of bounds
> and generates a narrow driver-default table, but intentionally avoids notifying
> the core via ethtool_rxfh_indir_lost(). The core's
> netif_is_rxfh_configured() remains true.
> 
> When the user subsequently increases the channel count, could this function
> incorrectly believe the narrow driver-generated fallback table is still a user-
> configured table? Since its bounds fit the new queue count, it seems this
> might permanently freeze the table at the smaller size.

The two steps are mutually exclusive, so the sequence cannot occur.

The premise is that a channel reduction can strand a user table. It cannot:
both ethtool entry points call ethtool_check_max_channel() (channels.c and
ioctl.c), which refuses the request. On this hardware:

  # ethtool -X ens1 equal 16
  # ethtool -L ens1 combined 4
  netlink error: requested channel counts are too low for existing
  indirection table (15)

Note ethtool_get_max_rxfh_channel() derives that bound by calling
ops->get_rxfh(), i.e. the table the driver currently reports, so the guard
follows whatever mana last installed rather than a remembered user table.
mana_set_rxfh() also rejects any entry >= apc->num_queues, so a user table
is never out of range when it is installed.

That leaves exactly one way for mana_rss_table_keep() to return false: the
clamp in mana_init_port(), where the device reports fewer queues after a
reset or resume. But that clamp is

        if (apc->num_queues > apc->max_queues)
                apc->num_queues = apc->max_queues;

so num_queues ends up equal to max_queues, and max_queues only ever shrinks -
both assignments in mana_init_port() are guarded by "if (apc->max_queues >
...)", and the only initialisation is in mana_probe_port(), which does not run
on resume. A subsequent increase would need new_count > max_combined ==
apc->max_queues, which the core rejects with "requested channel count exceeds
maximum".

So the fallback and the later increase cannot both happen to one port.

On the wider point of not calling ethtool_rxfh_indir_lost(): 16 drivers test
netif_is_rxfh_configured(), and two call ethtool_rxfh_indir_lost() - bnxt
(__bnxt_reserve_rings(), when firmware grants fewer rings than asked) and
mlx5e (mlx5e_attach_netdev(), when max_nch drops). The other fourteen, among
them ice, idpf, hns3, fbnic, gve and nfp, keep a user table when one is set,
regenerate the default otherwise, and do not notify the core. This patch
follows that majority.

mana cannot follow bnxt or mlx5e here even if it wanted to. All three drivers
require the netdev instance lock for the notification, since it sends
ETHTOOL_MSG_RSS_NTF; mlx5e says so at the rtnl_lock()/netdev_lock() pair in
mlx5e_attach_netdev(). The difference is the call sites, not the capability.
bnxt reaches its call only with dev->lock held, and mlx5e keys off
reg_state == NETREG_REGISTERED. mana_alloc_queues() is reached from ndo_open
with dev->lock held and from mana_attach() on the reset and resume paths
without it, and the netdev is registered in both cases, so neither taking the
lock nor testing reg_state works. Making mana notify would mean holding
netdev_lock across mana_attach() in the TX-timeout handler and the resume
loop, which is not something I would put in a fix for net.


> 
> > +
> >  int mana_disable_vport_rx(struct mana_port_context *apc)  {
> >  	return mana_cfg_vport_steering(apc, TRI_STATE_FALSE, false, false,
> > @@ -3621,7 +3643,20 @@ int mana_alloc_queues(struct net_device
> *ndev)
> >  		goto destroy_rxq;
> >  	}
> >
> > -	mana_rss_table_init(apc);
> > +	/* Keep a user-configured table across the rebuild: its entries are
> > +	 * queue indices and stay meaningful while they are all still in range.
> > +	 * Only a driver-generated table is regenerated here.
> > +	 *
> > +	 * A table that cannot be kept is replaced by the default without
> > +	 * telling the core, which keeps reporting the table as user
> > +	 * configured. That is what this function did for every table before,
> > +	 * and reporting it here is not an option: ethtool_rxfh_indir_lost()
> > +	 * sends ETHTOOL_MSG_RSS_NTF, which requires the netdev instance
> lock,
> > +	 * and this runs both with that lock held (ndo_open) and without it
> > +	 * (mana_attach() from the reset and resume paths).
> > +	 */
> > +	if (!mana_rss_table_keep(apc))
> > +		mana_rss_table_init(apc);
> 
> [Severity: High]
> If mana_rss_table_keep() incorrectly returns true due to the
> desynchronization described above, does it skip the table regeneration and
> leave newly added queues unutilized?

Yes, that consequence would follow - it is the same branch. But the
antecedent cannot hold, for the reasons in my reply to the first half: the
fallback and a later increase cannot both happen to one port.

Worth separating the two cases, though, because the symptom you describe is
also the intended behaviour here. When a real user table is carried across an
increase, the added queues do stay unsteered:

  # ethtool -X ens1 equal 2
  # ethtool -L ens1 combined 16      /* queues 2..15 get no traffic */

That is deliberate. The alternative is to silently discard a table the user
asked for, which is what this patch exists to stop, and it is what ice
(ice_set_channels()), idpf, hns3, fbnic and the rest do as well. The user
recovers with another ethtool -X, and "ethtool -x" always shows the table
actually in use.

So unsteered queues after a grow are not a bug signature by themselves. The
bug would be reaching that state with a table the user never asked for, and
that needs the desync, which cannot occur.

In summary, this patch behavior is consistent with other drivers on this scenario.


> 
> [ ... ]
> 
> [Severity: Critical]
> This is a pre-existing issue, but is there a Use-After-Free in the stats collection
> due to the ad-hoc port_is_up synchronization flag?
> 
> Looking at mana_get_stats64(), the reader path relies on the ad-hoc
> port_is_up flag and dereferences the queue structure, but can be preempted
> before accessing it:
> 
> drivers/net/ethernet/microsoft/mana/mana_en.c:mana_get_stats64() {
> 	if (!apc->port_is_up)
> 		return;
> ...
> 	for (q = 0; q < num_queues; q++) {
> 		rx_stats = &apc->rxqs[q]->stats;
> ...
> }
> 
> Concurrently, a teardown path clears the flag and proceeds to free the
> underlying memory structures without waiting for concurrent readers:
> 
> drivers/net/ethernet/microsoft/mana/mana_en.c:mana_detach() {
> 	apc->port_st_save = apc->port_is_up;
> 	apc->port_is_up = false;
> 
> 	/* Ensure port state updated before txq state */
> 	smp_wmb();
> 
> 	netif_tx_disable(ndev);
> 
> 	if (apc->port_st_save) {
> 		err = mana_dealloc_queues(ndev);
> ...
> }
> 
> The memory is then synchronously freed, guaranteeing a Use-After-Free if a
> reader is mid-execution:
> 
> drivers/net/ethernet/microsoft/mana/mana_en.c:mana_destroy_txq() { ...
> 		mana_deinit_txq(apc, &apc->tx_qp[i]->txq);
> 
> 		kvfree(apc->tx_qp[i]);
> ...
> }
> 
> Could a preempted reader accessing the freed memory lead to memory
> corruption?

This is a pre-existing issue and out of scope from this patch.

I needed, I suggest submitting another patch to fix this.



> 
> --
> Sashiko AI
> review * https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%
> 2Fsashiko.dev%2F%23%2Fpatchset%2F20260905004401.3937066-1-
> longli%40microsoft.com%3Fpart%3D1&data=05%7C02%7Clongli%40microsoft.
> com%7C18ce764cb5f6435395b608df0bb00dee%7C72f988bf86f141af91ab2d7c
> d011db47%7C1%7C0%7C639242522911822221%7CUnknown%7CTWFpbGZsb
> 3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkF
> OIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=IML49OomMP48q
> RaUNNmX7zbI%2BIQ31FQZH5OhErqzovQ%3D&reserved=0

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 2 (1)

- [Medium] Undisclosed TX-side consequence of preserving the indirection
  table.
- [Low] The commit message states: "The driver never clears the core's
  IFF_RXFH_CONFIGURED, so netif_is_rxfh_configured() keeps reporting a…
- [Low] When the surviving table cannot be honoured (an entry >=
  apc->num_queues after mana_init_port() lowered the queue count),…

Pre-existing issues:
- [Low] Incomplete lockset for apc->indir_table on the ethtool read side.

--

net: mana: do not discard an ethtool-configured RSS table on a queue rebuild

mana_alloc_queues() no longer regenerates the RSS indirection table from the
driver default when the table was installed by the user, so an "ethtool -X"
table now survives operations that rebuild the queues.  A table whose
entries point past the last queue is still replaced by the default.  The RSS
ethtool operations are also opted into rtnl_lock() via
ETHTOOL_OP_NEEDS_RTNL_RSS.

A couple of questions about the commit message and the new code below.

>     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.

[Severity: Low]
Is IFF_RXFH_CONFIGURED still the right name here?  A tree-wide grep for
IFF_RXFH_CONFIGURED at this commit returns nothing.  The state that
netif_is_rxfh_configured() consults is now:

net/ethtool/common.c:
bool netif_is_rxfh_configured(const struct net_device *dev)
{
	return dev->ethtool->rss_indir_user_size;
}

set and cleared by the core in net/ethtool/rss.c and net/ethtool/ioctl.c.
The mechanism described matches, only the symbol name looks stale.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 45a7520491a65..396b741fa86ed 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -3310,6 +3310,28 @@ static void mana_rss_table_init(struct mana_port_context *apc)
>  			ethtool_rxfh_indir_default(i, apc->num_queues);
>  }
>  
> +/* Whether the current indirection table can be kept for apc->num_queues,
> + * rather than rebuilt from the driver default.
> + *
> + * Only a user table ("ethtool -X") is worth keeping; a driver-generated one
> + * is rebuilt so that it spreads over every queue. An entry pointing past the
> + * last queue cannot be kept: mana_config_rss() uses these entries to index
> + * apc->rxqs[], which holds apc->num_queues pointers.
> + */
> +static bool mana_rss_table_keep(struct mana_port_context *apc)
> +{
> +	u32 i;
> +
> +	if (!netif_is_rxfh_configured(apc->ndev))
> +		return false;
> +
> +	for (i = 0; i < apc->indir_table_sz; i++)
> +		if (apc->indir_table[i] >= apc->num_queues)
> +			return false;
> +
> +	return true;
> +}

[Severity: Medium]
Does keeping the table also pin the TX side?  In this driver
apc->indir_table selects the transmit queue too:

mana_en.c:mana_get_tx_queue() {
	...
	txq = apc->indir_table[hash & (apc->indir_table_sz - 1)];
	...
}

reached from mana_select_queue(), and the struct member itself is commented
"Indirection Table for RX & TX" in include/net/mana/mana.h.

Before this change every rebuild ran mana_rss_table_init() and restored
index % num_queues, so this sequence ended up with both RX and TX spread
over all 16 queues:

	ethtool -X ens1 equal 2
	ethtool -L ens1 combined 16

After the change mana_rss_table_keep() returns true, since every entry (0
and 1) is below the new apc->num_queues of 16, so the two-entry table
survives mana_set_channels() -> mana_attach() -> mana_alloc_queues().
netif_set_real_num_tx_queues() then publishes 16 TX queues while
mana_get_tx_queue() can only ever return 0 or 1.

The changelog lists the rebuild paths that are affected but describes the
change purely in terms of the RSS indirection table.  Would it be worth
mentioning that TX queue selection becomes sticky in the same way?

> @@ -3625,7 +3647,20 @@ int mana_alloc_queues(struct net_device *ndev)
>  		goto destroy_rxq;
>  	}
>  
> -	mana_rss_table_init(apc);
> +	/* Keep a user-configured table across the rebuild: its entries are
> +	 * queue indices and stay meaningful while they are all still in range.
> +	 * Only a driver-generated table is regenerated here.
> +	 *
> +	 * A table that cannot be kept is replaced by the default without
> +	 * telling the core, which keeps reporting the table as user
> +	 * configured. That is what this function did for every table before,
> +	 * and reporting it here is not an option: ethtool_rxfh_indir_lost()
> +	 * sends ETHTOOL_MSG_RSS_NTF, which requires the netdev instance lock,
> +	 * and this runs both with that lock held (ndo_open) and without it
> +	 * (mana_attach() from the reset and resume paths).
> +	 */
> +	if (!mana_rss_table_keep(apc))
> +		mana_rss_table_init(apc);

[Severity: Low]
This is the acknowledged case, so mostly a question about whether it can be
narrowed rather than a new problem.

When mana_attach() -> mana_init_port() lowers apc->num_queues and an entry
falls out of range, this branch installs the driver default while
dev->ethtool->rss_indir_user_size stays set, so netif_is_rxfh_configured()
and "ethtool -x" keep advertising a user table the user never installed.

The stated reason for skipping ethtool_rxfh_indir_lost() does hold up: its
own assert accepts RTNL,

net/ethtool/common.c:ethtool_rxfh_indir_lost() {
	WARN_ONCE(!rtnl_is_locked() &&
		  !lockdep_is_held_type(&dev->ethtool->rss_lock, -1),
		  "RSS context lock assertion failed\n");
	...
	dev->ethtool->rss_indir_user_size = 0;
	ethtool_rss_notify(dev, ETHTOOL_MSG_RSS_NTF, 0);
}

but the notification lands in ethnl_notify(), which runs
netdev_assert_locked_ops_compat(dev).  mana registers net_shaper_ops, so
with CONFIG_NET_SHAPER=y that reduces to lockdep_assert_held(&dev->lock),
which the rtnl-only reset and resume paths do not hold.

Is there a way to reach the same result without the notification, so that
the core's view and the programmed table agree in this case?

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> index ece7ff9cc409a..e3acaf17efabc 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> @@ -871,7 +871,8 @@ const struct ethtool_ops mana_ethtool_ops = {
>  	.op_needs_rtnl		= ETHTOOL_OP_NEEDS_RTNL_SCHANNELS |
>  				  ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM |
>  				  ETHTOOL_OP_NEEDS_RTNL_SPFLAGS |
> -				  ETHTOOL_OP_NEEDS_RTNL_GLINK,
> +				  ETHTOOL_OP_NEEDS_RTNL_GLINK |
> +				  ETHTOOL_OP_NEEDS_RTNL_RSS,

[Severity: Low]
This isn't a problem introduced by this patch, but does the new flag cover
the read side as well?

The core only consults ETHTOOL_OP_NEEDS_RTNL_RSS for ETHTOOL_MSG_RSS_SET
and for the SRSSH / SRXFH / SRXFHINDIR ioctls:

net/ethtool/common.h:
	case ETHTOOL_MSG_RSS_SET:
		return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_RSS;

so mana_get_rxfh() still runs with the netdev instance lock plus rss_lock
only, while the rebuild writes the same array entry by entry holding RTNL
alone when it comes from mana_attach():

mana_en.c:mana_rss_table_init() {
	for (i = 0; i < apc->indir_table_sz; i++)
		apc->indir_table[i] =
			ethtool_rxfh_indir_default(i, apc->num_queues);
}

CPU0: mana_per_port_queue_reset_work_handler() -> rtnl_lock() ->
      mana_attach() -> mana_alloc_queues() -> mana_rss_table_init()
CPU1: ethtool -x -> rss_prepare_get() -> mana_get_rxfh()

Can "ethtool -x" print a mix of old and new entries in that window?  The
hardware itself is programmed from the driver copy by mana_config_rss(), so
this looks limited to the reported table.  The write/read race the patch set
out to close on the set side does get closed by the new flag.

For what it is worth, the freed-table variant of this does not appear to
exist: mana_attach()'s failure path calls mana_cleanup_port_context(), which
only frees apc->rxqs and the debugfs dentry, and mana_cleanup_indir_table()
is only reached from mana_probe_port() before register_netdev() and from
mana_remove() after unregister_netdevice().

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260905004401.3937066-1-longli%40microsoft.com

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

[Jakub Kicinski]

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.
 
> Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 37 ++++++++++++++++++-
>  .../ethernet/microsoft/mana/mana_ethtool.c    |  3 +-
>  2 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 7a1ac853e3abcd28c4a1e5c6987ec631a18ad840..97386e17b9421aedefa25bab6fbf0b7b1f2996d4 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -3306,6 +3306,28 @@ static void mana_rss_table_init(struct mana_port_context *apc)
>  			ethtool_rxfh_indir_default(i, apc->num_queues);
>  }
>  
> +/* Whether the current indirection table can be kept for apc->num_queues,
> + * rather than rebuilt from the driver default.
> + *
> + * Only a user table ("ethtool -X") is worth keeping; a driver-generated one
> + * is rebuilt so that it spreads over every queue. An entry pointing past the
> + * last queue cannot be kept: mana_config_rss() uses these entries to index
> + * apc->rxqs[], which holds apc->num_queues pointers.
> + */
> +static bool mana_rss_table_keep(struct mana_port_context *apc)
> +{
> +	u32 i;
> +
> +	if (!netif_is_rxfh_configured(apc->ndev))
> +		return false;
> +
> +	for (i = 0; i < apc->indir_table_sz; i++)
> +		if (apc->indir_table[i] >= apc->num_queues)
> +			return false;
> +
> +	return true;
> +}
> +
>  int mana_disable_vport_rx(struct mana_port_context *apc)
>  {
>  	return mana_cfg_vport_steering(apc, TRI_STATE_FALSE, false, false,
> @@ -3621,7 +3643,20 @@ int mana_alloc_queues(struct net_device *ndev)
>  		goto destroy_rxq;
>  	}
>  
> -	mana_rss_table_init(apc);
> +	/* Keep a user-configured table across the rebuild: its entries are
> +	 * queue indices and stay meaningful while they are all still in range.
> +	 * Only a driver-generated table is regenerated here.
> +	 *
> +	 * A table that cannot be kept is replaced by the default without
> +	 * telling the core, which keeps reporting the table as user
> +	 * configured. That is what this function did for every table before,
> +	 * and reporting it here is not an option: ethtool_rxfh_indir_lost()
> +	 * sends ETHTOOL_MSG_RSS_NTF, which requires the netdev instance lock,
> +	 * and this runs both with that lock held (ndo_open) and without it
> +	 * (mana_attach() from the reset and resume paths).
> +	 */
> +	if (!mana_rss_table_keep(apc))
> +		mana_rss_table_init(apc);
>  
>  	err = mana_config_rss(apc, TRI_STATE_TRUE, true, true);
>  	if (err) {
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> index ece7ff9cc409a806b6a6de70a85b44874bfa6dad..e3acaf17efabcd62f7cffabdb8404cf16b917272 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> @@ -871,7 +871,8 @@ const struct ethtool_ops mana_ethtool_ops = {
>  	.op_needs_rtnl		= ETHTOOL_OP_NEEDS_RTNL_SCHANNELS |
>  				  ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM |
>  				  ETHTOOL_OP_NEEDS_RTNL_SPFLAGS |
> -				  ETHTOOL_OP_NEEDS_RTNL_GLINK,
> +				  ETHTOOL_OP_NEEDS_RTNL_GLINK |
> +				  ETHTOOL_OP_NEEDS_RTNL_RSS,
>  	.get_ethtool_stats	= mana_get_ethtool_stats,
>  	.get_sset_count		= mana_get_sset_count,
>  	.get_strings		= mana_get_strings,

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

[Long Li]

> 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.