avoid unnecessary userspace routing of packets
Summary:
for initial or zero-rtt packets, or in the case of short-header packets, if the
worker-id already matches, we do not need to do any userspace routing of
packets.
MODIFY
quic/server/QuicServerWorker.cpp
quic/server/QuicServerWorker.cpp
@@ -420,8 +420,49 @@ void QuicServerWorker::forwardNetworkData(
}
return;
}
+
+ bool userSpacePktRoute;
+ if (routingData.isUsingClientConnId) {
+ // no need to route further if it's an initial packet or zero-rtt packet
+ userSpacePktRoute = false;
+ } else {
+ // try matching the workerId in the destination Connection Id
+ auto connIdParam =
+ connIdAlgo_->parseConnectionId(routingData.destinationConnId);
+ if (UNLIKELY(connIdParam.hasError())) {
+ // not enough information to keep this packet
+ VLOG(3) << folly::format(
+ "Dropping packet due to DCID parsing error={}, , errorCode={}, routingInfo={}",
+ connIdParam.error().what(),
+ folly::to<std::string>(connIdParam.error().errorCode()),
+ logRoutingInfo(routingData.destinationConnId));
+ QUIC_STATS(
+ statsCallback_, onPacketDropped, PacketDropReason::PARSE_ERROR);
+ return;
+ }
+ if (LIKELY(connIdParam->workerId == workerId_)) {
+ userSpacePktRoute = false;
+ } else {
+ VLOG(3) << folly::format(
+ "worker-id mismatch in received packet, routingInfo={}",
+ logRoutingInfo(routingData.destinationConnId));
+ userSpacePktRoute = true;
+ }
+ }
+
+ if (userSpacePktRoute) {
callback_->routeDataToWorker(
- client, std::move(routingData), std::move(networkData), isForwardedData);
+ client,
+ std::move(routingData),
+ std::move(networkData),
+ isForwardedData);
+ } else {
+ dispatchPacketData(
+ client,
+ std::move(routingData),
+ std::move(networkData),
+ isForwardedData);
+ }
}
void QuicServerWorker::setPacingTimer(

MODIFY
quic/server/test/QuicServerTest.cpp
quic/server/test/QuicServerTest.cpp
@@ -153,21 +153,6 @@ class QuicServerWorkerTest : public Test {
worker_->setCongestionControllerFactory(
std::make_shared<DefaultCongestionControllerFactory>());
transportInfoCb_ = (MockQuicStats*)worker_->getTransportStatsCallback();
-
- auto cb = [&](const folly::SocketAddress& addr,
- std::unique_ptr<RoutingData>& routingData,
- std::unique_ptr<NetworkData>& networkData,
- bool isForwardedData) {
- worker_->dispatchPacketData(
- addr,
- std::move(*routingData.get()),
- std::move(*networkData.get()),
- isForwardedData);
- };
-
- EXPECT_CALL(*workerCb_, routeDataToWorkerLong(_, _, _, _))
- .WillRepeatedly(Invoke(cb));
-
socketFactory_ = std::make_unique<MockQuicUDPSocketFactory>();
EXPECT_CALL(*socketFactory_, _make(_, _)).WillRepeatedly(Return(nullptr));
worker_->setNewConnectionSocketFactory(socketFactory_.get());
@@ -1055,16 +1040,6 @@ void QuicServerWorkerTakeoverTest::testNoPacketForwarding(
Buf /* data */,
size_t len,
ConnectionId /* connId */) {
- auto cb = [&](const folly::SocketAddress& addr,
- std::unique_ptr<RoutingData>& /* routingData */,
- std::unique_ptr<NetworkData>& /* networkData */,
- bool isForwardedData) {
- EXPECT_EQ(addr.getIPAddress(), clientAddr.getIPAddress());
- EXPECT_EQ(addr.getPort(), clientAddr.getPort());
- EXPECT_FALSE(isForwardedData);
- };
- EXPECT_CALL(*takeoverWorkerCb_, routeDataToWorkerLong(_, _, _, _))
- .WillOnce(Invoke(cb));
EXPECT_CALL(*transportInfoCb_, onPacketReceived());
EXPECT_CALL(*transportInfoCb_, onRead(len));
EXPECT_CALL(*transportInfoCb_, onPacketForwarded()).Times(0);
@@ -1163,19 +1138,6 @@ void QuicServerWorkerTakeoverTest::testPacketForwarding(
return data->computeChainDataLength();
}));
takeoverWorker_->startPacketForwarding(folly::SocketAddress("0", 0));
-
- auto cb = [&](const folly::SocketAddress& client,
- std::unique_ptr<RoutingData>& routingData,
- std::unique_ptr<NetworkData>& networkData,
- bool isForwardedData) {
- takeoverWorker_->dispatchPacketData(
- client,
- std::move(*routingData.get()),
- std::move(*networkData.get()),
- isForwardedData);
- };
- EXPECT_CALL(*takeoverWorkerCb_, routeDataToWorkerLong(_, _, _, _))
- .WillOnce(Invoke(cb));
EXPECT_CALL(*transportInfoCb_, onPacketReceived());
EXPECT_CALL(*transportInfoCb_, onRead(len));
EXPECT_CALL(*transportInfoCb_, onPacketForwarded()).Times(1);
@@ -1226,40 +1188,11 @@ TEST_F(QuicServerWorkerTakeoverTest, QuicServerTakeoverProcessForwardedPkt) {
size_t bufLen =
std::min(workerBufLen, writtenData->computeChainDataLength());
memcpy(workerBuf, writtenData->buffer(), bufLen);
-
- // test processing of the forwarded packet
- auto cb = [&](const folly::SocketAddress& addr,
- std::unique_ptr<RoutingData>& /* routingData */,
- std::unique_ptr<NetworkData>& networkData,
- bool isForwardedData) {
- // verify that it is the original client address
- EXPECT_EQ(addr.getIPAddress(), clientAddr.getIPAddress());
- EXPECT_EQ(addr.getPort(), clientAddr.getPort());
- // 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(isForwardedData);
- };
- EXPECT_CALL(*takeoverWorkerCb_, routeDataToWorkerLong(_, _, _, _))
- .WillOnce(Invoke(cb));
-
takeoverCb->onDataAvailable(
client, bufLen, false, OnDataAvailableParams());
return bufLen;
}));
- auto workerCb = [&](const folly::SocketAddress& client,
- std::unique_ptr<RoutingData>& routingData,
- std::unique_ptr<NetworkData>& networkData,
- bool isForwardedData) {
- takeoverWorker_->dispatchPacketData(
- client,
- std::move(*routingData.get()),
- std::move(*networkData.get()),
- isForwardedData);
- };
- EXPECT_CALL(*takeoverWorkerCb_, routeDataToWorkerLong(_, _, _, _))
- .WillOnce(Invoke(workerCb));
+
EXPECT_CALL(*transportInfoCb_, onPacketReceived());
EXPECT_CALL(*transportInfoCb_, onRead(len));
EXPECT_CALL(*transportInfoCb_, onPacketForwarded()).Times(1);
@@ -1334,9 +1267,8 @@ class QuicServerTest : public Test {
if (stats) {
CHECK_EQ(evbs.size(), 1);
EXPECT_CALL(*transportStatsFactory_, make())
- .WillRepeatedly(Invoke([stats]() {
- return std::unique_ptr<MockQuicStats>(stats);
- }));
+ .WillRepeatedly(Invoke(
+ [stats]() { return std::unique_ptr<MockQuicStats>(stats); }));
} else {
EXPECT_CALL(*transportStatsFactory_, make()).WillRepeatedly(Invoke([&]() {
auto mockInfoCb = std::make_unique<NiceMock<MockQuicStats>>();

avoid unnecessary userspace routing of packets

Summary:
for initial or zero-rtt packets, or in the case of short-header packets, if the
worker-id already matches, we do not need to do any userspace routing of
packets.

Reviewed By: mjoras

Differential Revision: D21219335

fbshipit-source-id: 988614530960a621a02c0375f4a507b333f66e69