Summary: Fix wrong order of fields in a VLOG message.
Reviewed By: lnicco
Differential Revision: D32106255
fbshipit-source-id: 50367a5000cb2033ecafaa6ef389971bc88a9a60
Summary:
The StreamManager monitors can now have a reference to a new observer interface QuicStreamPrioritiesObserver. If the reference is set, the observer is notified whenever streams are created, removed, or change priorities
QuicTransportBase implements this observer interface. It uses the event notifications to control a connection's background mode based upon the new background mode parameters: priority level threshold, and target utilization factor. When these parameters are set, and all active streams have a priority lower than the threshold, the connection's congestion controller is set to use only the defined utilization factor of the available bandwidth.
Reviewed By: bschlinker
Differential Revision: D31562505
fbshipit-source-id: 9c74fa834301f745d97e851741b911795f756023
Summary: Add global counters for QUIC stream reset and connection close
Reviewed By: lnicco
Differential Revision: D30877776
fbshipit-source-id: 4ee7c3f7c3c1cf190f464e74d616e0b5f1539d3a
Summary:
This diff introduces split callbacks to QUIC.
Old: callback:
* `ConnectionCallback`
Two new callbacks:
* `ConnectionSetupCallback`
* `ConnectionStreamsCallback`
For now the idea is that both old and new callbacks will live side by side and mvfst will decide which callbacks scheme to use via the `useSplitConnectionCallbacks` flag supplied on both QUIC client and server creation.
Reviewed By: mjoras
Differential Revision: D30407129
fbshipit-source-id: 7b726c2652ebe00b47f44bf291cffb9d9f6c884b
Summary: Do not call onAppRateLimited() before connection is ready
Reviewed By: bschlinker
Differential Revision: D30563194
fbshipit-source-id: dd8c2da656041f9219a645935b6c4c3c85b27816
Summary:
This is another diff preparing mvfst for the new split callbacks.
This change introduces a callback dispatcher. Right now, it simply calls the existing monolithic callback everywhere. In future diffs, this dispatcher will be using the `useSplitCallbacks` flag to decide which callback object to call.
Reviewed By: mjoras
Differential Revision: D30404458
fbshipit-source-id: 5bb919185b845acc3e8ce7c596c0769b541ff69d
Summary:
For the split QUIC callbacks, onConnectionError() will be present in both connection setup and normal connection callbacks.
This diff has no functional changes rather just separates the code that will change (in diffs higher up the stack) into a few functions for easiness.
Reviewed By: mjoras, lnicco
Differential Revision: D30400469
fbshipit-source-id: df259f201b74c51704e4284cbf94ad485f80a1ab
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: It's useful at the end of a connection to know if we tried DSR.
Reviewed By: jbeshay
Differential Revision: D30545282
fbshipit-source-id: bbb2f3408f7a2d5666676c9e2583bf8fd9f18911
Summary: Expose usedZeroRtt to transport info
Reviewed By: mjoras
Differential Revision: D29863937
fbshipit-source-id: 9b42e654fda89f5a383023f0c4187cd1457adfea
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:
For each `QuicServerTransport` created by `QuicServerWorker`, we are able to set the `cmsg`s to be sent whenever there's a write.
With this we can set things like TOS/SO_MARK per connection
Reviewed By: bschlinker
Differential Revision: D29313608
fbshipit-source-id: d60c97f65681086ae1079b6b95beade95158ec59
Summary: This probably never made much sense as a policy. The application can reasonably expect to be able to set a read callback after the stream first becomes readable.
Reviewed By: afrind
Differential Revision: D29563483
fbshipit-source-id: 215a6aed130108248a0b4cbc36c02c8da70bf721
Summary:
Add a new QuicSocket API that sets the maximum pacing rate in Bytes per second to be used if pacing is enabled. This setting is passed down to the Pacer which enforces it.
The change also includes:
- Refactoring the setPacingRate function of the Pacer to make it more consistent with the other refreshPacingRate function.
- A new flag for testing the MAX_PACING_RATE using tperf.
Reviewed By: mjoras
Differential Revision: D29276789
fbshipit-source-id: 818d86707084b2697f7417b4a47e62cbbce65c73
Summary: invoke updateWriteLooper to make sure a datagram can be sent when it's the only thing to send
Reviewed By: mjoras
Differential Revision: D29416414
fbshipit-source-id: 77cbfc655415b589839bc8e642720498b45ceb85
Summary: Remove a check that only allowed getting and setting the receive window on writable streams. This had prevented a receiver from getting/setting the receive window of a unidirectional stream initiated by the peer.
Reviewed By: mjoras
Differential Revision: D29366097
fbshipit-source-id: 3bc5cba826663b3499c19fe26bdd9a070682e533
Summary:
make getDatagramSizeLimit return the maximum datagram payload size that the
peer can accept.
This is currently the most conservative length, considering the maximum length
of a QUIC short header packet and a datagram frame header
Reviewed By: mjoras
Differential Revision: D28784866
fbshipit-source-id: cce8944a77f6f7b2d3535758c3c29ad88382710f
Summary: DeliveryCallback is a ByteEventCallback, so callers shouldn't need to change.
Reviewed By: bschlinker, mjoras
Differential Revision: D28562237
fbshipit-source-id: d7d41bbc3cbc708f7ecda5086fcba46b6a7847b0
Summary: When multiple observers attached and one is removed, removal code can end up calling `observerDetach` on the wrong observer. Simplified and fixed removal logic and added new tests.
Reviewed By: yangchi
Differential Revision: D27033221
fbshipit-source-id: d200fd2243a678890758b2652b61d16887f073dd
Summary:
Track the number of stream bytes written to the wire and expose in `quic::TransportInfo`.
Implemented as two counters:
- `totalStreamBytesSent` is a count of all stream bytes written to the wire; the sum of the lengths of stream frames in all packets sent
- `totalNewStreamBytesSent` is a count of all *new* stream bytes written to the wire -- a stream byte is new if it has not been transmitted before; the sum of the lengths of stream frames that have never been transmitted before in all packets sent
We can derive the total number of stream bytes retransmitted as `totalStreamBytesSent - totalNewStreamBytesSent`. We may want to deprecate the existing `totalBytesRetransmitted` counter because the name of that counter does not make it clear that it is stream bytes, and because only includes some types of retransmissions.
While `totalNewStreamBytesSent` is already captured as `flowControlState.sumCurWriteOffset`, I am not reusing that counter here to avoid having a dependency on the information we track for flow control, and to allow all relevant information to be captured in `LossState` (where other relevant fields already exist).
Reviewed By: yangchi
Differential Revision: D28061085
fbshipit-source-id: 73486a4ba3fc8f12959f68702dc58e61fdc21b65
Summary:
Previously we were using a single flag appDataSentEvents to enable 3 callbacks
(startWritingFromAppLimited, packetsWritten and appRateLimited). This commit
creates separate flags to enable each of these callbacks, we have use-cases
where Observers might want only packetsWritten events and not the others.
Also, this commit refactors the logic to deliver these 3 callbacks to all
observers into a separate method so they can be unit tested easily.
Reviewed By: bschlinker
Differential Revision: D27925011
fbshipit-source-id: 7f7436dfc3d50c3abcb8ec121b221d20e30b0c4b
Summary: This was missed when peekError was added
Reviewed By: mjoras, lnicco
Differential Revision: D27973063
fbshipit-source-id: dbf1112a4ce6822401ba5125de853fe922acd85e
Summary:
As per design, we are not supposed to honor duplicate byte event registrations. Duplicate is defined by a registration for the same <stream ID, stream offset, callback*> tuple twice.
Currently, there is a bug where-in we can register the same byte event a second time *if* there is another byte event for a higher offset that is registered in-between, for the same stream.
Example Sequence that triggers the bug:
1. Registration for Stream ID =10, offset = 50, callback = A. This will be successful.
2. Registration for Stream ID = 10, offset = 55, callback = <doesn't matter>. This will be successful.
3. Registration for Stream ID = 10, offset = 50, callback = A. This is a duplicate of #1. This *should not* be honored, but it was being honored (this was the bug).
This commit fixes the bug, #3 will return INVALID_OPERATION.
Reviewed By: bschlinker, lnicco
Differential Revision: D27946237
fbshipit-source-id: f9751ac90159284f86a285e5e0ebb23b83041d2a
Summary:
Diff D27846813 (98b5e2aa8f) introduced a fix for a corner case in the ByteEventRegistration logic where-in it was possible to register a byte event for the same stream ID, offset and ByteEvent type as long as they were not consecutive registrations. If there was a different byte event registration for the same stream ID in-between, then duplicates were being allowed.
However, that fix causes an unexpected crash in HQSession::dropConnectionSync().
This commit reverts the fix for duplicate byte event registrations until we are able to find the root cause for the crash and come up with a better fix. Note that we are still able to catch duplicate registrations as long as they are consecutive registrations for the same <stream ID, offset, callback>. With this fix reverted, we will not be able to catch non-consecutive duplicate registrations.
Reviewed By: bschlinker
Differential Revision: D27903179
fbshipit-source-id: b4da3128d5ba14fc764e1768fd27982553968997
Summary:
This assigns a DSRPacketizationRequestSender to a QUIC stream, and let
it own it.
Reviewed By: mjoras
Differential Revision: D27668523
fbshipit-source-id: 4beba6ddf247801368c3e1c24a0a4956490d45cd
Summary:
When registerByteEvent is called for a particular stream ID and offset, it is
possible that the byte event has already occurred in the past. In this case, we
schedule a lambda to be executed in the *next* iteration of the event loop, to
deliver the byte event to the callback.
There are 2 bugs in the current implementation of runEvbAsync which executes
the scheduled lambda.
1. It is possible that multiple recipients register for the same byte event on
the same stream ID and offset. If one of the recipients is already delivered
the byte event, then there is a possibility that the lambda can redeliver the
same notification a second time, which is not desirable.
2. There is a race condition where-in between the time the lambda was scheduled to
the time the lambda was excecuted, it is possible that the byte event gets
delivered when we process callbacks on receiving network data. In this
situation, there is a bug in the lambda where-in if there is another byte event
registered for any other offset for the same stream ID (regardless of the
callback), we will falsely deliver the byte event that was requested, resulting
in double delivery of the same byte event.
This commit fixes both bugs by checking that the requested byte event still
exists in the ByteEventDetail deque (comparing the offset as well as callback),
before erasing and delivering it.
Reviewed By: bschlinker
Differential Revision: D27846813
fbshipit-source-id: 099b79af2580fba5561ec33c4be45a8cf84c39e8
Summary: The kitchen sink must grow. Nice to know when the conn ends what the version was.
Reviewed By: lnicco
Differential Revision: D27868680
fbshipit-source-id: 2770ed2c647d76affbb52597a5a2a6720eabfe66
Summary:
This diff adds error callback API during the time PeekCallback() is applied, following D9724616, D988728, D15312250, design doc https://fb.quip.com/8P9WAaF43VOn
The peekError() API implementation is added into unidirectional stream dispatcher, class HQUnidirStreamDispatcher.
Reviewed By: mjoras
Differential Revision: D27514260
fbshipit-source-id: b65cc0e46a5d520f46f9dc218c9a65be99e6df55
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:
This commit demonstrates the use of ByteEvents in maps. Intentionally didn't define the custom comparator and hash function inside the quic library because we want consumers of ByteEvents to define equality of 2 ByteEvents in their own way.
This will help recipients of ByteEventCallback keep track of pending,
received and cancelled ByteEvents easily.
In addition, the behavior of registerByteEventCallback was modified to NOT allow duplicate registrations. A pair of registrations is considered duplicate if they both have the same stream ID, stream offset, ByteEvent type and recipient callback pointer. In such cases, registerByteEventCallback returns an INVALID_OPERATION error.
This is critical to make sure that ByteEvents work well in maps, which may be used by recipients.
Reviewed By: bschlinker
Differential Revision: D27100623
fbshipit-source-id: 084a4765fa6c98fdc1b98414fbd30582cf1e5139
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. 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:
Add an onByteEventRegistered callback that will be invoked when the
registration for a ByteEvent callback is successful.
This makes it easy for recipients of the callback to track the successful
registration AND callback recipt in a single place (the recipient class). For
example, if all successfully registered ByteEvents have been received, the
recipient can destroy itself safely, knowing that no more ByteEvents will be
sent to it.
Reviewed By: bschlinker
Differential Revision: D27100624
fbshipit-source-id: dbfeff1f4cf2367587fdb73cbd334165b3b159de
Summary: If we idle timeout when there are non control streams open, there could be some sort of issue especially with protocols like HTTP/3.
Reviewed By: yangchi
Differential Revision: D27017861
fbshipit-source-id: 319b839a641fa417a5026adf607c7bd0070cb66c
Summary:
Previously, we only had support for notifying observers when a QUIC socket
became application rate limited.
This change adds a similar notification when a socket does not become
application rate limited, i.e when the application *starts* writing data to the
socket - either for the first time on a newly established socket or after a
previous block of writes were written and the app had no more to write.
In addition, we include the number of outstanding packets (only those that are
carrying app data in them) so that observers can use this data to timestamp the
start and end of periods where the socket performs app data writes.
Reviewed By: yangchi
Differential Revision: D26559598
fbshipit-source-id: 0a8df7082b83e2ffad9b5addceca29cc03897243
Summary:
Instead of writing real data into the transport, we want to support a
use case where only its metadata is written to the transport. Sending of the
real data is delegated to another entity in such setup.
Reviewed By: mjoras
Differential Revision: D26131772
fbshipit-source-id: 4fcfa3a1626203f63c61898e6de089a3079d043d