Summary:
This change just adds a (currently no-op) flag that will be used in diffs up the stack.
The idea here is that I'll add split QUIC connection callback interfaces that will live side by side with existing single monolithic callback for now. We will experiment with split callbacks on small scale to see that there is no regressions and then will phase out the old callback gradually.
This flag is to control which callback(s) to use.
Reviewed By: mjoras
Differential Revision: D30399667
fbshipit-source-id: 8fc4e4a005e93cf6d48a987f49edee33b90dbbf1
Summary:
- Removed packetNum field from CipherUnavailable struct.
- Removed all instances referring to the field and fixed tests accordingly.
Reviewed By: mjoras
Differential Revision: D29968168
fbshipit-source-id: 9802b8cd66f43f2a8d54340f2d00639ee4679aaf
Summary: This doesn't belong in the generic state. Untangling it is a little difficult, but I think this solution is cleaner than having it in the generic state.
Reviewed By: JunqiWang
Differential Revision: D29856391
fbshipit-source-id: 1042109ed29cd1d20d139e08548d187b469c8398
Summary: As in title, this doesn't need to be in the base state.
Reviewed By: JunqiWang
Differential Revision: D29855140
fbshipit-source-id: 8d3a4b12fd6b93b2277020d56862915e084f1c05
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:
If some 0RTT packets are lost we will not detect the loss until we get a short header ACK from the server or the loss timeout expires. This could potentially take a long time.
This adds an option which will optionally retransmit any 0RTT data as soon as the handshake is complete.
Reviewed By: lnicco
Differential Revision: D29111647
fbshipit-source-id: 3c1924ce178a01eaa20a94561df82a59733b8b71
Summary: Prior to this any socket level error message would kill the connection. This doesn't make sense when happy eyeballs is ongoing.
Reviewed By: jbeshay
Differential Revision: D28979505
fbshipit-source-id: 29d1a3b3f0db56a073433a04b888241fe7b91767
Summary: Without this we won't actually retransmit the 0RTT data on the second socket until they are declared lost after the handshake is done.
Reviewed By: JunqiWang
Differential Revision: D28942340
fbshipit-source-id: 192c9ca0544bfde9610720ca1b02322709f0e61e
Summary: As in title. The socket delivering errors this way is probably a bad sign.
Reviewed By: yangchi
Differential Revision: D28432809
fbshipit-source-id: 8c7fcbd4f6a9aa58fb19e73f489bd321dde76761
Summary: This diff is the encode and decode support of Datagram frame.
Reviewed By: mjoras, yangchi
Differential Revision: D20983883
fbshipit-source-id: 1a72a87e6ce3601b71fececca872a9d20bf7820e
Summary: as title, this is a testing only API
Reviewed By: mjoras
Differential Revision: D27746039
fbshipit-source-id: cf87836810a4579f622152ccb17aca49a0d605e3
Summary:
This is another bandaid around our incorrect not updating of 0-RTT transport parameters on the client. The long term fix requires us to update all the parameters based on the final ones from the server. This will require a bit of a refactor to this area of the code as well.
Until we do that, manually set the stateless reset token so that 0-RTT clients can actually parse server stateless resets.
Reviewed By: yangchi
Differential Revision: D27875279
fbshipit-source-id: 1d08a53f2a9d876635dcd884b6bba316224543dd
Summary: Keep on server for now but disallow it in code for the client.
Reviewed By: yangchi
Differential Revision: D27726584
fbshipit-source-id: c567d9db82c36b6e60d438d839709f0330b8db50
Summary: Right now we are running the handshakeConfirmed code a lot on the client. This is excessive. We only need to run the code if we haven't already dropped the cipher.
Reviewed By: yangchi
Differential Revision: D27725974
fbshipit-source-id: ca325c132debdd280e447ca30876488b879ff13c
Summary:
This can happen when we don't get the server handshake data in time, but it is especially bad with 0RTT when it is potentially a full flight of 1RTT data that is dropped while we wait for the handshake PTO.
Note this leverages the existing CipherUnavailable mechanism, but processes them in a much more simple way than the server side. Additionally, only 1-RTT packets need to be buffered.
Reviewed By: yangchi, lnicco
Differential Revision: D27634184
fbshipit-source-id: db5ba0b9f07176d106f709c7a11d83d0fc8281b7
Summary:
On receiving a QUIC packet, if the packet has no frames we should end the connection with PROTOCOL_VIOLATION.
This fixes the error reported by h3spec test `/QUIC servers/MUST send PROTOCOL_VIOLATION on no frames [Transport 12.4]/`
This change adds the check right after a packet is successfully parsed for both the client and server.
Reviewed By: mjoras
Differential Revision: D27483874
fbshipit-source-id: 9b648709e6985f151ba0ffc973aa05c28683fbe9
Summary:
As before we will now aggressively send probes on all spaces with probes available when the PTO timer fires.
This time with more unit tests and some bug fixes.
Reviewed By: yangchi
Differential Revision: D27338523
fbshipit-source-id: 8a9ccb90ed691e996fab4afa2f132c0f99044fbc
Summary: As in title. There's a bug here somewhere with empty write loops we need to find.
Reviewed By: yangchi
Differential Revision: D27279100
fbshipit-source-id: e1d26fbf8d6df1590d464a6504a8b940b46794e0
Summary:
Previously we would only send probes for the first space which had one available, i.e. Initial before Handshake before AppData. Since we only have one PTO timer this can lead to situations where we perpetually probe with only Initials, which can significantly delay the handshake if we should have probed with Handshakes.
With this diff we will keep the single PTO timer but aggressively write more probes from all spaces if they are available.
Additionally this refactors some counters into EnumArrays
Reviewed By: yangchi
Differential Revision: D27235199
fbshipit-source-id: ef3614a833bf0f02f5806846a1335fa7ac2a4dc8
Summary:
From what I can tell this looks like a bug in recvMesg where we weren't calling QUIC_STATS on packet received for recvMesg. Not completely sure what I'm doing here but seems possibly legit.
When I started using a pool in D25017895 that had quic_config "should_recv_batch" on, my canaries showed no data for packets received. This seems to be the cause but I'm not sure if this is intentional or just something missed.
Some more context here: https://fb.workplace.com/groups/1528424050798314/permalink/2439895836317793/
Reviewed By: lnicco
Differential Revision: D26407505
fbshipit-source-id: d80fb6473bd23af0100cb56fb66fe313dcbf60fa
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:
We were using the LifecycleObserver and InstrumentationObserver classes
separately, to generate and receive callbacks.
This change migrates both these to use the unified Observer callback class and
adjusts the unit tests.
Reviewed By: bschlinker
Differential Revision: D25845845
fbshipit-source-id: c489400f5d70bccadbcc1d957136c5ade36b65ff
Summary:
(1) Receiving a valid retry packet is a HappyEyeballs signal. We should
use that to update the happy eyeballs state.
(2) HappyEyeballs state should also be preserved when we undo client connection
state for retry.
Reviewed By: mjoras
Differential Revision: D25728713
fbshipit-source-id: 4ff06879f5a05e6fb4faeb1e9f330e251d3dbcb6
Summary:
If we are still doing happyeyeballs, and receives packet, the main
socket has to be present whatever the situation is, and the second socket has
to be present if we claim the second one wins
Reviewed By: lnicco
Differential Revision: D25726770
fbshipit-source-id: 17ba616dde9affed6a2bcba8d64e4e0a4e8ac9b0
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:
The test case relies on the client transport hits max PTO within 10s
to trigger a read error callback. Sometimes, it takes longer than 10s to hit
max PTO under stress runs
Reviewed By: mjoras
Differential Revision: D25065653
fbshipit-source-id: c728084178126452d0c7c885cb7215878cf4e3b7
Summary:
When I tested against picoquic, I found that an exception was thrown here: https://fburl.com/diffusion/qsnzingx.
The reason is that we modify `conn.initialDestinationConnectionId` in the event of a retry, so it's no longer the original destination connection id. By separately keeping track of the original destination connection id, we can solve this issue.
Reviewed By: mjoras
Differential Revision: D23722127
fbshipit-source-id: 94be08812e675feaf46f5af86e7304af84c1910c
Summary: We need to increase those parameter Ids above 0xFF00, and xmit them.
Reviewed By: mjoras
Differential Revision: D24115117
fbshipit-source-id: cf8b4970ba8640a83bde22c1f9fdfa1d3e53e3f9
Summary: Since the DCID is often empty for clients, this is otherwise not a very useful field.
Reviewed By: yangchi
Differential Revision: D23998639
fbshipit-source-id: b8949ca6913ed270e5ebd0a0c5335b224f817774
Summary: We have this counter but don't increment it, might as well.
Reviewed By: yangchi
Differential Revision: D23916621
fbshipit-source-id: 0a8cb947f1941e04789a0144a4fec87239caef50
Summary:
Similar to raise timeout, client can choose the probe timeout via transport
parameter. This timeout might not end up being useful because by the time the
recommended timeout (15s) expires, either:
i. the probe gets acked, d6d send a larger probe or sleeps if the upper bound
is found
ii. the probe is lost, then quic will likely determine its loss faster than
15s, upon which a PMTU blackhole is recognized
So adding this is mostly for mvfst to be compliant with the d6d spec, and
potentially useful if we want to control probe sending rate upon ack, e.g. send
the next probe after 1s upon ack instead of immediately.
Reviewed By: yangchi
Differential Revision: D23700182
fbshipit-source-id: 18b740d05343591d6afa086b9fae746e6c71aca5
Summary: This is a temporary hack until we properly implement 0-rtt transport parameter updating. Now after 0-rtt we will use the packet size from the peer as the max packet size, if the can ignore setting is set.
Reviewed By: xttjsn
Differential Revision: D23690019
fbshipit-source-id: b4dbf5702e81e52ccd437e0fa68f4d156c7123be
Summary:
As a second attempt to add d6d, I tried to be as non-intrusive as possible, by de-coupling all state that d6d needs from the existing transport state.
To reduce complexity, I made the assumption that, as a starter, only server does the probing. To make it easy to control d6d in different connection settings, both the server and the client has a toggle `enabled`. It is only when both the server and client are `enabled`, that server will do probing for a connection.
Among all the changes, this adds:
- Two transport parameters:
- `d6d_base_pmtu`: this is the base PMTU client advertises to server during handshake. A valid presence of this value indicates that clients d6d module is `enabled`. Although this config value is not used by server, I kept it in `D6DConfig` to avoid complexity and make it possible for future extension where clients might also do probing.
- `d6d_raise_timeout`: this is the raise timeout client advertises to server during handshake. It is the amount of time d6d "sleeps" after it finds an upper bond. It is optional. We want this because depending on the network conditions we might need to adjust this timeout.
Reviewed By: mjoras
Differential Revision: D23409623
fbshipit-source-id: bad6df443cc13dc4d69532342f182cb919c5a7dd
Summary:
First step towards d6d. Semantically we need to separate the old `udpSendPacketLen` into `peerMaxPacketSize` as well as `currPMTU`. The former is directly tied to the peer's max_packet_size transport parameter whereas the second is controlled by d6d. To get the actual udp mss, call `conn_->getUdpSendPacketLen()`, which will use the minimum of the two if d6d is enabled, otherwise it will fallback to use `peerMaxPacketSize` only.
During processClientInitialParams and processServerInitialParams, we no longer need to check whether `canIgnorePathMTU` is set because that logic is moved to `setUdpSendPacketLen`. If d6d is enabled, we set both `peerMaxPacketSize` and `currPMTU` to `packetSize` because receiving an initial packet of size x indicates both that the peer accepts x-sized packet and that the PMTU is at least x.
Many call sites and tests are changed.
Faebook:
For now, d6d is considered enabled if `canIgnorePathMTU==false` and `turnoffPMTUD==true`. Down the road, from semantic & practical POV at least one of them should be renamed to something like `enableD6D`, since enabling d6d implies turning off PMTUD and that we should not ignore PMTU. We can keep one for the sake of testing.
Reviewed By: mjoras
Differential Revision: D22049806
fbshipit-source-id: 7a9b30b7e2519c132101509be56a9e63b803dc93
