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

172 Commits

Author SHA1 Message Date
Max Motovilov
db6e2321f9 QuicServerTransport now accepts abstract socket and event base in constructor
Summary: Replaced folly-specific argument types in constructor with abstract interfaces. Existing code unaffected.

Reviewed By: jbeshay, sharmafb

Differential Revision: D53493336

fbshipit-source-id: 67f5ef4a38995a2554084f2d75399b13887eb500
2024-02-23 07:33:00 -08:00
Joseph Beshay
702ce9e410 Key update support: Add quic transport stats for key update [5/x]
Summary: Adds new stats for: key updates initiated, received and succeeded.

Reviewed By: mjoras

Differential Revision: D53109623

fbshipit-source-id: 1c9473590a54135662a9cdd8c240a3225246c432
2024-02-01 15:41:27 -08:00
Joseph Beshay
aeacf40ae8 Key update support: Add support for initiating periodic key updates for both client and server [4/x]
Summary:
Allow the server/client transport to initiate periodic key update. It's defaulted to being disabled.

The new logic for initiating and verifying a key update was handled correctly by the peer is consolidated in QuicTransportFunctions.

Reviewed By: mjoras

Differential Revision: D53109624

fbshipit-source-id: 0c3a944978fc0e0a84252da953dc116aa7c26379
2024-02-01 15:41:27 -08:00
Joseph Beshay
cf6f6b98ec Fix active connection counter in QuicStats, and make it work for the client too
Summary:
The current open connection counting is broken. To fix it this change:
- moves the counter to be for both the server and client transport
- calls onNewConnection() when the transport is ready.
- calls onConnectionClose() when the transport is closed after it was ready.

Reviewed By: lnicco, frankfeir

Differential Revision: D52638628

fbshipit-source-id: bcff7a8c671f8eede53c22e698e1b95332c56959
2024-01-12 08:59:15 -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
Kyle Nekritz
8c68b18df3 Unconditionally defer control of writing session tickets to quic layer.
Summary: There is no reason to let the TLS layer control this.

Reviewed By: jbeshay

Differential Revision: D51267586

fbshipit-source-id: 6f70b7f6ba51a9a022195f7e2c1683b5323fde7a
2023-11-29 18:06:03 -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
Joseph Beshay
c1432e1c7b Do not update handshake-related transport settings from the 0-rtt ticket
Summary:
When the server accepts a 0-rtt ticket, it updates the connection's transport settings with the contents of the ticket. This is the value used included in the next ticket it sends the client. However, the handshake layer has a copy of the original transport parameters that was created with the first received packet. This copy in the handshake layer does not get updated. This can cause a mismatch between the value sent to the client in the handshake, and the value encoded inside the ticket.

This change avoid using the 0-rtt ticket for updating any transport settings that are also included in the handshake transport params.

Reviewed By: hanidamlaj

Differential Revision: D51121317

fbshipit-source-id: 55e71965185dff553d16d4c5fbcb1e2f9acdc690
2023-11-10 14:27:23 -08:00
Brandon Schlinker
04facac67d Cleanup and modularize receive path, improve timestamp support [15/x]
Summary:
This diff drops `NetworkDataSingle` in favor of `ReceivedPacket`. The latter contains a `ReceivedPacket::Timings` field that has the same `receiveTimePoint` currently in `NetworkDataSingle`, while also providing other useful signals.

--

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: D48739219

fbshipit-source-id: fc2cdb7b425d68c729dd3bec00b6c6ff3c4bf8ec
2023-11-05 19:58:46 -08:00
Brandon Schlinker
05b98a99db Cleanup and modularize receive path, improve timestamp support [11/x]
Summary:
This diff:
1. Introduces a new `ReceivedPacket::Timings` structure, which will be expanded upon in subsequent diffs.
2. Adds a `ReceivedPacket::Timings` field to each `ReceivedPacket`
3. Uses the accessors added in the previous diff (D48724715) to populate the `ReceivedPacket::Timings` structure in each `ReceivedPacket` held by a `NetworkData` object. This is done by propagating the `NetworkData::receiveTimePoint` field to all `ReceivedPacket` held in a `NetworkData.` This propagation occurs each time a `ReceivedPacket` is added. The value is propagated again if the `NetworkData::receiveTimePoint` field is updated by looping over all previously added `ReceivedPacket`.

--

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

Differential Revision: D48725209

fbshipit-source-id: 580e7d7d1f3587f9947774b5ed19e9985df404c9
2023-11-05 19:58:46 -08:00
Timm Böttger
5fca38ef00 Use MaybeManagedPtr for QuickSocket for optional lifetime management
Summary:
`connSetupCallback_` and `connCallback_` in `QuicTransportBase.h` are raw pointers, which delegates the responsibility to keep these callbacks alive to the caller.

There are use cases where it would be convenient to be able to tie the lifetime of the callback to the Quic transport, e.g,. as long as the Quic transport is alive, it keeps the callbacks alive as well.

This diff uses MaybeManagedPtr to achieve this lifetime tie if desired. A MaybeManagedPtr intialized with a shared pointer manages lifetime of the contained object, whereas a MaybeManagedPtr initialized with a raw pointer does not manage lifetime of the contained object. This way caller can decide to pass in a shared ptr or raw pointer and achieve the desired behavior.

Note that we cannot simply use a shared_ptr for that. Using a shared_ptr would potentially mean that callbacks passed are destroyed when the transport is destroyed. Callbacks would not be destroyed if they were managed by a shared_ptr already, but this is something we cannot assume for every case. This would thus be a change in semantics to the current implementation, where the callbacks can outlive the transport.

Reviewed By: mjoras

Differential Revision: D49502381

fbshipit-source-id: 771a9328b99dc4f94f8e9679f9caf98af9180428
2023-11-03 09:37:21 -07:00
Joseph Beshay
71e0934e13 Allow including a cwnd_hint in the 0-rtt app token
Summary: This enables the server to include a cwnd hint in the 0-rtt ticket it sends to the client.

Reviewed By: mjoras

Differential Revision: D43131826

fbshipit-source-id: 742e4e531027ec6618a1b761c450b507368e5a2f
2023-10-25 09:45:07 -07: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
Brandon Schlinker
086822ca76 Cleanup and modularize receive path, improve timestamp support [2/x]
Summary:
This diff:
- Changes `NetworkDataSingle` to have `ReceivedPacket` instead of `Buf`, in line with earlier change to `NetworkData` in D48714615

--

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: mjoras

Differential Revision: D48714796

fbshipit-source-id: d96c2abc81e7c27a01bcd0dd552f274a0c1ede26
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
Joseph Beshay
db728cd1a3 Add CONN_MIGRATION knob to enable/disable connection migration at the server
Summary: As title.

Reviewed By: kvtsoy

Differential Revision: D48246018

fbshipit-source-id: 9d905035d11d8d34586b5a7deaf0c7b379a39048
2023-08-16 10:24:01 -07:00
Christian Clauss
b8396fc119 Fix typos discovered by codespell
Summary:
`codespell --ignore-words-list=arithmetics,atleast,crate,crated,deriver,ect,hel,onl,startin,whats --skip="*.lock"`
* https://pypi.org/project/codespell

X-link: https://github.com/facebookincubator/mvfst/pull/307

Reviewed By: hanidamlaj, lnicco

Differential Revision: D47809078

Pulled By: kvtsoy

fbshipit-source-id: 566557f2389746db541ff265a5dec8d6404b3701
2023-07-26 17:10:41 -07:00
Konstantin Tsoy
73edee8252 Back out "Fix typos discovered by codespell"
Summary:
Original commit changeset: 337824bc37bc

Original Phabricator Diff: D47722462

Reviewed By: jbeshay, terrelln, lnicco

Differential Revision: D47801753

fbshipit-source-id: 795ffcccbc2223608e2a707ec2e5bcc7dd974eb3
2023-07-26 12:49:13 -07:00
Joseph Beshay
d85f726e36 Consolidate logic for setting the congestion controller in one function
Summary: Ensure all CCA-related settings are included in setCongestionControl(), and that it's called from the knob setting the congestion controller.

Reviewed By: kvtsoy

Differential Revision: D47741830

fbshipit-source-id: ff1d2347581c61a58f2caaff8189126930bf4e04
2023-07-25 19:48:01 -07:00
Facebook Community Bot
9d89b66485 Re-sync with internal repository 2023-07-25 09:45:22 -07:00
Joseph Beshay
cbc802ea1a Remove 3 unused quic transport knobs
Summary: As title.

Reviewed By: kvtsoy

Differential Revision: D47534110

fbshipit-source-id: f613c012004315134db114f9f0f026455c8d8d8d
2023-07-24 20:00:07 -07:00
Joseph Beshay
7db19c1f19 Add new CC_CONFIG knob to dynamically control CCA features
Summary:
Adds a new CC_CONFIG knob that carries a JSON blob that gets parsed into the transportSetting.ccaConfig struct.

Sample knob values:
```
{"52397": "{\"conservativeRecovery\": false}"}
{"52397": "{\"conservativeRecovery\": true, \"ackFrequencyConfig\":{\"minRttDivisor\": 77}}"}
```

Reviewed By: hanidamlaj

Differential Revision: D47309438

fbshipit-source-id: 0a4e941b9e4231bd333174472827044b1b49ac96
2023-07-14 14:55:29 -07:00
Joseph Beshay
471fa6f8dc Change BBRConfig to CongestionControlConfig
Summary: Rename transportSetting.BBRConfig to CongestionControlConfig to prepare it for being used by other CCAs too.

Reviewed By: hanidamlaj, mjoras

Differential Revision: D47309439

fbshipit-source-id: 56d0ddc9752789709c54a4f7cc595f94c656e49e
2023-07-14 14:55:29 -07:00
Konstantin Tsoy
4a0dd1e2a4 QuicAsyncUDPSocketWrapper
Reviewed By: jbeshay

Differential Revision: D46379200

fbshipit-source-id: f6a7c1cf68108872e05e6fd8adb7f00aae22b2ed
2023-07-11 15:21:15 -07:00
generatedunixname89002005287564
131da0d4bd quic_0
Reviewed By: DenisYaroshevskiy

Differential Revision: D47061673

fbshipit-source-id: c3e06ffc905015fa5e54a98dc67efe5732d700fe
2023-06-28 07:23:56 -07:00
Hani Damlaj
debb68639d remove WindowedCounter
Summary: as title, cleaning up some unused files & references

Reviewed By: mjoras

Differential Revision: D46843918

fbshipit-source-id: 08be5696d76f4f23df0f07bfd1ede5703d358691
2023-06-26 22:16:40 -07:00
Joseph Beshay
931abc64af Separate transport settings for pacing tick and the pacing timer resolution
Summary:
The pacingTimerTickInterval transport setting conflates two options: the minimum interval a pacer can use, and the resolution of the underlying timer. This means that a higher value leads to lower timer resolution and less pacing accuracy.

This change adds a separate parameter for the timer resolution to ensure that a larger pacing tick does not degrade the pacer accuracy.

Reviewed By: mjoras

Differential Revision: D46564066

fbshipit-source-id: 0d0e54077b80da03e6e6c9baaab49a4c969966b6
2023-06-20 15:18:38 -07:00
Konstantin Tsoy
adf16f9a07 Remove libccp from mvfst
Summary: We don't use it, and the OSS lib hasn't been updated in a while.

Reviewed By: mjoras

Differential Revision: D46707559

fbshipit-source-id: ec102a52183a736cfb1c0241600816a837062108
2023-06-15 18:17:53 -07:00
Konstantin Tsoy
dccfc706b5 QuicEventBase wrapper
Summary:
Create and use an actual wrapper around folly::EventBase.
Later an interface will be added that the wrapper will be implementing.

Reviewed By: jbeshay

Differential Revision: D45822401

fbshipit-source-id: 3b33f796c31043ec2881b753a9b60943bdf91f1d
2023-06-15 17:12:24 -07:00
Matt Joras
373b9e24ba Only discount full packets from the packet limit.
Summary:
Right now small packets will cause us to subtract from the packet limit during write loops. This is generally okay when using just non-DSR data. For DSR though there is an issue where if we only write a single small packet from the non-DSR path (like an ACK) we will discount it from the amount of packets the DSR path can write.

The existing workaround for this is to only discount the non-DSR path write if the number of bytes written is less than half a packet's worth. This heuristic is incomplete though and doesn't consider the cases like the following:
1. Packet limit is 5
2. Normal path writes 4 packets of size 1232, 1232, 1232, 20.
3. The DSR path only gets one packet.

The reason for this is because the normal path also wrote full packets, but ended with a short packet. The account for this we just have to discount in terms of full packet's written. In the above example, the DSR path would get the chance to write two packets. Thus we would exceed the packet limit by one packet, but it would get us closer to the desired amount of data on the wire.

Reviewed By: kvtsoy

Differential Revision: D46633105

fbshipit-source-id: 00d92499ab73fc746ea5fdb6ff31e10f06b98666
2023-06-13 16:42:05 -07:00
Matt Joras
8a7fedfe1d Add time-based QUIC_STATS
Summary:
The idea here is to add a notion of time-based sampling of certain QUIC_STATS. This allows accounting to be done via consistent distributions for comparisons.

For now limit to the server, and only implement for inflight bytes, SRTT, and CCA bandwidth.

Reviewed By: jbeshay

Differential Revision: D46410903

fbshipit-source-id: a5db1ec720a0f8bf54e04d66c0d68686660e8eaa
2023-06-12 09:48:10 -07:00
Matt Joras
6ecdb35ade Minimum packets per stream before next() moves forward
Summary:
The idea here is to allow a way to do incremental stream priorities while switching between streams as aggressively.

Achieving this is somewhat tricky. The easiest place to track this is to make it so the iterator in QuicPriorityQueue no-op next() until a counter reaches an increment.

This is especially impacftul for DSR, where round robining per packet is almost pathologically bad both for CPU impact but also spurious losses and low bandwidth estimates.

Thoughts?

(Note: this ignores all push blocking failures!)

Reviewed By: kvtsoy

Differential Revision: D46268308

fbshipit-source-id: cd5b924141365f61f8a3363bc9cb38a62e5c94cf
2023-06-01 14:11:31 -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
541b224580 Don't write if CCA out of bytes
Summary: We shouldn't attempt to write DSR data if we run out of congestion control bytes. This will just cause an empty DSR write, which shouldn't have any negative impact but it causes tperf spam.

Reviewed By: jbeshay

Differential Revision: D46126190

fbshipit-source-id: 390e04c5d6182a2e0e61d63e806522d1ea410a30
2023-05-24 10:52:01 -07:00
Matt Joras
96b2c1b37d Control write loop time limit from knob.
Summary:
This has been hardcoded to SRTT/25 for a long time. However, especially when using DSR this might not be the most appropriate since it can start to get close to real world SRTTs.

Control it via a knob, and also add the behavior such that setting it to 0 effectively disables the time limit.

Reviewed By: jbeshay

Differential Revision: D46084438

fbshipit-source-id: 7dc619fdff1bd5c3f8654c4936200e0340ef94f2
2023-05-22 16:15:24 -07:00
Matt Joras
b89882a772 Add TransportSetting and knob for default priority.
Summary: Allows the default stream priority (which also means scheduling policy) to be set via TransportSettings.

Reviewed By: jbeshay, hanidamlaj

Differential Revision: D45881729

fbshipit-source-id: fcb72022cd6eac2f1dafc55173d5a46a72a95dbc
2023-05-18 13:51:53 -07:00
Matt Joras
c95b297cb1 ACK_FREQUENCY Knob logging fix.
Summary: Messed up the log line.

Reviewed By: jbeshay

Differential Revision: D45913954

fbshipit-source-id: c3199dd6ecf114e75b934759c2b9054d38c9f17d
2023-05-17 14:14:08 -07:00
Joseph Beshay
a6897c8284 Fix typo in log for PACING_TIMER_TICK knob
Summary: The log for PACING_TIMER_TICK knob was referring to another knob name. This fixes it.

Reviewed By: sharmafb

Differential Revision: D45834217

fbshipit-source-id: c26c481633218dc165a65efcf029e7c636947dea
2023-05-15 10:02:12 -07:00
Matt Joras
51333a1583 Don't subtract out small non DSR packets from packet limit.
Summary: For non-DSR streams we can fill a packet that has e.g. stream limit updates and flow control with stream data. With DSR, we can't do that and thus packets that contain this data end up counting against the packet limit. This means that we often end up writing fewer packets per write loop when using DSR. Account for this by basically discounting the non-DSR packet in a write loop if it was "small".

Reviewed By: shodoco

Differential Revision: D45754930

fbshipit-source-id: ca494a4bd042de05c12a4d45e80dd5582fe12248
2023-05-10 18:52:17 -07:00
Matt Joras
25e8eb2f4e Randomly write DSR or non DSR first.
Summary:
Due to the fact that store DSR and non DSR streams in separate priority queues, we end up having to make a decision on which to write first.

Longer term we should merge them into a single priority queue, but in the meantime to mitigate potential starvation, randomly flip flop between which we write first in a given write loop.

Reviewed By: jbeshay, shodoco

Differential Revision: D45701230

fbshipit-source-id: 894179e457a4d6f7364767108f9290ff267eb977
2023-05-09 14:27:17 -07:00
Matt Joras
b5b3ad47b7 Add optional "ACK clocking" mode to the write looper.
Summary:
We've long seen issues where paced congestion controllers can't keep up with the sending rate at low RTTs. While we have one mitigation for this, it only works after the pacing timer has fired. This introduces a new behavior that effectively makes the write looper ACK clock some of the time.

The way it works is that when we call updateWriteLooper, if we've already missed the next pacing wakeup or it is going to fire soon (within 1ms), we cancel it and do a write in this loop. This effectively gives us multiple opportunities to write each burst, e.g. after receiving an ACK.

Reviewed By: kvtsoy

Differential Revision: D45052984

fbshipit-source-id: 591b6f5f374f4886dffa1e9be78df02d17fcc27c
2023-04-18 15:48:23 -07:00
Hani Damlaj
a0e456bc03 migrate folly::none assignment operator to .reset()
Summary: - .reset() is probably fractionally less costly than the assignment operator?

Reviewed By: sharmafb

Differential Revision: D43579041

fbshipit-source-id: 4838b6c21e94197782cf56866950be1dbf65b106
2023-03-03 05:39:57 -08:00
Konstantin Tsoy
6beae9d280 Protect server transport in a callback
Reviewed By: hanidamlaj

Differential Revision: D43546517

fbshipit-source-id: ede330167c5281ff8975acbf1bdbc27e420597d8
2023-02-24 11:12:37 -08: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
Brandon Schlinker
925472bdfc Store weak_ptr to SocketObserverContainer in ConnectionStateBase
Summary:
This diff changes `QuicConnectionStateBase` so that it stores a `std::weak_ptr<SocketObserverContainer>` instead of a `std::shared_ptr<SocketObserverContainer>`.

- `QuicConnectionStateBase` needs a pointer to the `SocketObserverContainer` so that loss / ACK / other processing logic can access the observer container and send the observers notifications. There may not be a `SocketObserverContainer` if the `QuicTransportBase` implementation does not support it.
- A `SocketObserverContainer` must not outlive the instance of the `QuicTransportBase` implementation that it is associated with. This is because observers are notified that the object being observed has been destroyed when the container is destroyed, and thus if the container outlives the lifetime of the transport, then the observers will think the transport is still alive when it is in fact dead.
- By storing a weak pointer to the `SocketObserverContainer` in the `QuicConnectionStateBase`, we provide access to the observer container without extending its lifetime. In parallel, because it is a managed pointer, we avoid the possibility of dereferencing a stale pointer (e.g., a pointer pointing to an object that has since been destroyed).

Reviewed By: mjoras

Differential Revision: D42856161

fbshipit-source-id: f35558a21fea91ba794adcf9b573dd48a626ea1f
2023-01-30 22:45:59 -08:00
Matt Joras
1275798146 Make the AckState for Initial/Handshake a unique_ptr
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
2022-12-20 11:08:43 -08:00
Sharad Jaiswal (Eng)
328c78d0e2 Add received packets timestamps to AckState
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
2022-11-15 20:14:57 -08:00
Joseph Beshay
772b47adaa Do not use LOG_EVERY_N
Summary:
Fixes build error from Github actions
```
D:\a\mvfst\mvfst\quic\server\QuicServerTransport.cpp(1072): error C3861:
'__sync_add_and_fetch': identifier not found

```

Reviewed By: hanidamlaj

Differential Revision: D40327603

fbshipit-source-id: afae1b661322e80156c7d69b577f35f41b67ee73
2022-11-10 16:17:32 -08:00
Matt Joras
953235fbc7 Track largest packet acked per DSR stream.
Summary:
The normal loss detection only considers the reordering threshold per packet by comparing it with the largest ACKed packet in the ACK. This is largely based on the fact that QUIC packet numbers are monotonically increasing.

However, when using DSR there is a potential for a different host to be writing the packets to the wire. When there are multiple hosts doing this, there is a degree of reordering that is expected to happen.

Thus we can change the loss detection logic to adjust the largest packet ACKed and use the largest packet ACKed for a given DSR stream when the packet is a DSR packet. This allows for the natural reordering while detecting loss within the DSR sender's timeline of sent packets.

Note that this probably doesn't catch all of the nuances that can happen with the reordering, but it is a good start.

Reviewed By: kvtsoy

Differential Revision: D40647572

fbshipit-source-id: d84c6cd8040fb8c044ddd68fb1abc049ccddfc44
2022-10-27 14:35:11 -07:00
Matt Joras
c8f357156e Differentiate ACK_FREQUENCY policy early in connection.
Summary: This changes the experimental behavior of BBR sending ACK_FREQUENCY frames. Now instead of using the static ack eliciting threshold, early in the connection BBR will set the threshold to 2 which has been empirically shown to be a good "initial" value.

Reviewed By: jbeshay

Differential Revision: D40438721

fbshipit-source-id: 184b8025581b6152eea97f9b557b6343d980322d
2022-10-17 17:54:45 -07:00