1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-11-24 04:01:07 +03:00
Commit Graph

120 Commits

Author SHA1 Message Date
Joseph Beshay
b45c82b884 ACK_EXTENDED frame support
Summary:
This introduces a new frame type for acks (ACK_EXTENDED) that can carry optional fields depending on the features supported by the peer. The currently supported features set will include ECN count fields, and Receive Timstamp fields. This enables a quic connection to report both ECN counts and receive timestamps, which is not possible otherwise because they use different frame types.

Support for the extended ack as well as the set of features that can be included in it is negotiated through a new transport parameter (extended_ack_supported = 0xff0a004). Its value indicates which features are supported by the local transport. The value is an integer which is evaluated against the following bitmasks:
```
  ECN_COUNTS = 0x01,
  RECEIVE_TIMESTAMPS = 0x02,
```

This diff introduces the transport parameter and negotiates the supported features between the peers of the connection. The parameter is cached in the psk cache so the client can remember the server config. It is also encoded inside the 0-rtt ticket so the server can reject it if its local config has changed.

The following diffs add reading and writing the frame itself.

The ACK_EXTENDED frame itself will have the following format
```
ACK_EXTENDED Frame {
  Type (i) = 0xB1
  // Fields of the existing ACK (type=0x02) frame:
  Largest Acknowledged (i),
  ACK Delay (i),
  ACK Range Count (i),
  First ACK Range (i),
  ACK Range (..) ...,
  Extended Ack Features (i),
  // Optional ECN counts (if bit 0 is set in Features)
  [ECN Counts (..)],
  // Optional Receive Timestamps (if bit 1 is set in Features)
  [Receive Timestamps (..)]
}

// Fields from the existing ACK_ECN frame
ECN Counts {
  ECT0 Count (i),
  ECT1 Count (i),
  ECN-CE Count (i),
}

// Fields from the existing ACK_RECEIVE_TIMESTAMPS frame
Receive Timestamps {
  Timestamp Range Count (i),
  Timestamp Ranges (..) ...,
}

Timestamp Range {
  Gap (i),
  Timestamp Delta Count (i),
  Timestamp Delta (i) ...,
}
```

Reviewed By: sharmafb

Differential Revision: D68931151

fbshipit-source-id: 44c8c83d2f434abca97c4e85f0fa7502736cddc1
2025-02-24 12:32:50 -08:00
Aman Sharma
e23203533a Remove versions of writeStreamFrameData that operate on Buf and BufQueue
Summary: This is a step in the process of moving away from `folly::IOBuf` in our codebase

Reviewed By: jbeshay

Differential Revision: D68731820

fbshipit-source-id: b8e6f86cceb99d5fa5450f18ad5116d05ae9e13c
2025-01-29 16:16:16 -08:00
Nicholas Ormrod
37cf3495d8 namespace-concat in fbcode/quic
Summary:
Concatenate adjacent namespaces + format

This is a codemod. It was automatically generated and will be landed once it is approved and tests are passing in sandcastle.
You have been added as a reviewer by Sentinel or Butterfly.

Autodiff project: nc
Autodiff partition: fbcode.quic
Autodiff bookmark: ad.nc.fbcode.quic

Reviewed By: hanidamlaj

Differential Revision: D65365244

fbshipit-source-id: 0bbaa7684d03caf8fc8eff3439a0865940398220
2024-11-01 18:34:56 -07:00
Aman Sharma
2369ecb69b Use iovec instead of IOBuf in QuicAsyncUDPSocket::write and QuicAsyncUDPSocket::writeGSO
Summary: See title

Reviewed By: mjoras

Differential Revision: D61048705

fbshipit-source-id: 60dc63cc67f63be6f0ac6cbe0e766172a8c79d7c
2024-10-02 15:13:23 -07:00
Aman Sharma
bc386475e5 Integrate RangeChain into write path of QUIC stack
Summary: See title

Reviewed By: mjoras

Differential Revision: D58216871

fbshipit-source-id: 9afc08946a676ec967c998416a6470d4884af550
2024-08-15 05:46:08 -07:00
Zale Young
9262e38151 Implement hash and hmac in fizz
Summary:
- implement `hash` and `hmac` functions in fizz using the `hash_init()`, `hash_update()` and `hash_final()` functions from the Hasher primitive.
- change the `openssl::Sha<T>::hash` and `hmac` functions to simply call to fizz's implementation. This will be removed soon, once callsites are updated to simply reference fizz's implementations directly.

Reviewed By: mingtaoy

Differential Revision: D59351668

fbshipit-source-id: 609869ab5f1e6095758e95c46bf24a9b1f86c988
2024-07-11 07:06:27 -07:00
Matt Joras
aefc9e369b Introduce quic::Optional
Summary:
The idea here is to make it so we can swap out the type we are using for optionality. In the near term we are going to try swapping towards one that more aggressively tries to save size.

For now there is no functional change and this is just a big aliasing diff.

Reviewed By: sharmafb

Differential Revision: D57633896

fbshipit-source-id: 6eae5953d47395b390016e59cf9d639f3b6c8cfe
2024-06-11 11:02:02 -07:00
Matt Joras
6ce29d58ff remove isHandshake
Summary: This is effectively an unused field encoding duplicated information, but it was widespread.

Reviewed By: kvtsoy

Differential Revision: D57289922

fbshipit-source-id: ca1499e2576e5ae28e3880b865a29c2b8d9a3d1b
2024-06-11 11:02:02 -07:00
Gabriel Schell
433d82d614 Move default cert flag to a separate method
Summary: Reintroduction of D57737100 with fix for Thrift build

Differential Revision: D57970247

fbshipit-source-id: 55815037522533aadf01f04813a4edb6b050a30b
2024-05-30 14:03:06 -07:00
Aristidis Papaioannou
3658ca221c Revert D57737100: Move default cert flag to a separate method
Differential Revision:
D57737100

Original commit changeset: 166163ddd721

Original Phabricator Diff: D57737100

fbshipit-source-id: 9d62da7f00ab36a3250a82366dc30b4467eacf5a
2024-05-29 22:23:43 -07:00
Gabriel Schell
05b4eb9238 Move default cert flag to a separate method
Summary: Changed the Interfaces and Implementations of CertManager and DelegatedCertManager to include a new default method.

Reviewed By: AjanthanAsogamoorthy

Differential Revision: D57737100

fbshipit-source-id: 166163ddd7213c49bb58fadf72b6b314adfaed10
2024-05-29 15:48:34 -07:00
Joseph Beshay
9add439090 Maitain ingress ECN counts in the AckState for writing ACKs
Summary:
Keep track of ECN values in incoming packets and update the AckState that is used for echoing the counts to the peer in ACK_ECN frames.

Reading and writing the ACK_ECN frames is in the next change.

Reviewed By: kvtsoy

Differential Revision: D54927783

fbshipit-source-id: 44471c6224ee8578aaacc0d1a1c54370ef6d2ffe
2024-05-15 11:51:15 -07:00
Joseph Beshay
7f2cb16408 Refactor QuicClient and Server transports to process ReceivedUdpPackets with attached metadata
Summary: ReceivedUdpPacket has attached metadata (currently timings and later in the stack, tos values). Rather than passing the metadata separately in the read path for the client and the server, this propagates the ReceivedUdpPacket further up so this metadata is easier to use in the rest of the stack.

Reviewed By: mjoras

Differential Revision: D54912162

fbshipit-source-id: c980d5b276704f5bba780ac16a380bbb4e5bedad
2024-05-09 11:05:32 -07:00
Zale Young
f03879a771 4/n update fizz dependencies in quic
Summary: `fizz::`-> `fizz::openssl::`

Reviewed By: rnandan08

Differential Revision: D56609792

fbshipit-source-id: 182991321ef43bba95a898b3d3166c281f6a220c
2024-05-06 16:10:17 -07:00
Aman Sharma
e0892777c3 Remove excessive moving/copying of outstanding packets [1/n]
Summary:
I'm doing the following things in this diff, with the purpose of reducing the number of copies/moves of outstanding packets:
* Making the `AckPacket::builder` take in pointers to structures rather than moving them (the moves are in effect copies, since there's no indirection within the structures)

Reviewed By: jbeshay

Differential Revision: D55542406

fbshipit-source-id: aa64f89a3540d898343e2154ced976524f476960
2024-04-19 13:56:44 -07:00
Aman Sharma
9cdb1cd251 Remove unused fields in OutstandingPacketMetadata [2/n]
Summary: Neither QUIC not TransportMonitor is using the `packetsInflight` field of the `OutstandingPacketMetadata`

Reviewed By: hanidamlaj

Differential Revision: D55926288

fbshipit-source-id: 32efd66add1e6374a8d3446ff635fe582b36e644
2024-04-12 16:29:04 -07:00
Aman Sharma
5fc474a2f9 Remove unused fields in OutstandingPacketMetadata [1/n]
Summary: Neither QUIC not TransportMonitor is using the `totalBodyBytesSent` field of the `OutstandingPacketMetadata`

Reviewed By: hanidamlaj

Differential Revision: D55897240

fbshipit-source-id: 521f8a016f838dd1fe0593daa7a4e45c4fd222cf
2024-04-12 14:19:16 -07:00
Aman Sharma
303c405e10 Create IOBuf on the stack using folly::IOBuf::wrapBufferAsValue for body
Summary: This is similar to the previous commit. We use a stack-based IOBuf instead of allocating one on the heap. This saves CPU because allocations/deallocations on the stack are a lot cheaper.

Reviewed By: hanidamlaj

Differential Revision: D53101722

fbshipit-source-id: dd59a7eca6498db19472a62f954db3e2f2f27a42
2024-01-29 15:19:12 -08:00
Aman Sharma
b2db063139 Create IOBuf on the stack using folly::IOBuf::wrapBufferAsValue for headers
Summary: Creating an IOBuf on the heap when we use `folly::IOBuf::wrapBuffer` is expensive.

Reviewed By: hanidamlaj

Differential Revision: D52506216

fbshipit-source-id: eed2b77beae0419b542b0461303785cc175e3518
2024-01-26 14:01:08 -08:00
Kyle Nekritz
364942346a Use ALPN in QUIC tests.
Summary: This is in preparation for enforcing ALPN requirement in the QUIC layer.

Reviewed By: mjoras

Differential Revision: D51285567

fbshipit-source-id: 3cc3c2fbde169ec9603b3309d0abda74aa698602
2024-01-26 10:55:13 -08:00
Zale Young
0ec82b1e74 split out CertUtils and (Peer|Self)CertImpl from Certificate.h
Summary:
As part of the ongoing work to create a version of fizz that does depend on OpenSSL, this diff begins the refactors of `Certificate.h`. In this diff:
- since `CertUtils`, `PeerCertImpl` and `SelfCertImpl` need to depend on OpenSSL, move them to different translation units than the `PeerCert` and `SelfCert` interfaces
- rename `(Peer|Self)CertImpl` to `OpenSSL(Peer|Self)CertImpl`

Main changes are in:
- `protocol/Certificate.h` and `.cpp`, deleted `-inl.h`
- `protocol/OpenSSLSelfCertImpl.h` and `-inl.h` (split from Certificate.h and renamed from SelfCertImpl)
- `protocol/OpenSSLPeerCertImpl.h` and `-inl.h` (split from Certificate.h and renamed from PeerCertImpl)
- `protocol/CertUtils.h`, `.cpp`and`-inl.h` (Split from Certificate.h)
- `protocol/TARGETS` and `BUCK` - new targets `protocol:cert_utils`, `protocol:openssl_self_cert_impl`, `protocol:openssl_peer_cert_impl`
- added new includes and targets to various dependencies.

In the next diff, we'll use a compler flag `#if` guard the PeerCert and SelfCert interfaces to conditionally inherit from OpenSSL. This makes it so that the `Factory` interface and `ClientProtocol` do not need to depend on OpenSSL.
We'll also create a buck config to conditionally select the openssl depednencies (and pass in the compiler flag).

Reviewed By: mingtaoy

Differential Revision: D51866623

fbshipit-source-id: d91cc4a39cabc2ac6879f16845ab8a7487d64c11
2024-01-12 12:18:16 -08:00
Joseph Beshay
ead139adef Move all mvfst use-cases to the new Eventbase, Timer, and Socket interfaces
Summary:
This is the major transition that updates mvfst code to use the new interfaces. The new Folly implementations of the interfaces maintain all the existing behavior of folly types so this should not introduce any functional change. The core changes are:
- Update the BatchWriters to use the new interfaces.
- Update the FunctionLooper to use the new interfaces.
- Change QuicServerTransport to take the folly types and wrap them in the new types for use in the QuicTransportBase.

The rest of the diff is for updating all the existing uses of the QuicTrasnport to initialize the necessary types and pass them to the QUIC transport instead of directly passing folly types.

Reviewed By: mjoras

Differential Revision: D51413481

fbshipit-source-id: 5ed607e12b9a52b96148ad9b4f8f43899655d936
2023-12-14 00:24:12 -08:00
Brandon Schlinker
83ad2ad99d Cleanup and modularize receive path, improve timestamp support [20/x]
Summary:
This diff renames `ReceivedPacket` to `ReceivedUdpPacket` to clarify that it maps to a UDP packet and not a QUIC packet. A single UDP packet can contain multiple QUIC packets due to coalescing.

--

This diff is part of a larger stack focused on the following:

- **Cleaning up client and server UDP packet receive paths while improving testability.** We currently have multiple receive paths for client and server. Capabilities vary significantly and there are few tests. For instance:
  - The server receive path supports socket RX timestamps, abet incorrectly in that it does not store timestamp per packet. In comparison, the client receive path does not currently support socket RX timestamps, although the code in `QuicClientTransport::recvmsg` and `QuicClientTransport::recvmmsg` makes reference to socket RX timestamps, making it confusing to understand the capabilities available when tracing through the code. This complicates the tests in `QuicTypedTransportTests`, as we have to disable test logic that depends on socket RX timestamps for client tests.
  - The client currently has three receive paths, and none of them are well tested.

- **Modularize and abstract components in the receive path.** This will make it easier to mock/fake the UDP socket and network layers.
  - `QuicClientTransport` and `QuicServerTransport` currently contain UDP socket handling logic that operates over lower layer primitives such `cmsg` and `io_vec` (see `QuicClientTransport::recvmmsg` and `...::recvmsg` as examples).
  - Because this UDP socket handling logic is inside of the mvfst transport implementations, it is difficult to test this logic in isolation and mock/fake the underlying socket and network layers. For instance, injecting a user space network emulator that operates at the socket layer would require faking `folly::AsyncUDPSocket`, which is non-trivial given that `AsyncUDPSocket` does not abstract away intricacies arising from the aforementioned lower layer primitives.
  - By shifting this logic into an intermediate layer between the transport and the underlying UDP socket, it will be easier to mock out the UDP socket layer when testing functionality at higher layers, and inject fake components when we want to emulate the network between a mvfst client and server. It will also be easier for us to have unit tests focused on testing interactions between the UDP socket implementation and this intermediate layer.

- **Improving receive path timestamping.** We only record a single timestamp per `NetworkData` at the moment, but (1) it is possible for a `NetworkData` to have multiple packets, each with their own timestamps, and (2) we should be able to record both userspace and socket timestamps.

Reviewed By: silver23arrow

Differential Revision: D48788809

fbshipit-source-id: 3793c30212d545e226f3e5337289bc2601dfa553
2023-11-28 07:47:57 -08:00
Brandon Schlinker
d9f0575c62 Cleanup and modularize receive path, improve timestamp support [19/x]
Summary:
This diff renames `updateLargestReceivedPacketNum` to `addPacketToAckState`, and changes the function signature so that it takes the structure of `ReceivedPacket::Timings` instead of just a single receive timestamp. This allows us to plumb through steady clock and socket timestamps.

The current caller to `updateLargestReceivedPacketNum` had a `ReceivedPacket::Timings` and just passed the`receiveTimePoint` field from that structure to to `updateLargestReceivedPacketNum`

```
uint64_t distanceFromExpectedPacketNum = updateLargestReceivedPacketNum(
    conn, ackState, packetNum, readData.udpPacket.timings.receiveTimePoint);
```

Now, we just pass the entire `ReceivedPacket::Timings` structure and extract `receiveTimePoint` inside:

```
uint64_t distanceFromExpectedPacketNum = addPacketToAckState(
    conn, ackState, packetNum, readData.udpPacket.timings);
```

--

This diff is part of a larger stack focused on the following:

- **Cleaning up client and server UDP packet receive paths while improving testability.** We currently have multiple receive paths for client and server. Capabilities vary significantly and there are few tests. For instance:
  - The server receive path supports socket RX timestamps, abet incorrectly in that it does not store timestamp per packet. In comparison, the client receive path does not currently support socket RX timestamps, although the code in `QuicClientTransport::recvmsg` and `QuicClientTransport::recvmmsg` makes reference to socket RX timestamps, making it confusing to understand the capabilities available when tracing through the code. This complicates the tests in `QuicTypedTransportTests`, as we have to disable test logic that depends on socket RX timestamps for client tests.
  - The client currently has three receive paths, and none of them are well tested.

- **Modularize and abstract components in the receive path.** This will make it easier to mock/fake the UDP socket and network layers.
  - `QuicClientTransport` and `QuicServerTransport` currently contain UDP socket handling logic that operates over lower layer primitives such `cmsg` and `io_vec` (see `QuicClientTransport::recvmmsg` and `...::recvmsg` as examples).
  - Because this UDP socket handling logic is inside of the mvfst transport implementations, it is difficult to test this logic in isolation and mock/fake the underlying socket and network layers. For instance, injecting a user space network emulator that operates at the socket layer would require faking `folly::AsyncUDPSocket`, which is non-trivial given that `AsyncUDPSocket` does not abstract away intricacies arising from the aforementioned lower layer primitives.
  - By shifting this logic into an intermediate layer between the transport and the underlying UDP socket, it will be easier to mock out the UDP socket layer when testing functionality at higher layers, and inject fake components when we want to emulate the network between a mvfst client and server. It will also be easier for us to have unit tests focused on testing interactions between the UDP socket implementation and this intermediate layer.

- **Improving receive path timestamping.** We only record a single timestamp per `NetworkData` at the moment, but (1) it is possible for a `NetworkData` to have multiple packets, each with their own timestamps, and (2) we should be able to record both userspace and socket timestamps.

Differential Revision: D48785086

fbshipit-source-id: 48a424e3e27918a8efe41918e0bcfa57337d9337
2023-11-28 07:47:57 -08:00
Hani Damlaj
4eefa3bebd remove QuicVersion::QUIC_DRAFT
Summary: - as title, deprecate QuicVersion::QUIC_DRAFT

Reviewed By: jbeshay

Differential Revision: D50466992

fbshipit-source-id: 2b2776d142142bc07fd27053a68dd635959a7f31
2023-11-09 19:30:07 -08:00
Brandon Schlinker
a1445434b0 Cleanup and modularize receive path, improve timestamp support [5/x]
Summary:
This diff changes `QuicAsyncUDPSocketWrapper` so that it is an abstraction layer that inherits from `QuicAsyncUDPSocketType`, instead of simply being a container with aliases.
- Key changes in `QuicAsyncUDPSocketWrapper.h`, the rest of the updates switch us from using `QuicAsyncUDPSocketType` to `QuicAsyncUDPSocketWrapper`.
- It's difficult to mock the UDP socket today given that we expose the entire `folly::AsyncUDPSocket` type to the higher layers of the QUIC stack. This complicates testing and emulation because any mock / fake has to implement low level primitives like `recvmmsg`, and because the `folly::AsyncUDPSocket` interface can change over time.
- Pure virtual functions will be defined in `QuicAsyncUDPSocketWrapper` in a follow up diff to start creating an interface between the higher layers of the mvfst QUIC stack and the UDP socket, and this interface will abstract away lower layer details such as `cmsgs` and `io_vec`, and instead focus on populating higher layer structures such as `NetworkData` and `ReceivedPacket` (D48714615). This will make it easier for us to mock or fake the UDP socket.

This diff relies on changes to `folly::MockAsyncUDPSocket` introduced in D48717389.

--

This diff is part of a larger stack focused on the following:

- **Cleaning up client and server UDP packet receive paths while improving testability.** We currently have multiple receive paths for client and server. Capabilities vary significantly and there are few tests. For instance:
  - The server receive path supports socket RX timestamps, abet incorrectly in that it does not store timestamp per packet. In comparison, the client receive path does not currently support socket RX timestamps, although the code in `QuicClientTransport::recvmsg` and `QuicClientTransport::recvmmsg` makes reference to socket RX timestamps, making it confusing to understand the capabilities available when tracing through the code. This complicates the tests in `QuicTypedTransportTests`, as we have to disable test logic that depends on socket RX timestamps for client tests.
  - The client currently has three receive paths, and none of them are well tested.

- **Modularize and abstract components in the receive path.** This will make it easier to mock/fake the UDP socket and network layers.
  - `QuicClientTransport` and `QuicServerTransport` currently contain UDP socket handling logic that operates over lower layer primitives such `cmsg` and `io_vec` (see `QuicClientTransport::recvmmsg` and `...::recvmsg` as examples).
  - Because this UDP socket handling logic is inside of the mvfst transport implementations, it is difficult to test this logic in isolation and mock/fake the underlying socket and network layers. For instance, injecting a user space network emulator that operates at the socket layer would require faking `folly::AsyncUDPSocket`, which is non-trivial given that `AsyncUDPSocket` does not abstract away intricacies arising from the aforementioned lower layer primitives.
  - By shifting this logic into an intermediate layer between the transport and the underlying UDP socket, it will be easier to mock out the UDP socket layer when testing functionality at higher layers, and inject fake components when we want to emulate the network between a mvfst client and server. It will also be easier for us to have unit tests focused on testing interactions between the UDP socket implementation and this intermediate layer.

- **Improving receive path timestamping.** We only record a single timestamp per `NetworkData` at the moment, but (1) it is possible for a `NetworkData` to have multiple packets, each with their own timestamps, and (2) we should be able to record both userspace and socket timestamps.

Reviewed By: jbeshay, hanidamlaj

Differential Revision: D48717388

fbshipit-source-id: 4f34182a69ab1e619e454da19e357a6a2ee2b9ab
2023-09-21 07:57:58 -07:00
Konstantin Tsoy
264bf20d9a Update flow control settings names to reflect that these are indeed flow
Summary: Update flow control settings names to reflect that these are indeed flow control

Reviewed By: jbeshay

Differential Revision: D48137685

fbshipit-source-id: a48372e21cdd529480e25785a9bd5de456427ef3
2023-08-18 10:21:24 -07:00
Konstantin Tsoy
4a0dd1e2a4 QuicAsyncUDPSocketWrapper
Reviewed By: jbeshay

Differential Revision: D46379200

fbshipit-source-id: f6a7c1cf68108872e05e6fd8adb7f00aae22b2ed
2023-07-11 15:21:15 -07:00
Matt Joras
e3d67efb5a Track a sequence number for non-DSR packets.
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
2023-06-27 16:56:11 -07:00
Matt Joras
6b79afa978 Include cipher overhead in DSR packet metadata
Summary: This is a pretty glaring omission. Prior to this we were writing packets of the correct size to the wire, but recording their length in the metadata incorrectly, discounting the cipher overhead. In practice this means a discount of 16 bytes for every DSR packet, which likely leads to lower bandwidth estimations and other problematic CCA effects, by ~1.3%.

Reviewed By: kvtsoy

Differential Revision: D46659896

fbshipit-source-id: 255d3e9f490d2ed3c2d41aa62f5027338457731b
2023-06-12 17:02:16 -07:00
Matt Joras
35a2d34843 Use a single queue for scheduling DSR and non-DSR streams.
Summary:
The write loop functions for DSR or non-DSR are segmented today. As such, so are the schedulers. Mirroring this, we also currently store the DSR and non-DSR streams in separate write queues. This makes it impossible to effectively balance between the two without potential priority inversions or starvation.

Combining them into a single queue eliminates this possibility, but is not entirely straightforward.

The main difficulty comes from the schedulers. The `StreamFrameScheduler` for non-DSR data essentially loops over the control stream queue and the normal write queue looking for the next stream to write to a given packet. When the queues are segmented things are nice and easy. When they are combined, we have to deal with the potential that the non-DSR scheduler will hit a stream with only DSR data. Simply bailing isn't quite correct, since it will just cause an empty write loop. To fix that we need check, after we are finished writing a packet, if the next scheduled stream only has DSR data. If it does, we need to ensure `hasPendingData()` returns false.

The same needs to be done in reverse for the DSR stream scheduler.

The last major compication is that we need another loop which wraps the two individual write loop functions, and calls both functions until the packet limit is exhausted or there's no more data to write. This is to handle the case where there are, for example, two active streams with the same incremental priority, and one is DSR and the other is not. In this case each write loop we want to write `packetLimit` packets, flip flopping between DSR and non DSR packets.

This kind of round robining is pathologically bad for DSR, and a future diff will experiment with changing the round robin behavior such that we write a minimum number of packets per stream before moving on to the next stream.

This change also contains some other refactors, such as eliminating `updateLossStreams` from the stream manager.

(Note: this ignores all push blocking failures!)

Reviewed By: kvtsoy

Differential Revision: D46249067

fbshipit-source-id: 56a37c02fef51908c1336266ed40ac6d99bd14d4
2023-06-01 14:11:31 -07:00
Matt Joras
5fb246b019 Implement duplicate packet stat.
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
2023-04-20 11:48:45 -07:00
Matt Joras
bc1277caa0 Store less metadata for padding frames.
Summary:
Padding frames by definition have no semantics. There also can be quite a large number of them, which makes their storage expensive.

Optimize for this by effectively coalescing multiple contiguous padding frames into a single frame.

(Note: this ignores all push blocking failures!)

Reviewed By: kvtsoy

Differential Revision: D44636497

fbshipit-source-id: 4c1cd88123c4fdf40ec9c553885f46669f7326da
2023-04-18 11:47:11 -07:00
Ilango Purushothaman
e1fd9c7880 Create OutstandingPacket wrapper
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
2023-03-15 03:10:18 -07:00
Konstantin Tsoy
377260f704 Remove d6d code
Summary: we're not using it

Reviewed By: mjoras

Differential Revision: D43482344

fbshipit-source-id: 05ac6792848e32e7c1bcf53a2df172852b5def62
2023-02-23 20:11:24 -08:00
Duc Nguyen
e3d2b6b3ee Fix -Wmissing-prototypes error
Reviewed By: bschlinker

Differential Revision: D40363029

fbshipit-source-id: 27b41f99b5d279f4e204561b3e93b179345ec784
2022-10-18 09:30:47 -07:00
Joseph Beshay
bd56a9bd4e Fix logic for handling the ACK_FREQUENCY frame reorder threshold
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
2022-09-08 11:15:23 -07:00
Konstantin Tsoy
0df2e6ba17 Check that certain test util functions write crypto frame successfully
Summary: Check that certain test util functions write crypto frame successfully

Reviewed By: mjoras

Differential Revision: D36604546

fbshipit-source-id: 39e8d9b7fe4080679d0401ddf4c6ef17e8b33d7b
2022-05-23 13:20:09 -07:00
Brandon Schlinker
8b83739e89 Add largestAckedPacket to AckEvent
Summary:
`largestAcked` is a field in the `AckFrame` defined in section 19.3 ("ACK Frames") of RFC9000. This diff adds it to the `AckFrame` as `largestAckedPacket` after renaming the existing field of the same name to `largestNewlyAckedPacket` in D34896383

Some changes to the ACK unit tests were made as part of this change to ensure we're tracking everything correctly.

Reviewed By: mjoras

Differential Revision: D34951525

fbshipit-source-id: c84262a24a93bc05c8c817c5e8d5da229d971e36
2022-03-28 23:26:30 -07:00
Brandon Schlinker
f6b0417eee Rename largestAckedPacket to largestNewlyAckedPacket in AckEvent
Summary:
`largestAcked` is a field in the `AckFrame` defined in section 19.3 ("ACK Frames") of RFC9000.

In `AckEvent`, the field we currently call `largestAckedPacked` is not the same as this field — it actually represents the "largest newly ACKed (outstanding) packet" processed during that call to `processAckFrame`. Changing the name of the field to `largestNewlyAckedPacket` to make this clear and avoid confusion.

The next diff in stack adds the proper `largestAcked` field from the `AckFrame` itself.

Reviewed By: mjoras

Differential Revision: D34896383

fbshipit-source-id: 2a9bda5661e38a880a4fe56dd9881cb27575ed08
2022-03-28 23:26:30 -07:00
Brandon Schlinker
28932d2bf8 Add RTT sample w/o delay to AckEvent, remove min check
Summary:
This diff makes two changes to the RTT samples stored in `AckEvent`.

(1) `AckEvent` currently contains the RTT sample associated with the event. This diff adds two new related fields:
- a field that contains the ACK delay provided in the `AckFrame`
- a field that contains the RTT sample with the ACK delay subtracted from it

(2) The logic in `AckHandlers.cpp` currently updates `ack.mrttSample` if the RTT sample being considered is less than the current value of `ack.mrttSample`:

```
// update AckEvent RTTs, which are used by CCA and other procesisng
ack.mrttSample =
    std::min(ack.mrttSample.value_or(rttSample), rttSample);
```

However, this is superfluous at best and confusing in general:
- There is a 1:1 relationship between an `AckEvent` and an `AckFrame`
- Each `AckFrame` can result in at most one RTT measurement

Thus, there can be only one RTT measurement per `AckEvent`, and we do not need to perform this comparison when updating `ack.mrttSample`

Reviewed By: mjoras

Differential Revision: D32588081

fbshipit-source-id: b63673a5bdd9c2830083e97572d422647932e65b
2022-03-28 23:26:30 -07:00
Brandon Schlinker
feb8455dc6 Add RTT with ACK delay removed to TransportInfo
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
2022-03-28 23:26:30 -07:00
Sotirios Delimanolis
c90402a54f Refactor from Future to SemiFuture
Summary: Migrating usage of `folly::Future` to `folly::SemiFuture`.

Reviewed By: knekritz

Differential Revision: D31024165

fbshipit-source-id: 4dcb2f401cea37556345f00eff04c7fd610e5eb6
2022-02-23 13:36:55 -08:00
Konstantin Tsoy
cecc1ba279 Introduce QuicError struct
Summary: Instead of using std::pair everywhere

Reviewed By: mjoras

Differential Revision: D34146686

fbshipit-source-id: dfe48f43775de868aba06a5b9b5a004e5793bdbb
2022-02-14 16:00:21 -08:00
Konstantin Tsoy
3e6af08a0f Separate QuicReadCodec::parseLongHeaderPacket() into two functions
Summary: Separate QuicReadCodec::parseLongHeaderPacket() into two functions

Reviewed By: mjoras

Differential Revision: D33959155

fbshipit-source-id: 8f3f76ab6ef44ffa4ae6c7aecbe3c734aab97f0e
2022-02-07 22:14:09 -08:00
Hani Damlaj
00e67c1bf9 mvfst License Header Update
Reviewed By: lnicco

Differential Revision: D33587012

fbshipit-source-id: 972eb440f0156c9c04aa6e8787561b18295c1a97
2022-01-18 13:56:12 -08:00
Hani Damlaj
2660a288b3 Update Company Name
Summary: - as title

Reviewed By: lnicco

Differential Revision: D33513410

fbshipit-source-id: 282b6f512cf83b9abb7990402661135b658f7bd1
2022-01-13 12:07:48 -08:00
Brandon Schlinker
0513d392c0 PacketNum in AckEvent
Summary: Adds the packet number to `AckEvent::AckPacket`.

Reviewed By: jbeshay, mjoras

Differential Revision: D31916955

fbshipit-source-id: bfda025b1fd5bdbe04d5a0ff69387fd53ee6cbe3
2021-12-12 17:04:46 -08:00
Brandon Schlinker
8cd4d11fac PacketNumberSpace in AckEvent
Summary:
Adds the packet number space to `AckEvent`.

In tandem, adopts a builder pattern for `AckEvent` to ensure that expected fields are populated.

Reviewed By: mjoras

Differential Revision: D31365819

fbshipit-source-id: 94e9bb4b9b6386d9e3b572819a070e556aca5970
2021-12-12 17:04:46 -08:00
Brandon Schlinker
489c0175da Store stream details in AckEvent
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
2021-12-12 17:04:46 -08:00