net: mana: add ethtool private flag for full-page RX buffers

PATCH Superseded Dipayaan Roy 3 participants 8 messages Jul 11 – Jul 29

Subsystems (6)

Networking Network drivers BPF InfiniBand +2 more subsystems

lore

Share & tools

lore

Dipayaan Roy

author

2026-07-11 04:14 UTC #0 lore raw

v12 cover net: mana: add ethtool private flag for full-page RX buffers

On some ARM64 platforms with 4K PAGE_SIZE, utilizing page_pool fragments for allocation in the RX refill path (~2kB buffer per fragment) causes 15-20% throughput regression under high connection counts (>16 TCP streams at 180+ Gbps). Using full-page buffers on these platforms shows no regression and restores line-rate performance.

This behavior is observed on a single platform; other platforms perform better with page_pool fragments, indicating this is not a page_pool issue but platform-specific.

This series adds an ethtool private flag "full-page-rx" to let the user opt in to one RX buffer per page:

  ethtool --set-priv-flags eth0 full-page-rx on

There is no behavioral change by default. The flag can be persisted via udev rule for affected platforms.

Patches 2 and 4 harden the detach/attach path so that ethtool operations (ring size, channel count, priv-flags) can recover the port via the queue_reset_work handler when mana_attach() fails, instead of leaving the port permanently dead.

This series depends on the following fixes now merged in net-next:
commit 17bfe0a8c014 ("net: mana: Add NULL guards in teardown path to prevent panic on attach failure") commit 5b05aa36ee24 ("net: mana: Skip redundant detach on already-detached port")

Changes in v12:
- Added patch 2 to ensure mana_detach() always completes its full teardown even if mana_dealloc_queues() fails, keeping port state consistent for recovery.
- Added patch 4 to schedule queue_reset_work when mana_attach() fails during ethtool ring size or channel count changes, with fallback values that maximize recovery chances.
Changes in v11:
- Rebased on net-next
Changes in v10:
- Rebased on net-next which now includes the prerequisite fixes.
- Recovery logic in mana_set_priv_flags() leverages the idempotent mana_detach() from the merged fixes.
Changes in v9:
- Added correct tree.
Changes in v8:
- Fixed queue_reset_work recovery by restoring port_is_up before scheduling reset so the handler can properly re-attach.
- Simplified "err && schedule_port_reset" to "schedule_port_reset".
Changes in v7:
- Rebased onto net-next.
- Retained private flag approach after David Wei's testing on Grace (ARM64) confirmed that fragment mode outperforms full-page mode on other platforms, validating this is a single-platform workaround rather than a generic issue.
Changes in v6:
- Added missed maintainers.
Changes in v5:
- Split prep refactor into separate patch (patch 1/2)
Changes in v4:
- Dropping the smbios string parsing and add ethtool priv flag to reconfigure the queues with full page rx buffers.
Changes in v3:
- changed u8* to char*
Changes in v2:
- separate reading string index and the string, remove inline.

Dipayaan Roy (4):
net: mana: refactor mana_get_strings() and mana_get_sset_count() to
use switch
net: mana: do not bail out of mana_detach on dealloc failure
net: mana: force full-page RX buffers via ethtool private flag
net: mana: recover port on attach failure in ethtool operations

 drivers/net/ethernet/microsoft/mana/mana_en.c |  26 +-
 .../ethernet/microsoft/mana/mana_ethtool.c    | 223 +++++++++++++++---
 include/net/mana/mana.h                       |   8 +
 3 files changed, 220 insertions(+), 37 deletions(-)

Signature · 1 line

2.43.0

Dipayaan Roy

author

2026-07-11 04:14 UTC #1 lore raw

PATCH v12 1/4 net: mana: refactor mana_get_strings() and mana_get_sset_count() to use switch

Refactor mana_get_strings() and mana_get_sset_count() from if/else to switch statements in preparation for adding ethtool private flags support which requires handling ETH_SS_PRIV_FLAGS.

No functional change.

Reviewed-by Haiyang Zhang <[email protected]>

Signed-off-by Dipayaan Roy <[email protected]>

---
 .../ethernet/microsoft/mana/mana_ethtool.c    | 75 ++++++++++++-------
 1 file changed, 46 insertions(+), 29 deletions(-)

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +46 ~~-29~~

index 9e31e2595ae3..482cd16009ab 100644
@@ -138,53 +138,70 @@ static int mana_get_sset_count(struct net_device *ndev, int stringset)
 	struct mana_port_context *apc = netdev_priv(ndev);
 	unsigned int num_queues = apc->num_queues;
 
-	if (stringset != ETH_SS_STATS)
+	switch (stringset) {
+	case ETH_SS_STATS:
+		return ARRAY_SIZE(mana_eth_stats) +
+		       ARRAY_SIZE(mana_phy_stats) +
+		       ARRAY_SIZE(mana_hc_stats)  +
+		       num_queues * (MANA_STATS_RX_COUNT + MANA_STATS_TX_COUNT);
+	default:
 		return -EINVAL;
-
-	return ARRAY_SIZE(mana_eth_stats) + ARRAY_SIZE(mana_phy_stats) + ARRAY_SIZE(mana_hc_stats) +
-			num_queues * (MANA_STATS_RX_COUNT + MANA_STATS_TX_COUNT);
+	}
 }
 
-static void mana_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
+static void mana_get_strings_stats(struct mana_port_context *apc, u8 **data)
 {
-	struct mana_port_context *apc = netdev_priv(ndev);
 	unsigned int num_queues = apc->num_queues;
 	int i, j;
 
-	if (stringset != ETH_SS_STATS)
-		return;
 	for (i = 0; i < ARRAY_SIZE(mana_eth_stats); i++)
-		ethtool_puts(&data, mana_eth_stats[i].name);
+		ethtool_puts(data, mana_eth_stats[i].name);
 
 	for (i = 0; i < ARRAY_SIZE(mana_hc_stats); i++)
-		ethtool_puts(&data, mana_hc_stats[i].name);
+		ethtool_puts(data, mana_hc_stats[i].name);
 
 	for (i = 0; i < ARRAY_SIZE(mana_phy_stats); i++)
-		ethtool_puts(&data, mana_phy_stats[i].name);
+		ethtool_puts(data, mana_phy_stats[i].name);
 
 	for (i = 0; i < num_queues; i++) {
-		ethtool_sprintf(&data, "rx_%d_packets", i);
-		ethtool_sprintf(&data, "rx_%d_bytes", i);
-		ethtool_sprintf(&data, "rx_%d_xdp_drop", i);
-		ethtool_sprintf(&data, "rx_%d_xdp_tx", i);
-		ethtool_sprintf(&data, "rx_%d_xdp_redirect", i);
-		ethtool_sprintf(&data, "rx_%d_pkt_len0_err", i);
+		ethtool_sprintf(data, "rx_%d_packets", i);
+		ethtool_sprintf(data, "rx_%d_bytes", i);
+		ethtool_sprintf(data, "rx_%d_xdp_drop", i);
+		ethtool_sprintf(data, "rx_%d_xdp_tx", i);
+		ethtool_sprintf(data, "rx_%d_xdp_redirect", i);
+		ethtool_sprintf(data, "rx_%d_pkt_len0_err", i);
 		for (j = 0; j < MANA_RXCOMP_OOB_NUM_PPI - 1; j++)
-			ethtool_sprintf(&data, "rx_%d_coalesced_cqe_%d", i, j + 2);
+			ethtool_sprintf(data,
+					"rx_%d_coalesced_cqe_%d",
+					i,
+					j + 2);
 	}
 
 	for (i = 0; i < num_queues; i++) {
-		ethtool_sprintf(&data, "tx_%d_packets", i);
-		ethtool_sprintf(&data, "tx_%d_bytes", i);
-		ethtool_sprintf(&data, "tx_%d_xdp_xmit", i);
-		ethtool_sprintf(&data, "tx_%d_tso_packets", i);
-		ethtool_sprintf(&data, "tx_%d_tso_bytes", i);
-		ethtool_sprintf(&data, "tx_%d_tso_inner_packets", i);
-		ethtool_sprintf(&data, "tx_%d_tso_inner_bytes", i);
-		ethtool_sprintf(&data, "tx_%d_long_pkt_fmt", i);
-		ethtool_sprintf(&data, "tx_%d_short_pkt_fmt", i);
-		ethtool_sprintf(&data, "tx_%d_csum_partial", i);
-		ethtool_sprintf(&data, "tx_%d_mana_map_err", i);
+		ethtool_sprintf(data, "tx_%d_packets", i);
+		ethtool_sprintf(data, "tx_%d_bytes", i);
+		ethtool_sprintf(data, "tx_%d_xdp_xmit", i);
+		ethtool_sprintf(data, "tx_%d_tso_packets", i);
+		ethtool_sprintf(data, "tx_%d_tso_bytes", i);
+		ethtool_sprintf(data, "tx_%d_tso_inner_packets", i);
+		ethtool_sprintf(data, "tx_%d_tso_inner_bytes", i);
+		ethtool_sprintf(data, "tx_%d_long_pkt_fmt", i);
+		ethtool_sprintf(data, "tx_%d_short_pkt_fmt", i);
+		ethtool_sprintf(data, "tx_%d_csum_partial", i);
+		ethtool_sprintf(data, "tx_%d_mana_map_err", i);
+	}
+}
+
+static void mana_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
+{
+	struct mana_port_context *apc = netdev_priv(ndev);
+
+	switch (stringset) {
+	case ETH_SS_STATS:
+		mana_get_strings_stats(apc, &data);
+		break;
+	default:
+		break;
 	}
 }

Signature · 1 line

2.43.0

Dipayaan Roy

author

2026-07-11 04:14 UTC #2 lore raw

PATCH v12 2/4 net: mana: do not bail out of mana_detach on dealloc failure

In reply to Dipayaan Roy

mana_detach() sets port_is_up = false before calling mana_dealloc_queues(). If that call were to fail and return early, netif_device_detach() and mana_cleanup_port_context() are skipped, leaving the port in an inconsistent state where port_is_up is false but netif_device_present() still returns true. A subsequent mana_detach() from the reset work handler would then overwrite port_st_save with false, causing mana_attach() to skip queue allocation and leave the port permanently dead.

Remove the early return so that mana_detach() always completes its full teardown. mana_dealloc_queues() already performs best-effort cleanup regardless of internal errors (and in practice cannot fail here since port_is_up is already false), so continuing to netif_device_detach() and mana_cleanup_port_context() is safe and ensures the state is always consistent for recovery.

Signed-off-by Dipayaan Roy <[email protected]>

---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

drivers/net/ethernet/microsoft/mana/mana_en.c +1 -3

index 89e7f59f635d..5e3c7a2a2b49 100644
@@ -3707,10 +3707,8 @@ int mana_detach(struct net_device *ndev, bool from_close)
 
 	if (apc->port_st_save) {
 		err = mana_dealloc_queues(ndev);
-		if (err) {
+		if (err)
 			netdev_err(ndev, "%s failed to deallocate queues: %d\n", __func__, err);
-			return err;
-		}
 	}
 
 	if (!from_close) {

Signature · 1 line

2.43.0

Dipayaan Roy

author

2026-07-11 04:14 UTC #3 lore raw

PATCH v12 3/4 net: mana: force full-page RX buffers via ethtool private flag

In reply to Dipayaan Roy

On some ARM64 platforms with 4K PAGE_SIZE, page_pool fragment allocation in the RX refill path can cause 15-20% throughput regression under high connection counts (>16 TCP streams).

Add an ethtool private flag "full-page-rx" that allows the user to force one RX buffer per page, bypassing the page_pool fragment path. This restores line-rate (180+ Gbps) performance on affected platforms.

Usage:
ethtool --set-priv-flags eth0 full-page-rx on

There is no behavioral change by default. The flag must be explicitly enabled by the user or udev rule.

The existing single-buffer-per-page logic for XDP and jumbo frames is consolidated into a new helper mana_use_single_rxbuf_per_page() which is now the single decision point for both the automatic and user-controlled paths.

Reviewed-by Jacob Keller <[email protected]>

Reviewed-by Haiyang Zhang <[email protected]>

Signed-off-by Dipayaan Roy <[email protected]>

---
 drivers/net/ethernet/microsoft/mana/mana_en.c |  22 +++-
 .../ethernet/microsoft/mana/mana_ethtool.c    | 100 ++++++++++++++++++
 include/net/mana/mana.h                       |   8 ++
 3 files changed, 128 insertions(+), 2 deletions(-)

drivers/net/ethernet/microsoft/mana/mana_en.c +20 -2

index 5e3c7a2a2b49..3e5c52e4886b 100644
@@ -755,6 +755,25 @@ static void *mana_get_rxbuf_pre(struct mana_rxq *rxq, dma_addr_t *da)
 	return va;
 }
 
+static bool
+mana_use_single_rxbuf_per_page(struct mana_port_context *apc, u32 mtu)
+{
+	/* On some platforms with 4K PAGE_SIZE, page_pool fragment allocation
+	 * in the RX refill path (~2kB buffer) can cause significant throughput
+	 * regression under high connection counts. Allow user to force one RX
+	 * buffer per page via ethtool private flag to bypass the fragment
+	 * path.
+	 */
+	if (apc->priv_flags & BIT(MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF))
+		return true;
+
+	/* For xdp and jumbo frames make sure only one packet fits per page. */
+	if (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 || mana_xdp_get(apc))
+		return true;
+
+	return false;
+}
+
 /* Get RX buffer's data size, alloc size, XDP headroom based on MTU */
 static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
 			       int mtu, u32 *datasize, u32 *alloc_size,

@@ -765,8 +784,7 @@ static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
 	/* Calculate datasize first (consistent across all cases) */
 	*datasize = mtu + ETH_HLEN;
 
-	/* For xdp and jumbo frames make sure only one packet fits per page */
-	if (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 || mana_xdp_get(apc)) {
+	if (mana_use_single_rxbuf_per_page(apc, mtu)) {
 		if (mana_xdp_get(apc)) {
 			*headroom = XDP_PACKET_HEADROOM;
 			*alloc_size = PAGE_SIZE;

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +100 -0

140 lines not on this page · raw message

include/net/mana/mana.h +8 -0

index 226b61504596..768d9f9bf167 100644
@@ -31,6 +31,12 @@ enum TRI_STATE {
 	TRI_STATE_TRUE = 1
 };
 
+/* MANA ethtool private flag bit positions */
+enum mana_priv_flag_bits {
+	MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF = 0,
+	MANA_PRIV_FLAG_MAX,
+};
+
 /* Number of entries for hardware indirection table must be in power of 2 */
 #define MANA_INDIRECT_TABLE_MAX_SIZE 512
 #define MANA_INDIRECT_TABLE_DEF_SIZE 64

@@ -565,6 +571,8 @@ struct mana_port_context {
 	u32 rxbpre_headroom;
 	u32 rxbpre_frag_count;
 
+	u32 priv_flags;
+
 	struct bpf_prog *bpf_prog;
 
 	/* Create num_queues EQs, SQs, SQ-CQs, RQs and RQ-CQs, respectively. */

Signature · 1 line

2.43.0

Dipayaan Roy

author

2026-07-11 04:14 UTC #4 lore raw

PATCH v12 4/4 net: mana: recover port on attach failure in ethtool operations

In reply to Dipayaan Roy

When mana_attach() fails during ethtool ring size or channel count changes, the port is left in a broken state with no recovery mechanism, requiring manual intervention to bring the port back up.

On VM SKUs without a netvsc fallback interface, this results in complete loss of network connectivity to the VM.

Fix by scheduling queue_reset_work when mana_attach() fails. The preceding patch ensures mana_detach() always completes its full teardown (netif_device_detach + cleanup), so the reset handler's mana_detach() takes the "already detached" early return, preserving port_st_save for a successful mana_attach() recovery.

When mana_attach() fails, choose retry values that maximize recovery
chances: if the operation was an increase, fall back to the previous working values; if it was a decrease but still above default, fall back to defaults; otherwise use the minimum supported values.

Reviewed-by Haiyang Zhang <[email protected]>

Tested-by Aditya Garg <[email protected]>

Signed-off-by Dipayaan Roy <[email protected]>

---
 .../ethernet/microsoft/mana/mana_ethtool.c    | 48 +++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +45 -3

index f77509818d07..71e69d5a9a04 100644
@@ -646,6 +646,7 @@ static int mana_set_channels(struct net_device *ndev,
 	struct mana_port_context *apc = netdev_priv(ndev);
 	unsigned int new_count = channels->combined_count;
 	unsigned int old_count = apc->num_queues;
+	bool schedule_port_reset = false;
 	int err;
 
 	/* Set channel_changing to block RDMA from grabbing the vport

@@ -675,8 +676,19 @@ static int mana_set_channels(struct net_device *ndev,
 	apc->num_queues = new_count;
 	err = mana_attach(ndev);
 	if (err) {
-		apc->num_queues = old_count;
 		netdev_err(ndev, "mana_attach failed: %d\n", err);
+
+		/* Choose a retry queue count that maximizes recovery
+		 * chances in the reset work handler.
+		 */
+		if (old_count < new_count)
+			apc->num_queues = old_count;
+		else if (new_count > MANA_DEF_NUM_QUEUES)
+			apc->num_queues = MANA_DEF_NUM_QUEUES;
+		else
+			apc->num_queues = 1;
+
+		schedule_port_reset = true;
 	}
 
 out:

@@ -685,6 +697,11 @@ static int mana_set_channels(struct net_device *ndev,
 	mutex_lock(&apc->vport_mutex);
 	apc->channel_changing = false;
 	mutex_unlock(&apc->vport_mutex);
+
+	if (schedule_port_reset)
+		queue_work(apc->ac->per_port_queue_reset_wq,
+			   &apc->queue_reset_work);
+
 	return err;
 }

@@ -707,6 +724,7 @@ static int mana_set_ringparam(struct net_device *ndev,
 			      struct netlink_ext_ack *extack)
 {
 	struct mana_port_context *apc = netdev_priv(ndev);
+	bool schedule_port_reset = false;
 	u32 new_tx, new_rx;
 	u32 old_tx, old_rx;
 	int err;

@@ -752,11 +770,35 @@ static int mana_set_ringparam(struct net_device *ndev,
 	err = mana_attach(ndev);
 	if (err) {
 		netdev_err(ndev, "mana_attach failed: %d\n", err);
-		apc->tx_queue_size = old_tx;
-		apc->rx_queue_size = old_rx;
+		NL_SET_ERR_MSG_FMT(extack, "failed to change ring params: %d",
+				   err);
+
+		/* Choose retry ring sizes that maximize recovery
+		 * chances in the reset work handler. Handle RX and
+		 * TX independently.
+		 */
+		if (old_rx < new_rx)
+			apc->rx_queue_size = old_rx;
+		else if (new_rx > DEF_RX_BUFFERS_PER_QUEUE)
+			apc->rx_queue_size = DEF_RX_BUFFERS_PER_QUEUE;
+		else
+			apc->rx_queue_size = MIN_RX_BUFFERS_PER_QUEUE;
+
+		if (old_tx < new_tx)
+			apc->tx_queue_size = old_tx;
+		else if (new_tx > DEF_TX_BUFFERS_PER_QUEUE)
+			apc->tx_queue_size = DEF_TX_BUFFERS_PER_QUEUE;
+		else
+			apc->tx_queue_size = MIN_TX_BUFFERS_PER_QUEUE;
+
+		schedule_port_reset = true;
 	}
 out:
 	mana_pre_dealloc_rxbufs(apc);
+
+	if (schedule_port_reset)
+		queue_work(apc->ac->per_port_queue_reset_wq,
+			   &apc->queue_reset_work);
 	return err;
 }

Signature · 1 line

2.43.0

[email protected]

AI review

2026-07-12 04:14 UTC #5 lore raw

Re: [PATCH net-next v12 4/4] net: mana: recover port on attach failure in ethtool operations

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

New issues:
- [High] The newly introduced background recovery via queue_reset_work races with administrative interface state changes (ip link set down), causing the port to become permanently stuck in a broken state.
- [High] Clearing channel_changing before queuing queue_reset_work in mana_set_channels creates a race window where the RDMA driver can claim the vport, permanently defeating the recovery.

Pre-existing issues:
- [Critical] A failure in mana_attach() leaves the driver vulnerable to a guaranteed NULL pointer dereference when the port is later brought up via ip link set up.
- [High] Missing RDMA synchronization via the channel_changing flag in mana_set_ringparam, mana_change_mtu, and mana_xdp_set allows the RDMA driver to hijack the vport during configuration changes.
- [Medium] The fix for mana_attach() failures leaving the port in a broken state was applied to ethtool ring size and channel count changes, but missed MTU changes and XDP program attachment. --

--- Patch [4]: [PATCH net-next v12 4/4] net: mana: recover port on attach failure in ethtool operations --- commit 750fd303cde77a7c9585144b5c7341d83090dbb3
Author: Dipayaan Roy <[email protected]>

net: mana: recover port on attach failure in ethtool operations

This patch adds a recovery mechanism for mana_attach() failures during ethtool ring size or channel count changes by scheduling queue_reset_work, while adjusting retry values to maximize the chance of recovery.

Quoted 13 lines

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +3 -1

index f77509818d07f..71e69d5a9a04a 100644
@@ -675,8 +676,19 @@ static int mana_set_channels(struct net_device *ndev,
 	apc->num_queues = new_count;
 	err = mana_attach(ndev);
 	if (err) {
-		apc->num_queues = old_count;
 		netdev_err(ndev, "mana_attach failed: %d\n", err);
+
+		/* Choose a retry queue count that maximizes recovery
+		 * chances in the reset work handler.

drivers/net/ethernet/microsoft/mana/mana_ethtool.c static int mana_set_channels(struct net_device *ndev, · line 692

Quoted 10 lines

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +9 -0

+		 */
+		if (old_count < new_count)
+			apc->num_queues = old_count;
+		else if (new_count > MANA_DEF_NUM_QUEUES)
+			apc->num_queues = MANA_DEF_NUM_QUEUES;
+		else
+			apc->num_queues = 1;
+
+		schedule_port_reset = true;
 	}

[Severity: Critical]
This is a pre-existing issue, but a failure in mana_attach() leaves the driver vulnerable to a NULL pointer dereference when the port is later brought up manually.

If mana_attach() fails, its failure path calls mana_cleanup_port_context() which frees apc->rxqs:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_cleanup_port_context() {
    ...
    kfree(apc->rxqs);
    apc->rxqs = NULL;
    ...
}

If the user later brings the interface up, mana_open() triggers the queue allocation chain without reinitializing the port context:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_open() {
    ...
    err = mana_alloc_queues(ndev);
    ...
}

Which dereferences the NULL pointer in mana_add_rx_queues():

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_add_rx_queues() {
    ...
    apc->rxqs[i] = rxq;
    ...
}

Is there a mechanism to prevent this sequence if recovery fails?

drivers/net/ethernet/microsoft/mana/mana_ethtool.c static int mana_set_channels(struct net_device *ndev, · line 699

Quoted 5 lines

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +0 -0

 out:

@@ -685,6 +697,11 @@ static int mana_set_channels(struct net_device *ndev,
 	mutex_lock(&apc->vport_mutex);
 	apc->channel_changing = false;
 	mutex_unlock(&apc->vport_mutex);

[Severity: High]
Could clearing channel_changing before the reset work executes create a race condition where the RDMA driver claims the vport?

If mana_set_channels() fails in mana_attach() and drops vport_mutex while setting apc->channel_changing = false, a concurrent RDMA QP creation could call mana_cfg_vport() and successfully claim the hardware vport since vport_use_count is 0.

When the queued reset work later calls mana_attach() -> mana_create_vport(), it would see vport_use_count as 1 and fail with -EBUSY, preventing the recovery from succeeding.

Should channel_changing remain true until the recovery completes?

drivers/net/ethernet/microsoft/mana/mana_ethtool.c static int mana_set_channels(struct net_device *ndev, · line 706

Quoted 7 lines

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +5 -0

+
+	if (schedule_port_reset)
+		queue_work(apc->ac->per_port_queue_reset_wq,
+			   &apc->queue_reset_work);
+
 	return err;
 }

[Severity: High]
Can this asynchronous recovery race with administrative interface state changes, leaving the port in a state where it cannot be brought up or down?

If an admin runs ip link set down before the reset workqueue executes:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_close() {
    if (!apc->port_is_up)
        return 0;
    ...
}

Because mana_close() returns early, apc->port_st_save remains true, while the kernel state transitions to DOWN.

When the reset work runs, it calls mana_attach() which sees port_st_save == true, reallocates queues, increments vport_use_count to 1, and sets apc->port_is_up = true.

If the admin then runs ip link set up, mana_open() will fail in mana_create_vport() with -EBUSY because vport_use_count is already 1. A subsequent ip link set down will do nothing because the kernel state is already DOWN.

Does the reset work handler need to synchronize against administrative state changes?

[ ... ]

drivers/net/ethernet/microsoft/mana/mana_ethtool.c static int mana_set_ringparam(struct net_device *ndev, · line 771

Quoted 3 lines

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +0 -0

@@ -752,11 +770,35 @@ static int mana_set_ringparam(struct net_device *ndev,
 	err = mana_attach(ndev);
 	if (err) {

[Severity: High]
This isn't a bug introduced by this patch, but does the detach and attach sequence here lack synchronization against the RDMA driver?

Unlike mana_set_channels(), mana_set_ringparam() does not acquire vport_mutex or set apc->channel_changing = true before calling mana_detach().

When it calls mana_detach(ndev, false), vport_use_count drops to 0. A concurrent RDMA workload could call mana_cfg_vport() and claim the vport during this window.

When the Ethernet driver subsequently calls mana_attach(), it would fail with -EBUSY.

This same lack of synchronization appears to be present in mana_change_mtu() and mana_xdp_set().

Quoted 24 lines

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +21 -2

 		netdev_err(ndev, "mana_attach failed: %d\n", err);
-		apc->tx_queue_size = old_tx;
-		apc->rx_queue_size = old_rx;
+		NL_SET_ERR_MSG_FMT(extack, "failed to change ring params: %d",
+				   err);
+
+		/* Choose retry ring sizes that maximize recovery
+		 * chances in the reset work handler. Handle RX and
+		 * TX independently.
+		 */
+		if (old_rx < new_rx)
+			apc->rx_queue_size = old_rx;
+		else if (new_rx > DEF_RX_BUFFERS_PER_QUEUE)
+			apc->rx_queue_size = DEF_RX_BUFFERS_PER_QUEUE;
+		else
+			apc->rx_queue_size = MIN_RX_BUFFERS_PER_QUEUE;
+
+		if (old_tx < new_tx)
+			apc->tx_queue_size = old_tx;
+		else if (new_tx > DEF_TX_BUFFERS_PER_QUEUE)
+			apc->tx_queue_size = DEF_TX_BUFFERS_PER_QUEUE;
+		else
+			apc->tx_queue_size = MIN_TX_BUFFERS_PER_QUEUE;
+

drivers/net/ethernet/microsoft/mana/mana_ethtool.c static int mana_set_ringparam(struct net_device *ndev, · line 803

Quoted 10 lines

drivers/net/ethernet/microsoft/mana/mana_ethtool.c +5 -0

+		schedule_port_reset = true;
 	}
 out:
 	mana_pre_dealloc_rxbufs(apc);
+
+	if (schedule_port_reset)
+		queue_work(apc->ac->per_port_queue_reset_wq,
+			   &apc->queue_reset_work);
 	return err;
 }

[Severity: Medium]
This is a pre-existing issue, but should this same recovery mechanism be applied to MTU and XDP program changes?

If mana_attach() fails in mana_change_mtu() or mana_xdp_set(), they return immediately without scheduling queue_reset_work. This leaves the port detached, which eventually leads to the NULL pointer dereference in mana_open() if the user tries to bring the interface up manually.

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

Paolo Abeni

maintainer

2026-07-23 10:18 UTC #6 lore raw

Re: [PATCH net-next v12 4/4] net: mana: recover port on attach failure in ethtool operations

In reply to Dipayaan Roy

Quoted 13 lines · Dipayaan Roy

When mana_attach() fails during ethtool ring size or channel count changes, the port is left in a broken state with no recovery mechanism, requiring manual intervention to bring the port back up.
On VM SKUs without a netvsc fallback interface, this results in complete loss of network connectivity to the VM.
Fix by scheduling queue_reset_work when mana_attach() fails. The preceding patch ensures mana_detach() always completes its full teardown (netif_device_detach + cleanup), so the reset handler's mana_detach() takes the "already detached" early return, preserving port_st_save for a successful mana_attach() recovery.

Quoted 8 lines

When mana_attach() fails, choose retry values that maximize recovery
chances: if the operation was an increase, fall back to the previous working values; if it was a decrease but still above default, fall back to defaults; otherwise use the minimum supported values.
Reviewed-by Haiyang Zhang <[email protected]>
Tested-by Aditya Garg <[email protected]>
Signed-off-by Dipayaan Roy <[email protected]>

sashiko gemini sees quite a bit of problem with the above approach:

https://sashiko.dev/#/patchset/20260711041415.3008868-1-dipayanroy%40linux.microsoft.com

Also, changing user-setting in case of fallback is quite a bad thing to be avoided.

Skimming over the code, it looks like than pre-allocate all the resources and to a replace instead of detach/attach() looks feasible. I suggest investigating such path. I also think that this last patch (and possibly 2/4) could/should be in separate series.

Finally and most relevant thing; AI review comments should be explicitly acknowledged by either debating them or addressing them. In both cases a reply on the ML is expected.

Dipayaan Roy

author

2026-07-29 06:15 UTC #7 lore raw

Re: [PATCH net-next v12 4/4] net: mana: recover port on attach failure in ethtool operations

Quoted 34 lines · Paolo Abeni

Quoted 13 lines · Dipayaan Roy
When mana_attach() fails during ethtool ring size or channel count changes, the port is left in a broken state with no recovery mechanism, requiring manual intervention to bring the port back up.
On VM SKUs without a netvsc fallback interface, this results in complete loss of network connectivity to the VM.
Fix by scheduling queue_reset_work when mana_attach() fails. The preceding patch ensures mana_detach() always completes its full teardown (netif_device_detach + cleanup), so the reset handler's mana_detach() takes the "already detached" early return, preserving port_st_save for a successful mana_attach() recovery.
Quoted 8 lines
When mana_attach() fails, choose retry values that maximize recovery
chances: if the operation was an increase, fall back to the previous working values; if it was a decrease but still above default, fall back to defaults; otherwise use the minimum supported values.
Reviewed-by Haiyang Zhang <[email protected]>
Tested-by Aditya Garg <[email protected]>
Signed-off-by Dipayaan Roy <[email protected]>
sashiko gemini sees quite a bit of problem with the above approach:
https://sashiko.dev/#/patchset/20260711041415.3008868-1-dipayanroy%40linux.microsoft.com
Also, changing user-setting in case of fallback is quite a bad thing to be avoided.
Skimming over the code, it looks like than pre-allocate all the resources and to a replace instead of detach/attach() looks feasible. I suggest investigating such path. I also think that this last patch (and possibly 2/4) could/should be in separate series.

Quoted 5 lines

Finally and most relevant thing; AI review comments should be explicitly acknowledged by either debating them or addressing them. In both cases a reply on the ML is expected.
/P

Hi Paolo,

Thank you for the review.

Accepting the feedback on 4/4 and working on the pre-allocate-and-replace approach you suggested. We are discussing with the HW/NIC team and investigating pre-allocating the queue resources from HW while a set already exists, as with this patch we are trying to handle and recover from failures caused due to HW/NIC resource exhaustion for queues.

I will drop 4/4 from this series in v13 and stick to only the ethtool private flag changes. Will send out a separate series for the problem currently discussed in 4/4 once all the above points are concluded.

For 2/4 I will send it as a fixes patch to net as it will be common for all functions calling mana_detach.

Regards
Dipayaan Roy