Skip to content

Commit 30c6f0b

Browse files
fuyuanliPaolo Abeni
authored andcommitted
tcp: fix mishandling when the sack compression is deferred.
In this patch, we mainly try to handle sending a compressed ack correctly if it's deferred. Here are more details in the old logic: When sack compression is triggered in the tcp_compressed_ack_kick(), if the sock is owned by user, it will set TCP_DELACK_TIMER_DEFERRED and then defer to the release cb phrase. Later once user releases the sock, tcp_delack_timer_handler() should send a ack as expected, which, however, cannot happen due to lack of ICSK_ACK_TIMER flag. Therefore, the receiver would not sent an ack until the sender's retransmission timeout. It definitely increases unnecessary latency. Fixes: 5d9f426 ("tcp: add SACK compression") Suggested-by: Eric Dumazet <edumazet@google.com> Signed-off-by: fuyuanli <fuyuanli@didiglobal.com> Signed-off-by: Jason Xing <kerneljasonxing@gmail.com> Link: https://lore.kernel.org/netdev/20230529113804.GA20300@didi-ThinkCentre-M920t-N000/ Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://lore.kernel.org/r/20230531080150.GA20424@didi-ThinkCentre-M920t-N000 Signed-off-by: Paolo Abeni <pabeni@redhat.com>
1 parent 4d56304 commit 30c6f0b

3 files changed

Lines changed: 15 additions & 4 deletions

File tree

include/net/tcp.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,7 @@ void tcp_reset(struct sock *sk, struct sk_buff *skb);
632632
void tcp_skb_mark_lost_uncond_verify(struct tcp_sock *tp, struct sk_buff *skb);
633633
void tcp_fin(struct sock *sk);
634634
void tcp_check_space(struct sock *sk);
635+
void tcp_sack_compress_send_ack(struct sock *sk);
635636

636637
/* tcp_timer.c */
637638
void tcp_init_xmit_timers(struct sock *);

net/ipv4/tcp_input.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4530,7 +4530,7 @@ static void tcp_sack_maybe_coalesce(struct tcp_sock *tp)
45304530
}
45314531
}
45324532

4533-
static void tcp_sack_compress_send_ack(struct sock *sk)
4533+
void tcp_sack_compress_send_ack(struct sock *sk)
45344534
{
45354535
struct tcp_sock *tp = tcp_sk(sk);
45364536

net/ipv4/tcp_timer.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,19 @@ static int tcp_write_timeout(struct sock *sk)
290290
void tcp_delack_timer_handler(struct sock *sk)
291291
{
292292
struct inet_connection_sock *icsk = inet_csk(sk);
293+
struct tcp_sock *tp = tcp_sk(sk);
293294

294-
if (((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
295-
!(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
295+
if ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))
296+
return;
297+
298+
/* Handling the sack compression case */
299+
if (tp->compressed_ack) {
300+
tcp_mstamp_refresh(tp);
301+
tcp_sack_compress_send_ack(sk);
302+
return;
303+
}
304+
305+
if (!(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
296306
return;
297307

298308
if (time_after(icsk->icsk_ack.timeout, jiffies)) {
@@ -312,7 +322,7 @@ void tcp_delack_timer_handler(struct sock *sk)
312322
inet_csk_exit_pingpong_mode(sk);
313323
icsk->icsk_ack.ato = TCP_ATO_MIN;
314324
}
315-
tcp_mstamp_refresh(tcp_sk(sk));
325+
tcp_mstamp_refresh(tp);
316326
tcp_send_ack(sk);
317327
__NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKS);
318328
}

0 commit comments

Comments
 (0)