Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(udp/windows): set socket option IPV6_RECVECN #2125

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Jan 5, 2025

On Windows, to enable ECN reporting, one sets the IPV6_RECVECN socket option. The Windows IP stack will then report ECN markings via the IPV6_ECN message type.

Previously quinn-udp would use IPV6_ECN for both. With this commit IPV6_RECVECN is used to set the socket option and IPV6_ECN is used to read the option.

Given that both constants evaluate to the same value (i.e. 50) the previous behavior does not result in an actual bug.

See also:

On Windows, to enable ECN reporting, one sets the `IPV6_RECVECN` socket option.
The Windows IP stack will then report ECN markings via the `IPV6_ECN` message
type.

Previously `quinn-udp` would use `IPV6_ECN` for both. With this commit
`IPV6_RECVECN` is used to set the socket option and `IPV6_ECN` is used to read
the option.

Given that both constants evaluate to the same value (i.e. `50`) the previous
behavior does not result in an actual bug.

See also:

- https://learn.microsoft.com/en-us/windows/win32/winsock/ipproto-ipv6-socket-options
- https://www.ietf.org/archive/id/draft-duke-tsvwg-udp-ecn-01.html
- https://docs.rs/windows-sys/latest/windows_sys/Win32/Networking/WinSock/constant.IPV6_RECVECN.html
- https://docs.rs/windows-sys/latest/windows_sys/Win32/Networking/WinSock/constant.IPV6_ECN.html
@mxinden mxinden marked this pull request as ready for review January 5, 2025 16:39
@mxinden mxinden requested review from djc and Ralith as code owners January 5, 2025 16:39
@Ralith Ralith added this pull request to the merge queue Jan 5, 2025
Merged via the queue into quinn-rs:main with commit c32e2e2 Jan 5, 2025
20 checks passed
mxinden added a commit to mxinden/quinn that referenced this pull request Jan 6, 2025
Equivalent to quinn-rs#2125 for IPv4.

On Windows, to enable ECN reporting on IPv4, one sets the `IP_RECVECN` socket
option. The Windows IP stack will then report ECN markings via the `IP_ECN`
message type.

Previously `quinn-udp` would use `IP_ECN` for both. With this commit
`IP_RECVECN` is used to set the socket option and `IP_ECN` is used to read the
option.

Given that both constants evaluate to the same value (i.e. `50`) the previous
behavior does not result in an actual bug.

See also:

- https://learn.microsoft.com/en-us/windows/win32/winsock/ipproto-ipv6-socket-options
- https://www.ietf.org/archive/id/draft-duke-tsvwg-udp-ecn-01.html
- https://docs.rs/windows-sys/latest/windows_sys/Win32/Networking/WinSock/constant.IP_RECVECN.html
- https://docs.rs/windows-sys/latest/windows_sys/Win32/Networking/WinSock/constant.IP_ECN.html
github-merge-queue bot pushed a commit that referenced this pull request Jan 6, 2025
Equivalent to #2125 for IPv4.

On Windows, to enable ECN reporting on IPv4, one sets the `IP_RECVECN` socket
option. The Windows IP stack will then report ECN markings via the `IP_ECN`
message type.

Previously `quinn-udp` would use `IP_ECN` for both. With this commit
`IP_RECVECN` is used to set the socket option and `IP_ECN` is used to read the
option.

Given that both constants evaluate to the same value (i.e. `50`) the previous
behavior does not result in an actual bug.

See also:

- https://learn.microsoft.com/en-us/windows/win32/winsock/ipproto-ipv6-socket-options
- https://www.ietf.org/archive/id/draft-duke-tsvwg-udp-ecn-01.html
- https://docs.rs/windows-sys/latest/windows_sys/Win32/Networking/WinSock/constant.IP_RECVECN.html
- https://docs.rs/windows-sys/latest/windows_sys/Win32/Networking/WinSock/constant.IP_ECN.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants