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 2017/08/23 00:58:14 UTC
[trafficserver] branch quic-latest updated: Cleanup FrameDispatcher
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 28dd682 Cleanup FrameDispatcher
28dd682 is described below
commit 28dd6829690dd5ee39c6bffbe9c92afd8fa594cc
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Wed Aug 23 09:57:34 2017 +0900
Cleanup FrameDispatcher
---
iocore/net/P_QUICNetVConnection.h | 15 ++--
iocore/net/QUICNetVConnection.cc | 29 ++++++--
iocore/net/quic/Mock.h | 6 ++
iocore/net/quic/QUICCongestionController.cc | 6 ++
iocore/net/quic/QUICCongestionController.h | 1 +
iocore/net/quic/QUICFlowController.cc | 6 ++
iocore/net/quic/QUICFlowController.h | 3 +-
iocore/net/quic/QUICFrameDispatcher.cc | 95 ++++--------------------
iocore/net/quic/QUICFrameDispatcher.h | 18 +----
iocore/net/quic/QUICFrameHandler.h | 3 +
iocore/net/quic/QUICLossDetector.cc | 6 ++
iocore/net/quic/QUICLossDetector.h | 1 +
iocore/net/quic/QUICStreamManager.cc | 8 +-
iocore/net/quic/QUICStreamManager.h | 5 +-
iocore/net/quic/test/Makefile.am | 6 --
iocore/net/quic/test/test_QUICFrameDispatcher.cc | 15 ++--
16 files changed, 98 insertions(+), 125 deletions(-)
diff --git a/iocore/net/P_QUICNetVConnection.h b/iocore/net/P_QUICNetVConnection.h
index 9c00741..d42c63d 100644
--- a/iocore/net/P_QUICNetVConnection.h
+++ b/iocore/net/P_QUICNetVConnection.h
@@ -179,6 +179,7 @@ public:
virtual void transmit_frame(std::unique_ptr<QUICFrame, QUICFrameDeleterFunc> frame) override;
// QUICConnection (QUICFrameHandler)
+ std::vector<QUICFrameType> interests() override;
void handle_frame(std::shared_ptr<const QUICFrame> frame) override;
private:
@@ -197,12 +198,14 @@ private:
// TODO: use custom allocator and make them std::unique_ptr or std::shared_ptr
// or make them just member variables.
- QUICVersionNegotiator *_version_negotiator = nullptr;
- QUICHandshake *_handshake_handler = nullptr;
- QUICCrypto *_crypto = nullptr;
- std::shared_ptr<QUICLossDetector> _loss_detector = nullptr;
- std::shared_ptr<QUICStreamManager> _stream_manager = nullptr;
- QUICFrameDispatcher *_frame_dispatcher = nullptr;
+ QUICVersionNegotiator *_version_negotiator = nullptr;
+ QUICHandshake *_handshake_handler = nullptr;
+ QUICCrypto *_crypto = nullptr;
+ QUICLossDetector *_loss_detector = nullptr;
+ QUICFrameDispatcher *_frame_dispatcher = nullptr;
+ QUICStreamManager *_stream_manager = nullptr;
+ QUICFlowController *_flow_controller = nullptr;
+ QUICCongestionController *_congestion_controller = nullptr;
Queue<QUICPacket> _packet_recv_queue;
Queue<QUICPacket> _packet_send_queue;
diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index d779a39..d841232 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -89,17 +89,21 @@ QUICNetVConnection::start(SSL_CTX *ssl_ctx)
{
this->_version_negotiator = new QUICVersionNegotiator(&this->_packet_factory, this);
this->_crypto = new QUICCrypto(ssl_ctx, this);
+ this->_frame_dispatcher = new QUICFrameDispatcher();
this->_packet_factory.set_crypto_module(this->_crypto);
- // FIXME Should these have to be shared_ptr?
- this->_loss_detector = std::make_shared<QUICLossDetector>(this);
- this->_stream_manager = std::make_shared<QUICStreamManager>();
+ // Create frame handlers
+ this->_stream_manager = new QUICStreamManager();
this->_stream_manager->init(this, &this->_application_map);
+ this->_flow_controller = new QUICFlowController();
+ this->_congestion_controller = new QUICCongestionController();
+ this->_loss_detector = new QUICLossDetector(this);
- std::shared_ptr<QUICFlowController> flowController = std::make_shared<QUICFlowController>();
- std::shared_ptr<QUICCongestionController> congestionController = std::make_shared<QUICCongestionController>();
- this->_frame_dispatcher =
- new QUICFrameDispatcher(this, this->_stream_manager, flowController, congestionController, this->_loss_detector);
+ this->_frame_dispatcher->add_handler(this);
+ this->_frame_dispatcher->add_handler(this->_stream_manager);
+ this->_frame_dispatcher->add_handler(this->_flow_controller);
+ this->_frame_dispatcher->add_handler(this->_congestion_controller);
+ this->_frame_dispatcher->add_handler(this->_loss_detector);
QUICConfig::scoped_config params;
@@ -153,8 +157,11 @@ QUICNetVConnection::free(EThread *t)
delete this->_version_negotiator;
delete this->_handshake_handler;
delete this->_crypto;
+ delete this->_loss_detector;
delete this->_frame_dispatcher;
- // XXX _loss_detector and _stream_manager are std::shared_ptr
+ delete this->_stream_manager;
+ delete this->_flow_controller;
+ delete this->_congestion_controller;
// TODO: clear member variables like `UnixNetVConnection::free(EThread *t)`
this->mutex.clear();
@@ -313,6 +320,12 @@ QUICNetVConnection::close(QUICError error)
}
}
+std::vector<QUICFrameType>
+QUICNetVConnection::interests()
+{
+ return {QUICFrameType::CONNECTION_CLOSE};
+}
+
void
QUICNetVConnection::handle_frame(std::shared_ptr<const QUICFrame> frame)
{
diff --git a/iocore/net/quic/Mock.h b/iocore/net/quic/Mock.h
index d208edb..7848e5b 100644
--- a/iocore/net/quic/Mock.h
+++ b/iocore/net/quic/Mock.h
@@ -121,6 +121,12 @@ public:
{
}
+ std::vector<QUICFrameType>
+ interests() override
+ {
+ return {QUICFrameType::CONNECTION_CLOSE};
+ }
+
void
handle_frame(std::shared_ptr<const QUICFrame> f) override
{
diff --git a/iocore/net/quic/QUICCongestionController.cc b/iocore/net/quic/QUICCongestionController.cc
index 2584c35..f10ef19 100644
--- a/iocore/net/quic/QUICCongestionController.cc
+++ b/iocore/net/quic/QUICCongestionController.cc
@@ -25,6 +25,12 @@
const static char *tag = "quic_congestion_controller";
+std::vector<QUICFrameType>
+QUICCongestionController::interests()
+{
+ return {QUICFrameType::ACK, QUICFrameType::STREAM};
+}
+
void
QUICCongestionController::handle_frame(std::shared_ptr<const QUICFrame> frame)
{
diff --git a/iocore/net/quic/QUICCongestionController.h b/iocore/net/quic/QUICCongestionController.h
index a579a48..9faab4c 100644
--- a/iocore/net/quic/QUICCongestionController.h
+++ b/iocore/net/quic/QUICCongestionController.h
@@ -30,6 +30,7 @@
class QUICCongestionController : public QUICFrameHandler
{
public:
+ virtual std::vector<QUICFrameType> interests() override;
virtual void handle_frame(std::shared_ptr<const QUICFrame>) override;
private:
diff --git a/iocore/net/quic/QUICFlowController.cc b/iocore/net/quic/QUICFlowController.cc
index 273715a..a160867 100644
--- a/iocore/net/quic/QUICFlowController.cc
+++ b/iocore/net/quic/QUICFlowController.cc
@@ -25,6 +25,12 @@
const static char *tag = "quic_flow_controller";
+std::vector<QUICFrameType>
+QUICFlowController::interests()
+{
+ return {QUICFrameType::MAX_DATA, QUICFrameType::MAX_STREAM_DATA, QUICFrameType::BLOCKED, QUICFrameType::STREAM};
+}
+
void
QUICFlowController::handle_frame(std::shared_ptr<const QUICFrame> frame)
{
diff --git a/iocore/net/quic/QUICFlowController.h b/iocore/net/quic/QUICFlowController.h
index 6009d2c..5ede6cd 100644
--- a/iocore/net/quic/QUICFlowController.h
+++ b/iocore/net/quic/QUICFlowController.h
@@ -27,9 +27,10 @@
// TODO Implement flow controll
// Flow controll will be required for the 2nd implementation draft
-class QUICFlowController : QUICFrameHandler
+class QUICFlowController : public QUICFrameHandler
{
public:
+ virtual std::vector<QUICFrameType> interests() override;
virtual void handle_frame(std::shared_ptr<const QUICFrame>) override;
private:
diff --git a/iocore/net/quic/QUICFrameDispatcher.cc b/iocore/net/quic/QUICFrameDispatcher.cc
index 724e04f..e5ba11e 100644
--- a/iocore/net/quic/QUICFrameDispatcher.cc
+++ b/iocore/net/quic/QUICFrameDispatcher.cc
@@ -34,16 +34,13 @@ const static char *tag = "quic_frame_handler";
//
// Frame Dispatcher
//
-QUICFrameDispatcher::QUICFrameDispatcher(QUICConnection *connection, const std::shared_ptr<QUICStreamManager> smgr,
- const std::shared_ptr<QUICFlowController> fctlr,
- const std::shared_ptr<QUICCongestionController> cctlr,
- const std::shared_ptr<QUICLossDetector> ld)
+
+void
+QUICFrameDispatcher::add_handler(QUICFrameHandler *handler)
{
- this->_connection = connection;
- streamManager = smgr;
- flowController = fctlr;
- congestionController = cctlr;
- lossDetector = ld;
+ for (QUICFrameType t : handler->interests()) {
+ this->_handlers[static_cast<uint8_t>(t)].push_back(handler);
+ }
}
bool
@@ -61,84 +58,18 @@ QUICFrameDispatcher::receive_frames(const uint8_t *payload, uint16_t size)
}
cursor += frame->size();
+ QUICFrameType type = frame->type();
+
// TODO: check debug build
- if (frame->type() != QUICFrameType::PADDING) {
+ if (type != QUICFrameType::PADDING) {
Debug(tag, "frame type %d, size %zu", static_cast<int>(frame->type()), frame->size());
}
- // FIXME We should probably use a mapping table. All the objects has the common interface (QUICFrameHandler).
- switch (frame->type()) {
- case QUICFrameType::PADDING: {
- // NOTE: do nothing
- break;
- }
- case QUICFrameType::RST_STREAM: {
- streamManager->handle_frame(frame);
- should_send_ack = true;
- break;
- }
- case QUICFrameType::CONNECTION_CLOSE: {
- this->_connection->handle_frame(frame);
- should_send_ack = true;
- break;
- }
- case QUICFrameType::GOAWAY: {
- should_send_ack = true;
- break;
- }
- case QUICFrameType::MAX_DATA: {
- flowController->handle_frame(frame);
- should_send_ack = true;
- break;
- }
- case QUICFrameType::MAX_STREAM_DATA: {
- flowController->handle_frame(frame);
- should_send_ack = true;
- break;
- }
- case QUICFrameType::MAX_STREAM_ID: {
- should_send_ack = true;
- break;
- }
- case QUICFrameType::PING: {
- should_send_ack = true;
- break;
- }
- case QUICFrameType::BLOCKED: {
- flowController->handle_frame(frame);
- should_send_ack = true;
- break;
- }
- case QUICFrameType::STREAM_BLOCKED: {
- should_send_ack = true;
- break;
- }
- case QUICFrameType::STREAM_ID_NEEDED: {
- should_send_ack = true;
- break;
- }
- case QUICFrameType::NEW_CONNECTION_ID: {
- should_send_ack = true;
- break;
- }
- case QUICFrameType::ACK: {
- congestionController->handle_frame(frame);
- this->lossDetector->handle_frame(frame);
- break;
- }
- case QUICFrameType::STREAM: {
- streamManager->handle_frame(frame);
- flowController->handle_frame(frame);
- congestionController->handle_frame(frame);
- should_send_ack = true;
- break;
- }
- default:
- // Unknown frame
- Debug(tag, "Unknown frame type: %02x", static_cast<unsigned int>(frame->type()));
- ink_assert(false);
+ should_send_ack |= (type != QUICFrameType::PADDING && type != QUICFrameType::ACK);
- break;
+ std::vector<QUICFrameHandler *> handlers = this->_handlers[static_cast<uint8_t>(type)];
+ for (auto h : handlers) {
+ h->handle_frame(frame);
}
}
return should_send_ack;
diff --git a/iocore/net/quic/QUICFrameDispatcher.h b/iocore/net/quic/QUICFrameDispatcher.h
index 2fd7f34..b186542 100644
--- a/iocore/net/quic/QUICFrameDispatcher.h
+++ b/iocore/net/quic/QUICFrameDispatcher.h
@@ -24,30 +24,20 @@
#pragma once
#include "QUICFrame.h"
-
-class QUICConnection;
-class QUICStreamManager;
-class QUICFlowController;
-class QUICCongestionController;
-class QUICLossDetector;
+#include "QUICFrameHandler.h"
+#include <vector>
class QUICFrameDispatcher
{
public:
- QUICFrameDispatcher(QUICConnection *connection, const std::shared_ptr<QUICStreamManager> smgr,
- const std::shared_ptr<QUICFlowController> fctlr, const std::shared_ptr<QUICCongestionController> cctlr,
- const std::shared_ptr<QUICLossDetector> ld);
/*
* Returns true if ACK frame should be sent
*/
bool receive_frames(const uint8_t *payload, uint16_t size);
- std::shared_ptr<QUICStreamManager> streamManager = nullptr;
- std::shared_ptr<QUICFlowController> flowController = nullptr;
- std::shared_ptr<QUICCongestionController> congestionController = nullptr;
- std::shared_ptr<QUICLossDetector> lossDetector = nullptr;
+ void add_handler(QUICFrameHandler *handler);
private:
- QUICConnection *_connection = nullptr;
QUICFrameFactory _frame_factory;
+ std::vector<QUICFrameHandler *> _handlers[256];
};
diff --git a/iocore/net/quic/QUICFrameHandler.h b/iocore/net/quic/QUICFrameHandler.h
index d4554fe..64b09d9 100644
--- a/iocore/net/quic/QUICFrameHandler.h
+++ b/iocore/net/quic/QUICFrameHandler.h
@@ -23,10 +23,13 @@
#pragma once
+#include <vector>
#include <QUICFrame.h>
class QUICFrameHandler
{
public:
+ virtual ~QUICFrameHandler(){};
+ virtual std::vector<QUICFrameType> interests() = 0;
virtual void handle_frame(std::shared_ptr<const QUICFrame> frame) = 0;
};
diff --git a/iocore/net/quic/QUICLossDetector.cc b/iocore/net/quic/QUICLossDetector.cc
index 4c10314..4c6bd34 100644
--- a/iocore/net/quic/QUICLossDetector.cc
+++ b/iocore/net/quic/QUICLossDetector.cc
@@ -72,6 +72,12 @@ QUICLossDetector::event_handler(int event, Event *edata)
return EVENT_CONT;
}
+std::vector<QUICFrameType>
+QUICLossDetector::interests()
+{
+ return {QUICFrameType::ACK};
+}
+
void
QUICLossDetector::handle_frame(std::shared_ptr<const QUICFrame> frame)
{
diff --git a/iocore/net/quic/QUICLossDetector.h b/iocore/net/quic/QUICLossDetector.h
index 7d6f325..0b93b46 100644
--- a/iocore/net/quic/QUICLossDetector.h
+++ b/iocore/net/quic/QUICLossDetector.h
@@ -45,6 +45,7 @@ public:
int event_handler(int event, Event *edata);
+ std::vector<QUICFrameType> interests() override;
virtual void handle_frame(std::shared_ptr<const QUICFrame>) override;
void on_packet_sent(std::unique_ptr<const QUICPacket> packet);
diff --git a/iocore/net/quic/QUICStreamManager.cc b/iocore/net/quic/QUICStreamManager.cc
index 19ef3e2..9b08fd2 100644
--- a/iocore/net/quic/QUICStreamManager.cc
+++ b/iocore/net/quic/QUICStreamManager.cc
@@ -33,11 +33,17 @@ ClassAllocator<QUICStream> quicStreamAllocator("quicStreamAllocator");
int
QUICStreamManager::init(QUICFrameTransmitter *tx, QUICApplicationMap *app_map)
{
- this->_tx = tx;
+ this->_tx = tx;
this->_app_map = app_map;
return 0;
}
+std::vector<QUICFrameType>
+QUICStreamManager::interests()
+{
+ return {QUICFrameType::STREAM, QUICFrameType::RST_STREAM};
+}
+
void
QUICStreamManager::handle_frame(std::shared_ptr<const QUICFrame> frame)
{
diff --git a/iocore/net/quic/QUICStreamManager.h b/iocore/net/quic/QUICStreamManager.h
index 08daacc..ae80bcd 100644
--- a/iocore/net/quic/QUICStreamManager.h
+++ b/iocore/net/quic/QUICStreamManager.h
@@ -36,6 +36,7 @@ public:
QUICStreamManager(){};
int init(QUICFrameTransmitter *tx, QUICApplicationMap *app_map);
+ virtual std::vector<QUICFrameType> interests() override;
virtual void handle_frame(std::shared_ptr<const QUICFrame>) override;
void send_frame(std::unique_ptr<QUICFrame, QUICFrameDeleterFunc> frame);
@@ -45,8 +46,8 @@ private:
QUICStream *_find_or_create_stream(QUICStreamId stream_id);
QUICStream *_find_stream(QUICStreamId id);
- QUICApplicationMap *_app_map = nullptr;
- QUICFrameTransmitter *_tx = nullptr;
+ QUICApplicationMap *_app_map = nullptr;
+ QUICFrameTransmitter *_tx = nullptr;
private:
void _handle_stream_frame(std::shared_ptr<const QUICStreamFrame>);
diff --git a/iocore/net/quic/test/Makefile.am b/iocore/net/quic/test/Makefile.am
index c09f670..d58b46e 100644
--- a/iocore/net/quic/test/Makefile.am
+++ b/iocore/net/quic/test/Makefile.am
@@ -166,9 +166,6 @@ test_QUICFrameDispatcher_SOURCES = \
test_QUICFrameDispatcher_LDADD = \
$(top_builddir)/lib/ts/libtsutil.la \
$(top_builddir)/iocore/eventsystem/libinkevent.a \
- $(top_builddir)/lib/records/librecords_p.a \
- $(top_builddir)/mgmt/libmgmt_p.la \
- $(top_builddir)/lib/ts/libtsutil.la \
$(top_builddir)/proxy/shared/libUglyLogStubs.a \
@LIBTCL@ \
@HWLOC_LIBS@
@@ -192,9 +189,6 @@ test_QUICStreamState_LDADD = \
$(top_builddir)/lib/ts/libtsutil.la \
$(top_builddir)/iocore/eventsystem/libinkevent.a \
$(top_builddir)/iocore/net/quic/libquic.a \
- $(top_builddir)/lib/records/librecords_p.a \
- $(top_builddir)/mgmt/libmgmt_p.la \
- $(top_builddir)/lib/ts/libtsutil.la \
$(top_builddir)/proxy/shared/libUglyLogStubs.a \
@LIBTCL@ \
@HWLOC_LIBS@
diff --git a/iocore/net/quic/test/test_QUICFrameDispatcher.cc b/iocore/net/quic/test/test_QUICFrameDispatcher.cc
index dfb6737..35c6058 100644
--- a/iocore/net/quic/test/test_QUICFrameDispatcher.cc
+++ b/iocore/net/quic/test/test_QUICFrameDispatcher.cc
@@ -33,11 +33,16 @@ TEST_CASE("QUICFrameHandler", "[quic]")
QUICStreamFrame streamFrame(payload, 1, 0x03, 0);
auto connection = new MockQUICConnection();
- auto streamManager = std::make_shared<MockQUICStreamManager>();
- auto flowController = std::make_shared<MockQUICFlowController>();
- auto congestionController = std::make_shared<MockQUICCongestionController>();
- auto lossDetector = std::make_shared<MockQUICLossDetector>();
- QUICFrameDispatcher quicFrameDispatcher(connection, streamManager, flowController, congestionController, lossDetector);
+ auto streamManager = new MockQUICStreamManager();
+ auto flowController = new MockQUICFlowController();
+ auto congestionController = new MockQUICCongestionController();
+ auto lossDetector = new MockQUICLossDetector();
+ QUICFrameDispatcher quicFrameDispatcher;
+ quicFrameDispatcher.add_handler(connection);
+ quicFrameDispatcher.add_handler(streamManager);
+ quicFrameDispatcher.add_handler(flowController);
+ quicFrameDispatcher.add_handler(congestionController);
+ quicFrameDispatcher.add_handler(lossDetector);
// Initial state
CHECK(connection->getTotalFrameCount() == 0);
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].