quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

fix(quinn-udp): use TOS for IPv4-mapped IPv6 dst addrs #1765

Closed mxinden closed 7 months ago

mxinden commented 7 months ago

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 https://github.com/mozilla/neqo/pull/1604.

//CC @larseggert

(Thank you for quinn-udp! :heart: )

Ralith commented 7 months ago

Nice catch, thanks! I'm excited for the opportunity to share all this fiddly obscure stuff.

Ralith commented 7 months ago

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.

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 7 months ago

Test is running and succeeding on all platforms now (no more ignore). @Ralith thank you for the help. Mind taking another look?