grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
From: Andrei Borzenkov <arvidjaar@gmail.com>
To: The development of GNU GRUB <grub-devel@gnu.org>, kernel-team@fb.com
Cc: Josef Bacik <jbacik@fb.com>
Subject: Re: [PATCH] tcp: add window scaling support
Date: Mon, 17 Aug 2015 19:15:16 +0300	[thread overview]
Message-ID: <55D20894.2010605@gmail.com> (raw)
In-Reply-To: <1439395075-3225623-1-git-send-email-jbacik@fb.com>

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.

> 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.

>   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.


  reply	other threads:[~2015-08-17 16:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-12 15:57 [PATCH] tcp: add window scaling support Josef Bacik
2015-08-17 16:15 ` Andrei Borzenkov [this message]
2015-08-18 17:48   ` Josef Bacik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55D20894.2010605@gmail.com \
    --to=arvidjaar@gmail.com \
    --cc=grub-devel@gnu.org \
    --cc=jbacik@fb.com \
    --cc=kernel-team@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).