Re: [PATCH] tcp: ack when we get an OOO/lost packet

[Andrei Borzenkov]

On Wed, Aug 12, 2015 at 6:16 PM, Josef Bacik <address@hidden> wrote:
> While adding tcp window scaling support I was finding that I'd get some packet
> loss or reordering when transferring from large distances and grub would just
> timeout.  This is because we weren't ack'ing when we got our OOO packet, so 
> the
> sender didn't know it needed to retransmit anything, so eventually it would 
> fill
> the window and stop transmitting, and we'd time out.  Fix this by ACK'ing when
> we don't find our next sequence numbered packet.  With this fix I no longer 
> time
> out.  Thanks,

I have a feeling that your description is misleading. Patch simply
sends duplicated ACK, but partner does not know what has been received
and what has not, so it must wait for ACK timeout anyway before
retransmitting. What this patch may fix would be lost ACK packet
*from* GRUB, by increasing rate of ACK packets it sends. Do you have
packet trace for timeout case, ideally from both sides simultaneously?

Did you consider implementing receive side SACK BTW? You have the
right environment to test it :)

>
> Signed-off-by: Josef Bacik <address@hidden>
> ---
>  grub-core/net/tcp.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c
> index 25720b1..6b411dd 100644
> --- a/grub-core/net/tcp.c
> +++ b/grub-core/net/tcp.c
> @@ -902,7 +902,10 @@ grub_net_recv_tcp_packet (struct grub_net_buff *nb,
>           grub_priority_queue_pop (sock->pq);
>         }
>        if (grub_be_to_cpu32 (tcph->seqnr) != sock->their_cur_seq)
> -       return GRUB_ERR_NONE;
> +       {
> +         ack (sock);
> +         return GRUB_ERR_NONE;
> +       }
>        while (1)
>         {
>           nb_top_p = grub_priority_queue_top (sock->pq);
> --
> 1.8.1
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel