From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZRlVx-00046A-TQ for mharc-grub-devel@gnu.org; Tue, 18 Aug 2015 14:21:37 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51468) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZRlVr-000450-B7 for grub-devel@gnu.org; Tue, 18 Aug 2015 14:21:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZRlVo-0004Ra-H9 for grub-devel@gnu.org; Tue, 18 Aug 2015 14:21:31 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:2273) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZRlVo-0004RH-Am for grub-devel@gnu.org; Tue, 18 Aug 2015 14:21:28 -0400 Received: from pps.filterd (m0004060 [127.0.0.1]) by mx0b-00082601.pphosted.com (8.14.5/8.14.5) with SMTP id t7IHkbA5014151; Tue, 18 Aug 2015 10:49:21 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=message-id : date : from : mime-version : to : subject : references : in-reply-to : content-type : content-transfer-encoding; s=facebook; bh=KCp8ge2zmNXSASh2Oz6Xj3vG/ZKSJNI/m7mBbS/a1Co=; b=L+bf0FJLIbUaw0kCOvxf1l/BRIx3ZmtUDietWvORvjcfvOvnDg9DVdWT6xZy0q8saBrZ ql9s04rcAb7YzivkyaURxhBpn0mm7HZ73oOC1GQbVs1es+5Tdl2fQwUPoFAQxnJpudeQ 0eDHRtJhKCqzr1F/kUxUImLxxMSB2Qg/YtM= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0b-00082601.pphosted.com with ESMTP id 1wc8c6rbuf-2 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Tue, 18 Aug 2015 10:49:21 -0700 Received: from localhost.localdomain (192.168.52.123) by mail.thefacebook.com (192.168.16.20) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 18 Aug 2015 10:48:26 -0700 Message-ID: <55D36FE4.4010506@fb.com> Date: Tue, 18 Aug 2015 10:48:20 -0700 From: Josef Bacik User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Andrei Borzenkov , The development of GNU GRUB , Subject: Re: [PATCH] tcp: add window scaling support References: <1439395075-3225623-1-git-send-email-jbacik@fb.com> <55D20894.2010605@gmail.com> In-Reply-To: <55D20894.2010605@gmail.com> Content-Type: text/plain; charset="utf-8"; format=flowed 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:5.14.151, 1.0.33, 0.0.0000 definitions=2015-08-18_09:2015-08-18, 2015-08-18, 1970-01-01 signatures=0 Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by mx0b-00082601.pphosted.com id t7IHkbA5014151 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x X-Received-From: 67.231.153.30 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: Tue, 18 Aug 2015 18:21:36 -0000 On 08/17/2015 09:15 AM, Andrei Borzenkov wrote: > 12.08.2015 18:57, Josef Bacik =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> 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. Thanks, >> > > I do not think having 1M TCP window by default is good idea. We probabl= y > could make it configurable if there are evidences that it is really > required. Ok I'll add a config option. > >> Signed-off-by: Josef Bacik >> --- >> grub-core/net/tcp.c | 42 +++++++++++++++++++++++++++++------------- >> 1 file changed, 29 insertions(+), 13 deletions(-) >> >> diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c >> index 5da8b11..57a6d4e 100644 >> --- a/grub-core/net/tcp.c >> +++ b/grub-core/net/tcp.c >> @@ -106,6 +106,18 @@ struct tcphdr >> grub_uint16_t urgent; >> } GRUB_PACKED; >> >> +struct tcp_scale_opt { >> + grub_uint8_t kind; >> + grub_uint8_t length; >> + grub_uint8_t scale; >> +} GRUB_PACKED; >> + >> +struct tcp_synhdr { >> + struct tcphdr tcphdr; >> + struct tcp_scale_opt scale_opt; >> + grub_uint8_t padding; >> +}; >> + > > No, this does not scale. Just allocate large enough buffer and use > pointers inside it. We may want to support multiple options at some poi= nt. Fair enough. > >> struct tcp_pseudohdr >> { >> grub_uint32_t src; >> @@ -555,7 +567,7 @@ grub_net_tcp_open (char *server, >> grub_net_tcp_socket_t socket; >> static grub_uint16_t in_port =3D 21550; >> struct grub_net_buff *nb; >> - struct tcphdr *tcph; >> + struct tcp_synhdr *tcph; >> int i; >> grub_uint8_t *nbd; >> grub_net_link_level_address_t ll_target_addr; >> @@ -617,20 +629,24 @@ grub_net_tcp_open (char *server, >> } >> >> tcph =3D (void *) nb->data; >> + grub_memset(tcph, 0, sizeof (*tcph)); >> socket->my_start_seq =3D grub_get_time_ms (); >> socket->my_cur_seq =3D socket->my_start_seq + 1; >> - socket->my_window =3D 8192; >> - tcph->seqnr =3D grub_cpu_to_be32 (socket->my_start_seq); >> - tcph->ack =3D grub_cpu_to_be32_compile_time (0); >> - tcph->flags =3D grub_cpu_to_be16_compile_time ((5 << 12) | TCP_SYN)= ; >> - tcph->window =3D grub_cpu_to_be16 (socket->my_window); >> - tcph->urgent =3D 0; >> - tcph->src =3D grub_cpu_to_be16 (socket->in_port); >> - tcph->dst =3D grub_cpu_to_be16 (socket->out_port); >> - tcph->checksum =3D 0; >> - tcph->checksum =3D grub_net_ip_transport_checksum (nb, GRUB_NET_IP_= TCP, >> - &socket->inf->address, >> - &socket->out_nla); >> + socket->my_window =3D 32768; >> + tcph->tcphdr.seqnr =3D grub_cpu_to_be32 (socket->my_start_seq); >> + tcph->tcphdr.ack =3D grub_cpu_to_be32_compile_time (0); >> + tcph->tcphdr.flags =3D grub_cpu_to_be16_compile_time ((6 << 12) | >> TCP_SYN); >> + tcph->tcphdr.window =3D grub_cpu_to_be16 (socket->my_window); >> + tcph->tcphdr.urgent =3D 0; >> + tcph->tcphdr.src =3D grub_cpu_to_be16 (socket->in_port); >> + tcph->tcphdr.dst =3D grub_cpu_to_be16 (socket->out_port); >> + tcph->tcphdr.checksum =3D 0; >> + tcph->scale_opt.kind =3D 3; >> + tcph->scale_opt.length =3D 3; >> + tcph->scale_opt.scale =3D 5; >> + tcph->tcphdr.checksum =3D grub_net_ip_transport_checksum (nb, >> GRUB_NET_IP_TCP, >> + &socket->inf->address, >> + &socket->out_nla); >> >> tcp_socket_register (socket); >> >> > > There is one more place - grub_net_tcp_accept. Not sure if it ever be > used though. So I only care about connections we open, not about connections that are=20 opened to us which is why I did it this way. If we start to care about=20 how fast grub can transfer stuff _to_ somebody else we can add it there=20 as well. Thanks, Josef