improve logic for fast-retransmitting packets on incoming SACK

src/utp_stream.cpp

@@ -1533,40 +1533,37 @@ void utp_socket_impl::parse_sack(boost::uint16_t const packet_ack, boost::uint8_
 			mask <<= 1;
 		}
 	}
-	UTP_LOGV("%8p: got SACK first:%d %s our_seq_nr:%u\n"
+	UTP_LOGV("%8p: got SACK first:%d %s our_seq_nr:%u fast_resend_seq_nr:%d\n"
-		, static_cast<void*>(this), ack_nr, bitmask.c_str(), m_seq_nr);
+		, static_cast<void*>(this), ack_nr, bitmask.c_str(), m_seq_nr, m_fast_resend_seq_nr);
 #endif
 
-	// the number of acked packets past the fast re-send sequence number
+	boost::array<boost::uint16_t, 5> resend;
-	// this is used to determine if we should trigger more fast re-sends
+	int num_to_resend = 0;
-	int dups = 0;
 
-	// the sequence number of the last ACKed packet
+	// this was implicitly lost
-	int last_ack = packet_ack;
+
+	if (!compare_less_wrap((packet_ack + 1) & ACK_MASK, m_fast_resend_seq_nr, ACK_MASK))
+	{
+		resend[num_to_resend++] = (packet_ack + 1) & ACK_MASK;
+	}
 
 	// for each byte
-	for (boost::uint8_t const* end = ptr + size; ptr != end; ++ptr)
+	boost::uint8_t const* const start = ptr;
+	boost::uint8_t const* const end = ptr + size;
+	for (; ptr != end; ++ptr)
 	{
-		unsigned char bitfield = unsigned(*ptr);
+		unsigned char const bitfield = unsigned(*ptr);
 		unsigned char mask = 1;
 		// for each bit
 		for (int i = 0; i < 8; ++i)
 		{
 			if (mask & bitfield)
 			{
-				last_ack = ack_nr;
-				if (m_fast_resend_seq_nr == ack_nr)
-					m_fast_resend_seq_nr = (m_fast_resend_seq_nr + 1) & ACK_MASK;
-
-				if (compare_less_wrap(m_fast_resend_seq_nr, ack_nr, ACK_MASK)) ++dups;
 				// this bit was set, ack_nr was received
 				packet* p = m_outbuf.remove(ack_nr);
 				if (p)
 				{
 					*acked_bytes += p->size - p->header_size;
-					// each ACKed packet counts as a duplicate ack
-					UTP_LOGV("%8p: duplicate_acks:%u fast_resend_seq_nr:%u\n"
-						, static_cast<void*>(this), m_duplicate_acks, m_fast_resend_seq_nr);
 					ack_packet(p, now, min_rtt, ack_nr);
 				}
 				else
@@ -1576,6 +1573,11 @@ void utp_socket_impl::parse_sack(boost::uint16_t const packet_ack, boost::uint8_
 					maybe_inc_acked_seq_nr();
 				}
 			}
+			else if (!compare_less_wrap(ack_nr, m_fast_resend_seq_nr, ACK_MASK)
+				&& num_to_resend < resend.size())
+			{
+				resend[num_to_resend++] = ack_nr;
+			}
 
 			mask <<= 1;
 			ack_nr = (ack_nr + 1) & ACK_MASK;
@@ -1588,30 +1590,74 @@ void utp_socket_impl::parse_sack(boost::uint16_t const packet_ack, boost::uint8_
 		if (ack_nr == m_seq_nr) break;
 	}
 
+	// now, scan the bits in reverse, and count the number of ACKed packets. Only
+	// lost packets followed by 'dup_ack_limit' packets may be resent
+	// start with the sequence number represented by the last bit in the SACK
+	// bitmask
+	int last_resend = (packet_ack + 1 + size * 8) & ACK_MASK;
+
+	// the number of acked packets past the fast re-send sequence number
+	// this is used to determine if we should trigger more fast re-sends
+	int dups = 0;
+
+	for (boost::uint8_t const* i = end; i != start; --i)
+	{
+		unsigned char const bitfield = unsigned(i[-1]);
+		unsigned char mask = 0x80;
+		// for each bit
+		for (int k = 0; k < 8; ++k)
+		{
+			if (mask & bitfield) ++dups;
+			if (dups > dup_ack_limit) break;
+			last_resend = (last_resend - 1) & ACK_MASK;
+			mask >>= 1;
+		}
+		if (dups > dup_ack_limit) break;
+	}
+
+	// we did not get enough packets acked in this message to warrant a resend
+	if (dups <= dup_ack_limit)
+	{
+		UTP_LOGV("%8p: only %d ACKs in SACK, requires more than %d to trigger fast retransmit\n"
+			, static_cast<void*>(this), dups, dup_ack_limit);
+		num_to_resend = 0;
+	}
+
+	// now we need to (likely) prune the tail of the resend list, since all
+	// "unacked" packets that weren't followed by an acked one, don't count
+	while (num_to_resend > 0 && !compare_less_wrap(resend[num_to_resend - 1], last_resend, ACK_MASK))
+	{
+		--num_to_resend;
+	}
+
 	TORRENT_ASSERT(m_outbuf.at((m_acked_seq_nr + 1) & ACK_MASK) || ((m_seq_nr - m_acked_seq_nr) & ACK_MASK) <= 1);
 
-	// we received more than dup_ack_limit ACKs in this SACK message.
+	bool cut_cwnd = true;
-	// trigger fast re-send
-	if (dups >= dup_ack_limit && compare_less_wrap(m_fast_resend_seq_nr, last_ack, ACK_MASK))
-	{
-		experienced_loss(m_fast_resend_seq_nr);
-		int num_resent = 0;
 
-		// only re-sending a single packet per sack
+	// we received more than dup_ack_limit ACKs in this SACK message.
-		// appears to improve performance by making it
+	// trigger fast re-send. This is not an equal check because 3 identical ACKS
-		// less likely to loose the re-sent packet. Because
+	// are only 2 duplicates
-		// when that happens, we must time-out in order
+	for (int i = 0; i < num_to_resend; ++i)
-		// to continue, which takes a long time.
-		if (m_fast_resend_seq_nr != last_ack)
 	{
-			packet* p = m_outbuf.at(m_fast_resend_seq_nr);
+		boost::uint16_t const pkt_seq = resend[i];
-			m_fast_resend_seq_nr = (m_fast_resend_seq_nr + 1) & ACK_MASK;
+
+		packet* p = m_outbuf.at(pkt_seq);
+		UTP_LOGV("%8p: Packet %d lost. (fast_resend_seq_nr:%d trigger fast-resend)\n"
+			, static_cast<void*>(this), pkt_seq, m_fast_resend_seq_nr);
 		if (p)
 		{
-				++num_resent;
+			if (resend_packet(p, true))
-				if (resend_packet(p, true)) m_duplicate_acks = 0;
+			{
+				m_duplicate_acks = 0;
+				m_fast_resend_seq_nr = (pkt_seq + 1) & ACK_MASK;
 			}
 		}
+
+		if (cut_cwnd)
+		{
+			experienced_loss(pkt_seq);
+			cut_cwnd = false;
+		}
 	}
 }