Bug 2653 - Packet reordering leads to a wrong timestamp saved
Packet reordering leads to a wrong timestamp saved
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: tcp
unspecified
All All
: P3 normal
Assigned To: natale.patriciello
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-10 03:54 UTC by natale.patriciello
Modified: 2017-12-13 13:06 UTC (History)
4 users (show)

See Also:


Attachments
patch to not save smaller timestamp (765 bytes, patch)
2017-09-14 04:27 UTC, natale.patriciello
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description natale.patriciello 2017-02-10 03:54:23 UTC
We currently save the timestamp received in this way:

void
TcpSocketBase::ProcessOptionTimestamp (const Ptr<const TcpOption> option,
                                        const SequenceNumber32 &seq)
{
   NS_LOG_FUNCTION (this << option);
 
   Ptr<const TcpOptionTS> ts = DynamicCast<const TcpOptionTS> (option);
 
   m_tcb->m_rcvTimestampValue = ts->GetTimestamp ();
   m_tcb->m_rcvTimestampEchoReply = ts->GetEcho ();
...


The function is called regardless of the sequence number of the segment. It means that, if there is a reordering in the network, then we could end up saving a wrong timestamp, instead of the most recent one.

This behaviour should be checked also in the ledbat implementation.

As steps, I suggest to do the following:

* Create a test case in which the error model reorders the packets
* Check the timestamp saved and echoed back
* Check if the behaviour is consistent with https://tools.ietf.org/html/rfc7323#section-3.1
Comment 1 Tommaso Pecorella 2017-02-10 08:26:34 UTC
With the current implementation of ErrorChannel, implementing a test where packets needs to be reordered is a matter of calling ErrorChannel::SetJumpingMode.
Comment 2 natale.patriciello 2017-09-14 04:27:02 UTC
Created attachment 2917 [details]
patch to not save smaller timestamp

This avoids to save smaller timestamps; however, a test case is still missing
Comment 3 natale.patriciello 2017-12-13 13:06:42 UTC
fixed in changeset 13213:ce31dcaf0e58