Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3] gve: leave refcount headroom in the RX page bias
@ 2026-11-03  9:06 Amaan Cheval
  2026-11-04 15:46 ` Maciej Fijalkowski
  2026-11-04 21:09 ` netdev-bot+sashiko
  0 siblings, 2 replies; 3+ messages in thread
From: Amaan Cheval @ 2026-11-03  9:06 UTC (permalink / raw)
  To: netdev
  Cc: Amaan Cheval, joshwash, hramamurthy, andrew+netdev, davem,
	edumazet, kuba, pabeni, bcf, jordanrhee, przemyslaw.kitszel,
	pkaligineedi, maolson, csully, willemb, yangchun, npl, awogbemila,
	shailend, nktgrg, maxyuan, bpf, john.fastabend, hawk, sdf, daniel,
	ast, linux-kernel, stable

gve's GQI and DQO-QPL RX paths set every RX page's refcount to INT_MAX and
hand references to the stack by decrementing page_info->pagecnt_bias, to
avoid an atomic op per packet. This leaves the page no headroom: while a
fresh page's buffer sits in an skb its refcount is already INT_MAX, so a
single get_page() from the stack wraps it negative.

splice() of received TCP data into a pipe takes such a reference, and the
onward splice into a socket then fails: skb_splice_from_iter() sees
page_count() < 1, sendpage_ok() fails and the send returns -EIO (since
commit 2e910b95329c ("net: Add a function to splice pages into an
skbuff for MSG_SPLICE_PAGES")). tee() of the pipe fails with -EFAULT via
try_get_page(). On GQI the reuse check gve_rx_can_recycle_buffer() also
sees page_count() < pagecnt_bias, WARNs and calls gve_schedule_reset(),
so a task that merely pins a fresh RX page (e.g. holds spliced data in
an undrained tee() pipe) can reset the NIC.

Use USHRT_MAX in a u16, as igb, igc, ixgbe, i40e and hns3 do: the bias
only has to make the per-packet atomic rare, so a small value leaves
page_count() ample headroom below INT_MAX for the stack's references.
Refill by adding a fixed batch and setting the bias to it, which keeps
the rule `references = page_count() - bias` intact.

Reproduced on gVNIC in GQI-QPL mode (GCE n2-standard-8, 6.12) and in
DQO-RDA mode (GCE c3-standard-8, 6.12, where DQO-RDA still uses the
bias) with the help of relay.py -- a small splice(2) TCP proxy --
relaying a download while the RX ring is refilled with fresh pages:

  ethtool -G eth0 rx 512 && ethtool -G eth0 rx 1024   # fresh RX pages
  ./relay.py $HOST 8080 &                             # splice()s both ways
  for i in $(seq 200); do curl -so /dev/null \
      --connect-to $HOST:80:127.0.0.1:8080 http://$HOST/file; done

Most downloads failed with EIO before the fix (tee() with EFAULT) and
all completed after it. relay.py is at the Link below.

This bug was found and fixed with the help of an AI coding assistant
(Claude Code) over an interactive session; it read the driver and
splice/pipe code and ran the tests above.

Link: https://github.com/amaanc/kernel-bias
Fixes: 9b8dd5e5ea48 ("gve: DQO: Add RX path")
Fixes: 58401b2a46e7 ("gve: Add rx buffer pagecnt bias")
Fixes: 82fd151d38d9 ("gve: Reduce alloc and copy costs in the GQ rx path")
Cc: stable@vger.kernel.org
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Tested-by: Harshitha Ramamurthy <hramamurthy@google.com>
Signed-off-by: Amaan Cheval <amaan.cheval@gmail.com>
---
v3:
 - Refill when the bias reaches 0 again, as in v1. DQO-QPL is not
   supposed to support XDP at all, and it will be blocked explicitly in
   a separate patch, so the v2 refill-at-1 change is not needed.
 - Added Reviewed-by and Tested-by from Harshitha Ramamurthy.
v2: https://lore.kernel.org/netdev/20261102112613.18334-1-amaan.cheval@gmail.com/
 - Refill when the bias reaches 1 instead of 0 so the driver keeps a
   reference of its own across every handoff. At bias 0 a consumer that
   returns its reference synchronously, such as an AF_XDP socket in copy
   mode on an XDP_REDIRECT from DQO-QPL, freed a page still registered in
   the QPL.
 - Commit message: explain the refill point, drop the build notes.
 - Re-tested on 6.12.113 (c3-standard-8, DQO-RDA, which still uses the
   bias on 6.12): no EIO/EFAULT from splice/tee and no NIC reset after
   applying the patch. DQO-QPL is untested; I have no access to that
   hardware.
v1: https://lore.kernel.org/netdev/20261031173449.2439047-1-amaan.cheval@gmail.com/

Built allmodconfig W=1, no new warnings.

 drivers/net/ethernet/google/gve/gve.h                |  2 +-
 .../net/ethernet/google/gve/gve_buffer_mgmt_dqo.c    |  4 ++--
 drivers/net/ethernet/google/gve/gve_rx.c             |  8 ++++----
 drivers/net/ethernet/google/gve/gve_utils.c          | 12 +++---------
 drivers/net/ethernet/google/gve/gve_utils.h          |  8 +++++++-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
index b7ce6e8632db..18d7edbf4b3b 100644
--- a/drivers/net/ethernet/google/gve/gve.h
+++ b/drivers/net/ethernet/google/gve/gve.h
@@ -119,7 +119,7 @@ struct gve_rx_slot_page_info {
 	void *page_address;
 	u32 page_offset; /* offset to write to in page */
 	unsigned int buf_size;
-	int pagecnt_bias; /* expected pagecnt if only the driver has a ref */
+	u16 pagecnt_bias; /* expected pagecnt if only the driver has a ref */
 	u16 pad; /* adjustment for rx padding */
 	u8 can_flip; /* tracks if the networking stack is using the page */
 };
diff --git a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
index 6880d1531ba5..571f851d6dcd 100644
--- a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
+++ b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
@@ -149,8 +149,8 @@ int gve_alloc_qpl_page_dqo(struct gve_rx_ring *rx,
 	buf_state->last_single_ref_offset = 0;
 
 	/* The page already has 1 ref. */
-	page_ref_add(buf_state->page_info.page, INT_MAX - 1);
-	buf_state->page_info.pagecnt_bias = INT_MAX;
+	page_ref_add(buf_state->page_info.page, GVE_PAGECNT_BIAS - 1);
+	buf_state->page_info.pagecnt_bias = GVE_PAGECNT_BIAS;
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/google/gve/gve_rx.c b/drivers/net/ethernet/google/gve/gve_rx.c
index 81ea800e66e9..6533fff06155 100644
--- a/drivers/net/ethernet/google/gve/gve_rx.c
+++ b/drivers/net/ethernet/google/gve/gve_rx.c
@@ -152,8 +152,8 @@ static void gve_setup_rx_buffer(struct gve_rx_ring *rx,
 	page_info->buf_size = rx->packet_buffer_size;
 	*slot_addr = cpu_to_be64(addr);
 	/* The page already has 1 ref */
-	page_ref_add(page, INT_MAX - 1);
-	page_info->pagecnt_bias = INT_MAX;
+	page_ref_add(page, GVE_PAGECNT_BIAS - 1);
+	page_info->pagecnt_bias = GVE_PAGECNT_BIAS;
 }
 
 static int gve_rx_alloc_buffer(struct gve_priv *priv, struct device *dev,
@@ -230,8 +230,8 @@ static int gve_rx_prefill_pages(struct gve_rx_ring *rx,
 			rx->qpl_copy_pool[j].buf_size = rx->packet_buffer_size;
 
 			/* The page already has 1 ref. */
-			page_ref_add(page, INT_MAX - 1);
-			rx->qpl_copy_pool[j].pagecnt_bias = INT_MAX;
+			page_ref_add(page, GVE_PAGECNT_BIAS - 1);
+			rx->qpl_copy_pool[j].pagecnt_bias = GVE_PAGECNT_BIAS;
 		}
 	}
 
diff --git a/drivers/net/ethernet/google/gve/gve_utils.c b/drivers/net/ethernet/google/gve/gve_utils.c
index caed38cd5f2b..cb6fba3122d5 100644
--- a/drivers/net/ethernet/google/gve/gve_utils.c
+++ b/drivers/net/ethernet/google/gve/gve_utils.c
@@ -93,15 +93,9 @@ void gve_dec_pagecnt_bias(struct gve_rx_slot_page_info *page_info)
 {
 	page_info->pagecnt_bias--;
 	if (page_info->pagecnt_bias == 0) {
-		int pagecount = page_count(page_info->page);
-
-		/* If we have run out of bias - set it back up to INT_MAX
-		 * minus the existing refs.
-		 */
-		page_info->pagecnt_bias = INT_MAX - pagecount;
-
-		/* Set pagecount back up to max. */
-		page_ref_add(page_info->page, INT_MAX - pagecount);
+		/* Out of bias: take another batch of references. */
+		page_info->pagecnt_bias = GVE_PAGECNT_BIAS;
+		page_ref_add(page_info->page, GVE_PAGECNT_BIAS);
 	}
 }
 
diff --git a/drivers/net/ethernet/google/gve/gve_utils.h b/drivers/net/ethernet/google/gve/gve_utils.h
index bf2e9a0adb36..316f97607351 100644
--- a/drivers/net/ethernet/google/gve/gve_utils.h
+++ b/drivers/net/ethernet/google/gve/gve_utils.h
@@ -25,7 +25,13 @@ struct sk_buff *gve_rx_copy_data(struct net_device *dev, struct napi_struct *nap
 struct sk_buff *gve_rx_copy(struct net_device *dev, struct napi_struct *napi,
 			    struct gve_rx_slot_page_info *page_info, u16 len);
 
-/* Decrement pagecnt_bias. Set it back to INT_MAX if it reached zero. */
+/* Up-front references gve holds on an RX page, so handing a buffer to the
+ * stack is a pagecnt_bias decrement, not a get_page(). Keep it in a u16 so
+ * the 32-bit page refcount keeps the headroom every page needs below INT_MAX.
+ */
+#define GVE_PAGECNT_BIAS USHRT_MAX
+
+/* Decrement pagecnt_bias. Top it up by GVE_PAGECNT_BIAS if it reached zero. */
 void gve_dec_pagecnt_bias(struct gve_rx_slot_page_info *page_info);
 
 void gve_add_napi(struct gve_priv *priv, int ntfy_idx,

base-commit: 21f57ec673a1917547c808cfa81890485fffa03b
-- 
2.47.3


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

* Re: [PATCH net v3] gve: leave refcount headroom in the RX page bias
  2026-11-03  9:06 [PATCH net v3] gve: leave refcount headroom in the RX page bias Amaan Cheval
@ 2026-11-04 15:46 ` Maciej Fijalkowski
  2026-11-04 21:09 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: Maciej Fijalkowski @ 2026-11-04 15:46 UTC (permalink / raw)
  To: Amaan Cheval
  Cc: netdev, joshwash, hramamurthy, andrew+netdev, davem, edumazet,
	kuba, pabeni, bcf, jordanrhee, przemyslaw.kitszel, pkaligineedi,
	maolson, csully, willemb, yangchun, npl, awogbemila, shailend,
	nktgrg, maxyuan, bpf, john.fastabend, hawk, sdf, daniel, ast,
	linux-kernel, stable

On Tue, Nov 03, 2026 at 02:36:48PM +0530, Amaan Cheval wrote:
> gve's GQI and DQO-QPL RX paths set every RX page's refcount to INT_MAX and
> hand references to the stack by decrementing page_info->pagecnt_bias, to
> avoid an atomic op per packet. This leaves the page no headroom: while a
> fresh page's buffer sits in an skb its refcount is already INT_MAX, so a
> single get_page() from the stack wraps it negative.

Hi Amaan,

by headroom you mean here the space for additional refcount bumps? A bit
confusing as my head tends to correlate 'headroom' with packet layout ;)

one minor comment below.

> 
> splice() of received TCP data into a pipe takes such a reference, and the
> onward splice into a socket then fails: skb_splice_from_iter() sees
> page_count() < 1, sendpage_ok() fails and the send returns -EIO (since
> commit 2e910b95329c ("net: Add a function to splice pages into an
> skbuff for MSG_SPLICE_PAGES")). tee() of the pipe fails with -EFAULT via
> try_get_page(). On GQI the reuse check gve_rx_can_recycle_buffer() also
> sees page_count() < pagecnt_bias, WARNs and calls gve_schedule_reset(),
> so a task that merely pins a fresh RX page (e.g. holds spliced data in
> an undrained tee() pipe) can reset the NIC.
> 
> Use USHRT_MAX in a u16, as igb, igc, ixgbe, i40e and hns3 do: the bias
> only has to make the per-packet atomic rare, so a small value leaves
> page_count() ample headroom below INT_MAX for the stack's references.
> Refill by adding a fixed batch and setting the bias to it, which keeps
> the rule `references = page_count() - bias` intact.
> 
> Reproduced on gVNIC in GQI-QPL mode (GCE n2-standard-8, 6.12) and in
> DQO-RDA mode (GCE c3-standard-8, 6.12, where DQO-RDA still uses the
> bias) with the help of relay.py -- a small splice(2) TCP proxy --
> relaying a download while the RX ring is refilled with fresh pages:
> 
>   ethtool -G eth0 rx 512 && ethtool -G eth0 rx 1024   # fresh RX pages
>   ./relay.py $HOST 8080 &                             # splice()s both ways
>   for i in $(seq 200); do curl -so /dev/null \
>       --connect-to $HOST:80:127.0.0.1:8080 http://$HOST/file; done
> 
> Most downloads failed with EIO before the fix (tee() with EFAULT) and
> all completed after it. relay.py is at the Link below.
> 
> This bug was found and fixed with the help of an AI coding assistant
> (Claude Code) over an interactive session; it read the driver and
> splice/pipe code and ran the tests above.
> 
> Link: https://github.com/amaanc/kernel-bias
> Fixes: 9b8dd5e5ea48 ("gve: DQO: Add RX path")
> Fixes: 58401b2a46e7 ("gve: Add rx buffer pagecnt bias")
> Fixes: 82fd151d38d9 ("gve: Reduce alloc and copy costs in the GQ rx path")
> Cc: stable@vger.kernel.org
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
> Tested-by: Harshitha Ramamurthy <hramamurthy@google.com>
> Signed-off-by: Amaan Cheval <amaan.cheval@gmail.com>
> ---
> v3:
>  - Refill when the bias reaches 0 again, as in v1. DQO-QPL is not
>    supposed to support XDP at all, and it will be blocked explicitly in
>    a separate patch, so the v2 refill-at-1 change is not needed.
>  - Added Reviewed-by and Tested-by from Harshitha Ramamurthy.
> v2: https://lore.kernel.org/netdev/20261102112613.18334-1-amaan.cheval@gmail.com/
>  - Refill when the bias reaches 1 instead of 0 so the driver keeps a
>    reference of its own across every handoff. At bias 0 a consumer that
>    returns its reference synchronously, such as an AF_XDP socket in copy
>    mode on an XDP_REDIRECT from DQO-QPL, freed a page still registered in
>    the QPL.
>  - Commit message: explain the refill point, drop the build notes.
>  - Re-tested on 6.12.113 (c3-standard-8, DQO-RDA, which still uses the
>    bias on 6.12): no EIO/EFAULT from splice/tee and no NIC reset after
>    applying the patch. DQO-QPL is untested; I have no access to that
>    hardware.
> v1: https://lore.kernel.org/netdev/20261031173449.2439047-1-amaan.cheval@gmail.com/
> 
> Built allmodconfig W=1, no new warnings.
> 
>  drivers/net/ethernet/google/gve/gve.h                |  2 +-
>  .../net/ethernet/google/gve/gve_buffer_mgmt_dqo.c    |  4 ++--
>  drivers/net/ethernet/google/gve/gve_rx.c             |  8 ++++----
>  drivers/net/ethernet/google/gve/gve_utils.c          | 12 +++---------
>  drivers/net/ethernet/google/gve/gve_utils.h          |  8 +++++++-
>  5 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
> index b7ce6e8632db..18d7edbf4b3b 100644
> --- a/drivers/net/ethernet/google/gve/gve.h
> +++ b/drivers/net/ethernet/google/gve/gve.h
> @@ -119,7 +119,7 @@ struct gve_rx_slot_page_info {
>  	void *page_address;
>  	u32 page_offset; /* offset to write to in page */
>  	unsigned int buf_size;
> -	int pagecnt_bias; /* expected pagecnt if only the driver has a ref */
> +	u16 pagecnt_bias; /* expected pagecnt if only the driver has a ref */
>  	u16 pad; /* adjustment for rx padding */
>  	u8 can_flip; /* tracks if the networking stack is using the page */
>  };
> diff --git a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> index 6880d1531ba5..571f851d6dcd 100644
> --- a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> +++ b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> @@ -149,8 +149,8 @@ int gve_alloc_qpl_page_dqo(struct gve_rx_ring *rx,
>  	buf_state->last_single_ref_offset = 0;
>  
>  	/* The page already has 1 ref. */
> -	page_ref_add(buf_state->page_info.page, INT_MAX - 1);
> -	buf_state->page_info.pagecnt_bias = INT_MAX;
> +	page_ref_add(buf_state->page_info.page, GVE_PAGECNT_BIAS - 1);
> +	buf_state->page_info.pagecnt_bias = GVE_PAGECNT_BIAS;
>  
>  	return 0;
>  }
> diff --git a/drivers/net/ethernet/google/gve/gve_rx.c b/drivers/net/ethernet/google/gve/gve_rx.c
> index 81ea800e66e9..6533fff06155 100644
> --- a/drivers/net/ethernet/google/gve/gve_rx.c
> +++ b/drivers/net/ethernet/google/gve/gve_rx.c
> @@ -152,8 +152,8 @@ static void gve_setup_rx_buffer(struct gve_rx_ring *rx,
>  	page_info->buf_size = rx->packet_buffer_size;
>  	*slot_addr = cpu_to_be64(addr);
>  	/* The page already has 1 ref */
> -	page_ref_add(page, INT_MAX - 1);
> -	page_info->pagecnt_bias = INT_MAX;
> +	page_ref_add(page, GVE_PAGECNT_BIAS - 1);
> +	page_info->pagecnt_bias = GVE_PAGECNT_BIAS;
>  }
>  
>  static int gve_rx_alloc_buffer(struct gve_priv *priv, struct device *dev,
> @@ -230,8 +230,8 @@ static int gve_rx_prefill_pages(struct gve_rx_ring *rx,
>  			rx->qpl_copy_pool[j].buf_size = rx->packet_buffer_size;
>  
>  			/* The page already has 1 ref. */
> -			page_ref_add(page, INT_MAX - 1);
> -			rx->qpl_copy_pool[j].pagecnt_bias = INT_MAX;
> +			page_ref_add(page, GVE_PAGECNT_BIAS - 1);
> +			rx->qpl_copy_pool[j].pagecnt_bias = GVE_PAGECNT_BIAS;
>  		}
>  	}
>  
> diff --git a/drivers/net/ethernet/google/gve/gve_utils.c b/drivers/net/ethernet/google/gve/gve_utils.c
> index caed38cd5f2b..cb6fba3122d5 100644
> --- a/drivers/net/ethernet/google/gve/gve_utils.c
> +++ b/drivers/net/ethernet/google/gve/gve_utils.c
> @@ -93,15 +93,9 @@ void gve_dec_pagecnt_bias(struct gve_rx_slot_page_info *page_info)
>  {
>  	page_info->pagecnt_bias--;
>  	if (page_info->pagecnt_bias == 0) {
> -		int pagecount = page_count(page_info->page);
> -
> -		/* If we have run out of bias - set it back up to INT_MAX
> -		 * minus the existing refs.
> -		 */
> -		page_info->pagecnt_bias = INT_MAX - pagecount;
> -
> -		/* Set pagecount back up to max. */
> -		page_ref_add(page_info->page, INT_MAX - pagecount);
> +		/* Out of bias: take another batch of references. */
> +		page_info->pagecnt_bias = GVE_PAGECNT_BIAS;
> +		page_ref_add(page_info->page, GVE_PAGECNT_BIAS);
>  	}
>  }
>  
> diff --git a/drivers/net/ethernet/google/gve/gve_utils.h b/drivers/net/ethernet/google/gve/gve_utils.h
> index bf2e9a0adb36..316f97607351 100644
> --- a/drivers/net/ethernet/google/gve/gve_utils.h
> +++ b/drivers/net/ethernet/google/gve/gve_utils.h
> @@ -25,7 +25,13 @@ struct sk_buff *gve_rx_copy_data(struct net_device *dev, struct napi_struct *nap
>  struct sk_buff *gve_rx_copy(struct net_device *dev, struct napi_struct *napi,
>  			    struct gve_rx_slot_page_info *page_info, u16 len);
>  
> -/* Decrement pagecnt_bias. Set it back to INT_MAX if it reached zero. */
> +/* Up-front references gve holds on an RX page, so handing a buffer to the
> + * stack is a pagecnt_bias decrement, not a get_page(). Keep it in a u16 so
> + * the 32-bit page refcount keeps the headroom every page needs below INT_MAX.
> + */
> +#define GVE_PAGECNT_BIAS USHRT_MAX

IMHO code would be more readable with plain USHRT_MAX usage then you know
immediately what value is being used. Other drivers do so at least.

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> +
> +/* Decrement pagecnt_bias. Top it up by GVE_PAGECNT_BIAS if it reached zero. */
>  void gve_dec_pagecnt_bias(struct gve_rx_slot_page_info *page_info);
>  
>  void gve_add_napi(struct gve_priv *priv, int ntfy_idx,
> 
> base-commit: 21f57ec673a1917547c808cfa81890485fffa03b
> -- 
> 2.47.3
> 
> 

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

* Re: [PATCH net v3] gve: leave refcount headroom in the RX page bias
  2026-11-03  9:06 [PATCH net v3] gve: leave refcount headroom in the RX page bias Amaan Cheval
  2026-11-04 15:46 ` Maciej Fijalkowski
@ 2026-11-04 21:09 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-11-04 21:09 UTC (permalink / raw)
  To: amaan.cheval
  Cc: netdev, joshwash, hramamurthy, andrew+netdev, davem, edumazet,
	kuba, pabeni, bcf, jordanrhee, przemyslaw.kitszel, pkaligineedi,
	maolson, csully, willemb, yangchun, npl, awogbemila, shailend,
	nktgrg, maxyuan, bpf, john.fastabend, hawk, sdf, daniel, ast,
	linux-kernel, stable

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

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

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

- [Critical] This patch refills the bias only after
  gve_dec_pagecnt_bias() reaches 0, and it lowers the per-page bias from…

--

gve: leave refcount headroom in the RX page bias

This patch lowers the gve RX page refcount bias from INT_MAX to USHRT_MAX
and stores it in a u16. This leaves page_count() room for references the
stack takes with get_page(), such as splice() and tee(). When the bias
reaches 0, it is refilled with a fixed batch of GVE_PAGECNT_BIAS.

Link: https://github.com/amaanc/kernel-bias

> diff --git a/drivers/net/ethernet/google/gve/gve_utils.c b/drivers/net/ethernet/google/gve/gve_utils.c
> index caed38cd5f2b3..cb6fba3122d5b 100644
> --- a/drivers/net/ethernet/google/gve/gve_utils.c
> +++ b/drivers/net/ethernet/google/gve/gve_utils.c
> @@ -93,15 +93,9 @@ void gve_dec_pagecnt_bias(struct gve_rx_slot_page_info *page_info)
>  {
>  	page_info->pagecnt_bias--;
>  	if (page_info->pagecnt_bias == 0) {
> -		int pagecount = page_count(page_info->page);
> -
> -		/* If we have run out of bias - set it back up to INT_MAX
> -		 * minus the existing refs.
> -		 */
> -		page_info->pagecnt_bias = INT_MAX - pagecount;
> -
> -		/* Set pagecount back up to max. */
> -		page_ref_add(page_info->page, INT_MAX - pagecount);
> +		/* Out of bias: take another batch of references. */
> +		page_info->pagecnt_bias = GVE_PAGECNT_BIAS;
> +		page_ref_add(page_info->page, GVE_PAGECNT_BIAS);
>  	}
>  }

[Severity: Critical]

Can this free a DQO-QPL RX page that is still registered with the device?

Because the refill happens only at 0, the 1 -> 0 transition hands the
driver's last reference to the consumer. With INT_MAX a page needed about
2^31 handoffs to get there. With GVE_PAGECNT_BIAS it happens every 65534
handoffs.

On DQO-QPL, gve_xdp_done_dqo() redirects the buffer before it touches the
bias:

	case XDP_REDIRECT:
		err = xdp_do_redirect(priv->dev, xdp, xprog);
		if (unlikely(err))
			goto err;
		gve_reuse_buffer(rx, buf_state);

gve_reg_xdp_info() registers QPL rings as MEM_TYPE_PAGE_SHARED. If the
redirect target is an AF_XDP socket in copy mode, the page is released
synchronously:

xdp_do_redirect()
  xsk_rcv()
    xdp_return_buff()
      __xdp_return()
        page_frag_free()
          put_page_testzero()
          free_frozen_pages()

Suppose the page is at pagecnt_bias == 1 and page_count == 1. That is the
normal recyclable state once all earlier consumers have released their
references. The QPL page then goes back to the buddy allocator while it is
still DMA mapped, registered in the QPL and owned by the buf_state.

After that, gve_reuse_buffer()->gve_dec_pagecnt_bias() calls
page_ref_add(page, GVE_PAGECNT_BIAS) on the freed page.
gve_try_recycle_buf() then sees page_count - bias == 0 and posts the page
back to the NIC. Could the device then DMA packet data into memory that
has already been handed to another user?

The v3 notes say XDP on DQO-QPL "will be blocked explicitly in a separate
patch". In this tree, gve_xdp(), gve_verify_xdp_configuration() and
gve_set_xdp() do not check queue_format.

gve_set_netdev_xdp_features() sets xdp_features = 0 for DQO-QPL, but
nothing on the attach path enforces it. xp_assign_dev() also falls back to
copy mode when NETDEV_XDP_ACT_XSK is missing.

That note is below the --- line, so it will not be in the applied commit.
The patch is also tagged for stable without naming the prerequisite.

The commit message also says:

  Use USHRT_MAX in a u16, as igb, igc, ixgbe, i40e and hns3 do

Those drivers refill at bias == 1, so they always keep a reference. For
example, igb_can_reuse_rx_page() does:

	if (unlikely(pagecnt_bias == 1)) {
		page_ref_add(page, USHRT_MAX - 1);
		rx_buffer->pagecnt_bias = USHRT_MAX;
	}

Would it be safer to refill at 1 here, as v2 did, so the driver never
gives up its last reference on a QPL page?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103090651.35399-1-amaan.cheval%40gmail.com

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

end of thread, other threads:[~2026-11-04 21:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03  9:06 [PATCH net v3] gve: leave refcount headroom in the RX page bias Amaan Cheval
2026-11-04 15:46 ` Maciej Fijalkowski
2026-11-04 21:09 ` 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