On Sat, Jun 29, 2013 at 7:57 AM, Timo Teras [off-list ref] wrote:

Hi,

I'm reviewing changes since 3.9 to net-next and observed that, the
tunnel refactoring had the following change in ip_gre xmit path.

In ip_tunnel_xmit() mtu is now calculated as:
        if (df)
                mtu = dst_mtu(&rt->dst) - dev->hard_header_len
                                        - sizeof(struct iphdr);
        else
                mtu = skb_dst(skb) ? dst_mtu(skb_dst(skb)) : dev->mtu;

And it used to be in ip_gre.c: ipgre_tunnel_xmit():
        if (df)
                mtu = dst_mtu(&rt->dst) - dev->hard_header_len - tunnel->hlen;
        else
                mtu = skb_dst(skb) ? dst_mtu(skb_dst(skb)) : dev->mtu;

I notice that tunnel->hlen is replaced with sizeof(struct iphdr), but
in case of GRE those are not the same thing. And the refactored
ip_gre.c does not set hard_header_len either. So it would like the mtu
is now miscalculated (planning to give a full test-spin for net-next
next week).

It seems the tunnel->hlen used to be the full length, including
sizeof(struct iphdr).

But the new, refactored code seems exclude sizeof(struct iphdr) from
the tunnel->hlen. So would the following be appropriate?

This is ip-tunnel layer, skb has gre header pushed. so mtu does not
need to account gre header when compared to skb->len.
But I missed one comparison for mtu check where iph->tot-len is used
rather that skb-len, which is correct length.

gre module is using iph->tot_len for pmtu check which is wrong for
gre-tap device. This bug is there even before restructuring.
I will send out patch for ip-tunnels code for now.

Thanks.