Skip to content

Commit

Permalink
tcp (fixes #1043): Fix BBR incorrect minRtt updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@any-ket
any-ket committed Jul 3, 2024
1 parent 4d29357 commit 3be702f
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 48 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Changes from ns-3.42 to ns-3-dev

### New API

* (tcp) A new trace source `TcpSocketBase::LastRtt` has been added for tracing the last RTT sample observed. The existing trace source `TcpSocketBase::Rtt` is still providing the smoothed RTT, although it had been incorrectly documented as providing the last RTT.

### Changes to existing API

* (lr-wpan) Attribute `macBeaconPayload` in `MacPibAttributes` is now a std::vector<uint8_t> instead of a packet pointer.
Expand Down
2 changes: 1 addition & 1 deletion src/internet/model/tcp-bbr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ TcpBbr::CongestionStateSet(Ptr<TcpSocketState> tcb, const TcpSocketState::TcpCon
if (newState == TcpSocketState::CA_OPEN && !m_isInitialized)
{
NS_LOG_DEBUG("CongestionStateSet triggered to CA_OPEN :: " << newState);
m_minRtt = tcb->m_lastRtt.Get() != Time::Max() ? tcb->m_lastRtt.Get() : Time::Max();
m_minRtt = tcb->m_srtt.Get() != Time::Max() ? tcb->m_srtt.Get() : Time::Max();
m_minRttStamp = Simulator::Now();
m_priorCwnd = tcb->m_cWnd;
tcb->m_ssThresh = tcb->m_initialSsThresh;
Expand Down
133 changes: 90 additions & 43 deletions src/internet/model/tcp-socket-base.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ TcpSocketBase::GetTypeId()
MakeTraceSourceAccessor(&TcpSocketBase::m_rto),
"ns3::TracedValueCallback::Time")
.AddTraceSource("RTT",
"Last RTT sample",
"Smoothed RTT",
MakeTraceSourceAccessor(&TcpSocketBase::m_srttTrace),
"ns3::TracedValueCallback::Time")
.AddTraceSource("LastRTT",
"RTT of the last (S)ACKed packet",
MakeTraceSourceAccessor(&TcpSocketBase::m_lastRttTrace),
"ns3::TracedValueCallback::Time")
.AddTraceSource("NextTxSequence",
Expand Down Expand Up @@ -322,6 +326,10 @@ TcpSocketBase::TcpSocketBase()

ok = m_tcb->TraceConnectWithoutContext("RTT", MakeCallback(&TcpSocketBase::UpdateRtt, this));
NS_ASSERT(ok == true);

ok = m_tcb->TraceConnectWithoutContext("LastRTT",
MakeCallback(&TcpSocketBase::UpdateLastRtt, this));
NS_ASSERT(ok == true);
}

TcpSocketBase::TcpSocketBase(const TcpSocketBase& sock)
Expand Down Expand Up @@ -458,6 +466,10 @@ TcpSocketBase::TcpSocketBase(const TcpSocketBase& sock)

ok = m_tcb->TraceConnectWithoutContext("RTT", MakeCallback(&TcpSocketBase::UpdateRtt, this));
NS_ASSERT(ok == true);

ok = m_tcb->TraceConnectWithoutContext("LastRTT",
MakeCallback(&TcpSocketBase::UpdateLastRtt, this));
NS_ASSERT(ok == true);
}

TcpSocketBase::~TcpSocketBase()
Expand Down Expand Up @@ -1989,7 +2001,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber,
else if (ackNumber == oldHeadSequence)
{
// DupAck. Artificially call PktsAcked: after all, one segment has been ACKed.
m_congestionControl->PktsAcked(m_tcb, 1, m_tcb->m_lastRtt);
m_congestionControl->PktsAcked(m_tcb, 1, m_tcb->m_srtt);
}
else if (ackNumber > oldHeadSequence)
{
Expand Down Expand Up @@ -2057,7 +2069,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber,
// This partial ACK acknowledge the fact that one segment has been
// previously lost and now successfully received. All others have
// been processed when they come under the form of dupACKs
m_congestionControl->PktsAcked(m_tcb, 1, m_tcb->m_lastRtt);
m_congestionControl->PktsAcked(m_tcb, 1, m_tcb->m_srtt);
NewAck(ackNumber, m_isFirstPartialAck);

if (m_isFirstPartialAck)
Expand Down Expand Up @@ -2085,7 +2097,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber,
// of RecoveryPoint.
else if (ackNumber < m_recover && m_tcb->m_congState == TcpSocketState::CA_LOSS)
{
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_lastRtt);
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_srtt);
m_congestionControl->IncreaseWindow(m_tcb, segsAcked);

NS_LOG_DEBUG(" Cong Control Called, cWnd=" << m_tcb->m_cWnd
Expand All @@ -2100,7 +2112,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber,
}
else if (m_tcb->m_congState == TcpSocketState::CA_CWR)
{
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_lastRtt);
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_srtt);
// TODO: need to check behavior if marking is compounded by loss
// and/or packet reordering
if (!m_congestionControl->HasCongControl() && segsAcked >= 1)
Expand All @@ -2113,15 +2125,15 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber,
{
if (m_tcb->m_congState == TcpSocketState::CA_OPEN)
{
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_lastRtt);
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_srtt);
}
else if (m_tcb->m_congState == TcpSocketState::CA_DISORDER)
{
if (segsAcked >= oldDupAckCount)
{
m_congestionControl->PktsAcked(m_tcb,
segsAcked - oldDupAckCount,
m_tcb->m_lastRtt);
m_tcb->m_srtt);
}

if (!isDupack)
Expand Down Expand Up @@ -2158,7 +2170,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber,
// TODO: check consistency for dynamic segment size
segsAcked =
static_cast<uint32_t>(ackNumber - oldHeadSequence) / m_tcb->m_segmentSize;
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_lastRtt);
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_srtt);
m_congestionControl->CwndEvent(m_tcb, TcpSocketState::CA_EVENT_COMPLETE_CWR);
m_congestionControl->CongestionStateSet(m_tcb, TcpSocketState::CA_OPEN);
m_tcb->m_congState = TcpSocketState::CA_OPEN;
Expand All @@ -2177,7 +2189,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber,
// can increase cWnd)
segsAcked = (ackNumber - m_recover) / m_tcb->m_segmentSize;

m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_lastRtt);
m_congestionControl->PktsAcked(m_tcb, segsAcked, m_tcb->m_srtt);

m_congestionControl->CongestionStateSet(m_tcb, TcpSocketState::CA_OPEN);
m_tcb->m_congState = TcpSocketState::CA_OPEN;
Expand Down Expand Up @@ -3622,59 +3634,88 @@ TcpSocketBase::ReceivedData(Ptr<Packet> p, const TcpHeader& tcpHeader)
}
}

Time
TcpSocketBase::CalculateRttSample(const TcpHeader& tcpHeader, const RttHistory& rttHistory)
{
NS_LOG_FUNCTION(this);
SequenceNumber32 ackSeq = tcpHeader.GetAckNumber();
Time rtt;

if (!rttHistory.retx && ackSeq >= (rttHistory.seq + SequenceNumber32(rttHistory.count)))
{ // Ok to use this sample
if (m_timestampEnabled && tcpHeader.HasOption(TcpOption::TS))
{
Ptr<const TcpOptionTS> ts;
ts = DynamicCast<const TcpOptionTS>(tcpHeader.GetOption(TcpOption::TS));
rtt = TcpOptionTS::ElapsedTimeFromTsValue(ts->GetEcho());
if (rtt.IsZero())
{
NS_LOG_LOGIC("TcpSocketBase::EstimateRtt - RTT calculated from TcpOption::TS "
"is zero, approximating to 1us.");
NS_LOG_DEBUG("RTT calculated from TcpOption::TS is zero, updating rtt to 1us.");
rtt = MicroSeconds(1);
}
}
else
{
// Elapsed time since the packet was transmitted
rtt = Simulator::Now() - rttHistory.time;
}
}
return rtt;
}

void
TcpSocketBase::EstimateRtt(const TcpHeader& tcpHeader)
{
NS_LOG_FUNCTION(this);
SequenceNumber32 ackSeq = tcpHeader.GetAckNumber();
Time m = Time(0.0);
Time rtt;

// An ack has been received, calculate rtt and log this measurement
// Note we use a linear search (O(n)) for this since for the common
// case the ack'ed packet will be at the head of the list
if (!m_history.empty())
{
RttHistory& h = m_history.front();
if (!h.retx && ackSeq >= (h.seq + SequenceNumber32(h.count)))
{ // Ok to use this sample
if (m_timestampEnabled && tcpHeader.HasOption(TcpOption::TS))
{
Ptr<const TcpOptionTS> ts;
ts = DynamicCast<const TcpOptionTS>(tcpHeader.GetOption(TcpOption::TS));
m = TcpOptionTS::ElapsedTimeFromTsValue(ts->GetEcho());
if (m.IsZero())
{
NS_LOG_LOGIC("TcpSocketBase::EstimateRtt - RTT calculated from TcpOption::TS "
"is zero, approximating to 1us.");
m = MicroSeconds(1);
}
}
else
RttHistory& earliestTransmittedPktHistory = m_history.front();
rtt = CalculateRttSample(tcpHeader, earliestTransmittedPktHistory);

// Store ACKed packet that has the latest transmission time to update `lastRtt`
RttHistory latestTransmittedPktHistory = earliestTransmittedPktHistory;

// Delete all ACK history with seq <= ack
while (!m_history.empty())
{
RttHistory& rttHistory = m_history.front();
if ((rttHistory.seq + SequenceNumber32(rttHistory.count)) > ackSeq)
{
m = Simulator::Now() - h.time; // Elapsed time
break; // Done removing
}

latestTransmittedPktHistory = rttHistory;
m_history.pop_front(); // Remove
}
}

// Now delete all ack history with seq <= ack
while (!m_history.empty())
{
RttHistory& h = m_history.front();
if ((h.seq + SequenceNumber32(h.count)) > ackSeq)
// In case of multiple packets being ACKed in a single acknowledgement, `m_lastRtt` is
// RTT of the last (S)ACKed packet calculated using the data packet with the latest
// transmission time
Time lastRtt = CalculateRttSample(tcpHeader, latestTransmittedPktHistory);
if (!lastRtt.IsZero())
{
break; // Done removing
NS_LOG_DEBUG("Last RTT sample updated to: " << lastRtt);
m_tcb->m_lastRtt = lastRtt;
}
m_history.pop_front(); // Remove
}

if (!m.IsZero())
if (!rtt.IsZero())
{
m_rtt->Measurement(m); // Log the measurement
m_rtt->Measurement(rtt); // Log the measurement
// RFC 6298, clause 2.4
m_rto = Max(m_rtt->GetEstimate() + Max(m_clockGranularity, m_rtt->GetVariation() * 4),
m_minRto);
m_tcb->m_lastRtt = m_rtt->GetEstimate();
m_tcb->m_minRtt = std::min(m_tcb->m_lastRtt.Get(), m_tcb->m_minRtt);
NS_LOG_INFO(this << m_tcb->m_lastRtt << m_tcb->m_minRtt);
m_tcb->m_srtt = m_rtt->GetEstimate();
m_tcb->m_minRtt = std::min(m_tcb->m_srtt.Get(), m_tcb->m_minRtt);
NS_LOG_INFO(this << m_tcb->m_srtt << m_tcb->m_minRtt);
}
}

Expand Down Expand Up @@ -4525,6 +4566,12 @@ TcpSocketBase::UpdateBytesInFlight(uint32_t oldValue, uint32_t newValue) const

void
TcpSocketBase::UpdateRtt(Time oldValue, Time newValue) const
{
m_srttTrace(oldValue, newValue);
}

void
TcpSocketBase::UpdateLastRtt(Time oldValue, Time newValue) const
{
m_lastRttTrace(oldValue, newValue);
}
Expand Down Expand Up @@ -4626,12 +4673,12 @@ TcpSocketBase::UpdatePacingRate()
<< m_tcb->m_ssThresh);
factor = static_cast<double>(m_tcb->m_pacingCaRatio) / 100;
}
Time lastRtt = m_tcb->m_lastRtt.Get(); // Get underlying Time value
NS_LOG_DEBUG("Last RTT is " << lastRtt.GetSeconds());
Time srtt = m_tcb->m_srtt.Get(); // Get underlying Time value
NS_LOG_DEBUG("Smoothed RTT is " << srtt.GetSeconds());

// Multiply by 8 to convert from bytes per second to bits per second
DataRate pacingRate((std::max(m_tcb->m_cWnd, m_tcb->m_bytesInFlight) * 8 * factor) /
lastRtt.GetSeconds());
srtt.GetSeconds());
if (pacingRate < m_tcb->m_maxPacingRate)
{
NS_LOG_DEBUG("Pacing rate updated to: " << pacingRate);
Expand Down
26 changes: 26 additions & 0 deletions src/internet/model/tcp-socket-base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,11 @@ class TcpSocketBase : public TcpSocket
/**
* \brief Callback pointer for RTT trace chaining
*/
TracedCallback<Time, Time> m_srttTrace;

/**
* \brief Callback pointer for Last RTT trace chaining
*/
TracedCallback<Time, Time> m_lastRttTrace;

/**
Expand Down Expand Up @@ -444,6 +449,13 @@ class TcpSocketBase : public TcpSocket
*/
void UpdateRtt(Time oldValue, Time newValue) const;

/**
* \brief Callback function to hook to TcpSocketState lastRtt
* \param oldValue old lastRtt value
* \param newValue new lastRtt value
*/
void UpdateLastRtt(Time oldValue, Time newValue) const;

/**
* \brief Install a congestion control algorithm on this socket
*
Expand Down Expand Up @@ -1049,6 +1061,20 @@ class TcpSocketBase : public TcpSocket
*/
virtual void ReceivedData(Ptr<Packet> packet, const TcpHeader& tcpHeader);

/**
* \brief Calculate RTT sample for the ACKed packet
*
* Per RFC 6298 (Section 3),
* If `m_timestampsEnabled` is true, calculate RTT using timestamps option.
* Otherwise, return RTT as the elapsed time since the packet was transmitted.
* If ACKed packed was a retrasmitted packet, return zero time.
*
* \param tcpHeader the packet's TCP header
* \param rttHistory the ACKed packet's RTT History
* \returns the RTT sample
*/
virtual Time CalculateRttSample(const TcpHeader& tcpHeader, const RttHistory& rttHistory);

/**
* \brief Take into account the packet for RTT estimation
* \param tcpHeader the packet's TCP header
Expand Down
7 changes: 6 additions & 1 deletion src/internet/model/tcp-socket-state.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ TcpSocketState::GetTypeId()
MakeTraceSourceAccessor(&TcpSocketState::m_bytesInFlight),
"ns3::TracedValueCallback::Uint32")
.AddTraceSource("RTT",
"Last RTT sample",
"Smoothed RTT",
MakeTraceSourceAccessor(&TcpSocketState::m_srtt),
"ns3::TracedValueCallback::Time")
.AddTraceSource("LastRTT",
"RTT of the last (S)ACKed packet",
MakeTraceSourceAccessor(&TcpSocketState::m_lastRtt),
"ns3::TracedValueCallback::Time");
return tid;
Expand Down Expand Up @@ -120,6 +124,7 @@ TcpSocketState::TcpSocketState(const TcpSocketState& other)
m_minRtt(other.m_minRtt),
m_bytesInFlight(other.m_bytesInFlight),
m_isCwndLimited(other.m_isCwndLimited),
m_srtt(other.m_srtt),
m_lastRtt(other.m_lastRtt),
m_ecnMode(other.m_ecnMode),
m_useEcn(other.m_useEcn),
Expand Down
7 changes: 4 additions & 3 deletions src/internet/model/tcp-socket-state.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,10 @@ class TcpSocketState : public Object

Time m_minRtt{Time::Max()}; //!< Minimum RTT observed throughout the connection

TracedValue<uint32_t> m_bytesInFlight{0}; //!< Bytes in flight
bool m_isCwndLimited{false}; //!< Whether throughput is limited by cwnd
TracedValue<Time> m_lastRtt{Seconds(0.0)}; //!< Last RTT sample collected
TracedValue<uint32_t> m_bytesInFlight{0}; //!< Bytes in flight
bool m_isCwndLimited{false}; //!< Whether throughput is limited by cwnd
TracedValue<Time> m_srtt; //!< Smoothed RTT
TracedValue<Time> m_lastRtt; //!< RTT of the last (S)ACKed packet

Ptr<TcpRxBuffer> m_rxBuffer; //!< Rx buffer (reordering buffer)

Expand Down

0 comments on commit 3be702f

Please sign in to comment.