netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: Florian Westphal <fw@strlen.de>,
	netdev@vger.kernel.org, aconole@redhat.com
Subject: Re: [PATCH net-next 1/3] udp_tunnel: allow to turn off path mtu discovery on encap sockets
Date: Sat, 18 Jul 2020 11:02:46 -0600	[thread overview]
Message-ID: <9e47f521-b3dc-f116-658b-d6897b0ddf20@gmail.com> (raw)
In-Reply-To: <20200718085645.7420da02@elisabeth>

On 7/18/20 12:56 AM, Stefano Brivio wrote:
> On Fri, 17 Jul 2020 09:04:51 -0600
> David Ahern <dsahern@gmail.com> wrote:
> 
>> On 7/17/20 6:27 AM, Stefano Brivio wrote:
>>>>  
>>>>> Note that this doesn't work as it is because of a number of reasons
>>>>> (skb doesn't have a dst, pkt_type is not PACKET_HOST), and perhaps we
>>>>> shouldn't be using icmp_send(), but at a glance that looks simpler.    
>>>>
>>>> Yes, it also requires that the bridge has IP connectivity
>>>> to reach the inner ip, which might not be the case.  
>>>
>>> If the VXLAN endpoint is a port of the bridge, that needs to be the
>>> case, right? Otherwise the VXLAN endpoint can't be reached.
>>>   
>>>>> Another slight preference I have towards this idea is that the only
>>>>> known way we can break PMTU discovery right now is by using a bridge,
>>>>> so fixing the problem there looks more future-proof than addressing any
>>>>> kind of tunnel with this problem. I think FoU and GUE would hit the
>>>>> same problem, I don't know about IP tunnels, sticking that selftest
>>>>> snippet to whatever other test in pmtu.sh should tell.    
>>>>
>>>> Every type of bridge port that needs to add additional header on egress
>>>> has this problem in the bridge scenario once the peer of the IP tunnel
>>>> signals a PMTU event.  
>>>
>>> Yes :(
>>
>> The vxlan/tunnel device knows it is a bridge port, and it knows it is
>> going to push a udp and ip{v6} header. So why not use that information
>> in setting / updating the MTU? That's what I was getting at on Monday
>> with my comment about lwtunnel_headroom equivalent.
> 
> If I understand correctly, you're proposing something similar to my
> earlier draft from:
> 
> 	<20200713003813.01f2d5d3@elisabeth>
> 	https://lore.kernel.org/netdev/20200713003813.01f2d5d3@elisabeth/
> 
> the problem with it is that it wouldn't help: the MTU is already set to
> the right value for both port and bridge in the case Florian originally
> reported.

I am definitely hand waving; I have not had time to create a setup
showing the problem. Is there a reproducer using only namespaces?

> 
> Also, given the implications on overriding configured MTUs, and
> introducing (further) IP logic into the bridge, if Florian's idea of
> injecting ICMP messages could be implemented in a generic function:
> 
> On Wed, 15 Jul 2020 16:33:56 +0200
> Florian Westphal <fw@strlen.de> wrote:
> 
>> Yes, it might be possible to move the proposed icmp inject into
>> skb_tunnel_check_pmtu() -- it gets the needed headroom passed as arg,
>> it could detect when device driver is in a bridge and it already knows
>> when skb has no dst entry that it a pmtu change could be propagated to.
> 
> I think that would be preferable: then it's fixed for all tunnels in a
> generic, probably simpler way, without those two issues.
> 
> But then again, we're talking about Linux bridge. Unfortunately this
> doesn't fix the problem with Open vSwitch either.
> 

Of course. (insert sarcastic jab at ovs here)

  reply	other threads:[~2020-07-18 17:04 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-12 20:07 [PATCH net-next 0/3] vxlan, geneve: allow to turn off PMTU updates on encap socket Florian Westphal
2020-07-12 20:07 ` [PATCH net-next 1/3] udp_tunnel: allow to turn off path mtu discovery on encap sockets Florian Westphal
2020-07-12 22:38   ` Stefano Brivio
2020-07-13  8:04     ` Florian Westphal
2020-07-13 10:04       ` Stefano Brivio
2020-07-13 10:51         ` Numan Siddique
2020-07-14 20:38           ` Aaron Conole
2020-07-15 11:58             ` Stefano Brivio
2020-07-13 13:25       ` David Ahern
2020-07-13 14:02         ` Florian Westphal
2020-07-13 14:41           ` David Ahern
2020-07-13 14:59             ` Florian Westphal
2020-07-13 15:57               ` Stefano Brivio
2020-07-13 16:22                 ` Florian Westphal
2020-07-14 12:33                   ` Stefano Brivio
2020-07-14 12:33           ` Stefano Brivio
2020-07-15 12:42             ` Florian Westphal
2020-07-15 13:35               ` Stefano Brivio
2020-07-15 14:33                 ` Florian Westphal
2020-07-17 12:27                   ` Stefano Brivio
2020-07-17 15:04                     ` David Ahern
2020-07-17 18:43                       ` Florian Westphal
2020-07-18  6:56                       ` Stefano Brivio
2020-07-18 17:02                         ` David Ahern [this message]
2020-07-18 17:58                           ` Stefano Brivio
2020-07-18 18:04                             ` Stefano Brivio
2020-07-19 18:43                             ` David Ahern
2020-07-19 21:49                               ` Stefano Brivio
2020-07-20  3:19                                 ` David Ahern
2020-07-26 17:01                                   ` Stefano Brivio
2020-07-12 20:07 ` [PATCH net-next 2/3] vxlan: allow to disable path mtu learning on encap socket Florian Westphal
2020-07-16 19:33   ` Jakub Kicinski
2020-07-17 10:13     ` Florian Westphal
2020-07-12 20:07 ` [PATCH net-next 3/3] geneve: allow disabling of pmtu detection on encap sk Florian Westphal
2020-07-12 22:39 ` [PATCH net-next 0/3] vxlan, geneve: allow to turn off PMTU updates on encap socket Stefano Brivio

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=9e47f521-b3dc-f116-658b-d6897b0ddf20@gmail.com \
    --to=dsahern@gmail.com \
    --cc=aconole@redhat.com \
    --cc=fw@strlen.de \
    --cc=netdev@vger.kernel.org \
    --cc=sbrivio@redhat.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).