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

refactor(udp): introduce log facade #1935

Merged
merged 1 commit into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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");
Ralith marked this conversation as resolved.
Show resolved Hide resolved
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