Re: [PATCH] tcp: Always send an ACK segment once the handshake is completed

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]

On Sat, Sep 23, 2023 at 12:06:37AM +0200, Stefano Brivio wrote:
> The reporter is running a SMTP server behind pasta, and the client
> waits for the server's banner before sending any data. In turn, the
> server waits for our ACK after sending SYN,ACK, which never comes.
> 
> If we use the ACK_IF_NEEDED indication to tcp_send_flag(), given that
> there's no pending data, we delay sending the ACK segment at the end
> of the three-way handshake until we have some data to send to the
> server.
> 
> This was actually intended, as I thought we would lower the latency
> for new connections, but we can't assume that the client will start
> sending data first (SMTP is the typical example where this doesn't
> happen).
> 
> And, trying out this patch with SSH (where the client starts sending
> data first), the reporter actually noticed we have a lower latency
> by forcing an ACK right away. Comparing a capture before the patch:
> 
> 13:07:14.007704 IP 10.1.2.1.42056 > 10.1.2.140.1234: Flags [S], seq 1797034836, win 65535, options [mss 4096,nop,wscale 7], length 0
> 13:07:14.007769 IP 10.1.2.140.1234 > 10.1.2.1.42056: Flags [S.], seq 2297052481, ack 1797034837, win 65480, options [mss 65480,nop,wscale 7], length 0
> 13:07:14.008462 IP 10.1.2.1.42056 > 10.1.2.140.1234: Flags [.], seq 1:22, ack 1, win 65535, length 21
> 13:07:14.008496 IP 10.1.2.140.1234 > 10.1.2.1.42056: Flags [.], ack 22, win 512, length 0
> 13:07:14.011799 IP 10.1.2.140.1234 > 10.1.2.1.42056: Flags [P.], seq 1:515, ack 22, win 512, length 514
> 
> and after:
> 
> 13:10:26.165364 IP 10.1.2.1.59508 > 10.1.2.140.1234: Flags [S], seq 4165939595, win 65535, options [mss 4096,nop,wscale 7], length 0
> 13:10:26.165391 IP 10.1.2.140.1234 > 10.1.2.1.59508: Flags [S.], seq 985607380, ack 4165939596, win 65480, options [mss 65480,nop,wscale 7], length 0
> 13:10:26.165418 IP 10.1.2.1.59508 > 10.1.2.140.1234: Flags [.], ack 1, win 512, length 0
> 13:10:26.165683 IP 10.1.2.1.59508 > 10.1.2.140.1234: Flags [.], seq 1:22, ack 1, win 512, length 21
> 13:10:26.165698 IP 10.1.2.140.1234 > 10.1.2.1.59508: Flags [.], ack 22, win 512, length 0
> 13:10:26.167107 IP 10.1.2.140.1234 > 10.1.2.1.59508: Flags [P.], seq 1:515, ack 22, win 512, length 514
> 
> the latency between the initial SYN segment and the first data
> transmission actually decreases from 792µs to 334µs. This is not
> statistically relevant as we have a single measurement, but it can't
> be that bad, either.
> 
> Reported-by: cr3bs (from IRC)
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tcp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 76b7b8d..39844eb 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2562,7 +2562,7 @@ static void tcp_conn_from_sock_finish(struct ctx *c, struct tcp_tap_conn *conn,
>  	 * dequeue waiting for SYN,ACK from tap -- check now.
>  	 */
>  	tcp_data_from_sock(c, conn);
> -	tcp_send_flag(c, conn, ACK_IF_NEEDED);
> +	tcp_send_flag(c, conn, ACK);
>  }
>  
>  /**

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]