Summary:
Now we won't have a zero PTO and we will properly clear out the outstanding packets.
Note that this cipher dropping is not what the draft prescribes, instead dropping both the initial and handshake ciphers when we know 1-rtt communication is functioning.
Reviewed By: yangchi
Differential Revision: D20388737
fbshipit-source-id: 0b89eb80c8faa796ab09eda3eaa10a00dcf7bae9
Summary:
Currently the packet builder contructor will encode the packet
builder. This is fine when the builder creates its own output buffer. If later
on we decides not to use this builder, or it fails to build packet, the buffer
will be thrown away. But once the builder uses a buffer provided by caller, and
will be reused, we can no longer just throw it away if we decide not to use
this builder. So we have to delay the header encoding until we know we will use
the builder.
This is still not enough to solve the case where we want to use this builder,
it builds, then it fails . For that, we will need to retreat the tail position
of the IOBuf.
Reviewed By: mjoras
Differential Revision: D21000658
fbshipit-source-id: 4d758b3e260463b17c870618ba68bd4b898a7d4c
Summary: This is useful when you want to ensure that the IOBuf you pass in is encrypted inplace, as opposed to potentially creating a new one.
Reviewed By: yangchi
Differential Revision: D21135253
fbshipit-source-id: 89b6e718fc8da1324685c390c721a564bb77d01d
Summary:
To prepare for another configuration of this chain. With this, now we
can land all the outstanding GSO optimization diffs without having to worry
about breaking the production code.
Reviewed By: mjoras
Differential Revision: D20838453
fbshipit-source-id: 807a0c546305864e0d70f8989f31d3de3b812278
Summary:
As it turns out, the extra indirection from storing a unique_ptr is not worse than the gain from using an `F14ValueMap` versus an `F14VectorMap`.
This reduces the `find` cost measurably in profiles, and doesn't appear to have any real negative effects otherwise.
Reviewed By: yangchi
Differential Revision: D20923854
fbshipit-source-id: a75c4649ea3dbf0e6c89ebfe0d31d082bbdc31fd
Summary:
Currently we return the exact writable bytes number from a real
congestion controller or Path Challenger. This diff round the number up to the
nearest multiple of packet length. Doing so can greatly reduce weird bytes
counting/checking bugs we have around packet writing.
Reviewed By: mjoras
Differential Revision: D20265678
fbshipit-source-id: 2973dde3acc4b2008337127482185f34e16efb43
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:
In a recent diff this was mistakenly changed to be the earliest sent
time, which makes the loss alarm duration much shorter, then we end up firing
them a lot.
Reviewed By: mjoras
Differential Revision: D19826034
fbshipit-source-id: e190d30655533fdb220e92eddc5754fb9abd007b
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: PTO sending behavior has to be changed to implement per-number-packet-space PTO. This refactoring unifies the PTO sending code in order to avoid duplicating it in both functions.
Reviewed By: yangchi
Differential Revision: D19673324
fbshipit-source-id: 99b4ef5f86039ffec2b4330a6d890c5fc8df2cfd
Summary:
The fizz `encrypt` API will opportunistically use inplace encryption if the `IOBuf` you pass it is correctly sized. If it is not it will manually create the output buffer.
It turns out doing inplace versus encrypting with a given target output is a bit faster in microbenchmarks, and it will also allow experimenting with caching `IOBuf`s on the send path.
Reviewed By: knekritz
Differential Revision: D19506291
fbshipit-source-id: 3ef41290538ceac34a344114badbd167e2c25a50
Summary:
(1) The first change is the pacing rate calculation is simplified. It
removes the interval calculation and just uses the timer tick as the interval.
Then it calculates the burst size from there. For most cases these two
calculation should land at the same result, except when the
`cwnd < minBurstSize * tick / RTT`. In that case, the current calculation would
spread writes evenly across one RTT, assuming no new Ack arrives during the RTT;
while the new calculation uses the first a few ticks to finish the cwnd amount
of data.
(2) Then this diff changes how we compensate late timer. Now the pacer will
maintain a nextWriteTime_ and lastWriteTime_, which makes it easier to
calculate time elapsed since last write. Then each time writer tries to write,
it will be allowed to write timeElapsed * pacingRate. This is much more
intuitive than the current logic.
(3) The diff also adds pacing limited tracking into the pacer. An expected
pacing rate is cached when pacing rate is refreshed by congestion controller.
Then with packets sent out, Pacer keeps calculating the current send rate. When
the send rate is lower, Pacer sets pacingLimited_ to true. Otherwise false.
Only when the connection is not pacing limited, the lastWriteTime_ will be
packet sent time, otherwise it will be set to the last nextWriteTime_. In other
words: if the send rate is lower than expected, we use the expected send time
instead of real send time to calculate time elapsed, to allow higher late
timer compenstation, to give pacer a chance to catch up.
(4) Finally this diff removes the token collecting behavior in the pacer. I
think having tokens increaed, instead of reset, when an ack refreshes the pacing
rate or when we compensate late time, is quite confusing to some people. After
all the above changes, I found tperf can still sustain good throughput without
always increase tokens, and rally actualy gives even better results. So i think
we can remove this part of the pacer that's potentially very confusing to
people who don't know how we got there.
Reviewed By: mjoras
Differential Revision: D19252744
fbshipit-source-id: b83e4a01fc812fc52117f3ec0f5c3be1badf211f
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:
Before we have pacing, we limit write function to loop at most 5
times. Then pacing came in, and pacing burst is used as the limit when pacing
is enabled. Then pacing token was introduced to increase send rate when pacing
is enabled. Then when cwnd is large enough, pacing burst size can be really
large, that means this write function can loop for long time. When use loopback
as network interface, for example, the write function can loop more than 1 RTT,
which delays receive time when peer packets already arrived, which leads to
both wrong RTT estimation and wrong BBR bandwidth estimation.
This diff limits the write to a fraction of RTT as well, and current default
value will be 1/25 SRTT.
Reviewed By: mjoras
Differential Revision: D18864699
fbshipit-source-id: 0b57ee4138e4788d132152a4aa363959065f6f7f
Summary: There's no reason for creating a temporary here just to then later emplace it into outstandingPackets.
Reviewed By: yangchi
Differential Revision: D19069802
fbshipit-source-id: 4505ed414a80b24ecd0111fedc21714412fa8c53
Summary: As it turns out these end up being hot functions. Most of the hotness is it taking the cache miss, but we're able to reduce it from .9% exclusive each to .1-.3% exclusive each by inlining it (which eliminates a `callq` instruction for each).
Reviewed By: yangchi
Differential Revision: D19069685
fbshipit-source-id: c971a4d1c26a7e48008c36a129e0a842a27ca87f
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:
Don't use IOBufQueue for most operations in mvfst and use BufQueue instead. Since BufQueue did not support a splitAtMost, added it in instead.
The only place that we still use IOBufQueue is in crypto because fizz still requires it
Reviewed By: mjoras
Differential Revision: D18846960
fbshipit-source-id: 4320b7f8614f8d2c75f6de0e6b786d33650e9656
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:
As the first step of deprecating quic trace in favor of qlog, this
diff removes the packet_sent event
Reviewed By: sharma95
Differential Revision: D18708001
fbshipit-source-id: 5c12c13b284f00cc7d3075086e10f517ede904cb
Summary:
The retransmission buffer tracks stream frame data we have sent that is currently unacked. We keep this as a sorted `deque`. This isn't so bad for performance, but we can do better if we break ourselves of the requirement that it be sorted (removing a binary search on ACK).
To do this we make the buffer a map of offset -> `StreamBuffer`.
There were two places that were dependent on the sorted nature of the list.
1. For partial reliablity we call `shrinkBuffers` to remove all unacked buffers less than an offset. For this we now have to do it with a full traversal of the retransmission buffer instead of only having to do an O(offset) search. In the future we could make this better by only lazily deleting from the retransmission buffer on ACK or packet loss.
2. We used the start of the retransmission buffer to determine if a delivery callback could be fired for a given offset. We need some new state to track this. Instead of tracking unacked buffers, we now track acked ranges using the existing `IntervalSet`. This set should be small for the typical case, as we think most ACKs will come in order and just cause existing ranges to merge.
Reviewed By: yangchi
Differential Revision: D18609467
fbshipit-source-id: 13cd2164352f1183362be9f675c1bdc686426698
Summary:
Use the Path rate limiter introduced in the previous diff.
When we initialize path validation of an unvalidated peer address,
enable pathValidationRateLimit.
When we receive a proper PATH_RESPONSE frame, disable this limit.
If this limit is enabled, we will check the pathValidationLimiter for
the amount of bytes we are allowed to write.
Change the migration tests in QuicServerTransportTest to use this new limiter
instead of writableByteLimits.
Update shouldWriteData to directly use the new congestionControlWritableBytes
function.
Reviewed By: yangchi
Differential Revision: D18145774
fbshipit-source-id: 1fe4fd5be7486077c58b0d1285dfb03f6c62831c
Summary:
We maintain the invariant that a buffer cannot be in the loss buffer and retransmission buffers at the same time. As the retransmission buffer holds all unacknowledged data that isn't marked lost, it is very likely to be larger than the loss buffer. This makes the existing case to check for cloning very expensive.
Instead search the loss buffer first, change the search of the loss buffer to a binary search, and elide the double search.
Reviewed By: yangchi
Differential Revision: D18203444
fbshipit-source-id: 66a4e424d61c4b0e3cad12c7eca009ad3d6c5a0d
Summary:
Add a token value into the pacer. This is so that when there is not
enough application data to consume all the burst size in current event loop, we
can accumulate the unused sending credit and use the later when new data comes
in. Each time a packet is sent, we consume 1 token. On pakcet loss, we clear
all tokens. Each time there is an ack and we refresh pacing rate, token
increases by calculated burst size. It is also increased when timer drifts
during writes. When there is available tokens, there is no delay of writing out
packets, and the burst size is current token amount.
Reviewed By: siyengar
Differential Revision: D17670053
fbshipit-source-id: 6abc3acce39e0ece90248c52c3d73935a9878e02
Summary: Use the custom variant type for errors.
Reviewed By: yangchi
Differential Revision: D17826935
fbshipit-source-id: 2bf0c3e1cc8ca84b504d201fd6c2a4266878b715
Summary:
Use the custom variant type for write frames as well, now that
we use them for read frames.
Reviewed By: mjoras
Differential Revision: D17776862
fbshipit-source-id: 47093146d0f1565c22e5393ed012c70e2e23d279
Summary:
The retransmission buffer list is sorted by offset. New written
always has higher offset than existing ones. Thus the binary search isn't
necessary.
Reviewed By: mjoras
Differential Revision: D17477354
fbshipit-source-id: d413a5c84c3831b257d5c1e6375bec56a763926b
Summary: Beside the beginning of a connection where we do handshake and have packets from all 3 spaces can possibly in the outstanding packet list, for most parf of the connection, this list only has packets from AppData space. That means the packet numbers are strictly ascending for most parf of a connection. Then linear search from back can potentially reduce the number of times we do the packet number compare.
Reviewed By: mjoras
Differential Revision: D17500221
fbshipit-source-id: 15eafa439e889e45d3e4a84d9e9dad0e2c743c9d
Summary:
We always generate a new std::function object for such header
builders. It's ok to move them.
Reviewed By: mjoras
Differential Revision: D17479758
fbshipit-source-id: 88fbdbc5c7b7403fffd642d8f1f84e1cd57115fa
Summary:
Make a custom variant type for PacketHeader. By not relying on boost::variant
this reduces the code size of the implementation.
This uses a combination of a union type as well as a enum type to emulate a variant
Reviewed By: yangchi
Differential Revision: D17187589
fbshipit-source-id: 00c2b9b8dd3f3e73af766d84888b13b9d867165a
Summary:
Implement sending stream limit updates in a windowed fashion, so that as a peer exhausts its streams we will grant it additional credit. This is implemented by having the stream manager check if an update is needed on removing streams, and the api layer potentially sending an update after it initiates the check for closed streams.
This also makes some driveby changes to use `std::lower_bound` instead of `std::find` for the sorted collections in the stream manager.
Reviewed By: yangchi
Differential Revision: D16808229
fbshipit-source-id: f6e3460d43e4d165e362164be00c0cec27cf1e79
Summary: These were changed to varints. To support this we need to do some extra horrible version plumbing. I don't want to keep this long term but it works for now.
Reviewed By: yangchi
Differential Revision: D16293568
fbshipit-source-id: a9ea9083be160aa3e6b338a7d70d7f00e44ec5ab
Summary:
This diff adds a DebugState in the QuicConnectionState to track the
reason we schedule transport WriteLooper to run, the reason we end up not
writing and the number of times such write happens consecutively. And when it
reaches a predefined limit, we trigger a callback on a loop detector interface.
Reviewed By: kvtsoy
Differential Revision: D15433881
fbshipit-source-id: d903342c00bc4dccf8d7320726368d23295bec66
Summary: Rename `add` to `addPacket`, since `add` is an ambiguous function name, especially as we include more events.
Reviewed By: sharma95
Differential Revision: D16102141
fbshipit-source-id: 73196a5f1bcc681e3bb0a26fd9edaf34f59bf710
Summary:
This ensure a lot of code do not depend on fizz anymore.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/26
Reviewed By: mjoras, JunqiWang
Differential Revision: D16030663
Pulled By: yangchi
fbshipit-source-id: a3cc34905a6afb657da194e2166434425e7e163c
Summary:
This introduce quic::Aead as a simple typedef to fizz::Aead and update the codebase to use quic::Aead . This should not impact the functionality of the code in any way.
This is a first step toward introducing an interface that is specific for mvfst so that mvfst can swap fizz for something else.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/12
Reviewed By: JunqiWang
Differential Revision: D15335324
Pulled By: mjoras
fbshipit-source-id: fef166a9a5c2cbae08ad9511d0abd749f330c221
Summary:
Current app-limited is defined as the connection doesn't have a no
non-control stream. I'm changing this to be app-idle. And app-limited will be
changed to a state in connection where app isn't sending bytes fast enough to
keep congestion controller probing higher bandwidth.
Reviewed By: mjoras
Differential Revision: D15456199
fbshipit-source-id: e084bc133284ae37ee6da8ebb3c12f505011521e
