Summary:
This commit completely removes the DSR (Direct Server Return) feature from the codebase. DSR was a zero-copy QUIC packet transmission mechanism that allowed backend servers to directly packetize and send data to clients, bypassing the transport layer.
Components removed:
**QUIC (fbcode/quic/)**
- Deleted quic/dsr/ directory with all DSR implementation files
- Removed BufferMeta and WriteBufferMeta from StreamData
- Removed writeBufMeta() and setDSRPacketizationRequestSender() from QuicSocket
- Removed DSR tracking from stream state (dsrSender, writeBufMeta, retransmission/loss buffers)
- Removed DSR packet tracking fields (isDSRPacket, nonDsrPacketSequenceNumber)
- Removed DSR logic from loss detection, flow control, packet scheduling
- Removed TperfDSRSender tool and DSR test infrastructure
**ti/bigcache**
- Deleted ti/bigcache/dsr/ directory (16 files: Cookie, PacketizationContext, SocketProvider, etc.)
- Removed ~1000 lines of DSR code from BigCacheCommandHandler (header/impl/inline)
- Removed XSK (AF_XDP) initialization and container management from MCBigCache.cpp
- Removed DSR RPC methods: async_eb_bigcacheGetDSR, async_eb_establishPacketizationSink, async_eb_release
- Removed getDSRBigcacheValueAsync(), createDSRDelegationRequest() from McrouterCache
- Removed DSR-specific stats namespace (dsrstats) and metrics
- Cleaned up BUCK dependencies removing quic/dsr and XSK references
**proxygen HTTP infrastructure**
- Removed allowDSR() virtual method from HTTPMessageFilter base class and 50+ filter implementations
- Removed setDSRRequestSender() interface from HTTPTransaction::Transport and HTTPSink
- Removed DSRRequestSender class and sendHeadersWithDelegate() from HTTPTransaction
- Removed BufferMeta struct and all related methods from HTTPTransaction (getBufferMeta, clearBufferMeta, etc.)
- Removed bufMeta_ member and references from HQSession::HQStreamTransportBase
- Removed isDelegated_ flag and delegated transaction checks from HTTPTransaction
- Removed cache DSR methods: canDSR(), performDSRDelegateAndScheduleCallbacks(), onDelegateSuccess(), onDelegateMiss()
- Removed DSR delegation from HTTPResponseCache and McrouterCache
**Test infrastructure fixes**
- Removed quic/dsr/test:mocks dependency from proxygen session test TARGETS
- Fixed PTM test to remove nonDsrPacketSequenceNumber field references
- Cleaned up MockQuicSocketDriver to remove DSR mock infrastructure (BufferMeta, writeBufMeta)
- Removed sendHeadersWithDelegate() from HTTPSessionMocks test helpers
- Commented out DSR-specific tests in HQDownstreamSessionTest (DelegateResponse, DelegateResponseError)
- Removed MockQuicDSRRequestSender from HQSessionMocks
- Disabled BigcacheSRPacketizer build targets (depends on removed quic/dsr infrastructure)
- Fixed WebTransportFilter, CdnFilters, and other components after removing DSR infrastructure
The codebase now exclusively uses standard QUIC stream writes through pendingWrites buffers.
Reviewed By: kvtsoy
Differential Revision: D86992558
fbshipit-source-id: a3814eaf735accdce989c45da8101aac8e8c831f
Summary: We forgot to ship this for all QUIC versions. This was flagged by the external interop runner.
Reviewed By: kvtsoy
Differential Revision: D86609198
fbshipit-source-id: f6469c81072b3b6824a80d5431eb0ff094131bfd
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 changes adds a new packet scheduler only responsible for writing path challenger and path responses for a specific path. This will be used for probing and responding for probes on both the active and probing paths.
In this change, it is not connected to the transport. Probe packets are still scheduled with other simple frames. Later on this will be disabled and the PathValidationScheduler will write them.
Reviewed By: kvtsoy
Differential Revision: D81729528
fbshipit-source-id: 5c84cb78815f72adaecf61aeff85910cb4baeee4
Summary:
Multiple probes in the same write loop should not be clones of the same packet. This change enforces that.
Previously, we would end up with the same outstanding packet being cloned multiple times.
Reviewed By: zxjtan
Differential Revision: D79111386
fbshipit-source-id: b321fbcc7de31caa7e66f0c16f1880e91347f31f
Summary: Continuing the theme, mostly involved getting rid of it from the stream writes and connecting two sides of Expecteds.
Reviewed By: kvtsoy
Differential Revision: D74585096
fbshipit-source-id: d81e6cbbc09981d0f1519494cf87e93e93e892db
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:
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:
These cases were missed due to the fact that writeFrame was returning something different.
Also, revert the change in the codec for receive timestamps that was using value_or and suppressing something too large being encoded.
Reviewed By: jbeshay, kvtsoy
Differential Revision: D72904909
fbshipit-source-id: 47e415e7b12208c8a2917325ed42f51c63992687
Summary: Continuing the theme, removing it from QuicInteger which ends up being in a lot of the write paths.
Reviewed By: kvtsoy
Differential Revision: D72757026
fbshipit-source-id: 99a6ab2caea8fb495b1cb466172611002968e527
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: Add some that were missing in the codec. Also replace all FOLLY_DISCARD with [[nodiscard]] since we support it everywhere.
Reviewed By: kvtsoy
Differential Revision: D72571687
fbshipit-source-id: af296858eedcb033dcae9db1c5a3a2318e4acea7
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: The logic for deciding which ACK type to write was duplicated in QuicPacketScheduler and QuicPacketRebuilder. This refactors the logic out into a separate QuicAckScheduler so it can be tested for correction and reused in both places.
Reviewed By: sharmafb
Differential Revision: D69933311
fbshipit-source-id: e4f45688a5d258dd2a57f9f7844407f3efad5f49
Summary:
Write the new ACK_EXTENDED frame. When any of the ACK_EXTENDED features are enabled locally and supported by the peer, this frame type will take precedence over ACK_ECN and ACK_RECEIVETIMESTAMPS. See first diff in the stack for the features.
Frame format:
```
ACK_EXTENDED Frame {
Type (i) = 0xB1
// Fields of the existing ACK (type=0x02) frame:
Largest Acknowledged (i),
ACK Delay (i),
ACK Range Count (i),
First ACK Range (i),
ACK Range (..) ...,
Extended Ack Features (i),
// Optional ECN counts (if bit 0 is set in Features)
[ECN Counts (..)],
// Optional Receive Timestamps (if bit 1 is set in Features)
[Receive Timestamps (..)]
}
// Fields from the existing ACK_ECN frame
ECN Counts {
ECT0 Count (i),
ECT1 Count (i),
ECN-CE Count (i),
}
// Fields from the existing ACK_RECEIVE_TIMESTAMPS frame
Receive Timestamps {
Timestamp Range Count (i),
Timestamp Ranges (..) ...,
}
Timestamp Range {
Gap (i),
Timestamp Delta Count (i),
Timestamp Delta (i) ...,
}
```
Reviewed By: sharmafb
Differential Revision: D68931148
fbshipit-source-id: 0fb4bac23e121f82a11602daabc1ec7084db43dd
Summary: This adds a knob which puts a fixed amount of padding at the start of short header packets. This is useful to test the consequences of e.g. a larger CID or the impacts of a smaller packet size.
Reviewed By: jbeshay
Differential Revision: D69603113
fbshipit-source-id: b92ba78682eed21b7d75e38c9584a93481e2eb2f
Summary:
This replaces three separate functions for writing ACK, ACK_ECN, and ACK_RECEIVE_TIMESTAMPS with one function that can write all three.
Previously, we have two paths:
1. Default path which writes the base ACK frame and optionally adds the ECN counts if the frame type is ACK_ECN.
2. If ACK_RECEIVE_TIMESTAMPS are supported, another path would take the ART config and write that frame with the ART fields.
Path #2 does not support ECN marks.
This change consolidates ACK writing into a single path which takes ART config and frame type. It decides which fields to include based upon the type and config passed. This change does not add any new frame types but it prepares for one that can carry both ECN counts and ART fields.
Differential Revision: D68931147
fbshipit-source-id: 47b425b30f00b6c76574bc768d0ec249c60a0aa7
Summary: Restore the buggy version of `cloneAllPacketsWithCryptoFrame`, which did allow up to two packets to be cloned in the same PTO flight but would only clone them at most once, as an optional mode of operation.
Reviewed By: mjoras
Differential Revision: D67994014
fbshipit-source-id: 16f66fd48a9a4410427ba21dea6e7f6d1eee48e6
Summary: With reliable resets, we can end up in a situation where we send two reliable resets, but only one has been egressed and ACKed before the stream closes. Therefore, we need to take care to ensure that `conn_.pendingEvents.resets` doesn't contain the `streamId` for the stream being removed.
Reviewed By: kvtsoy
Differential Revision: D67108782
fbshipit-source-id: 183512a579a6882edef15579a7427f0afccb253f
Summary:
I'm modifying the `QuicPacketScheduler` so that we only send RESET_STREAM_AT frames when we've sent out all of the data upto the reliable offset. While this is not something that's mandated by the spec, it greatly simplifies the accounting of flow control if we do it this way.
In more detail, if we allowed the egressing of the RESET_STREAM_AT frame before all of the reliable data was sent out, we'd need coordination between the StreamFrameScheduler and the RstStreamScheduler, so as to ensure that we are correctly accounting for flow control. In addition to that, we'd likely have to have a "flow control accounted for" offset in each stream state, so as to ensure that we don't "double count" flow control between the two schedulers. This adds significant complexity, so we're just sending RESET_STREAM_AT frames when we've sent out all of the data upto the reliable offset.
Reviewed By: mjoras
Differential Revision: D66709346
fbshipit-source-id: 01ca8659ae80fcdbd0e599f480fde10a11f0a56b
Summary:
In CloningScheduler, in the loop for selecting a candidate packet for cloning under the "cloneAllPacketsWithCryptoFrame" mode of operation, skip a packet containing a CRYPTO frame only if both it and the most recent outstanding packet are clones of the same packet.
Otherwise, it would clone the CRYPTO packets no more than once.
Reviewed By: mjoras
Differential Revision: D64411438
fbshipit-source-id: eac9e3dbb4c6d2536b1259af08f3c9647ef06ad8
Summary:
A packet in the CloningScheduler can be in three states:
1. Not cloned yet
2. Cloned, not processed yet
3. Cloned and processed
Currently, CloningScheduler will clone a packet in state 1, moving it to state 2, and continue cloning the packet while it is in state 2 without proceeding to subsequent packets.
This change makes it so that a packet in state 2 that has a CRYPTO frame will be skipped over, so that the next packet (which will be in state 1) has a chance to also be cloned.
Extra: Fix `DoNotCloneProcessedClonedPacket` to have the first packet be the already-processed one. If the second packet is the only one already processed, then of course the first one would be scheduled, letting the test speciously pass.
Reviewed By: mjoras
Differential Revision: D62770483
fbshipit-source-id: 5b00958ab4a787615338debacbe9bd2a0f6f74a4
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:
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:
Read echoed ECN counts from incoming ACK_ECN frames.
Write ECN counts from AckState into outgoing ACK_ECN frames.
This also logs both the ECN counts in the read/writter packets in qlog.
Reviewed By: kvtsoy
Differential Revision: D54967248
fbshipit-source-id: 68b910865515271abfd1fa61fc43ce1cb12f30a7
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:
The idea here is to allow a way to do incremental stream priorities while switching between streams as aggressively.
Achieving this is somewhat tricky. The easiest place to track this is to make it so the iterator in QuicPriorityQueue no-op next() until a counter reaches an increment.
This is especially impacftul for DSR, where round robining per packet is almost pathologically bad both for CPU impact but also spurious losses and low bandwidth estimates.
Thoughts?
(Note: this ignores all push blocking failures!)
Reviewed By: kvtsoy
Differential Revision: D46268308
fbshipit-source-id: cd5b924141365f61f8a3363bc9cb38a62e5c94cf
Summary:
The write loop functions for DSR or non-DSR are segmented today. As such, so are the schedulers. Mirroring this, we also currently store the DSR and non-DSR streams in separate write queues. This makes it impossible to effectively balance between the two without potential priority inversions or starvation.
Combining them into a single queue eliminates this possibility, but is not entirely straightforward.
The main difficulty comes from the schedulers. The `StreamFrameScheduler` for non-DSR data essentially loops over the control stream queue and the normal write queue looking for the next stream to write to a given packet. When the queues are segmented things are nice and easy. When they are combined, we have to deal with the potential that the non-DSR scheduler will hit a stream with only DSR data. Simply bailing isn't quite correct, since it will just cause an empty write loop. To fix that we need check, after we are finished writing a packet, if the next scheduled stream only has DSR data. If it does, we need to ensure `hasPendingData()` returns false.
The same needs to be done in reverse for the DSR stream scheduler.
The last major compication is that we need another loop which wraps the two individual write loop functions, and calls both functions until the packet limit is exhausted or there's no more data to write. This is to handle the case where there are, for example, two active streams with the same incremental priority, and one is DSR and the other is not. In this case each write loop we want to write `packetLimit` packets, flip flopping between DSR and non DSR packets.
This kind of round robining is pathologically bad for DSR, and a future diff will experiment with changing the round robin behavior such that we write a minimum number of packets per stream before moving on to the next stream.
This change also contains some other refactors, such as eliminating `updateLossStreams` from the stream manager.
(Note: this ignores all push blocking failures!)
Reviewed By: kvtsoy
Differential Revision: D46249067
fbshipit-source-id: 56a37c02fef51908c1336266ed40ac6d99bd14d4
Summary: We shouldn't have config in the codec types. Instead solve the plumbing problem more explicitly, and only define the config in one place.
Reviewed By: jbeshay
Differential Revision: D45881730
fbshipit-source-id: fab6c967a38172f16e57a8978b10460fd196902e
Summary: The abstraction layer here was not great
Reviewed By: afrind
Differential Revision: D43427138
fbshipit-source-id: 7fb7cfe278995e58f05bda3354ef53bc407af2a2
Summary:
Fixing dependency loop introduced by D37799050 (96abc8160d)
Running `autodeps` yields the following patch:
```
--- a/xplat/quic/state/TARGETS
+++ b/xplat/quic/state/TARGETS
@@ -43,8 +43,8 @@
exported_deps = [
"//folly:random",
"//quic:constants",
+ "//quic/codec:codec",
"//quic/codec:types",
- "//quic/common:circular_deque",
"//quic/common:interval_set",
],
)
```
If this patch is applied, there is a circular dependency loop between `//quic/codec:codec` and `//quic/state:ack_states` by way of `//quic/codec:types`; this loop was introduced by D37799050 (96abc8160d).
Fixed by separating out headers files and targets. In parallel, renamed structures used for writing ACK frames (which were the reason this loop occurred) to make their role clear.
Differential Revision: D42281359
fbshipit-source-id: 8514c99f3fe72ff1d942d7f303e4a209838c7623
Summary:
We previously made the decision to disallow the writing of a FIN on a DSR stream from a non-DSR stream frame as there's not really much value in allowing it.
The code that was previously allowing this to work has a bad bug. If the DSR bytes for a stream were written completely before the non-DSR bytes had been written, the scheduling code would set `canSetFin = true`, which would cause a FIN to be written at the `bufMetaStartingOffset`. This would race with the FIN sent by the backend and, if not cause a connection error, cause an HTTP truncated body of length 0.
We can avoid this by finishing the job of not allowing DSR streams to be FIN'd by non-DSR stream frames.
Reviewed By: hanidamlaj
Differential Revision: D41479274
fbshipit-source-id: 66b321db72fdb4a16378e33048e3fb97133ced92
Summary: We need to do this to maintain consistency with the schedulers. Prior to this fix when we ran out of connection flow control and had lost DSR data the non-DSR stream scheduler would do an empty write _and_ the DSR stream scheduler would do an empty write. To fully resolve the issue we need to track the streams separately (note that the same stream can be in both sets).
Reviewed By: jbeshay
Differential Revision: D40003361
fbshipit-source-id: fd3e567bc8a2bc8b71d4c4543894053fa6e24dc4
Summary:
When 0-rtt is used and an initial packet is lost, the retransmission of this packet may end up shorter than allowed because the padding frames are subject to the cwnd.
This change makes the padding of initial packets not subject to the cwnd.
Reviewed By: mjoras
Differential Revision: D39594390
fbshipit-source-id: 2d714c921f243f8a59577a6edaaeaa1c7e2be815
