[RFC net-next] ovpn: allocate smaller skb when TCP headroom exceeds u16

[RFC net-next] ovpn: allocate smaller skb when TCP headroom exceeds u16

[Ralf Lici]

Hi all,

While testing openvpn over TCP under high traffic conditions,
specifically on the same machine using net namespaces (with veth pairs
interconnecting them), we consistently hit a warning in
skb_reset_network_header. The culprit is an attempt to store an offset
(skb->data - skb->head) larger than U16_MAX in skb->network_header,
which is a u16. This leads to packet drops.

In ovpn_tcp_recv, we're handed an skb from __strp_rcv and need to
linearize it and pull up to the beginning of the openvpn packet. If it's
a data-channel packet, we then pull an additional 24 bytes of openvpn
encapsulation header so that skb->data points to the inner IP packet.
This is necessary for authentication, decryption, and reinjection into
the networking stack of the decapsulated packet, but when the skb is too
large, the network header offset overflows the field.

AFAWCT, these oversized skbs can result from:
- GRO,
- TCP skb coalescing (tcp_try_coalesce, skb_try_coalesce),
- streamparser (__strp_rcv appends more skbs when an openvpn packet
  spans multiple skbs).

Note that this issue is likely affecting espintcp as well, since its
logic similarly involves extracting discrete packets from a coalesced
TCP stream handed off by streamparser, and reinjecting them into the
stack.

We've brainstormed a few possible directions, though we haven't yet
assessed their feasibility:
- introduce a u32 field in struct tcp_sock to limit skb->len during TCP
  coalescing (each socket user can set the limit if needed);
- modify strp to build an skb containing only the relevant frags for the
  current openvpn packet in frag_list.

In this patch, we implement a solution entirely contained within ovpn:
we allocate a new skb and copy the content of the current openvpn packet
into it. This avoids the large headroom issue, but it’s not ideal
because the kernel keeps coalescing skbs while we effectively undo that
work, which isn’t very efficient.

We're sending this RFC to gather ideas and suggestions on how best to
address this issue. Any thoughts or guidance would be appreciated.

Thanks.

Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 drivers/net/ovpn/crypto_aead.c |  3 --
 drivers/net/ovpn/proto.h       |  4 ++
 drivers/net/ovpn/tcp.c         | 80 +++++++++++++++++++++++++++-------
 3 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ovpn/crypto_aead.c b/drivers/net/ovpn/crypto_aead.c
index 2cca759feffa..3b2182984a54 100644
--- a/drivers/net/ovpn/crypto_aead.c
+++ b/drivers/net/ovpn/crypto_aead.c
@@ -23,9 +23,6 @@
 #include "proto.h"
 #include "skb.h"
 
-#define OVPN_AUTH_TAG_SIZE	16
-#define OVPN_AAD_SIZE		(OVPN_OPCODE_SIZE + OVPN_NONCE_WIRE_SIZE)
-
 #define ALG_NAME_AES		"gcm(aes)"
 #define ALG_NAME_CHACHAPOLY	"rfc7539(chacha20,poly1305)"
 
diff --git a/drivers/net/ovpn/proto.h b/drivers/net/ovpn/proto.h
index b7d285b4d9c1..0a9d54386ebb 100644
--- a/drivers/net/ovpn/proto.h
+++ b/drivers/net/ovpn/proto.h
@@ -49,6 +49,10 @@
 
 #define OVPN_PEER_ID_UNDEF		0x00FFFFFF
 
+#define OVPN_AUTH_TAG_SIZE	16
+#define OVPN_AAD_SIZE		(OVPN_OPCODE_SIZE + OVPN_NONCE_WIRE_SIZE)
+#define OVPN_HEADER_SIZE	(OVPN_AUTH_TAG_SIZE + OVPN_AAD_SIZE)
+
 /**
  * ovpn_opcode_from_skb - extract OP code from skb at specified offset
  * @skb: the packet to extract the OP code from
diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
index b7348da9b040..301fcb1c0495 100644
--- a/drivers/net/ovpn/tcp.c
+++ b/drivers/net/ovpn/tcp.c
@@ -70,39 +70,87 @@ static void ovpn_tcp_to_userspace(struct ovpn_peer *peer, struct sock *sk,
 	peer->tcp.sk_cb.sk_data_ready(sk);
 }
 
-static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
+/* takes ownership of orig_skb */
+static struct sk_buff *ovpn_tcp_skb_packet(const struct ovpn_peer *peer,
+					   struct sk_buff *orig_skb,
+					   const int full_len, const int offset)
 {
-	struct ovpn_peer *peer = container_of(strp, struct ovpn_peer, tcp.strp);
-	struct strp_msg *msg = strp_msg(skb);
-	size_t pkt_len = msg->full_len - 2;
-	size_t off = msg->offset + 2;
-	u8 opcode;
+	struct sk_buff *ovpn_skb = orig_skb;
+	const int pkt_len = full_len - 2;
+	int pkt_offset = offset + 2;
+	int err;
+
+	/* If the final headroom will overflow a u16 we will not be able to
+	 * reset the network header to it so we need to create a new smaller
+	 * skb with the content of this packet.
+	 */
+	if (unlikely(skb_headroom(orig_skb) + pkt_offset + OVPN_HEADER_SIZE >
+		     U16_MAX)) {
+		ovpn_skb = netdev_alloc_skb(peer->ovpn->dev, full_len);
+		if (!ovpn_skb) {
+			ovpn_skb = orig_skb;
+			goto err;
+		}
+
+		skb_copy_header(ovpn_skb, orig_skb);
+		pkt_offset = 2;
+
+		/* copy the entire openvpn packet + 2 bytes length */
+		err = skb_copy_bits(orig_skb, offset,
+				    skb_put(ovpn_skb, full_len), full_len);
+		kfree(orig_skb);
+		if (err) {
+			net_warn_ratelimited("%s: skb_copy_bits failed for peer %u\n",
+					     netdev_name(peer->ovpn->dev),
+					     peer->id);
+			goto err;
+		}
+	}
 
 	/* ensure skb->data points to the beginning of the openvpn packet */
-	if (!pskb_pull(skb, off)) {
+	if (!pskb_pull(ovpn_skb, pkt_offset)) {
 		net_warn_ratelimited("%s: packet too small for peer %u\n",
-				     netdev_name(peer->ovpn->dev), peer->id);
+				     netdev_name(peer->ovpn->dev),
+				     peer->id);
 		goto err;
 	}
 
 	/* strparser does not trim the skb for us, therefore we do it now */
-	if (pskb_trim(skb, pkt_len) != 0) {
+	if (pskb_trim(ovpn_skb, pkt_len) != 0) {
 		net_warn_ratelimited("%s: trimming skb failed for peer %u\n",
-				     netdev_name(peer->ovpn->dev), peer->id);
+				     netdev_name(peer->ovpn->dev),
+				     peer->id);
 		goto err;
 	}
 
+	return ovpn_skb;
+err:
+	kfree(ovpn_skb);
+	return NULL;
+}
+
+static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
+{
+	struct ovpn_peer *peer = container_of(strp, struct ovpn_peer, tcp.strp);
+	struct strp_msg *msg = strp_msg(skb);
+	struct sk_buff *ovpn_skb = NULL;
+	u8 opcode;
+
+	ovpn_skb = ovpn_tcp_skb_packet(peer, skb, msg->full_len, msg->offset);
+	if (!ovpn_skb)
+		goto err;
+
 	/* we need the first 4 bytes of data to be accessible
 	 * to extract the opcode and the key ID later on
 	 */
-	if (!pskb_may_pull(skb, OVPN_OPCODE_SIZE)) {
+	if (!pskb_may_pull(ovpn_skb, OVPN_OPCODE_SIZE)) {
 		net_warn_ratelimited("%s: packet too small to fetch opcode for peer %u\n",
 				     netdev_name(peer->ovpn->dev), peer->id);
 		goto err;
 	}
 
 	/* DATA_V2 packets are handled in kernel, the rest goes to user space */
-	opcode = ovpn_opcode_from_skb(skb, 0);
+	opcode = ovpn_opcode_from_skb(ovpn_skb, 0);
 	if (unlikely(opcode != OVPN_DATA_V2)) {
 		if (opcode == OVPN_DATA_V1) {
 			net_warn_ratelimited("%s: DATA_V1 detected on the TCP stream\n",
@@ -113,8 +161,8 @@ static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
 		/* The packet size header must be there when sending the packet
 		 * to userspace, therefore we put it back
 		 */
-		skb_push(skb, 2);
-		ovpn_tcp_to_userspace(peer, strp->sk, skb);
+		skb_push(ovpn_skb, 2);
+		ovpn_tcp_to_userspace(peer, strp->sk, ovpn_skb);
 		return;
 	}
 
@@ -126,7 +174,7 @@ static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
 	if (WARN_ON(!ovpn_peer_hold(peer)))
 		goto err_nopeer;
 
-	ovpn_recv(peer, skb);
+	ovpn_recv(peer, ovpn_skb);
 	return;
 err:
 	/* take reference for deferred peer deletion. should never fail */
@@ -135,7 +183,7 @@ static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
 	schedule_work(&peer->tcp.defer_del_work);
 	dev_dstats_rx_dropped(peer->ovpn->dev);
 err_nopeer:
-	kfree_skb(skb);
+	kfree_skb(ovpn_skb);
 }
 
 static int ovpn_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
-- 
2.51.1

Re: [RFC net-next] ovpn: allocate smaller skb when TCP headroom exceeds u16

[Sabrina Dubroca]

2025-11-13, 13:21:43 +0100, Ralf Lici wrote:
> Hi all,
> 
> While testing openvpn over TCP under high traffic conditions,
> specifically on the same machine using net namespaces (with veth pairs
> interconnecting them), we consistently hit a warning in
> skb_reset_network_header. The culprit is an attempt to store an offset
> (skb->data - skb->head) larger than U16_MAX in skb->network_header,
> which is a u16. This leads to packet drops.
> 
> In ovpn_tcp_recv, we're handed an skb from __strp_rcv and need to
> linearize it and pull up to the beginning of the openvpn packet. If it's

We don't currently linearize (= move all the data into ->head), right?

> a data-channel packet, we then pull an additional 24 bytes of openvpn
> encapsulation header so that skb->data points to the inner IP packet.
> This is necessary for authentication, decryption, and reinjection into
> the networking stack of the decapsulated packet, but when the skb is too
> large, the network header offset overflows the field.
> 
> AFAWCT, these oversized skbs can result from:
> - GRO,
> - TCP skb coalescing (tcp_try_coalesce, skb_try_coalesce),
> - streamparser (__strp_rcv appends more skbs when an openvpn packet
>   spans multiple skbs).
>
> Note that this issue is likely affecting espintcp as well, since its
> logic similarly involves extracting discrete packets from a coalesced
> TCP stream handed off by streamparser, and reinjecting them into the
> stack.

Most likely yes. I'll see if I can reproduce the problem on espintcp.

> We've brainstormed a few possible directions, though we haven't yet
> assessed their feasibility:
> - introduce a u32 field in struct tcp_sock to limit skb->len during TCP
>   coalescing (each socket user can set the limit if needed);

I doubt the TCP maintainers would accept a patch to TCP for a problem
that affects only (some of) the users of strp.

> - modify strp to build an skb containing only the relevant frags for the
>   current openvpn packet in frag_list.

This would penalize the other users of strp. It may make sense to
introduce such a mechanism in strp, but only on request (eg via a bool
in strp_init, a flag in the cb struct).

> In this patch, we implement a solution entirely contained within ovpn:
> we allocate a new skb and copy the content of the current openvpn packet
> into it. This avoids the large headroom issue, but it’s not ideal
> because the kernel keeps coalescing skbs while we effectively undo that
> work, which isn’t very efficient.

Well, that coalescing is useful, and the un-coalescing is necessary
(because even without this offset problem, we have to get back the
individual packets from the stream).


Copying the full contents (full_len) of the openvpn packet seems a bit
heavy when what we want is "pull and get rid of that extra space at
the head". It seems pskb_extract would do the job without manual
handling in ovpn and without copying the entire payload? (but it will
clone the skb and realloc the head every time, so we'd only want to
call it in the "offset too big" case)

> We're sending this RFC to gather ideas and suggestions on how best to
> address this issue. Any thoughts or guidance would be appreciated.

One thing I'm a bit concerned about is if those reduced skbs need to
be re-sent somewhere else. Then we don't have any headroom to push a
new header and we'll have to realloc again to create some space. OTOH
it doesn't really make sense to carry 65kB of extra data through the
stack.



A few comments on the implementation:

[...]
> diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
> index b7348da9b040..301fcb1c0495 100644
> --- a/drivers/net/ovpn/tcp.c
> +++ b/drivers/net/ovpn/tcp.c
> @@ -70,39 +70,87 @@ static void ovpn_tcp_to_userspace(struct ovpn_peer *peer, struct sock *sk,
>  	peer->tcp.sk_cb.sk_data_ready(sk);
>  }
>  
> -static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
> +/* takes ownership of orig_skb */
> +static struct sk_buff *ovpn_tcp_skb_packet(const struct ovpn_peer *peer,
> +					   struct sk_buff *orig_skb,
> +					   const int full_len, const int offset)
>  {
> -	struct ovpn_peer *peer = container_of(strp, struct ovpn_peer, tcp.strp);
> -	struct strp_msg *msg = strp_msg(skb);
> -	size_t pkt_len = msg->full_len - 2;
> -	size_t off = msg->offset + 2;
> -	u8 opcode;
> +	struct sk_buff *ovpn_skb = orig_skb;
> +	const int pkt_len = full_len - 2;
> +	int pkt_offset = offset + 2;
> +	int err;
> +
> +	/* If the final headroom will overflow a u16 we will not be able to
> +	 * reset the network header to it so we need to create a new smaller
> +	 * skb with the content of this packet.
> +	 */
> +	if (unlikely(skb_headroom(orig_skb) + pkt_offset + OVPN_HEADER_SIZE >
> +		     U16_MAX)) {
> +		ovpn_skb = netdev_alloc_skb(peer->ovpn->dev, full_len);

From my reading of __strp_recv, strp already gave us a fresh clone, do
we need to reallocate a full skb?

> +		if (!ovpn_skb) {
> +			ovpn_skb = orig_skb;
> +			goto err;
> +		}
> +
> +		skb_copy_header(ovpn_skb, orig_skb);
> +		pkt_offset = 2;
> +
> +		/* copy the entire openvpn packet + 2 bytes length */
> +		err = skb_copy_bits(orig_skb, offset,
> +				    skb_put(ovpn_skb, full_len), full_len);
> +		kfree(orig_skb);
> +		if (err) {
> +			net_warn_ratelimited("%s: skb_copy_bits failed for peer %u\n",
> +					     netdev_name(peer->ovpn->dev),
> +					     peer->id);
> +			goto err;
> +		}
> +	}
>  
>  	/* ensure skb->data points to the beginning of the openvpn packet */
> -	if (!pskb_pull(skb, off)) {
> +	if (!pskb_pull(ovpn_skb, pkt_offset)) {
>  		net_warn_ratelimited("%s: packet too small for peer %u\n",
> -				     netdev_name(peer->ovpn->dev), peer->id);
> +				     netdev_name(peer->ovpn->dev),
> +				     peer->id);
>  		goto err;
>  	}
>  
>  	/* strparser does not trim the skb for us, therefore we do it now */
> -	if (pskb_trim(skb, pkt_len) != 0) {
> +	if (pskb_trim(ovpn_skb, pkt_len) != 0) {
>  		net_warn_ratelimited("%s: trimming skb failed for peer %u\n",
> -				     netdev_name(peer->ovpn->dev), peer->id);
> +				     netdev_name(peer->ovpn->dev),
> +				     peer->id);
>  		goto err;
>  	}
>  
> +	return ovpn_skb;
> +err:
> +	kfree(ovpn_skb);

This needs to be kfree_skb/consume_skb in all cases where you're
freeing an skb.

> +	return NULL;
> +}

-- 
Sabrina

Re: [RFC net-next] ovpn: allocate smaller skb when TCP headroom exceeds u16

[Ralf Lici]

On Tue, 2025-11-18 at 17:17 +0100, Sabrina Dubroca wrote:
> 2025-11-13, 13:21:43 +0100, Ralf Lici wrote:
> > Hi all,
> > 
> > While testing openvpn over TCP under high traffic conditions,
> > specifically on the same machine using net namespaces (with veth
> > pairs
> > interconnecting them), we consistently hit a warning in
> > skb_reset_network_header. The culprit is an attempt to store an
> > offset
> > (skb->data - skb->head) larger than U16_MAX in skb->network_header,
> > which is a u16. This leads to packet drops.
> > 
> > In ovpn_tcp_recv, we're handed an skb from __strp_rcv and need to
> > linearize it and pull up to the beginning of the openvpn packet. If
> > it's
> 
> We don't currently linearize (= move all the data into ->head), right?

Actually, my understanding is that we effectively do, though it happens
implicitly in the crypto layer rather than in the RX callback.

While ovpn_tcp_rcv pulls up to the beginning of the openvpn packet in
the skb, the subsequent call to ovpn_aead_decrypt invokes skb_cow_data
to ensure the *whole* skb is writable for decryption. Since strparser
hands us cloned skbs, skb_cow_data calls __pskb_pull_tail(skb,
__skb_pagelen(skb)). As I understand it, this forces a full
linearization for every single packet in the huge skb.

> 
> > a data-channel packet, we then pull an additional 24 bytes of
> > openvpn
> > encapsulation header so that skb->data points to the inner IP
> > packet.
> > This is necessary for authentication, decryption, and reinjection
> > into
> > the networking stack of the decapsulated packet, but when the skb is
> > too
> > large, the network header offset overflows the field.
> > 
> > AFAWCT, these oversized skbs can result from:
> > - GRO,
> > - TCP skb coalescing (tcp_try_coalesce, skb_try_coalesce),
> > - streamparser (__strp_rcv appends more skbs when an openvpn packet
> >   spans multiple skbs).
> > 
> > Note that this issue is likely affecting espintcp as well, since its
> > logic similarly involves extracting discrete packets from a
> > coalesced
> > TCP stream handed off by streamparser, and reinjecting them into the
> > stack.
> 
> Most likely yes. I'll see if I can reproduce the problem on espintcp.

Thanks!

> 
> > We've brainstormed a few possible directions, though we haven't yet
> > assessed their feasibility:
> > - introduce a u32 field in struct tcp_sock to limit skb->len during
> > TCP
> >   coalescing (each socket user can set the limit if needed);
> 
> I doubt the TCP maintainers would accept a patch to TCP for a problem
> that affects only (some of) the users of strp.

Perfectly understandable. I will drop this idea.

> 
> > - modify strp to build an skb containing only the relevant frags for
> > the
> >   current openvpn packet in frag_list.
> 
> This would penalize the other users of strp. It may make sense to
> introduce such a mechanism in strp, but only on request (eg via a bool
> in strp_init, a flag in the cb struct).

I agree that we should keep strparser generic. Since this issue is
specific to how ovpn/espintcp handle the resulting skbs (decapsulation
and header resetting), it is probably better to handle the output within
the users rather than complicating the strparser API.

> 
> > In this patch, we implement a solution entirely contained within
> > ovpn:
> > we allocate a new skb and copy the content of the current openvpn
> > packet
> > into it. This avoids the large headroom issue, but it’s not ideal
> > because the kernel keeps coalescing skbs while we effectively undo
> > that
> > work, which isn’t very efficient.
> 
> Well, that coalescing is useful, and the un-coalescing is necessary
> (because even without this offset problem, we have to get back the
> individual packets from the stream).
> 
> 
> Copying the full contents (full_len) of the openvpn packet seems a bit
> heavy when what we want is "pull and get rid of that extra space at
> the head". It seems pskb_extract would do the job without manual
> handling in ovpn and without copying the entire payload? (but it will
> clone the skb and realloc the head every time, so we'd only want to
> call it in the "offset too big" case)

Thanks, I was not aware of this function and I believe it would indeed
solve the overflow issue. However, I'm not sure it would be more
efficient. Since the problem arises from huge offsets on non-linear
skbs, AFAICT it's almost certain that we'd take the
pskb_carve_inside_nonlinear path in pskb_carve. That function will
produce a fully non-linear cloned skb by removing the pages that are not
needed. But, following the skb_cow_data explanation above, we would end
up linearizing the content of this skb in the end anyway. So I fear
pskb_extract might not be beneficial in this scenario.

> 
> > We're sending this RFC to gather ideas and suggestions on how best
> > to
> > address this issue. Any thoughts or guidance would be appreciated.
> 
> One thing I'm a bit concerned about is if those reduced skbs need to
> be re-sent somewhere else. Then we don't have any headroom to push a
> new header and we'll have to realloc again to create some space. OTOH
> it doesn't really make sense to carry 65kB of extra data through the
> stack.

That's a valid concern and I have the feeling that there must be a
"standard" way of creating an skb with some content and a reasonable
headroom. I thought netdev_alloc_skb could be a good choice because it
builds an skb with NET_SKB_PAD headeroom, but I'm not sure if that's the
proper size. 

> 
> 
> A few comments on the implementation:
> 
> [...]
> > diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
> > index b7348da9b040..301fcb1c0495 100644
> > --- a/drivers/net/ovpn/tcp.c
> > +++ b/drivers/net/ovpn/tcp.c
> > @@ -70,39 +70,87 @@ static void ovpn_tcp_to_userspace(struct
> > ovpn_peer *peer, struct sock *sk,
> >  	peer->tcp.sk_cb.sk_data_ready(sk);
> >  }
> >  
> > -static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff
> > *skb)
> > +/* takes ownership of orig_skb */
> > +static struct sk_buff *ovpn_tcp_skb_packet(const struct ovpn_peer
> > *peer,
> > +					   struct sk_buff
> > *orig_skb,
> > +					   const int full_len,
> > const int offset)
> >  {
> > -	struct ovpn_peer *peer = container_of(strp, struct
> > ovpn_peer, tcp.strp);
> > -	struct strp_msg *msg = strp_msg(skb);
> > -	size_t pkt_len = msg->full_len - 2;
> > -	size_t off = msg->offset + 2;
> > -	u8 opcode;
> > +	struct sk_buff *ovpn_skb = orig_skb;
> > +	const int pkt_len = full_len - 2;
> > +	int pkt_offset = offset + 2;
> > +	int err;
> > +
> > +	/* If the final headroom will overflow a u16 we will not be
> > able to
> > +	 * reset the network header to it so we need to create a
> > new smaller
> > +	 * skb with the content of this packet.
> > +	 */
> > +	if (unlikely(skb_headroom(orig_skb) + pkt_offset +
> > OVPN_HEADER_SIZE >
> > +		     U16_MAX)) {
> > +		ovpn_skb = netdev_alloc_skb(peer->ovpn->dev,
> > full_len);
> 
> From my reading of __strp_recv, strp already gave us a fresh clone, do
> we need to reallocate a full skb?

As explained above, the reason is the call to skb_cow_data in
ovpn_aead_decrypt, which, in the end, would linearize the whole cloned
skb.

> 
> > +		if (!ovpn_skb) {
> > +			ovpn_skb = orig_skb;
> > +			goto err;
> > +		}
> > +
> > +		skb_copy_header(ovpn_skb, orig_skb);
> > +		pkt_offset = 2;
> > +
> > +		/* copy the entire openvpn packet + 2 bytes length
> > */
> > +		err = skb_copy_bits(orig_skb, offset,
> > +				    skb_put(ovpn_skb, full_len),
> > full_len);
> > +		kfree(orig_skb);
> > +		if (err) {
> > +			net_warn_ratelimited("%s: skb_copy_bits
> > failed for peer %u\n",
> > +					     netdev_name(peer-
> > >ovpn->dev),
> > +					     peer->id);
> > +			goto err;
> > +		}
> > +	}
> >  
> >  	/* ensure skb->data points to the beginning of the openvpn
> > packet */
> > -	if (!pskb_pull(skb, off)) {
> > +	if (!pskb_pull(ovpn_skb, pkt_offset)) {
> >  		net_warn_ratelimited("%s: packet too small for peer
> > %u\n",
> > -				     netdev_name(peer->ovpn->dev),
> > peer->id);
> > +				     netdev_name(peer->ovpn->dev),
> > +				     peer->id);
> >  		goto err;
> >  	}
> >  
> >  	/* strparser does not trim the skb for us, therefore we do
> > it now */
> > -	if (pskb_trim(skb, pkt_len) != 0) {
> > +	if (pskb_trim(ovpn_skb, pkt_len) != 0) {
> >  		net_warn_ratelimited("%s: trimming skb failed for
> > peer %u\n",
> > -				     netdev_name(peer->ovpn->dev),
> > peer->id);
> > +				     netdev_name(peer->ovpn->dev),
> > +				     peer->id);
> >  		goto err;
> >  	}
> >  
> > +	return ovpn_skb;
> > +err:
> > +	kfree(ovpn_skb);
> 
> This needs to be kfree_skb/consume_skb in all cases where you're
> freeing an skb.

Yep, sorry. Something got lost during the refactoring.

> 
> > +	return NULL;
> > +}

Thanks a lot for your input.

-- 
Ralf Lici
Mandelbit Srl