Subject: Question about NewReno
To: None <tech-net@netbsd.org>
From: Charles M. Hannum <root@ihack.net>
List: tech-net
Date: 01/04/2005 02:08:17
While looking at some other ongoing work, I noticed this bit of code
in tcp_input():

                                if (TCP_TIMER_ISARMED(tp, TCPT_REXMT) == 0 ||
                                    th->th_ack != tp->snd_una)
                                        tp->t_dupacks = 0;
                                else if (++tp->t_dupacks == tcprexmtthresh) {
                                        tcp_seq onxt = tp->snd_nxt;
                                        u_int win =
                                            min(tp->snd_wnd, tp->snd_cwnd) /
                                            2 / tp->t_segsz;
                                        if (tcp_do_newreno && SEQ_LT(th->th_ack,
                                            tp->snd_recover)) {
                                                /*
                                                 * False fast retransmit after
                                                 * timeout.  Do not cut window.
                                                 */
                                                tp->snd_cwnd += tp->t_segsz;
                                                tp->t_dupacks = 0;
                                                (void) tcp_output(tp);
                                                goto drop;
                                        }


I don't understand the "newreno" portion.  Any time we receive an ack,
we reset snd_recover=th_una=th_ack; therefore, there are only two ways
we can ever see th_ack<snd_recover on a future packet:

1) We receive a duplicate of an ack already processed.  In this case,
   th_ack<th_una, and we do not count this as a duplicate ack.  (See
   the preceding if block.)

2) We set snd_recover somewhere else.  Currently this only happens in
   the fast retransmit path.  So, this is telling us that a fast
   retransmit already happened.  However, we wouldn't do another fast
   retransmit anyway, because dupacks would now exceed the threshold.

That is to say, neither case can happen.

If I were to hazard a guess, I would say that snd_recover was supposed
to be changed in the *slow* retransmit path, specifically to signal to
fast retransmit that we are already in the process of doing a slow
retransmit.  This would seem to correspond with the comment as well.

Bypassing fast retransmit in this case would make sense.  However, why
do we increase the congestion window and send another packet in that
case?  This seems like a half-baked attempt to switch to fast
retransmit in response to the duplicate acks, but it doesn't really do
that.  (If we wanted to do that, we would ~ignore the ack that hit the
treshold and instead just increase cwnd to ssthresh (since slow
retransmit has pulled it all the way back to 1 packet), and let the
next one hit the dupacks>tcprexmtthresh case below and increase cwnd
further then.)

Does anyone have further insight into this?