Skip to content

Commit

Permalink
refactor(udp): introduce log facade
Browse files Browse the repository at this point in the history
Add `log` facade that either dispatches to `tracing` or `log`, or to a no-op
implementation.
  • Loading branch information
mxinden authored and djc committed Aug 2, 2024
1 parent 2ba4966 commit 244b44d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 25 deletions.
30 changes: 24 additions & 6 deletions quinn-udp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ use std::{
time::{Duration, Instant},
};

#[cfg(all(feature = "direct-log", not(feature = "tracing")))]
use log::warn;
#[cfg(feature = "tracing")]
use tracing::warn;

#[cfg(any(unix, windows))]
mod cmsg;

Expand All @@ -58,6 +53,29 @@ mod imp;
#[path = "fallback.rs"]
mod imp;

#[allow(unused_imports, unused_macros)]
mod log {
#[cfg(all(feature = "direct-log", not(feature = "tracing")))]
pub(crate) use log::{debug, error, info, trace, warn};

#[cfg(feature = "tracing")]
pub(crate) use tracing::{debug, error, info, trace, warn};

#[cfg(not(any(feature = "direct-log", feature = "tracing")))]
mod no_op {
macro_rules! trace ( ($($tt:tt)*) => {{}} );
macro_rules! debug ( ($($tt:tt)*) => {{}} );
macro_rules! info ( ($($tt:tt)*) => {{}} );
macro_rules! log_warn ( ($($tt:tt)*) => {{}} );
macro_rules! error ( ($($tt:tt)*) => {{}} );

pub(crate) use {debug, error, info, log_warn as warn, trace};
}

#[cfg(not(any(feature = "direct-log", feature = "tracing")))]
pub(crate) use no_op::*;
}

pub use imp::UdpSocketState;

/// Number of UDP packets to send/receive at a time
Expand Down Expand Up @@ -139,7 +157,7 @@ fn log_sendmsg_error(
let last_send_error = &mut *last_send_error.lock().expect("poisend lock");
if now.saturating_duration_since(*last_send_error) > IO_ERROR_LOG_INTERVAL {
*last_send_error = now;
warn!(
log::warn!(
"sendmsg error: {:?}, Transmit: {{ destination: {:?}, src_ip: {:?}, enc: {:?}, len: {:?}, segment_size: {:?} }}",
err, transmit.destination, transmit.src_ip, transmit.ecn, transmit.contents.len(), transmit.segment_size);
}
Expand Down
17 changes: 6 additions & 11 deletions quinn-udp/src/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,11 @@ use std::{
time::Instant,
};

#[cfg(all(feature = "direct-log", not(feature = "tracing")))]
use log::{debug, error};
use socket2::SockRef;
#[cfg(feature = "tracing")]
use tracing::{debug, error};

use super::{
cmsg, log_sendmsg_error, EcnCodepoint, RecvMeta, Transmit, UdpSockRef, IO_ERROR_LOG_INTERVAL,
cmsg, log::debug, log_sendmsg_error, EcnCodepoint, RecvMeta, Transmit, UdpSockRef,
IO_ERROR_LOG_INTERVAL,
};

// Defined in netinet6/in6.h on OpenBSD, this is not yet exported by the libc crate
Expand Down Expand Up @@ -89,11 +86,10 @@ impl UdpSocketState {
// older macos versions also don't have the flag and will error out if we don't ignore it
#[cfg(not(any(target_os = "openbsd", target_os = "netbsd")))]
if is_ipv4 || !io.only_v6()? {
#[allow(unused_variables)]
if let Err(err) = set_socket_option(&*io, libc::IPPROTO_IP, libc::IP_RECVTOS, OPTION_ON)
if let Err(_err) =
set_socket_option(&*io, libc::IPPROTO_IP, libc::IP_RECVTOS, OPTION_ON)
{
#[cfg(any(feature = "tracing", feature = "direct-log"))]
debug!("Ignoring error setting IP_RECVTOS on socket: {err:?}");
debug!("Ignoring error setting IP_RECVTOS on socket: {_err:?}");
}
}

Expand Down Expand Up @@ -289,8 +285,7 @@ fn send(
// Prevent new transmits from being scheduled using GSO. Existing GSO transmits
// may already be in the pipeline, so we need to tolerate additional failures.
if state.max_gso_segments() > 1 {
#[cfg(any(feature = "tracing", feature = "direct-log"))]
error!("got transmit error, halting segmentation offload");
crate::log::error!("got transmit error, halting segmentation offload");
state
.max_gso_segments
.store(1, std::sync::atomic::Ordering::Relaxed);
Expand Down
9 changes: 1 addition & 8 deletions quinn-udp/src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,12 @@ use std::{
};

use libc::{c_int, c_uint};
#[cfg(all(feature = "direct-log", not(feature = "tracing")))]
use log::{debug, error};
use once_cell::sync::Lazy;
#[cfg(feature = "tracing")]
use tracing::{debug, error};
use windows_sys::Win32::Networking::WinSock;

use crate::{
cmsg::{self, CMsgHdr},
log::{debug, error},
log_sendmsg_error, EcnCodepoint, RecvMeta, Transmit, UdpSockRef, IO_ERROR_LOG_INTERVAL,
};

Expand Down Expand Up @@ -64,7 +61,6 @@ impl UdpSocketState {

// We don't support old versions of Windows that do not enable access to `WSARecvMsg()`
if WSARECVMSG_PTR.is_none() {
#[cfg(any(feature = "tracing", feature = "direct-log"))]
error!("network stack does not support WSARecvMsg function");

return Err(io::Error::from(io::ErrorKind::Unsupported));
Expand Down Expand Up @@ -392,7 +388,6 @@ const OPTION_ON: u32 = 1;
static WSARECVMSG_PTR: Lazy<WinSock::LPFN_WSARECVMSG> = Lazy::new(|| {
let s = unsafe { WinSock::socket(WinSock::AF_INET as _, WinSock::SOCK_DGRAM as _, 0) };
if s == WinSock::INVALID_SOCKET {
#[cfg(any(feature = "tracing", feature = "direct-log"))]
debug!(
"ignoring WSARecvMsg function pointer due to socket creation error: {}",
io::Error::last_os_error()
Expand Down Expand Up @@ -422,13 +417,11 @@ static WSARECVMSG_PTR: Lazy<WinSock::LPFN_WSARECVMSG> = Lazy::new(|| {
};

if rc == -1 {
#[cfg(any(feature = "tracing", feature = "direct-log"))]
debug!(
"ignoring WSARecvMsg function pointer due to ioctl error: {}",
io::Error::last_os_error()
);
} else if len as usize != mem::size_of::<WinSock::LPFN_WSARECVMSG>() {
#[cfg(any(feature = "tracing", feature = "direct-log"))]
debug!("ignoring WSARecvMsg function pointer due to pointer size mismatch");
wsa_recvmsg_ptr = None;
}
Expand Down

0 comments on commit 244b44d

Please sign in to comment.