Summary: This diff adds a `postwrite` method that is called at the end of each write loop invocation to allow users to customize writeloops with a before/after type of pattern. This is used for the implementation of request-cost tracking at the protocol layer as tracked in T235176826. This was previously discussed with Joseph Beshay on that task.
Reviewed By: jbeshay
Differential Revision: D84533955
fbshipit-source-id: ae1598040494c2cfc02b225972b39e6d2312bd5f
Summary:
* Added an explicit destructor to `QuicTransportBaseLite` that is the same as its derived QuicTransportBase.
* Simplified `QuicTransportBase` destructor to `= default` since the parent class now handles all cleanup.
Reviewed By: mjoras, jbeshay
Differential Revision: D84391365
fbshipit-source-id: 61034b46283c8e13eec26c473a597337e4868a17
Summary:
In mvfst, currently, keep alive frequency is 0.85 times the idle timeout.
I want to see if decoupling keep alives from idle timeout helps with long-living mobile connections like DGW where there are a lot of variables like app backgrounding, NAT rebindings etc.
I added my general thoughts in [this doc](https://docs.google.com/document/d/1WSpzsVlDsv_xQvyN0Wv-_Q2pqh7Jk4KITB3jzh6Vf6U/edit?tab=t.0#heading=h.4i12opxnysp2).
Reviewed By: jbeshay
Differential Revision: D83571470
fbshipit-source-id: 76e109e80ef94d7e42a0382346038c02f6dd8179
Summary: Remove "num non control streams" from the idle timeout error message. Instead add the value of the timeout.
Differential Revision: D83587812
fbshipit-source-id: df9de80534575ac4c255b6c4ec7447cf1764e5f1
Summary:
When path validation fails for the current path and without a valid fallback path, the path validation timeout unbinds the server transport from the worker. This can destroy the transport while the path manager is still operating on it.
Using the shared guard in the pathValidationTimeoutExpired callback prevents this from happening.
Reviewed By: kvtsoy
Differential Revision: D83602778
fbshipit-source-id: 850d5a0d584f399fc1a5699a868e02e3647527af
Summary:
This is a major change that adds support for handling path probes in the QuicServerTransport. To that end, this change:
- Incorporates the new QuicPathManager into the read and write paths of the QuicTransportBaseLite.
- Separates the connection migration logic from path probing to enable handling probe-only packets that could fail or succeed without impacting the active path.
- Tracks the path information in the outstanding packets and when updating the connection state for probing packets.
- Makes minor changes for the client transport to be able to accommodate the previous changes.
This change does NOT include:
- Client logic to initiate or handle path probes
- Server logic to rotate CIDs on migration.
Reviewed By: mjoras
Differential Revision: D79122987
fbshipit-source-id: 8db01b007b7312f51d7431cb10abb35b43d42794
Summary:
This diff is the first in the stack to add QUIC active connection migration support to Mvfst.
This adds the local address as a parameter to the functions in the read path for the client and server. This is necessary to allow the client to differentiate packets arriving on different sockets and handle them correctly. The different paths are paths being probed vs. the primary path, which is the one carrying non-probing packets.
Reviewed By: mjoras
Differential Revision: D80222873
fbshipit-source-id: 0d1ecd82b1c704280243047143ffb5b58cee4fd3
Summary:
Meta:
Special handling for MVFST_PRIMING connections on QUIC Server
* Do not write any packets back to the client
* Close connection early: as soon as data for the client is ready on a bidirectional stream -- using that as a proxy for "HTTP request stream"
* Do not bump the "connection closed with zero bytes written" counter to not add noise to current monitoring
Reviewed By: kvtsoy
Differential Revision: D79491915
fbshipit-source-id: c914968f37816d725790a12f3c1c136777c94d00
Summary:
Meta:
Fix for D68804551 comment about handling v4/v6 in QUIC 0-RTT priming.
The issue was that priming packets need to be generated ahead of time without knowing which address family (IPv4 or IPv6) they will be used on later.
Without this change we would set different length for ipv4 and ipv6
- IPv4: 1252 bytes (kDefaultV4UDPSendPacketLen)
- IPv6: 1232 bytes (kDefaultV6UDPSendPacketLen)
For priming connections, this change ensures we always use the smaller, conservative packet size (1232 bytes = kDefaultUDPSendPacketLen) that works on both IPv4 and IPv6 networks.
Used CCA mostly for the tests -- removed most of the text since it's way too verbose
Reviewed By: JunqiWang
Differential Revision: D78899669
fbshipit-source-id: 28efbb388c1b251936b019aab8f69a840de70768
Summary:
Meta:
The priming data vector vector ownerhip was not moved, that made us save duplicate packets.
For example: the onPrimingDataAvailable callbacks is currently invoked twice
* 1 - after "writing" the initial
* 2 - after "writing" the first flight of 0-RTT packets
If primingData is not moved correctly the callbacks would look like this:
1. onPrimingDataAvailable([initial])
2. onPrimingDataAvailable([initial, 0RTT packets])
so we would end up with a duplicate initial. This is why the tests were originally written with relaxed checks "CHECK_GE(packets.size(), 2)" instead of "CHECK_EQ(packets.size(),2)"
Reviewed By: mjoras
Differential Revision: D78054738
fbshipit-source-id: 5779d6b0513473c9d37416ca5686726144bd3a37
Summary: [QUIC] Make priming feature force a new QUIC version
Reviewed By: hanidamlaj
Differential Revision: D76637472
fbshipit-source-id: a242212fe46d024c0dd07cd387bc8d49c9648458
Summary:
This is an API break, but it should mostly be a manageable one. We want to be able to compile mvfst internally without exceptions, and folly::Optional is one dependency that makes this challenging. Additionally, we already have an imported secondary optional type for performance/struct size reasons, tiny-optional.
This second optional interface is mostly compatible in an API sense (including the use of std::nullopt) with std::optional. Thus our approach is to remove the dependency on folly::Optional, and offer a quic::Optional instead.
The next diff will properly vendor tiny-optional so that quic::Optional is an independent version of it.
Reviewed By: sharmafb, kvtsoy
Differential Revision: D74133131
fbshipit-source-id: 715f8bb5043ba3bb876cacfe54236887e0686b30
Summary:
When the transport is in priming mode it saves all packets instead of writing them on the wire, and feeds them to a callback for the caller to get the data.
Meta:
Priming mode is used to get all packets before 1-RTT cipher is available, in order for them to get replayed later.
Reviewed By: kvtsoy
Differential Revision: D71290230
fbshipit-source-id: 230650cb1e5901069dda4ef850c9c724bf33b6be
Summary:
Previously,
* `RawBuf` was a typealias for `std::unique_ptr<folly::IOBuf>`
* `Buf` was a typealias for `folly::IOBuf`
In this diff,
* `Buf` is a typealias for `folly::IOBuf`
* `BufPtr` is a typealias for `std::unique_ptr<folly::IOBuf>`
Reviewed By: hanidamlaj
Differential Revision: D73206576
fbshipit-source-id: 454bf6ccfce3d6571e5e931889263ed98cc24af3
Summary: More in the theme of returning Expected instead of throwing. For the folly case, we keep the try/catches in there and translate to Expected. For Libev, we convert directly to Expected.
Reviewed By: kvtsoy
Differential Revision: D73217128
fbshipit-source-id: d00a978f24e3b29a77a8ac99a19765ae49f64df8
Summary:
This adds the new priority queue implementation and a TransportSetting that controls whether it should be used or not. The default is still the old priority queue, so this diff should not introduce any functional changes in production code.
One key difference is that with the new queue, streams with new data that become connection flow control blocked are *removed* from the queue, and added back once more flow control comes. I think this will make the scheduler slightly more efficient at writing low-priority loss streams when there's high-pri data and no connection flow control, since it doesn't need to skip over those streams when building the packet.
If this diff regresses build size, D72476484 should get it back.
Reviewed By: mjoras
Differential Revision: D72476486
fbshipit-source-id: 9665cf3f66dcdbfd57d2199d5c832529a68cfac0
Summary:
Migrating mvfst priority API to be abstract, based on new classes is quic/priority. For now, it requires applications use `HTTPPriorityQueue::Priority`, to be compatible with the hardcoded `deprecated::PriorityQueue` implementation and apps cannot yet change the queue impl. Eventually the application will have full control of the queue.
There are minor functional changes in this diff:
1. Priority QLog types changed from int/bool to string
2. Any PAUSED stream has priority `u=7,i` if paused streams are disabled (previously explicitly settable to any priority)
Reviewed By: jbeshay
Differential Revision: D68696110
fbshipit-source-id: 5a4721b08248ac75d725f51b5cb3e5d5de206d86
Summary: As in title, this is more of a theme on adding an Expected return.
Reviewed By: kvtsoy
Differential Revision: D72579218
fbshipit-source-id: 25735535368838f1a4315667cd7e9e9b5df1c485
Summary: I started with the QuicStreamManager, but it turns out that the path from the manager up to the close path touches a LOT, and so this is a big diff. The strategy is basically the same everywhere, add a folly::Expected and check it on every function and enforce that with [[nodiscard]]
Reviewed By: kvtsoy
Differential Revision: D72347215
fbshipit-source-id: 452868b541754d2ecab646d6c3cbd6aacf317d7f
Summary:
Adds more logging for:
- Number of initial packets received
- Number of unique crypto frames received
- Time elapsed to get the last meaningful initial crypto frame.
- named groups, psk status, and ech status from TLS layer.
Reviewed By: mjoras
Differential Revision: D71910754
fbshipit-source-id: ea8524213ba296727079800bb167ec080143c10b
Summary:
Context: T210787480
I want to add a client-side qlog that will allow us to inspect client behavior during the 0rtt bug.
Reviewed By: jbeshay
Differential Revision: D71145234
fbshipit-source-id: 7816f0a759ba4f60107aaf40c4376ced7c5d03f8
Summary:
All according to plan: https://fburl.com/gdoc/pebccgi1
Changing function definitions to return errors while still throwing.
Reviewed By: sharmafb
Differential Revision: D69567329
fbshipit-source-id: 5d40ee32fe185d5674785632a9a13e4cef996988
Summary: I'm making a `checkpoint` function in the QUIC API. This is to be used in conjunction with reliable resets. When we send data on a stream and want to mark which offset will constitute the reliable size in a future call to resetStreamReliably, we call this function. This function can potentially be called multiple times on a stream to advance the offset, but it is an error to call it after sending a reset.
Reviewed By: jbeshay
Differential Revision: D68592906
fbshipit-source-id: e301385a04dffdb9f23daa805ee74c574e4393c2
Summary:
With reliable resets, we'll want to cancel byte events where the offset is beyond what needs to be reliably delivered. Right now, we only have the functionality to cancel all byte events, or only those up to a certain point.
I'm adding a more generic `cancelByteEventCallbacksForStreamInternal` that takes in a function that can be used to determine which byte events need to be removed.
Reviewed By: afrind
Differential Revision: D67353621
fbshipit-source-id: fa7e758ee6cd40d247392108c28118b604ad6dbc
Summary: I'm moving the implementation of `resetStream` to `resetStreamInternal` so that we can reuse a large chunk of the functionality when we implement reliable resets.
Reviewed By: kvtsoy
Differential Revision: D67303220
fbshipit-source-id: 16efce46570a0742e570d9b0b34a9477f9efbbdc
Summary: There's no need to loop through all the streams if we're just resetting one.
Reviewed By: hanidamlaj
Differential Revision: D67304631
fbshipit-source-id: 4817459ca7d1c1fd906b7640a0089c1c52e6e485
Summary: This is just some additional plumbing that we need in order to send reliable resets. It's still not hooked up end-to-end, so we can't yet send reliable resets.
Reviewed By: mjoras
Differential Revision: D66772108
fbshipit-source-id: ee189959751ed03e294c6ef418d75d8ad4959690
Summary:
setReadCallback didn't function properly during shutdown
1) it was completely ignored when state_ == CLOSING
2) cancelAllAppCallbacks made a copy of readCallbacks_
This is problematic for application constructs that use groups of streams -- eg: HTTP WebTransport. When one stream (the WebTransport session) is reset during shutdown, it needs to clean up any dependent streams as well, including preventing them from getting error callbacks.
The fix is to use only the streamId's from readCallbacksCopy for iteration, but look up the actual callback value from readCallbacks_.
Note: there's an as yet unhandled corner case which is that a readError callback installs a new stream read callback, but it seems far fetched enough I'm not including a fix here.
Reviewed By: sharmafb
Differential Revision: D48159644
fbshipit-source-id: c9d522a6b200538193969d60d242a505831e4cd0
Summary:
This re-enables the experimental pacer for bbr2 and bbr_testing, and removes fireLoopEarly by default.
FireLoopEarly can still be enabled via a knob.
Reviewed By: ritengupta, kvtsoy
Differential Revision: D65919713
fbshipit-source-id: e5d08a807be091c11326e9705601bd9e6bf79991
Summary:
When using msvc and compiling quic with C++20 there's a weird issue that the Looper is initialized with the wrong callbacks.
I suspect that this is because the constructor was creating the lambda as part of the header file.
Moving the constructor code to the cpp file fixed the issue
Reviewed By: hanidamlaj, kvtsoy
Differential Revision: D65994029
fbshipit-source-id: 9c5f47b62d0e0c74b5bce05f926e437e6aa2a3ab
Summary:
There are many files in this diff, but the relevant ones are:
* quic/api/QuicSocket.h
* quic/api/QuicSocketLite.h
* quic/api/QuicTransportBase.h
* quic/api/QuicTransportBaseLite.cpp
* quic/observer/SocketObserverContainer.h
The purpose of this is to include the observer functionality within the Lite class. Ideally, we'd remove it from the Lite class, but that's going to require some more detailed changes to the interfaces we're using, because observers are used quite a lot within common code (for example, AckHandlers.cpp).
I'll make those changes some time after the QuicTransportBase split, as they're going to take a while.
Reviewed By: jbeshay, mjoras, kvtsoy
Differential Revision: D65685667
fbshipit-source-id: feb07bcf35d6af2e5c2b538ff463b01b67c6aff9
Summary: This is pretty much the same as D65605100, which was reverted due to a cycle detection ([link](https://fb.workplace.com/groups/mobile.sheriffs/posts/27342968238658432/?comment_id=27343054638649792)). It turns out that the issue was that I didn't run `xplat/cross_plat_devx/somerge_maps/compute_merge_maps.py`.
Reviewed By: jbeshay
Differential Revision: D65761171
fbshipit-source-id: fc852ec13e54ea6ea45b4fda6b4556a78f38fc7f
