Do not add IP_TOS cmsg on Android

[link2xt]

Android usually comes with old Linux kernels, and Linux <3.13 does not support IP_TOS cmsg type, returning EINVAL error from sendmmsg without sending anything.

To avoid this problem we do not try to set TOS on Android at all.

Fixes #1511

[link2xt]

Clippy failure is fixed in #1506, ignore it.

IPv6 is out of scope, we currently only use IPv4. It probably also returns EINVAL, but I did not test it and plan to fix it only if we actually encounter this issue.

[djc]

LGTM, thanks for following up!

[link2xt]

Just tested that it still works after rebasing.

[dignifiedquire]

Maybe EINVAL should be not ignored to avoid swallowing potential issues ?

[link2xt]

Maybe EINVAL should be not ignored to avoid swallowing potential issues ?

EINVAL (errno 22 on Linux) corresponds to io::ErrorKind::InvalidInput. A more fail-fast approach can be used here: https://github.com/quinn-rs/quinn/blob/02037e72fa6b7f994a2db411977bcffacbb8b02c/quinn-udp/src/unix.rs#L224-L230

Another option is to set some flag in our socket state and switch to "fallback mode" where no potentially unsupported features are used, but this adds its own complexity, a need to retry a call and so on. I do not see an immediate need for this on Androids, until we find an Android application for which ECN measurably improves the performance I would rather just always use this fallback mode.

[link2xt]

I rebased the PR to include clippy fixes from #1506.

[link2xt]

@djc Could this be merged? We can then at least update to the main branch, although a 0.10 release would be nice too. Or maybe push it to 0.9 branch as 0.9.4.

[djc]

I'm honestly not very satisified with the approach of disabling ECN for all Android users when mostly very old devices are actually impacted, so I would like to at least discuss alternative strategies of dealing with this. I'd also like review from Ralith on this stuff before merging.

Given that we're both volunteers, pinging within 24h after submitting a PR seems a little pushy IMO.

(In general, I'd be fine with creating a 0.9.4 release. 0.10 will have to wait until after rustls 0.21 has been released.)

[link2xt]

Given that we're both volunteers, pinging within 24h after submitting a PR seems a little pushy IMO.

Ok, I just saw that you approved and also left a reaction on the rebase comment, so if you decide not to merge then something is probably missing that I should fix with the PR.

Regarding alternative approaches, we can try to set ECN by default but switch it off as soon as we get a single io::ErrorKind::InvalidInput, that could also work.

[djc]

Given that we're both volunteers, pinging within 24h after submitting a PR seems a little pushy IMO.

Ok, I just saw that you approved and also left a reaction on the rebase comment, so if you decide not to merge then something is probably missing that I should fix with the PR.

Fair enough, missed that context -- too much GitHub stuff to keep all the context around. Sorry for changing my mind, I'd rather optimize for newer/future devices while still trying to support the old stuff.

Regarding alternative approaches, we can try to set ECN by default but switch it off as soon as we get a single io::ErrorKind::InvalidInput, that could also work.

Yeah, that might be nicer. I guess we won't know for sure if the InvalidInput is referring to the TOS value, which seems a little tricky?

(Also relating to releases, note that this is actually part of quinn-udp, for which we can just push out further 0.3.x releases from the main branch.)

[link2xt]

I guess we won't know for sure if the InvalidInput is referring to the TOS value, which seems a little tricky?

Yes, we can only guess and disable everything that can cause problems, but for now it is only IP_TOS. I made an implementation of this approach at #1516.

[link2xt]

Closing in favor of #1516.