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/04/11 05:54:13 UTC

[trafficserver] 01/02: Fix flow control on 0-RTT scenario

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

commit 49c465c225c5d8a74e1a81b49f980d4bb021b171
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Wed Apr 11 12:18:02 2018 +0900

    Fix flow control on 0-RTT scenario
---
 iocore/net/QUICNetVConnection.cc      | 10 ++++++----
 iocore/net/quic/QUICFlowController.cc | 24 ++++++++++--------------
 iocore/net/quic/QUICFlowController.h  | 10 +++++++++-
 iocore/net/quic/QUICStream.cc         |  8 ++++----
 iocore/net/quic/QUICStream.h          |  2 +-
 iocore/net/quic/QUICStreamManager.cc  | 19 +++++++------------
 6 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index cf30f76..fcf9ca1 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -202,8 +202,8 @@ QUICNetVConnection::start()
   this->_stream_manager         = new QUICStreamManager(this->connection_id(), this->_application_map);
   this->_congestion_controller  = new QUICCongestionController(this->connection_id());
   this->_loss_detector          = new QUICLossDetector(this, this->_congestion_controller);
-  this->_remote_flow_controller = new QUICRemoteConnectionFlowController(0);
-  this->_local_flow_controller  = new QUICLocalConnectionFlowController(0);
+  this->_remote_flow_controller = new QUICRemoteConnectionFlowController(UINT64_MAX);
+  this->_local_flow_controller  = new QUICLocalConnectionFlowController(UINT64_MAX);
   this->_path_validator         = new QUICPathValidator();
 
   this->_frame_dispatcher->add_handler(this);
@@ -897,6 +897,8 @@ QUICNetVConnection::_state_handshake_process_handshake_packet(QUICPacketUPtr pac
 QUICErrorUPtr
 QUICNetVConnection::_state_handshake_process_zero_rtt_protected_packet(QUICPacketUPtr packet)
 {
+  this->_stream_manager->init_flow_control_params(this->_handshake_handler->local_transport_parameters(),
+                                                  this->_handshake_handler->remote_transport_parameters());
   this->_start_application();
   return this->_recv_and_ack(std::move(packet));
 }
@@ -1365,8 +1367,8 @@ QUICNetVConnection::_init_flow_control_params(const std::shared_ptr<const QUICTr
     remote_initial_max_data = remote_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_DATA);
   }
 
-  this->_local_flow_controller->forward_limit(local_initial_max_data * 1024);
-  this->_remote_flow_controller->forward_limit(remote_initial_max_data * 1024);
+  this->_local_flow_controller->set_limit(local_initial_max_data);
+  this->_remote_flow_controller->set_limit(remote_initial_max_data);
   Debug("quic_flow_ctrl", "Connection [%" PRIx64 "] [LOCAL] %" PRIu64 "/%" PRIu64, static_cast<uint64_t>(this->_quic_connection_id),
         this->_local_flow_controller->current_offset(), this->_local_flow_controller->current_limit());
   Debug("quic_flow_ctrl", "Connection [%" PRIx64 "] [REMOTE] %" PRIu64 "/%" PRIu64,
diff --git a/iocore/net/quic/QUICFlowController.cc b/iocore/net/quic/QUICFlowController.cc
index 67a2898..13cccc3 100644
--- a/iocore/net/quic/QUICFlowController.cc
+++ b/iocore/net/quic/QUICFlowController.cc
@@ -27,7 +27,7 @@
 //
 // QUICFlowController
 //
-uint32_t
+uint64_t
 QUICFlowController::credit()
 {
   return this->current_limit() - this->current_offset();
@@ -42,20 +42,14 @@ QUICFlowController::current_offset()
 QUICOffset
 QUICFlowController::current_limit()
 {
-  // if _limit is 0, the limit is not set yet.
-  if (this->_limit) {
-    return this->_limit;
-  } else {
-    return UINT64_MAX;
-  }
+  return this->_limit;
 }
 
 int
 QUICFlowController::update(QUICOffset offset)
 {
   if (this->_offset <= offset) {
-    // Assume flow control is not initialized if the limit was 0
-    if (this->_limit != 0 && offset > this->_limit) {
+    if (offset > this->_limit) {
       return -1;
     }
     this->_offset = offset;
@@ -81,6 +75,13 @@ QUICFlowController::set_threshold(uint64_t threshold)
   this->_threshold = threshold;
 }
 
+void
+QUICFlowController::set_limit(QUICOffset limit)
+{
+  ink_assert(this->_limit == UINT64_MAX || this->_limit == limit);
+  this->_limit = limit;
+}
+
 QUICFrameUPtr
 QUICFlowController::generate_frame()
 {
@@ -107,11 +108,6 @@ QUICRemoteFlowController::update(QUICOffset offset)
 {
   int ret = QUICFlowController::update(offset);
 
-  // Assume flow control is not initialized if the limit was 0
-  if (this->_limit == 0) {
-    return ret;
-  }
-
   // Send BLOCKED(_STREAM) frame
   if (!this->_blocked && offset > this->_limit) {
     this->_frame   = this->_create_frame();
diff --git a/iocore/net/quic/QUICFlowController.h b/iocore/net/quic/QUICFlowController.h
index 930d9f0..0510113 100644
--- a/iocore/net/quic/QUICFlowController.h
+++ b/iocore/net/quic/QUICFlowController.h
@@ -29,7 +29,7 @@
 class QUICFlowController
 {
 public:
-  uint32_t credit();
+  uint64_t credit();
   QUICOffset current_offset();
   QUICOffset current_limit();
 
@@ -38,7 +38,15 @@ public:
    */
   virtual int update(QUICOffset offset);
   virtual void forward_limit(QUICOffset limit);
+
   void set_threshold(uint64_t threshold);
+
+  /**
+   * This is only for flow controllers initialized without a limit (== UINT64_MAX).
+   * Once a limit is set, it should be updated with forward_limit().
+   */
+  void set_limit(QUICOffset limit);
+
   QUICFrameUPtr generate_frame();
 
 protected:
diff --git a/iocore/net/quic/QUICStream.cc b/iocore/net/quic/QUICStream.cc
index dff5737..81525c8 100644
--- a/iocore/net/quic/QUICStream.cc
+++ b/iocore/net/quic/QUICStream.cc
@@ -63,7 +63,7 @@ QUICStream::~QUICStream()
 }
 
 void
-QUICStream::init_flow_control_params(uint32_t recv_max_stream_data, uint32_t send_max_stream_data)
+QUICStream::init_flow_control_params(uint64_t recv_max_stream_data, uint64_t send_max_stream_data)
 {
   this->_flow_control_buffer_size = recv_max_stream_data;
   this->_local_flow_controller.forward_limit(recv_max_stream_data);
@@ -400,8 +400,8 @@ QUICStream::generate_frame(uint16_t connection_credit, uint16_t maximum_frame_si
   bool fin         = false;
 
   len = std::min(data_len, static_cast<int64_t>(
-                             std::min(static_cast<uint32_t>(maximum_frame_size),
-                                      std::min(this->_remote_flow_controller.credit(), static_cast<uint32_t>(connection_credit)))));
+                             std::min(static_cast<uint64_t>(maximum_frame_size),
+                                      std::min(this->_remote_flow_controller.credit(), static_cast<uint64_t>(connection_credit)))));
   if (len >= bytes_avail) {
     fin = this->_fin;
   }
@@ -429,7 +429,7 @@ QUICStream::generate_frame(uint16_t connection_credit, uint16_t maximum_frame_si
     this->_write_vio.ndone += len;
     this->_signal_write_event();
     this->_state.update_with_sent_frame(*frame);
-  } else {
+  } else if (ret != 0) {
     QUICStreamDebug("Flow Controller blocked sending a STREAM frame");
     frame = this->_remote_flow_controller.generate_frame();
   }
diff --git a/iocore/net/quic/QUICStream.h b/iocore/net/quic/QUICStream.h
index a5d7639..5899e24 100644
--- a/iocore/net/quic/QUICStream.h
+++ b/iocore/net/quic/QUICStream.h
@@ -55,7 +55,7 @@ public:
   int state_stream_open(int event, void *data);
   int state_stream_closed(int event, void *data);
 
-  void init_flow_control_params(uint32_t recv_max_stream_data, uint32_t send_max_stream_data);
+  void init_flow_control_params(uint64_t recv_max_stream_data, uint64_t send_max_stream_data);
 
   QUICStreamId id() const;
   QUICOffset final_offset();
diff --git a/iocore/net/quic/QUICStreamManager.cc b/iocore/net/quic/QUICStreamManager.cc
index 6fbf258..3725d1c 100644
--- a/iocore/net/quic/QUICStreamManager.cc
+++ b/iocore/net/quic/QUICStreamManager.cc
@@ -55,15 +55,7 @@ QUICStreamManager::init_flow_control_params(const std::shared_ptr<const QUICTran
   // Setup a stream for Handshake
   QUICStream *stream = this->_find_stream(STREAM_ID_FOR_HANDSHAKE);
   if (stream) {
-    uint32_t local_initial_max_stream_data  = 0;
-    uint32_t remote_initial_max_stream_data = 0;
-    if (this->_local_tp) {
-      local_initial_max_stream_data = local_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA);
-    }
-    if (this->_remote_tp) {
-      remote_initial_max_stream_data = remote_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA);
-    }
-    stream->init_flow_control_params(local_initial_max_stream_data, remote_initial_max_stream_data);
+    stream->init_flow_control_params(UINT64_MAX, UINT64_MAX);
   }
 
   if (this->_local_tp) {
@@ -249,9 +241,12 @@ QUICStreamManager::_find_or_create_stream(QUICStreamId stream_id)
       return nullptr;
     }
 
-    uint32_t local_max_stream_data  = 0;
-    uint32_t remote_max_stream_data = 0;
-    if (this->_local_tp) {
+    uint64_t local_max_stream_data  = 0;
+    uint64_t remote_max_stream_data = 0;
+    if (stream_id == STREAM_ID_FOR_HANDSHAKE) {
+      local_max_stream_data  = UINT64_MAX;
+      remote_max_stream_data = UINT64_MAX;
+    } else if (this->_local_tp) {
       local_max_stream_data  = this->_local_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA),
       remote_max_stream_data = this->_remote_tp->getAsUInt32(QUICTransportParameterId::INITIAL_MAX_STREAM_DATA);
     } else {

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