Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 15 Sep 2010 16:04:45 +0200
From:      Andre Oppermann <andre@freebsd.org>
To:        Lawrence Stewart <lstewart@freebsd.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r212653 - head/sys/netinet
Message-ID:  <4C90D27D.4070306@freebsd.org>
In-Reply-To: <4C90B326.4000208@freebsd.org>
References:  <201009151039.o8FAdU4H030416@svn.freebsd.org> <4C90B326.4000208@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 15.09.2010 13:51, Lawrence Stewart wrote:
> On 09/15/10 20:39, Andre Oppermann wrote:
>> Author: andre
>> Date: Wed Sep 15 10:39:30 2010
>> New Revision: 212653
>> URL: http://svn.freebsd.org/changeset/base/212653
>>
>> Log:
>>    Change the default MSS for IPv4 and IPv6 TCP connections from an
>>    artificial power-of-2 rounded number to their real values specified
>>    in RFC879 and RFC2460.
>>
>>    From the history and existing comments it appears that the rounded
>>    numbers were intended to be advantageous for the kernel and mbuf
>>    system.  However this hasn't been the case at for at least a long
>>    time.  The mbuf clusters used in tcp_output() have enough space
>>    to hold the larger real value for the default MSS for both IPv4 and
>>    IPv6.  Note that the default MSS is only used when path MTU discovery
>>    is disabled.
>>
>>    Update and expand related comments.
>>
>>    Reviewed by:	lsteward (including some word-smithing)
>
> For the record, I reviewed and fully support the functional changes made
> by this patch, but explicitly objected to and offered an alternate for
> the proposed comment wording changes.
>
> Andre, given that we had a disagreement about the comment wording, I
> would have preferred it if you had noted in your commit log that I had
> raised an objection to or at least not reviewed/endorsed the comment
> changes.

I've adapted many of your suggestions on the wording compared to my
first version.  For some parts I felt that my wording/description was
more appropriate.  In the end neither of our wordings is plain wrong or
factually incorrect.

> It's not important enough an issue to spend any more time on, but I'm a
> bit upset to see this committed with an acknowledgement to my review and
> word-smithing, much of which ended up being ignored (which is fine, but
> then don't put my name to it).

I apologize for not having made your different opinion to the wording
clear enough in the commit message.  My intent was to communicate that
you not only reviewed the functional change but also provided input on
the wording (which I in fact did not incorporate to some extent but not
entirely).

Below is the wording proposed by Lawrence:
/*
  * The default Maximum Segment Size (MSS) to use when we do not have specific
  * knowledge (e.g. via path MTU discovery) that the destination host is prepared
  * to accept larger datagrams. The smallest allowable IP datagram MTU and
  * optionless IP/TCP header lengths are used for the calculation as per RFC879.
  * For IPv4 (RFC791): 576 - 20 - 20 = 536.
  * For IPv6 (RFC2460): 1280 - 40 - 20 = 1220.
  */
#define	TCP_MSS		536
#define	TCP6_MSS	1220

  * Limit the lowest MSS we accept for path MTU discovery and the TCP SYN MSS
  * option. Allowing low values of MSS can consume significant resources and be
  * used to mount a resource exhaustion attack. Connections requesting lower MSS
  * values will be rounded up to this value and the IP_DF flag will be cleared to
  * allow fragmentation along the path.
  *
  * See tcp_subr.c tcp_minmss SYSCTL declaration for more comments. Setting this
  * SYSCTL to "0" disables the minmss check.
  *
  * The default value is fine for TCP over IPv4 across the Internet's smallest
  * known link MTU (256 bytes for AX.25 packet radio). However, a connection is
  * very unlikely to come across such low MTU interfaces (anno domini 2003).
  */
#define	TCP_MINMSS 216

-- 
Andre



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C90D27D.4070306>