[prev in list] [next in list] [prev in thread] [next in thread] 

List:       grub-devel
Subject:    Re: [PATCH] tcp: add window scaling support
From:       Josef Bacik <jbacik () fb ! com>
Date:       2015-08-18 17:48:20
Message-ID: 55D36FE4.4010506 () fb ! com
[Download RAW message or body]

On 08/17/2015 09:15 AM, Andrei Borzenkov wrote:
> 12.08.2015 18:57, Josef Bacik пишет:
>> 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 probably
> 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 <jbacik@fb.com>
>> ---
>>   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 point.

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 = 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 = (void *) nb->data;
>> +  grub_memset(tcph, 0, sizeof (*tcph));
>>     socket->my_start_seq = grub_get_time_ms ();
>>     socket->my_cur_seq = socket->my_start_seq + 1;
>> -  socket->my_window = 8192;
>> -  tcph->seqnr = grub_cpu_to_be32 (socket->my_start_seq);
>> -  tcph->ack = grub_cpu_to_be32_compile_time (0);
>> -  tcph->flags = grub_cpu_to_be16_compile_time ((5 << 12) | TCP_SYN);
>> -  tcph->window = grub_cpu_to_be16 (socket->my_window);
>> -  tcph->urgent = 0;
>> -  tcph->src = grub_cpu_to_be16 (socket->in_port);
>> -  tcph->dst = grub_cpu_to_be16 (socket->out_port);
>> -  tcph->checksum = 0;
>> -  tcph->checksum = grub_net_ip_transport_checksum (nb, GRUB_NET_IP_TCP,
>> -                           &socket->inf->address,
>> -                           &socket->out_nla);
>> +  socket->my_window = 32768;
>> +  tcph->tcphdr.seqnr = grub_cpu_to_be32 (socket->my_start_seq);
>> +  tcph->tcphdr.ack = grub_cpu_to_be32_compile_time (0);
>> +  tcph->tcphdr.flags = grub_cpu_to_be16_compile_time ((6 << 12) |
>> TCP_SYN);
>> +  tcph->tcphdr.window = grub_cpu_to_be16 (socket->my_window);
>> +  tcph->tcphdr.urgent = 0;
>> +  tcph->tcphdr.src = grub_cpu_to_be16 (socket->in_port);
>> +  tcph->tcphdr.dst = grub_cpu_to_be16 (socket->out_port);
>> +  tcph->tcphdr.checksum = 0;
>> +  tcph->scale_opt.kind = 3;
>> +  tcph->scale_opt.length = 3;
>> +  tcph->scale_opt.scale = 5;
>> +  tcph->tcphdr.checksum = 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 
opened to us which is why I did it this way.  If we start to care about 
how fast grub can transfer stuff _to_ somebody else we can add it there 
as well.  Thanks,

Josef

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[prev in list] [next in list] [prev in thread] [next in thread]