* linux 3.13: problems with isatap tunnel device and UFO @ 2014-02-07 17:47 Wolfgang Walter 2014-02-07 17:56 ` Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Wolfgang Walter @ 2014-02-07 17:47 UTC (permalink / raw) To: netdev; +Cc: Hannes Frederic Sowa 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-07 17:47 linux 3.13: problems with isatap tunnel device and UFO Wolfgang Walter @ 2014-02-07 17:56 ` Hannes Frederic Sowa 2014-02-07 18:17 ` Wolfgang Walter 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-07 17:56 UTC (permalink / raw) To: Wolfgang Walter; +Cc: netdev 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-07 17:56 ` Hannes Frederic Sowa @ 2014-02-07 18:17 ` Wolfgang Walter 2014-02-07 22:22 ` Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Wolfgang Walter @ 2014-02-07 18:17 UTC (permalink / raw) To: Hannes Frederic Sowa; +Cc: netdev 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-07 18:17 ` Wolfgang Walter @ 2014-02-07 22:22 ` Hannes Frederic Sowa 2014-02-08 23:17 ` Wolfgang Walter 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-07 22:22 UTC (permalink / raw) To: Wolfgang Walter; +Cc: netdev 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-07 22:22 ` Hannes Frederic Sowa @ 2014-02-08 23:17 ` Wolfgang Walter 2014-02-11 2:44 ` Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Wolfgang Walter @ 2014-02-08 23:17 UTC (permalink / raw) To: netdev, Hannes Frederic Sowa 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-08 23:17 ` Wolfgang Walter @ 2014-02-11 2:44 ` Hannes Frederic Sowa 2014-02-11 17:42 ` Wolfgang Walter 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-11 2:44 UTC (permalink / raw) To: Wolfgang Walter; +Cc: netdev 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-11 2:44 ` Hannes Frederic Sowa @ 2014-02-11 17:42 ` Wolfgang Walter 2014-02-17 16:09 ` Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Wolfgang Walter @ 2014-02-11 17:42 UTC (permalink / raw) To: netdev; +Cc: Hannes Frederic Sowa 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-11 17:42 ` Wolfgang Walter @ 2014-02-17 16:09 ` Hannes Frederic Sowa 2014-02-20 2:44 ` Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-17 16:09 UTC (permalink / raw) To: Wolfgang Walter; +Cc: netdev, xiyou.wangcong [+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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-17 16:09 ` Hannes Frederic Sowa @ 2014-02-20 2:44 ` Hannes Frederic Sowa 2014-02-20 2:54 ` Hannes Frederic Sowa 2014-02-21 0:43 ` Cong Wang 0 siblings, 2 replies; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-20 2:44 UTC (permalink / raw) To: Wolfgang Walter, netdev, xiyou.wangcong, eric.dumazet 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: ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-20 2:44 ` Hannes Frederic Sowa @ 2014-02-20 2:54 ` Hannes Frederic Sowa 2014-02-21 0:43 ` Cong Wang 1 sibling, 0 replies; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-20 2:54 UTC (permalink / raw) To: Wolfgang Walter, netdev, xiyou.wangcong, eric.dumazet 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... :/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-20 2:44 ` Hannes Frederic Sowa 2014-02-20 2:54 ` Hannes Frederic Sowa @ 2014-02-21 0:43 ` Cong Wang 2014-02-21 1:19 ` Hannes Frederic Sowa 1 sibling, 1 reply; 19+ messages in thread From: Cong Wang @ 2014-02-21 0:43 UTC (permalink / raw) To: Wolfgang Walter, Linux Kernel Network Developers, Cong Wang, Eric Dumazet [-- 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); ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: linux 3.13: problems with isatap tunnel device and UFO 2014-02-21 0:43 ` Cong Wang @ 2014-02-21 1:19 ` Hannes Frederic Sowa 2014-02-23 21:05 ` [PATCH net] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-21 1:19 UTC (permalink / raw) To: Cong Wang; +Cc: Wolfgang Walter, Linux Kernel Network Developers, Eric Dumazet 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling 2014-02-21 1:19 ` Hannes Frederic Sowa @ 2014-02-23 21:05 ` Hannes Frederic Sowa 2014-02-23 21:24 ` [PATCH net v2] " Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-23 21:05 UTC (permalink / raw) To: Cong Wang, Wolfgang Walter, Linux Kernel Network Developers, Eric Dumazet 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH net v2] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling 2014-02-23 21:05 ` [PATCH net] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling Hannes Frederic Sowa @ 2014-02-23 21:24 ` Hannes Frederic Sowa 2014-02-23 23:48 ` [PATCH net v3] " Hannes Frederic Sowa 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-23 21:24 UTC (permalink / raw) To: Cong Wang, Wolfgang Walter, Linux Kernel Network Developers, Eric Dumazet 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling 2014-02-23 21:24 ` [PATCH net v2] " Hannes Frederic Sowa @ 2014-02-23 23:48 ` Hannes Frederic Sowa 2014-02-25 23:27 ` David Miller 2014-03-07 14:13 ` Wolfgang Walter 0 siblings, 2 replies; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-23 23:48 UTC (permalink / raw) To: Cong Wang, Wolfgang Walter, Linux Kernel Network Developers, Eric Dumazet, therbert 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling 2014-02-23 23:48 ` [PATCH net v3] " Hannes Frederic Sowa @ 2014-02-25 23:27 ` David Miller 2014-02-26 13:47 ` Hannes Frederic Sowa 2014-03-07 14:13 ` Wolfgang Walter 1 sibling, 1 reply; 19+ messages in thread From: David Miller @ 2014-02-25 23:27 UTC (permalink / raw) To: hannes; +Cc: xiyou.wangcong, linux, netdev, eric.dumazet, therbert 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling 2014-02-25 23:27 ` David Miller @ 2014-02-26 13:47 ` Hannes Frederic Sowa 2014-02-26 18:45 ` David Miller 0 siblings, 1 reply; 19+ messages in thread From: Hannes Frederic Sowa @ 2014-02-26 13:47 UTC (permalink / raw) To: David Miller; +Cc: xiyou.wangcong, linux, netdev, eric.dumazet, therbert 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling 2014-02-26 13:47 ` Hannes Frederic Sowa @ 2014-02-26 18:45 ` David Miller 0 siblings, 0 replies; 19+ messages in thread From: David Miller @ 2014-02-26 18:45 UTC (permalink / raw) To: hannes; +Cc: xiyou.wangcong, linux, netdev, eric.dumazet, therbert 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling 2014-02-23 23:48 ` [PATCH net v3] " Hannes Frederic Sowa 2014-02-25 23:27 ` David Miller @ 2014-03-07 14:13 ` Wolfgang Walter 1 sibling, 0 replies; 19+ messages in thread From: Wolfgang Walter @ 2014-03-07 14:13 UTC (permalink / raw) To: Hannes Frederic Sowa, Linux Kernel Network Developers 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2014-03-07 14:13 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-02-07 17:47 linux 3.13: problems with isatap tunnel device and UFO Wolfgang Walter 2014-02-07 17:56 ` Hannes Frederic Sowa 2014-02-07 18:17 ` Wolfgang Walter 2014-02-07 22:22 ` Hannes Frederic Sowa 2014-02-08 23:17 ` Wolfgang Walter 2014-02-11 2:44 ` Hannes Frederic Sowa 2014-02-11 17:42 ` Wolfgang Walter 2014-02-17 16:09 ` Hannes Frederic Sowa 2014-02-20 2:44 ` Hannes Frederic Sowa 2014-02-20 2:54 ` Hannes Frederic Sowa 2014-02-21 0:43 ` Cong Wang 2014-02-21 1:19 ` Hannes Frederic Sowa 2014-02-23 21:05 ` [PATCH net] ipv4: ipv6: better estimate tunnel header cut for correct ufo handling Hannes Frederic Sowa 2014-02-23 21:24 ` [PATCH net v2] " Hannes Frederic Sowa 2014-02-23 23:48 ` [PATCH net v3] " Hannes Frederic Sowa 2014-02-25 23:27 ` David Miller 2014-02-26 13:47 ` Hannes Frederic Sowa 2014-02-26 18:45 ` David Miller 2014-03-07 14:13 ` Wolfgang Walter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.