fix(quinn-udp): use TOS for IPv4-mapped IPv6 dst addrs #1765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When handling [IPv4-mapped IPv6 addresses](https://doc.rust-lang.org/std/net/struct.Ipv6Addr.html#ipv4-mapped-ipv6-addresses) use IPv4 TOS (type of service) instead of IPv6 Traffic Class to encode ECN value. Otherwise ECN value is ignored by OS.
Ralith
reviewed
Feb 21, 2024
|
Nice catch, thanks! I'm excited for the opportunity to share all this fiddly obscure stuff. |
I think the change is reasonable on the face of it, and the rest comes down to empirical kernel behavior, which is what we have CI for. To wit, it looks like Windows and FreeBSD are unhappy, but I'm not sure it's with the meat of the change. Perhaps they don't like the test trying to bind a socket to an IPv6-mapped IPv4 address? |
mxinden
commented
Feb 22, 2024
Ralith
reviewed
Feb 22, 2024
|
Test is running and succeeding on all platforms now (no more |
mxinden
added a commit
to mxinden/neqo
that referenced
this pull request
Feb 23, 2024
github-merge-queue bot
pushed a commit
to mozilla/neqo
that referenced
this pull request
Feb 26, 2024
* Initial commit * refactor(server): replace mio with tokio * Move ready logic into fn * Extend expect docs * Restrict tokio features * Only process datagram once * Remove superfluous pub * fmt * Fix send path * Fix receive path * Instantiate socket state once * Fix busy loop * Have neqo-client use quinn-udp * Add TODO * Await writable * Unify tx and rx * Introduce wrapper type Socket * Move bind to common * Check if datagram was sent as a whole and avoid allocation * Make into_data pub(crate) * Refactor send * Reference issue * Remove debugs * Fix test * Reduce diff * Reduce diff * Pin quinn-udp to rev * Address clippy lints * fmt * fmt * clippy * clippy * Pass None ttl Not yet supported by quinn-udp. * Debug race condition on Windows * Debug windows failure * Have receiver use random port * Test with Ect1 instead of Ce Windows does not allow setting Ce: > Your application isn't allowed to specify the Congestion Encountered (CE) code point when sending datagrams. The send will return with error WSAEINVAL. https://learn.microsoft.com/en-us/windows/win32/winsock/winsock-ecn * Revert "Debug windows failure" This reverts commit e9ac36c. * Revert "Debug race condition on Windows" This reverts commit 6f330d3. * Fold tos_tx * Add reason to clippy lint ignore * fix: include quinn-udp IPv4-mapped IPv6 patch quinn-rs/quinn#1765 --------- Co-authored-by: Lars Eggert <lars@eggert.org>
mxinden
added a commit
to mxinden/neqo
that referenced
this pull request
May 13, 2024
`neqo-bin` has been importing `quinn-udp` as a git reference, in order to include quinn-rs/quinn#1765. The quinn project has since released `quinn-udp` `v0.5.0`. This commit upgrades `neqo-bin` to use `quinn-udp` `v0.5.0`. `quinn-udp` has dropped `sendmmsg` support in the `v0.5.0` release (quinn-rs/quinn@ee08826). `neqo-bin` does not (yet) use `sendmmsg`. This might change in the future (mozilla#1693).
mxinden
added a commit
to mxinden/neqo
that referenced
this pull request
May 13, 2024
`neqo-bin` has been importing `quinn-udp` as a git reference, in order to include quinn-rs/quinn#1765. The quinn project has since released `quinn-udp` `v0.5.0`. This commit upgrades `neqo-bin` to use `quinn-udp` `v0.5.0`. `quinn-udp` now takes a data reference (`&[u8]`) instead of owned data (`bytes::Bytes`) on its send path, thus no longer requiring `neqo-bin` to convert, but simply pass a reference. See quinn-rs/quinn#1729 (comment) for details. `quinn-udp` has dropped `sendmmsg` support in the `v0.5.0` release (quinn-rs/quinn@ee08826). `neqo-bin` does not (yet) use `sendmmsg`. This might change in the future (mozilla#1693).
github-merge-queue bot
pushed a commit
to mozilla/neqo
that referenced
this pull request
May 13, 2024
* feat(bin): use quinn-udp crates.io release instead of git ref `neqo-bin` has been importing `quinn-udp` as a git reference, in order to include quinn-rs/quinn#1765. The quinn project has since released `quinn-udp` `v0.5.0`. This commit upgrades `neqo-bin` to use `quinn-udp` `v0.5.0`. `quinn-udp` now takes a data reference (`&[u8]`) instead of owned data (`bytes::Bytes`) on its send path, thus no longer requiring `neqo-bin` to convert, but simply pass a reference. See quinn-rs/quinn#1729 (comment) for details. `quinn-udp` has dropped `sendmmsg` support in the `v0.5.0` release (quinn-rs/quinn@ee08826). `neqo-bin` does not (yet) use `sendmmsg`. This might change in the future (#1693). * remove impl From<Datagram> for Vec<u8>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When handling IPv4-mapped IPv6 addresses (e.g.
::ffff:127.0.0.1) use IPv4 TOS (type of service) instead of IPv6 Traffic Class to encode ECN value. Otherwise ECN value is ignored by OS.Tested on Ubuntu 23.10.
Setting this as draft for now. I am not familiar enough with IPv4-mapped IPv6 addresses across operating systems to feel confident in the patch proposed here. Input welcome.
Needed for mozilla/neqo#1604.
//CC @larseggert
(Thank you for
quinn-udp! ❤️ )