FreeBSD tcp backoff problem

[Anumita Biswas]

Hi,

I work on a stack which is derived from FreeBSD. We have found a problem in
the stack which shows up on TCP connections that do not use timestamps as
follows. TCP backs off its retransmissions exponentially even though forward
progress is being made.

Appliance(our stack) sends data
Client sends ack, but appliance does not receive it
Appliance times out and resends packet
Client sends ack,
Appliance receives this ack

This sequence continues. But each time the timeout goes up as 16, 32, 64,
64, 64 etc. Since each retransmitted packet is acked, the appliance should
not continue to back off.

The problem seems to be that t_rxtshift is not being reset when the ack is
received. Normally t_rxtshift will be set to zero in tcp_xmit_timer() which
is called from tcp_input() when a packet with a valid round trip time is
received. When times stamps are not being used, as is the case with this
connection, tcp_xmit_timer() is only called if t_rtttime is non-zero.
However, it is set to zero when the retransmission timeout happens. Thus,
tcp_xmit_timers() is never called during the sequence of packets shown
above.

So like in this case:
 if (tlen == 0) {
                        if (SEQ_GT(th->th_ack, tp->snd_una) &&
                            SEQ_LEQ(th->th_ack, tp->snd_max) &&
                            tp->snd_cwnd >= tp->snd_wnd &&
                            ((!tcp_do_newreno && !tp->sack_enable &&
                              tp->t_dupacks < tcprexmtthresh) ||
                             ((tcp_do_newreno || tp->sack_enable) &&
... etc. ...
                                 */
                                if ((to.to_flags & TOF_TS) != 0 &&
                                    to.to_tsecr) {
                                        tcp_xmit_timer(tp,
                                            ticks - to.to_tsecr + 1);
                                } else if (tp->t_rtttime &&
                                            SEQ_GT(th->th_ack, tp->t_rtseq))
{
                                        tcp_xmit_timer(tp,
                                                        ticks -
tp->t_rtttime);
                                }
Since timestamps are not in use, and tp->t_rtttime is 0 as we just had a
retransmission, we don't bring down tp->t_rxtshift to 0.

There is a comment in the code subsequently,
                               /*
                                 * If all outstanding data are acked, stop
                                 * retransmit timer, otherwise restart timer
                                 * using current (possibly backed-off)
value.
                                 * If process is waiting for space,
                                 * wakeup/selwakeup/signal.  If data
                                 * are ready to send, let tcp_output
                                 * decide between more output or persist.

which seems to indicate that we should use possibly backed off value when
restarting the retransmit timer. But we dont do that when timestamps are in
use. So the comment is confusing. But when timestamps are not in use,
t_rxtshift is not brought down to 0.

Would it make sense to correct the comment and introduce an else condition
here:
                                if ((to.to_flags & TOF_TS) != 0 &&
                                    to.to_tsecr) {
                                        tcp_xmit_timer(tp,
                                            ticks - to.to_tsecr + 1);
                                } else if (tp->t_rtttime &&
                                            SEQ_GT(th->th_ack, tp->t_rtseq))
{
                                        tcp_xmit_timer(tp,
                                                        ticks -
tp->t_rtttime);
                                }
                               else {
                                     tp->t_rxtshift = 0;
                               }

We might need a similar change when we receive more than 3 dupacks in
tcp_input and don't call tcp_xmit_timer(). Though I don't know if in that
case, tp->t_rtttime will be 0. I also dont know if we should be initializing
anything else besides tp->t_rxtshift in this else part.

Any comments on this would be appreciated.

thanks,
Anumita.