Summary:
Previously, we would continue to respond with the minimum size
stateless reset packet.
Reviewed By: jbeshay
Differential Revision: D55346064
fbshipit-source-id: 71d71cfa279b0a96e63c4cea8927387e18c0d1c3
Summary:
`-Wextra-semi` or `-Wextra-semi-stmt`
If the code compiles, this is safe to land.
Reviewed By: palmje
Differential Revision: D54362219
fbshipit-source-id: 70c9403e5faf25788e5057d4abec1bbe87de73ec
Summary: - as title, integrity is fixed length (16 bytes) and does not need to be an IOBuf
Reviewed By: knekritz, mjoras
Differential Revision: D54392008
fbshipit-source-id: dde1ac87d1355c389b805060e2080eae1d77ab87
Summary:
The current open connection counting is broken. To fix it this change:
- moves the counter to be for both the server and client transport
- calls onNewConnection() when the transport is ready.
- calls onConnectionClose() when the transport is closed after it was ready.
Reviewed By: lnicco, frankfeir
Differential Revision: D52638628
fbshipit-source-id: bcff7a8c671f8eede53c22e698e1b95332c56959
Summary:
- there's no need to publicly expose QuicServerWorker::WorkerCallback overrides in QuicServer
- modified `::routeDataToWorker` function signature to take an additional EventBase argument, set to the EventBase of the QuicServerWorker that is invoking the cb function
- additional argument used in the next diff to fetch the associated worker with the eventbase
Reviewed By: sharmafb
Differential Revision: D52435341
fbshipit-source-id: 779cf3aba86f408dbb746aa0a573d0618d7e4cb4
Summary: Breaking these two types apart in D51413481 and using them separately for the client and server resulted in the TimerFD not being used anymore for non-mobile clients even when it's available. The combines them back into one type and restore the compile-time selection to use the TimerFD when not on mobile.
Reviewed By: sharmafb
Differential Revision: D52570625
fbshipit-source-id: 3f6117d67558f4506f9fe9efd9d0ce0ad2efbbbc
Summary:
Rather than having a separate copy of applySocketOptions inside QuicServerWorker to work on FollyAsyncUDPSocketAlias (folly::AsyncUDPSocket), this templatizes the SocketUtil function so it can be used with both types and avoid duplication of the socket option filtering logic.
Since the header file only had one other one-line function, I moved that function into the header file as well and removed the SocketUtil.cpp file.
Reviewed By: mjoras, frankfeir
Differential Revision: D52182467
fbshipit-source-id: 7c8aff164edafb12b26746f766ac6e5fc20c88d5
Summary:
This is the major transition that updates mvfst code to use the new interfaces. The new Folly implementations of the interfaces maintain all the existing behavior of folly types so this should not introduce any functional change. The core changes are:
- Update the BatchWriters to use the new interfaces.
- Update the FunctionLooper to use the new interfaces.
- Change QuicServerTransport to take the folly types and wrap them in the new types for use in the QuicTransportBase.
The rest of the diff is for updating all the existing uses of the QuicTrasnport to initialize the necessary types and pass them to the QUIC transport instead of directly passing folly types.
Reviewed By: mjoras
Differential Revision: D51413481
fbshipit-source-id: 5ed607e12b9a52b96148ad9b4f8f43899655d936
Summary: Add visibility into the number of handshakes. We are already getting some partial visibility unintentionally through SLB_downstream_idle_conn.sum, but that metric includes actual idle connections and I need an explicit metric to set up rate limiting thresholds correctly.
Reviewed By: dddmello
Differential Revision: D50760028
fbshipit-source-id: 5c9063f80f7863b0b1a3e4d4c94c7b2207596c1f
Summary:
This diff:
1. Introduces a new `ReceivedPacket::Timings` structure, which will be expanded upon in subsequent diffs.
2. Adds a `ReceivedPacket::Timings` field to each `ReceivedPacket`
3. Uses the accessors added in the previous diff (D48724715) to populate the `ReceivedPacket::Timings` structure in each `ReceivedPacket` held by a `NetworkData` object. This is done by propagating the `NetworkData::receiveTimePoint` field to all `ReceivedPacket` held in a `NetworkData.` This propagation occurs each time a `ReceivedPacket` is added. The value is propagated again if the `NetworkData::receiveTimePoint` field is updated by looping over all previously added `ReceivedPacket`.
--
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
Differential Revision: D48725209
fbshipit-source-id: 580e7d7d1f3587f9947774b5ed19e9985df404c9
Summary:
This diff makes the fields in `NetworkData` private so that they can only be manipulated via accessors.
- This will be used in a subsequent diff to make it possible to populate a new `ReceivedPacket::Timings` structure in each `ReceivedPacket` held by a `NetworkData` object with the timing information currently held in `NetworkData.`
- These accessors are temporary and being used to split up the stack, so relying on existing tests to provide coverage instead of adding new tests here.
--
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
Differential Revision: D48724715
fbshipit-source-id: 0230ea3feea525fa15b908161f444f4f6d9d4b39
Summary:
**Overall Context**
Want to safely experiment with resuming client initial CWND at a value that is right for the user. To start, we want them to send us their last known CWND on connection start.
**In this diff**
Add a flag for `useCwndHintsInSessionTicket` (to be used later) and map both params to `MVFST_EXPERIMENTAL2`.
Reviewed By: jbeshay
Differential Revision: D50711975
fbshipit-source-id: 27790e4453ddc73a728596d10438a70de60f87c3
Summary:
This diff changes `QuicAsyncUDPSocketWrapper` so that it is an abstraction layer that inherits from `QuicAsyncUDPSocketType`, instead of simply being a container with aliases.
- Key changes in `QuicAsyncUDPSocketWrapper.h`, the rest of the updates switch us from using `QuicAsyncUDPSocketType` to `QuicAsyncUDPSocketWrapper`.
- It's difficult to mock the UDP socket today given that we expose the entire `folly::AsyncUDPSocket` type to the higher layers of the QUIC stack. This complicates testing and emulation because any mock / fake has to implement low level primitives like `recvmmsg`, and because the `folly::AsyncUDPSocket` interface can change over time.
- Pure virtual functions will be defined in `QuicAsyncUDPSocketWrapper` in a follow up diff to start creating an interface between the higher layers of the mvfst QUIC stack and the UDP socket, and this interface will abstract away lower layer details such as `cmsgs` and `io_vec`, and instead focus on populating higher layer structures such as `NetworkData` and `ReceivedPacket` (D48714615). This will make it easier for us to mock or fake the UDP socket.
This diff relies on changes to `folly::MockAsyncUDPSocket` introduced in D48717389.
--
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, hanidamlaj
Differential Revision: D48717388
fbshipit-source-id: 4f34182a69ab1e619e454da19e357a6a2ee2b9ab
Summary:
This diff:
- Changes `NetworkData` to have `vector<ReceivedPacket>` instead of `vector<IOBuf>` to make it easier to associate metadata with individual UDP packets.
--
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: mjoras
Differential Revision: D48714615
fbshipit-source-id: b21a88f14156ed6398308223f50b2334328586ee
Summary: We intend to use this feature. The socket stores whether it is successful and can be queried later via getTXTime
Reviewed By: kvtsoy
Differential Revision: D48531750
fbshipit-source-id: 15da7eb5f3c499a82a77f13610c8f1d95736d066
Summary: This is needed for the new socket abstraction implementation later.
Reviewed By: jbeshay, lnicco
Differential Revision: D46669712
fbshipit-source-id: 5adde6679386689e7f63992ed769ff4a777f59e3
Summary: we can elide explicitly setting `isUsingClientConnId` member variable in RoutingData since it can be derived from logically OR'ing two other fields: isUsing0Rtt and isInitial
Reviewed By: mjoras
Differential Revision: D46189397
fbshipit-source-id: 69a3fdbc90712cc928b5202975782d88370e043d
Summary:
The pacingTimerTickInterval transport setting conflates two options: the minimum interval a pacer can use, and the resolution of the underlying timer. This means that a higher value leads to lower timer resolution and less pacing accuracy.
This change adds a separate parameter for the timer resolution to ensure that a larger pacing tick does not degrade the pacer accuracy.
Reviewed By: mjoras
Differential Revision: D46564066
fbshipit-source-id: 0d0e54077b80da03e6e6c9baaab49a4c969966b6
Summary: We don't use it, and the OSS lib hasn't been updated in a while.
Reviewed By: mjoras
Differential Revision: D46707559
fbshipit-source-id: ec102a52183a736cfb1c0241600816a837062108
Summary: There's no point in starting the timer until there's connections.
Reviewed By: kvtsoy
Differential Revision: D46753434
fbshipit-source-id: ff7c2fc76ddbbea0d672f59e6571c96dbc850254
Summary:
The idea here is to add a notion of time-based sampling of certain QUIC_STATS. This allows accounting to be done via consistent distributions for comparisons.
For now limit to the server, and only implement for inflight bytes, SRTT, and CCA bandwidth.
Reviewed By: jbeshay
Differential Revision: D46410903
fbshipit-source-id: a5db1ec720a0f8bf54e04d66c0d68686660e8eaa
Summary: We have VLOG(3) for various dropped packet reasons, adding VLOG(3) for CANNOT_FORWARD_DATA and CONNECTION_NOT_FOUND cases.
Reviewed By: mjoras
Differential Revision: D43632936
fbshipit-source-id: 81c3eb69c6d84a9224646ded7042963af3f9b130
Summary: Add the socket create hook to QuicServerWorker::bind in order for more applications to call this hook function.
Reviewed By: jbeshay
Differential Revision: D43130289
fbshipit-source-id: 76bdb2673eb51d11d216ca80c83aafeda86a21c4
Summary: The existing PacketDropReason values cover many branches in the code making it impossible to isolate the reason for a PARSE_ERROR, INVALID_PACKET, CONNECTION_NOT_FOUND. This change breaks them down into more values that are each used in a single branch.
Reviewed By: mjoras
Differential Revision: D39149490
fbshipit-source-id: 28cbe1ea6c4a06cf55960058edaa48c28ed4d2ef
Summary:
- continually issuing new connection ids to peer as old connections ids are retired through RETIRE_CONN_ID frames
- add logic to parse and act on receiving RETIRE_CONN_ID frame
Reviewed By: mjoras
Differential Revision: D38443561
fbshipit-source-id: 82fb679f482fd69c7b3a3385693d2e5575e92703
Summary: Returning nullptr indicates that it cannot support making a transport at the moment, so respond with a VN as a terminal signal.
Reviewed By: kvtsoy
Differential Revision: D37014231
fbshipit-source-id: e9905a97709cfcdb75d757b11258711c110077e9
Summary: Upgrading glog from 0.4.0 to 0.5.0 broke the windows build for some time. This change skips calling LOG_EVERY_N for Windows to restore the build. It is a stop-gap measure until logging is migrated to folly XLOG.
Reviewed By: kvtsoy
Differential Revision: D38371427
fbshipit-source-id: 9711607a348f0473e3e922d7f627217b3948c45d
Summary: Although there is no flaw in this code, the static analysis does not see that the remaining variable controls the flow path and considers the use of the data after move a failure.
Reviewed By: mjoras
Differential Revision: D37321326
fbshipit-source-id: 54c147f4e9840bd7e5c7a6122495be66044c7708
