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 9576d28 fix #2793 rework quic ack creator 9576d28 is described below commit 9576d280286361a61832eb0e9ff1a2e86947aebf Author: scw00 <sc...@apache.org> AuthorDate: Fri Nov 17 10:59:46 2017 +0800 fix #2793 rework quic ack creator --- iocore/net/quic/QUICAckFrameCreator.cc | 110 ++++++++++++++++++----- iocore/net/quic/QUICAckFrameCreator.h | 28 ++++-- iocore/net/quic/test/test_QUICAckFrameCreator.cc | 74 +++++++++++++++ 3 files changed, 183 insertions(+), 29 deletions(-) diff --git a/iocore/net/quic/QUICAckFrameCreator.cc b/iocore/net/quic/QUICAckFrameCreator.cc index f352d83..89b9525 100644 --- a/iocore/net/quic/QUICAckFrameCreator.cc +++ b/iocore/net/quic/QUICAckFrameCreator.cc @@ -28,14 +28,11 @@ int QUICAckFrameCreator::update(QUICPacketNumber packet_number, bool acknowledgable) { - if (this->_packet_count == MAXIMUM_PACKET_COUNT) { + if (this->_packet_numbers.size() == MAXIMUM_PACKET_COUNT) { return -1; } - if (packet_number > this->_largest_ack_number) { - this->_largest_ack_number = packet_number; - this->_largest_ack_received_time = Thread::get_hrtime(); - } - this->_packet_numbers[this->_packet_count++] = packet_number - this->_last_ack_number; + + this->_packet_numbers.push_back(packet_number); if (acknowledgable && !this->_can_send) { this->_can_send = true; } @@ -48,10 +45,10 @@ QUICAckFrameCreator::create() { std::unique_ptr<QUICAckFrame, QUICFrameDeleterFunc> ack_frame = {nullptr, QUICFrameDeleter::delete_null_frame}; if (this->_can_send) { - ack_frame = this->_create_ack_frame(); - this->_last_ack_number = this->_largest_ack_number; - this->_can_send = false; - this->_packet_count = 0; + ack_frame = this->_create_ack_frame(); + this->_can_send = false; + this->_packet_count = 0; + this->_packet_numbers.clear(); } return ack_frame; } @@ -67,37 +64,102 @@ void QUICAckFrameCreator::_sort_packet_numbers() { // TODO Find more smart way - std::sort(this->_packet_numbers, this->_packet_numbers + this->_packet_count); } std::unique_ptr<QUICAckFrame, QUICFrameDeleterFunc> QUICAckFrameCreator::_create_ack_frame() { std::unique_ptr<QUICAckFrame, QUICFrameDeleterFunc> ack_frame = {nullptr, QUICFrameDeleter::delete_null_frame}; - this->_sort_packet_numbers(); - uint16_t start = this->_packet_numbers[0]; - uint8_t gap = 0; - int i; + this->_packet_numbers.sort(); + QUICPacketNumber largest_ack_number = this->_packet_numbers.largest_ack_number(); + QUICPacketNumber last_ack_number = largest_ack_number; + + size_t i = 0; + uint8_t gap = 0; uint64_t length = 0; - for (i = 0, length = 0; i < this->_packet_count; ++i, ++length) { - if (this->_packet_numbers[i] == start + length) { + + while (i < this->_packet_numbers.size()) { + if (this->_packet_numbers[i] == last_ack_number) { + last_ack_number--; + length++; + i++; continue; } + if (ack_frame) { ack_frame->ack_block_section()->add_ack_block({gap, length}); } else { - uint16_t delay = (Thread::get_hrtime() - this->_largest_ack_received_time) / 1000; // TODO Milliseconds? - ack_frame = QUICFrameFactory::create_ack_frame(this->_largest_ack_number, delay, length); + uint16_t delay = (Thread::get_hrtime() - this->_packet_numbers.largest_ack_received_time()) / 1000; // TODO Milliseconds? + ack_frame = QUICFrameFactory::create_ack_frame(largest_ack_number, delay, length); } - gap = this->_packet_numbers[i] - this->_packet_numbers[i - 1] - 1; - start = this->_packet_numbers[i]; - length = 0; + + gap = last_ack_number - this->_packet_numbers[i]; + last_ack_number = this->_packet_numbers[i]; + length = 0; } + if (ack_frame) { ack_frame->ack_block_section()->add_ack_block({gap, length}); } else { - uint16_t delay = (Thread::get_hrtime() - this->_largest_ack_received_time) / 1000; // TODO Milliseconds? - ack_frame = QUICFrameFactory::create_ack_frame(this->_largest_ack_number, delay, length); + uint16_t delay = (Thread::get_hrtime() - this->_packet_numbers.largest_ack_received_time()) / 1000; // TODO Milliseconds? + ack_frame = QUICFrameFactory::create_ack_frame(largest_ack_number, delay, length); } return ack_frame; } + +void +QUICAckPacketNumbers::push_back(QUICPacketNumber packet_number) +{ + if (packet_number > this->_largest_ack_number) { + this->_largest_ack_received_time = Thread::get_hrtime(); + this->_largest_ack_number = packet_number; + } + + this->_packet_numbers.push_back(packet_number); +} + +QUICPacketNumber +QUICAckPacketNumbers::front() +{ + return this->_packet_numbers.front(); +} + +QUICPacketNumber +QUICAckPacketNumbers::back() +{ + return this->_packet_numbers.back(); +} + +size_t +QUICAckPacketNumbers::size() +{ + return this->_packet_numbers.size(); +} + +void +QUICAckPacketNumbers::clear() +{ + this->_packet_numbers.clear(); + this->_largest_ack_number = 0; + this->_largest_ack_received_time = 0; +} + +QUICPacketNumber +QUICAckPacketNumbers::largest_ack_number() +{ + return this->_largest_ack_number; +} + +ink_hrtime +QUICAckPacketNumbers::largest_ack_received_time() +{ + return this->_largest_ack_received_time; +} + +void +QUICAckPacketNumbers::sort() +{ + // TODO Find more smart way + std::sort(this->_packet_numbers.begin(), this->_packet_numbers.end(), + [](QUICPacketNumber a, QUICPacketNumber b) -> bool { return b < a; }); +} diff --git a/iocore/net/quic/QUICAckFrameCreator.h b/iocore/net/quic/QUICAckFrameCreator.h index 7251747..da59c5e 100644 --- a/iocore/net/quic/QUICAckFrameCreator.h +++ b/iocore/net/quic/QUICAckFrameCreator.h @@ -27,6 +27,28 @@ #include "QUICTypes.h" #include "QUICFrame.h" +class QUICAckPacketNumbers +{ +public: + void push_back(QUICPacketNumber packet_number); + QUICPacketNumber front(); + QUICPacketNumber back(); + size_t size(); + void clear(); + void sort(); + + QUICPacketNumber largest_ack_number(); + ink_hrtime largest_ack_received_time(); + + const QUICPacketNumber &operator[](int i) const { return this->_packet_numbers[i]; } + +private: + QUICPacketNumber _largest_ack_number = 0; + ink_hrtime _largest_ack_received_time = 0; + + std::vector<QUICPacketNumber> _packet_numbers; +}; + class QUICAckFrameCreator { public: @@ -57,11 +79,7 @@ public: private: bool _can_send = false; - QUICPacketNumber _largest_ack_number = 0; - QUICPacketNumber _last_ack_number = 0; - ink_hrtime _largest_ack_received_time = 0; - - uint16_t _packet_numbers[MAXIMUM_PACKET_COUNT]; + QUICAckPacketNumbers _packet_numbers; uint16_t _packet_count = 0; void _sort_packet_numbers(); diff --git a/iocore/net/quic/test/test_QUICAckFrameCreator.cc b/iocore/net/quic/test/test_QUICAckFrameCreator.cc index a519687..1ca0691 100644 --- a/iocore/net/quic/test/test_QUICAckFrameCreator.cc +++ b/iocore/net/quic/test/test_QUICAckFrameCreator.cc @@ -64,6 +64,80 @@ TEST_CASE("QUICAckFrameCreator", "[quic]") CHECK(frame != nullptr); CHECK(frame->num_blocks() == 1); CHECK(frame->largest_acknowledged() == 10); + CHECK(frame->ack_block_section()->first_ack_block_length() == 1); + CHECK(frame->ack_block_section()->begin()->gap() == 2); +} + +TEST_CASE("QUICAckFrameCreator_loss_recover", "[quic]") +{ + QUICAckFrameCreator creator; + std::unique_ptr<QUICAckFrame, QUICFrameDeleterFunc> frame = {nullptr, nullptr}; + + // Initial state + frame = creator.create(); + CHECK(frame == nullptr); + + creator.update(2, true); + creator.update(5, true); + creator.update(6, true); + creator.update(8, true); + creator.update(9, true); + + frame = creator.create(); + CHECK(frame != nullptr); + CHECK(frame->num_blocks() == 2); + CHECK(frame->largest_acknowledged() == 9); CHECK(frame->ack_block_section()->first_ack_block_length() == 2); + CHECK(frame->ack_block_section()->begin()->gap() == 1); + + frame = creator.create(); + CHECK(frame == nullptr); + + creator.update(7, true); + creator.update(4, true); + frame = creator.create(); + CHECK(frame != nullptr); + CHECK(frame->num_blocks() == 1); + CHECK(frame->largest_acknowledged() == 7); + CHECK(frame->ack_block_section()->first_ack_block_length() == 1); CHECK(frame->ack_block_section()->begin()->gap() == 2); } + +TEST_CASE("QUICAckFrameCreator_QUICAckPacketNumbers", "[quic]") +{ + QUICAckPacketNumbers packet_numbers; + + CHECK(packet_numbers.size() == 0); + CHECK(packet_numbers.largest_ack_number() == 0); + CHECK(packet_numbers.largest_ack_received_time() == 0); + + Thread::get_hrtime_updated(); + + packet_numbers.push_back(3); + CHECK(packet_numbers.size() == 1); + CHECK(packet_numbers.largest_ack_number() == 3); + + ink_hrtime ti = packet_numbers.largest_ack_received_time(); + CHECK(packet_numbers.largest_ack_received_time() != 0); + + Thread::get_hrtime_updated(); + + packet_numbers.push_back(2); + CHECK(packet_numbers.size() == 2); + CHECK(packet_numbers.largest_ack_number() == 3); + CHECK(packet_numbers.largest_ack_received_time() == ti); + + Thread::get_hrtime_updated(); + + packet_numbers.push_back(10); + CHECK(packet_numbers.size() == 3); + CHECK(packet_numbers.largest_ack_number() == 10); + CHECK(packet_numbers.largest_ack_received_time() > ti); + + Thread::get_hrtime_updated(); + + packet_numbers.clear(); + CHECK(packet_numbers.size() == 0); + CHECK(packet_numbers.largest_ack_number() == 0); + CHECK(packet_numbers.largest_ack_received_time() == 0); +} -- To stop receiving notification emails like this one, please contact ['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].