* [PATCH net-next] ip6_gre: Fix MTU setting
@ 2016-05-09 22:28 Tom Herbert
2016-05-09 22:44 ` Alexander Duyck
0 siblings, 1 reply; 6+ messages in thread
From: Tom Herbert @ 2016-05-09 22:28 UTC (permalink / raw)
To: davem, netdev, alexander.duyck; +Cc: kernel-team
In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
configuration. For hard_header_len and mtu calculation include
IPv6 header and encapsulation overhead.
In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
the configuration. Revert to setting hard_header_len instead of
needed_headroom.
Tested:
./ip link add name tun8 type ip6gretap remote \
2401:db00:20:911a:face:0:27:0 local \
2401:db00:20:911a:face:0:25:0 ttl 225
Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
./ip link add name tun8 type ip6gretap remote \
2401:db00:20:911a:face:0:27:0 local \
2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
net/ipv6/ip6_gre.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 47b671a..6d0aa94 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -700,7 +700,7 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
struct net_device *dev = t->dev;
struct __ip6_tnl_parm *p = &t->parms;
struct flowi6 *fl6 = &t->fl.u.ip6;
- int addend = sizeof(struct ipv6hdr) + 4;
+ int t_hlen;
if (dev->type != ARPHRD_ETHER) {
memcpy(dev->dev_addr, &p->laddr, sizeof(struct in6_addr));
@@ -727,16 +727,11 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
else
dev->flags &= ~IFF_POINTOPOINT;
- /* Precalculate GRE options length */
- if (t->parms.o_flags&(GRE_CSUM|GRE_KEY|GRE_SEQ)) {
- if (t->parms.o_flags&GRE_CSUM)
- addend += 4;
- if (t->parms.o_flags&GRE_KEY)
- addend += 4;
- if (t->parms.o_flags&GRE_SEQ)
- addend += 4;
- }
- t->hlen = addend;
+ t->tun_hlen = gre_calc_hlen(t->parms.o_flags);
+
+ t->hlen = t->tun_hlen;
+
+ t_hlen = t->hlen + sizeof(struct ipv6hdr);
if (p->flags & IP6_TNL_F_CAP_XMIT) {
int strict = (ipv6_addr_type(&p->raddr) &
@@ -750,10 +745,11 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
return;
if (rt->dst.dev) {
- dev->hard_header_len = rt->dst.dev->hard_header_len + addend;
+ dev->hard_header_len = rt->dst.dev->hard_header_len +
+ t_hlen;
if (set_mtu) {
- dev->mtu = rt->dst.dev->mtu - addend;
+ dev->mtu = rt->dst.dev->mtu - t_hlen;
if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
dev->mtu -= 8;
if (dev->type == ARPHRD_ETHER)
@@ -1027,11 +1023,12 @@ static int ip6gre_tunnel_init_common(struct net_device *dev)
tunnel->tun_hlen = gre_calc_hlen(tunnel->parms.o_flags);
- t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
+ tunnel->hlen = tunnel->tun_hlen;
- dev->needed_headroom = LL_MAX_HEADER + t_hlen + 4;
- dev->mtu = ETH_DATA_LEN - t_hlen - 4;
+ t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
+ dev->hard_header_len = LL_MAX_HEADER + t_hlen;
+ dev->mtu = ETH_DATA_LEN - t_hlen;
if (!(tunnel->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
dev->mtu -= 8;
--
2.8.0.rc2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ip6_gre: Fix MTU setting
2016-05-09 22:28 [PATCH net-next] ip6_gre: Fix MTU setting Tom Herbert
@ 2016-05-09 22:44 ` Alexander Duyck
2016-05-09 22:49 ` Tom Herbert
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Duyck @ 2016-05-09 22:44 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
> configuration. For hard_header_len and mtu calculation include
> IPv6 header and encapsulation overhead.
>
> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
> the configuration. Revert to setting hard_header_len instead of
> needed_headroom.
>
> Tested:
>
> ./ip link add name tun8 type ip6gretap remote \
> 2401:db00:20:911a:face:0:27:0 local \
> 2401:db00:20:911a:face:0:25:0 ttl 225
>
> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>
> ./ip link add name tun8 type ip6gretap remote \
> 2401:db00:20:911a:face:0:27:0 local \
> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>
> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>
So this gives me the correct result now. However we still need patch
3 / 11 from the set you submitted next week in order to be able to
even transmit because the flags are otherwise mangled.
Tested-by: Alexander Duyck <aduyck@mirantis.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ip6_gre: Fix MTU setting
2016-05-09 22:44 ` Alexander Duyck
@ 2016-05-09 22:49 ` Tom Herbert
2016-05-09 23:10 ` Alexander Duyck
0 siblings, 1 reply; 6+ messages in thread
From: Tom Herbert @ 2016-05-09 22:49 UTC (permalink / raw)
To: Alexander Duyck; +Cc: David Miller, Netdev, Kernel Team
On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>> configuration. For hard_header_len and mtu calculation include
>> IPv6 header and encapsulation overhead.
>>
>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>> the configuration. Revert to setting hard_header_len instead of
>> needed_headroom.
>>
>> Tested:
>>
>> ./ip link add name tun8 type ip6gretap remote \
>> 2401:db00:20:911a:face:0:27:0 local \
>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>
>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>
>> ./ip link add name tun8 type ip6gretap remote \
>> 2401:db00:20:911a:face:0:27:0 local \
>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>
>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>
>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>
> So this gives me the correct result now. However we still need patch
> 3 / 11 from the set you submitted next week in order to be able to
> even transmit because the flags are otherwise mangled.
>
> Tested-by: Alexander Duyck <aduyck@mirantis.com>
Okay, I'll repost the flags patch as a standalone. Do you have any
other issues then in this path?
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ip6_gre: Fix MTU setting
2016-05-09 22:49 ` Tom Herbert
@ 2016-05-09 23:10 ` Alexander Duyck
2016-05-09 23:14 ` Alexander Duyck
2016-05-09 23:47 ` Tom Herbert
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Duyck @ 2016-05-09 23:10 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
On Mon, May 9, 2016 at 3:49 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>>> configuration. For hard_header_len and mtu calculation include
>>> IPv6 header and encapsulation overhead.
>>>
>>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>>> the configuration. Revert to setting hard_header_len instead of
>>> needed_headroom.
>>>
>>> Tested:
>>>
>>> ./ip link add name tun8 type ip6gretap remote \
>>> 2401:db00:20:911a:face:0:27:0 local \
>>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>>
>>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>>
>>> ./ip link add name tun8 type ip6gretap remote \
>>> 2401:db00:20:911a:face:0:27:0 local \
>>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>>
>>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>>
>>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>>
>> So this gives me the correct result now. However we still need patch
>> 3 / 11 from the set you submitted next week in order to be able to
>> even transmit because the flags are otherwise mangled.
>>
>> Tested-by: Alexander Duyck <aduyck@mirantis.com>
>
> Okay, I'll repost the flags patch as a standalone. Do you have any
> other issues then in this path?
This piece no.
Like I said in the other email I did find two other issues. First in
__gre6_xmit I think you want to set the inner protocol to "protocol",
not "proto". Second is the issue with ip6_tnl_xmit stomping on
skb->transport_header. The second one is the big one that is blocking
things badly right now. That is the one that keeps me from doing
anything with any sort of offload as the headers are jumping around
out of order with the outer transport header having an offset that is
further in than the inner mac header.
I'd be fine with you submitting the whole set as one piece with the
other 11 patches you had just as long as we get those core pieces
fixed in the patch set.
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ip6_gre: Fix MTU setting
2016-05-09 23:10 ` Alexander Duyck
@ 2016-05-09 23:14 ` Alexander Duyck
2016-05-09 23:47 ` Tom Herbert
1 sibling, 0 replies; 6+ messages in thread
From: Alexander Duyck @ 2016-05-09 23:14 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
On Mon, May 9, 2016 at 4:10 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 9, 2016 at 3:49 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>>>> configuration. For hard_header_len and mtu calculation include
>>>> IPv6 header and encapsulation overhead.
>>>>
>>>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>>>> the configuration. Revert to setting hard_header_len instead of
>>>> needed_headroom.
>>>>
>>>> Tested:
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>>>
>>>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>>>
>>>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>>>
>>>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>>>
>>> So this gives me the correct result now. However we still need patch
>>> 3 / 11 from the set you submitted next week in order to be able to
>>> even transmit because the flags are otherwise mangled.
>>>
>>> Tested-by: Alexander Duyck <aduyck@mirantis.com>
>>
>> Okay, I'll repost the flags patch as a standalone. Do you have any
>> other issues then in this path?
>
> This piece no.
Actually I just realized there are two other bits that need to be updated.
I just copied/pasted the below out of a text console so I am sure the
white-space formatting is wrong. These two spots should be fixed as
well as they are using the wrong flag type.
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index ec209f4d3312..ee62ec469ab3 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -343,7 +343,7 @@ static struct ip6_tnl *ip6gre_tunnel_locate(struct net *net,
goto failed_free;
/* Can use a lockless transmit, unless we generate output sequences */
- if (!(nt->parms.o_flags & GRE_SEQ))
+ if (!(nt->parms.o_flags & TUNNEL_SEQ))
dev->features |= NETIF_F_LLTX;
dev_hold(dev);
@@ -1314,7 +1314,7 @@ static int ip6gre_newlink(struct net *src_net,
struct net_device *dev,
dev->features |= GRE6_FEATURES;
dev->hw_features |= GRE6_FEATURES;
- if (!(nt->parms.o_flags & GRE_SEQ)) {
+ if (!(nt->parms.o_flags & TUNNEL_SEQ)) {
/* TCP segmentation offload is not supported when we
* generate output sequences.
*/
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ip6_gre: Fix MTU setting
2016-05-09 23:10 ` Alexander Duyck
2016-05-09 23:14 ` Alexander Duyck
@ 2016-05-09 23:47 ` Tom Herbert
1 sibling, 0 replies; 6+ messages in thread
From: Tom Herbert @ 2016-05-09 23:47 UTC (permalink / raw)
To: Alexander Duyck; +Cc: David Miller, Netdev, Kernel Team
On Mon, May 9, 2016 at 4:10 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 9, 2016 at 3:49 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>>>> configuration. For hard_header_len and mtu calculation include
>>>> IPv6 header and encapsulation overhead.
>>>>
>>>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>>>> the configuration. Revert to setting hard_header_len instead of
>>>> needed_headroom.
>>>>
>>>> Tested:
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>>>
>>>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>>>
>>>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>>>
>>>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>>>
>>> So this gives me the correct result now. However we still need patch
>>> 3 / 11 from the set you submitted next week in order to be able to
>>> even transmit because the flags are otherwise mangled.
>>>
>>> Tested-by: Alexander Duyck <aduyck@mirantis.com>
>>
>> Okay, I'll repost the flags patch as a standalone. Do you have any
>> other issues then in this path?
>
> This piece no.
>
> Like I said in the other email I did find two other issues. First in
> __gre6_xmit I think you want to set the inner protocol to "protocol",
> not "proto". Second is the issue with ip6_tnl_xmit stomping on
> skb->transport_header. The second one is the big one that is blocking
> things badly right now. That is the one that keeps me from doing
> anything with any sort of offload as the headers are jumping around
> out of order with the outer transport header having an offset that is
> further in than the inner mac header.
>
You mean "skb->transport_header = skb->network_header;"? It doesn't
look right to me either, but that line has been around a while....
> I'd be fine with you submitting the whole set as one piece with the
> other 11 patches you had just as long as we get those core pieces
> fixed in the patch set.
>
Probably just as easy to break it up.
> - Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-05-09 23:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-09 22:28 [PATCH net-next] ip6_gre: Fix MTU setting Tom Herbert
2016-05-09 22:44 ` Alexander Duyck
2016-05-09 22:49 ` Tom Herbert
2016-05-09 23:10 ` Alexander Duyck
2016-05-09 23:14 ` Alexander Duyck
2016-05-09 23:47 ` Tom Herbert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox