mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] net: mana: reserve RX buffer headroom to fix forwarding performance
@ 2026-09-23 14:45 Hamza Mahfooz
  2026-09-27  5:14 ` Narcisa Vasile
  2026-09-27 15:27 ` netdev-bot+sashiko
  0 siblings, 2 replies; 3+ messages in thread
From: Hamza Mahfooz @ 2026-09-23 14:45 UTC (permalink / raw)
  To: netdev
  Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
	Simon Horman, Erni Sri Satya Vennela, Dipayaan Roy, Aditya Garg,
	Jacob Keller, Saurabh Sengar, linux-hyperv, bpf, linux-kernel,
	Hamza Mahfooz, stable

Commit 730ff06d3f5c ("net: mana: Use page pool fragments for RX buffers
instead of full pages to improve memory efficiency.") started handing
out RX buffers with zero headroom so that two buffers fit into one page
at the default MTU.

The MANA TX path, however, stores the per scatter-gather entry DMA
mappings in `struct mana_skb_head` at skb->head, and mana_start_xmit()
therefore calls skb_cow_head(skb, MANA_HEADROOM). The port advertises
this requirement as ndev->needed_headroom = MANA_HEADROOM.

As a result every packet that is received and then forwarded out of a
MANA port fails the skb_cow() in ip_forward() and gets reallocated and
copied by pskb_expand_head(). This is invisible to a plain RX or TX
workload, but it puts a full skb reallocation plus memcpy on the hot
path of every single forwarded packet, which is exactly what a
router/NVA workload does.

Restore the headroom. Note that reserving MANA_HEADROOM (232) is not
enough: ip_forward() asks for LL_RESERVED_SPACE(dev), which rounds
hard_header_len + needed_headroom up to HH_DATA_MOD and is 256 bytes on
ethernet. Use LL_RESERVED_SPACE() directly so the value keeps tracking
both constants.

At the default MTU on a 4K page this means a buffer no longer fits twice
into a page (SKB_DATA_ALIGN(1500 + MANA_RXBUF_PAD + 256) = 2112), so the
frag-vs-single decision is now made by computing the real buffer size
instead of comparing the MTU against PAGE_SIZE / 2. The page_pool
fragment path is still used wherever at least two buffers genuinely fit,
e.g. on 16K and 64K page sizes.

Measured on an Azure VM with a MANA NIC acting as a forwarding NVA (UDP,
1400 byte payload, 4 streams, 8 Gbps offered, only the forwarding
node's kernel differs), 8 runs each, median:

                  forwarded pps    throughput
  before              272,830       3.06 Gbps
  after               390,560       4.37 Gbps   (+43%)

perf on the forwarding node, same workload:

                  memset_orig   __pi_memcpy   pskb_expand_head
  before             10.07%         3.96%         present
  after               0.94%         0.64%         gone

Cc: stable@vger.kernel.org
Fixes: 730ff06d3f5c ("net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.")
Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 58 ++++++++++++++-----
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 591fb4191d90d..e3f3b33ba9062 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -758,6 +758,36 @@ static void *mana_get_rxbuf_pre(struct mana_rxq *rxq, dma_addr_t *da)
 	return va;
 }
 
+/* RX buffers must be allocated with enough headroom for the TX path:
+ * mana_start_xmit() stores the SGE DMA mappings in struct mana_skb_head at
+ * skb->head, which is why the port advertises ndev->needed_headroom =
+ * MANA_HEADROOM.
+ *
+ * An skb that is forwarded out of a MANA port has to satisfy
+ * skb_cow(skb, LL_RESERVED_SPACE(dev) + ...) in ip_forward(), so reserve
+ * LL_RESERVED_SPACE() here rather than just MANA_HEADROOM - it rounds
+ * hard_header_len + needed_headroom up to HH_DATA_MOD and is therefore
+ * larger. Reserving less makes every forwarded packet get reallocated and
+ * copied by pskb_expand_head().
+ */
+static u32 mana_get_rxbuf_headroom(struct mana_port_context *apc)
+{
+	u32 headroom = LL_RESERVED_SPACE(apc->ndev);
+
+	if (mana_xdp_get(apc))
+		return max_t(u32, headroom, XDP_PACKET_HEADROOM);
+
+	return headroom;
+}
+
+static u32 mana_get_rxbuf_size(struct mana_port_context *apc, u32 mtu)
+{
+	u32 len = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD +
+				 mana_get_rxbuf_headroom(apc));
+
+	return ALIGN(len, MANA_RX_FRAG_ALIGNMENT);
+}
+
 static bool
 mana_use_single_rxbuf_per_page(struct mana_port_context *apc, u32 mtu)
 {
@@ -770,11 +800,16 @@ mana_use_single_rxbuf_per_page(struct mana_port_context *apc, u32 mtu)
 	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))
+	/* For xdp make sure only one packet fits per page. */
+	if (mana_xdp_get(apc))
 		return true;
 
-	return false;
+	/* Only use the page_pool fragment path when at least two buffers,
+	 * including the headroom each of them has to reserve, actually fit
+	 * into one page. Otherwise the fragment path degenerates into one
+	 * buffer per page while still paying the fragment accounting cost.
+	 */
+	return PAGE_SIZE / mana_get_rxbuf_size(apc, mtu) < 2;
 }
 
 /* Get RX buffer's data size, alloc size, XDP headroom based on MTU */
@@ -782,20 +817,19 @@ static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
 			       int mtu, u32 *datasize, u32 *alloc_size,
 			       u32 *headroom, u32 *frag_count)
 {
-	u32 len, buf_size;
+	u32 buf_size;
 
 	/* Calculate datasize first (consistent across all cases) */
 	*datasize = mtu + ETH_HLEN;
 
+	*headroom = mana_get_rxbuf_headroom(apc);
+
 	if (mana_use_single_rxbuf_per_page(apc, mtu)) {
-		if (mana_xdp_get(apc)) {
-			*headroom = XDP_PACKET_HEADROOM;
+		if (mana_xdp_get(apc))
 			*alloc_size = PAGE_SIZE;
-		} else {
-			*headroom = 0; /* no support for XDP */
+		else
 			*alloc_size = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD +
 						     *headroom);
-		}
 
 		*frag_count = 1;
 
@@ -809,11 +843,7 @@ static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
 	}
 
 	/* Standard MTU case - optimize for multiple packets per page */
-	*headroom = 0;
-
-	/* Calculate base buffer size needed */
-	len = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
-	buf_size = ALIGN(len, MANA_RX_FRAG_ALIGNMENT);
+	buf_size = mana_get_rxbuf_size(apc, mtu);
 
 	/* Calculate how many packets can fit in a page */
 	*frag_count = PAGE_SIZE / buf_size;
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: mana: reserve RX buffer headroom to fix forwarding performance
  2026-09-23 14:45 [PATCH net] net: mana: reserve RX buffer headroom to fix forwarding performance Hamza Mahfooz
@ 2026-09-27  5:14 ` Narcisa Vasile
  2026-09-27 15:27 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: Narcisa Vasile @ 2026-09-27  5:14 UTC (permalink / raw)
  To: Hamza Mahfooz
  Cc: netdev, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
	Simon Horman, Erni Sri Satya Vennela, Dipayaan Roy, Aditya Garg,
	Jacob Keller, Saurabh Sengar, linux-hyperv, bpf, linux-kernel,
	stable

On Wed, Sep 23, 2026 at 10:45:00AM -0400, Hamza Mahfooz wrote:
> Commit 730ff06d3f5c ("net: mana: Use page pool fragments for RX buffers
> instead of full pages to improve memory efficiency.") started handing
> out RX buffers with zero headroom so that two buffers fit into one page
> at the default MTU.
> 
> The MANA TX path, however, stores the per scatter-gather entry DMA
> mappings in `struct mana_skb_head` at skb->head, and mana_start_xmit()
> therefore calls skb_cow_head(skb, MANA_HEADROOM). The port advertises
> this requirement as ndev->needed_headroom = MANA_HEADROOM.
> 
> As a result every packet that is received and then forwarded out of a
> MANA port fails the skb_cow() in ip_forward() and gets reallocated and
> copied by pskb_expand_head(). This is invisible to a plain RX or TX
> workload, but it puts a full skb reallocation plus memcpy on the hot
> path of every single forwarded packet, which is exactly what a
> router/NVA workload does.
> 
> Restore the headroom. Note that reserving MANA_HEADROOM (232) is not
> enough: ip_forward() asks for LL_RESERVED_SPACE(dev), which rounds
> hard_header_len + needed_headroom up to HH_DATA_MOD and is 256 bytes on
> ethernet. Use LL_RESERVED_SPACE() directly so the value keeps tracking
> both constants.
> 
> At the default MTU on a 4K page this means a buffer no longer fits twice
> into a page (SKB_DATA_ALIGN(1500 + MANA_RXBUF_PAD + 256) = 2112), so the
> frag-vs-single decision is now made by computing the real buffer size
> instead of comparing the MTU against PAGE_SIZE / 2. The page_pool
> fragment path is still used wherever at least two buffers genuinely fit,
> e.g. on 16K and 64K page sizes.
> 
> Measured on an Azure VM with a MANA NIC acting as a forwarding NVA (UDP,
> 1400 byte payload, 4 streams, 8 Gbps offered, only the forwarding
> node's kernel differs), 8 runs each, median:
> 
>                   forwarded pps    throughput
>   before              272,830       3.06 Gbps
>   after               390,560       4.37 Gbps   (+43%)
> 
> perf on the forwarding node, same workload:
> 
>                   memset_orig   __pi_memcpy   pskb_expand_head
>   before             10.07%         3.96%         present
>   after               0.94%         0.64%         gone
> 
> Cc: stable@vger.kernel.org
> Fixes: 730ff06d3f5c ("net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.")
> Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> ---
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 58 ++++++++++++++-----
>  1 file changed, 44 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 591fb4191d90d..e3f3b33ba9062 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -758,6 +758,36 @@ static void *mana_get_rxbuf_pre(struct mana_rxq *rxq, dma_addr_t *da)
>  	return va;
>  }
>  
> +/* RX buffers must be allocated with enough headroom for the TX path:
> + * mana_start_xmit() stores the SGE DMA mappings in struct mana_skb_head at
> + * skb->head, which is why the port advertises ndev->needed_headroom =
> + * MANA_HEADROOM.
> + *
> + * An skb that is forwarded out of a MANA port has to satisfy
> + * skb_cow(skb, LL_RESERVED_SPACE(dev) + ...) in ip_forward(), so reserve
> + * LL_RESERVED_SPACE() here rather than just MANA_HEADROOM - it rounds
> + * hard_header_len + needed_headroom up to HH_DATA_MOD and is therefore
> + * larger. Reserving less makes every forwarded packet get reallocated and
> + * copied by pskb_expand_head().
> + */

nit: maybe trim this comment to make it easier to read. For example, 
"Reserve enough headroom to satisfy the skb_cow() call in ip_forward()
and avoid reallocation."

> +static u32 mana_get_rxbuf_headroom(struct mana_port_context *apc)
> +{
> +	u32 headroom = LL_RESERVED_SPACE(apc->ndev);
> +
> +	if (mana_xdp_get(apc))
> +		return max_t(u32, headroom, XDP_PACKET_HEADROOM);
> +
This implies that the headroom could now be greater than XDP_PACKET_HEADROOM,
in XDP case. Should we then use the actual headroom value, instead
of assuming XDP_PACKET_HEADROOM, in mana_run_xdp() when preparing the buffer:

  line 94:	xdp_prepare_buff(xdp, buf_va, XDP_PACKET_HEADROOM, pkt_len, true);

Does MANA_XDP_MTU_MAX need to be updated?

> +	return headroom;
> +}
> +

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: mana: reserve RX buffer headroom to fix forwarding performance
  2026-09-23 14:45 [PATCH net] net: mana: reserve RX buffer headroom to fix forwarding performance Hamza Mahfooz
  2026-09-27  5:14 ` Narcisa Vasile
@ 2026-09-27 15:27 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 15:27 UTC (permalink / raw)
  To: hamzamahfooz
  Cc: netdev, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
	edumazet, kuba, pabeni, ast, daniel, hawk, john.fastabend, sdf,
	horms, ernis, dipayanroy, gargaditya, jacob.e.keller, ssengar,
	linux-hyperv, bpf, linux-kernel, stable

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] With XDP attached, the patch sets rxq->headroom to max_t(u32,
  LL_RESERVED_SPACE(ndev), XDP_PACKET_HEADROOM) in…
- [Low] mana_set_priv_flags() (mana_ethtool.c) keeps its own copy of the
  old rule for when one buffer per page is forced anyway: `ndev->mtu +…
- [Low] The comment above mana_get_rxbuf_cfg() still reads "Get RX
  buffer's data size, alloc size, XDP headroom based on MTU".

--

net: mana: reserve RX buffer headroom to fix forwarding performance

This patch reserves LL_RESERVED_SPACE(ndev) of headroom in MANA RX buffers,
so forwarded packets no longer have to be reallocated by
pskb_expand_head() to satisfy skb_cow() in ip_forward(). With XDP attached,
the headroom is the larger of that value and XDP_PACKET_HEADROOM. The
frag-vs-single-buffer decision now uses the computed buffer size instead of
comparing the MTU against PAGE_SIZE / 2.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 591fb4191d90d..e3f3b33ba9062 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -758,6 +758,36 @@ static void *mana_get_rxbuf_pre(struct mana_rxq *rxq, dma_addr_t *da)

[ ... ]

> +static u32 mana_get_rxbuf_headroom(struct mana_port_context *apc)
> +{
> +	u32 headroom = LL_RESERVED_SPACE(apc->ndev);
> +
> +	if (mana_xdp_get(apc))
> +		return max_t(u32, headroom, XDP_PACKET_HEADROOM);

[Severity: High]
With XDP attached, can rxq->headroom now be larger than
XDP_PACKET_HEADROOM while the XDP path still assumes 256?

MANA_HEADROOM is sizeof(struct mana_skb_head), and that size grows with
MAX_SKB_FRAGS. CONFIG_MAX_SKB_FRAGS can be anywhere from 17 to 45. With
17 or 18 frags, LL_RESERVED_SPACE(ndev) is 256. With 19 to 45 frags it
is 272 to 592, and then max_t() returns that value instead of 256.

The NIC writes each frame at va + rxq->headroom in mana_get_rxfrag():

    *da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
                         DMA_FROM_DEVICE);

However, mana_run_xdp() in mana_bpf.c still does:

    xdp_prepare_buff(xdp, buf_va, XDP_PACKET_HEADROOM, pkt_len, true);

On XDP_PASS, mana_build_skb() then reserves
xdp->data - xdp->data_hard_start, which is 256 rather than
rxq->headroom:

    skb_reserve(skb, xdp->data - xdp->data_hard_start);

In those configs, would the XDP program, the XDP_PASS skb, and the
XDP_TX/REDIRECT/AF_XDP frames all start (headroom - 256) bytes before
the real frame? The leading bytes would be stale page_pool contents, and
the tail of the frame would be cut off.

There is a related question about the MTU limit in include/net/mana/mana.h:

    #define MANA_XDP_MTU_MAX (PAGE_SIZE - MANA_RXBUF_PAD - XDP_PACKET_HEADROOM)

This still budgets 256 bytes of headroom, while the XDP branch of
mana_get_rxbuf_cfg() keeps *alloc_size = PAGE_SIZE. mana_xdp_set() uses
MANA_XDP_MTU_MAX both for the MTU check and to set max_mtu.

At the largest accepted XDP MTU, does the DMA window now run
(headroom - 256) bytes into the skb_shared_info area that
napi_build_skb() initializes afterwards? If so, fixing only the
xdp_prepare_buff() offset would leave this limit wrong.

Before this patch the XDP headroom was always exactly
XDP_PACKET_HEADROOM, so these values always matched.

> +
> +	return headroom;
> +}
> +
> +static u32 mana_get_rxbuf_size(struct mana_port_context *apc, u32 mtu)
> +{
> +	u32 len = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD +
> +				 mana_get_rxbuf_headroom(apc));
> +
> +	return ALIGN(len, MANA_RX_FRAG_ALIGNMENT);
> +}
> +
>  static bool
>  mana_use_single_rxbuf_per_page(struct mana_port_context *apc, u32 mtu)
>  {
> @@ -770,11 +800,16 @@ mana_use_single_rxbuf_per_page(struct mana_port_context *apc, u32 mtu)
>  	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))
> +	/* For xdp make sure only one packet fits per page. */
> +	if (mana_xdp_get(apc))
>  		return true;
>  
> -	return false;
> +	/* Only use the page_pool fragment path when at least two buffers,
> +	 * including the headroom each of them has to reserve, actually fit
> +	 * into one page. Otherwise the fragment path degenerates into one
> +	 * buffer per page while still paying the fragment accounting cost.
> +	 */
> +	return PAGE_SIZE / mana_get_rxbuf_size(apc, mtu) < 2;

[Severity: Low]
Should mana_set_priv_flags() in mana_ethtool.c use this same rule? It
still has its own copy of the old one:

		if (ndev->mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 ||
		    mana_xdp_get(apc))
			return 0;

The comment above that check says single-buffer-per-page "is already
forced regardless of this flag", so the detach/attach cycle can be
skipped "since nothing changes".

On 4K pages with an MTU of about 1459 to 1714, which includes the default
1500, the old check is false. The new rule here already forces one buffer
per page in that range (buf_size is 2112 at MTU 1500).

Toggling the full-page-rx private flag at those MTUs would still run
mana_pre_alloc_rxbufs(), mana_detach() and mana_attach(), even though the
RX layout doesn't change. The mismatch only goes this way: a
reconfiguration that is actually needed is never skipped.

>  }
>  
>  /* Get RX buffer's data size, alloc size, XDP headroom based on MTU */

[Severity: Low]
This isn't a bug, but this comment still describes the headroom output as
"XDP headroom". After this patch, *headroom is always set from
mana_get_rxbuf_headroom(). Without XDP, that is LL_RESERVED_SPACE(ndev),
reserved for the TX-path struct mana_skb_head.

[ ... ]

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-27 15:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 14:45 [PATCH net] net: mana: reserve RX buffer headroom to fix forwarding performance Hamza Mahfooz
2026-09-27  5:14 ` Narcisa Vasile
2026-09-27 15:27 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®