From 244b44d8cf790879588615d2cb347b59e18f0b4c Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 23 Jul 2024 16:19:22 +0200 Subject: [PATCH] refactor(udp): introduce log facade Add `log` facade that either dispatches to `tracing` or `log`, or to a no-op implementation. --- quinn-udp/src/lib.rs | 30 ++++++++++++++++++++++++------ quinn-udp/src/unix.rs | 17 ++++++----------- quinn-udp/src/windows.rs | 9 +-------- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/quinn-udp/src/lib.rs b/quinn-udp/src/lib.rs index 0337b9c05..bffdb5c17 100644 --- a/quinn-udp/src/lib.rs +++ b/quinn-udp/src/lib.rs @@ -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; @@ -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 @@ -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); } diff --git a/quinn-udp/src/unix.rs b/quinn-udp/src/unix.rs index 8ce5f29f2..34def460e 100644 --- a/quinn-udp/src/unix.rs +++ b/quinn-udp/src/unix.rs @@ -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 @@ -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:?}"); } } @@ -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); diff --git a/quinn-udp/src/windows.rs b/quinn-udp/src/windows.rs index 475a71657..623713788 100644 --- a/quinn-udp/src/windows.rs +++ b/quinn-udp/src/windows.rs @@ -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, }; @@ -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)); @@ -392,7 +388,6 @@ const OPTION_ON: u32 = 1; static WSARECVMSG_PTR: Lazy = 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() @@ -422,13 +417,11 @@ static WSARECVMSG_PTR: Lazy = 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::() { - #[cfg(any(feature = "tracing", feature = "direct-log"))] debug!("ignoring WSARecvMsg function pointer due to pointer size mismatch"); wsa_recvmsg_ptr = None; }