From owner-svn-src-all@FreeBSD.ORG Wed Sep 15 14:04:44 2010 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2CA351065695 for ; Wed, 15 Sep 2010 14:04:44 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.freebsd.org (Postfix) with ESMTP id 90F4A8FC22 for ; Wed, 15 Sep 2010 14:04:43 +0000 (UTC) Received: (qmail 71645 invoked from network); 15 Sep 2010 13:59:31 -0000 Received: from localhost (HELO [127.0.0.1]) ([127.0.0.1]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 15 Sep 2010 13:59:31 -0000 Message-ID: <4C90D27D.4070306@freebsd.org> Date: Wed, 15 Sep 2010 16:04:45 +0200 From: Andre Oppermann User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.8) Gecko/20100802 Thunderbird/3.1.2 MIME-Version: 1.0 To: Lawrence Stewart References: <201009151039.o8FAdU4H030416@svn.freebsd.org> <4C90B326.4000208@freebsd.org> In-Reply-To: <4C90B326.4000208@freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r212653 - head/sys/netinet X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Sep 2010 14:04:44 -0000 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