[PATCH net 0/3] pull request: fixes for ovpn 2026-05-04

[PATCH net 0/3] pull request: fixes for ovpn 2026-05-04

[Antonio Quartulli]

Hello netdev team,

Here is a respin of the previous PR, after gathering an extra review
from sashiko. This PR also includes an extra fix for the selftests.

Patch 1 is ensuring that the MAC header offset is reset before
deliering the packet to the upper layer. Not doing so could trick other
parts of the networking stack into wrong calculations.

Patch 1 fixes the inconsistent call context for gro_cells_receive() and
dev_dstats_rx_add().
On a TCP connection, we can end up calling gro_cells_receive() from
process context, which is unexpected (and could potentially trigger
deadlocks). At the same time, also dev_dstats_rx_add() should not be
invoked concurrently on the same CPU (i.e. from a softirq).
For this reason we're wrapping the aforementioned calls within
local_bh_disable/enable().


Ideally all selftests should always succeed now.

Please pull or let me know of any issue.

Thanks,
	Antonio


The following changes since commit bd3a4795d5744f59a1f485379f1303e5e606f377:

  selftests: tls: add test for data loss on small pipe (2026-05-02 18:27:14 -0700)

are available in the Git repository at:

  https://github.com/OpenVPN/ovpn-net-next.git tags/ovpn-net-20260504

for you to fetch changes up to 201ba706318d460a2ea660e3652610be62532a70:

  selftests: ovpn: reduce ping count in test.sh (2026-05-05 00:31:11 +0200)

----------------------------------------------------------------
Includes changes:
* ensure MAC header offset is reset before delivering packet
* ensure gro_cells_receive() and dstats_dev_add() are called with BH
  disabled
* reduce ping count in selftest to ensure it completes within
  timeout

----------------------------------------------------------------
Qingfang Deng (1):
      ovpn: reset MAC header before passing skb up

Ralf Lici (2):
      ovpn: ensure packet delivery happens with BH disabled
      selftests: ovpn: reduce ping count in test.sh

 drivers/net/ovpn/io.c                    | 7 +++++++
 tools/testing/selftests/net/ovpn/test.sh | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

[PATCH net 1/3] ovpn: reset MAC header before passing skb up

[Antonio Quartulli]

From: Qingfang Deng <qingfang.deng@linux.dev>

After decapsulating a packet, the skb->mac_header still points to the
outer transport header.

Fix this by calling skb_reset_mac_header() in ovpn_netdev_write() to
ensure the MAC header points to the beginning of
the inner IP/network packet, as expected by the rest of the stack.

Reported-by: Minqiang Chen <ptpt52@gmail.com>
Fixes: 8534731dbf2d ("ovpn: implement packet processing")
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/io.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index db43a1f8a07a..d92bb87be2b2 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -85,6 +85,7 @@ static void ovpn_netdev_write(struct ovpn_peer *peer, struct sk_buff *skb)
 	skb_scrub_packet(skb, true);
 
 	/* network header reset in ovpn_decrypt_post() */
+	skb_reset_mac_header(skb);
 	skb_reset_transport_header(skb);
 	skb_reset_inner_headers(skb);
 
-- 
2.53.0

Re: [PATCH net 1/3] ovpn: reset MAC header before passing skb up

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Antonio Quartulli <antonio@openvpn.net>:

On Tue,  5 May 2026 01:03:03 +0200 you wrote:
> From: Qingfang Deng <qingfang.deng@linux.dev>
> 
> After decapsulating a packet, the skb->mac_header still points to the
> outer transport header.
> 
> Fix this by calling skb_reset_mac_header() in ovpn_netdev_write() to
> ensure the MAC header points to the beginning of
> the inner IP/network packet, as expected by the rest of the stack.
> 
> [...]

Here is the summary with links:
  - [net,1/3] ovpn: reset MAC header before passing skb up
    https://git.kernel.org/netdev/net/c/a200cdbf9593
  - [net,2/3] ovpn: ensure packet delivery happens with BH disabled
    https://git.kernel.org/netdev/net/c/c539cb30f93f
  - [net,3/3] selftests: ovpn: reduce ping count in test.sh
    https://git.kernel.org/netdev/net/c/201ba706318d

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

[PATCH net 2/3] ovpn: ensure packet delivery happens with BH disabled

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

ovpn injects decrypted packets into the netdev RX path through
ovpn_netdev_write() which invokes gro_cells_receive() and
dev_dstats_rx_add().

ovpn_netdev_write() is normally called in softirq context,
however, in case of TCP connections it may also be invoked
process context.

When this happens gro_cells_receive() will throw a warning:

  [  230.183747][   T12] WARNING: net/core/gro_cells.c:30 at gro_cells_receive+0x708/0xaa0, CPU#1: kworker/u16:0/12

and lockdep will also report a potential inconsistent lock state:

  WARNING: inconsistent lock state
  7.0.0-rc4+ #246 Tainted: G        W
  --------------------------------
  inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.

because attempts to acquire gro_cells->bh_lock by both
contexts may lead to a deadlock.

At the same time, dev_dstats_rx_add() does not expect to race
with a softirq (which may happen when invoked in process context),
because the latter may access its per-cpu state and corrupt
it.

Fix all this by invoking local_bh_disable/enable() around
gro_cells_receive() and dev_dstats_rx_add() to ensure that
bottom halves are always disabled before calling both of
them.

Fixes: 11851cbd60ea ("ovpn: implement TCP transport")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/io.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index d92bb87be2b2..22c555dd962e 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -91,12 +91,18 @@ static void ovpn_netdev_write(struct ovpn_peer *peer, struct sk_buff *skb)
 
 	/* cause packet to be "received" by the interface */
 	pkt_len = skb->len;
+	/* we may get here in process context in case of TCP connections,
+	 * therefore we have to disable BHs to ensure gro_cells_receive()
+	 * and dev_dstats_rx_add() do not get corrupted or enter deadlock
+	 */
+	local_bh_disable();
 	ret = gro_cells_receive(&peer->ovpn->gro_cells, skb);
 	if (likely(ret == NET_RX_SUCCESS)) {
 		/* update RX stats with the size of decrypted packet */
 		ovpn_peer_stats_increment_rx(&peer->vpn_stats, pkt_len);
 		dev_dstats_rx_add(peer->ovpn->dev, pkt_len);
 	}
+	local_bh_enable();
 }
 
 void ovpn_decrypt_post(void *data, int ret)
-- 
2.53.0

Re: [PATCH net 2/3] ovpn: ensure packet delivery happens with BH disabled

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ovpn: ensure packet delivery happens with BH disabled

This commit wraps gro_cells_receive() and dev_dstats_rx_add() with
local_bh_disable() and local_bh_enable() in ovpn_netdev_write(). This
prevents deadlocks and per-cpu state corruption when packet delivery occurs
in process context for TCP connections.

> diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
> --- a/drivers/net/ovpn/io.c
> +++ b/drivers/net/ovpn/io.c
> @@ -91,12 +91,18 @@ static void ovpn_netdev_write(struct ovpn_peer *peer, struct sk_buff *skb)
>  
>  	/* cause packet to be "received" by the interface */
>  	pkt_len = skb->len;
> +	/* we may get here in process context in case of TCP connections,
> +	 * therefore we have to disable BHs to ensure gro_cells_receive()
> +	 * and dev_dstats_rx_add() do not get corrupted or enter deadlock
> +	 */
> +	local_bh_disable();
>  	ret = gro_cells_receive(&peer->ovpn->gro_cells, skb);
>  	if (likely(ret == NET_RX_SUCCESS)) {
>  		/* update RX stats with the size of decrypted packet */
>  		ovpn_peer_stats_increment_rx(&peer->vpn_stats, pkt_len);
>  		dev_dstats_rx_add(peer->ovpn->dev, pkt_len);
>  	}
> +	local_bh_enable();
>  }

This protects the success path, but do the error and drop paths also need
BH protection?

If ovpn_decrypt_post() is called in process context for TCP connections, an
error condition jumps to the drop label:

ovpn_decrypt_post() {
        ...
        if (unlikely(ret < 0))
                goto drop;
        ...
drop:
        if (unlikely(skb))
                dev_dstats_rx_dropped(peer->ovpn->dev);
        ...
}

Since dev_dstats_rx_dropped() updates the same per-CPU dstats structure
without disabling bottom halves, could it still be vulnerable to softirq
preemption and stat corruption?

Additionally, do the TX paths in the TCP implementation have similar issues?
It looks like ovpn_tcp_send_skb() and ovpn_tcp_send_sock_skb() might call
dev_dstats_tx_dropped() without disabling bottom halves.

Similarly, ovpn_tcp_send_sock() uses preempt_disable() around
dev_dstats_tx_add(), which does not prevent softirqs from running upon
return from a hardware interrupt on the same CPU. Does this leave a similar
race window open?
-- 
pw-bot: cr

Re: [PATCH net 2/3] ovpn: ensure packet delivery happens with BH disabled

[Antonio Quartulli]

Hi Jakub

On 06/05/2026 03:00, Jakub Kicinski wrote:
[...]
> This protects the success path, but do the error and drop paths also need
> BH protection?
> 
> If ovpn_decrypt_post() is called in process context for TCP connections, an
> error condition jumps to the drop label:
> 
> ovpn_decrypt_post() {
>          ...
>          if (unlikely(ret < 0))
>                  goto drop;
>          ...
> drop:
>          if (unlikely(skb))
>                  dev_dstats_rx_dropped(peer->ovpn->dev);
>          ...
> }
> 
> Since dev_dstats_rx_dropped() updates the same per-CPU dstats structure
> without disabling bottom halves, could it still be vulnerable to softirq
> preemption and stat corruption?
> 

Actually we were already looking into this.
However, since this needs a separate analysis, I wanted to get this 
fixed in a follow up patch.

Would it be ok to pull this PR as is, so we don't hold back the 
outstanding fixes?

Then we will address the issue highlighted by Sashiko in a new patch.
The problem is similar, but may need to be fixed differently.


Regards,

-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH net 2/3] ovpn: ensure packet delivery happens with BH disabled

[Jakub Kicinski]

On Wed, 6 May 2026 11:00:20 +0200 Antonio Quartulli wrote:
> > ovpn_decrypt_post() {
> >          ...
> >          if (unlikely(ret < 0))
> >                  goto drop;
> >          ...
> > drop:
> >          if (unlikely(skb))
> >                  dev_dstats_rx_dropped(peer->ovpn->dev);
> >          ...
> > }
> > 
> > Since dev_dstats_rx_dropped() updates the same per-CPU dstats structure
> > without disabling bottom halves, could it still be vulnerable to softirq
> > preemption and stat corruption?
> >   
> 
> Actually we were already looking into this.
> However, since this needs a separate analysis, I wanted to get this 
> fixed in a follow up patch.
> 
> Would it be ok to pull this PR as is, so we don't hold back the 
> outstanding fixes?
> 
> Then we will address the issue highlighted by Sashiko in a new patch.
> The problem is similar, but may need to be fixed differently.


Ugh, fair. The way the AI formatted the output made me think
it's an error path in the same function, which would make sense 
to address with a single patch.. Pulling now.

[PATCH net 3/3] selftests: ovpn: reduce ping count in test.sh

[Antonio Quartulli]

From: Ralf Lici <ralf@mandelbit.com>

The second stage of test.sh ("run baseline data traffic") performs a
basic connectivity check with ping -qfc 500 -w 3.  On slower CI
instances this is too strict for TCP: the RTT is high enough that 500
echo requests do not reliably complete within 3 seconds, so the stage
flakes and the test fails even though the ovpn setup is healthy.

Reduce the packet count to 100 for both the plain and 3000-byte pings in
that stage.  This still verifies peer setup, key exchange, routing, and
data-path traffic, without making the basic connectivity check depend on
timing out under load.

Fixes: 959bc330a439 ("testing/selftests: add test tool and scripts for ovpn module")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/test.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
index b50dbe45a4d0..c06e3135fbef 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -98,10 +98,10 @@ ovpn_run_basic_traffic() {
 		sleep 0.3
 		ovpn_cmd_ok "send baseline traffic to peer ${p}" \
 			ip netns exec ovpn_peer0 \
-			ping -qfc 500 -w 3 5.5.5.$((p + 1))
+			ping -qfc 100 -w 3 5.5.5.$((p + 1))
 		ovpn_cmd_ok "send large-payload traffic to peer ${p}" \
 			ip netns exec ovpn_peer0 \
-			ping -qfc 500 -s 3000 -w 3 5.5.5.$((p + 1))
+			ping -qfc 100 -s 3000 -w 3 5.5.5.$((p + 1))
 
 		wait "${tcpdump_pid1}" || return 1
 		wait "${tcpdump_pid2}" || return 1
-- 
2.53.0