Subject: Re: netinet/tcp_input.c - 1323 timestamp handling broken
To: None <tech-net@netbsd.org>
From: Michael van Elst <mlelstv@serpens.de>
List: tech-net
Date: 04/21/2005 21:06:29
rpaulo@netbsd-pt.org (Rui Paulo) writes:

>On 2005-04-17, David Brownlee <abs@absd.org> wrote:

>> +       if (opti.ts_present && opti.ts_ecr) {
>> +               u_int32_t now;
>> +
>> +               /*
>> +                * Calculate the RTT from the returned time stamp and the
>> +                * connection's time base.  If the time stamp is later than
>> +                * the current time, fall back to non-1323 RTT calculation.
>> +                */
>> +               now = TCP_TIMESTAMP(tp);
>> +               if (SEQ_GEQ(now, opti.ts_ecr))
>> +                       opti.ts_ecr = now - opti.ts_ecr + 1;
>> +               else
>> +                       opti.ts_ecr = 0;
>> +       }


>This is somewhat a problem. This patch closes a bug when two TCP packets
>come in with the same timestamp (see PR/22551), so removing that code is
>also a problem.


I don't know what the patch should do but it effectively replaces
opti.ts_ecr (which is a timestamp) with a computed round trip time
which is then used again as a timestamp later in the code as in:

            if (opti.ts_present && opti.ts_ecr)
                    tcp_xmit_timer(tp,
                      TCP_TIMESTAMP(tp) - opti.ts_ecr + 1);


To me the 'then' case looks completely bogus and should be removed,
the 'else' case protects against timestamps from the future (and
avoids the panic from PR/22551.



-- 
-- 
                                Michael van Elst
Internet: mlelstv@serpens.de
                                "A potential Snark may lurk in every tree."