Page MenuHomeFreeBSD

Fixing RTO timer during SACK loss recovery

Authored by rscheff on Jan 14 2020, 12:00 AM.



Since its inception, a partial ack (where the right edge of continous
data moves forwards) would clear the RTO timer. Thus if some sack
retransmission other than the initial segment retransmitted is lost,
the session would then be at the mercy of some other - longer waiting -
timer to make forward progress. The very first retransmission is typically the
most prone to encounter a still filled up queue. But that retransmissions is
covered by the still running RTO timer from the in-sequence phase at that point.

It is counter-intuitive to disable the RTO timer completely, once the very
first of possibly many dozends of packets that need to be retransmitted, makes
it to the receiver. It is more logical to restart the RTO timer when some
additional in-sequence data is acked.

Short of lost retransmission detection, this will help with a more timely
loss recovery, when SACK retransmissions still encounter a congested

Diff Detail

Lint OK
No Unit Test Coverage
Build Status
Buildable 28957
Build 26941: arc lint + arc unit

Event Timeline

Just found, that RFC2582, last paragraph of section 4 mentions the "slow-but-steady" reset (re-arm) of the RTO after each partial ack in the context of SACK:

one possibility for a more optimal
algorithm might be one that recovered more quickly from multiple
packet drops, and combined this with the Slow-but-Steady variant in
terms of resetting the retransmit timers.  We note, however, that
there is a limitation to the potential performance in this case in
the absence of the SACK option.

Note this was found in FBSD11.

Head seems to send out one rescue retransmission already (before explicitly implemented with 6675 support?), restarting the RTO timer again.

Maybe MFC this?

Tested in both bsd11 and bsd12. Without this fix, I can find a 2nd retransmission triggered by the persistent timer, not the RTO timer. But this means the persistent timer is still alive. Suggest revise this change.

Looks the persistent timer may be still alive. Suggest revise this change.

This revision now requires changes to proceed.Jan 27 2020, 9:18 PM
  • Merge branch 'master' into fix-sack-rto
  • ensure that persist and retransmit timers are not running in parallel
  • and also resetting the timeout backoff counter

I suspect this snippet here is not working as intended:

tcp_output.c-1517-              if (!tcp_timer_active(tp, TT_REXMT) &&
tcp_output.c-1518-                  ((sack_rxmit && tp->snd_nxt != tp->snd_max) ||
tcp_output.c-1519-                   (tp->snd_nxt != tp->snd_una))) {
tcp_output.c-1520-                      if (tcp_timer_active(tp, TT_PERSIST)) {
tcp_output.c-1521-                              tcp_timer_activate(tp, TT_PERSIST, 0);
tcp_output.c-1522-                              tp->t_rxtshift = 0;
tcp_output.c-1523-                      }
tcp_output.c:1524:                      tcp_timer_activate(tp, TT_REXMT, tp->t_rxtcur);

Because during SACK retransmissions, snd_nxt is not used by tcp_output to figure out which data to send next - unlike NewReno. So, most likely, this

(sack_rxmit && tp->snd_nxt != tp->snd_max)

never evaluates true...

Turned out that this change is unnecessary, when SACK loss recovery works properly.