Skip to contentSkip to conversation

gtp: disable BH before calling udp_tunnel_xmit_skb()

PATCH Applied David Carlier 4 participants 7 messages Apr 17 – Apr 21

Subsystems (4)
Networking Network drivers Stable branch GTP (GPRS Tunneling Protocol)
lore
Share & tools
lore

David Carlier

author
#0 lore raw

PATCH 1/1 gtp: disable BH before calling udp_tunnel_xmit_skb()

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 [email protected]
Signed-off-by David Carlier <[email protected]>
---
 drivers/net/gtp.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/gtp.c +2 -0
index 70b9e58b9b78..5150f2e4f66b 100644
@@ -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;
 }
Signature · 1 line
2.53.0
#1 lore raw
Quoted 11 lines · David Carlier

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.

Quoted 7 lines

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.

author
#2 lore raw

Hi Julian,

Quoted 17 lines · Justin Iurman
Quoted 15 lines · David Carlier

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.

Quoted 8 lines
Quoted 3 lines

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 !

#3 lore raw
Quoted 34 lines · David CARLIER

Hi Julian,

Quoted 17 lines · Justin Iurman
Quoted 15 lines · David Carlier

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.

Quoted 8 lines
Quoted 3 lines

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

Quoted 5 lines

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.

Jakub merged it already, so no need to respin. I guess we could revisit later if required.

Quoted 4 lines

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.

Since udp_tunnel_xmit_skb() is just another caller, I'd definitely do it in iptunnel_xmit() to centralize things (same for v6).

maintainer
#4 lore raw
In reply to Justin Iurman
Quoted 16 lines · On Mon, 20 Apr 2026 21:02:55 +0200 Justin Iurman wrote:
Quoted 15 lines · David Carlier

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.

Quoted 8 lines
Quoted 3 lines

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?

#5 lore raw
Quoted 21 lines · Jakub Kicinski
Quoted 20 lines · On Mon, 20 Apr 2026 21:02:55 +0200 Justin Iurman wrote:
Quoted 18 lines · David Carlier

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

Quoted 7 lines
Quoted 4 lines

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?

No worries, works for me!

CI Applied
#6 lore raw
In reply to David Carlier
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 5 lines · 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.

Quoted 7 lines

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!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html