linux 3.13: problems with isatap tunnel device and UFO

linux 3.13: problems with isatap tunnel device and UFO

[Wolfgang Walter]

Hello,

with kernel 3.13 I have a problem with isatap tunnels receiving fragmented 
ipv6 udp packets.

If I switch of ufo for the tunnel device the problem disappears:
	ethtool -K is0 ufo off

When UFO is on tcpdump shows the packets on the physical device but not on the 
tunnel device. The packets seem to be discarded.

With UFO off I can see them on is0 (and the packet is delivered to the 
socket).

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

Re: linux 3.13: problems with isatap tunnel device and UFO

[Hannes Frederic Sowa]

Hi!

On Fri, Feb 07, 2014 at 06:47:07PM +0100, Wolfgang Walter wrote:
> with kernel 3.13 I have a problem with isatap tunnels receiving fragmented 
> ipv6 udp packets.

Which was the last known version that did work? Also dropwatch and nstat could
give a first hint.

> If I switch of ufo for the tunnel device the problem disappears:
> 	ethtool -K is0 ufo off
> 
> When UFO is on tcpdump shows the packets on the physical device but not on the 
> tunnel device. The packets seem to be discarded.
> 
> With UFO off I can see them on is0 (and the packet is delivered to the 
> socket).

Thanks for the report, I'll look at it.

Greetings,

  Hannes

Re: linux 3.13: problems with isatap tunnel device and UFO

[Wolfgang Walter]

Am Freitag, 7. Februar 2014, 18:56:41 schrieb Hannes Frederic Sowa:
> Hi!
> 
> On Fri, Feb 07, 2014 at 06:47:07PM +0100, Wolfgang Walter wrote:
> > with kernel 3.13 I have a problem with isatap tunnels receiving fragmented
> > ipv6 udp packets.
> 
> Which was the last known version that did work?

I think 3.12 had no problems, but I'm not sure. I test this tonight.

> Also dropwatch and nstat
> could give a first hint.
> 
> > If I switch of ufo for the tunnel device the problem disappears:
> > 	ethtool -K is0 ufo off
> > 
> > When UFO is on tcpdump shows the packets on the physical device but not on
> > the tunnel device. The packets seem to be discarded.
> > 
> > With UFO off I can see them on is0 (and the packet is delivered to the
> > socket).
> 
> Thanks for the report, I'll look at it.
> 
> Greetings,
> 
>   Hannes
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

Re: linux 3.13: problems with isatap tunnel device and UFO

[Hannes Frederic Sowa]

Hi!

On Fri, Feb 07, 2014 at 07:17:40PM +0100, Wolfgang Walter wrote:
> Am Freitag, 7. Februar 2014, 18:56:41 schrieb Hannes Frederic Sowa:
> > Hi!
> > 
> > On Fri, Feb 07, 2014 at 06:47:07PM +0100, Wolfgang Walter wrote:
> > > with kernel 3.13 I have a problem with isatap tunnels receiving fragmented
> > > ipv6 udp packets.
> > 
> > Which was the last known version that did work?
> 
> I think 3.12 had no problems, but I'm not sure. I test this tonight.

Could you give me a bit more details on your setup, please?

I just tested a setup with UFO packets in sit tunnels and it worked
properly for me (on net).

Greetings,

  Hannes

Re: linux 3.13: problems with isatap tunnel device and UFO

[Wolfgang Walter]

Am Freitag, 7. Februar 2014, 23:22:27 schrieb Hannes Frederic Sowa:
> Hi!
> 
> On Fri, Feb 07, 2014 at 07:17:40PM +0100, Wolfgang Walter wrote:
> > Am Freitag, 7. Februar 2014, 18:56:41 schrieb Hannes Frederic Sowa:
> > > Hi!
> > > 
> > > On Fri, Feb 07, 2014 at 06:47:07PM +0100, Wolfgang Walter wrote:
> > > > with kernel 3.13 I have a problem with isatap tunnels receiving
> > > > fragmented
> > > > ipv6 udp packets.
> > > 
> > > Which was the last known version that did work?
> > 
> > I think 3.12 had no problems, but I'm not sure. I test this tonight.

3.12 is indeed fine. But this is probably because of:

ethtool -k is0
....
udp-fragmentation-offload: off [fixed]
....


> 
> Could you give me a bit more details on your setup, please?
> 
> I just tested a setup with UFO packets in sit tunnels and it worked
> properly for me (on net).
> 

host A (which shows the problem with kernel 3.13):

$ ip addr ls eth0
4: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP 
group default qlen 1000
    link/ether 11:22:33:44:55:66 brd ff:ff:ff:ff:ff:ff
    inet 192.168.1.1/24 brd 192.168.1.255 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 2001:1111:2222:aaaa:0:5efe:c0a8:101/120 scope global 
       valid_lft forever preferred_lft forever
    inet6 fe80::1322:33ff:fe44:5566/64 scope link 
       valid_lft forever preferred_lft forever

$ ip addr ls is0
14: is0: <NOARP,UP,LOWER_UP> mtu 1280 qdisc noqueue state UNKNOWN group 
default 
    link/sit 192.168.1.1 brd 0.0.0.0
    inet6 2001:1111:2222:aaaa:0:5efe:c0a8:101/64 scope global dynamic 
       valid_lft 85977sec preferred_lft 13977sec
    inet6 fe80::5efe:c0a8:101/64 scope link 
       valid_lft forever preferred_lft forever



The other host B is in the same isatap-subnet (but a different ipv4-subnet):

$ ip addr ls eth0
4: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP 
group default qlen 1000
    link/ether 11:22:33:44:55:ee brd ff:ff:ff:ff:ff:ff
    inet 192.168.10.1/24 brd 192.168.1.255 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 fe80::1322:33ff:fe44:55ee/64 scope link 
       valid_lft forever preferred_lft forever


$ ip addr ls is0
10: is0: <NOARP,UP,LOWER_UP> mtu 1280 qdisc noqueue state UNKNOWN group 
default 
    link/sit 192.168.10.1 brd 0.0.0.0
    inet6 2001:1111:2222:aaaa:0:5efe:c0a8:a01/64 scope global dynamic 
       valid_lft 85977sec preferred_lft 13977sec
    inet6 fe80::5efe:c0a8:a01/64 scope link 
       valid_lft forever preferred_lft forever


The application I see this is strongswan (ikev2). When it establishes an 
connection it sends udp-packets to large for is0 (here 1316 data-bytes, 
strongswan says).

For the tests I unloaded the netfilter modules so there should be no 
interference with the firewall or conntrack etc.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

Re: linux 3.13: problems with isatap tunnel device and UFO

[Hannes Frederic Sowa]

On Sun, Feb 09, 2014 at 12:17:15AM +0100, Wolfgang Walter wrote:
> host A (which shows the problem with kernel 3.13):
> 
> $ ip addr ls eth0
> 4: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP 
> group default qlen 1000
>     link/ether 11:22:33:44:55:66 brd ff:ff:ff:ff:ff:ff
>     inet 192.168.1.1/24 brd 192.168.1.255 scope global eth0
>        valid_lft forever preferred_lft forever
>     inet6 2001:1111:2222:aaaa:0:5efe:c0a8:101/120 scope global 
>        valid_lft forever preferred_lft forever
>     inet6 fe80::1322:33ff:fe44:5566/64 scope link 
>        valid_lft forever preferred_lft forever

What driver does this interface use?

ethtool -i eth0

Greetings,

  Hannes

Re: linux 3.13: problems with isatap tunnel device and UFO

[Wolfgang Walter]

Am Dienstag, 11. Februar 2014, 03:44:03 schrieb Hannes Frederic Sowa:
> On Sun, Feb 09, 2014 at 12:17:15AM +0100, Wolfgang Walter wrote:
> > host A (which shows the problem with kernel 3.13):
> > 
> > $ ip addr ls eth0
> > 4: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state
> > UP group default qlen 1000
> > 
> >     link/ether 11:22:33:44:55:66 brd ff:ff:ff:ff:ff:ff
> >     inet 192.168.1.1/24 brd 192.168.1.255 scope global eth0
> >        valid_lft forever preferred_lft forever
> >     inet6 2001:1111:2222:aaaa:0:5efe:c0a8:101/120 scope global
> >        valid_lft forever preferred_lft forever
> >     inet6 fe80::1322:33ff:fe44:5566/64 scope link
> >        valid_lft forever preferred_lft forever
> 
> What driver does this interface use?
> 
> ethtool -i eth0
> 

driver: r8169
version: 2.3LK-NAPI
firmware-version: rtl_nic/rtl8168e-1.fw
bus-info: 0000:05:00.0
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no

I see this also with another machine, here it is

driver: forcedeth
version: 0.64
firmware-version: 
bus-info: 0000:00:0a.0
supports-statistics: yes                                                        
supports-test: yes
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no


Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

Re: linux 3.13: problems with isatap tunnel device and UFO

[Hannes Frederic Sowa]

[+Cc Cong Wang]

Hi Cong!

In commit d949d826c09fb ("ipv6: Add generic UDP Tunnel segmentation") you
patched ip6_offload.c:

@@ -126,7 +128,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
                ipv6h = ipv6_hdr(skb);
                ipv6h->payload_len = htons(skb->len - skb->mac_len -
                                           sizeof(*ipv6h));
-               if (proto == IPPROTO_UDP) {
+               if (!tunnel && proto == IPPROTO_UDP) {
                        unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
                        fptr = (struct frag_hdr *)(skb_network_header(skb) +
                                unfrag_ip6hlen);


I wonder about the !tunnel exception. This now seems to be a problem in sit
ufo output path, where we don't update fragmentation offsets any more thus
generating invalid frames.

I am not too firm with segmentation in case of tunnels but don't we need to
always update the fragmentation offset no matter what, if upper gso callback
produced more segments?

I'll try to dig deeper...

Thanks,

  Hannes

Re: linux 3.13: problems with isatap tunnel device and UFO

[Hannes Frederic Sowa]

On Mon, Feb 17, 2014 at 05:09:16PM +0100, Hannes Frederic Sowa wrote:
> [+Cc Cong Wang]
> 
> Hi Cong!
> 
> In commit d949d826c09fb ("ipv6: Add generic UDP Tunnel segmentation") you
> patched ip6_offload.c:
> 
> @@ -126,7 +128,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>                 ipv6h = ipv6_hdr(skb);
>                 ipv6h->payload_len = htons(skb->len - skb->mac_len -
>                                            sizeof(*ipv6h));
> -               if (proto == IPPROTO_UDP) {
> +               if (!tunnel && proto == IPPROTO_UDP) {
>                         unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
>                         fptr = (struct frag_hdr *)(skb_network_header(skb) +
>                                 unfrag_ip6hlen);
> 
> 
> I wonder about the !tunnel exception. This now seems to be a problem in sit
> ufo output path, where we don't update fragmentation offsets any more thus
> generating invalid frames.
> 
> I am not too firm with segmentation in case of tunnels but don't we need to
> always update the fragmentation offset no matter what, if upper gso callback
> produced more segments?

Not perfect nor clean (well, I don't know).

The idea is to have the segmentation at the first guessed tunnel
header cut. I don't know how to deal with stacked tunnels yet, I guess
we need to have a bit more state in the skb. Just to maybe keep the discussion
going...

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index ecd2c3f..3eefe03 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1297,7 +1297,11 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 	segs = ERR_PTR(-EPROTONOSUPPORT);
 
 	/* Note : following gso_segment() might change skb->encapsulation */
-	udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
+	udpfrag = proto == IPPROTO_UDP;
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL|SKB_GSO_GRE))
+		udpfrag = udpfrag && !skb->encapsulation;
+	else if (skb->encapsulation)
+		udpfrag = udpfrag && encap;
 
 	ops = rcu_dereference(inet_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment))
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 1e8683b..d70c10b 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -89,7 +89,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	unsigned int unfrag_ip6hlen;
 	u8 *prevhdr;
 	int offset = 0;
-	bool tunnel;
+	bool encap, udpfrag;
 	int nhoff;
 
 	if (unlikely(skb_shinfo(skb)->gso_type &
@@ -110,8 +110,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
 		goto out;
 
-	tunnel = SKB_GSO_CB(skb)->encap_level > 0;
-	if (tunnel)
+	encap = SKB_GSO_CB(skb)->encap_level > 0;
+	if (encap)
 		features = skb->dev->hw_enc_features & netif_skb_features(skb);
 	SKB_GSO_CB(skb)->encap_level += sizeof(*ipv6h);
 
@@ -121,6 +121,12 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 
 	proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
 
+	udpfrag = proto == IPPROTO_UDP;
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL|SKB_GSO_GRE))
+		udpfrag = udpfrag && !skb->encapsulation;
+	else if (skb->encapsulation)
+		udpfrag = udpfrag && encap;
+
 	ops = rcu_dereference(inet6_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment)) {
 		skb_reset_transport_header(skb);
@@ -133,13 +139,9 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	for (skb = segs; skb; skb = skb->next) {
 		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
 		ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
-		if (tunnel) {
-			skb_reset_inner_headers(skb);
-			skb->encapsulation = 1;
-		}
 		skb->network_header = (u8 *)ipv6h - skb->head;
 
-		if (!tunnel && proto == IPPROTO_UDP) {
+		if (udpfrag) {
 			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
 			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
 			fptr->frag_off = htons(offset);
@@ -148,6 +150,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 			offset += (ntohs(ipv6h->payload_len) -
 				   sizeof(struct frag_hdr));
 		}
+		if (encap)
+			skb_reset_inner_headers(skb);
 	}
 
 out:

Re: linux 3.13: problems with isatap tunnel device and UFO

[Hannes Frederic Sowa]

On Thu, Feb 20, 2014 at 03:44:18AM +0100, Hannes Frederic Sowa wrote:
> On Mon, Feb 17, 2014 at 05:09:16PM +0100, Hannes Frederic Sowa wrote:
> > [+Cc Cong Wang]
> > 
> > Hi Cong!
> > 
> > In commit d949d826c09fb ("ipv6: Add generic UDP Tunnel segmentation") you
> > patched ip6_offload.c:
> > 
> > @@ -126,7 +128,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
> >                 ipv6h = ipv6_hdr(skb);
> >                 ipv6h->payload_len = htons(skb->len - skb->mac_len -
> >                                            sizeof(*ipv6h));
> > -               if (proto == IPPROTO_UDP) {
> > +               if (!tunnel && proto == IPPROTO_UDP) {
> >                         unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
> >                         fptr = (struct frag_hdr *)(skb_network_header(skb) +
> >                                 unfrag_ip6hlen);
> > 
> > 
> > I wonder about the !tunnel exception. This now seems to be a problem in sit
> > ufo output path, where we don't update fragmentation offsets any more thus
> > generating invalid frames.
> > 
> > I am not too firm with segmentation in case of tunnels but don't we need to
> > always update the fragmentation offset no matter what, if upper gso callback
> > produced more segments?
> 
> Not perfect nor clean (well, I don't know).
> 
> The idea is to have the segmentation at the first guessed tunnel
> header cut. I don't know how to deal with stacked tunnels yet, I guess
> we need to have a bit more state in the skb. Just to maybe keep the discussion
> going...

In-tunnel IPv6 fragmentation ids don't seem to be random at all... :/

Re: linux 3.13: problems with isatap tunnel device and UFO

[Cong Wang]

[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]

On Wed, Feb 19, 2014 at 6:44 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Mon, Feb 17, 2014 at 05:09:16PM +0100, Hannes Frederic Sowa wrote:
>> [+Cc Cong Wang]
>>
>> Hi Cong!
>>
>> In commit d949d826c09fb ("ipv6: Add generic UDP Tunnel segmentation") you
>> patched ip6_offload.c:
>>
>> @@ -126,7 +128,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>>                 ipv6h = ipv6_hdr(skb);
>>                 ipv6h->payload_len = htons(skb->len - skb->mac_len -
>>                                            sizeof(*ipv6h));
>> -               if (proto == IPPROTO_UDP) {
>> +               if (!tunnel && proto == IPPROTO_UDP) {
>>                         unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
>>                         fptr = (struct frag_hdr *)(skb_network_header(skb) +
>>                                 unfrag_ip6hlen);
>>
>>
>> I wonder about the !tunnel exception. This now seems to be a problem in sit
>> ufo output path, where we don't update fragmentation offsets any more thus
>> generating invalid frames.
>>
>> I am not too firm with segmentation in case of tunnels but don't we need to
>> always update the fragmentation offset no matter what, if upper gso callback
>> produced more segments?
>
> Not perfect nor clean (well, I don't know).
>
> The idea is to have the segmentation at the first guessed tunnel
> header cut. I don't know how to deal with stacked tunnels yet, I guess
> we need to have a bit more state in the skb. Just to maybe keep the discussion
> going...

Does the following patch help?

[-- Attachment #2: tmp.diff --]
[-- Type: text/plain, Size: 1484 bytes --]

diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 1e8683b..516b889 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -89,7 +89,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	unsigned int unfrag_ip6hlen;
 	u8 *prevhdr;
 	int offset = 0;
-	bool tunnel;
+	bool udpfrag, encap;
 	int nhoff;
 
 	if (unlikely(skb_shinfo(skb)->gso_type &
@@ -110,8 +110,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
 		goto out;
 
-	tunnel = SKB_GSO_CB(skb)->encap_level > 0;
-	if (tunnel)
+	encap = SKB_GSO_CB(skb)->encap_level > 0;
+	if (encap)
 		features = skb->dev->hw_enc_features & netif_skb_features(skb);
 	SKB_GSO_CB(skb)->encap_level += sizeof(*ipv6h);
 
@@ -130,16 +130,17 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	if (IS_ERR(segs))
 		goto out;
 
+	udpfrag = !!skb->encapsulation && proto == IPPROTO_UDP;
 	for (skb = segs; skb; skb = skb->next) {
 		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
 		ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
-		if (tunnel) {
+		if (encap) {
 			skb_reset_inner_headers(skb);
 			skb->encapsulation = 1;
 		}
 		skb->network_header = (u8 *)ipv6h - skb->head;
 
-		if (!tunnel && proto == IPPROTO_UDP) {
+		if (udpfrag) {
 			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
 			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
 			fptr->frag_off = htons(offset);

Re: linux 3.13: problems with isatap tunnel device and UFO

[Hannes Frederic Sowa]

On Thu, Feb 20, 2014 at 04:43:41PM -0800, Cong Wang wrote:
> On Wed, Feb 19, 2014 at 6:44 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > On Mon, Feb 17, 2014 at 05:09:16PM +0100, Hannes Frederic Sowa wrote:
> >> [+Cc Cong Wang]
> >>
> >> Hi Cong!
> >>
> >> In commit d949d826c09fb ("ipv6: Add generic UDP Tunnel segmentation") you
> >> patched ip6_offload.c:
> >>
> >> @@ -126,7 +128,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
> >>                 ipv6h = ipv6_hdr(skb);
> >>                 ipv6h->payload_len = htons(skb->len - skb->mac_len -
> >>                                            sizeof(*ipv6h));
> >> -               if (proto == IPPROTO_UDP) {
> >> +               if (!tunnel && proto == IPPROTO_UDP) {
> >>                         unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
> >>                         fptr = (struct frag_hdr *)(skb_network_header(skb) +
> >>                                 unfrag_ip6hlen);
> >>
> >>
> >> I wonder about the !tunnel exception. This now seems to be a problem in sit
> >> ufo output path, where we don't update fragmentation offsets any more thus
> >> generating invalid frames.
> >>
> >> I am not too firm with segmentation in case of tunnels but don't we need to
> >> always update the fragmentation offset no matter what, if upper gso callback
> >> produced more segments?
> >
> > Not perfect nor clean (well, I don't know).
> >
> > The idea is to have the segmentation at the first guessed tunnel
> > header cut. I don't know how to deal with stacked tunnels yet, I guess
> > we need to have a bit more state in the skb. Just to maybe keep the discussion
> > going...
> 
> Does the following patch help?

> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 1e8683b..516b889 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -89,7 +89,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>  	unsigned int unfrag_ip6hlen;
>  	u8 *prevhdr;
>  	int offset = 0;
> -	bool tunnel;
> +	bool udpfrag, encap;
>  	int nhoff;
>  
>  	if (unlikely(skb_shinfo(skb)->gso_type &
> @@ -110,8 +110,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>  	if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
>  		goto out;
>  
> -	tunnel = SKB_GSO_CB(skb)->encap_level > 0;
> -	if (tunnel)
> +	encap = SKB_GSO_CB(skb)->encap_level > 0;
> +	if (encap)
>  		features = skb->dev->hw_enc_features & netif_skb_features(skb);
>  	SKB_GSO_CB(skb)->encap_level += sizeof(*ipv6h);
>  
> @@ -130,16 +130,17 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>  	if (IS_ERR(segs))
>  		goto out;
>  
> +	udpfrag = !!skb->encapsulation && proto == IPPROTO_UDP;

The problem in general is that skb->encapsulation does not get reset as in GRE
or UDP_TUNNEL gso handler when traversing up the gso stack (e.g. from
another ipv6_gso_segment or inet_gso_segment.

skb->encapsulation is unreliable here.

Btw. IPv4 has the same problem and does not update fragment offsets,
so we cannot adapt the code from there.

Either we track inner_network_protocol in SKB_GSO_CB or try my proposal.

>  	for (skb = segs; skb; skb = skb->next) {
>  		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
>  		ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
> -		if (tunnel) {
> +		if (encap) {
>  			skb_reset_inner_headers(skb);
>  			skb->encapsulation = 1;
>  		}
>  		skb->network_header = (u8 *)ipv6h - skb->head;
>  
> -		if (!tunnel && proto == IPPROTO_UDP) {
> +		if (udpfrag) {
>  			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
>  			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
>  			fptr->frag_off = htons(offset);

Greetings,

  Hannes

[PATCH net] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling

[Hannes Frederic Sowa]

Currently the UFO fragmentation process does not correctly handle inner
UDP frames.

(The following tcpdumps are captured on the parent interface with ufo
disabled while tunnel has ufo enabled, 2000 bytes payload, mtu 1280,
both sit device):

IPv6:
16:39:10.031613 IP (tos 0x0, ttl 64, id 3208, offset 0, flags [DF], proto IPv6 (41), length 1300)
    192.168.122.151 > 1.1.1.1: IP6 (hlim 64, next-header Fragment (44) payload length: 1240) 2001::1 > 2001::8: frag (0x00000001:0|1232) 44883 > distinct: UDP, length 2000
16:39:10.031709 IP (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto IPv6 (41), length 844)
    192.168.122.151 > 1.1.1.1: IP6 (hlim 64, next-header Fragment (44) payload length: 784) 2001::1 > 2001::8: frag (0x00000001:0|776) 58979 > 46366: UDP, length 5471

We can see that fragmentation header offset is not correctly updated.
(fragmentation id handling is corrected by 916e4cf46d0204 ("ipv6: reuse
ip6_frag_id from ip6_ufo_append_data")).

IPv4:
16:39:57.737761 IP (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto IPIP (4), length 1296)
    192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id 57034, offset 0, flags [none], proto UDP (17), length 1276)
    192.168.99.1.35961 > 192.168.99.2.distinct: UDP, length 2000
16:39:57.738028 IP (tos 0x0, ttl 64, id 3210, offset 0, flags [DF], proto IPIP (4), length 792)
    192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id 57035, offset 0, flags [none], proto UDP (17), length 772)
    192.168.99.1.13531 > 192.168.99.2.20653: UDP, length 51109

In this case fragmentation id is incremented and offset is not updated.

First, I aligned inet_gso_segment and ipv6_gso_segment:
* align naming of flags
* ipv6_gso_segment: setting skb->encapsulation is unnecessary, as we
  always ensure that the state of this flag is left untouched when
  returning from upper gso segmenation function (in special udp_tunnel and
  gre)
* ipv6_gso_segment: move skb_reset_inner_headers below updating the
  fragmentation header data, we don't care for updating fragmentation
  header data
* remove currently unneeded comment indicating skb->encapsulation might
  get changed by upper gso_segment callback (gre and udp-tunnel reset
  encapsulation after segmentation on each fragment)

If we encounter an IPIP or SIT gso skb we now check for the protocol ==
IPPROTO_UDP and that we at least have already traversed another ip(6)
protocol header.

The reason why we have to special case GSO_IPIP and GSO_SIT is that
we reset skb->encapsulation to 0 while skb_mac_gso_segment the inner
protocol of GSO_UDP_TUNNEL or GSO_GRE packets.

Reported-by: Wolfgang Walter <linux@stwm.de>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Tom Herbert <therbert@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv4/af_inet.c     |  6 ++++--
 net/ipv6/ip6_offload.c | 20 ++++++++++++--------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index ecd2c3f..4661649 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1296,8 +1296,10 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 
 	segs = ERR_PTR(-EPROTONOSUPPORT);
 
-	/* Note : following gso_segment() might change skb->encapsulation */
-	udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
+		udpfrag = proto == IPPROTO_UDP && encap;
+	else
+		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
 
 	ops = rcu_dereference(inet_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment))
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 1e8683b..3d6dbde 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -89,7 +89,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	unsigned int unfrag_ip6hlen;
 	u8 *prevhdr;
 	int offset = 0;
-	bool tunnel;
+	bool encap, udpfrag;
 	int nhoff;
 
 	if (unlikely(skb_shinfo(skb)->gso_type &
@@ -110,8 +110,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
 		goto out;
 
-	tunnel = SKB_GSO_CB(skb)->encap_level > 0;
-	if (tunnel)
+	encap = SKB_GSO_CB(skb)->encap_level > 0;
+	if (encap)
 		features = skb->dev->hw_enc_features & netif_skb_features(skb);
 	SKB_GSO_CB(skb)->encap_level += sizeof(*ipv6h);
 
@@ -121,6 +121,12 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 
 	proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
 
+	/* Note : following gso_segment() might change skb->encapsulation */
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
+		udpfrag = proto == IPPROTO_UDP && encap;
+	else
+		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
+
 	ops = rcu_dereference(inet6_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment)) {
 		skb_reset_transport_header(skb);
@@ -133,13 +139,9 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	for (skb = segs; skb; skb = skb->next) {
 		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
 		ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
-		if (tunnel) {
-			skb_reset_inner_headers(skb);
-			skb->encapsulation = 1;
-		}
 		skb->network_header = (u8 *)ipv6h - skb->head;
 
-		if (!tunnel && proto == IPPROTO_UDP) {
+		if (udpfrag) {
 			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
 			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
 			fptr->frag_off = htons(offset);
@@ -148,6 +150,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 			offset += (ntohs(ipv6h->payload_len) -
 				   sizeof(struct frag_hdr));
 		}
+		if (encap)
+			skb_reset_inner_headers(skb);
 	}
 
 out:
-- 
1.8.5.3

[PATCH net v2] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling

[Hannes Frederic Sowa]

Currently the UFO fragmentation process does not correctly handle inner
UDP frames.

(The following tcpdumps are captured on the parent interface with ufo
disabled while tunnel has ufo enabled, 2000 bytes payload, mtu 1280,
both sit device):

IPv6:
16:39:10.031613 IP (tos 0x0, ttl 64, id 3208, offset 0, flags [DF], proto IPv6 (41), length 1300)
    192.168.122.151 > 1.1.1.1: IP6 (hlim 64, next-header Fragment (44) payload length: 1240) 2001::1 > 2001::8: frag (0x00000001:0|1232) 44883 > distinct: UDP, length 2000
16:39:10.031709 IP (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto IPv6 (41), length 844)
    192.168.122.151 > 1.1.1.1: IP6 (hlim 64, next-header Fragment (44) payload length: 784) 2001::1 > 2001::8: frag (0x00000001:0|776) 58979 > 46366: UDP, length 5471

We can see that fragmentation header offset is not correctly updated.
(fragmentation id handling is corrected by 916e4cf46d0204 ("ipv6: reuse
ip6_frag_id from ip6_ufo_append_data")).

IPv4:
16:39:57.737761 IP (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto IPIP (4), length 1296)
    192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id 57034, offset 0, flags [none], proto UDP (17), length 1276)
    192.168.99.1.35961 > 192.168.99.2.distinct: UDP, length 2000
16:39:57.738028 IP (tos 0x0, ttl 64, id 3210, offset 0, flags [DF], proto IPIP (4), length 792)
    192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id 57035, offset 0, flags [none], proto UDP (17), length 772)
    192.168.99.1.13531 > 192.168.99.2.20653: UDP, length 51109

In this case fragmentation id is incremented and offset is not updated.

First, I aligned inet_gso_segment and ipv6_gso_segment:
* align naming of flags
* ipv6_gso_segment: setting skb->encapsulation is unnecessary, as we
  always ensure that the state of this flag is left untouched when
  returning from upper gso segmenation function
* ipv6_gso_segment: move skb_reset_inner_headers below updating the
  fragmentation header data, we don't care for updating fragmentation
  header data
* remove currently unneeded comment indicating skb->encapsulation might
  get changed by upper gso_segment callback (gre and udp-tunnel reset
  encapsulation after segmentation on each fragment)

If we encounter an IPIP or SIT gso skb we now check for the protocol ==
IPPROTO_UDP and that we at least have already traversed another ip(6)
protocol header.

The reason why we have to special case GSO_IPIP and GSO_SIT is that
we reset skb->encapsulation to 0 while skb_mac_gso_segment the inner
protocol of GSO_UDP_TUNNEL or GSO_GRE packets.

Reported-by: Wolfgang Walter <linux@stwm.de>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Tom Herbert <therbert@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Changelog v2:

* Instead of removing the comment "/* Note : following gso_segment() might
  change skb->encapsulation */" I just moved it to the ip6_offload.c file by
  accident.

 net/ipv4/af_inet.c     |  6 ++++--
 net/ipv6/ip6_offload.c | 19 +++++++++++--------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index ecd2c3f..4661649 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1296,8 +1296,10 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 
 	segs = ERR_PTR(-EPROTONOSUPPORT);
 
-	/* Note : following gso_segment() might change skb->encapsulation */
-	udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
+		udpfrag = proto == IPPROTO_UDP && encap;
+	else
+		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
 
 	ops = rcu_dereference(inet_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment))
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 1e8683b..02281a1 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -89,7 +89,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	unsigned int unfrag_ip6hlen;
 	u8 *prevhdr;
 	int offset = 0;
-	bool tunnel;
+	bool encap, udpfrag;
 	int nhoff;
 
 	if (unlikely(skb_shinfo(skb)->gso_type &
@@ -110,8 +110,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
 		goto out;
 
-	tunnel = SKB_GSO_CB(skb)->encap_level > 0;
-	if (tunnel)
+	encap = SKB_GSO_CB(skb)->encap_level > 0;
+	if (encap)
 		features = skb->dev->hw_enc_features & netif_skb_features(skb);
 	SKB_GSO_CB(skb)->encap_level += sizeof(*ipv6h);
 
@@ -121,6 +121,11 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 
 	proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
 
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
+		udpfrag = proto == IPPROTO_UDP && encap;
+	else
+		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
+
 	ops = rcu_dereference(inet6_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment)) {
 		skb_reset_transport_header(skb);
@@ -133,13 +138,9 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	for (skb = segs; skb; skb = skb->next) {
 		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
 		ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
-		if (tunnel) {
-			skb_reset_inner_headers(skb);
-			skb->encapsulation = 1;
-		}
 		skb->network_header = (u8 *)ipv6h - skb->head;
 
-		if (!tunnel && proto == IPPROTO_UDP) {
+		if (udpfrag) {
 			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
 			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
 			fptr->frag_off = htons(offset);
@@ -148,6 +149,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 			offset += (ntohs(ipv6h->payload_len) -
 				   sizeof(struct frag_hdr));
 		}
+		if (encap)
+			skb_reset_inner_headers(skb);
 	}
 
 out:
-- 
1.8.5.3

[PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling

[Hannes Frederic Sowa]

Currently the UFO fragmentation process does not correctly handle inner
UDP frames.

(The following tcpdumps are captured on the parent interface with ufo
disabled while tunnel has ufo enabled, 2000 bytes payload, mtu 1280,
both sit device):

IPv6:
16:39:10.031613 IP (tos 0x0, ttl 64, id 3208, offset 0, flags [DF], proto IPv6 (41), length 1300)
    192.168.122.151 > 1.1.1.1: IP6 (hlim 64, next-header Fragment (44) payload length: 1240) 2001::1 > 2001::8: frag (0x00000001:0|1232) 44883 > distinct: UDP, length 2000
16:39:10.031709 IP (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto IPv6 (41), length 844)
    192.168.122.151 > 1.1.1.1: IP6 (hlim 64, next-header Fragment (44) payload length: 784) 2001::1 > 2001::8: frag (0x00000001:0|776) 58979 > 46366: UDP, length 5471

We can see that fragmentation header offset is not correctly updated.
(fragmentation id handling is corrected by 916e4cf46d0204 ("ipv6: reuse
ip6_frag_id from ip6_ufo_append_data")).

IPv4:
16:39:57.737761 IP (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto IPIP (4), length 1296)
    192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id 57034, offset 0, flags [none], proto UDP (17), length 1276)
    192.168.99.1.35961 > 192.168.99.2.distinct: UDP, length 2000
16:39:57.738028 IP (tos 0x0, ttl 64, id 3210, offset 0, flags [DF], proto IPIP (4), length 792)
    192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id 57035, offset 0, flags [none], proto UDP (17), length 772)
    192.168.99.1.13531 > 192.168.99.2.20653: UDP, length 51109

In this case fragmentation id is incremented and offset is not updated.

First, I aligned inet_gso_segment and ipv6_gso_segment:
* align naming of flags
* ipv6_gso_segment: setting skb->encapsulation is unnecessary, as we
  always ensure that the state of this flag is left untouched when
  returning from upper gso segmenation function
* ipv6_gso_segment: move skb_reset_inner_headers below updating the
  fragmentation header data, we don't care for updating fragmentation
  header data
* remove currently unneeded comment indicating skb->encapsulation might
  get changed by upper gso_segment callback (gre and udp-tunnel reset
  encapsulation after segmentation on each fragment)

If we encounter an IPIP or SIT gso skb we now check for the protocol ==
IPPROTO_UDP and that we at least have already traversed another ip(6)
protocol header.

The reason why we have to special case GSO_IPIP and GSO_SIT is that
we reset skb->encapsulation to 0 while skb_mac_gso_segment the inner
protocol of GSO_UDP_TUNNEL or GSO_GRE packets.

Reported-by: Wolfgang Walter <linux@stwm.de>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Tom Herbert <therbert@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Changelog v2:

* Instead of removing the comment "/* Note : following gso_segment() might
  change skb->encapsulation */" I just moved it to the ip6_offload.c file by
  accident.

Changelog v3 (only esthetic surgery):

* Added skb->encapsulation test to the condition where we test for GRE or
  UDP_TUNNEL gso packet, to be in line with udp_offload test for GSO_TUNNEL.
  Sorry for the noise, hopefully the last revision.

 net/ipv4/af_inet.c     |  7 +++++--
 net/ipv6/ip6_offload.c | 20 ++++++++++++--------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index ecd2c3f..19ab78a 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1296,8 +1296,11 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 
 	segs = ERR_PTR(-EPROTONOSUPPORT);
 
-	/* Note : following gso_segment() might change skb->encapsulation */
-	udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
+	if (skb->encapsulation &&
+	    skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
+		udpfrag = proto == IPPROTO_UDP && encap;
+	else
+		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
 
 	ops = rcu_dereference(inet_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment))
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 1e8683b..59f95af 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -89,7 +89,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	unsigned int unfrag_ip6hlen;
 	u8 *prevhdr;
 	int offset = 0;
-	bool tunnel;
+	bool encap, udpfrag;
 	int nhoff;
 
 	if (unlikely(skb_shinfo(skb)->gso_type &
@@ -110,8 +110,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
 		goto out;
 
-	tunnel = SKB_GSO_CB(skb)->encap_level > 0;
-	if (tunnel)
+	encap = SKB_GSO_CB(skb)->encap_level > 0;
+	if (encap)
 		features = skb->dev->hw_enc_features & netif_skb_features(skb);
 	SKB_GSO_CB(skb)->encap_level += sizeof(*ipv6h);
 
@@ -121,6 +121,12 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 
 	proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
 
+	if (skb->encapsulation &&
+	    skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
+		udpfrag = proto == IPPROTO_UDP && encap;
+	else
+		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
+
 	ops = rcu_dereference(inet6_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment)) {
 		skb_reset_transport_header(skb);
@@ -133,13 +139,9 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	for (skb = segs; skb; skb = skb->next) {
 		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
 		ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
-		if (tunnel) {
-			skb_reset_inner_headers(skb);
-			skb->encapsulation = 1;
-		}
 		skb->network_header = (u8 *)ipv6h - skb->head;
 
-		if (!tunnel && proto == IPPROTO_UDP) {
+		if (udpfrag) {
 			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
 			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
 			fptr->frag_off = htons(offset);
@@ -148,6 +150,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 			offset += (ntohs(ipv6h->payload_len) -
 				   sizeof(struct frag_hdr));
 		}
+		if (encap)
+			skb_reset_inner_headers(skb);
 	}
 
 out:
-- 
1.8.5.3

Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 24 Feb 2014 00:48:05 +0100

> Currently the UFO fragmentation process does not correctly handle inner
> UDP frames.
 ...
> In this case fragmentation id is incremented and offset is not updated.
> 
> First, I aligned inet_gso_segment and ipv6_gso_segment:
> * align naming of flags
> * ipv6_gso_segment: setting skb->encapsulation is unnecessary, as we
>   always ensure that the state of this flag is left untouched when
>   returning from upper gso segmenation function
> * ipv6_gso_segment: move skb_reset_inner_headers below updating the
>   fragmentation header data, we don't care for updating fragmentation
>   header data
> * remove currently unneeded comment indicating skb->encapsulation might
>   get changed by upper gso_segment callback (gre and udp-tunnel reset
>   encapsulation after segmentation on each fragment)
> 
> If we encounter an IPIP or SIT gso skb we now check for the protocol ==
> IPPROTO_UDP and that we at least have already traversed another ip(6)
> protocol header.
> 
> The reason why we have to special case GSO_IPIP and GSO_SIT is that
> we reset skb->encapsulation to 0 while skb_mac_gso_segment the inner
> protocol of GSO_UDP_TUNNEL or GSO_GRE packets.
> 
> Reported-by: Wolfgang Walter <linux@stwm.de>
> Cc: Cong Wang <xiyou.wangcong@gmail.com>
> Cc: Tom Herbert <therbert@google.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Applied, thanks Hannes.

Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling

[Hannes Frederic Sowa]

On Tue, Feb 25, 2014 at 06:27:46PM -0500, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Mon, 24 Feb 2014 00:48:05 +0100
> 
> > Currently the UFO fragmentation process does not correctly handle inner
> > UDP frames.
>  ...
> > In this case fragmentation id is incremented and offset is not updated.
> > 
> > First, I aligned inet_gso_segment and ipv6_gso_segment:
> > * align naming of flags
> > * ipv6_gso_segment: setting skb->encapsulation is unnecessary, as we
> >   always ensure that the state of this flag is left untouched when
> >   returning from upper gso segmenation function
> > * ipv6_gso_segment: move skb_reset_inner_headers below updating the
> >   fragmentation header data, we don't care for updating fragmentation
> >   header data
> > * remove currently unneeded comment indicating skb->encapsulation might
> >   get changed by upper gso_segment callback (gre and udp-tunnel reset
> >   encapsulation after segmentation on each fragment)
> > 
> > If we encounter an IPIP or SIT gso skb we now check for the protocol ==
> > IPPROTO_UDP and that we at least have already traversed another ip(6)
> > protocol header.
> > 
> > The reason why we have to special case GSO_IPIP and GSO_SIT is that
> > we reset skb->encapsulation to 0 while skb_mac_gso_segment the inner
> > protocol of GSO_UDP_TUNNEL or GSO_GRE packets.
> > 
> > Reported-by: Wolfgang Walter <linux@stwm.de>
> > Cc: Cong Wang <xiyou.wangcong@gmail.com>
> > Cc: Tom Herbert <therbert@google.com>
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> 
> Applied, thanks Hannes.

This bug is present and was reported on v3.13 kernels, so I also would propose
this for v3.13. I hoped it would be clear from the thread, but should have
stated this more clearly.

It really is only appropriate there as this problem was introduced with
61c1db7fae21ed ("ipv6: sit: add GSO/TSO support") and cb32f511a70be8
("ipip: add GSO/TSO support"), both introduced in v3.13.

Thanks,

  Hannes

Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Wed, 26 Feb 2014 14:47:35 +0100

> This bug is present and was reported on v3.13 kernels, so I also would propose
> this for v3.13. I hoped it would be clear from the thread, but should have
> stated this more clearly.
> 
> It really is only appropriate there as this problem was introduced with
> 61c1db7fae21ed ("ipv6: sit: add GSO/TSO support") and cb32f511a70be8
> ("ipip: add GSO/TSO support"), both introduced in v3.13.

Ok, queued up for -stable, thanks.

Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling

[Wolfgang Walter]

Hello Hannes,

my reply is a little bit late. I use the patch with 3.13.5 and 3.13.6 and it 
works fine with ISATAP. Thanks a lot.

Am Montag, 24. Februar 2014, 00:48:05 schrieb Hannes Frederic Sowa:
> Currently the UFO fragmentation process does not correctly handle inner
> UDP frames.
> 
> (The following tcpdumps are captured on the parent interface with ufo
> disabled while tunnel has ufo enabled, 2000 bytes payload, mtu 1280,
> both sit device):
> 
> IPv6:
> 16:39:10.031613 IP (tos 0x0, ttl 64, id 3208, offset 0, flags [DF], proto
> IPv6 (41), length 1300) 192.168.122.151 > 1.1.1.1: IP6 (hlim 64,
> next-header Fragment (44) payload length: 1240) 2001::1 > 2001::8: frag
> (0x00000001:0|1232) 44883 > distinct: UDP, length 2000 16:39:10.031709 IP
> (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto IPv6 (41), length
> 844) 192.168.122.151 > 1.1.1.1: IP6 (hlim 64, next-header Fragment (44)
> payload length: 784) 2001::1 > 2001::8: frag (0x00000001:0|776) 58979 >
> 46366: UDP, length 5471
> 
> We can see that fragmentation header offset is not correctly updated.
> (fragmentation id handling is corrected by 916e4cf46d0204 ("ipv6: reuse
> ip6_frag_id from ip6_ufo_append_data")).
> 
> IPv4:
> 16:39:57.737761 IP (tos 0x0, ttl 64, id 3209, offset 0, flags [DF], proto
> IPIP (4), length 1296) 192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id
> 57034, offset 0, flags [none], proto UDP (17), length 1276)
> 192.168.99.1.35961 > 192.168.99.2.distinct: UDP, length 2000
> 16:39:57.738028 IP (tos 0x0, ttl 64, id 3210, offset 0, flags [DF], proto
> IPIP (4), length 792) 192.168.122.151 > 1.1.1.1: IP (tos 0x0, ttl 64, id
> 57035, offset 0, flags [none], proto UDP (17), length 772)
> 192.168.99.1.13531 > 192.168.99.2.20653: UDP, length 51109
> 
> In this case fragmentation id is incremented and offset is not updated.
> 
> First, I aligned inet_gso_segment and ipv6_gso_segment:
> * align naming of flags
> * ipv6_gso_segment: setting skb->encapsulation is unnecessary, as we
>   always ensure that the state of this flag is left untouched when
>   returning from upper gso segmenation function
> * ipv6_gso_segment: move skb_reset_inner_headers below updating the
>   fragmentation header data, we don't care for updating fragmentation
>   header data
> * remove currently unneeded comment indicating skb->encapsulation might
>   get changed by upper gso_segment callback (gre and udp-tunnel reset
>   encapsulation after segmentation on each fragment)
> 
> If we encounter an IPIP or SIT gso skb we now check for the protocol ==
> IPPROTO_UDP and that we at least have already traversed another ip(6)
> protocol header.
> 
> The reason why we have to special case GSO_IPIP and GSO_SIT is that
> we reset skb->encapsulation to 0 while skb_mac_gso_segment the inner
> protocol of GSO_UDP_TUNNEL or GSO_GRE packets.
> 
> Reported-by: Wolfgang Walter <linux@stwm.de>
> Cc: Cong Wang <xiyou.wangcong@gmail.com>
> Cc: Tom Herbert <therbert@google.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> Changelog v2:
> 
> * Instead of removing the comment "/* Note : following gso_segment() might
>   change skb->encapsulation */" I just moved it to the ip6_offload.c file by
> accident.
> 
> Changelog v3 (only esthetic surgery):
> 
> * Added skb->encapsulation test to the condition where we test for GRE or
>   UDP_TUNNEL gso packet, to be in line with udp_offload test for GSO_TUNNEL.
> Sorry for the noise, hopefully the last revision.
> 
>  net/ipv4/af_inet.c     |  7 +++++--
>  net/ipv6/ip6_offload.c | 20 ++++++++++++--------
>  2 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index ecd2c3f..19ab78a 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1296,8 +1296,11 @@ static struct sk_buff *inet_gso_segment(struct
> sk_buff *skb,
> 
>  	segs = ERR_PTR(-EPROTONOSUPPORT);
> 
> -	/* Note : following gso_segment() might change skb->encapsulation */
> -	udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
> +	if (skb->encapsulation &&
> +	    skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
> +		udpfrag = proto == IPPROTO_UDP && encap;
> +	else
> +		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
> 
>  	ops = rcu_dereference(inet_offloads[proto]);
>  	if (likely(ops && ops->callbacks.gso_segment))
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 1e8683b..59f95af 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -89,7 +89,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff
> *skb, unsigned int unfrag_ip6hlen;
>  	u8 *prevhdr;
>  	int offset = 0;
> -	bool tunnel;
> +	bool encap, udpfrag;
>  	int nhoff;
> 
>  	if (unlikely(skb_shinfo(skb)->gso_type &
> @@ -110,8 +110,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff
> *skb, if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
>  		goto out;
> 
> -	tunnel = SKB_GSO_CB(skb)->encap_level > 0;
> -	if (tunnel)
> +	encap = SKB_GSO_CB(skb)->encap_level > 0;
> +	if (encap)
>  		features = skb->dev->hw_enc_features & netif_skb_features(skb);
>  	SKB_GSO_CB(skb)->encap_level += sizeof(*ipv6h);
> 
> @@ -121,6 +121,12 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff
> *skb,
> 
>  	proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
> 
> +	if (skb->encapsulation &&
> +	    skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
> +		udpfrag = proto == IPPROTO_UDP && encap;
> +	else
> +		udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
> +
>  	ops = rcu_dereference(inet6_offloads[proto]);
>  	if (likely(ops && ops->callbacks.gso_segment)) {
>  		skb_reset_transport_header(skb);
> @@ -133,13 +139,9 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff
> *skb, for (skb = segs; skb; skb = skb->next) {
>  		ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
>  		ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
> -		if (tunnel) {
> -			skb_reset_inner_headers(skb);
> -			skb->encapsulation = 1;
> -		}
>  		skb->network_header = (u8 *)ipv6h - skb->head;
> 
> -		if (!tunnel && proto == IPPROTO_UDP) {
> +		if (udpfrag) {
>  			unfrag_ip6hlen = ip6_find_1stfragopt(skb, &prevhdr);
>  			fptr = (struct frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
>  			fptr->frag_off = htons(offset);
> @@ -148,6 +150,8 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff
> *skb, offset += (ntohs(ipv6h->payload_len) -
>  				   sizeof(struct frag_hdr));
>  		}
> +		if (encap)
> +			skb_reset_inner_headers(skb);
>  	}
> 
>  out:

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts