You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2018/03/22 07:29:36 UTC

[trafficserver] branch quic-latest updated: Handle ACK block legnth and gap as ranges

This is an automated email from the ASF dual-hosted git repository.

maskit pushed a commit to branch quic-latest
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/quic-latest by this push:
     new f3cceb5  Handle ACK block legnth and gap as ranges
f3cceb5 is described below

commit f3cceb579ca3049da4a47efb7e551d38f8444824
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Thu Mar 22 16:27:49 2018 +0900

    Handle ACK block legnth and gap as ranges
    
    This fixes #3314
---
 iocore/net/quic/QUICFrame.cc                  | 34 +++++++++++++++++++++++++++
 iocore/net/quic/QUICFrame.h                   | 20 ++++++++++++++++
 iocore/net/quic/QUICLossDetector.cc           | 32 ++++++++++++-------------
 iocore/net/quic/QUICLossDetector.h            |  2 +-
 iocore/net/quic/test/test_QUICLossDetector.cc | 18 ++++++++++++++
 5 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/iocore/net/quic/QUICFrame.cc b/iocore/net/quic/QUICFrame.cc
index bb12bed..5663d7c 100644
--- a/iocore/net/quic/QUICFrame.cc
+++ b/iocore/net/quic/QUICFrame.cc
@@ -474,6 +474,40 @@ QUICAckFrame::_get_ack_block_section_offset() const
 }
 
 //
+// QUICAckFrame::PacketNumberRange
+//
+QUICAckFrame::PacketNumberRange::PacketNumberRange(PacketNumberRange &&a) noexcept
+{
+  this->_first = a._first;
+  this->_last  = a._last;
+}
+
+uint64_t
+QUICAckFrame::PacketNumberRange::first() const
+{
+  return this->_first;
+}
+
+uint64_t
+QUICAckFrame::PacketNumberRange::last() const
+{
+  return this->_last;
+}
+
+uint64_t
+QUICAckFrame::PacketNumberRange::size() const
+{
+  return this->_first - this->_last;
+}
+
+bool
+QUICAckFrame::PacketNumberRange::contains(QUICPacketNumber x) const
+{
+  return static_cast<uint64_t>(this->_last) <= static_cast<uint64_t>(x) &&
+         static_cast<uint64_t>(x) <= static_cast<uint64_t>(this->_first);
+}
+
+//
 // QUICAckFrame::AckBlock
 //
 uint64_t
diff --git a/iocore/net/quic/QUICFrame.h b/iocore/net/quic/QUICFrame.h
index 6d2044e..fb4729d 100644
--- a/iocore/net/quic/QUICFrame.h
+++ b/iocore/net/quic/QUICFrame.h
@@ -98,6 +98,26 @@ private:
 class QUICAckFrame : public QUICFrame
 {
 public:
+  class PacketNumberRange
+  {
+  public:
+    PacketNumberRange(QUICPacketNumber first, QUICPacketNumber last) : _first(first), _last(last) {}
+    PacketNumberRange(PacketNumberRange &&a) noexcept;
+    QUICPacketNumber first() const;
+    QUICPacketNumber last() const;
+    uint64_t size() const;
+    bool contains(QUICPacketNumber x) const;
+    bool
+    operator<(const PacketNumberRange &b) const
+    {
+      return static_cast<uint64_t>(this->first()) < static_cast<uint64_t>(b.first());
+    }
+
+  private:
+    QUICPacketNumber _first;
+    QUICPacketNumber _last;
+  };
+
   class AckBlock
   {
   public:
diff --git a/iocore/net/quic/QUICLossDetector.cc b/iocore/net/quic/QUICLossDetector.cc
index a4f0bb2..6849600 100644
--- a/iocore/net/quic/QUICLossDetector.cc
+++ b/iocore/net/quic/QUICLossDetector.cc
@@ -178,10 +178,14 @@ QUICLossDetector::_on_ack_received(const std::shared_ptr<const QUICAckFrame> &ac
               this->_retransmittable_outstanding.load(), this->_handshake_outstanding.load());
 
   // Find all newly acked packets.
-  for (auto acked_packet_number : this->_determine_newly_acked_packets(*ack_frame)) {
-    auto pi = this->_sent_packets.find(acked_packet_number);
-    if (pi != this->_sent_packets.end()) {
-      this->_on_packet_acked(pi->first, pi->second->bytes);
+  for (auto &&range : this->_determine_newly_acked_packets(*ack_frame)) {
+    for (auto ite = this->_sent_packets.begin(); ite != this->_sent_packets.end(); /* no increment here*/) {
+      auto tmp_ite = ite;
+      tmp_ite++;
+      if (range.contains(ite->first)) {
+        this->_on_packet_acked(ite->first, ite->second->bytes);
+      }
+      ite = tmp_ite;
     }
   }
 
@@ -439,24 +443,20 @@ QUICLossDetector::_retransmit_lost_packet(const QUICPacket &packet)
   this->_transmitter->retransmit_packet(packet);
 }
 
-std::set<QUICPacketNumber>
+std::set<QUICAckFrame::PacketNumberRange>
 QUICLossDetector::_determine_newly_acked_packets(const QUICAckFrame &ack_frame)
 {
-  std::set<QUICPacketNumber> packets;
+  std::set<QUICAckFrame::PacketNumberRange> numbers;
   QUICPacketNumber x = ack_frame.largest_acknowledged();
-  for (uint64_t i = 0; i <= ack_frame.ack_block_section()->first_ack_block_length(); ++i) {
-    packets.insert(x--);
-  }
+  numbers.insert({x, static_cast<uint64_t>(x) - ack_frame.ack_block_section()->first_ack_block_length()});
+  x -= ack_frame.ack_block_section()->first_ack_block_length() + 1;
   for (auto &&block : *(ack_frame.ack_block_section())) {
-    for (uint64_t i = 0; i <= block.gap(); ++i) {
-      x--;
-    }
-    for (uint64_t i = 0; i <= block.length(); ++i) {
-      packets.insert(x--);
-    }
+    x -= block.gap() + 1;
+    numbers.insert({x, static_cast<uint64_t>(x) - block.length()});
+    x -= block.length() + 1;
   }
 
-  return packets;
+  return numbers;
 }
 
 void
diff --git a/iocore/net/quic/QUICLossDetector.h b/iocore/net/quic/QUICLossDetector.h
index 924f221..861e426 100644
--- a/iocore/net/quic/QUICLossDetector.h
+++ b/iocore/net/quic/QUICLossDetector.h
@@ -129,7 +129,7 @@ private:
   void _on_loss_detection_alarm();
   void _retransmit_lost_packet(const QUICPacket &packet);
 
-  std::set<QUICPacketNumber> _determine_newly_acked_packets(const QUICAckFrame &ack_frame);
+  std::set<QUICAckFrame::PacketNumberRange> _determine_newly_acked_packets(const QUICAckFrame &ack_frame);
 
   void _retransmit_handshake_packets();
   void _send_one_packet();
diff --git a/iocore/net/quic/test/test_QUICLossDetector.cc b/iocore/net/quic/test/test_QUICLossDetector.cc
index 3fbfbb3..a5d8cdb 100644
--- a/iocore/net/quic/test/test_QUICLossDetector.cc
+++ b/iocore/net/quic/test/test_QUICLossDetector.cc
@@ -26,6 +26,7 @@
 #include "QUICLossDetector.h"
 #include "QUICEvents.h"
 #include "Mock.h"
+#include "ts/ink_hrtime.h"
 
 TEST_CASE("QUICLossDetector_Loss", "[quic]")
 {
@@ -153,3 +154,20 @@ TEST_CASE("QUICLossDetector_Loss", "[quic]")
     CHECK(cc->lost_packets.find(pn9) == cc->lost_packets.end());
   }
 }
+
+TEST_CASE("QUICLossDetector_HugeGap", "[quic]")
+{
+  MockQUICPacketTransmitter *tx    = new MockQUICPacketTransmitter();
+  MockQUICCongestionController *cc = new MockQUICCongestionController();
+  QUICLossDetector detector(tx, cc);
+
+  // Check initial state
+  CHECK(tx->retransmitted.size() == 0);
+
+  auto t1                           = Thread::get_hrtime();
+  std::shared_ptr<QUICAckFrame> ack = QUICFrameFactory::create_ack_frame(100000000, 100, 10000000);
+  ack->ack_block_section()->add_ack_block({20000000, 30000000});
+  detector.handle_frame(ack);
+  auto t2 = Thread::get_hrtime();
+  CHECK(t2 - t1 < HRTIME_MSECONDS(100));
+}

-- 
To stop receiving notification emails like this one, please contact
maskit@apache.org.