From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aQGtv-0006DB-IX for mharc-grub-devel@gnu.org; Mon, 01 Feb 2016 11:00:27 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQGtm-0005cN-Ld for grub-devel@gnu.org; Mon, 01 Feb 2016 11:00:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQGti-0000Nb-HD for grub-devel@gnu.org; Mon, 01 Feb 2016 11:00:18 -0500 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:23224) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQGti-0000NW-8W for grub-devel@gnu.org; Mon, 01 Feb 2016 11:00:14 -0500 Received: from pps.filterd (m0044010.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id u11FnxR8001276; Mon, 1 Feb 2016 08:00:12 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=subject : to : references : cc : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=facebook; bh=BQXvBjtTrCTVavPZHgOGbirohEGY4f68N92RG2vywj8=; b=O0PjG+KCPs/eZzDdxWUBOUvsz7zggmuHyX90UpX8++XoIFOGLdqZhBFVCon3VcvCYoia D8u0BE+AGQp2qGlJTLm2a0uGF3j0oWpp6zdDtGUGLsMo1ANF1gqCs4dNXR6t1+935LLX KHHKkV51rLUwk8m2upb4DCqyF1jTmGCzXj4= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0a-00082601.pphosted.com with ESMTP id 20t1tma3ce-1 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Mon, 01 Feb 2016 08:00:12 -0800 Received: from localhost.localdomain (192.168.52.123) by mail.TheFacebook.com (192.168.16.18) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 1 Feb 2016 08:00:08 -0800 Subject: Re: [PATCH V2] tcp: add window scaling and RTTM support To: Andrei Borzenkov , The development of GNU GRUB References: <1454104105-1741742-1-git-send-email-jbacik@fb.com> From: Josef Bacik Message-ID: <56AF8107.3000300@fb.com> Date: Mon, 1 Feb 2016 11:00:07 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-02-01_07:, , signatures=0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 67.231.145.42 Cc: Kernel Team X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Feb 2016 16:00:25 -0000 On 02/01/2016 03:43 AM, Andrei Borzenkov wrote: > On Sat, Jan 30, 2016 at 12:48 AM, Josef Bacik wrote: >> Sometimes we have to provision boxes across regions, such as California to >> Sweden. The http server has a 10 minute timeout, so if we can't get our 250mb >> image transferred fast enough our provisioning fails, which is not ideal. So >> add tcp window scaling on open connections and set the window size to 1mb. With >> this change we're able to get higher sustained transfers between regions and can >> transfer our image in well below 10 minutes. Without this patch we'd time out >> every time halfway through the transfer. >> >> RTTM is needed in order to make window scaling work well under heavy congestion >> or packet loss. In most cases grub could recover with just window scaling >> enabled, but on some machines the congestion would be so high that it would >> never recover and would timeout. >> >> I've made the window size configureable with the grub env variable >> "tcp_window_size". By default this is set to 1mb but can be configured to >> whatever a user wants, and we will calculate the appropriate window size and >> scale settings. Thanks, >> >> @@ -906,6 +1027,8 @@ grub_net_recv_tcp_packet (struct grub_net_buff *nb, >> return err; >> } >> >> + /* We only update the tsecr when we advance the window. */ >> + sock->cur_tsecr = tsecr; > > As far as I can tell, this does not agree with algorithm proposed in > RFC 7323. It recommends that TSecr be advanced on immediate segment > arrival and only if TSval is greater than previous one. I read it that > TSecr advance should be done before queue processing, probably at the > spot where we look for TS option. > > (2) If: > > SEG.TSval >= TS.Recent and SEG.SEQ <= Last.ACK.sent > > then SEG.TSval is copied to TS.Recent; otherwise, it is ignored. > > and > > It is important to note that the timestamp MUST be checked only when > a segment first arrives at the receiver, regardless of whether it is > in sequence or it must be queued for later delivery. So if you look further down it shows an example of what I've implemented, where we get packets out of order. We only increment TS.Recent (which in our case is sock->cur_tsecr) when we've gotten the next segment that we are expecting, which is what that bit (2) If: SEG.TSval >= TS.Recent and SEG.SEQ <= Last.ACK.sent is talking about. Look at the page 17 for the example, I was super confused about this as well until I saw the example they gave. I probably should check for SEG.TSval >= TS.Recent before resetting it, so I'll do that. > > Note that it also recommends that if TS option was negotiated, packets > without TS option should be discarded. Not sure if we need to follow > it. > I'll go ahead and do this just to be on the safe side. Thanks, Josef