lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-07-30 14:43:05 +03:00
Code Activity

Cleanup and modularize receive path, improve timestamp support [7/x]

Browse Source 
Summary:
This diff:
- Adds `QuicAsyncUDPSocketWrapperImpl` and changes existing instantiatons of `QuicAsyncUDPSocketWrapper` to instead instantiate `QuicAsyncUDPSocketWrapperImpl`. In follow up diffs, pure virtual functions will be added to `QuicAsyncUDPSocketWrapper` and implemented in `QuicAsyncUDPSocketWrapperImpl`. See D48717388 for more information.

--

This diff is part of a larger stack focused on the following:

- **Cleaning up client and server UDP packet receive paths while improving testability.** We currently have multiple receive paths for client and server. Capabilities vary significantly and there are few tests. For instance:
  - The server receive path supports socket RX timestamps, abet incorrectly in that it does not store timestamp per packet. In comparison, the client receive path does not currently support socket RX timestamps, although the code in `QuicClientTransport::recvmsg` and `QuicClientTransport::recvmmsg` makes reference to socket RX timestamps, making it confusing to understand the capabilities available when tracing through the code. This complicates the tests in `QuicTypedTransportTests`, as we have to disable test logic that depends on socket RX timestamps for client tests.
  - The client currently has three receive paths, and none of them are well tested.

- **Modularize and abstract components in the receive path.** This will make it easier to mock/fake the UDP socket and network layers.
  - `QuicClientTransport` and `QuicServerTransport` currently contain UDP socket handling logic that operates over lower layer primitives such `cmsg` and `io_vec` (see `QuicClientTransport::recvmmsg` and `...::recvmsg` as examples).
  - Because this UDP socket handling logic is inside of the mvfst transport implementations, it is difficult to test this logic in isolation and mock/fake the underlying socket and network layers. For instance, injecting a user space network emulator that operates at the socket layer would require faking `folly::AsyncUDPSocket`, which is non-trivial given that `AsyncUDPSocket` does not abstract away intricacies arising from the aforementioned lower layer primitives.
  - By shifting this logic into an intermediate layer between the transport and the underlying UDP socket, it will be easier to mock out the UDP socket layer when testing functionality at higher layers, and inject fake components when we want to emulate the network between a mvfst client and server. It will also be easier for us to have unit tests focused on testing interactions between the UDP socket implementation and this intermediate layer.

- **Improving receive path timestamping.** We only record a single timestamp per `NetworkData` at the moment, but (1) it is possible for a `NetworkData` to have multiple packets, each with their own timestamps, and (2) we should be able to record both userspace and socket timestamps.

Reviewed By: jbeshay, mjoras

Differential Revision: D48717592

fbshipit-source-id: e21368f5c1f3b37608fc1c88617e96b93a02f6e0
This commit is contained in:
Brandon Schlinker
2023-09-21 07:57:58 -07:00
committed by Facebook GitHub Bot
parent 19dc3e4921
commit ad3dd0ec01
17 changed files with 31 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
quic/api/QuicBatchWriterFactory.cpp
View File

@ -97,7 +97,7 @@ class ThreadLocalBatchWriterCache : public folly::AsyncTimeout {
if (evb && evb->getBackingEventBase() && !socket_) {
auto fd = writer->getAndResetFd();
if (fd >= 0) {
socket_ = std::make_unique<quic::QuicAsyncUDPSocketWrapper>(
socket_ = std::make_unique<quic::QuicAsyncUDPSocketWrapperImpl>(
evb->getBackingEventBase());
socket_->setFD(
quic::toNetworkFdType(fd),

2
quic/api/test/IoBufQuicBatchTest.cpp
View File

@ -21,7 +21,7 @@ namespace quic {
namespace testing {
void RunTest(int numBatch) {
folly::EventBase evb;
QuicAsyncUDPSocketWrapper sock(&evb);
QuicAsyncUDPSocketWrapperImpl sock(&evb);
auto batchWriter = BatchWriterPtr(new test::TestPacketBatchWriter(numBatch));
folly::SocketAddress peerAddress{"127.0.0.1", 1234};

12
quic/api/test/QuicBatchWriterTest.cpp
View File

@ -63,7 +63,7 @@ TEST_P(QuicBatchWriterTest, TestBatchingNone) {
TEST_P(QuicBatchWriterTest, TestBatchingGSOBase) {
bool useThreadLocal = GetParam();
folly::EventBase evb;
QuicAsyncUDPSocketWrapper sock(&evb);
QuicAsyncUDPSocketWrapperImpl sock(&evb);
sock.setReuseAddr(false);
sock.bind(folly::SocketAddress("127.0.0.1", 0));
gsoSupported_ = sock.getGSO() >= 0;
@ -93,7 +93,7 @@ TEST_P(QuicBatchWriterTest, TestBatchingGSOBase) {
TEST_P(QuicBatchWriterTest, TestBatchingGSOLastSmallPacket) {
bool useThreadLocal = GetParam();
folly::EventBase evb;
QuicAsyncUDPSocketWrapper sock(&evb);
QuicAsyncUDPSocketWrapperImpl sock(&evb);
sock.setReuseAddr(false);
sock.bind(folly::SocketAddress("127.0.0.1", 0));
gsoSupported_ = sock.getGSO() >= 0;
@ -135,7 +135,7 @@ TEST_P(QuicBatchWriterTest, TestBatchingGSOLastSmallPacket) {
TEST_P(QuicBatchWriterTest, TestBatchingGSOLastBigPacket) {
bool useThreadLocal = GetParam();
folly::EventBase evb;
QuicAsyncUDPSocketWrapper sock(&evb);
QuicAsyncUDPSocketWrapperImpl sock(&evb);
sock.setReuseAddr(false);
sock.bind(folly::SocketAddress("127.0.0.1", 0));
gsoSupported_ = sock.getGSO() >= 0;
@ -172,7 +172,7 @@ TEST_P(QuicBatchWriterTest, TestBatchingGSOLastBigPacket) {
TEST_P(QuicBatchWriterTest, TestBatchingGSOBatchNum) {
bool useThreadLocal = GetParam();
folly::EventBase evb;
QuicAsyncUDPSocketWrapper sock(&evb);
QuicAsyncUDPSocketWrapperImpl sock(&evb);
sock.setReuseAddr(false);
sock.bind(folly::SocketAddress("127.0.0.1", 0));
gsoSupported_ = sock.getGSO() >= 0;
@ -255,7 +255,7 @@ TEST_P(QuicBatchWriterTest, TestBatchingSendmmsg) {
TEST_P(QuicBatchWriterTest, TestBatchingSendmmsgGSOBatchNum) {
bool useThreadLocal = GetParam();
folly::EventBase evb;
QuicAsyncUDPSocketWrapper sock(&evb);
QuicAsyncUDPSocketWrapperImpl sock(&evb);
sock.setReuseAddr(false);
sock.bind(folly::SocketAddress("127.0.0.1", 0));
gsoSupported_ = sock.getGSO() >= 0;
@ -301,7 +301,7 @@ TEST_P(QuicBatchWriterTest, TestBatchingSendmmsgGSOBatchNum) {
TEST_P(QuicBatchWriterTest, TestBatchingSendmmsgGSOBatcBigSmallPacket) {
bool useThreadLocal = GetParam();
folly::EventBase evb;
QuicAsyncUDPSocketWrapper sock(&evb);
QuicAsyncUDPSocketWrapperImpl sock(&evb);
sock.setReuseAddr(false);
sock.bind(folly::SocketAddress("127.0.0.1", 0));
gsoSupported_ = sock.getGSO() >= 0;

2
quic/api/test/QuicStreamAsyncTransportTest.cpp
View File

@ -156,7 +156,7 @@ class QuicStreamAsyncTransportTest : public Test {
.WillOnce(Invoke([&p = promise]() mutable { p.setValue(); }));
clientEvb_.runInLoop([&]() {
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(&clientEvb_);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(&clientEvb_);
auto fizzClientContext =
FizzClientQuicHandshakeContext::Builder()
.setCertificateVerifier(test::createTestCertificateVerifier())

2
quic/client/connector/QuicConnector.cpp
View File

@ -55,7 +55,7 @@ void QuicConnector::connect(
return;
}
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(eventBase);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(eventBase);
quicClient_ = quic::QuicClientTransport::newClient(
eventBase,
std::move(sock),

2
quic/client/test/ClientStateMachineTest.cpp
View File

@ -107,7 +107,7 @@ TEST_F(ClientStateMachineTest, PreserveHappyeyabllsDuringUndo) {
client_->clientConnectionId = ConnectionId::createRandom(8);
client_->happyEyeballsState.finished = true;
client_->happyEyeballsState.secondSocket =
std::make_unique<QuicAsyncUDPSocketWrapper>(&evb);
std::make_unique<QuicAsyncUDPSocketWrapperImpl>(&evb);
auto newConn = undoAllClientStateForRetry(std::move(client_));
EXPECT_TRUE(newConn->happyEyeballsState.finished);
EXPECT_NE(nullptr, newConn->happyEyeballsState.secondSocket);

2
quic/client/test/QuicConnectorTest.cpp
View File

@ -32,7 +32,7 @@ class QuicConnectorTest : public Test {
auto verifier = createTestCertificateVerifier();
auto clientCtx = std::make_shared<fizz::client::FizzClientContext>();
auto pskCache = std::make_shared<BasicQuicPskCache>();
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(&eventBase_);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(&eventBase_);
auto fizzClientContext = FizzClientQuicHandshakeContext::Builder()
.setFizzClientContext(clientCtx)
.setCertificateVerifier(verifier)

6
quic/common/QuicAsyncUDPSocketWrapper.h
View File

@ -32,6 +32,12 @@ class QuicAsyncUDPSocketWrapper : public QuicAsyncUDPSocketType {
using ErrMessageCallback = QuicAsyncUDPSocketType::ErrMessageCallback;
};
class QuicAsyncUDPSocketWrapperImpl : public QuicAsyncUDPSocketWrapper {
public:
using QuicAsyncUDPSocketWrapper::QuicAsyncUDPSocketWrapper;
~QuicAsyncUDPSocketWrapperImpl() override = default;
};
int getSocketFd(const QuicAsyncUDPSocketWrapper& s);
NetworkFdType toNetworkFdType(int fd);

2
quic/common/testutil/MockAsyncUDPSocket.h
View File

@ -13,6 +13,6 @@
namespace quic::test {
using MockAsyncUDPSocket =
folly::test::MockAsyncUDPSocketT<quic::QuicAsyncUDPSocketWrapper>;
folly::test::MockAsyncUDPSocketT<quic::QuicAsyncUDPSocketWrapperImpl>;
} // namespace quic::test

4
quic/fizz/client/test/QuicClientTransportTest.cpp
View File

@ -89,7 +89,7 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
std::shared_ptr<TestingQuicClientTransport> createClient() {
pskCache_ = std::make_shared<BasicQuicPskCache>();
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(&eventbase_);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(&eventbase_);
auto fizzClientContext = FizzClientQuicHandshakeContext::Builder()
.setFizzClientContext(clientCtx)
.setCertificateVerifier(verifier)
@ -5795,7 +5795,7 @@ TEST(AsyncUDPSocketTest, CloseMultipleTimes) {
};
EventBase evb;
QuicAsyncUDPSocketWrapper socket(&evb);
QuicAsyncUDPSocketWrapperImpl socket(&evb);
TransportSettings transportSettings;
EmptyErrMessageCallback errMessageCallback;
EmptyReadCallback readCallback;

2
quic/samples/echo/EchoClient.h
View File

@ -196,7 +196,7 @@ class EchoClient : public quic::QuicSocket::ConnectionSetupCallback,
folly::SocketAddress addr(host_.c_str(), port_);
evb->runInEventBaseThreadAndWait([&] {
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(evb);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(evb);
auto fizzClientContext =
FizzClientQuicHandshakeContext::Builder()
.setCertificateVerifier(test::createTestCertificateVerifier())

2
quic/server/QuicReusePortUDPSocketFactory.h
View File

@ -19,7 +19,7 @@ class QuicReusePortUDPSocketFactory : public QuicUDPSocketFactory {
std::unique_ptr<QuicAsyncUDPSocketWrapper> make(folly::EventBase* evb, int)
override {
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(evb);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(evb);
sock->setReusePort(reusePort_);
sock->setReuseAddr(reuseAddr_);
return sock;

2
quic/server/QuicServerPacketRouter.cpp
View File

@ -172,7 +172,7 @@ void TakeoverPacketHandler::forwardPacket(Buf writeBuffer) {
std::unique_ptr<QuicAsyncUDPSocketWrapper> TakeoverPacketHandler::makeSocket(
folly::EventBase* evb) {
return std::make_unique<QuicAsyncUDPSocketWrapper>(evb);
return std::make_unique<QuicAsyncUDPSocketWrapperImpl>(evb);
}
void TakeoverPacketHandler::processForwardedPacket(

2
quic/server/QuicSharedUDPSocketFactory.h
View File

@ -18,7 +18,7 @@ class QuicSharedUDPSocketFactory : public QuicUDPSocketFactory {
std::unique_ptr<QuicAsyncUDPSocketWrapper> make(folly::EventBase* evb, int fd)
override {
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(evb);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(evb);
if (fd != -1) {
sock->setFD(
folly::NetworkSocket::fromFd(fd),

2
quic/server/async_tran/test/QuicAsyncTransportServerTest.cpp
View File

@ -84,7 +84,7 @@ class QuicAsyncTransportServerTest : public Test {
EXPECT_CALL(clientWriteCB_, writeSuccess_()).WillOnce(Return());
clientEvb_.runInEventBaseThreadAndWait([&]() {
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(&clientEvb_);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(&clientEvb_);
auto fizzClientContext =
FizzClientQuicHandshakeContext::Builder()
.setCertificateVerifier(test::createTestCertificateVerifier())

8
quic/server/test/QuicServerTest.cpp
View File

@ -2192,7 +2192,7 @@ class QuicServerTest : public Test {
folly::SocketAddress addr2("::1", 0);
std::unique_ptr<QuicAsyncUDPSocketWrapper> client;
evbThread_.getEventBase()->runInEventBaseThreadAndWait([&] {
client = std::make_unique<QuicAsyncUDPSocketWrapper>(
client = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(
evbThread_.getEventBase());
client->bind(addr2);
});
@ -2476,7 +2476,7 @@ class QuicServerTakeoverTest : public Test {
folly::SocketAddress clientAddr("::1", 0);
std::unique_ptr<QuicAsyncUDPSocketWrapper> client;
evbThread_.getEventBase()->runInEventBaseThreadAndWait([&] {
client = std::make_unique<QuicAsyncUDPSocketWrapper>(
client = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(
evbThread_.getEventBase());
client->bind(clientAddr);
});
@ -2624,7 +2624,7 @@ struct UDPReader : public QuicAsyncUDPSocketWrapper::ReadCallback {
void start(EventBase* evb, SocketAddress addr) {
evb_ = evb;
evb_->runInEventBaseThreadAndWait([&] {
client = std::make_unique<QuicAsyncUDPSocketWrapper>(evb_);
client = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(evb_);
client->bind(addr);
client->resumeRead(this);
});
@ -3231,7 +3231,7 @@ class ServerTransportParameters : public testing::Test {
.build();
auto client = std::make_shared<QuicClientTransport>(
&evb_,
std::make_unique<QuicAsyncUDPSocketWrapper>(&evb_),
std::make_unique<QuicAsyncUDPSocketWrapperImpl>(&evb_),
std::move(fizzClientContext));
client->addNewPeerAddress(server_->getAddress());
client->setHostname("::1");

2
quic/tools/tperf/tperf.cpp
View File

@ -648,7 +648,7 @@ class TPerfClient : public quic::QuicSocket::ConnectionSetupCallback,
void start() {
folly::SocketAddress addr(host_.c_str(), port_);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapper>(&eventBase_);
auto sock = std::make_unique<QuicAsyncUDPSocketWrapperImpl>(&eventBase_);
auto fizzClientContext =
FizzClientQuicHandshakeContext::Builder()
.setCertificateVerifier(test::createTestCertificateVerifier())