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 <[email protected]> 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 =
