Skip to content
Closed
Show file tree
Hide file tree
Changes from 12 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
6 changes: 3 additions & 3 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ jobs:
- name: Check
run: |
# shellcheck disable=SC2086
cargo check $BUILD_TYPE --locked --all-targets --features ci
cargo check $BUILD_TYPE --locked --all-targets --features ci,qlog

- name: Run tests and determine coverage
env:
Expand All @@ -133,9 +133,9 @@ jobs:
export DUMP_SIMULATION_SEEDS
# shellcheck disable=SC2086
if [ "$TOOLCHAIN" == "stable" ]; then
cargo llvm-cov test $BUILD_TYPE --locked --include-ffi --features ci --codecov --output-path codecov.json
cargo llvm-cov test $BUILD_TYPE --locked --include-ffi --features ci,qlog --codecov --output-path codecov.json
else
cargo test $BUILD_TYPE --locked --features ci
cargo test $BUILD_TYPE --locked --features ci,qlog
fi

- name: Run client/server transfer
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/sanitize.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ jobs:
PWD=$(pwd)
export LSAN_OPTIONS="suppressions=$PWD/suppressions.txt"
fi
cargo test --locked -Z build-std --features ci --target "$TARGET"
cargo test --locked -Z build-std --features ci,qlog --target "$TARGET"
cargo hack --workspace test --locked -Z build-std --target "$TARGET"

- name: Save simulation seeds artifact
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ is suited to a live deployment.
To build Neqo:

```shell
cargo build
cargo build --features qlog
Comment thread
larseggert marked this conversation as resolved.
Outdated
```
You can leave out `--features qlog` if you do not need QLOG support.

This will use a system-installed [NSS][NSS] library if it is new enough. (See "Build with Separate NSS/NSPR" below if NSS is not installed or it is deemed too old.)

Expand Down Expand Up @@ -65,7 +66,7 @@ Note: If you did not already compile NSS separately, you need to have

### QUIC logging

Enable generation of [QLOG][QLOG] logs with:
When compiled with `--features qlog`, enable generation of [QLOG][QLOG] logs with:
Comment thread
larseggert marked this conversation as resolved.
Outdated

```shell
target/debug/neqo-server '[::]:12345' --qlog-dir .
Expand Down
4 changes: 3 additions & 1 deletion neqo-bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ neqo-crypto = { path = "./../neqo-crypto" }
neqo-http3 = { path = "./../neqo-http3" }
neqo-transport = { path = "./../neqo-transport" }
neqo-udp = { path = "./../neqo-udp" }
qlog = { workspace = true }
qlog = { workspace = true, optional = true }
quinn-udp = { workspace = true }
regex = { workspace = true, features = ["unicode-perl"] }
rustc-hash = { workspace = true }
Expand All @@ -52,6 +52,8 @@ neqo-transport = { path = "./../neqo-transport", features = ["draft-29"] }
tokio = { version = "1", default-features = false, features = ["sync"] }

[features]
default = ["qlog"]
qlog = ["dep:qlog", "neqo-common/qlog", "neqo-transport/qlog", "neqo-http3/qlog"]
bench = ["neqo-bin/bench", "neqo-http3/bench", "neqo-transport/bench"]
fast-apple-datapath = ["quinn-udp/fast-apple-datapath"]
draft-29 = []
Expand Down
4 changes: 2 additions & 2 deletions neqo-bin/src/client/http3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ pub fn create_client(
.max_concurrent_push_streams(args.max_concurrent_push_streams),
);

let qlog = qlog_new(args, hostname, client.connection_id())?;
client.set_qlog(qlog);
client.set_qlog(qlog_new(args, hostname, client.connection_id())?);

if let Some(ech) = &args.ech {
client.enable_ech(ech)?;
}
Expand Down
18 changes: 16 additions & 2 deletions neqo-bin/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ use futures::{
future::{select, Either},
FutureExt as _, TryFutureExt as _,
};
use neqo_common::{qdebug, qerror, qinfo, qlog::Qlog, qwarn, Datagram, Role};
#[cfg(feature = "qlog")]
use neqo_common::Role;
use neqo_common::{qdebug, qerror, qinfo, qlog::Qlog, qwarn, Datagram};
use neqo_crypto::{
constants::{TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256},
init, Cipher, ResumptionToken,
Expand Down Expand Up @@ -52,6 +54,7 @@ pub enum Error {
Http3(neqo_http3::Error),
#[error(transparent)]
Io(io::Error),
#[cfg(feature = "qlog")]
#[error(transparent)]
Qlog(qlog::Error),
#[error(transparent)]
Expand Down Expand Up @@ -80,6 +83,7 @@ impl From<neqo_http3::Error> for Error {
}
}

#[cfg(feature = "qlog")]
impl From<qlog::Error> for Error {
fn from(err: qlog::Error) -> Self {
Self::Qlog(err)
Expand Down Expand Up @@ -528,19 +532,29 @@ impl<'a, H: Handler> Runner<'a, H> {
}
}

#[cfg_attr(
not(feature = "qlog"),
expect(unused_variables, reason = "Only used with qlog.")
)]
fn qlog_new(args: &Args, hostname: &str, cid: &ConnectionId) -> Res<Qlog> {
Comment thread
larseggert marked this conversation as resolved.
Outdated
let Some(qlog_dir) = args.shared.qlog_dir.clone() else {
return Ok(Qlog::disabled());
};

#[cfg(not(feature = "qlog"))]
return Err(Error::Argument(
"qlog feature not enabled, cannot use --qlog-dir",
));

// hostname might be an IPv6 address, e.g. `[::1]`. `:` is an invalid
// Windows file name character.
#[cfg(windows)]
#[cfg(all(feature = "qlog", windows))]
let hostname: String = hostname
.chars()
.map(|c| if c == ':' { '_' } else { c })
.collect();

#[cfg(feature = "qlog")]
Qlog::enabled_with_file(
qlog_dir,
Role::Client,
Expand Down
11 changes: 8 additions & 3 deletions neqo-bin/src/lib.rs
Comment thread
larseggert marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,13 @@ pub struct SharedArgs {
/// This client still only does HTTP/3 no matter what the ALPN says.
alpn: String,

#[arg(name = "qlog-dir", long, value_parser=clap::value_parser!(PathBuf))]
/// Enable QLOG logging and QLOG traces to this directory
#[arg(name = "qlog-dir", long, value_parser=clap::value_parser!(PathBuf), help=
if cfg!(feature = "qlog") {
"Enable QLOG logging and QLOG traces to this directory"
} else {
"QLOG logging is disabled"
}
)]
qlog_dir: Option<PathBuf>,

#[arg(name = "encoder-table-size", long, default_value = "16384")]
Expand Down Expand Up @@ -274,7 +279,7 @@ pub enum Error {
Argument(&'static str),
}

#[cfg(not(target_os = "netbsd"))] // FIXME: Test fails on NetBSD.
#[cfg(all(not(feature = "qlog"), target_os = "netbsd"))] // FIXME: Test fails on NetBSD.
Comment thread
larseggert marked this conversation as resolved.
Outdated
Comment thread
larseggert marked this conversation as resolved.
Outdated
#[cfg(test)]
#[cfg_attr(coverage_nightly, coverage(off))]
mod tests {
Expand Down
3 changes: 1 addition & 2 deletions neqo-bin/src/server/http3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ impl super::HttpServer for HttpServer {
self.server.process_multiple(dgrams, now, max_datagrams)
}

fn process_events(&mut self, _now: Instant) {
let now = Instant::now();
fn process_events(&mut self, now: Instant) {
while let Some(event) = self.server.next_event() {
match event {
Http3ServerEvent::Headers {
Expand Down
1 change: 1 addition & 0 deletions neqo-bin/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ impl From<neqo_http3::Error> for Error {
}
}

#[cfg(feature = "qlog")]
impl From<qlog::Error> for Error {
fn from(_err: qlog::Error) -> Self {
Self::Qlog
Expand Down
4 changes: 3 additions & 1 deletion neqo-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ enum-map = { workspace = true }
env_logger = { version = "0.10", default-features = false }
hex = { workspace = true, optional = true }
log = { workspace = true }
qlog = { workspace = true }
qlog = { workspace = true, optional = true }
strum = { workspace = true }
thiserror = { workspace = true }

Expand All @@ -34,6 +34,8 @@ test-fixture = { path = "../test-fixture" }
regex = { workspace = true }

[features]
default = ["qlog"]
qlog = ["dep:qlog", "test-fixture/qlog"]
bench = ["neqo-crypto/bench", "test-fixture/bench"]
build-fuzzing-corpus = ["hex/alloc"]
ci = []
Expand Down
55 changes: 49 additions & 6 deletions neqo-common/src/qlog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,50 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::time::Instant;
#[cfg(feature = "qlog")]
use std::{
cell::RefCell,
fmt::{self, Display},
fs::OpenOptions,
io::BufWriter,
path::PathBuf,
rc::Rc,
time::{Instant, SystemTime},
time::SystemTime,
};

use qlog::{
streamer::QlogStreamer, CommonFields, Configuration, TraceSeq, VantagePoint, VantagePointType,
};
use qlog::streamer::QlogStreamer;
#[cfg(feature = "qlog")]
use qlog::{CommonFields, Configuration, TraceSeq, VantagePoint, VantagePointType};

#[cfg(feature = "qlog")]
use crate::Role;

#[cfg(not(feature = "qlog"))]
#[expect(
clippy::module_inception,
clippy::module_name_repetitions,
reason = "This is for stubbing out qlog."
)]
mod qlog {
pub struct Error();
pub mod streamer {
pub struct QlogStreamer();
}
pub mod events {
pub struct Event();
pub struct EventData();
}
}

#[derive(Debug, Clone, Default)]
pub struct Qlog {
#[cfg(feature = "qlog")]
inner: Rc<RefCell<Option<SharedStreamer>>>,
}

pub struct SharedStreamer {
#[cfg(feature = "qlog")]
struct SharedStreamer {
qlog_path: PathBuf,
streamer: QlogStreamer,
}
Expand All @@ -36,6 +58,7 @@ impl Qlog {
/// # Errors
///
/// Will return `qlog::Error` if it cannot write to the new file.
#[cfg(feature = "qlog")]
Comment thread
larseggert marked this conversation as resolved.
pub fn enabled_with_file<D: Display>(
mut qlog_path: PathBuf,
role: Role,
Expand Down Expand Up @@ -72,10 +95,12 @@ impl Qlog {
/// # Errors
///
/// Will return `qlog::Error` if it cannot write to the new log.
#[cfg(feature = "qlog")]
pub fn enabled(mut streamer: QlogStreamer, qlog_path: PathBuf) -> Result<Self, qlog::Error> {
streamer.start_log()?;

Ok(Self {
#[cfg(feature = "qlog")]
Comment thread
larseggert marked this conversation as resolved.
Outdated
inner: Rc::new(RefCell::new(Some(SharedStreamer {
qlog_path,
streamer,
Expand All @@ -90,10 +115,15 @@ impl Qlog {
}

/// If logging enabled, closure may generate an event to be logged.
#[cfg_attr(
not(feature = "qlog"),
expect(unused_variables, clippy::unused_self, reason = "Only used with qlog.")
)]
Comment thread
larseggert marked this conversation as resolved.
pub fn add_event_with_instant<F>(&self, f: F, now: Instant)
where
F: FnOnce() -> Option<qlog::events::Event>,
{
#[cfg(feature = "qlog")]
self.add_event_with_stream(|s| {
if let Some(evt) = f() {
s.add_event_with_instant(evt, now)?;
Expand All @@ -103,10 +133,15 @@ impl Qlog {
}

/// If logging enabled, closure may generate an event to be logged.
#[cfg_attr(
not(feature = "qlog"),
expect(unused_variables, clippy::unused_self, reason = "Only used with qlog.")
)]
pub fn add_event_data_with_instant<F>(&self, f: F, now: Instant)
where
F: FnOnce() -> Option<qlog::events::EventData>,
{
#[cfg(feature = "qlog")]
self.add_event_with_stream(|s| {
if let Some(ev_data) = f() {
s.add_event_data_with_instant(ev_data, now)?;
Expand All @@ -117,10 +152,15 @@ impl Qlog {

/// If logging enabled, closure is given the Qlog stream to write events and
/// frames to.
#[cfg_attr(
not(feature = "qlog"),
expect(unused_variables, clippy::unused_self, reason = "Only used with qlog.")
)]
pub fn add_event_with_stream<F>(&self, f: F)
where
F: FnOnce(&mut QlogStreamer) -> Result<(), qlog::Error>,
{
#[cfg(feature = "qlog")]
if let Some(inner) = self.inner.borrow_mut().as_mut() {
if let Err(e) = f(&mut inner.streamer) {
log::error!("Qlog event generation failed with error {e}; closing qlog.");
Expand All @@ -130,12 +170,14 @@ impl Qlog {
}
}

#[cfg(feature = "qlog")]
impl fmt::Debug for SharedStreamer {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Qlog writing to {}", self.qlog_path.display())
}
}

#[cfg(feature = "qlog")]
impl Drop for SharedStreamer {
fn drop(&mut self) {
if let Err(e) = self.streamer.finish_log() {
Expand All @@ -145,6 +187,7 @@ impl Drop for SharedStreamer {
}

#[must_use]
#[cfg(feature = "qlog")]
pub fn new_trace(role: Role) -> TraceSeq {
TraceSeq {
vantage_point: VantagePoint {
Expand Down Expand Up @@ -173,7 +216,7 @@ pub fn new_trace(role: Role) -> TraceSeq {
}
}

#[cfg(test)]
#[cfg(all(test, feature = "qlog"))]
Comment thread
larseggert marked this conversation as resolved.
#[cfg_attr(coverage_nightly, coverage(off))]
mod test {
use qlog::events::Event;
Expand Down
4 changes: 3 additions & 1 deletion neqo-http3/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ neqo-common = { path = "./../neqo-common" }
neqo-crypto = { path = "./../neqo-crypto" }
neqo-qpack = { path = "./../neqo-qpack" }
neqo-transport = { path = "./../neqo-transport" }
qlog = { workspace = true }
qlog = { workspace = true, optional = true }
rustc-hash = { workspace = true}
sfv = { version = "0.14", default-features = false, features = ["parsed-types"] }
strum = { workspace = true}
Expand All @@ -39,6 +39,8 @@ neqo-transport = { path = "./../neqo-transport", features = ["draft-29"] }
test-fixture = { path = "../test-fixture" }

[features]
default = ["qlog"]
qlog = ["dep:qlog", "neqo-common/qlog", "neqo-transport/qlog", "neqo-qpack/qlog", "test-fixture/qlog"]
bench = ["neqo-common/bench", "neqo-crypto/bench", "neqo-qpack/bench", "neqo-transport/bench"]
disable-encryption = ["neqo-transport/disable-encryption", "neqo-crypto/disable-encryption"]
draft-29 = []
Expand Down
1 change: 1 addition & 0 deletions neqo-http3/src/buffered_send_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ impl BufferedStream {
return Ok(false);
}
let res = conn.stream_send_atomic(*stream_id, to_send)?;

if res {
qlog::h3_data_moved_down(conn.qlog_mut(), *stream_id, to_send.len(), now);
}
Expand Down
Loading