Summary:
All the functionality is a duplicated from `updateWritableStreams`.
Btw the only callsites were inside UT.
Reviewed By: kvtsoy
Differential Revision: D68351972
fbshipit-source-id: 7e17f38ffcffecea23a64f5d2d5c1dec7a8c43f5
Summary:
Concatenate adjacent namespaces + format
This is a codemod. It was automatically generated and will be landed once it is approved and tests are passing in sandcastle.
You have been added as a reviewer by Sentinel or Butterfly.
Autodiff project: nc
Autodiff partition: fbcode.quic
Autodiff bookmark: ad.nc.fbcode.quic
Reviewed By: hanidamlaj
Differential Revision: D65365244
fbshipit-source-id: 0bbaa7684d03caf8fc8eff3439a0865940398220
Summary: In `writeCryptoAndAckDataToSocket`, add an additional `writeProbingDataToSocket` call at the end that is limited to the number of CRYPTO frame-containing packets just written, gated by the new `TransportSetttings` field `immediatelyRetransmitInitialPackets`.
Reviewed By: mjoras
Differential Revision: D64485616
fbshipit-source-id: f0927a3796767700fd46673195e1cd4e1bbbcbeb
Summary: This kinda makes sense to just use as a default.
Reviewed By: kvtsoy, sharmafb
Differential Revision: D64066392
fbshipit-source-id: 0915f163c0483af6bec014bde61e82b6ee2ac6cb
Summary: As in title. Without this the keepalive option is less effective since a single PING packet loss can cause issues.
Reviewed By: kvtsoy
Differential Revision: D63397494
fbshipit-source-id: 7ef6b6f54189609e3a96409ac9c035c475dc0569
Summary: This is my second attempt at D61871891. This time, I ran `xplat/cross_plat_devx/somerge_maps/compute_merge_maps.py`, which generated quic/somerge_defs.bzl
Reviewed By: kvtsoy
Differential Revision: D61975459
fbshipit-source-id: bec62acb2b400f4a102574e8c882927f41b9330e
Summary: `PacketEvent` is a very inaccurate and misleading name. We're basically using this as an identifier for cloned packets, so `ClonedPacketIdentifier` is a much better.
Reviewed By: kvtsoy
Differential Revision: D61871891
fbshipit-source-id: f9c626d900c8b7ab7e231c9bad4c1629384ebb77
Summary:
**Context**
The `BufAccessor` is used to access a contiguous section of memory. Right now, it works with a `Buf` under the hood.
**Overall plan**
The plan is to change the `BufAccessor` to use a `uint8_t*` instead. Since we're using section of contiguous memory, there's no need to use a chained buffer abstraction here. This'll move us closer to deprecating the usage `folly::IOBuf`.
**What this diff is doing**
Most use cases of the `BufAccessor` look like the following:
```
auto buf = bufAccessor.obtain();
// Do something with buf, like calling trimEnd
bufAccessor.release(buf)
```
I'm adding APIs to the `BufAccessor` so that there's no need to `obtain()` and `release()` the `Buf`. We'd instead just call an API on the `BufAccessor`, which would call that same API on the underlying `folly::IOBuf`. Later on, we'll change the `BufAccessor` to use a `uint8_t*` under the hood.
I'm currently leaving in the `obtain()`, `release()`, and `buf()` APIs because Fizz and the AsyncUDPSocket expect `folly::IOBuf` as inputs in many of their APIs. Once those callsites are migrated off `folly::IOBuf`, we can remove these APIs.
Reviewed By: mjoras
Differential Revision: D60973166
fbshipit-source-id: 52aa3541d0c4878c7ee8525d70ac280508b61e24
Summary: We have a lot of optionals that are either integral values or std::chrono::microseconds. These end up wasting memory, where we can instead store sentinel values to encode whether the value is there or not. This reduces the effective range of the type by one value, but that is an acceptable tradeoff.
Reviewed By: kvtsoy
Differential Revision: D57684368
fbshipit-source-id: b406b86011f9b8169b6e5e925265f4829928cc63
Summary:
The idea here is to make it so we can swap out the type we are using for optionality. In the near term we are going to try swapping towards one that more aggressively tries to save size.
For now there is no functional change and this is just a big aliasing diff.
Reviewed By: sharmafb
Differential Revision: D57633896
fbshipit-source-id: 6eae5953d47395b390016e59cf9d639f3b6c8cfe
Summary: This is effectively an unused field encoding duplicated information, but it was widespread.
Reviewed By: kvtsoy
Differential Revision: D57289922
fbshipit-source-id: ca1499e2576e5ae28e3880b865a29c2b8d9a3d1b
Summary: I want to have visibility in to the current count of streams. There are already callback methods onNew/onClosed, unfortunately, the latter is not called when connections are closed, so I had to fix that to prevent the metric from uncontrolled growth. I am still not positive it's correct and was thinking of hooking into QuicStreamState destructor instead.
Reviewed By: kvtsoy
Differential Revision: D56195487
fbshipit-source-id: 901bce9473327a8f7ef1bc0f70af10899784d681
Summary: This reduces the number of stats callbacks when processing multiple packets in rapid succession.
Reviewed By: mjoras
Differential Revision: D56315022
fbshipit-source-id: 750024301f28b21e3125c144ead6f115706736a4
Summary: Neither QUIC nor TransportMonitor uses the `finObserved` field in the `StreamDetails`
Reviewed By: hanidamlaj
Differential Revision: D55936040
fbshipit-source-id: 0902bf7c30f85ada63cd3e9234f6a059b425cce5
Summary: Neither QUIC not TransportMonitor is using the `packetsInflight` field of the `OutstandingPacketMetadata`
Reviewed By: hanidamlaj
Differential Revision: D55926288
fbshipit-source-id: 32efd66add1e6374a8d3446ff635fe582b36e644
Summary: Neither QUIC not TransportMonitor is using the `totalBodyBytesSent` field of the `OutstandingPacketMetadata`
Reviewed By: hanidamlaj
Differential Revision: D55897240
fbshipit-source-id: 521f8a016f838dd1fe0593daa7a4e45c4fd222cf
Summary:
This stack adds key update support to Mvfst client and server. This diff adds the main logic for detecting key updates in the QuicReadCodec. When an update is successful, the server transport reacts to it by updating the write phase and cipher.
The high level design is as follows:
- The QuicReadCodec is responsible for detecting incoming key update attempts by the peer, as well as tracking any ongoing locally-initiated key updates.
- Upon detecting a successful key update, the QuicReadCodec updates its state. The Server/Client transport reacts to this change by updating its write phase and cipher.
- A locally initiated key update starts with updating the write phase and key, and signaling the read codec that a key update has been initiated.
- The read codec keeps this in a pending state until a packet is successfully received in the new phase.
- Functions for syncing the read/write phase on incoming key updates, as well as initiating and verifying outgoing key updates are abstracted in QuicTransportFunctions and are used by both the client and server transports.
- Common handshake functions used for rotating the keys are now in HandshakeLayer that is shared by both client and server handshakes.
Reviewed By: mjoras
Differential Revision: D53016559
fbshipit-source-id: 134e965dabd62917193544a9655a4eb8868ab7f8
Summary: This is similar to the previous commit. We use a stack-based IOBuf instead of allocating one on the heap. This saves CPU because allocations/deallocations on the stack are a lot cheaper.
Reviewed By: hanidamlaj
Differential Revision: D53101722
fbshipit-source-id: dd59a7eca6498db19472a62f954db3e2f2f27a42
Summary: Creating an IOBuf on the heap when we use `folly::IOBuf::wrapBuffer` is expensive.
Reviewed By: hanidamlaj
Differential Revision: D52506216
fbshipit-source-id: eed2b77beae0419b542b0461303785cc175e3518
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:
- Remove setCustomTransportParameter, which (based on the quic v19 rfc), verifies whether a parameter is within the private range [0xff00, 0xffff]
> Values with the first byte in the range 0x00 to 0xfe (in hexadecimal) are assigned via the Specification Required policy [RFC8126].
- Consolidating adding MaxStreamGroups transport parameter into all other transport parameters extension.
More specifically, `QuicClientTransport::maybeEnableStreamGroups()` logic is now moved into `QuicClientTransport::setSupportedExtensionTransportParameters()`
Reviewed By: mjoras
Differential Revision: D50461610
fbshipit-source-id: 802b546c8364586cdcf36a230b156ca140c57ce4
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 limits the writableBytes by the available tokens (ie `ThrottlingSignalProvider::bytesToSend`), if the connection is being throttled and that value is provided by the throttling signal provider.
Note that these throttling signals to the transport are provided only for the connections that belong to a specific QE experiment.
Reviewed By: silver23arrow
Differential Revision: D47850261
fbshipit-source-id: 8c52e66198db6d1fee252cacea69b82963a1601a
Summary: There's no need to call write when there's only one packet, since write just wraps writeGSO. This simplifies the use of options and will allow for setting a TXTIME when there's only one packet to send.
Reviewed By: kvtsoy
Differential Revision: D48526018
fbshipit-source-id: d934ddce8d3a35febb58ff253fc7a9bed3ef975c
Summary: Update flow control settings names to reflect that these are indeed flow control
Reviewed By: jbeshay
Differential Revision: D48137685
fbshipit-source-id: a48372e21cdd529480e25785a9bd5de456427ef3
Summary: Use WriteOptions instead of a single int to allow for txTime setting too
Reviewed By: mjoras
Differential Revision: D48218906
fbshipit-source-id: 00a1d4905b54ec0614860f9cdd8b58c9d6e6ef9a
Summary: Turns out these functions have been returning 0 for bytes written for a while. Nothing was using them (yet), so there wasn't any functional breakage.
Reviewed By: kvtsoy
Differential Revision: D46653336
fbshipit-source-id: 765b3363c1fc0729e8239d1293ddcc4ae4bb9c79
Summary:
As in title.
(Note: this ignores all push blocking failures!)
Reviewed By: hanidamlaj, lnicco
Differential Revision: D46111136
fbshipit-source-id: b5aec9f1daa32acd200dd1550eb0bb3a9a44a4f2
Summary:
This has been hardcoded to SRTT/25 for a long time. However, especially when using DSR this might not be the most appropriate since it can start to get close to real world SRTTs.
Control it via a knob, and also add the behavior such that setting it to 0 effectively disables the time limit.
Reviewed By: jbeshay
Differential Revision: D46084438
fbshipit-source-id: 7dc619fdff1bd5c3f8654c4936200e0340ef94f2
Summary:
When using ACK_FREQUENCY and high reordering thresholds, the normal tricks for drawing ACKs with PTOs are slightly defeated.
Thankfully we can get around this by issuing IMMEDIATE_ACKs when probing. The strategy is basically to replace one of the usual probes with an IMMEDIATE_ACK probe.
Also, include ACKs in these probes (and the PING probes), since there's not much reason not to.
Reviewed By: jbeshay
Differential Revision: D45965703
fbshipit-source-id: 9b98473312a239e17a0b8040f45d197b1a2c8484
Summary:
To get reliable packet destruction events, created a `OutstandingPacketWrapper` wrapper that wraps the current `OutstandingPacket` class, so callback functions can be added to the wrapper instead of the underlying object itself.
#### Why do we need this wrapper?
`std::deque::erase` does not guarantee that appropriate object destructors will be called (only that the number of destructions = number of objects erased). This is a real problem as packet destruction events are then no longer reliable (OutstandingPacket object is wrong!). The wrapper class handles this condition by detecting it in the move assignment constructor (called during erase) and calls the appropriate packet destruction callback before packets are moved. If we did the same fix in the old OutstandingPacket, we have to make sure the callback is called before all the fields of OutstandingPacket are moved - this is not scaleable. Hence a wrapper with the underlying object and a destruction callback function.
I also disabled copy construction for OutstandingPacket (otherwise we will get duplicate OnPacketDestroyed callbacks and cannot track packets reliably). Removing packet copies also improves performance. Some code changes (in tests mostly) are mostly in service of this particular change.
Reviewed By: bschlinker
Differential Revision: D43896148
fbshipit-source-id: c295d3c4dba2368aa66f06df5fc82b473a03fb4d
Summary:
LLVM-15 has a warning `-Wunused-but-set-variable` which we treat as an error because it's so often diagnostic of a code issue. Unused variables can compromise readability or, worse, performance.
This diff either (a) removes an unused variable and, possibly, it's associated code, or (b) qualifies the variable with `[[maybe_unused]]`, mostly in cases where the variable _is_ used, but, eg, in an `assert` statement that isn't present in production code.
- If you approve of this diff, please use the "Accept & Ship" button :-)
Reviewed By: luciang
Differential Revision: D42465115
fbshipit-source-id: 0379c82451b1902c185bc3b083c7ee68264b8977
Summary:
We don't need to carry these states after the handshake is confirmed, so make them pointers instead. This will facilitate adding a structure to the AckState for tracking duplicate packets.
(Note: this ignores all push blocking failures!)
Reviewed By: hanidamlaj
Differential Revision: D41626895
fbshipit-source-id: d8ac960b3672b9bb9adaaececa53a1203ec801e0
Summary:
Track the total (cumulative) amount of time that the connection has been application limited and store this information in `OutstandingPacketMetadata`. If this value is the same for two `OutstandingPacketMetadata` then we know that the transport did not become application limited between when those two packets were sent (or, less likely, the transport was application limited for less than one microsecond given the microsecond resolution of the timestamp).
We store the amount of time spent application limited instead of a count of the number of application limited events because the implications of being application limited are time dependent.
Tests show that we need to be able to inject a mockable clock. That's been an issue for some time; will work on in a subsequent diff.
Differential Revision: D41714879
fbshipit-source-id: 9fd4fe321d85639dc9fb5c2cd51713c481cbeb22
Summary: The code has changed since this was made an inlined vector. We now see higher cost from copy operations due to this rather than being able to move the data around.
Reviewed By: bschlinker
Differential Revision: D36151209
fbshipit-source-id: 0b10558c6bd8ebfea9bb960aac36a6c4044fc95f
Summary:
- PTOs should not be subject to congestion control limits
- Quickly recover from PTOs being writableBytesLimited by calling onPTOAlarm() as soon as we become unblocked
Reviewed By: mjoras
Differential Revision: D35480409
fbshipit-source-id: 51500db6fff17a7badefea8bda7f63141e97f746
