tcp: (fixes #1349) Detect lost retransmissions via SACK of later-sent data

Assisted-by: Claude Fable 5

Author: tomhenderson

src/internet/model/tcp-tx-buffer.cc

@@ -774,6 +774,7 @@ TcpTxBuffer::Update(const TcpOptionSack::SackList& list, const Callback<void, Tc
     NS_LOG_INFO("Updating scoreboard, got " << list.size() << " blocks to analyze");
 
     uint32_t bytesSacked = 0;
+    Time newestSackedSentTime = Time::Min();
 
     for (auto option_it = list.begin(); option_it != list.end(); ++option_it)
     {
@@ -827,6 +828,7 @@ TcpTxBuffer::Update(const TcpOptionSack::SackList& list, const Callback<void, Tc
                     (*item_it)->m_sacked = true;
                     m_sackedOut += (*item_it)->m_packet->GetSize();
                     bytesSacked += (*item_it)->m_packet->GetSize();
+                    newestSackedSentTime = std::max(newestSackedSentTime, (*item_it)->m_lastSent);
 
                     if (m_highestSack.first == m_sentList.end() ||
                         m_highestSack.second <= beginOfCurrentPacket + pktSize)
@@ -862,6 +864,7 @@ TcpTxBuffer::Update(const TcpOptionSack::SackList& list, const Callback<void, Tc
     if (bytesSacked > 0)
     {
         NS_ASSERT_MSG(m_highestSack.first != m_sentList.end(), "Buffer status: " << *this);
+        MarkRetransmittedSegmentsLost(newestSackedSentTime);
         UpdateLostCount();
     }
 
@@ -922,6 +925,35 @@ TcpTxBuffer::UpdateLostCount()
     ConsistencyCheck();
 }
 
+void
+TcpTxBuffer::MarkRetransmittedSegmentsLost(const Time& sackedSentTime)
+{
+    NS_LOG_FUNCTION(this << sackedSentTime);
+
+    SequenceNumber32 beginOfCurrentPacket = m_firstByteSeq;
+    for (auto it = m_sentList.begin(); it != m_sentList.end(); ++it)
+    {
+        TcpTxItem* item = *it;
+        if (beginOfCurrentPacket >= m_highestSack.second)
+        {
+            break;
+        }
+        if (item->m_retrans && !item->m_sacked && item->m_lastSent < sackedSentTime)
+        {
+            item->m_retrans = false;
+            m_retrans -= item->m_packet->GetSize();
+            if (!item->m_lost)
+            {
+                item->m_lost = true;
+                m_lostOut += item->m_packet->GetSize();
+            }
+            NS_LOG_INFO("Retransmission of segment " << *item << " deemed lost");
+        }
+        beginOfCurrentPacket += item->m_packet->GetSize();
+    }
+    ConsistencyCheck();
+}
+
 bool
 TcpTxBuffer::IsLost(const SequenceNumber32& seq) const
 {

src/internet/model/tcp-tx-buffer.h

@@ -424,6 +424,27 @@ class TcpTxBuffer : public Object
      */
     void UpdateLostCount();
 
+    /**
+     * @brief Mark as lost the retransmitted segments that were sent before
+     *        a newly SACKed segment
+     *
+     * A SACK for a segment that was transmitted after a retransmission
+     * implies that the retransmission itself was dropped by the network:
+     * had it been delivered, the receiver would have acknowledged it before
+     * generating the SACK for the later segment. This is the time-based
+     * loss detection of RFC 8985 (RACK) applied to retransmissions, with a
+     * zero reordering window; Linux implemented the equivalent
+     * sequence-based heuristic as tcp_mark_lost_retrans() before adopting
+     * RACK. Clearing the retransmitted flag makes the segment eligible
+     * again for retransmission via NextSeg(); without this detection, a
+     * lost retransmission stalls the connection until the retransmission
+     * timeout fires.
+     *
+     * @param sackedSentTime Latest transmission time among the newly SACKed
+     *        segments
+     */
+    void MarkRetransmittedSegmentsLost(const Time& sackedSentTime);
+
     /**
      * @brief Remove the size specified from the lostOut, retrans, sacked count
      *