Skip to content

Fallback to not using ECN if IP_TOS is not supported #1516

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

Merged
merged 2 commits into from
Mar 17, 2023

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Mar 16, 2023

On Linux <3.13 sendmsg/sendmmsg system calls
return EINVAL without sending anything if they encounter an IP_TOS cmsg. To ensure we can still send
on such systems, remember if we got an EINVAL error and switch to "fallback mode" in which we do not try to transmit ECN at all and only use well-supported features.

@link2xt
Copy link
Contributor Author

link2xt commented Mar 16, 2023

This is an alternative to #1512

@djc djc requested a review from Ralith March 16, 2023 15:23
@djc
Copy link
Member

djc commented Mar 16, 2023

Seems reasonable to me!

On Linux <3.13 sendmsg/sendmmsg system calls
return EINVAL without sending anything if they encounter
an `IP_TOS` cmsg. To ensure we can still send
on such systems, remember if we got an EINVAL error
and switch to "fallback mode" in which we do not try
to transmit ECN at all and only use well-supported features.
@link2xt link2xt force-pushed the ecn-einval-fallback branch from 3cf3e31 to a39bcc7 Compare March 16, 2023 15:25
@link2xt link2xt force-pushed the ecn-einval-fallback branch from b05abc4 to b60a547 Compare March 16, 2023 15:48
@link2xt
Copy link
Contributor Author

link2xt commented Mar 16, 2023

I tested, it works on my Android with old kernel 3.10.108.

@Ralith
Copy link
Collaborator

Ralith commented Mar 17, 2023

Thanks!

@djc djc merged commit 11b34a7 into quinn-rs:main Mar 17, 2023
@flub flub mentioned this pull request Mar 20, 2023
25 tasks
@djc djc mentioned this pull request May 8, 2023
3 tasks
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 12, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 12, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 13, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
mxinden added a commit to mxinden/quinn that referenced this pull request Dec 16, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

quinn-rs#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes quinn-rs#1975.
github-merge-queue bot pushed a commit that referenced this pull request Dec 17, 2024
Android API level < 26 does not support the `libc::IP_TOS` control message.
`sendmsg` calls with `libc::IP_TOS` return `libc::EINVAL`.

#1516 added a fallback, not setting
`libc::IP_TOS` on consecutive calls to `sendmsg` after a failure with
`libc::EINVAL`. The current datagram would be dropped. Consecutive datagrams
passed to `sendmsg` would succeed as they would be sent without `libc::IP_TOS`
through the fallback.

Instead of dropping the first datagram on `libc::EINVAL`, this commit adds a
retry for it without `libc::IP_TOS`.

This is e.g. relevant for Neqo. When establishing a QUIC connection, dropping
the first datagram [delays connection establishment by
100ms](https://github.com/mozilla/neqo/blob/3001a3a56f2274eaafaa956fb394f0817f526ae7/neqo-transport/src/rtt.rs#L28).
With the retry introduced in this commit, delay due to unsupported
`libc::IP_TOS` should be negligeable.

Closes #1975.
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.

3 participants