From c8144356338995a277593bc08315d3b1ddcc8cb3 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Thu, 6 Jan 2022 11:58:24 -0800 Subject: tcp: update snd_congestion only during congestion If running without sacks, if snd_una does not cover snd_congestion fast recovery can be missed but the two heuristics from RFC6582 should avoid that. Also snd_congestion was used as a means of inferring if the connection recently exited congestion while setting the persist timer but that does not always work correctly if not congested. Type: fix Signed-off-by: Florin Coras Change-Id: I94d4ac738cdd4f7f23f62e97dd63059de1cd4af9 --- src/vnet/tcp/tcp.c | 4 ++++ src/vnet/tcp/tcp_input.c | 9 --------- src/vnet/tcp/tcp_timer.h | 32 ++++++++++---------------------- 3 files changed, 14 insertions(+), 31 deletions(-) (limited to 'src') diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c index 3d3cc886024..ffe5c895cc9 100644 --- a/src/vnet/tcp/tcp.c +++ b/src/vnet/tcp/tcp.c @@ -71,6 +71,10 @@ tcp_add_del_adjacency (tcp_connection_t * tc, u8 is_add) static void tcp_cc_init (tcp_connection_t * tc) { + /* As per RFC 6582 initialize "recover" to iss */ + if (tcp_opts_sack_permitted (&tc->rcv_opts)) + tc->snd_congestion = tc->iss; + tc->cc_algo->init (tc); } diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c index 11491a68257..64b9334cbea 100644 --- a/src/vnet/tcp/tcp_input.c +++ b/src/vnet/tcp/tcp_input.c @@ -802,15 +802,6 @@ tcp_cc_update (tcp_connection_t * tc, tcp_rate_sample_t * rs) /* If a cumulative ack, make sure dupacks is 0 */ tc->rcv_dupacks = 0; - - /* When dupacks hits the threshold we only enter fast retransmit if - * cumulative ack covers more than snd_congestion. Should snd_una - * wrap this test may fail under otherwise valid circumstances. - * Therefore, proactively update snd_congestion when wrap detected. */ - if (PREDICT_FALSE - (seq_leq (tc->snd_congestion, tc->snd_una - tc->bytes_acked) - && seq_gt (tc->snd_congestion, tc->snd_una))) - tc->snd_congestion = tc->snd_una - 1; } /** diff --git a/src/vnet/tcp/tcp_timer.h b/src/vnet/tcp/tcp_timer.h index 5f14a71ee7d..7f7dbf193eb 100644 --- a/src/vnet/tcp/tcp_timer.h +++ b/src/vnet/tcp/tcp_timer.h @@ -51,6 +51,13 @@ tcp_timer_update (tcp_timer_wheel_t * tw, tcp_connection_t * tc, u8 timer_id, timer_id, interval); } +always_inline u8 +tcp_timer_is_active (tcp_connection_t *tc, tcp_timers_e timer) +{ + return tc->timers[timer] != TCP_TIMER_HANDLE_INVALID || + (tc->pending_timers & (1 << timer)); +} + always_inline void tcp_retransmit_timer_set (tcp_timer_wheel_t * tw, tcp_connection_t * tc) { @@ -73,19 +80,6 @@ tcp_persist_timer_set (tcp_timer_wheel_t * tw, tcp_connection_t * tc) clib_max ((u32) tc->rto * TCP_TO_TIMER_TICK, 1)); } -always_inline void -tcp_persist_timer_update (tcp_timer_wheel_t * tw, tcp_connection_t * tc) -{ - u32 interval; - - if (seq_leq (tc->snd_una, tc->snd_congestion + tc->burst_acked)) - interval = 1; - else - interval = clib_max ((u32) tc->rto * TCP_TO_TIMER_TICK, 1); - - tcp_timer_update (tw, tc, TCP_TIMER_PERSIST, interval); -} - always_inline void tcp_persist_timer_reset (tcp_timer_wheel_t * tw, tcp_connection_t * tc) { @@ -98,21 +92,15 @@ tcp_retransmit_timer_update (tcp_timer_wheel_t * tw, tcp_connection_t * tc) if (tc->snd_una == tc->snd_nxt) { tcp_retransmit_timer_reset (tw, tc); - if (tc->snd_wnd < tc->snd_mss) - tcp_persist_timer_update (tw, tc); + if (tc->snd_wnd < tc->snd_mss && + !tcp_timer_is_active (tc, TCP_TIMER_PERSIST)) + tcp_persist_timer_set (tw, tc); } else tcp_timer_update (tw, tc, TCP_TIMER_RETRANSMIT, clib_max ((u32) tc->rto * TCP_TO_TIMER_TICK, 1)); } -always_inline u8 -tcp_timer_is_active (tcp_connection_t * tc, tcp_timers_e timer) -{ - return tc->timers[timer] != TCP_TIMER_HANDLE_INVALID - || (tc->pending_timers & (1 << timer)); -} - always_inline void tcp_timer_expire_timers (tcp_timer_wheel_t * tw, f64 now) { -- cgit 1.2.3-korg