public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Chris Arges <carges@cloudflare.com>
To: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: bpf@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	dtatulea@nvidia.com, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <borkmann@iogearbox.net>,
	netdev@vger.kernel.org, Eric Dumazet <eric.dumazet@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	tariqt@nvidia.com, memxor@gmail.com, john.fastabend@gmail.com,
	kernel-team@cloudflare.com, yan@cloudflare.com,
	jbrandeburg@cloudflare.com, arzeznik@cloudflare.com
Subject: Re: [PATCH bpf] cpumap: disable page_pool direct xdp_return need larger scope
Date: Thu, 14 Aug 2025 15:56:27 -0500	[thread overview]
Message-ID: <aJ5Ne4U2nLY5DmCL@861G6M3> (raw)
In-Reply-To: <175519587755.3008742.1088294435150406835.stgit@firesoul>

On 2025-08-14 20:24:37, Jesper Dangaard Brouer wrote:
> When running an XDP bpf_prog on the remote CPU in cpumap code
> then we must disable the direct return optimization that
> xdp_return can perform for mem_type page_pool.  This optimization
> assumes code is still executing under RX-NAPI of the original
> receiving CPU, which isn't true on this remote CPU.
> 
> The cpumap code already disabled this via helpers
> xdp_set_return_frame_no_direct() and xdp_clear_return_frame_no_direct(),
> but the scope didn't include xdp_do_flush().
> 
> When doing XDP_REDIRECT towards e.g devmap this causes the
> function bq_xmit_all() to run with direct return optimization
> enabled. This can lead to hard to find bugs.  The issue
> only happens when bq_xmit_all() cannot ndo_xdp_xmit all
> frames and them frees them via xdp_return_frame_rx_napi().
> 
> Fix by expanding scope to include xdp_do_flush().
> 
> Fixes: 11941f8a8536 ("bpf: cpumap: Implement generic cpumap")
> Found-by: Dragos Tatulea <dtatulea@nvidia.com>
> Reported-by: Chris Arges <carges@cloudflare.com>
> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
> ---
>  kernel/bpf/cpumap.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index b2b7b8ec2c2a..c46360b27871 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -186,7 +186,6 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu,
>  	struct xdp_buff xdp;
>  	int i, nframes = 0;
>  
> -	xdp_set_return_frame_no_direct();
>  	xdp.rxq = &rxq;
>  
>  	for (i = 0; i < n; i++) {
> @@ -231,7 +230,6 @@ static int cpu_map_bpf_prog_run_xdp(struct bpf_cpu_map_entry *rcpu,
>  		}
>  	}
>  
> -	xdp_clear_return_frame_no_direct();
>  	stats->pass += nframes;
>  
>  	return nframes;
> @@ -255,6 +253,7 @@ static void cpu_map_bpf_prog_run(struct bpf_cpu_map_entry *rcpu, void **frames,
>  
>  	rcu_read_lock();
>  	bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);
> +	xdp_set_return_frame_no_direct();
>  
>  	ret->xdp_n = cpu_map_bpf_prog_run_xdp(rcpu, frames, ret->xdp_n, stats);
>  	if (unlikely(ret->skb_n))
> @@ -264,6 +263,7 @@ static void cpu_map_bpf_prog_run(struct bpf_cpu_map_entry *rcpu, void **frames,
>  	if (stats->redirect)
>  		xdp_do_flush();
>  
> +	xdp_clear_return_frame_no_direct();
>  	bpf_net_ctx_clear(bpf_net_ctx);
>  	rcu_read_unlock();
>  
> 
>

FWIW, I tested this patch and could no longer reproduce the original issue.

Tested-By: Chris Arges <carges@cloudflare.com>

--chris

  reply	other threads:[~2025-08-14 20:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-14 18:24 [PATCH bpf] cpumap: disable page_pool direct xdp_return need larger scope Jesper Dangaard Brouer
2025-08-14 20:56 ` Chris Arges [this message]
2025-08-15  4:50 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJ5Ne4U2nLY5DmCL@861G6M3 \
    --to=carges@cloudflare.com \
    --cc=arzeznik@cloudflare.com \
    --cc=ast@kernel.org \
    --cc=borkmann@iogearbox.net \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=dtatulea@nvidia.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hawk@kernel.org \
    --cc=jbrandeburg@cloudflare.com \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@cloudflare.com \
    --cc=kuba@kernel.org \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=tariqt@nvidia.com \
    --cc=yan@cloudflare.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox