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:
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 diff changes `WriteAckFrameState::ReceivedPacket` so that it stores a complete `ReceivedUdpPacket::Timings` object, instead of just one field extracted from that object. This lets us have access to both user space and socket timestamps in our ACK RX timestamp handling code. See D48785086 for a similar change.
--
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: silver23arrow
Differential Revision: D48795108
fbshipit-source-id: 70471d2654a09cbf25e711af583c18084eb90ca0
Summary:
As title. This changes the NewTokenFrame (for writing) to hold the toke as an IOBuf instead of a string. This makes it consistent with the read side.
In the same change, the qlogger now hexlifies this buffer when writing the token to the qlog.
Reviewed By: hanidamlaj, mjoras
Differential Revision: D46955172
fbshipit-source-id: 8f5f575ad2f0a4ec3b4c01cb67defa1f4425cd74
Summary: There's no point in doing this for the DSR backend, as we don't use the frame metadata.
Reviewed By: hanidamlaj
Differential Revision: D46787595
fbshipit-source-id: 027fdc0dfe2e45b8b82d506446c8be9090cef939
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: 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:
Padding frames by definition have no semantics. There also can be quite a large number of them, which makes their storage expensive.
Optimize for this by effectively coalescing multiple contiguous padding frames into a single frame.
(Note: this ignores all push blocking failures!)
Reviewed By: kvtsoy
Differential Revision: D44636497
fbshipit-source-id: 4c1cd88123c4fdf40ec9c553885f46669f7326da
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:
- 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: Add the new IMMEDIATE_ACK frame from the ack frequency draft.
Reviewed By: mjoras
Differential Revision: D38523438
fbshipit-source-id: 79f4a26160ccf4333fb79897ab4ace2ed262fa01
Summary:
- Use varint reordering threshold (this is still being discussed, using varint until a decision is made)
- Track server's minAckDelay on client
- Track ack frequency frame sequence number
- Improve frame parsing error logs
- Add some unit tests
Reviewed By: hanidamlaj
Differential Revision: D38289108
fbshipit-source-id: 274e45115022ffd4e15b60dc57f77f1cce69bd82
Summary:
- Wrap the TokenLength in a QuicInteger
- Validate token written by server equals token parsed by client codec
Reviewed By: mjoras
Differential Revision: D33134186
fbshipit-source-id: fc003979d9bacdb9ab9bb563a300cedd7a99d58a
Summary:
- Issuing NewTokenFrames to clients, allowing them to verify their address in subsequent connections by including the token.
- add NewTokenFrame struct in the union type QuicSimpleFrame.
- Issued only once when the crypto handshake is complete.
- Testing includes validating token serialization & deserialization and asserting that the NewTokenFrame is only issued once on handshake completeness.
Reviewed By: mjoras
Differential Revision: D31673160
fbshipit-source-id: 9401ab1a4b878d8b4380d55afa531ec768f5f4cd
Summary:
These are either no longer relevant, are unlikely to be done, or are spculative enough that they don't deserve code space.
Hope here is to make our search for TODOs higher signal.
Reviewed By: lnicco
Differential Revision: D29769792
fbshipit-source-id: 7cfa62cdc15e72d8b7b0cd5dbb5913ea3ca3dc5a
Summary: This diff is the encode and decode support of Datagram frame.
Reviewed By: mjoras, yangchi
Differential Revision: D20983883
fbshipit-source-id: 1a72a87e6ce3601b71fececca872a9d20bf7820e
Summary:
For now the SendInstruction only contains stream information. One
SendInstruction is intended for one QUIC packet. The packet that eventually
will be built by backend also can only have one frame: A Stream Frame. Thus the
data length is skipped.
Reviewed By: mjoras
Differential Revision: D25985035
fbshipit-source-id: 94ef638565899893eff5b429d210234ff3f7f5cd
Summary: As in title. This doesn't actually send any frames, but implements basic support for the transport parameter and responding to the frames.
Reviewed By: yangchi
Differential Revision: D26134787
fbshipit-source-id: 2c48e01084034317c8f36f89c69d172e3cb42278
Summary:
I think this should just work without the trailing `_E`. It was added
when we mixed up our own union based variant and boost::variant. Some compiler
flags didn't like that. Now we no longer have mixed up cases, this should be
fine
Reviewed By: lnicco
Differential Revision: D25589393
fbshipit-source-id: 6430dc20f8e81af0329d89e6990c16826da168b8
Summary:
This introduces a new extension frame, the KnobFrame, and an implementation.
The idea with Knobs is that they are arbitrary key-values within a namespace that can be transmitted to the peer at any time. They provide an alternative to transport settings, by eschewing the standard transport setting space entirely.
The idea is simple, each knob has a "knobspace", "id", and value. The knobspace is a namespace in which the knob has semantic meaning (e.g. 0xfaceb00). The id and value are just that, arbitrary identifiers and values.
On receiving a knob it is the application's reponsibility to deal with it.
Reviewed By: mjoras
Differential Revision: D23601468
fbshipit-source-id: 63e5e4a7bdb76e11e8c952f1234f512a629ef348
Summary:
The problem with Ping being a simple frame:
(1) All SimpleFrames are in the same scheduler. So sending ping means we may
also send other frames which can be problematic if we send in Initial or
Handshake space
(2) Ping isn't retranmisttable. But other Simple frames are. So we are
certainly setting this wrong when we send pure Ping packet today.
That being said, there are cases where we need to treat Ping as retransmittable.
One is when it comes to update ack state: If peer sends us Ping, we may want to
Ack early rather than late. so it makes sense to treat Ping as retransmittable.
Another place is insertion into OutstandingPackets list. When our API user sends
Ping, then also add a Ping timeout. Without adding pure Ping packets into OP list,
we won't be able to track the acks to our Pings.
Reviewed By: mjoras
Differential Revision: D21763935
fbshipit-source-id: a04e97b50cf4dd4e3974320a4d2cc16eda48eef9
Summary:
Becuase when we clone an existing packet, the logic inside the current
writetStreamFrameHeader is no longer correct.
Reviewed By: mjoras
Differential Revision: D21383828
fbshipit-source-id: 8e6bbb048eefd97ca7cf17b89edc2f395f274a73
Summary: We need to internally represent them as uint64_t, in addition to parsing them as var ints.
Reviewed By: yangchi
Differential Revision: D21261466
fbshipit-source-id: 4ff7b9350cbf1dc0b58cb7ce00674430afd3ba53
Summary:
Other frames have been using the BufQueue version of frame writing.
Change this for Crypto streams as well
Reviewed By: mjoras
Differential Revision: D20947921
fbshipit-source-id: 1cfa0f3806e8d74e8c8a4864498e1c06b55d5292
Summary:
Different Builders now want to have its own way of writing or
appending write buffer. So let builders handle them.
This also add new APIs in BufWriter to copy data from IOBuf/BufQueue directly
into a destination IOBuf without cloning inbetween.
Reviewed By: mjoras
Differential Revision: D20821789
fbshipit-source-id: c0a24eb12378f64cf26c27d4232f610ed80fba84
Summary: This is without cipher dropping, but the frame is parseable and the server will send it at the correct time.
Reviewed By: yangchi, lnicco
Differential Revision: D20235013
fbshipit-source-id: 696c11ec573a530b3ed9f4185a2f6847ee08819f
Summary:
This eliminatees some tech debt by completely removing the notion of version from the core transport parameters structure and the app token for zero rtt.
Note that for the draft-27 changes we will need to temporarily re-introduce it, but to a different layer (the extension encoding itself).
Reviewed By: JunqiWang
Differential Revision: D20073578
fbshipit-source-id: 2b55af621566bf1c20e21dd17251116de1788fa0
Summary: This implements the handshake done signal and also cipher dropping.
Reviewed By: yangchi
Differential Revision: D19584922
fbshipit-source-id: a98bec8f1076393b051ff65a2d8aae7d572b42f5
Summary:
Previously we stored an `IntervalSet` in each `WriteAckFrame`. We don't need to do this, as by the time we are writing an `ACK` the `IntervalSet` is complete. Instead of bothering with the `IntervalSet` operations, we can simply serialize it to a reverse-sorted `vector.`
Additionally this has some micro-optimizations for filling the ACK frame, with a new function for getting varint size.
Reviewed By: yangchi
Differential Revision: D19397728
fbshipit-source-id: ba6958fb36a4681edaa8394b1bcbbec3472e177d
Summary: By modifying `IntervalSet` a bit we can make it so it takes a `folly::small_vector` as the container. We expect that for real traffic there will not generally be a lot of ACK blocks per frame, so optimize for that.
Reviewed By: siyengar
Differential Revision: D18919975
fbshipit-source-id: 199a2ea9ba5003382e2d7d99fc7a6de7e8aafdca
Summary: As a part of Draft 17, application close frame has been removed, we use connection close frame to represent both application close and connection close.
Reviewed By: mjoras
Differential Revision: D18580856
fbshipit-source-id: d274fa2d3dbc59b926bca5a2b8a20328ae582703
Summary: This isn't a huge source of cycles, but the flushing of the cache does show up in tperf. Turns out this is very easy to do, and exposed another minor problem which is that we were misusing `clear()` on the `IOBufQueue`. Instead we should have been using `move()` to reset it to being empty.
Reviewed By: siyengar
Differential Revision: D18758922
fbshipit-source-id: 6dcbc29e8bebd93b5787939176799e3df90ed94d
Summary:
`IOBufQueue` has some facilities for fast appending to the tail. This is not useful for us in the retransmission buffer usecase, and probably not at all. Flushing the tail cache from the `IOBufQueue` is expensive when we have to shuffle around the retransmission buffer queue on removal.
This diff replaces `IOBufQueue` with a bespoke version that only has some of the functionality.
This also changes the dependent peek APIs to use `IOBuf`s directly.
Reviewed By: siyengar, yangchi
Differential Revision: D18126437
fbshipit-source-id: a2fec0f45a72459855700c605bfd0d863a9067b7
