public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
@ 2026-04-17  5:54 David Carlier
  2026-04-20 19:02 ` Justin Iurman
  2026-04-20 21:59 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 5+ messages in thread
From: David Carlier @ 2026-04-17  5:54 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Harald Welte, Andrew Lunn, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Weiming Shi, osmocom-net-gprs, netdev, linux-kernel,
	David Carlier, stable

gtp_genl_send_echo_req() runs as a generic netlink doit handler in
process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
on softnet_data.xmit.recursion to track the tunnel xmit recursion level.

Without local_bh_disable(), the task may migrate between
dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
per-CPU counter pairing. The result is stale or negative recursion
levels that can later produce false-positive
SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.

The other udp_tunnel_xmit_skb() call sites in gtp.c are unaffected:
the data path runs under ndo_start_xmit and the echo response handlers
run from the UDP encap rx softirq, both with BH already disabled.

Fix it by disabling BH around the udp_tunnel_xmit_skb() call, mirroring
commit 2cd7e6971fc2 ("sctp: disable BH before calling
udp_tunnel_xmit_skb()").

Fixes: 6f1a9140ecda ("net: add xmit recursion limit to tunnel xmit functions")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 drivers/net/gtp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 70b9e58b9b78..5150f2e4f66b 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -2400,6 +2400,7 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 		return -ENODEV;
 	}
 
+	local_bh_disable();
 	udp_tunnel_xmit_skb(rt, sk, skb_to_send,
 			    fl4.saddr, fl4.daddr,
 			    inet_dscp_to_dsfield(fl4.flowi4_dscp),
@@ -2409,6 +2410,7 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 			    !net_eq(sock_net(sk),
 				    dev_net(gtp->dev)),
 			    false, 0);
+	local_bh_enable();
 	return 0;
 }
 
-- 
2.53.0


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

* Re: [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
  2026-04-17  5:54 [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb() David Carlier
@ 2026-04-20 19:02 ` Justin Iurman
  2026-04-20 19:44   ` David CARLIER
  2026-04-20 19:58   ` Jakub Kicinski
  2026-04-20 21:59 ` patchwork-bot+netdevbpf
  1 sibling, 2 replies; 5+ messages in thread
From: Justin Iurman @ 2026-04-20 19:02 UTC (permalink / raw)
  To: David Carlier, Pablo Neira Ayuso, Harald Welte, Andrew Lunn,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Weiming Shi, osmocom-net-gprs, netdev, linux-kernel, stable

On 4/17/26 07:54, David Carlier wrote:
> gtp_genl_send_echo_req() runs as a generic netlink doit handler in
> process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
> which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
> on softnet_data.xmit.recursion to track the tunnel xmit recursion level.
> 
> Without local_bh_disable(), the task may migrate between
> dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
> per-CPU counter pairing. The result is stale or negative recursion
> levels that can later produce false-positive
> SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.
> 
> The other udp_tunnel_xmit_skb() call sites in gtp.c are unaffected:
> the data path runs under ndo_start_xmit and the echo response handlers
> run from the UDP encap rx softirq, both with BH already disabled.
> 
> Fix it by disabling BH around the udp_tunnel_xmit_skb() call, mirroring
> commit 2cd7e6971fc2 ("sctp: disable BH before calling
> udp_tunnel_xmit_skb()").

Why not fix iptunnel_xmit() directly, rather than fixing all possible 
callers? Basically, jut like we did for lwtunnel_{output|xmit}(). The 
advantage would be that we no longer have to worry about BHs in the 
callers, and BHs would only be disabled when necessary.

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

* Re: [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
  2026-04-20 19:02 ` Justin Iurman
@ 2026-04-20 19:44   ` David CARLIER
  2026-04-20 19:58   ` Jakub Kicinski
  1 sibling, 0 replies; 5+ messages in thread
From: David CARLIER @ 2026-04-20 19:44 UTC (permalink / raw)
  To: Justin Iurman
  Cc: Pablo Neira Ayuso, Harald Welte, Andrew Lunn, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Weiming Shi, osmocom-net-gprs,
	netdev, linux-kernel, stable

Hi Julian,

On Mon, 20 Apr 2026 at 20:02, Justin Iurman <justin.iurman@gmail.com> wrote:
>
> On 4/17/26 07:54, David Carlier wrote:
> > gtp_genl_send_echo_req() runs as a generic netlink doit handler in
> > process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
> > which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
> > on softnet_data.xmit.recursion to track the tunnel xmit recursion level.
> >
> > Without local_bh_disable(), the task may migrate between
> > dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
> > per-CPU counter pairing. The result is stale or negative recursion
> > levels that can later produce false-positive
> > SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.
> >
> > The other udp_tunnel_xmit_skb() call sites in gtp.c are unaffected:
> > the data path runs under ndo_start_xmit and the echo response handlers
> > run from the UDP encap rx softirq, both with BH already disabled.
> >
> > Fix it by disabling BH around the udp_tunnel_xmit_skb() call, mirroring
> > commit 2cd7e6971fc2 ("sctp: disable BH before calling
> > udp_tunnel_xmit_skb()").
>
> Why not fix iptunnel_xmit() directly, rather than fixing all possible
> callers? Basically, jut like we did for lwtunnel_{output|xmit}(). The
> advantage would be that we no longer have to worry about BHs in the
> callers, and BHs would only be disabled when necessary.

Good point — your lwtunnel fix (c03a49f3093a) is a close parallel, and
  a central fix would avoid chasing callers one by one (sctp was patched
  last week, gtp is this one, and tipc/wireguard/ovpn genl paths look
  similar).

  Happy to respin as v2 with local_bh_disable/enable moved into
  iptunnel_xmit() (and ip6tunnel_xmit() for symmetry), and drop the
  gtp-local hunk. That would also supersede Xin Long's recent sctp
commit
  (2cd7e6971fc2), so I'll make sure to Cc him.

  One thing I'd like your take on before I send: iptunnel_xmit() feels
  like the natural home since it owns the recursion counter, but would
  you rather see it in udp_tunnel_xmit_skb()? I don't want to pick the
  wrong spot if you already have a preference.

Cheers !

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

* Re: [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
  2026-04-20 19:02 ` Justin Iurman
  2026-04-20 19:44   ` David CARLIER
@ 2026-04-20 19:58   ` Jakub Kicinski
  1 sibling, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2026-04-20 19:58 UTC (permalink / raw)
  To: Justin Iurman
  Cc: David Carlier, Pablo Neira Ayuso, Harald Welte, Andrew Lunn,
	Eric Dumazet, Paolo Abeni, Weiming Shi, osmocom-net-gprs, netdev,
	linux-kernel, stable

On Mon, 20 Apr 2026 21:02:55 +0200 Justin Iurman wrote:
> On 4/17/26 07:54, David Carlier wrote:
> > gtp_genl_send_echo_req() runs as a generic netlink doit handler in
> > process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
> > which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
> > on softnet_data.xmit.recursion to track the tunnel xmit recursion level.
> > 
> > Without local_bh_disable(), the task may migrate between
> > dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
> > per-CPU counter pairing. The result is stale or negative recursion
> > levels that can later produce false-positive
> > SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.
> > 
> > The other udp_tunnel_xmit_skb() call sites in gtp.c are unaffected:
> > the data path runs under ndo_start_xmit and the echo response handlers
> > run from the UDP encap rx softirq, both with BH already disabled.
> > 
> > Fix it by disabling BH around the udp_tunnel_xmit_skb() call, mirroring
> > commit 2cd7e6971fc2 ("sctp: disable BH before calling
> > udp_tunnel_xmit_skb()").  
> 
> Why not fix iptunnel_xmit() directly, rather than fixing all possible 
> callers? Basically, jut like we did for lwtunnel_{output|xmit}(). The 
> advantage would be that we no longer have to worry about BHs in the 
> callers, and BHs would only be disabled when necessary.

Oops, I pushed this already. The bot hasn't caught up yet.
Let's revisit this if we find another caller in process context?

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

* Re: [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
  2026-04-17  5:54 [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb() David Carlier
  2026-04-20 19:02 ` Justin Iurman
@ 2026-04-20 21:59 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-04-20 21:59 UTC (permalink / raw)
  To: David CARLIER
  Cc: pablo, laforge, andrew+netdev, edumazet, kuba, pabeni, bestswngs,
	osmocom-net-gprs, netdev, linux-kernel, stable

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 17 Apr 2026 06:54:08 +0100 you wrote:
> gtp_genl_send_echo_req() runs as a generic netlink doit handler in
> process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
> which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
> on softnet_data.xmit.recursion to track the tunnel xmit recursion level.
> 
> Without local_bh_disable(), the task may migrate between
> dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
> per-CPU counter pairing. The result is stale or negative recursion
> levels that can later produce false-positive
> SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.
> 
> [...]

Here is the summary with links:
  - gtp: disable BH before calling udp_tunnel_xmit_skb()
    https://git.kernel.org/netdev/net/c/5638504a2aa9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-04-20 22:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17  5:54 [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb() David Carlier
2026-04-20 19:02 ` Justin Iurman
2026-04-20 19:44   ` David CARLIER
2026-04-20 19:58   ` Jakub Kicinski
2026-04-20 21:59 ` patchwork-bot+netdevbpf

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