The tcp_retransmit_timer() function checks if tcp_retransmit_skb()
returns a value greater than 0, but tcp_retransmit_skb() returns
0 on success and negative error codes on failure. This means the
error handling branch is never executed when retransmission fails.

Fix this by changing the condition to check for != 0 instead of > 0.

Signed-off-by: Liu Congcong <fangwu.lcc@antgroup.com>
---
 net/ipv4/tcp_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 160080c9021d..4fbb387e7e7b 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -624,7 +624,7 @@ void tcp_retransmit_timer(struct sock *sk)
 	tcp_enter_loss(sk);
 
 	tcp_update_rto_stats(sk);
-	if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1) > 0) {
+	if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1)) {
 		/* Retransmission failed because of local congestion,
 		 * Let senders fight for local resources conservatively.
 		 */
-- 
2.17.0

On Mon, Jan 5, 2026 at 6:53 PM 刘聪聪(方勿) <fangwu.lcc@antgroup.com> wrote:
>
> The tcp_retransmit_timer() function checks if tcp_retransmit_skb()
> returns a value greater than 0, but tcp_retransmit_skb() returns
> 0 on success and negative error codes on failure.

This seems like a bogus claim to me.

tcp_retransmit_skb() can and should return >0 in some cases.

Time to provide a packetdrill test I guess.

   0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
   +0 bind(3, ..., ...) = 0
   +0 listen(3, 1) = 0

   +0 < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
   +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>

 +.02 < . 1:1(0) ack 1 win 257
   +0 accept(3, ..., ...) = 4
   // Set a 5s timeout
   +0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [5000], 4) = 0
   +0 write(4, ..., 11000) = 11000
   +0 > P. 1:10001(10000) ack 1

// TLP
 +.04~+.05 > P. 10001:11001(1000) ack 1
   +0 %{ assert tcpi_retransmits == 0, tcpi_retransmits;\
         assert tcpi_backoff     == 0, tcpi_backoff }%

   // Emulate a congestion  - no packets can get out for 3 seconds
   // Check that we retry every 500ms (6 rounds) w/o backoff
   +0 `tc qdisc replace dev tun0 root pfifo limit 0`
   +3 %{ assert tcpi_retransmits == 6, tcpi_retransmits;\
         assert tcpi_backoff     == 0, tcpi_backoff }%

   // Congestion is now relieved - the next retry should show up some time
   // Hopefully qdisc in the future can inform TCP right away to retry.
   +0 `tc qdisc replace dev tun0 root pfifo limit 1000`
 +.21~+.26 > . 1:1001(1000) ack 1
 +.02 < . 1:1(0) ack 1001 win 257
   +0 > . 1001:3001(2000) ack 1
   // Test the recurring timeout counter is reset. The backoff counter
   // remains one until a new RTT sample is acquired. We do not get a new
   // RTT sample b/c the ACK acks a rtx w/o TS options
   +0 %{ assert tcpi_retransmits == 0, tcpi_retransmits;\
         assert tcpi_backoff     == 1, tcpi_backoff }%

   // Emulate a longer local congestion - the next ACK should trigger
   // more transmission but none can succeed.
   +0 `tc qdisc replace dev tun0 root pfifo limit 0`
   +.02 < . 1:1(0) ack 3001 win 257

   // Socket has timed out after +5s of lack of progress as specified above
   +5.1 write(4, ..., 100) = -1 (ETIMEDOUT)


> This means the
> error handling branch is never executed when retransmission fails.
>
> Fix this by changing the condition to check for != 0 instead of > 0.
>
> Signed-off-by: Liu Congcong <fangwu.lcc@antgroup.com>
> ---
>  net/ipv4/tcp_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 160080c9021d..4fbb387e7e7b 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -624,7 +624,7 @@ void tcp_retransmit_timer(struct sock *sk)
>         tcp_enter_loss(sk);
>
>         tcp_update_rto_stats(sk);
> -       if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1) > 0) {
> +       if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1)) {
>                 /* Retransmission failed because of local congestion,
>                  * Let senders fight for local resources conservatively.
>                  */
> --
> 2.17.0
>