Implement fallback for sendmmsg and recvmmsg
#1504
Conversation
|
Sidenote: consider enabling automatic approval of workflow runs for new contributors. |
|
I changed the repo setting to only flag contributors from new GitHub members. |
quinn-udp/src/unix.rs
Outdated
| @@ -143,7 +143,7 @@ fn init(io: SockRef<'_>) -> io::Result<()> { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| #[cfg(not(any(target_os = "macos", target_os = "ios")))] | |||
| #[cfg(not(any(target_os = "macos", target_os = "ios", target_os = "android")))] | |||
There was a problem hiding this comment.
This seems like a pretty heavy hammer, which impairs performance for all Android builds on account of what's presumably a pretty small minority of very old phones.
There was a problem hiding this comment.
Another option would be to use something like tokio-rs/mio#1590: using syscall directly, so it can fail with ENOSYS and then switching to fallback.
But I am not sure it's worth the trouble, Android phones are usually clients handling a few connections and not sending much, so they do not benefit from such optimizations of the sending code.
There was a problem hiding this comment.
But we also have a similar recvmmsg problem, it is also not available with API 16.
There was a problem hiding this comment.
I'd prefer the dynamic fallback. It's a common enough Linux pattern, and lets us avoid baking in assumptions about use cases. I wouldn't be surprised if we ended up needing similar tricks on other platforms sooner or later either.
Even for typical mobile applications where CPU might not be a bottleneck, power use is still important, for example.
link2xt
force-pushed
the
android-sendmmsg
branch
from
67d29e3 to
707bec4
Compare
link2xt
changed the title
sendmmsg on Androidsendmmsg and recvmsg on Android
link2xt
force-pushed
the
android-sendmmsg
branch
4 times, most recently
from
5848279 to
4f80bed
Compare
link2xt
changed the title
sendmmsg and recvmsg on Androidsendmmsg and recvmmsg
|
This is ready for review now. |
link2xt
force-pushed
the
android-sendmmsg
branch
from
4f80bed to
127c016
Compare
link2xt
force-pushed
the
android-sendmmsg
branch
from
127c016 to
cdfe0fa
Compare
quinn-udp/src/unix.rs
Outdated
| match e.raw_os_error() { | ||
| Some(libc::ENOSYS) => { | ||
| // Fallback to `recvmsg`. | ||
| recvmmsg_fallback(sockfd, msgvec, vlen) |
There was a problem hiding this comment.
Don't know how much you care, but presumably we could store state on whether mmsg support is available somewhere, maybe in the UdpState, so that we don't have to fallback everytime? (I suppose that would come at the cost of one extra branch even for environments where mmsg support is available, but that should be a pretty cheap branch with prediction.)
There was a problem hiding this comment.
It's probably ok to have less performance and 2 syscalls per send/receive on systems which do not support sendmmsg/recvmmsg. We are going to only use it for backup transfer for now and it only needs to work good enough for transferring a file once reliably. Adding statefullness makes things more complex.
link2xt
force-pushed
the
android-sendmmsg
branch
from
cdfe0fa to
1426f78
Compare
Some Android phones use older 2.x kernels and do not support `sendmmsg` which is available only since Linux 3.0. Similarly, `recvmmsg` is only available since Linux 2.6.34.
> On x86-32, socketcall() was historically the only entry point for > the sockets API. However, starting in Linux 4.3, direct system > calls are provided on x86-32 for the sockets API. https://man7.org/linux/man-pages/man2/socketcall.2.html Androids x86 even goes as far as automatically dispatching advanced socket calls (e.g. `socket` or `recvmmsg`) through `socketcall` in its Bionic libc: > For example, socket() syscall on i386 actually becomes: > socketcall(__NR_socket, 1, *(rest of args on stack)). https://android.googlesource.com/platform/bionic/+/refs/tags/android-vts-14.0_r5/libc/SYSCALLS.TXT#19 In addition Android enables seccomp filtering since Android 8: https://android-developers.googleblog.com/2017/07/seccomp-filter-in-android-o.html Currently `quinn-udp` calls `recvmmsg` directly on Android x86: ``` rust let ret = libc::syscall(libc::SYS_recvmmsg, sockfd, msgvec, vlen, flags, timeout) as libc::c_int; ``` https://github.com/quinn-rs/quinn/blob/c0b1e281e3167d4f4c8496082cb1117c4a270ad8/quinn-udp/src/unix.rs#L447-L448 A direct `recvmmsg` through `libc::syscall(libc::SYS_recvmmsg` does not trigger the automatic Android x86 Bionic libc dispatch logic through `socketcall`, but instead calls the unimplemented `recvmmsg` syscall. Instead of triggering a `libc::ENOSYS`, seccomp disallows the call, thus leading to a panic of the application. This commit changes `quinn-udp` to use `libc::recvmmsg` on Android x86, thus leveraging Bionic's automatic dispatch of `recvmmsg` through `socketcall`. Note that this commit only uses `libc::recvmmsg` on Android x86, not any other Android or Linux variant. Thus quinn-udp would still support missing `libc::recvmmsg` on those systems. See quinn-rs#1504.
Some Android phones use older 2.x kernels
and do not support
sendmmsgwhich is available only since Linux 3.0.
Similarly,
recvmmsgis only availablesince Linux 2.6.34.