On Mon, May 16, 2016 at 12:28 PM, Tom Herbert <t...@herbertland.com> wrote: > On Mon, May 16, 2016 at 12:24 PM, Alexander Duyck > <alexander.du...@gmail.com> wrote: >> On Sun, May 15, 2016 at 4:42 PM, Tom Herbert <t...@herbertland.com> wrote: >>> Add encap_hlen and ip_tunnel_encap structure to ip6_tnl. Add functions >>> for getting encap hlen, setting up encap on a tunnel, performing >>> encapsulation operation. >>> >>> Signed-off-by: Tom Herbert <t...@herbertland.com> >>> --- >>> include/net/ip6_tunnel.h | 58 ++++++++++++++++++++++++++++++ >>> net/ipv4/ip_tunnel_core.c | 5 +++ >>> net/ipv6/ip6_tunnel.c | 89 >>> +++++++++++++++++++++++++++++++++++++++++------ >>> 3 files changed, 141 insertions(+), 11 deletions(-) >> >> So a bisect is pointing to this patch as causing a regression in IPv6 >> GRE throughput from 20 Gb/s to .04 Mb/s >> >> <...> >> >>> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c >>> index e79330f..9f0ea85 100644 >>> --- a/net/ipv6/ip6_tunnel.c >>> +++ b/net/ipv6/ip6_tunnel.c >>> @@ -1010,7 +1010,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct >>> net_device *dev, __u8 dsfield, >>> struct dst_entry *dst = NULL, *ndst = NULL; >>> struct net_device *tdev; >>> int mtu; >>> - unsigned int max_headroom = sizeof(struct ipv6hdr); >>> + unsigned int max_headroom = sizeof(struct ipv6hdr) + t->hlen; >>> int err = -1; >>> >>> /* NBMA tunnel */ >>> @@ -1063,7 +1063,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct >>> net_device *dev, __u8 dsfield, >>> t->parms.name); >>> goto tx_err_dst_release; >>> } >>> - mtu = dst_mtu(dst) - sizeof(*ipv6h); >>> + mtu = dst_mtu(dst) - sizeof(*ipv6h) - t->hlen; >>> if (encap_limit >= 0) { >>> max_headroom += 8; >>> mtu -= 8; >> >> So I am pretty sure this bit here is causing the regression. Your skb >> already has a GRE header added and it is included in skb->len. In the >> tests just below here you are comparing skb->len to mtu, but you now >> have the GRE header included twice so it is going to fail. Odds are >> this should be t->encap_hlen, and not t->hlen. >> > Good catch! Fixing now...
Actually I think the one other case above for max_headroom probably should be encap_hlen as well. After all we don't need to allocate headroom for something we have already placed in the skb. I'm still digging into the patch set. If I find anything else I will let you know. I'm hoping to be able to test ip6ip6 hardware tunnel offloads by the end of today. - Alex