MTU probing bug?

MTU probing bug?

[David Miller]

John, have a look at this code in tcp_write_timeout():

		mss = min(sysctl_tcp_base_mss,
		          tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2);
		mss = max(mss, 68 - tp->tcp_header_len);

That first line looks like it should be a max() instead
of a min().

tcp_base_mss is the smallest MSS we should use, therefore
we should make sure the "mss" is at least that large.

It's also possible that I misread the intention of this code :) From
what I read, it is trying to half the MSS in use and adjust the MTU
search low point to be based upon this new value.

Re: MTU probing bug?

[John Heffner]

David Miller wrote:
> John, have a look at this code in tcp_write_timeout():
> 
> 		mss = min(sysctl_tcp_base_mss,
> 		          tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2);
> 		mss = max(mss, 68 - tp->tcp_header_len);
> 
> That first line looks like it should be a max() instead
> of a min().
> 
> tcp_base_mss is the smallest MSS we should use, therefore
> we should make sure the "mss" is at least that large.
> 
> It's also possible that I misread the intention of this code :) From
> what I read, it is trying to half the MSS in use and adjust the MTU
> search low point to be based upon this new value.

No, the min() is what's intended here.  The base_mss is where you want 
to start searching from.  So, on black hole detection, you drop 
immediatly down to the base.  The base is configurable, because making 
it higher can make searching faster.  If it's still too high for some 
links, you halve it again on successive timeouts.

   -John

Re: MTU probing bug?

[David Miller]

From: John Heffner <jheffner@psc.edu>
Date: Tue, 25 Jul 2006 09:56:38 -0400

> David Miller wrote:
> > John, have a look at this code in tcp_write_timeout():
> > 
> > 		mss = min(sysctl_tcp_base_mss,
> > 		          tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2);
> > 		mss = max(mss, 68 - tp->tcp_header_len);
> > 
> > That first line looks like it should be a max() instead
> > of a min().
> > 
> > tcp_base_mss is the smallest MSS we should use, therefore
> > we should make sure the "mss" is at least that large.
> > 
> > It's also possible that I misread the intention of this code :) From
> > what I read, it is trying to half the MSS in use and adjust the MTU
> > search low point to be based upon this new value.
> 
> No, the min() is what's intended here.  The base_mss is where you want 
> to start searching from.  So, on black hole detection, you drop 
> immediatly down to the base.  The base is configurable, because making 
> it higher can make searching faster.  If it's still too high for some 
> links, you halve it again on successive timeouts.

Thanks for the clarification John.

I find it interesting that black hole detection is handled
different from a normal probe failure.  I guess here we are
dealing with a more significant failure, so we should start
at the thing which is most guarenteed to work.

Re: MTU probing bug?

[John Heffner]

David Miller wrote:
> I find it interesting that black hole detection is handled
> different from a normal probe failure.  I guess here we are
> dealing with a more significant failure, so we should start
> at the thing which is most guarenteed to work.

Black hole detection is substantially different than probe failure. 
It's an indication that your current effective MTU is too large, and 
must be reduced.  A black hole basically means that your current path 
information is no good, either because your initial values are wrong, or 
the path changed.  So, you really want to reset everything to 
conservative values.  After a failed probe, your effective MTU is still 
fine, you've just gotten some additional information about the path 
(that it doesn't support the higher MTU).

   -John