[lwip-devel] bug in LWIP TCP dupacks count

[solganik]

Hi It seems there is a bug in counting 3-duplicate ACKS introduced in
the following commit:

commit dee1d82c11f32544788528f964f61359b215375c
Author: kieranm <kieranm>
Date:   Thu Oct 29 15:48:57 2009 +0000

    re-work the fast retransmission code to follow algorithm from TCP/IP
    Illustrated



Current code zeroes duplicate acks if incoming packet contains data.
Which is incorrect, as though according to RFC such packet does not
count as Duplicate ACK, it does not say it resets the "counter".
Consider the following situation which is negatively affected by this code.

Lets assume that client and server are exchaning packets, lets call
packets from SERVER to Client SRV_CLIENT, and Client to Server
CLIENT_SRV.
So here is the flow

SRV_CLIENT1 ---------> Lets assume this packet is lost
SRV_CLIENT2
CLIENT_SRV1  (no data) (dupack #1 telling that client expects SRV_CLIENT1 seq)
--> Now client wants actually to send data to server, which is ok, as
server receive window is not affected
CLIENT_SRV2 ------> This packet will contain data that client wants to
tell server, not that ack number on this packet is still SRV_CLIENT1,
as client did not receive SRV_CLIENT1 packet.

In this situation, LWIP will loose count of duplicate acks, therefore
not letting the server fast recover from loss.

I think whole case of :

 /* If Clause (1) or more is true, but not a duplicate ack, reset
       * count of consecutive duplicate acks */
      if (!found_dupack) {
        pcb->dupacks = 0;
      }

is not required, as once entered "recovery" state there will be an
advance on the window once recovered and dupacks will be "zeroed".