Re: [PATCH net-next] net: tcp: refresh tcp_mstamp for compressed ack in timer

[Eric Dumazet]

On Thu, Oct 3, 2024 at 10:23 AM Menglong Dong <menglong8.dong@xxxxxxxxx> wrote:
>
> For now, we refresh the tcp_mstamp for delayed acks and keepalives, but
> not for the compressed ack in tcp_compressed_ack_kick().
>
> I have not found out the effact of the tcp_mstamp when sending ack, but
> we can still refresh it for the compressed ack to keep consistent.

This was a choice I made for the following reason :

delayed ack timer can happen sometime 40ms later. Thus the
tcp_mstamp_refresh(tp) was probably welcome.

Compressed ack timer is scheduled for min( 5% of RTT, 1ms). It is
usually in the 200 usec range.

So sending the prior tsval (for flow using TCP TS) was ok (and right
most of the time), and not changing PAWS or EDT logic.

Although I do not object to your patch, there is no strong argument
for it or against it.

Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>

>
> Signed-off-by: Menglong Dong <dongml2@xxxxxxxxxxxxxxx>
> ---
>  net/ipv4/tcp_timer.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 79064580c8c0..1f37a37f9c82 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -851,6 +851,7 @@ static enum hrtimer_restart tcp_compressed_ack_kick(struct hrtimer *timer)
>                          * LINUX_MIB_TCPACKCOMPRESSED accurate.
>                          */
>                         tp->compressed_ack--;
> +                       tcp_mstamp_refresh(tp);
>                         tcp_send_ack(sk);
>                 }
>         } else {
> --
> 2.39.5
>