From 6f1f4668323f5e88ff85ea1779900bc35cb70ca7 Mon Sep 17 00:00:00 2001 From: arvidn Date: Mon, 31 Dec 2018 15:50:12 +0100 Subject: [PATCH] improve logic for fast-retransmitting packets on incoming SACK --- src/utp_stream.cpp | 114 +++++++++++++++++++++++++++++++-------------- 1 file changed, 80 insertions(+), 34 deletions(-) diff --git a/src/utp_stream.cpp b/src/utp_stream.cpp index 8beb7f2dc..0dfbbdc53 100644 --- a/src/utp_stream.cpp +++ b/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" - , static_cast(this), ack_nr, bitmask.c_str(), m_seq_nr); + UTP_LOGV("%8p: got SACK first:%d %s our_seq_nr:%u fast_resend_seq_nr:%d\n" + , static_cast(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 - // this is used to determine if we should trigger more fast re-sends - int dups = 0; + boost::array resend; + int num_to_resend = 0; - // the sequence number of the last ACKed packet - int last_ack = packet_ack; + // this was implicitly lost + + 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(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(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. - // 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; + bool cut_cwnd = true; - // only re-sending a single packet per sack - // appears to improve performance by making it - // less likely to loose the re-sent packet. Because - // when that happens, we must time-out in order - // to continue, which takes a long time. - if (m_fast_resend_seq_nr != last_ack) + // we received more than dup_ack_limit ACKs in this SACK message. + // trigger fast re-send. This is not an equal check because 3 identical ACKS + // are only 2 duplicates + for (int i = 0; i < num_to_resend; ++i) + { + boost::uint16_t const pkt_seq = resend[i]; + + packet* p = m_outbuf.at(pkt_seq); + UTP_LOGV("%8p: Packet %d lost. (fast_resend_seq_nr:%d trigger fast-resend)\n" + , static_cast(this), pkt_seq, m_fast_resend_seq_nr); + if (p) { - packet* p = m_outbuf.at(m_fast_resend_seq_nr); - m_fast_resend_seq_nr = (m_fast_resend_seq_nr + 1) & ACK_MASK; - if (p) + if (resend_packet(p, true)) { - ++num_resent; - 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; + } } }