netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: Jesse Gross <jesse@nicira.com>
Cc: Pravin Shelar <pshelar@nicira.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	netdev <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>
Subject: Re: [PATCH net] gre: Fix MTU sizing check for gretap tunnels
Date: Thu, 11 Jul 2013 17:13:50 -0700	[thread overview]
Message-ID: <51DF4A3E.8070102@gmail.com> (raw)
In-Reply-To: <CAEP_g=9oLO-LO8UD01VdGESRJrE2yhAPFpuoO_Qrwh=G7FvdhQ@mail.gmail.com>

On 07/11/2013 04:36 PM, Jesse Gross wrote:
> On Thu, Jul 11, 2013 at 4:19 PM, Pravin Shelar <pshelar@nicira.com> wrote:
>> On Thu, Jul 11, 2013 at 3:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> On Thu, 2013-07-11 at 15:24 -0700, Pravin Shelar wrote:
>>>> On Thu, Jul 11, 2013 at 1:12 PM, Alexander Duyck
>>>> <alexander.h.duyck@intel.com> wrote:
>>>>> This change fixes an MTU sizing issue seen with gretap tunnels when non-gso
>>>>> packets are sent from the interface.
>>>>>
>>>>> In my case I was able to reproduce the issue by simply sending a ping of
>>>>> 1421 bytes with the gretap interface created on a device with a standard
>>>>> 1500 mtu.
>>>>>
>>>>> This fix is based on the fact that the tunnel mtu is already adjusted by
>>>>> dev->hard_header_len so it would make sense that any packets being compared
>>>>> against that mtu should also be adjusted by hard_header_len and the tunnel
>>>>> header instead of just the tunnel header.
>>>>>
>>>> we can simplify code by not doing dev->hard_header_len adjustment to tunnel-mtu.
>>>>
>>>> And right thing would be adjusting tunnel-mtu according to rt->dst.dev
>>>> header-len so that we get mtu for out going path.
>>> What's the mtu value we want to put in the ICMP message ?
>>>
>>>
>> I think it should be max payload that tunnel-device can take for that
>> route. Something like (route_mtu - (tunnel_header_len + iph_len +
>> route_dev->header_len))
>>
>> gre is been using dev->hard_header_len rather than
>> rt_dev->hard_header_len for long time which does not look right.
> I think that it is trying to use the tunnel device's header length to
> get the payload length. The MTU of the output device should already
> take into account its header length. I agree that the code is hard to
> read right now though

That is what I assume as well.  The only issue was the fact that after
the recent changes the calculation was off as it was including the
Ethernet header size when calculating the network packet size in the
case of gretap.  Prior to recent changes this code just pulled the
network packet size out of the network header when comparing it to the MTU.

Thanks,

Alex

  reply	other threads:[~2013-07-12  0:13 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-11 20:12 [PATCH net] gre: Fix MTU sizing check for gretap tunnels Alexander Duyck
2013-07-11 21:52 ` Eric Dumazet
2013-07-11 23:30   ` David Miller
2013-07-11 22:24 ` Pravin Shelar
2013-07-11 22:45   ` Eric Dumazet
2013-07-11 23:19     ` Pravin Shelar
2013-07-11 23:30       ` David Miller
2013-07-11 23:36       ` Jesse Gross
2013-07-12  0:13         ` Alexander Duyck [this message]
2013-07-12  1:11 ` Cong Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51DF4A3E.8070102@gmail.com \
    --to=alexander.duyck@gmail.com \
    --cc=alexander.h.duyck@intel.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=jesse@nicira.com \
    --cc=netdev@vger.kernel.org \
    --cc=pshelar@nicira.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).