Skip to content

Conversation

flub
Copy link
Collaborator

@flub flub commented May 22, 2025

No description provided.

thomaseizinger and others added 30 commits April 12, 2025 16:20
At present, each of the various `send` functions within `quinn-dup`
handle the error case of `EMSGSIZE` individually. This code duplication
can be avoided by moving this error handling to the
`UdpSocketState::send` function.

Additionally, contrary to what the comment states, this error code can
also be returned if the provided datagram is simply to large which can
happen with some very big GSO batches.
Returning `Ok(())` at the top of the loop in case we have successfully
sent the message reduced the indentation of the remaining error handling
code.
Similarly as for the `send` functions, using early-return to break out
of the `recv` loops reduced the indentation of the remaining error
handling code and therefore makes the function easier to read.
By moving the comment on top of the match-arm, we can avoid using a
block for it.
Returning the error is already covered by the catch-all branch.
For consistency with the `send` functions, we perform the error handling
with `match` blocks instead of `if`.
In quinn-rs#2154, we added a piece of logic to `cmsg_nxt_hdr` that -
within the `apple-fast-datapath` code - inspects the `cmsg_len`
property and checks if we can actually fit another `cmsghdr` into it.

This however only makes sense when we are _reading_ a buffer of cmsgs
that has been initialised by the kernel.

When we are _sending_ datagrams, the cmsg buffer is initialised with
all zeros and for every cmsg we want to attach to a datagram, we
advance the buffer using CMSG_NXTHDR. In this case, all the fields
within `cmsghdr` are 0 initialised and it doesn't make sense to check
their length. In fact, doing so results in a false-positive panic
triggered by `Encoder::push` because it thinks that the cmsg buffer is
too short to accomodate the message it wants to write.

To fix this issue, we move the check to the `Iterator` implementation,
guarded by the `apple_fast` cfg. This allows us to use the
`cmsg_nxt_hdr` function for both reading and writing cmsgs.
Looks to the following for writing inspiration:

- https://docs.rs/tokio/latest/tokio/io/trait.AsyncRead.html#tymethod.poll_read
- https://doc.rust-lang.org/stable/std/io/trait.Read.html#tymethod.read

Achieves:

- Reflowing to 100 columns.
- Style-consistency with code-escaping and initial-line period.
- Distinguishing first lines between the two methods' doc comments.
- Hyperlink to SendStream::finish
- General writing style.
Bumps [rand](https://github.com/rust-random/rand) from 0.9.0 to 0.9.1.
- [Release notes](https://github.com/rust-random/rand/releases)
- [Changelog](https://github.com/rust-random/rand/blob/master/CHANGELOG.md)
- [Commits](rust-random/rand@0.9.0...rand_core-0.9.1)

---
updated-dependencies:
- dependency-name: rand
  dependency-version: 0.9.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
`quinn-udp` with fast-apple-datapath previously did not initialize the
control message array memory before passing it to `recvmsg_x`.

On MacOS 10.15 `recvmsg_x` does not seem to set `msg_controllen`. Thus
`CMSG_NXTHDR` reads beyond the control messages written by `recvmsg_x`,
into the unitinialized memory region.

With this commit, the control message array is initialized (with zeroes)
before passing it to `recvmsg_x`, thus no longer reading unset control
messages.

See quinn-rs#2214 for details.
In order to perform GSO, an application needs to batch datagrams
together into a big buffer. As part of such a batch, the last datagram
is allowed to be less than the `segment_size` as that still allows the
kernel to unambigously segment the content into individual packets.

When GSO gets disabled at runtime due to lack of driver support,
applications need to segment such a prepared batch themselves into
individual `Transmit`s. Doing that may lead to situations in which a
`Transmit` is handed to `quinn-udp` which still has the original
`segment_size` set, yet its content is shorter than that.

Not all kernels like that. Specifically, it has been observed that
Android is particularly picky about these parameters.

We already sanitise the `segment_size` such that it is not set when it
is equal to the content length. To further increase the robustness of
`quinn-udp`, we extend this check to only set it when it is strictly
less than the content length.

Related: quinn-rs#2201
Related: quinn-rs#2145
Related: firezone/firezone#8932
TokenMemoryCache is a new implementation of TokenStore.

TokenMemoryCache is designed to store up to 2 tokens per server (this is
configurable) for up to 256 servers (this is also configurable), with a
LRU eviction policy. This is so that it works harmoniously with
rustls::ClientSessionMemoryCache, which by default stores resumption
state for up to 256 servers with a LRU eviction policy.
Deletes SimpleTokenStore. Test code can now rely on TokenMemoryCache,
which is set by default.
- Adds default feature `bloom`.
- It enables a new BloomTokenLog implementation of TokenLog.
- It enables a dependency on the `fastbloom` crate.
Unless the bloom feature is disabled, in which case it still falls back
to NoneTokenLog.
It makes sense to do this now that we have a default token log that's
actually able to accept tokens.
Tests no longer reconfigure config to use SimpleTokenLog if the bloom
feature is enabled, instead preferring to use the default BloomTokenLog.
Commit fixes compile errors and warnings that previously occurred with
certain combinations of features that our CI was not catching.
The existing powerset check has problems:

- It takes too long.
- It failed to catch all the bugs fixed in the previous commit.

This commit fixes it as such:

- `--depth 3` is passed in which limits execution time.
- `--group-features` groups are removed which would have caught the bugs
  fixed in the previous commit.
- `--clean-per-run` is removed as it's no longer necessary and slows it
  down.

Experimentation indicates that a depth of 3 takes roughly a few minutes
to execute and catches bugs, whereas a depth of 4 takes roughly 40
minutes to run and does not catch any additional bugs.
We maintain explicit send stream future implementation structs for
`Write`, `WriteAll`, `WriteChunk`, and `WriteAllChunks`. However, these
are private. We can significantly simplify this code by instead leaning
more directly on functionality such as async functions and
`std::future::poll_fn`.
There are no public APIs that accept BytesSource. This commit makes it
not be re-exported publically.
Changes the implementation of `SendStream::stopped` such that the
returned future is static and no longer lifetime-bound onto a mutable
reference to the send stream.

This allows to use the stopped future with combinators or in a separate
task while still sending on the stream concurrently.

Internally, this is done changing the implementation of the stopped
notification to use a cloneable tokio::sync::Notify instead of storing
a single waker.
dependabot bot and others added 7 commits May 14, 2025 09:27
Bumps [getrandom](https://github.com/rust-random/getrandom) from 0.3.2 to 0.3.3.
- [Changelog](https://github.com/rust-random/getrandom/blob/master/CHANGELOG.md)
- [Commits](rust-random/getrandom@v0.3.2...v0.3.3)

---
updated-dependencies:
- dependency-name: getrandom
  dependency-version: 0.3.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [rustls-platform-verifier](https://github.com/rustls/rustls-platform-verifier) from 0.5.1 to 0.5.3.
- [Release notes](https://github.com/rustls/rustls-platform-verifier/releases)
- [Changelog](https://github.com/rustls/rustls-platform-verifier/blob/main/CHANGELOG)
- [Commits](rustls/rustls-platform-verifier@v/0.5.1...v/0.5.3)

---
updated-dependencies:
- dependency-name: rustls-platform-verifier
  dependency-version: 0.5.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
This commit makes there be fewer places where SendStream::execute_poll
is called directly. AsyncWrite implementations now go through poll_write,
and poll_write now goes through the write future. This means that
execute_poll is now only called directly from write and write_chunks.
This commit uses an allow directive to suppress the following error:

```
warning: the `Err`-variant returned from this function is very large
   --> quinn-proto/src/endpoint.rs:525:10
    |
525 |     ) -> Result<(ConnectionHandle, Connection), AcceptError> {
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 136 bytes
    |
    = help: try reducing the size of `endpoint::AcceptError`, for example by boxing large elements or replacing it with `Box<endpoint::AcceptError>`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
    = note: `#[warn(clippy::result_large_err)]` on by default
```

Because AcceptError and all of its descendants fields are public,
excepting a descendant that contains a usize, it is impossible to reduce
the byte size of AcceptError without semver breakage.
Clippy is warning that the size of RetryError (both proto and quinn's
versions) are excessively large for a Result Err variant. This function
fixes these warnings by boxing the contents of both.
@Arqu Arqu added this to iroh May 22, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh May 22, 2025
@flub flub merged commit 8c4e5d7 into multipath-quinn-0.11.x May 22, 2025
13 checks passed
@flub flub deleted the flub/merge-main branch May 22, 2025 14:24
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

9 participants