Fix infinite PING

maskit · Jul 22, 2020 · 92a34b0 · 92a34b0
1 parent 42c4054
commit 92a34b0
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 6 deletions.
diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
@@ -1165,6 +1165,7 @@ QUICNetVConnection::_state_handshake_process_packet(const QUICPacket &packet)
     error = this->_state_handshake_process_handshake_packet(static_cast<const QUICHandshakePacketR &>(packet));
     if (this->_pp_key_info.is_decryption_key_available(QUICKeyPhase::INITIAL) && this->netvc_context == NET_VCONNECTION_IN) {
       this->_pp_key_info.drop_keys(QUICKeyPhase::INITIAL);
+      this->_loss_detector->on_packet_number_space_discarded(QUICPacketNumberSpace::INITIAL);
       this->_minimum_encryption_level = QUICEncryptionLevel::HANDSHAKE;
     }
     break;
@@ -1543,6 +1544,7 @@ QUICNetVConnection::_state_common_send_packet()
         if (this->_pp_key_info.is_encryption_key_available(QUICKeyPhase::INITIAL) && packet->type() == QUICPacketType::HANDSHAKE &&
             this->netvc_context == NET_VCONNECTION_OUT) {
           this->_pp_key_info.drop_keys(QUICKeyPhase::INITIAL);
+          this->_loss_detector->on_packet_number_space_discarded(QUICPacketNumberSpace::INITIAL);
           this->_minimum_encryption_level = QUICEncryptionLevel::HANDSHAKE;
         }
 

diff --git a/iocore/net/quic/Mock.h b/iocore/net/quic/Mock.h
@@ -555,6 +555,10 @@ class MockQUICCongestionController : public QUICCongestionController
   on_packet_acked(const QUICSentPacketInfo &acked_packet) override
   {
   }
+  void
+  on_packet_number_space_discarded(size_t bytes_in_flight) override
+  {
+  }
   virtual void
   process_ecn(const QUICAckFrame &ack_frame, QUICPacketNumberSpace pn_space, ink_hrtime largest_acked_time_sent) override
   {
@@ -719,6 +723,12 @@ class MockQUICLossDetector : public QUICLossDetector
   {
   }
 
+  void
+  on_packet_number_space_discarded(QUICPacketNumberSpace pn_space)
+  {
+    this->_cc.on_packet_number_space_discarded(0);
+  }
+
 private:
   QUICPinger _pinger;
   QUICPadder _padder;

diff --git a/iocore/net/quic/QUICCongestionController.h b/iocore/net/quic/QUICCongestionController.h
@@ -41,10 +41,12 @@ class QUICCongestionController
   virtual void process_ecn(const QUICAckFrame &ack, QUICPacketNumberSpace pn_space, ink_hrtime largest_acked_packet_time_sent) = 0;
   virtual void on_packets_acked(const std::vector<QUICSentPacketInfoUPtr> &packets)                                            = 0;
   virtual void on_packets_lost(const std::map<QUICPacketNumber, QUICSentPacketInfoUPtr> &packets)                              = 0;
-  virtual void add_extra_credit()                                                                                              = 0;
-  virtual void reset()                                                                                                         = 0;
-  virtual uint32_t credit() const                                                                                              = 0;
-  virtual uint32_t bytes_in_flight() const                                                                                     = 0;
+  // The function signature is different from the pseudo code because LD takes care of most of the processes
+  virtual void on_packet_number_space_discarded(size_t bytes_in_flight) = 0;
+  virtual void add_extra_credit()                                       = 0;
+  virtual void reset()                                                  = 0;
+  virtual uint32_t credit() const                                       = 0;
+  virtual uint32_t bytes_in_flight() const                              = 0;
 
   // Debug
   virtual uint32_t congestion_window() const = 0;

diff --git a/iocore/net/quic/QUICLossDetector.cc b/iocore/net/quic/QUICLossDetector.cc
@@ -163,6 +163,29 @@ QUICLossDetector::on_datagram_received()
   }
 }
 
+void
+QUICLossDetector::on_packet_number_space_discarded(QUICPacketNumberSpace pn_space)
+{
+  ink_assert(pn_space != QUICPacketNumberSpace::APPLICATION_DATA);
+  size_t bytes_in_flight = 0;
+  for (auto it = this->_sent_packets[static_cast<int>(pn_space)].begin();
+       it != this->_sent_packets[static_cast<int>(pn_space)].end();) {
+    auto ret = this->_remove_from_sent_packet_list(it, pn_space);
+    auto &pi = ret.first;
+    if (pi->in_flight) {
+      bytes_in_flight += pi->sent_bytes;
+    }
+    it = ret.second;
+  }
+  this->_cc->on_packet_number_space_discarded(bytes_in_flight);
+  // Reset the loss detection and PTO timer
+  this->_time_of_last_ack_eliciting_packet[static_cast<int>(pn_space)] = 0;
+  this->_loss_time[static_cast<int>(pn_space)]                         = 0;
+  this->_rtt_measure->set_pto_count(0);
+  this->_set_loss_detection_timer();
+  QUICLDDebug("[%s] Packets have been discarded because keys for the space are discarded", QUICDebugNames::pn_space(pn_space));
+}
+
 void
 QUICLossDetector::reset()
 {
@@ -257,7 +280,7 @@ QUICLossDetector::_on_ack_received(const QUICAckFrame &ack_frame, QUICPacketNumb
   }
   this->_cc->on_packets_acked(newly_acked_packets);
 
-  QUICLDDebug("[%s] Newly acked %lu Lost %lu Unacked packets %lu (%u ack eliciting)", QUICDebugNames::pn_space(pn_space),
+  QUICLDDebug("[%s] Newly acked:%lu Lost:%lu Unacked packets:%lu (%u ack eliciting)", QUICDebugNames::pn_space(pn_space),
               newly_acked_packets.size(), lost_packets.size(), this->_sent_packets[index].size(),
               this->_ack_eliciting_outstanding.load());
 
@@ -401,7 +424,7 @@ QUICLossDetector::_set_loss_detection_timer()
   // PTO Duration
   ink_hrtime timeout = this->_get_pto_time_and_space(pn_space);
   update_timer(timeout);
-  QUICLDDebug("[%s] PTO timeout will be set: %" PRId64 "ms", QUICDebugNames::pn_space(pn_space),
+  QUICLDDebug("[%s] PTO timeout has been set: %" PRId64 "ms", QUICDebugNames::pn_space(pn_space),
               (timeout - this->_time_of_last_ack_eliciting_packet[static_cast<int>(pn_space)]) / HRTIME_MSECOND);
 }
 

diff --git a/iocore/net/quic/QUICLossDetector.h b/iocore/net/quic/QUICLossDetector.h
@@ -59,6 +59,9 @@ class QUICLossDetector : public Continuation, public QUICFrameHandler
 
   void on_packet_sent(QUICSentPacketInfoUPtr packet_info, bool in_flight = true);
   void on_datagram_received();
+  // OnPacketNumberSpaceDiscarded is on Congestion Control section but having it here makes more sense because most processes are
+  // for LD.
+  void on_packet_number_space_discarded(QUICPacketNumberSpace pn_space);
   QUICPacketNumber largest_acked_packet_number(QUICPacketNumberSpace pn_space);
   void update_ack_delay_exponent(uint8_t ack_delay_exponent);
   void reset();

diff --git a/iocore/net/quic/QUICNewRenoCongestionController.cc b/iocore/net/quic/QUICNewRenoCongestionController.cc
@@ -184,6 +184,12 @@ QUICNewRenoCongestionController::on_packets_lost(const std::map<QUICPacketNumber
   }
 }
 
+void
+QUICNewRenoCongestionController::on_packet_number_space_discarded(size_t bytes_in_flight)
+{
+  this->_bytes_in_flight -= bytes_in_flight;
+}
+
 bool
 QUICNewRenoCongestionController::_check_credit() const
 {

diff --git a/iocore/net/quic/QUICNewRenoCongestionController.h b/iocore/net/quic/QUICNewRenoCongestionController.h
@@ -37,6 +37,7 @@ class QUICNewRenoCongestionController : public QUICCongestionController
   void on_packet_acked(const QUICSentPacketInfo &acked_packet) override;
   virtual void on_packets_acked(const std::vector<QUICSentPacketInfoUPtr> &packets) override;
   virtual void on_packets_lost(const std::map<QUICPacketNumber, QUICSentPacketInfoUPtr> &packets) override;
+  void on_packet_number_space_discarded(size_t bytes_in_flight) override;
   void process_ecn(const QUICAckFrame &ack, QUICPacketNumberSpace pn_space, ink_hrtime largest_acked_packet_time_sent) override;
   uint32_t credit() const override;
   void reset() override;