[PATCH net v3] net: lwtunnel: disable BHs when required

[PATCH net v3] net: lwtunnel: disable BHs when required

[Justin Iurman]

v3:
- removed unrelated code cleanups
v2:
- https://lore.kernel.org/netdev/20250415173239.39781-1-justin.iurman@uliege.be/
v1:
- https://lore.kernel.org/netdev/20250403083956.13946-1-justin.iurman@uliege.be/

In lwtunnel_{output|xmit}(), dev_xmit_recursion() may be called in
preemptible scope for PREEMPT kernels. This patch disables BHs before
calling dev_xmit_recursion(). BHs are re-enabled only at the end, since
we must ensure the same CPU is used for both dev_xmit_recursion_inc()
and dev_xmit_recursion_dec() (and any other recursion levels in some
cases) in order to maintain valid per-cpu counters.

Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Closes: https://lore.kernel.org/netdev/CAADnVQJFWn3dBFJtY+ci6oN1pDFL=TzCmNbRgey7MdYxt_AP2g@mail.gmail.com/
Reported-by: Eduard Zingerman <eddyz87@gmail.com>
Closes: https://lore.kernel.org/netdev/m2h62qwf34.fsf@gmail.com/
Fixes: 986ffb3a57c5 ("net: lwtunnel: fix recursion loops")
Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
---
Cc: bpf <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Stanislav Fomichev <stfomichev@gmail.com>
Cc: Sebastian Sewior <bigeasy@linutronix.de>
Cc: Andrea Mayer <andrea.mayer@uniroma2.it>
Cc: Stefano Salsano <stefano.salsano@uniroma2.it>
Cc: Paolo Lungaroni <paolo.lungaroni@uniroma2.it>
---
 net/core/lwtunnel.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index e39a459540ec..60f27cb4e54f 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -333,6 +333,8 @@ int lwtunnel_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 	struct dst_entry *dst;
 	int ret;
 
+	local_bh_disable();
+
 	if (dev_xmit_recursion()) {
 		net_crit_ratelimited("%s(): recursion limit reached on datapath\n",
 				     __func__);
@@ -348,8 +350,10 @@ int lwtunnel_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 	lwtstate = dst->lwtstate;
 
 	if (lwtstate->type == LWTUNNEL_ENCAP_NONE ||
-	    lwtstate->type > LWTUNNEL_ENCAP_MAX)
-		return 0;
+	    lwtstate->type > LWTUNNEL_ENCAP_MAX) {
+		ret = 0;
+		goto out;
+	}
 
 	ret = -EOPNOTSUPP;
 	rcu_read_lock();
@@ -364,11 +368,13 @@ int lwtunnel_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 	if (ret == -EOPNOTSUPP)
 		goto drop;
 
-	return ret;
+	goto out;
 
 drop:
 	kfree_skb(skb);
 
+out:
+	local_bh_enable();
 	return ret;
 }
 EXPORT_SYMBOL_GPL(lwtunnel_output);
@@ -380,6 +386,8 @@ int lwtunnel_xmit(struct sk_buff *skb)
 	struct dst_entry *dst;
 	int ret;
 
+	local_bh_disable();
+
 	if (dev_xmit_recursion()) {
 		net_crit_ratelimited("%s(): recursion limit reached on datapath\n",
 				     __func__);
@@ -396,8 +404,10 @@ int lwtunnel_xmit(struct sk_buff *skb)
 	lwtstate = dst->lwtstate;
 
 	if (lwtstate->type == LWTUNNEL_ENCAP_NONE ||
-	    lwtstate->type > LWTUNNEL_ENCAP_MAX)
-		return 0;
+	    lwtstate->type > LWTUNNEL_ENCAP_MAX) {
+		ret = 0;
+		goto out;
+	}
 
 	ret = -EOPNOTSUPP;
 	rcu_read_lock();
@@ -412,11 +422,13 @@ int lwtunnel_xmit(struct sk_buff *skb)
 	if (ret == -EOPNOTSUPP)
 		goto drop;
 
-	return ret;
+	goto out;
 
 drop:
 	kfree_skb(skb);
 
+out:
+	local_bh_enable();
 	return ret;
 }
 EXPORT_SYMBOL_GPL(lwtunnel_xmit);
@@ -428,6 +440,8 @@ int lwtunnel_input(struct sk_buff *skb)
 	struct dst_entry *dst;
 	int ret;
 
+	DEBUG_NET_WARN_ON_ONCE(!in_softirq());
+
 	if (dev_xmit_recursion()) {
 		net_crit_ratelimited("%s(): recursion limit reached on datapath\n",
 				     __func__);
-- 
2.34.1

Re: [PATCH net v3] net: lwtunnel: disable BHs when required

[Simon Horman]

On Wed, Apr 16, 2025 at 06:07:16PM +0200, Justin Iurman wrote:
> v3:
> - removed unrelated code cleanups
> v2:
> - https://lore.kernel.org/netdev/20250415173239.39781-1-justin.iurman@uliege.be/
> v1:
> - https://lore.kernel.org/netdev/20250403083956.13946-1-justin.iurman@uliege.be/

Hi Justin,

There is probably no need to resend because of this,
but the changelog above belongs below the scissors ("---")
which will preserve it in mailing list archives and so on,
while excluding it from git history.

> 
> In lwtunnel_{output|xmit}(), dev_xmit_recursion() may be called in
> preemptible scope for PREEMPT kernels. This patch disables BHs before
> calling dev_xmit_recursion(). BHs are re-enabled only at the end, since
> we must ensure the same CPU is used for both dev_xmit_recursion_inc()
> and dev_xmit_recursion_dec() (and any other recursion levels in some
> cases) in order to maintain valid per-cpu counters.
> 
> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Closes: https://lore.kernel.org/netdev/CAADnVQJFWn3dBFJtY+ci6oN1pDFL=TzCmNbRgey7MdYxt_AP2g@mail.gmail.com/
> Reported-by: Eduard Zingerman <eddyz87@gmail.com>
> Closes: https://lore.kernel.org/netdev/m2h62qwf34.fsf@gmail.com/
> Fixes: 986ffb3a57c5 ("net: lwtunnel: fix recursion loops")
> Signed-off-by: Justin Iurman <justin.iurman@uliege.be>

I agree that this patch is in keeping with the solution discussed
at the links above.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v3] net: lwtunnel: disable BHs when required

[Jakub Kicinski]

On Wed, 16 Apr 2025 18:07:16 +0200 Justin Iurman wrote:
> In lwtunnel_{output|xmit}(), dev_xmit_recursion() may be called in
> preemptible scope for PREEMPT kernels. This patch disables BHs before
> calling dev_xmit_recursion(). BHs are re-enabled only at the end, since
> we must ensure the same CPU is used for both dev_xmit_recursion_inc()
> and dev_xmit_recursion_dec() (and any other recursion levels in some
> cases) in order to maintain valid per-cpu counters.

Applied last night by Paolo, thanks!