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 2019/02/07 02:25:08 UTC
[trafficserver] branch quic-latest updated: Use QUICFrameGenerators
interface instead of using individual classes
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 6a057e7 Use QUICFrameGenerators interface instead of using individual classes
6a057e7 is described below
commit 6a057e7d813ce0d2e17dc40abf5d3395f9faa7e7
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Wed Feb 6 16:38:51 2019 +0900
Use QUICFrameGenerators interface instead of using individual classes
---
iocore/net/P_QUICNetVConnection.h | 2 +
iocore/net/QUICNetVConnection.cc | 160 ++++++++++++-----------------------
iocore/net/quic/QUICFrame.cc | 12 +++
iocore/net/quic/QUICFrame.h | 2 +
iocore/net/quic/QUICHandshake.cc | 3 +
iocore/net/quic/QUICPathValidator.cc | 3 +
iocore/net/quic/QUICPinger.cc | 16 ++--
iocore/net/quic/QUICPinger.h | 5 +-
8 files changed, 88 insertions(+), 115 deletions(-)
diff --git a/iocore/net/P_QUICNetVConnection.h b/iocore/net/P_QUICNetVConnection.h
index 13bd7b3..6aac61b 100644
--- a/iocore/net/P_QUICNetVConnection.h
+++ b/iocore/net/P_QUICNetVConnection.h
@@ -276,6 +276,8 @@ private:
QUICAltConnectionManager *_alt_con_manager = nullptr;
QUICPathValidator *_path_validator = nullptr;
+ std::vector<QUICFrameGenerator *> _frame_generators;
+
QUICPacketReceiveQueue _packet_recv_queue = {this->_packet_factory, this->_ph_protector};
QUICConnectionErrorUPtr _connection_error = nullptr;
diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index 7ef01c5..d5d940d 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -282,6 +282,17 @@ QUICNetVConnection::start()
this->_path_validator = new QUICPathValidator();
this->_stream_manager = new QUICStreamManager(this, &this->_rtt_measure, this->_application_map);
+ // Register frame generators
+ this->_frame_generators.push_back(this->_handshake_handler); // CRYPTO
+ this->_frame_generators.push_back(this->_path_validator); // PATH_CHALLENGE, PATH_RESPOSNE
+ this->_frame_generators.push_back(this->_local_flow_controller); // MAX_DATA
+ this->_frame_generators.push_back(this->_remote_flow_controller); // DATA_BLOCKED
+ this->_frame_generators.push_back(this); // NEW_TOKEN
+ this->_frame_generators.push_back(this->_stream_manager); // STREAM, MAX_STREAM_DATA, STREAM_DATA_BLOCKED
+ this->_frame_generators.push_back(&this->_ack_frame_manager); // ACK
+ this->_frame_generators.push_back(&this->_pinger); // PING
+
+ // Register frame handlers
this->_frame_dispatcher->add_handler(this);
this->_frame_dispatcher->add_handler(this->_stream_manager);
this->_frame_dispatcher->add_handler(this->_path_validator);
@@ -492,7 +503,7 @@ QUICNetVConnection::handle_received_packet(UDPPacket *packet)
void
QUICNetVConnection::ping()
{
- this->_pinger.trigger(QUICEncryptionLevel::ONE_RTT);
+ this->_pinger.request(QUICEncryptionLevel::ONE_RTT);
}
void
@@ -996,6 +1007,7 @@ QUICNetVConnection::_state_handshake_process_initial_packet(QUICPacketUPtr packe
this->_alt_con_manager =
new QUICAltConnectionManager(this, *this->_ctable, this->_peer_quic_connection_id, params->instance_id(),
params->num_alt_connection_ids(), params->preferred_address());
+ this->_frame_generators.push_back(this->_alt_con_manager);
this->_frame_dispatcher->add_handler(this->_alt_con_manager);
}
error = this->_handshake_handler->start(packet.get(), &this->_packet_factory, this->_alt_con_manager->preferred_address());
@@ -1354,125 +1366,56 @@ QUICNetVConnection::_packetize_frames(QUICEncryptionLevel level, uint64_t max_pa
SCOPED_MUTEX_LOCK(frame_transmitter_lock, this->_frame_transmitter_mutex, this_ethread());
std::vector<QUICFrameInfo> frames;
- // CRYPTO
- frame = this->_handshake_handler->generate_frame(level, UINT16_MAX, max_frame_size);
- while (frame) {
- ++frame_count;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
- frame = this->_handshake_handler->generate_frame(level, UINT16_MAX, max_frame_size);
+ if (this->_has_ack_only_packet_out) {
+ // Sent too much ack_only packet. At this moment we need to packetize a ping frame
+ // to force peer send ack frame.
+ this->_pinger.request(level);
}
- // NEW_TOKEN
- frame = this->generate_frame(level, UINT16_MAX, max_frame_size);
- if (frame) {
- ++frame_count;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
- }
-
- // PATH_CHALLENGE, PATH_RESPOSNE
- frame = this->_path_validator->generate_frame(level, UINT16_MAX, max_frame_size);
- if (frame) {
- ++frame_count;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
- }
-
- // NEW_CONNECTION_ID
- if (this->_alt_con_manager) {
- frame = this->_alt_con_manager->generate_frame(level, UINT16_MAX, max_frame_size);
- while (frame) {
- ++frame_count;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
-
- frame = this->_alt_con_manager->generate_frame(level, UINT16_MAX, max_frame_size);
- }
- }
-
- // MAX_DATA
- frame = this->_local_flow_controller->generate_frame(level, UINT16_MAX, max_frame_size);
- if (frame) {
- ++frame_count;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
- }
-
- // DATA_BLOCKED
- if (this->_remote_flow_controller->credit() == 0 && this->_stream_manager->will_generate_frame(level)) {
- frame = this->_remote_flow_controller->generate_frame(level, UINT16_MAX, max_frame_size);
- if (frame) {
- ++frame_count;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
- }
- }
-
- // STREAM, MAX_STREAM_DATA, STREAM_DATA_BLOCKED
- if (!this->_path_validator->is_validating()) {
- frame = this->_stream_manager->generate_frame(level, this->_remote_flow_controller->credit(), max_frame_size);
- while (frame) {
- ++frame_count;
- probing |= frame->is_probing_frame();
- if (frame->type() == QUICFrameType::STREAM) {
- int ret = this->_remote_flow_controller->update(this->_stream_manager->total_offset_sent());
- QUICFCDebug("[REMOTE] %" PRIu64 "/%" PRIu64, this->_remote_flow_controller->current_offset(),
- this->_remote_flow_controller->current_limit());
- ink_assert(ret == 0);
+ bool ack_only = true;
+ for (auto g : this->_frame_generators) {
+ while (g->will_generate_frame(level)) {
+ // FIXME will_generate_frame should receive more parameters so we don't need extra checks
+ if (g == this->_remote_flow_controller && !this->_stream_manager->will_generate_frame(level)) {
+ break;
}
- this->_store_frame(buf, len, max_frame_size, frame, frames);
-
- if (++this->_stream_frames_sent % MAX_CONSECUTIVE_STREAMS == 0) {
+ if (g == this->_stream_manager && this->_path_validator->is_validating()) {
break;
}
- frame = this->_stream_manager->generate_frame(level, this->_remote_flow_controller->credit(), max_frame_size);
- }
- }
+ // Common block
+ frame = g->generate_frame(level, this->_remote_flow_controller->credit(), max_frame_size);
+ if (frame) {
+ ++frame_count;
+ probing |= frame->is_probing_frame();
+ if (frame->is_flow_controlled()) {
+ int ret = this->_remote_flow_controller->update(this->_stream_manager->total_offset_sent());
+ QUICFCDebug("[REMOTE] %" PRIu64 "/%" PRIu64, this->_remote_flow_controller->current_offset(),
+ this->_remote_flow_controller->current_limit());
+ ink_assert(ret == 0);
+ }
+ this->_store_frame(buf, len, max_frame_size, frame, frames);
- // ACK
- if (frame_count == 0) {
- if (this->_ack_frame_manager.will_generate_frame(level)) {
- frame = this->_ack_frame_manager.generate_frame(level, UINT16_MAX, max_frame_size);
- }
- } else {
- frame = this->_ack_frame_manager.generate_frame(level, UINT16_MAX, max_frame_size);
- }
+ // FIXME ACK frame should have priority
+ if (frame->type() == QUICFrameType::STREAM) {
+ if (++this->_stream_frames_sent % MAX_CONSECUTIVE_STREAMS == 0) {
+ break;
+ }
+ }
- bool ack_only = false;
- if (frame != nullptr) {
- if (frame_count == 0) {
- ack_only = true;
- }
- ++frame_count;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
- }
+ if (ack_only && frame->type() != QUICFrameType::ACK) {
+ ack_only = false;
+ this->_pinger.cancel(level);
+ }
- QUICConfig::scoped_config params;
- if (ack_only) {
- if (this->_has_ack_only_packet_out) {
- // Sent too much ack_only packet. At this moment we need to packetize a ping frame
- // to force peer send ack frame.
- // Just call trigger to not send multiple PING frames, because application could request sending PING.
- this->_pinger.trigger(level);
- } else {
- this->_has_ack_only_packet_out = true;
+ } else {
+ // Move to next generator
+ break;
+ }
}
- } else if (!ack_only) {
- this->_has_ack_only_packet_out = false;
}
- // PING
- frame = this->_pinger.generate_frame(level, UINT16_MAX, max_frame_size);
- if (frame) {
- ++frame_count;
- ack_only = false;
- this->_has_ack_only_packet_out = false;
- probing |= frame->is_probing_frame();
- this->_store_frame(buf, len, max_frame_size, frame, frames);
- }
+ this->_has_ack_only_packet_out = ack_only;
// Schedule a packet
if (len != 0) {
@@ -1926,6 +1869,7 @@ QUICNetVConnection::_switch_to_established_state()
pref_addr_buf = remote_tp->getAsBytes(QUICTransportParameterId::PREFERRED_ADDRESS, len);
this->_alt_con_manager = new QUICAltConnectionManager(this, *this->_ctable, this->_peer_quic_connection_id,
params->instance_id(), 1, {pref_addr_buf, len});
+ this->_frame_generators.push_back(this->_alt_con_manager);
this->_frame_dispatcher->add_handler(this->_alt_con_manager);
}
} else {
diff --git a/iocore/net/quic/QUICFrame.cc b/iocore/net/quic/QUICFrame.cc
index 3530fe1..a32a9d3 100644
--- a/iocore/net/quic/QUICFrame.cc
+++ b/iocore/net/quic/QUICFrame.cc
@@ -84,6 +84,12 @@ QUICFrame::is_probing_frame() const
return false;
}
+bool
+QUICFrame::is_flow_controlled() const
+{
+ return false;
+}
+
QUICFrameId
QUICFrame::id() const
{
@@ -250,6 +256,12 @@ QUICStreamFrame::size() const
return size;
}
+bool
+QUICStreamFrame::is_flow_controlled() const
+{
+ return true;
+}
+
size_t
QUICStreamFrame::store(uint8_t *buf, size_t *len, size_t limit) const
{
diff --git a/iocore/net/quic/QUICFrame.h b/iocore/net/quic/QUICFrame.h
index 7b7a1a9..94002a2 100644
--- a/iocore/net/quic/QUICFrame.h
+++ b/iocore/net/quic/QUICFrame.h
@@ -62,6 +62,7 @@ public:
virtual QUICFrameType type() const;
virtual size_t size() const = 0;
virtual bool is_probing_frame() const;
+ virtual bool is_flow_controlled() const;
virtual size_t store(uint8_t *buf, size_t *len, size_t limit) const = 0;
virtual int debug_msg(char *msg, size_t msg_len) const;
virtual void parse(const uint8_t *buf, size_t len){};
@@ -94,6 +95,7 @@ public:
QUICFrameUPtr clone() const override;
virtual QUICFrameType type() const override;
virtual size_t size() const override;
+ virtual bool is_flow_controlled() const override;
virtual size_t store(uint8_t *buf, size_t *len, size_t limit) const override;
virtual int debug_msg(char *msg, size_t msg_len) const override;
virtual void parse(const uint8_t *buf, size_t len) override;
diff --git a/iocore/net/quic/QUICHandshake.cc b/iocore/net/quic/QUICHandshake.cc
index cde0495..51349de 100644
--- a/iocore/net/quic/QUICHandshake.cc
+++ b/iocore/net/quic/QUICHandshake.cc
@@ -351,6 +351,9 @@ QUICHandshake::generate_frame(QUICEncryptionLevel level, uint64_t connection_cre
{
QUICFrameUPtr frame = QUICFrameFactory::create_null_frame();
+ // CRYPTO frame is not flow-controlled
+ connection_credit = UINT64_MAX;
+
if (!this->_is_level_matched(level)) {
return frame;
}
diff --git a/iocore/net/quic/QUICPathValidator.cc b/iocore/net/quic/QUICPathValidator.cc
index 8cac22a..40e2d14 100644
--- a/iocore/net/quic/QUICPathValidator.cc
+++ b/iocore/net/quic/QUICPathValidator.cc
@@ -131,6 +131,9 @@ QUICPathValidator::generate_frame(QUICEncryptionLevel level, uint64_t connection
{
QUICFrameUPtr frame = QUICFrameFactory::create_null_frame();
+ // PATH_CHALLENGE and PATH_RESPONSE are not flow-controlled
+ connection_credit = UINT64_MAX;
+
if (!this->_is_level_matched(level)) {
return frame;
}
diff --git a/iocore/net/quic/QUICPinger.cc b/iocore/net/quic/QUICPinger.cc
index a559b7d..c34e820 100644
--- a/iocore/net/quic/QUICPinger.cc
+++ b/iocore/net/quic/QUICPinger.cc
@@ -24,15 +24,21 @@
#include "QUICPinger.h"
void
-QUICPinger::trigger(QUICEncryptionLevel level)
+QUICPinger::request(QUICEncryptionLevel level)
{
- this->_need_to_fire[static_cast<int>(level)] = true;
+ ++this->_need_to_fire[static_cast<int>(level)];
+}
+
+void
+QUICPinger::cancel(QUICEncryptionLevel level)
+{
+ --this->_need_to_fire[static_cast<int>(level)];
}
bool
QUICPinger::will_generate_frame(QUICEncryptionLevel level)
{
- return this->_need_to_fire[QUICTypeUtil::pn_space_index(level)];
+ return this->_need_to_fire[QUICTypeUtil::pn_space_index(level)] > 0;
}
QUICFrameUPtr
@@ -44,10 +50,10 @@ QUICPinger::generate_frame(QUICEncryptionLevel level, uint64_t connection_credit
return frame;
}
- if (this->_need_to_fire[static_cast<int>(level)] && maximum_frame_size > 0) {
+ if (this->_need_to_fire[static_cast<int>(level)] > 0 && maximum_frame_size > 0) {
// don't care ping frame lost or acked
frame = QUICFrameFactory::create_ping_frame(0, nullptr);
- this->_need_to_fire[static_cast<int>(level)] = false;
+ this->_need_to_fire[static_cast<int>(level)] = 0;
}
return frame;
diff --git a/iocore/net/quic/QUICPinger.h b/iocore/net/quic/QUICPinger.h
index 8f06b1d..6658eb2 100644
--- a/iocore/net/quic/QUICPinger.h
+++ b/iocore/net/quic/QUICPinger.h
@@ -33,7 +33,8 @@ class QUICPinger : public QUICFrameGenerator
public:
QUICPinger() {}
- void trigger(QUICEncryptionLevel level);
+ void request(QUICEncryptionLevel level);
+ void cancel(QUICEncryptionLevel level);
// QUICFrameGenerator
bool will_generate_frame(QUICEncryptionLevel level) override;
@@ -41,7 +42,7 @@ public:
private:
// Initial, 0/1-RTT, and Handshake
- bool _need_to_fire[4] = {false};
+ uint64_t _need_to_fire[4] = {0};
// QUICFrameGenerator
std::vector<QUICEncryptionLevel>