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

Provide musl libc compatibility #696

Merged
merged 9 commits into from
Apr 24, 2023
69 changes: 69 additions & 0 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ jobs:
- stable
- beta
- 1.65.0
target:
- ""
os: [ubuntu-latest]
features:
- ""
Expand Down Expand Up @@ -52,6 +54,39 @@ jobs:
files: lcov.info
fail_ci_if_error: false

build-musl:
name: Build+test-musl
runs-on: ${{ matrix.os }}
strategy:
matrix:
rust:
- stable
- beta
- 1.65.0
target:
- "x86_64-unknown-linux-musl"
os: [ubuntu-latest]
features:
- ""
- "--features sentry"
- "--features rfc-algorithm"
steps:
- name: Checkout sources
uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab
with:
persist-credentials: false
- name: Install ${{ matrix.rust }} toolchain
uses: actions-rs/toolchain@16499b5e05bf2e26879000db0c1d13f7e13fa3af
with:
toolchain: ${{ matrix.rust }}
override: true
- name: cargo build
run: cargo build ${{ matrix.features }}
- name: cargo test
run: cargo test
env:
RUST_BACKTRACE: 1

unused:
name: Unused dependencies
runs-on: ubuntu-latest
Expand Down Expand Up @@ -185,6 +220,40 @@ jobs:
command: clippy
args: --target armv7-unknown-linux-gnueabihf --workspace --all-targets -- -D warnings

clippy-musl:
name: ClippyMusl
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab
with:
persist-credentials: false
- name: Install rust toolchain
uses: actions-rs/toolchain@16499b5e05bf2e26879000db0c1d13f7e13fa3af
with:
toolchain: stable
override: true
default: true
components: clippy
target: x86_64-unknown-linux-musl
# Use zig as our C compiler for convenient cross-compilation. We run into rustls having a dependency on `ring`.
# This crate uses C and assembly code, and because of its build scripts, `cargo clippy` needs to be able to compile
# that code for our target.
- uses: goto-bus-stop/setup-zig@869a4299cf8ac7db4ebffaec36ad82a682f88acb
with:
version: 0.9.0
- name: Install cargo-zigbuild
uses: taiki-e/install-action@f0b89cda51dc27169e64a0dbc20182a1ec84d8ff
with:
tool: cargo-zigbuild
- name: Run clippy
uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505
env:
TARGET_CC: "/home/runner/.cargo/bin/cargo-zigbuild zig cc -- -target x86_64-linux-musl"
with:
command: clippy
args: --target x86_64-unknown-linux-musl --workspace --all-targets -- -D warnings

fuzz:
name: Smoke-test fuzzing targets
runs-on: ubuntu-20.04
Expand Down
5 changes: 4 additions & 1 deletion ntp-daemon/src/spawn/standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,10 @@ mod tests {
|| addr2 != orig_addr
|| addr3 != orig_addr
|| addr4 != orig_addr
|| addr5 != orig_addr
|| addr5 != orig_addr,
"{:?} should not be in {:?}",
orig_addr,
[addr1, addr2, addr3, addr4, addr5],
);
}

Expand Down
53 changes: 49 additions & 4 deletions ntp-os-clock/src/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use ntp_proto::{NtpClock, NtpDuration, NtpLeapIndicator, NtpTimestamp, PollInter

// Libc has no good other way of obtaining this, so let's at least make our functions
// more readable.
#[cfg(target_os = "linux")]
#[cfg(all(target_os = "linux", target_env = "gnu"))]
pub(crate) const EMPTY_TIMEX: libc::timex = libc::timex {
modes: 0,
offset: 0,
Expand Down Expand Up @@ -53,6 +53,34 @@ pub(crate) const EMPTY_TIMEX: libc::timex = libc::timex {
__unused11: 0,
};

#[cfg(all(target_os = "linux", target_env = "musl"))]
pub(crate) const EMPTY_TIMEX: libc::timex = libc::timex {
modes: 0,
offset: 0,
freq: 0,
maxerror: 0,
esterror: 0,
status: 0,
constant: 0,
precision: 0,
tolerance: 0,
time: libc::timeval {
tv_sec: 0,
tv_usec: 0,
},
tick: 0,
ppsfreq: 0,
jitter: 0,
shift: 0,
stabil: 0,
jitcnt: 0,
calcnt: 0,
errcnt: 0,
stbcnt: 0,
tai: 0,
__padding: [0; 11],
};

#[cfg(any(target_os = "freebsd", target_os = "macos"))]
pub(crate) const EMPTY_TIMEX: libc::timex = libc::timex {
modes: 0,
Expand Down Expand Up @@ -181,11 +209,20 @@ impl UnixNtpClock {
}

fn ntp_adjtime(timex: &mut libc::timex) -> Result<(), Error> {
#[cfg(any(target_os = "freebsd", target_os = "macos", target_env = "gnu"))]
use libc::ntp_adjtime as adjtime;

// ntp_adjtime is equivalent to adjtimex for our purposes
//
// https://man7.org/linux/man-pages/man2/adjtimex.2.html
#[cfg(all(target_os = "linux", target_env = "musl"))]
use libc::adjtimex as adjtime;

// We don't care about the time status, so the non-error
// information in the return value of ntp_adjtime can be ignored.
// The ntp_adjtime call is safe because the reference always
// points to a valid libc::timex.
if unsafe { libc::ntp_adjtime(timex) } == -1 {
if unsafe { adjtime(timex) } == -1 {
Err(convert_errno())
} else {
Ok(())
Expand All @@ -206,7 +243,15 @@ impl UnixNtpClock {

let mut timespec = self.clock_gettime()?;

timespec.tv_sec += offset_secs as libc::time_t;
timespec.tv_sec += {
// this looks a little strange. it is to work around the `libc::time_t` type being
// deprectated for musl in rust's libc.
Copy link
Contributor Author

@sanmai-NL sanmai-NL Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to explicitly silence this warning instead. There's no recommended migration path. It's questionable whether the deprecation will be upheld. See: rust-lang/libc#1848

(Typo: deprecated.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I read the issue, and ... it's really unclear what they actually want to happen there. it must eventually be changed because that is just what musl expects now, right? I'll amend this to ignore the warning, and link to the issue

if true {
offset_secs as _
} else {
timespec.tv_sec
}
};
timespec.tv_nsec += offset_nanos as libc::c_long;

self.clock_settime(timespec)?;
Expand All @@ -221,7 +266,7 @@ impl UnixNtpClock {
let mut timex = libc::timex {
modes: libc::ADJ_SETOFFSET | libc::MOD_NANO,
time: libc::timeval {
tv_sec: secs as libc::time_t,
tv_sec: secs as _,
tv_usec: nanos as libc::suseconds_t,
},
..crate::unix::EMPTY_TIMEX
Expand Down
4 changes: 2 additions & 2 deletions ntp-udp/src/hwtimestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn set_hardware_timestamp(
};

let fd = udp_socket.as_raw_fd();
cerr(unsafe { libc::ioctl(fd, libc::SIOCSHWTSTAMP as libc::c_ulong, &mut ifreq) })?;
cerr(unsafe { libc::ioctl(fd, libc::SIOCSHWTSTAMP as _, &mut ifreq) })?;

Ok(())
}
Expand All @@ -51,7 +51,7 @@ fn get_hardware_timestamp(
};

let fd = udp_socket.as_raw_fd();
cerr(unsafe { libc::ioctl(fd, libc::SIOCGHWTSTAMP as libc::c_ulong, &mut ifreq) })?;
cerr(unsafe { libc::ioctl(fd, libc::SIOCGHWTSTAMP as _, &mut ifreq) })?;

Ok(tstamp_config)
}
Expand Down
38 changes: 26 additions & 12 deletions ntp-udp/src/raw_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ mod set_timestamping_options {
}

mod recv_message {
use std::{io::IoSliceMut, marker::PhantomData, net::SocketAddr, os::unix::prelude::AsRawFd};
use std::{
io::IoSliceMut, marker::PhantomData, mem::MaybeUninit, net::SocketAddr,
os::unix::prelude::AsRawFd,
};

use tracing::warn;

Expand All @@ -177,6 +180,17 @@ mod recv_message {
Error,
}

fn empty_msghdr() -> libc::msghdr {
// On `target_env = "musl"`, there are several private padding fields.
// the position of these padding fields depends on the system endianness,
// so keeping making them public does not really help.
//
// Safety:
//
// all fields are either integer or pointer types. For those types, 0 is a valid value
unsafe { MaybeUninit::<libc::msghdr>::zeroed().assume_init() }
}

pub(crate) fn receive_message<'a>(
socket: &std::net::UdpSocket,
packet_buf: &mut [u8],
Expand All @@ -190,15 +204,15 @@ mod recv_message {
let mut buf_slice = IoSliceMut::new(packet_buf);
let mut addr = zeroed_sockaddr_storage();

let mut mhdr = libc::msghdr {
msg_control: control_buf.as_mut_ptr().cast::<libc::c_void>(),
msg_controllen: control_buf.len() as _,
msg_iov: (&mut buf_slice as *mut IoSliceMut).cast::<libc::iovec>(),
msg_iovlen: 1,
msg_flags: 0,
msg_name: (&mut addr as *mut libc::sockaddr_storage).cast::<libc::c_void>(),
msg_namelen: std::mem::size_of::<libc::sockaddr_storage>() as u32,
};
let mut mhdr = empty_msghdr();

mhdr.msg_control = control_buf.as_mut_ptr().cast::<libc::c_void>();
mhdr.msg_controllen = control_buf.len() as _;
mhdr.msg_iov = (&mut buf_slice as *mut IoSliceMut).cast::<libc::iovec>();
mhdr.msg_iovlen = 1;
mhdr.msg_flags = 0;
mhdr.msg_name = (&mut addr as *mut libc::sockaddr_storage).cast::<libc::c_void>();
mhdr.msg_namelen = std::mem::size_of::<libc::sockaddr_storage>() as u32;
sanmai-NL marked this conversation as resolved.
Show resolved Hide resolved

let receive_flags = match queue {
MessageQueue::Normal => 0,
Expand Down Expand Up @@ -439,8 +453,8 @@ pub(crate) mod timestamping_config {
},
};

const SIOCETHTOOL: u64 = 0x8946;
cerr(unsafe { libc::ioctl(fd, SIOCETHTOOL as libc::c_ulong, &ifr) }).unwrap();
// SIOCETHTOOL = 0x8946 (Ethtool interface) Linux ioctl request
cerr(unsafe { libc::ioctl(fd, 0x8946, &ifr) }).unwrap();

let support = EnableTimestamps {
rx_software: tsi.so_timestamping & libc::SOF_TIMESTAMPING_RX_SOFTWARE != 0,
Expand Down