Summary:
- adds logic to drop retry packet if we've processed an initial packet
from RFC9000:
> After the client has received and processed an Initial or Retry packet from the server, it MUST discard any subsequent Retry packets that it receives.
Reviewed By: jbeshay, mjoras
Differential Revision: D45536695
fbshipit-source-id: c6f9532478ab249905683ca1fff6e692a978a0d2
Summary:
Somehow we never implemented this stat despite having it for ages.
It's relatively easy to do, we just need to check whether an entry was inserted to the IntervalSet we are already using for tracking what to ACK.
Note that this has the limitation that when the ACK interval set is cleared out (on ACK of ACK), we will no longer be able to detect duplicates. This is something we can tune later.
Reviewed By: kvtsoy
Differential Revision: D45131856
fbshipit-source-id: aad4e07e1a9cd5b2dc5dec60424f7cee15906c7e
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: This change ensures that the stream is not destroyed before onEOM callback so that onEOM can access stream specific data.
Reviewed By: mjoras
Differential Revision: D42784838
fbshipit-source-id: ce619be1a2b6a542a517a7b4f32c2b3d37aa30fd
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:
Store timestamps/packet numbers of recently received packets in AckState.
- The maximum number of packets stored is controlled by kMaxReceivedPktsTimestampsStored.
- The packet number of entries in the deque is guarenteed to increase
monotonically because an entry is only added for a received packet
if the packet number is greater than the packet number of the last
element in the deque (e.g., entries are not added for packets that
arrive out of order relative to previously received packets).
Reviewed By: bschlinker
Differential Revision: D37799023
fbshipit-source-id: 3b6bf2ba8ea15219a87bbdc2724fe23eebe66b70
Summary: PacketDropReason was converted to a Better_Enum
Reviewed By: jbeshay
Differential Revision: D40350056
fbshipit-source-id: a6af9ccf0fc7c4358a0481de5cca6f69d1beb438
Summary: Retransmitting 0-rtt packets when 0-rtt is accepted can retransmit more packets than necessary when ACKs from the peer are delayed.
Reviewed By: mjoras
Differential Revision: D40274224
fbshipit-source-id: f43337c56341cdaa8504eab6d2b9b81d39f66a1f
Summary: The existing PacketDropReason values cover many branches in the code making it impossible to isolate the reason for a PARSE_ERROR, INVALID_PACKET, CONNECTION_NOT_FOUND. This change breaks them down into more values that are each used in a single branch.
Reviewed By: mjoras
Differential Revision: D39149490
fbshipit-source-id: 28cbe1ea6c4a06cf55960058edaa48c28ed4d2ef
Summary:
This is a longstanding off by one. When a stream frame is ACKed the largest ACKed offset contained is the stream frame offset + len - 1, unless there's an fin.
(Note: this ignores all push blocking failures!)
Reviewed By: kvtsoy
Differential Revision: D39085665
fbshipit-source-id: cf353c7e72466085a27537d2c95748781f70f79a
Summary: - usage of these new tokens should not overlap
Reviewed By: mjoras
Differential Revision: D35323558
fbshipit-source-id: c9152cbb7029f64396661ea9091567ef5a75f4dc
Summary: In order to provide more information about datagrams when we receive them, I'm adding a wrapper around the BufQueue that we currently hold so we can include receiveTimePoint and any other metadata we might want to expose for the Datagram API.
Reviewed By: mjoras
Differential Revision: D33994358
fbshipit-source-id: 805f182cd350908320639bc07eb6f3b7349bbc05
Summary:
Maine change is `MockConnectionCallback` -> `MockConnectionSetupCallback` + `MockConnectionCallbackNew`.
Everything else is changing tests to use the two new classes.
Differential Revision: D33076321
fbshipit-source-id: a938b63ce59f07f549b3e725caad8785348db7ed
Summary: When there is not enough space in the QUIC packet to write a datagram frame, we should not dequeue the datagram
Reviewed By: mjoras
Differential Revision: D33109603
fbshipit-source-id: 937a5a0ffe55c7f88e39faf224e7ad06ca599708
Summary:
Adds stream specific details to `AckEvent` for each ACKed packet, including:
- the number of stream frame bytes newly ACKed
- the number of stream frame bytes newly ACKed by a packet containing a retransmission of said frames (meaning that the original packet was lost, or that the packet with the retransmission arrived at the receiver and corresponding ACK arrived at the sender faster (unlikely), or that the receiver ACK including the original packet was lost, and the ACK interval was pruned too quickly)
- the change in the stream delivery offset for each stream triggered by ACK arrival (if any)
- the stream intervals (and more specifically, the stream frames) that were "dupacked" by the ACK of this packet, meaning that ACK of some other packet had already marked those frames / stream bytes as delivered
Information about "dupacked" packets will not be available until processing of ACKs for packets spuriously marked as lost is enabled; this will come in a follow up diff.
Reviewed By: mjoras
Differential Revision: D31915844
fbshipit-source-id: b8056ea1b5f6093b61e5463c6b4c11cd83bd2916
Summary:
Introduces `QuicTypedTransportTest`, a `TYPED_TEST` that will enable us to test various implementations of `QuicTransportBase` with only one set of tests.
Today, we write separate tests in `QuicClientTransportTest` and `QuicServerTransportTest` for the same functionality. Due to this overhead, I've noticed that in some cases we don't have proper test coverage, and this leads to bugs (see the off by one issue with ACK byte events).
`QuicTypedTransportTest` is an effort to fix this by making it easier to write tests that all transports should be able to support. `QuicTypedTransportTestBase` serves as a shim layer between the test and the specific test transport implementation. In following diffs, I extend the functionality of `QuicTypedTransportTest` further to make it easier to test cases where packets arrive from the remote (peer), including stream and reset packets.
Reviewed By: mjoras
Differential Revision: D32560835
fbshipit-source-id: e08387364f614c37ed2126dbfb91b30230d116a0
Summary: Making it possible to reuse the foundation of `QuicClientTransportTest` elsewhere. Will be building a test that uses `TYPED_TEST` to test against both transports.
Reviewed By: jbeshay
Differential Revision: D31886977
fbshipit-source-id: a9d9a335aaa0c5f5ea6cd2249fe23b35d926b5c9
Summary:
## Issue
When we enabled the JK to set QUIC server's rate limit to zero (D30703498), causing it to send retry packets to all incoming client hellos, we found that android clients were crashing (T99705615).
We were able to reproduce the bug with LigerIntegrationTest and HQClient backed by HQServer, causing the clients to hang.
## Fix
- When receiving a retry packet from the server, migrate the outstandings, stream manager, among other required fields to the new connection state.
- QuicStreamManager & QuicStreamState constructors to facilitate migrating to a new connection state.
Reviewed By: kvtsoy
Differential Revision: D31006905
fbshipit-source-id: 0490ceee1bef52b94c91019426d791e212820508
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:
Update QUIC to V1 and HTTP/3 to h3. In order to support interop, hq-interop ALPN has been added too.
This change maintains support for draft-27 and draft-29 implementations.
Reviewed By: mjoras
Differential Revision: D29714556
fbshipit-source-id: 0685928ef4bede0b5511e59572e9c86ccc867320
Summary:
This is Linux specific stuff.
(Note: this ignores all push blocking failures!)
Reviewed By: lnicco
Differential Revision: D29188261
fbshipit-source-id: 711fcf79c1eb2ef78a02945f395c39880f4fa61e
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
