Skip to content

Commit

Permalink
Bug 1916412 - fix Android x86 recvmmsg seccomp failure r=glandium,sup…
Browse files Browse the repository at this point in the history
…ply-chain-reviewers,sunil

https://bugzilla.mozilla.org/show_bug.cgi?id=1910360 replaced NSPR with
quinn-udp for HTTP3 QUIC UDP IO on Firefox Nightly. Calls to `recvmmsg` were
prohibited by seccomp on Android x86. See
https://bugzilla.mozilla.org/show_bug.cgi?id=1910594, upstream quinn-udp
tracking issue: quinn-rs/quinn#1947 and upstream
quinn-udp fix: quinn-rs/quinn#1966

Now that the upstream fix is merged and released, this commit upgrads neqo_glue
to use quinn-udp `v0.5.6`. In addition, given the fix, quinn-udp can now be used
on Android x86 Firefox Nightly. Thus this commit also removes the conditional
around the `network.http.http3.use_nspr_for_io` pref.

Differential Revision: https://phabricator.services.mozilla.com/D220890
  • Loading branch information
mxinden committed Oct 31, 2024
1 parent b08b6e8 commit e3bd5c4
Show file tree
Hide file tree
Showing 14 changed files with 598 additions and 371 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13994,13 +13994,7 @@
# Use NSPR for HTTP3 UDP IO
- name: network.http.http3.use_nspr_for_io
type: RelaxedAtomicBool
# Always use NSPR on Android x86 until
# https://bugzilla.mozilla.org/show_bug.cgi?id=1916412 is fixed.
#if defined(ANDROID) && !defined(HAVE_64BIT_BUILD)
value: true
#else
value: @IS_NOT_NIGHTLY_BUILD@
#endif
mirror: always

# Send and receive IP ECN marks. Noop if network.http.http3.use_nspr_for_io is
Expand Down
5 changes: 5 additions & 0 deletions supply-chain/audits.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3801,6 +3801,11 @@ criteria = "safe-to-deploy"
version = "0.5.4"
notes = "This is a small crate, providing safe wrappers around various low-level networking specific operating system features. Given that the Rust standard library does not provide safe wrappers for these low-level features, safe wrappers need to be build in the crate itself, i.e. `quinn-udp`, thus requiring `unsafe` code."

[[audits.quinn-udp]]
who = "Max Inden <mail@max-inden.de>"
criteria = "safe-to-deploy"
delta = "0.5.4 -> 0.5.6"

[[audits.quote]]
who = "Nika Layzell <nika@thelayzells.com>"
criteria = "safe-to-deploy"
Expand Down
2 changes: 1 addition & 1 deletion third_party/rust/quinn-udp/.cargo-checksum.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"files":{"Cargo.toml":"bf505df0c4f9254fa37950bad863cb838a8a7d2be4c8d3f28bdd679f945ef8cf","LICENSE-APACHE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","LICENSE-MIT":"4b2d0aca6789fa39e03d6738e869ea0988cceba210ca34ebb59c15c463e93a04","benches/throughput.rs":"8f00856e6c6f1dd8c7dd6c8a22b36c6f42dfa4d709edf3348de75d32e22c71fb","src/cmsg/mod.rs":"23d898d72c5aabda93d987526fdd78231bb5907bce2b6b2d292a56bdfd977f86","src/cmsg/unix.rs":"1a4089e5e61536a3c370c6b1bc891036ec2d0e2e78105fbb5b8227705e905d34","src/cmsg/windows.rs":"6fb936ec4a283efc5796872e777441e3039c40589073865644a8ef7936af4f4b","src/fallback.rs":"7fe9666b0bf508d1b5ec0b3690bb7add94c8f213cb51a263c9959e22a5094ad0","src/lib.rs":"72be7f797a3a11e452e7764fadadebc43ae7f9c14ba7fa80aedbbee71aa889c7","src/unix.rs":"fbc9a6ab281cc66500e6afa8b9ebdee73ca281ca14732e8076d9b1f10f431de7","src/windows.rs":"e741a7bdd86d7fcb856db855f9308af01e69387c00e6a726d322f1f4d3046b74","tests/tests.rs":"51bcf6d3f1a3fcf7d481ae966eb679f88341886ff4277f5747df3340ed709d09"},"package":"8bffec3605b73c6f1754535084a85229fa8a30f86014e6c81aeec4abb68b0285"}
{"files":{"Cargo.toml":"865febc6bb7b0a6f4d0758779480f829f96fcd6a614b64db05b5ea53e902fd5c","LICENSE-APACHE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","LICENSE-MIT":"4b2d0aca6789fa39e03d6738e869ea0988cceba210ca34ebb59c15c463e93a04","benches/throughput.rs":"095137508f85b68174978ff968cade74587751484402ca09269ffc2631d97f34","build.rs":"8e81067cac9fbe675619c3314d5aa06d99cf54c332812a837a227eeab41c92e1","src/cmsg/mod.rs":"63d6ea7126341fededdaef14260a7eed715ad3f507d4da586dbab814f581a54d","src/cmsg/unix.rs":"7917bce2f3c8e844eca2e4cfea82669b2a31cf311321dc42532626db4ee42de8","src/cmsg/windows.rs":"6fb936ec4a283efc5796872e777441e3039c40589073865644a8ef7936af4f4b","src/fallback.rs":"6378c177db7ba0eb88115b63f1ec9e17b05f53b1daae2c1e215520f103145585","src/lib.rs":"9672bd2003d779c95d11a85d05a5dac5d421a9d5dcd9f1475de94aca93f23f73","src/unix.rs":"b8e595499055115d15bfb95259c0c585934adf55f61e365bcc9fc47ab8fa9cdd","src/windows.rs":"ab1928d18bed62162a0f2c96158d808d7a2962045ab47c9efa0ecf60e2a2c060","tests/tests.rs":"3ab6c02756098d3933542baff06fa1f2ad6bba11852466f6843b8a42a9cc97c0"},"package":"e346e016eacfff12233c243718197ca12f148c84e1e84268a896699b41c71780"}
45 changes: 39 additions & 6 deletions third_party/rust/quinn-udp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,16 @@

[package]
edition = "2021"
rust-version = "1.66"
rust-version = "1.70.0"
name = "quinn-udp"
version = "0.5.4"
version = "0.5.6"
build = "build.rs"
autobins = false
autoexamples = false
autotests = false
autobenches = false
description = "UDP sockets with ECN information for the QUIC transport protocol"
readme = false
keywords = ["quic"]
categories = [
"network-programming",
Expand All @@ -26,8 +32,22 @@ repository = "https://github.com/quinn-rs/quinn"
[package.metadata.docs.rs]
all-features = true

[lib]
name = "quinn_udp"
path = "src/lib.rs"
bench = false

[[test]]
name = "tests"
path = "tests/tests.rs"

[[bench]]
name = "throughput"
path = "benches/throughput.rs"
harness = false

[dependencies.libc]
version = "0.2.113"
version = "0.2.158"

[dependencies.log]
version = "0.4"
Expand All @@ -44,22 +64,35 @@ default-features = false

[dev-dependencies.criterion]
version = "0.5"
features = ["async_tokio"]
default-features = false

[dev-dependencies.tokio]
version = "1.28.1"
features = [
"sync",
"rt",
"rt-multi-thread",
"net",
]

[build-dependencies.cfg_aliases]
version = "0.2"

[features]
default = [
"tracing",
"log",
]
direct-log = ["dep:log"]
fast-apple-datapath = []
log = ["tracing/log"]

[target."cfg(any(target_os = \"linux\", target_os = \"windows\"))"]

[target."cfg(windows)".dependencies.once_cell]
version = "1.19"

[target."cfg(windows)".dependencies.windows-sys]
version = "0.52"
version = ">=0.52, <=0.59"
features = [
"Win32_Foundation",
"Win32_System_IO",
Expand Down
143 changes: 98 additions & 45 deletions third_party/rust/quinn-udp/benches/throughput.rs
Original file line number Diff line number Diff line change
@@ -1,75 +1,128 @@
use std::{
cmp::min,
io::{ErrorKind, IoSliceMut},
net::{Ipv4Addr, Ipv6Addr, UdpSocket},
};

use criterion::{criterion_group, criterion_main, Criterion};
use quinn_udp::{RecvMeta, Transmit, UdpSocketState};
use std::cmp::min;
use std::{io::IoSliceMut, net::UdpSocket, slice};
use tokio::{io::Interest, runtime::Runtime};

use quinn_udp::{RecvMeta, Transmit, UdpSocketState, BATCH_SIZE};

pub fn criterion_benchmark(c: &mut Criterion) {
const TOTAL_BYTES: usize = 10 * 1024 * 1024;
// Maximum GSO buffer size is 64k.
const MAX_BUFFER_SIZE: usize = u16::MAX as usize;
const SEGMENT_SIZE: usize = 1280;

let send = UdpSocket::bind("[::1]:0")
.or_else(|_| UdpSocket::bind("127.0.0.1:0"))
.unwrap();
let recv = UdpSocket::bind("[::1]:0")
.or_else(|_| UdpSocket::bind("127.0.0.1:0"))
.unwrap();
let max_segments = min(
UdpSocketState::new((&send).into())
.unwrap()
.max_gso_segments(),
MAX_BUFFER_SIZE / SEGMENT_SIZE,
);
let dst_addr = recv.local_addr().unwrap();
let send_state = UdpSocketState::new((&send).into()).unwrap();
let recv_state = UdpSocketState::new((&recv).into()).unwrap();
// Reverse non-blocking flag set by `UdpSocketState` to make the test non-racy
recv.set_nonblocking(false).unwrap();

let mut receive_buffer = vec![0; MAX_BUFFER_SIZE];
let mut meta = RecvMeta::default();

for gso_enabled in [false, true] {
let mut group = c.benchmark_group(format!("gso_{}", gso_enabled));
group.throughput(criterion::Throughput::Bytes(TOTAL_BYTES as u64));
let rt = Runtime::new().unwrap();
let _guard = rt.enter();

let (send_state, send_socket) = new_socket();
let (recv_state, recv_socket) = new_socket();
let dst_addr = recv_socket.local_addr().unwrap();

let mut permutations = vec![];
for gso_enabled in [
false,
#[cfg(any(target_os = "linux", target_os = "windows", apple))]
true,
] {
for gro_enabled in [false, true] {
#[cfg(target_os = "windows")]
if gso_enabled && !gro_enabled {
// Windows requires receive buffer to fit entire datagram on GRO
// enabled socket.
//
// OS error: "A message sent on a datagram socket was larger
// than the internal message buffer or some other network limit,
// or the buffer used to receive a datagram into was smaller
// than the datagram itself."
continue;
}

for recvmmsg_enabled in [false, true] {
permutations.push((gso_enabled, gro_enabled, recvmmsg_enabled));
}
}
}

let segments = if gso_enabled { max_segments } else { 1 };
let msg = vec![0xAB; SEGMENT_SIZE * segments];
for (gso_enabled, gro_enabled, recvmmsg_enabled) in permutations {
let mut group = c.benchmark_group(format!(
"gso_{}_gro_{}_recvmmsg_{}",
gso_enabled, gro_enabled, recvmmsg_enabled
));
group.throughput(criterion::Throughput::Bytes(TOTAL_BYTES as u64));

let gso_segments = if gso_enabled {
send_state.max_gso_segments()
} else {
1
};
let msg = vec![0xAB; min(MAX_DATAGRAM_SIZE, SEGMENT_SIZE * gso_segments)];
let transmit = Transmit {
destination: dst_addr,
ecn: None,
contents: &msg,
segment_size: gso_enabled.then_some(SEGMENT_SIZE),
src_ip: None,
};
let gro_segments = if gro_enabled {
recv_state.gro_segments()
} else {
1
};
let batch_size = if recvmmsg_enabled { BATCH_SIZE } else { 1 };

group.bench_function("throughput", |b| {
b.iter(|| {
b.to_async(&rt).iter(|| async {
let mut receive_buffers = vec![vec![0; SEGMENT_SIZE * gro_segments]; batch_size];
let mut receive_slices = receive_buffers
.iter_mut()
.map(|buf| IoSliceMut::new(buf))
.collect::<Vec<_>>();
let mut meta = vec![RecvMeta::default(); batch_size];

let mut sent: usize = 0;
let mut received: usize = 0;
while sent < TOTAL_BYTES {
send_state.send((&send).into(), &transmit).unwrap();
send_socket.writable().await.unwrap();
send_socket
.try_io(Interest::WRITABLE, || {
send_state.send((&send_socket).into(), &transmit)
})
.unwrap();
sent += transmit.contents.len();

let mut received_segments = 0;
while received_segments < segments {
let n = recv_state
.recv(
(&recv).into(),
&mut [IoSliceMut::new(&mut receive_buffer)],
slice::from_mut(&mut meta),
)
.unwrap();
assert_eq!(n, 1);
received_segments += meta.len / meta.stride;
while received < sent {
recv_socket.readable().await.unwrap();
let n = match recv_socket.try_io(Interest::READABLE, || {
recv_state.recv((&recv_socket).into(), &mut receive_slices, &mut meta)
}) {
Ok(n) => n,
// recv.readable() can lead to false positives. Try again.
Err(e) if e.kind() == ErrorKind::WouldBlock => continue,
e => e.unwrap(),
};
received += meta.iter().map(|m| m.len).take(n).sum::<usize>();
}
assert_eq!(received_segments, segments);
}
})
});
}
}

fn new_socket() -> (UdpSocketState, tokio::net::UdpSocket) {
let socket = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0))
.or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0)))
.unwrap();

(
UdpSocketState::new((&socket).into()).unwrap(),
tokio::net::UdpSocket::from_std(socket).unwrap(),
)
}

criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);

const MAX_IP_UDP_HEADER_SIZE: usize = 48;
const MAX_DATAGRAM_SIZE: usize = u16::MAX as usize - MAX_IP_UDP_HEADER_SIZE;
26 changes: 26 additions & 0 deletions third_party/rust/quinn-udp/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use cfg_aliases::cfg_aliases;

fn main() {
// Setup cfg aliases
cfg_aliases! {
// Platforms
apple: {
any(
target_os = "macos",
target_os = "ios",
target_os = "tvos",
target_os = "visionos"
)
},
bsd: {
any(
target_os = "freebsd",
target_os = "openbsd",
target_os = "netbsd"
)
},
// Convenience aliases
apple_fast: { all(apple, feature = "fast-apple-datapath") },
apple_slow: { all(apple, not(feature = "fast-apple-datapath")) },
}
}
4 changes: 2 additions & 2 deletions third_party/rust/quinn-udp/src/cmsg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<'a, M: MsgHdr> Encoder<'a, M> {
/// # Panics
/// - If insufficient buffer space remains.
/// - If `T` has stricter alignment requirements than `M::ControlMessage`
pub(crate) fn push<T: Copy + ?Sized>(&mut self, level: c_int, ty: c_int, value: T) {
pub(crate) fn push<T: Copy>(&mut self, level: c_int, ty: c_int, value: T) {
assert!(mem::align_of::<T>() <= mem::align_of::<M::ControlMessage>());
let space = M::ControlMessage::cmsg_space(mem::size_of_val(&value));
assert!(
Expand Down Expand Up @@ -72,7 +72,7 @@ impl<'a, M: MsgHdr> Encoder<'a, M> {

// Statically guarantees that the encoding operation is "finished" before the control buffer is read
// by `sendmsg` like API.
impl<'a, M: MsgHdr> Drop for Encoder<'a, M> {
impl<M: MsgHdr> Drop for Encoder<'_, M> {
fn drop(&mut self) {
self.hdr.set_control_len(self.len as _);
}
Expand Down
23 changes: 23 additions & 0 deletions third_party/rust/quinn-udp/src/cmsg/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,29 @@ impl MsgHdr for libc::msghdr {
}
}

#[cfg(apple_fast)]
impl MsgHdr for crate::imp::msghdr_x {
type ControlMessage = libc::cmsghdr;

fn cmsg_first_hdr(&self) -> *mut Self::ControlMessage {
let selfp = self as *const _ as *mut libc::msghdr;
unsafe { libc::CMSG_FIRSTHDR(selfp) }
}

fn cmsg_nxt_hdr(&self, cmsg: &Self::ControlMessage) -> *mut Self::ControlMessage {
let selfp = self as *const _ as *mut libc::msghdr;
unsafe { libc::CMSG_NXTHDR(selfp, cmsg) }
}

fn set_control_len(&mut self, len: usize) {
self.msg_controllen = len as _;
}

fn control_len(&self) -> usize {
self.msg_controllen as _
}
}

/// Helpers for [`libc::cmsghdr`]
impl CMsgHdr for libc::cmsghdr {
fn cmsg_len(length: usize) -> usize {
Expand Down
Loading

0 comments on commit e3bd5c4

Please sign in to comment.