[PATCH net] esp: skip GRO for fragmented packets

[PATCH net] esp: skip GRO for fragmented packets

[Sabrina Dubroca]

Currently, ESP4 GRO doesn't work for fragmented packets, so let's send
these through the normal path.

Fixes: 7785bba299a8 ("esp: Add a software GRO codepath")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
Steffen, if you prefer to drop this patch and fix this properly,
that's okay for me. I can't look much deeper into this right now and
it's broken on current net/master.

It seems like the first fragment gets dropped, at least I don't see it
on tcpdump on the RX machine.

 net/ipv4/esp4_offload.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index 1de442632406..ab5faca28e19 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -38,6 +38,9 @@ static struct sk_buff **esp4_gro_receive(struct sk_buff **head,
 	__be32 spi;
 	int err;
 
+	if (ip_is_fragment(ip_hdr(skb)))
+		goto flush;
+
 	skb_pull(skb, offset);
 
 	if ((err = xfrm_parse_spi(skb, IPPROTO_ESP, &spi, &seq)) != 0)
@@ -78,6 +81,7 @@ static struct sk_buff **esp4_gro_receive(struct sk_buff **head,
 	return ERR_PTR(-EINPROGRESS);
 out:
 	skb_push(skb, offset);
+flush:
 	NAPI_GRO_CB(skb)->same_flow = 0;
 	NAPI_GRO_CB(skb)->flush = 1;
 
-- 
2.12.2

Re: [PATCH net] esp: skip GRO for fragmented packets

[Steffen Klassert]

On Thu, Apr 27, 2017 at 12:31:14PM +0200, Sabrina Dubroca wrote:
> Currently, ESP4 GRO doesn't work for fragmented packets, so let's send
> these through the normal path.
> 
> Fixes: 7785bba299a8 ("esp: Add a software GRO codepath")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
> Steffen, if you prefer to drop this patch and fix this properly,
> that's okay for me. I can't look much deeper into this right now and
> it's broken on current net/master.

I did a fix for this last week, but forgot to submit it.
We can fix this in inet_gro_receive(), as no GRO handler
can really handle fragmented packets.

I'll plan to fix it with this patch:

>From 44a2fc882bb310b66d9cc5c89405d0669a26cd45 Mon Sep 17 00:00:00 2001
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 20 Apr 2017 09:44:58 +0200
Subject: [PATCH RFC] ipv4: Don't pass IP fragments to upper layer GRO handlers.

Upper layer GRO handlers can not handle IP fragments, so
exit GRO processing in this case. This also fixes ESP GRO
because the packet must be reassembled before we can
decapsulate, otherwise we get authentication failures.

This also aligns IPv4 to IPv6 where packets with fragmentation
headers are not passed to upper layer GRO handlers.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/af_inet.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index d1a1170..f3dad16 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1343,6 +1343,9 @@ struct sk_buff **inet_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 	if (*(u8 *)iph != 0x45)
 		goto out_unlock;
 
+	if (ip_is_fragment(iph))
+		goto out_unlock;
+
 	if (unlikely(ip_fast_csum((u8 *)iph, 5)))
 		goto out_unlock;
 
-- 
2.7.4

Re: [PATCH net] esp: skip GRO for fragmented packets

[Sabrina Dubroca]

2017-04-27, 12:43:35 +0200, Steffen Klassert wrote:
> On Thu, Apr 27, 2017 at 12:31:14PM +0200, Sabrina Dubroca wrote:
> > Currently, ESP4 GRO doesn't work for fragmented packets, so let's send
> > these through the normal path.
> > 
> > Fixes: 7785bba299a8 ("esp: Add a software GRO codepath")
> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > ---
> > Steffen, if you prefer to drop this patch and fix this properly,
> > that's okay for me. I can't look much deeper into this right now and
> > it's broken on current net/master.
> 
> I did a fix for this last week, but forgot to submit it.
> We can fix this in inet_gro_receive(), as no GRO handler
> can really handle fragmented packets.
> 
> I'll plan to fix it with this patch:

Yeah, that looks okay to me, thanks.
Let's make sure it ends up in 4.11 (or an early 4.11.x).

-- 
Sabrina