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/08/22 01:37:48 UTC
[trafficserver] 02/03: Fix retransmittable flag on packetizing
frames
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
commit 4b13b8c5d2f7d60b44522d43476fe4e77970f54a
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Aug 21 16:24:29 2018 +0900
Fix retransmittable flag on packetizing frames
---
iocore/net/QUICNetVConnection.cc | 16 +++++++++-------
iocore/net/quic/QUICPacket.cc | 5 +++--
iocore/net/quic/QUICPacket.h | 3 ++-
iocore/net/quic/test/test_QUICVersionNegotiator.cc | 8 ++++----
4 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index 87b5ed1..e43d9c4 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -1293,16 +1293,15 @@ QUICNetVConnection::_packetize_frames(QUICEncryptionLevel level, uint64_t max_pa
size_t len = 0;
ats_unique_buf buf = ats_unique_malloc(max_packet_size);
QUICFrameUPtr frame(nullptr, nullptr);
- bool retransmittable = false;
SCOPED_MUTEX_LOCK(packet_transmitter_lock, this->_packet_transmitter_mutex, this_ethread());
SCOPED_MUTEX_LOCK(frame_transmitter_lock, this->_frame_transmitter_mutex, this_ethread());
- bool will_be_ack_only = true;
+ bool ack_only = true;
if (this->_connection_error || this->_packet_retransmitter.will_generate_frame(level) ||
this->_handshake_handler->will_generate_frame(level) || this->_stream_manager->will_generate_frame(level) ||
this->_path_validator->will_generate_frame(level)) {
- will_be_ack_only = false;
+ ack_only = false;
}
// CRYPTO
@@ -1314,7 +1313,7 @@ QUICNetVConnection::_packetize_frames(QUICEncryptionLevel level, uint64_t max_pa
}
// ACK
- if (will_be_ack_only) {
+ if (ack_only) {
if (this->_ack_frame_creator.will_generate_frame(level)) {
frame = this->_ack_frame_creator.generate_frame(level, UINT16_MAX, max_frame_size);
}
@@ -1382,7 +1381,8 @@ QUICNetVConnection::_packetize_frames(QUICEncryptionLevel level, uint64_t max_pa
}
}
- packet = this->_build_packet(level, std::move(buf), len, retransmittable);
+ // Packet is retransmittable if it's not ack only packet
+ packet = this->_build_packet(level, std::move(buf), len, !ack_only);
}
return packet;
@@ -1440,6 +1440,7 @@ QUICNetVConnection::_recv_and_ack(QUICPacketUPtr packet)
int ret = this->_local_flow_controller->update(this->_stream_manager->total_offset_received());
QUICFCDebug("[LOCAL] %" PRIu64 "/%" PRIu64, this->_local_flow_controller->current_offset(),
this->_local_flow_controller->current_limit());
+
if (ret != 0) {
return QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::FLOW_CONTROL_ERROR));
}
@@ -1464,8 +1465,9 @@ QUICNetVConnection::_build_packet(ats_unique_buf buf, size_t len, bool retransmi
case QUICPacketType::INITIAL: {
QUICConnectionId dcid =
(this->netvc_context == NET_VCONNECTION_OUT) ? this->_original_quic_connection_id : this->_peer_quic_connection_id;
- packet = this->_packet_factory.create_initial_packet(
- dcid, this->_quic_connection_id, this->largest_acked_packet_number(QUICEncryptionLevel::INITIAL), std::move(buf), len);
+ packet = this->_packet_factory.create_initial_packet(dcid, this->_quic_connection_id,
+ this->largest_acked_packet_number(QUICEncryptionLevel::INITIAL),
+ std::move(buf), len, retransmittable);
break;
}
case QUICPacketType::RETRY: {
diff --git a/iocore/net/quic/QUICPacket.cc b/iocore/net/quic/QUICPacket.cc
index 82e0f78..ebd9f68 100644
--- a/iocore/net/quic/QUICPacket.cc
+++ b/iocore/net/quic/QUICPacket.cc
@@ -1117,13 +1117,14 @@ QUICPacketFactory::create_version_negotiation_packet(QUICConnectionId dcid, QUIC
QUICPacketUPtr
QUICPacketFactory::create_initial_packet(QUICConnectionId destination_cid, QUICConnectionId source_cid,
- QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len)
+ QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len,
+ bool retransmittable)
{
int index = QUICTypeUtil::pn_space_index(QUICEncryptionLevel::INITIAL);
QUICPacketNumber pn = this->_packet_number_generator[index].next();
QUICPacketHeaderUPtr header = QUICPacketHeader::build(QUICPacketType::INITIAL, QUICKeyPhase::INITIAL, destination_cid, source_cid,
pn, base_packet_number, this->_version, std::move(payload), len);
- return this->_create_encrypted_packet(std::move(header), true);
+ return this->_create_encrypted_packet(std::move(header), retransmittable);
}
QUICPacketUPtr
diff --git a/iocore/net/quic/QUICPacket.h b/iocore/net/quic/QUICPacket.h
index b43b4e4..4760bf2 100644
--- a/iocore/net/quic/QUICPacket.h
+++ b/iocore/net/quic/QUICPacket.h
@@ -391,7 +391,8 @@ public:
QUICPacketUPtr create(IpEndpoint from, ats_unique_buf buf, size_t len, QUICPacketNumber base_packet_number,
QUICPacketCreationResult &result);
QUICPacketUPtr create_initial_packet(QUICConnectionId destination_cid, QUICConnectionId source_cid,
- QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len);
+ QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len,
+ bool retransmittable);
QUICPacketUPtr create_retry_packet(QUICConnectionId destination_cid, QUICConnectionId source_cid, ats_unique_buf payload,
size_t len, bool retransmittable);
QUICPacketUPtr create_handshake_packet(QUICConnectionId destination_cid, QUICConnectionId source_cid,
diff --git a/iocore/net/quic/test/test_QUICVersionNegotiator.cc b/iocore/net/quic/test/test_QUICVersionNegotiator.cc
index 78142b6..a4e6842 100644
--- a/iocore/net/quic/test/test_QUICVersionNegotiator.cc
+++ b/iocore/net/quic/test/test_QUICVersionNegotiator.cc
@@ -40,7 +40,7 @@ TEST_CASE("QUICVersionNegotiator - Server Side", "[quic]")
// Negotiate version
packet_factory.set_version(QUIC_SUPPORTED_VERSIONS[0]);
- QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0);
+ QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0, true);
vn.negotiate(initial_packet.get());
CHECK(vn.status() == QUICVersionNegotiationStatus::NEGOTIATED);
@@ -58,7 +58,7 @@ TEST_CASE("QUICVersionNegotiator - Server Side", "[quic]")
// Negotiate version
packet_factory.set_version(QUIC_SUPPORTED_VERSIONS[0]);
- QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0);
+ QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0, true);
vn.negotiate(initial_packet.get());
CHECK(vn.status() == QUICVersionNegotiationStatus::NEGOTIATED);
@@ -76,7 +76,7 @@ TEST_CASE("QUICVersionNegotiator - Server Side", "[quic]")
// Negotiate version
packet_factory.set_version(QUIC_EXERCISE_VERSIONS);
- QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0);
+ QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0, true);
vn.negotiate(initial_packet.get());
CHECK(vn.status() == QUICVersionNegotiationStatus::NOT_NEGOTIATED);
@@ -118,7 +118,7 @@ TEST_CASE("QUICVersionNegotiator - Client Side", "[quic]")
// Negotiate version
packet_factory.set_version(QUIC_EXERCISE_VERSIONS);
- QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0);
+ QUICPacketUPtr initial_packet = packet_factory.create_initial_packet({}, {}, 0, ats_unique_malloc(0), 0, true);
// Server send VN packet based on Initial packet
QUICPacketUPtr vn_packet =