Summary:
Previously, the maximum stored rx timestamps was controlled by a constant `kMaxReceivedPktsTimestampsStored`. Make this programmable via MC so different values can be pushed based on server and client criteria (via GK groups).
(Note: this ignores all push blocking failures!)
Reviewed By: mjoras
Differential Revision: D45343231
fbshipit-source-id: aafa799925da2c11e14394d11fa4855f7107daf4
Summary:
This is essentially a further extension of the previous idead of having a sequence number per-DSR stream.
The rationale is the same -- to reduce spurious loss declared from the reordering threshold.
The primary case this works around is the following
```
<!DSR><!DSR><!DSR><DSR><DSR><DSR><!DSR>
```
Suppose we get an ACK for [0-1,3-5] leaving only packet number 2 remaining.
The naive reordering threshold will declare packet number 2 as lost. However, in principle this is arguably wrong since when considered on the timeline of !DSR packets, the gap does not exceed the reordering threshold.
To accomodate this we need to track a monotonically increasing sequence number for each non-DSR packet written, and store that in the packet metadata. This way we can use that for the reordering comparison rather than the packet number itself.
When there is no DSR packets ever written to a connection this should devolve to an identical result to using the packet number, as they will increment together.
Reviewed By: kvtsoy
Differential Revision: D46742386
fbshipit-source-id: 2983746081c7b6282358416e2bb1bcc80861be58
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:
We don't need to carry these states after the handshake is confirmed, so make them pointers instead. This will facilitate adding a structure to the AckState for tracking duplicate packets.
(Note: this ignores all push blocking failures!)
Reviewed By: hanidamlaj
Differential Revision: D41626895
fbshipit-source-id: d8ac960b3672b9bb9adaaececa53a1203ec801e0
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:
The current logic for handling the ACK_FREQUENCY frame mistakenly uses the frame's reorderThreshold for deciding losses, rather than triggering immediate acknowledgements.
This change fixes the logic by tracking the out-of-order distance and comparing that against the reorderThreshold.
Reviewed By: mjoras
Differential Revision: D39331920
fbshipit-source-id: 125fd99dce9b2725ea0d5b26236f48f72db53c48
Summary: Make `min(minRTT - ACK delay)` obsered so far over the life of the connection available in `TransportInfo`. In addition, expose the last RTT and last RTT's AckDelay value as well.
Reviewed By: mjoras
Differential Revision: D28385923
fbshipit-source-id: a58000ac8bd9fdc63caa0b636bdb83905cd6b448
Summary:
RFC9002 states the following in section 5.3
> 5.3. Estimating smoothed_rtt and rttvar
> Therefore, when adjusting an RTT sample using peer-reported acknowledgment delays, an endpoint [...] MUST NOT subtract the acknowledgment delay from the RTT sample if the resulting value is smaller than the min_rtt. This limits the underestimation of the smoothed_rtt due to a misreporting peer
However, today we subtract the ACK delay from an RTT sample used to estimate `smoothed_rtt` if it is the first RTT sample, as we will not yet have a minimum. This can lead to underestimation of `smoothed_rtt` if the peer is misreporting, and furthermore can create a situation during which `smoothed_rtt` is lower than `min_rtt`.
Resolving by not allowing `smoothed_rtt` to be adjusted by ACK delay for the first RTT sample, since doing so would lead to `smoothed_rtt` being lower than `min_rtt`.
Reviewed By: jbeshay
Differential Revision: D34792648
fbshipit-source-id: e0d6207ff73cdc13e0b40193abd27abce7142499
Summary: As titled. Reduces confusion, as we always expect this to be populated.
Differential Revision: D31887097
fbshipit-source-id: 153b05bae8abd559fe49d2c07c64d2ad0d92a809
Summary:
- Skip ACK-only response to initial crypto data from client
- Enabled through experimental transport settings
Reviewed By: mjoras
Differential Revision: D31863653
fbshipit-source-id: b290db0399dd7a3be41078c4a839b484da864f2f
Summary:
Introruced a new DetailsPerStream struct inside the outstanding packet metadata
so we can easily track all streams carrying app data within a packet and each
stream's bytes sent (both new and total).
With this, consumers can easily deduce the number of retransmitted bytes per
stream using the StreamDetails members:
newStreamBytesSent - streamBytesSent.
Reviewed By: bschlinker
Differential Revision: D28063916
fbshipit-source-id: d915514f85f24f4889cfac2f7a66bfd2d6b0a4af
Summary:
The goal of this change is to fix the bug that ```ackState.needsToSendActImmediately``` can be set to false even if it has been set to true. It leads to an issue when a batch of packets are consumed.
**Changes**
1. No longer set **false** for ```ackState.needsToSendAckImmediately```. If it is ever set to **true**, it continues to be **true**. Otherwise, it sticks to the default value of **false**.
1. Set ```ackState.scheduleAckTimeout``` to **true** only if ```ackState.needsToSendAckImmediately``` is **false**.
1. Always set ```ackState.numRxPacketsRecvd``` and ```ackState.numNonRxPacketsRecvd``` to **0** if ```ackState.needsToSendAckImmediately``` is **true**.
Reviewed By: yangchi
Differential Revision: D28241894
fbshipit-source-id: 988ac973beaa6b4348d99aa0bd6036318a6e1778
Summary:
Previously, we maintained state and counters to count both, header and body
bytes together. This commit introduces additional counters and state to keep
track of just the body bytes that were sent and acked etc. Body bytes received
will be implemented later.
Reviewed By: bschlinker
Differential Revision: D27312049
fbshipit-source-id: 33f169c9168dfda625e86de45df7c00d1897ba7e
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:
- Adds counter of the number of ack-eliciting packets sent; useful for understanding lost / retransmission numbers
- Adds the following to `OutstandingPacketMetadata`
- # of packets sent and # of ack-eliciting packets sent; this enables indexing of each packet when processed by an observer on loss / RTT event, including understanding its ordering in the flow of sent packets.
- # of packets in flight; useful during loss analysis to understand if self-induced congestion could be culprit
- Fixes the inflightBytes counter in `OutstandingPacketMetadata`; it was previously _not_ including the packets own bytes, despite saying that it was.
Right now we are passing multiple integers into the `OutstandingPacket` constructor. I've switched to passing in `LossState` to avoid passing in two more integers, although I will need to come back and clean up the existing ones.
Differential Revision: D25861702
fbshipit-source-id: e34c0edcb136bc1a2a6aeb898ecbb4cf11d0aa2c
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:
Due to high number of RTT samples I refactored the OutstandingPacket to
split the packet data and packet metrics. We know have access to metrics
without the need of saving the packet data.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/178
Reviewed By: mjoras
Differential Revision: D23711641
Pulled By: bschlinker
fbshipit-source-id: 53791f1f6f6e184f37afca991a873af05909fbd2
Summary:
This is following a similar pattern than what was done for the client side.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/160
Reviewed By: yangchi
Differential Revision: D23560951
Pulled By: xttjsn
fbshipit-source-id: 351417cbfa3230112fff4c4de59b307f88389cf6
Summary:
This diff:
1) introduces `EnumArray` - effectively an `std::array` indexed by an enum
2) changes loss times and `lastRetransmittablePacketSentTime` inside `LossState` to be an `EnumArray` indexed by `PacketNumberSpace`
3) makes the method `isHandshakeDone()` available for both client and server handshakes.
4) uses all those inputs to determine PTO timers in `earliestTimeAndSpace()`
Reviewed By: yangchi
Differential Revision: D19650864
fbshipit-source-id: d72e4a0cf61d2dcb76f0a7f4037c36a7c8156942
Summary:
It is useful to be able to control the ACK generation frequency based on how many packets have been received. This adds 3 tunables: a threshold, and an ACK frequency below and above that thresold.
Note this keeps the default so there is no behavior change.
Reviewed By: yangchi
Differential Revision: D19455514
fbshipit-source-id: 53b765d4c872bc6b7c19d5ea859f8eee86d9b5ff
Summary:
Previously we track them since we thought we can get some additional
RTT samples. But these are bad RTT samples since peer can delays the acking of
pure acks. Now we no longer trust such RTT samples, there is no reason to keep
tracking pure ack packets.
Reviewed By: mjoras
Differential Revision: D18946081
fbshipit-source-id: 0a92d88e709edf8475d67791ba064c3e8b7f627a
Summary: The state machine logic is quite abstruse, this modifies it to make it more readable.
Reviewed By: siyengar
Differential Revision: D18488301
fbshipit-source-id: c6fd52973880931e34904713e8b147f56d0c4629
Summary: inspect PN, and when it reaches 2^62 -2 trigger a transport close through a pending event.
Reviewed By: yangchi
Differential Revision: D18239661
fbshipit-source-id: 1a218678099016693149e12ff121e2a39b95aecc
Summary: This fixes an issue with rtt computation. The first rtt sample was incorrect because minrtt gets set to the first rtt sample, hence ack delay never get subtracted. This diff allows ack delay to be subtracted if the minrtt is the first one.
Reviewed By: yangchi
Differential Revision: D17610197
fbshipit-source-id: 3e7087f37ef14848d80c0ab33094834a4fc8974b
Summary: Centralize this logic into the pacer interface. But instead of having an explicit canBePaced() function (and we already have a dozen other canBePaced functions or boolean flags in other layers), the pacer just use 0us and default write batch size per loop to answer pacing rate queries when it doesn't meet the condition to properly pace the transport writes.
Reviewed By: mjoras
Differential Revision: D16912631
fbshipit-source-id: 0343b126ca5de315af6823067316526b1004cc6c
Summary: Add transportStateUpdate event so it can be part of qlog.
Reviewed By: mjoras
Differential Revision: D16342467
fbshipit-source-id: 109189275d44996850b82646bab4a733a3a4c7a1
Summary:
Currently we handle crypto timer before loss timer. And currently loss
time is for AppData only since for the other two Packet Number spaces, crypto
timer will take care of it. This diff extends loss times to 3 loss times, and
handle them before handle crypto timer. The rational here is that loss time is
shorter than crypto timer, so this will make retransmission during crypto
handshake more aggressive. For app data, this diff doesn't change anything.
Reviewed By: sharma95
Differential Revision: D15552405
fbshipit-source-id: bd5c24b0622c72325ffdea36d0802d4939bae854
Summary: Replace hard coded stateless reset token with a token from the stateless reset token generator.
Reviewed By: yangchi
Differential Revision: D15481858
fbshipit-source-id: 30c96843c38c616600466b2fabb6defd5fcc5799
Summary: This is no longer part of the spec. It's up to the application how to handle reset
Reviewed By: lnicco
Differential Revision: D15107164
fbshipit-source-id: 2a1fe0c552bd7f054e84ef86a01a78c379b0a483
Summary: This is step 1 for removing reset on reset, since the send side may need to transition to waiting for a reset ack while the read side is an any state.
Reviewed By: lnicco
Differential Revision: D15075849
fbshipit-source-id: 1e094942a8a1ca9a01d4161cd6309b4136a9cfbf
