netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Fan Du <fengyuleidian0615@gmail.com>
To: John Heffner <johnwheffner@gmail.com>
Cc: Fan Du <fan.du@intel.com>, Eric Dumazet <edumazet@google.com>,
	David Miller <davem@davemloft.net>,
	Netdev <netdev@vger.kernel.org>
Subject: Re: [PATCHv3 net-next 3/4] ipv4: shrink current mss for tcp PMTU blackhole detection
Date: Tue, 03 Mar 2015 17:18:19 +0800	[thread overview]
Message-ID: <54F57C5B.3080709@gmail.com> (raw)
In-Reply-To: <CABrhC0nXCtWCgNm6Gz8UCXg=7Vt1pN0Q+aqO1TYZ74JK45Tq-g@mail.gmail.com>

于 2015年03月03日 04:32, John Heffner 写道:
> On Mon, Mar 2, 2015 at 4:29 AM, Fan Du<fengyuleidian0615@gmail.com>  wrote:
>> >Timeout indicates search_high should be set to the new mtu corresponding to
>> >current_mss no
>> >matter how we change search_low. So the best shot here IMO would be updating
>> >search_high
>> >with current_mss, which in return makes the search window*slide*  from right
>> >to left, and
>> >the probing will converge in good speed eventually.
>> >
>> >So my thoughts is:
>> >@@ -113,6 +113,7 @@ static void tcp_mtu_probing(struct inet_connection_sock
>> >*icsk, struct sock *sk)
>> >                         struct tcp_sock *tp = tcp_sk(sk);
>> >                         int mss;
>> >
>> >+                       icsk_mtup.search_high = tcp_mss_to_mtu(sk,
>> >tcp_current_mss(sk));
>> >                         mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)
>>>> >>>1;
>> >                         mss = min(net->ipv4.sysctl_tcp_base_mss, mss);
>> >                         mss = max(mss, 68 - tp->tcp_header_len);
> Search_high should be adjusted downward only when you're quite certain
> that you've gotten a negative signal.  There are many possible reasons
> for successive timeouts including intermittent disconnection, and they
> should not have a persistent (or very long-term) effect on MTU.  Leave
> search_high where it is until a working MTU can be established, then
> probe upward until probing can give you confidence you've found a new
> ceiling, or gotten back to the old one.
>
> If you think the current approach is broken, it would help to see a
> concrete demonstration of how it's deficient (a real packet trace is
> good!), and how a different approach work better.

 > With original approach(doubling mss), mss is not in between search_low and search_high,
 > it always equates search_low(subtract headers), the potential mss in case of blackhole
 > is 256 and 128, after doubling, it will become 512 and 256 eventually no matter how
 > route changes, even mtu reduced from 1500 to 1100 in intermediate node.

As for above statement, my test scenario is simple, using vxlan tunnel to connect two
docker instances on two hosts. All mtu default to 1500, run iperf between docker instances.
After the connection is setup, adjust the iperf sender host physical eth0 mtu to
1100, and after a couple of seconds, mss will be set to 512.

And after using binary search, actually, search_high will be set to probe_size trigger from
tcp_fastretrans_alert by my investigation. So the searching window does slide to left, thus
I will drop this patch. Maybe this is because my test method is not practical. And I believe
what you said about:
> There are many possible reasons for successive timeouts including intermittent disconnection,
 > and they should not have a persistent (or very long-term) effect on MTU.

Never mind, no matter whether to adjust search_high, the reprobe timer will restore search_high
to maximum allowed, and once again, new mss will be available.

I will resend the rest of patches after incorporating your comments for reviewing.
Thanks for your feedback.

  reply	other threads:[~2015-03-03  9:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-28  3:23 [PATCHv3 net-next 0/4] Improvements for TCP PMTU Fan Du
2015-02-28  3:23 ` [PATCHv3 net-next 1/4] ipv4: Raise tcp PMTU probe mss base size Fan Du
2015-02-28  3:23 ` [PATCHv3 net-next 2/4] ipv4: Use binary search to choose tcp PMTU probe_size Fan Du
2015-02-28 23:20   ` John Heffner
2015-03-02  9:29     ` Fan Du
2015-02-28  3:23 ` [PATCHv3 net-next 3/4] ipv4: shrink current mss for tcp PMTU blackhole detection Fan Du
2015-02-28 23:39   ` John Heffner
2015-03-02  9:29     ` Fan Du
2015-03-02 20:32       ` John Heffner
2015-03-03  9:18         ` Fan Du [this message]
2015-02-28  3:23 ` [PATCHv3 net-next 4/4] ipv4: Create probe timer for tcp PMTU as per RFC4821 Fan Du

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=54F57C5B.3080709@gmail.com \
    --to=fengyuleidian0615@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fan.du@intel.com \
    --cc=johnwheffner@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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).