diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 50840cd11..772fec96c 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -1888,7 +1888,7 @@ void QuicTransportBase::onNetworkData( SocketObserverInterface::PacketsReceivedEvent::ReceivedPacket:: Builder() .setPacketReceiveTime(networkData.receiveTimePoint) - .setPacketNumBytes(packet->computeChainDataLength()) + .setPacketNumBytes(packet.buf->computeChainDataLength()) .build()); } @@ -1904,7 +1904,8 @@ void QuicTransportBase::onNetworkData( for (auto& packet : networkData.packets) { onReadData( peer, - NetworkDataSingle(std::move(packet), networkData.receiveTimePoint)); + NetworkDataSingle( + std::move(packet.buf), networkData.receiveTimePoint)); if (conn_->peerConnectionError) { closeImpl(QuicError( QuicErrorCode(TransportErrorCode::NO_ERROR), "Peer closed")); diff --git a/quic/fizz/client/test/QuicClientTransportTestUtil.h b/quic/fizz/client/test/QuicClientTransportTestUtil.h index 421e55ff1..e7093517b 100644 --- a/quic/fizz/client/test/QuicClientTransportTestUtil.h +++ b/quic/fizz/client/test/QuicClientTransportTestUtil.h @@ -727,9 +727,9 @@ class QuicClientTransportTestBase : public virtual testing::Test { NetworkData&& data, bool writes = true, folly::SocketAddress* peer = nullptr) { - for (const auto& packetBuf : data.packets) { + for (const auto& packet : data.packets) { deliverDataWithoutErrorCheck( - peer == nullptr ? serverAddr : *peer, packetBuf->coalesce(), writes); + peer == nullptr ? serverAddr : *peer, packet.buf->coalesce(), writes); } } @@ -763,9 +763,9 @@ class QuicClientTransportTestBase : public virtual testing::Test { NetworkData&& data, bool writes = true, folly::SocketAddress* peer = nullptr) { - for (const auto& packetBuf : data.packets) { + for (const auto& packet : data.packets) { deliverData( - peer == nullptr ? serverAddr : *peer, packetBuf->coalesce(), writes); + peer == nullptr ? serverAddr : *peer, packet.buf->coalesce(), writes); } } diff --git a/quic/server/QuicServerWorker.cpp b/quic/server/QuicServerWorker.cpp index acd426e0f..244d33af4 100644 --- a/quic/server/QuicServerWorker.cpp +++ b/quic/server/QuicServerWorker.cpp @@ -885,7 +885,7 @@ void QuicServerWorker::dispatchPacketData( // If there is a token present, decrypt it (could be either a retry // token or a new token) - folly::io::Cursor cursor(networkData.packets.front().get()); + folly::io::Cursor cursor(networkData.packets.front().buf.get()); auto maybeEncryptedToken = maybeGetEncryptedToken(cursor); bool hasTokenSecret = transportSettings_.retryTokenSecret.hasValue(); diff --git a/quic/server/test/QuicServerTest.cpp b/quic/server/test/QuicServerTest.cpp index 83224a53c..6f0b3d615 100644 --- a/quic/server/test/QuicServerTest.cpp +++ b/quic/server/test/QuicServerTest.cpp @@ -46,7 +46,7 @@ namespace test { MATCHER_P(NetworkDataMatches, networkData, "") { for (size_t i = 0; i < arg.packets.size(); ++i) { folly::IOBufEqualTo eq; - bool equals = eq(*arg.packets[i], networkData); + bool equals = eq(*arg.packets[i].buf, networkData); if (equals) { return true; } @@ -1974,7 +1974,7 @@ TEST_F(QuicServerWorkerTakeoverTest, QuicServerTakeoverProcessForwardedPkt) { // the original data should be extracted after processing takeover // protocol related information EXPECT_EQ(networkData->packets.size(), 1); - EXPECT_TRUE(eq(*data, *(networkData->packets[0]))); + EXPECT_TRUE(eq(*data, *(networkData->packets[0].buf))); EXPECT_TRUE(isForwardedData); }; EXPECT_CALL(*takeoverWorkerCb_, routeDataToWorkerLong(_, _, _, _, _)) @@ -2168,7 +2168,7 @@ class QuicServerTest : public Test { auto, const auto& networkData) mutable { EXPECT_GT(networkData.packets.size(), 0); EXPECT_TRUE(folly::IOBufEqualTo()( - *networkData.packets[0], *expected)); + *networkData.packets[0].buf, *expected)); std::unique_lock lg(m); calledOnNetworkData = true; cv.notify_one(); @@ -2329,7 +2329,7 @@ TEST_F(QuicServerTest, RouteDataFromDifferentThread) { .WillOnce(Invoke([&](auto, const auto& networkData) { EXPECT_GT(networkData.packets.size(), 0); EXPECT_TRUE( - folly::IOBufEqualTo()(*networkData.packets[0], *initialData)); + folly::IOBufEqualTo()(*networkData.packets[0].buf, *initialData)); })); server_->routeDataToWorker( @@ -2429,7 +2429,7 @@ class QuicServerTakeoverTest : public Test { [&, expected = data.get()](auto, const auto& networkData) { EXPECT_GT(networkData.packets.size(), 0); EXPECT_TRUE(folly::IOBufEqualTo()( - *networkData.packets[0], *expected)); + *networkData.packets[0].buf, *expected)); baton.post(); })); return transport; @@ -2536,8 +2536,8 @@ class QuicServerTakeoverTest : public Test { .WillOnce( Invoke([&, expected = data.get()](auto, const auto& networkData) { EXPECT_GT(networkData.packets.size(), 0); - EXPECT_TRUE( - folly::IOBufEqualTo()(*networkData.packets[0], *expected)); + EXPECT_TRUE(folly::IOBufEqualTo()( + *networkData.packets[0].buf, *expected)); b1.post(); })); // new quic server receives the packet and forwards it @@ -3000,7 +3000,7 @@ TEST_F(QuicServerTest, ZeroRttPacketRoute) { [&, expected = data.get()](auto, const auto& networkData) { EXPECT_GT(networkData.packets.size(), 0); EXPECT_TRUE(folly::IOBufEqualTo()( - *networkData.packets[0], *expected)); + *networkData.packets[0].buf, *expected)); b.post(); })); return transport; @@ -3043,7 +3043,7 @@ TEST_F(QuicServerTest, ZeroRttPacketRoute) { const NetworkData& networkData) noexcept { EXPECT_GT(networkData.packets.size(), 0); EXPECT_EQ(peer, reader->getSocket().address()); - EXPECT_TRUE(folly::IOBufEqualTo()(*data, *networkData.packets[0])); + EXPECT_TRUE(folly::IOBufEqualTo()(*data, *networkData.packets[0].buf)); b1.post(); }; EXPECT_CALL(*transport, onNetworkData(_, _)).WillOnce(Invoke(verifyZeroRtt)); @@ -3094,8 +3094,8 @@ TEST_F(QuicServerTest, ZeroRttBeforeInitial) { EXPECT_CALL(*transport, onNetworkData(_, _)) .Times(2) .WillRepeatedly(Invoke([&](auto, auto& networkData) { - for (auto& buf : networkData.packets) { - receivedData.emplace_back(buf->clone()); + for (auto& packet : networkData.packets) { + receivedData.emplace_back(packet.buf->clone()); } if (receivedData.size() == 2) { b.post(); diff --git a/quic/state/StateData.h b/quic/state/StateData.h index e348c0106..e289de365 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -43,9 +43,17 @@ #include namespace quic { + +struct ReceivedPacket { + explicit ReceivedPacket(Buf&& bufIn) : buf(std::move(bufIn)) {} + + // data + Buf buf; +}; + struct NetworkData { TimePoint receiveTimePoint; - std::vector packets; + std::vector packets; size_t totalData{0}; NetworkData() = default; @@ -59,20 +67,29 @@ struct NetworkData { NetworkData(std::vector&& packetBufs, const TimePoint& receiveTime) : receiveTimePoint(receiveTime), - packets(std::move(packetBufs)), - totalData(0) { - for (const auto& buf : packets) { - totalData += buf->computeChainDataLength(); - } - } + packets([&packetBufs]() { + std::vector result; + result.reserve(packetBufs.size()); + for (auto& packetBuf : packetBufs) { + result.emplace_back(std::move(packetBuf)); + } + return result; + }()), + totalData([this]() { + size_t result = 0; + for (const auto& packet : packets) { + result += packet.buf->computeChainDataLength(); + } + return result; + }()) {} std::unique_ptr moveAllData() && { std::unique_ptr buf; - for (size_t i = 0; i < packets.size(); ++i) { + for (auto& packet : packets) { if (buf) { - buf->prependChain(std::move(packets[i])); + buf->prependChain(std::move(packet.buf)); } else { - buf = std::move(packets[i]); + buf = std::move(packet.buf); } } return buf;