|
Bugzilla – Full Text Bug Listing |
| Summary: | RTTEstimator fails to estimate RTT for the first few packets of a TCP flow | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | l.salameh |
| Component: | tcp | Assignee: | natale.patriciello |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | l.salameh, ns-bugs |
| Priority: | P5 | ||
| Version: | ns-3-dev | ||
| Hardware: | Mac Intel | ||
| OS: | Mac OS | ||
| Attachments: |
Fix for rtt estimator
Fix for RTT estimator issues. Testcase Nat's patch, avoid the API change TcpSocketBase patch from the current ns-3-dev Testcase improvement |
||
PS, this bug is more problematic when timestamp option is disabled. Another important problem with RTT estimation is that it doesn't work for SYN -> SYN/ACK packets on one end, and SYN/ACK -> ACK packets. The problem once again is the failure to record m_history for SYN packets. I have a modified patch that tackles this bug and the previous issue. Created attachment 2283 [details]
Fix for RTT estimator issues.
Created attachment 2284 [details]
Testcase
Added the testcase which control two things:
the first is for each ACK we need to have a valid estimate
the second is that we should control retransmission, which needs to be flagged as such
Created attachment 2285 [details]
Nat's patch, avoid the API change
This avoid the API change, adding the isRetransmission flag to Send* API.
Nat, the patch looks good and it works on my end. Fixed in 1188{7,8,9}, added the test case
Reopening this bug. Unfortunately the check:
if (seq == m_txBuffer->HeadSequence ()
&& m_txBuffer->HeadSequence () != m_highRxAckMark)
{
isRetransmission = true;
}
isn't correct. It never marks retransmissions correctly. The highest received ack, i.e. the duplicate ack will always equal the txbuffer head sequence on a retransmit.
I really don't see any better way to fix this than change the SendDataPacket API.
(In reply to l.salameh from comment #8) > Reopening this bug. Unfortunately the check: > > if (seq == m_txBuffer->HeadSequence () > && m_txBuffer->HeadSequence () != m_highRxAckMark) > { > isRetransmission = true; > } > > > isn't correct. It never marks retransmissions correctly. The highest > received ack, i.e. the duplicate ack will always equal the txbuffer head > sequence on a retransmit. > > I really don't see any better way to fix this than change the SendDataPacket > API. Hi, I see your comment only now. I'll use numbers instead of expressions: (1) is seq == m_txBuffer->HeadSequence () (2) is m_txBuffer->HeadSequence () != m_highRxAckMark I got the point for (2). Now, probably it was not a good choice (I re-read our conversation on IRC). So, we can use (3): (3) seq != m_highTxMark (which is the same as seq < m_highTxMark) m_highTxMark will contain the highest seq number we transmitted; if (3) holds, it means we have already transmitted that segment. Please note that it is the same check we are doing for doubling the RTO timer, so either this is correct, or we also doubled RTO timer incorrectly. Do you agree? Created attachment 2296 [details]
TcpSocketBase patch from the current ns-3-dev
This removes the old check and add seq < m_highTxMark (it can be !=).
I don't know why in the original TCP we checked for seq == m_txBuffer.HeadSequence()
Created attachment 2297 [details]
Testcase improvement
Check isRetransmission for current retransmission (with different cases: fast retransmission, rto, rto after fast retransmission)
Hi Natale, Actually, you are right. Checking: seq < m_highTxMark catches the case where we are retransmitting a whole bunch of sequence numbers after a timeout, which passing a retransmit flag by the caller does not. The patch looks OK. I think maybe we should keep seq != m_highTxMark rather than seq < m_highTxMark because ... urm sequence number space wrap around? Although operator < is overloaded for SequenceNumber to take care of this ... Fixed in 11914:6719ab7ed738 (sorry for the delay) |
Created attachment 2278 [details] Fix for rtt estimator Running an example TCP short flow (1 or 2 segments only) and enabling the RTTEstimator logs shows that no samples are taken and used to estimate the RTT. This is because, for the first few segments, in TCPSocketBase, m_history is not populated correctly. The data segment history, m_history is set up in: uint32_t TcpSocketBase::SendDataPacket (SequenceNumber32 seq, uint32_t maxSize, bool withAck) { NS_LOG_FUNCTION (this << seq << maxSize << withAck); bool isRetransmission = false; if ( seq == m_txBuffer->HeadSequence () ) { isRetransmission = true; } . . . // update the history of sequence numbers used to calculate the RTT if (isRetransmission == false) { // This is the next expected one, just log at end m_history.push_back (RttHistory (seq, sz, Simulator::Now () )); } ... Unfortunately, the isRetransmission flag is incorrectly set when we are at the start of the connection, i.e. for sequence number = 1, and sequence number = segsize (if initial cwnd = 1). When we have only one segment in our m_txBuffer, isRetransmission is incorrectly set to true. As a solution, we should pass whether the data sent is a retransmit to the TcpSocketBase::SendDataPacket function. Please see attached patch. The patch probably needs to be tested more thoroughly.