trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From masa...@apache.org
Subject [trafficserver] branch quic-latest updated: Return QUICError from QUICFrameHandler::handle_frame()
Date Wed, 23 Aug 2017 07:05:18 GMT
This is an automated email from the ASF dual-hosted git repository.

masaori 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 6d538d4  Return QUICError from QUICFrameHandler::handle_frame()
6d538d4 is described below

commit 6d538d4e4cb385325999150e67c0b1848aca6997
Author: Masaori Koshiba <masaori@apache.org>
AuthorDate: Wed Aug 23 16:05:01 2017 +0900

    Return QUICError from QUICFrameHandler::handle_frame()
---
 iocore/net/P_QUICNetVConnection.h                |  4 +-
 iocore/net/QUICNetVConnection.cc                 | 65 ++++++++++++++----------
 iocore/net/quic/Mock.h                           | 12 +++--
 iocore/net/quic/QUICCongestionController.cc      |  6 ++-
 iocore/net/quic/QUICCongestionController.h       |  2 +-
 iocore/net/quic/QUICFrameDispatcher.cc           | 18 ++++---
 iocore/net/quic/QUICFrameDispatcher.h            |  2 +-
 iocore/net/quic/QUICFrameHandler.h               |  4 +-
 iocore/net/quic/QUICLossDetector.cc              |  6 ++-
 iocore/net/quic/QUICLossDetector.h               |  2 +-
 iocore/net/quic/QUICStreamManager.cc             |  6 +--
 iocore/net/quic/QUICStreamManager.h              |  2 +-
 iocore/net/quic/test/test_QUICFrameDispatcher.cc |  5 +-
 13 files changed, 83 insertions(+), 51 deletions(-)

diff --git a/iocore/net/P_QUICNetVConnection.h b/iocore/net/P_QUICNetVConnection.h
index a2e6cf3..6d70c00 100644
--- a/iocore/net/P_QUICNetVConnection.h
+++ b/iocore/net/P_QUICNetVConnection.h
@@ -185,7 +185,7 @@ public:
 
   // QUICConnection (QUICFrameHandler)
   std::vector<QUICFrameType> interests() override;
-  void handle_frame(std::shared_ptr<const QUICFrame> frame) override;
+  QUICError handle_frame(std::shared_ptr<const QUICFrame> frame) override;
 
 private:
   QUICConnectionId _quic_connection_id;
@@ -221,7 +221,7 @@ private:
   std::unique_ptr<QUICPacket> _build_packet(ats_unique_buf buf, size_t len, bool retransmittable,
                                             QUICPacketType type = QUICPacketType::UNINITIALIZED);
 
-  void _recv_and_ack(const uint8_t *payload, uint16_t size, QUICPacketNumber packet_num);
+  QUICError _recv_and_ack(const uint8_t *payload, uint16_t size, QUICPacketNumber packet_numm);
 
   QUICError _state_handshake_process_initial_client_packet(std::unique_ptr<const QUICPacket>
packet);
   QUICError _state_handshake_process_client_cleartext_packet(std::unique_ptr<const QUICPacket>
packet);
diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index 1d44f42..f6343c3 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -322,9 +322,11 @@ QUICNetVConnection::interests()
   return {QUICFrameType::CONNECTION_CLOSE};
 }
 
-void
+QUICError
 QUICNetVConnection::handle_frame(std::shared_ptr<const QUICFrame> frame)
 {
+  QUICError error = QUICError(QUICErrorClass::NONE);
+
   switch (frame->type()) {
   case QUICFrameType::CONNECTION_CLOSE:
     DebugQUICCon("Enter state_connection_closed");
@@ -335,6 +337,8 @@ QUICNetVConnection::handle_frame(std::shared_ptr<const QUICFrame>
frame)
     ink_assert(false);
     break;
   }
+
+  return error;
 }
 
 // TODO: Timeout by active_timeout / inactive_timeout
@@ -542,26 +546,30 @@ QUICNetVConnection::_state_handshake_process_initial_client_packet(std::unique_p
     if (packet->type() != QUICPacketType::CLIENT_INITIAL) {
       return QUICError(QUICErrorClass::QUIC_TRANSPORT, QUICErrorCode::QUIC_INTERNAL_ERROR);
     }
-    if (packet->version()) {
-      if (this->_version_negotiator->negotiate(packet.get()) == QUICVersionNegotiationStatus::NEGOTIATED)
{
-        DebugQUICCon("Version negotiation succeeded: %x", packet->version());
-        this->_packet_factory.set_version(packet->version());
-        // Check integrity (QUIC-TLS-04: 6.1. Integrity Check Processing)
-        if (packet->has_valid_fnv1a_hash()) {
-          // Version 0x00000001 uses stream 0 for cryptographic handshake with TLS 1.3, but
newer version may not
-          this->_handshake_handler = new QUICHandshake(this, this->_crypto);
-          this->_application_map.set(STREAM_ID_FOR_HANDSHAKE, this->_handshake_handler);
-
-          this->_frame_dispatcher->receive_frames(packet->payload(), packet->payload_size());
-        } else {
-          DebugQUICCon("Invalid FNV-1a hash value");
-        }
-      } else {
-        DebugQUICCon("Version negotiation failed: %x", packet->version());
-      }
-    } else {
+
+    if (!packet->version()) {
       return QUICError(QUICErrorClass::QUIC_TRANSPORT, QUICErrorCode::QUIC_INTERNAL_ERROR);
     }
+
+    if (this->_version_negotiator->negotiate(packet.get()) != QUICVersionNegotiationStatus::NEGOTIATED)
{
+      DebugQUICCon("Version negotiation failed: %x", packet->version());
+      return QUICError(QUICErrorClass::QUIC_TRANSPORT, QUICErrorCode::QUIC_VERSION_NEGOTIATION_MISMATCH);
+    }
+
+    DebugQUICCon("Version negotiation succeeded: %x", packet->version());
+    this->_packet_factory.set_version(packet->version());
+    // Check integrity (QUIC-TLS-04: 6.1. Integrity Check Processing)
+    if (!packet->has_valid_fnv1a_hash()) {
+      DebugQUICCon("Invalid FNV-1a hash value");
+      return QUICError(QUICErrorClass::QUIC_TRANSPORT, QUICErrorCode::CRYPTOGRAPHIC_ERROR);
+    }
+
+    // Version 0x00000001 uses stream 0 for cryptographic handshake with TLS 1.3, but newer
version may not
+    this->_handshake_handler = new QUICHandshake(this, this->_crypto);
+    this->_application_map.set(STREAM_ID_FOR_HANDSHAKE, this->_handshake_handler);
+    bool should_send_ack;
+
+    return this->_frame_dispatcher->receive_frames(packet->payload(), packet->payload_size(),
should_send_ack);
   }
 
   return QUICError(QUICErrorClass::NONE);
@@ -570,13 +578,16 @@ QUICNetVConnection::_state_handshake_process_initial_client_packet(std::unique_p
 QUICError
 QUICNetVConnection::_state_handshake_process_client_cleartext_packet(std::unique_ptr<const
QUICPacket> packet)
 {
+  QUICError error = QUICError(QUICErrorClass::NONE);
+
   // The payload of this packet contains STREAM frames and could contain PADDING and ACK
frames
   if (packet->has_valid_fnv1a_hash()) {
-    this->_recv_and_ack(packet->payload(), packet->payload_size(), packet->packet_number());
+    error = this->_recv_and_ack(packet->payload(), packet->payload_size(), packet->packet_number());
   } else {
     DebugQUICCon("Invalid FNV-1a hash value");
+    return QUICError(QUICErrorClass::QUIC_TRANSPORT, QUICErrorCode::CRYPTOGRAPHIC_ERROR);
   }
-  return QUICError(QUICErrorClass::NONE);
+  return error;
 }
 
 QUICError
@@ -601,9 +612,7 @@ QUICNetVConnection::_state_connection_established_process_packet(std::unique_ptr
     DebugQUICCon("Decrypt Packet, pkt_num: %" PRIu64 ", header_len: %hu, payload_len: %zu",
packet->packet_number(),
                  packet->header_size(), plain_txt_len);
 
-    this->_recv_and_ack(plain_txt.get(), plain_txt_len, packet->packet_number());
-
-    return QUICError(QUICErrorClass::NONE);
+    return this->_recv_and_ack(plain_txt.get(), plain_txt_len, packet->packet_number());
   } else {
     DebugQUICCon("CRYPTOGRAPHIC Error");
 
@@ -699,16 +708,20 @@ QUICNetVConnection::_packetize_frames()
   }
 }
 
-void
+QUICError
 QUICNetVConnection::_recv_and_ack(const uint8_t *payload, uint16_t size, QUICPacketNumber
packet_num)
 {
-  bool should_send_ack = this->_frame_dispatcher->receive_frames(payload, size);
+  bool should_send_ack;
+  QUICError error = this->_frame_dispatcher->receive_frames(payload, size, should_send_ack);
+
   this->_ack_frame_creator.update(packet_num, should_send_ack);
   std::unique_ptr<QUICFrame, QUICFrameDeleterFunc> ack_frame = this->_ack_frame_creator.create_if_needed();
   if (ack_frame != nullptr) {
     this->transmit_frame(std::move(ack_frame));
     eventProcessor.schedule_imm(this, ET_CALL, QUIC_EVENT_PACKET_WRITE_READY, nullptr);
   }
+
+  return error;
 }
 
 std::unique_ptr<QUICPacket>
diff --git a/iocore/net/quic/Mock.h b/iocore/net/quic/Mock.h
index b697954..6e72a74 100644
--- a/iocore/net/quic/Mock.h
+++ b/iocore/net/quic/Mock.h
@@ -126,11 +126,13 @@ public:
     return {QUICFrameType::CONNECTION_CLOSE};
   }
 
-  void
+  QUICError
   handle_frame(std::shared_ptr<const QUICFrame> f) override
   {
     ++_frameCount[static_cast<int>(f->type())];
     ++_totalFrameCount;
+
+    return QUICError(QUICErrorClass::NONE);
   }
 
   uint32_t
@@ -258,11 +260,13 @@ public:
   MockQUICStreamManager() : QUICStreamManager() {}
 
   // Override
-  virtual void
+  virtual QUICError
   handle_frame(std::shared_ptr<const QUICFrame> f) override
   {
     ++_frameCount[static_cast<int>(f->type())];
     ++_totalFrameCount;
+
+    return QUICError(QUICErrorClass::NONE);
   }
 
   // for Test
@@ -305,11 +309,13 @@ public:
   MockQUICCongestionController() : QUICCongestionController() {}
 
   // Override
-  virtual void
+  virtual QUICError
   handle_frame(std::shared_ptr<const QUICFrame> f) override
   {
     ++_frameCount[static_cast<int>(f->type())];
     ++_totalFrameCount;
+
+    return QUICError(QUICErrorClass::NONE);
   }
 
   // for Test
diff --git a/iocore/net/quic/QUICCongestionController.cc b/iocore/net/quic/QUICCongestionController.cc
index f10ef19..6da901c 100644
--- a/iocore/net/quic/QUICCongestionController.cc
+++ b/iocore/net/quic/QUICCongestionController.cc
@@ -31,9 +31,11 @@ QUICCongestionController::interests()
   return {QUICFrameType::ACK, QUICFrameType::STREAM};
 }
 
-void
+QUICError
 QUICCongestionController::handle_frame(std::shared_ptr<const QUICFrame> frame)
 {
+  QUICError error = QUICError(QUICErrorClass::NONE);
+
   switch (frame->type()) {
   case QUICFrameType::STREAM:
   case QUICFrameType::ACK:
@@ -43,4 +45,6 @@ QUICCongestionController::handle_frame(std::shared_ptr<const QUICFrame>
frame)
     ink_assert(false);
     break;
   }
+
+  return error;
 }
diff --git a/iocore/net/quic/QUICCongestionController.h b/iocore/net/quic/QUICCongestionController.h
index 9faab4c..cf8191b 100644
--- a/iocore/net/quic/QUICCongestionController.h
+++ b/iocore/net/quic/QUICCongestionController.h
@@ -31,7 +31,7 @@ class QUICCongestionController : public QUICFrameHandler
 {
 public:
   virtual std::vector<QUICFrameType> interests() override;
-  virtual void handle_frame(std::shared_ptr<const QUICFrame>) override;
+  virtual QUICError handle_frame(std::shared_ptr<const QUICFrame>) override;
 
 private:
 };
diff --git a/iocore/net/quic/QUICFrameDispatcher.cc b/iocore/net/quic/QUICFrameDispatcher.cc
index 0799f22..3ac3f27 100644
--- a/iocore/net/quic/QUICFrameDispatcher.cc
+++ b/iocore/net/quic/QUICFrameDispatcher.cc
@@ -42,12 +42,13 @@ QUICFrameDispatcher::add_handler(QUICFrameHandler *handler)
   }
 }
 
-bool
-QUICFrameDispatcher::receive_frames(const uint8_t *payload, uint16_t size)
+QUICError
+QUICFrameDispatcher::receive_frames(const uint8_t *payload, uint16_t size, bool &should_send_ack)
 {
   std::shared_ptr<const QUICFrame> frame(nullptr);
-  uint16_t cursor      = 0;
-  bool should_send_ack = false;
+  uint16_t cursor = 0;
+  should_send_ack = false;
+  QUICError error = QUICError(QUICErrorClass::NONE);
 
   while (cursor < size) {
     frame = this->_frame_factory.fast_create(payload + cursor, size - cursor);
@@ -68,8 +69,13 @@ QUICFrameDispatcher::receive_frames(const uint8_t *payload, uint16_t size)
 
     std::vector<QUICFrameHandler *> handlers = this->_handlers[static_cast<uint8_t>(type)];
     for (auto h : handlers) {
-      h->handle_frame(frame);
+      error = h->handle_frame(frame);
+      // TODO: is there any case to continue this loop even if error?
+      if (error.cls != QUICErrorClass::NONE) {
+        return error;
+      }
     }
   }
-  return should_send_ack;
+
+  return error;
 }
diff --git a/iocore/net/quic/QUICFrameDispatcher.h b/iocore/net/quic/QUICFrameDispatcher.h
index b186542..0c61f39 100644
--- a/iocore/net/quic/QUICFrameDispatcher.h
+++ b/iocore/net/quic/QUICFrameDispatcher.h
@@ -33,7 +33,7 @@ public:
   /*
    * Returns true if ACK frame should be sent
    */
-  bool receive_frames(const uint8_t *payload, uint16_t size);
+  QUICError receive_frames(const uint8_t *payload, uint16_t size, bool &should_send_ack);
 
   void add_handler(QUICFrameHandler *handler);
 
diff --git a/iocore/net/quic/QUICFrameHandler.h b/iocore/net/quic/QUICFrameHandler.h
index 64b09d9..0f2a27f 100644
--- a/iocore/net/quic/QUICFrameHandler.h
+++ b/iocore/net/quic/QUICFrameHandler.h
@@ -30,6 +30,6 @@ class QUICFrameHandler
 {
 public:
   virtual ~QUICFrameHandler(){};
-  virtual std::vector<QUICFrameType> interests()                    = 0;
-  virtual void handle_frame(std::shared_ptr<const QUICFrame> frame) = 0;
+  virtual std::vector<QUICFrameType> interests()                         = 0;
+  virtual QUICError handle_frame(std::shared_ptr<const QUICFrame> frame) = 0;
 };
diff --git a/iocore/net/quic/QUICLossDetector.cc b/iocore/net/quic/QUICLossDetector.cc
index 4c6bd34..12a7368 100644
--- a/iocore/net/quic/QUICLossDetector.cc
+++ b/iocore/net/quic/QUICLossDetector.cc
@@ -78,9 +78,11 @@ QUICLossDetector::interests()
   return {QUICFrameType::ACK};
 }
 
-void
+QUICError
 QUICLossDetector::handle_frame(std::shared_ptr<const QUICFrame> frame)
 {
+  QUICError error = QUICError(QUICErrorClass::NONE);
+
   switch (frame->type()) {
   case QUICFrameType::ACK:
     this->_on_ack_received(std::dynamic_pointer_cast<const QUICAckFrame>(frame));
@@ -90,6 +92,8 @@ QUICLossDetector::handle_frame(std::shared_ptr<const QUICFrame> frame)
     ink_assert(false);
     break;
   }
+
+  return error;
 }
 
 void
diff --git a/iocore/net/quic/QUICLossDetector.h b/iocore/net/quic/QUICLossDetector.h
index 0b93b46..56d9650 100644
--- a/iocore/net/quic/QUICLossDetector.h
+++ b/iocore/net/quic/QUICLossDetector.h
@@ -46,7 +46,7 @@ public:
   int event_handler(int event, Event *edata);
 
   std::vector<QUICFrameType> interests() override;
-  virtual void handle_frame(std::shared_ptr<const QUICFrame>) override;
+  virtual QUICError handle_frame(std::shared_ptr<const QUICFrame>) override;
   void on_packet_sent(std::unique_ptr<const QUICPacket> packet);
 
 private:
diff --git a/iocore/net/quic/QUICStreamManager.cc b/iocore/net/quic/QUICStreamManager.cc
index fab8dea..6015f39 100644
--- a/iocore/net/quic/QUICStreamManager.cc
+++ b/iocore/net/quic/QUICStreamManager.cc
@@ -61,7 +61,7 @@ QUICStreamManager::init_flow_control_params(const QUICTransportParameters
&local
   stream->init_flow_control_params(local_tp.initial_max_stream_data(), remote_tp.initial_max_stream_data());
 }
 
-void
+QUICError
 QUICStreamManager::handle_frame(std::shared_ptr<const QUICFrame> frame)
 {
   QUICError error = QUICError(QUICErrorClass::NONE);
@@ -92,9 +92,7 @@ QUICStreamManager::handle_frame(std::shared_ptr<const QUICFrame> frame)
     break;
   }
 
-  if (error.cls != QUICErrorClass::NONE) {
-    // TODO return error
-  }
+  return error;
 }
 
 QUICError
diff --git a/iocore/net/quic/QUICStreamManager.h b/iocore/net/quic/QUICStreamManager.h
index 6df6db3..4abea4f 100644
--- a/iocore/net/quic/QUICStreamManager.h
+++ b/iocore/net/quic/QUICStreamManager.h
@@ -39,7 +39,7 @@ public:
 
   int init(QUICFrameTransmitter *tx, QUICConnection *qc, QUICApplicationMap *app_map);
   virtual std::vector<QUICFrameType> interests() override;
-  virtual void handle_frame(std::shared_ptr<const QUICFrame>) override;
+  virtual QUICError handle_frame(std::shared_ptr<const QUICFrame>) override;
   virtual void send_frame(std::unique_ptr<QUICFrame, QUICFrameDeleterFunc> frame);
   void send_stream_frame(std::unique_ptr<QUICStreamFrame, QUICFrameDeleterFunc> frame);
   virtual bool is_send_avail_more_than(uint64_t size);
diff --git a/iocore/net/quic/test/test_QUICFrameDispatcher.cc b/iocore/net/quic/test/test_QUICFrameDispatcher.cc
index 5b15819..8573dce 100644
--- a/iocore/net/quic/test/test_QUICFrameDispatcher.cc
+++ b/iocore/net/quic/test/test_QUICFrameDispatcher.cc
@@ -51,7 +51,8 @@ TEST_CASE("QUICFrameHandler", "[quic]")
   uint8_t buf[4096] = {0};
   size_t len        = 0;
   streamFrame.store(buf, &len);
-  quicFrameDispatcher.receive_frames(buf, len);
+  bool should_send_ack;
+  quicFrameDispatcher.receive_frames(buf, len, should_send_ack);
   CHECK(connection->getTotalFrameCount() == 0);
   CHECK(streamManager->getTotalFrameCount() == 1);
   CHECK(congestionController->getTotalFrameCount() == 1);
@@ -59,7 +60,7 @@ TEST_CASE("QUICFrameHandler", "[quic]")
   // CONNECTION_CLOSE frame
   QUICConnectionCloseFrame connectionCloseFrame({});
   connectionCloseFrame.store(buf, &len);
-  quicFrameDispatcher.receive_frames(buf, len);
+  quicFrameDispatcher.receive_frames(buf, len, should_send_ack);
   CHECK(connection->getTotalFrameCount() == 1);
   CHECK(streamManager->getTotalFrameCount() == 1);
   CHECK(congestionController->getTotalFrameCount() == 1);

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Mime
View raw message