Skip to content

Commit

Permalink
chore: Factor out packet logging (#2396)
Browse files Browse the repository at this point in the history
* chore: Factor out packet logging

Because `output_path` and `input_path` are getting long, and there is a lot of redundancy between the calls to `dump` and `qlog`.

* tos

* Minimize diff

* Minimize more

* More

* Less

* Fix len and tos

* clippy

* TODO

* Update neqo-transport/src/connection/mod.rs

Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>

---------

Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Martin Thomson <mt@lowentropy.net>
  • Loading branch information
larseggert and martinthomson authored Jan 31, 2025
1 parent 379722c commit 59ba66c
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 156 deletions.
52 changes: 0 additions & 52 deletions neqo-transport/src/connection/dump.rs

This file was deleted.

64 changes: 34 additions & 30 deletions neqo-transport/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use std::{
cell::RefCell,
cmp::{max, min},
fmt::{self, Debug},
fmt::{self, Debug, Write as _},
iter, mem,
net::{IpAddr, SocketAddr},
num::NonZeroUsize,
Expand Down Expand Up @@ -44,7 +44,7 @@ use crate::{
CloseError, Frame, FrameType, FRAME_TYPE_CONNECTION_CLOSE_APPLICATION,
FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT,
},
packet::{DecryptedPacket, PacketBuilder, PacketNumber, PacketType, PublicPacket},
packet::{self, DecryptedPacket, PacketBuilder, PacketNumber, PacketType, PublicPacket},
path::{Path, PathRef, Paths},
qlog,
quic_datagrams::{DatagramTracking, QuicDatagrams},
Expand All @@ -61,15 +61,13 @@ use crate::{
AppError, CloseReason, Error, Res, StreamId,
};

mod dump;
mod idle;
pub mod params;
mod saved;
mod state;
#[cfg(test)]
pub mod test_internal;

use dump::dump_packet;
use idle::IdleTimeout;
pub use params::ConnectionParameters;
use params::PreferredAddressConfig;
Expand Down Expand Up @@ -1586,15 +1584,9 @@ impl Connection {
Ok(payload) => {
// OK, we have a valid packet.
self.idle_timeout.on_packet_received(now);
dump_packet(
self,
path,
"-> RX",
payload.packet_type(),
payload.pn(),
&payload[..],
d.tos(),
d.len(),
self.log_packet(
packet::MetaData::new_in(path, d.tos(), packet.len(), &payload),
now,
);

#[cfg(feature = "build-fuzzing-corpus")]
Expand All @@ -1607,7 +1599,6 @@ impl Connection {
neqo_common::write_item_to_fuzzing_corpus(target, &payload[..]);
}

qlog::packet_received(&self.qlog, &packet, &payload, now);
let space = PacketNumberSpace::from(payload.packet_type());
if let Some(space) = self.acks.get_mut(space) {
if space.is_duplicate(payload.pn()) {
Expand Down Expand Up @@ -2440,22 +2431,14 @@ impl Connection {
continue;
}

dump_packet(
self,
path,
"TX ->",
pt,
pn,
&builder.as_ref()[payload_start..],
path.borrow().tos(),
builder.len() + aead_expansion,
);
qlog::packet_sent(
&self.qlog,
pt,
pn,
builder.len() - header_start + aead_expansion,
&builder.as_ref()[payload_start..],
self.log_packet(
packet::MetaData::new_out(
path,
pt,
pn,
builder.len() + aead_expansion,
&builder.as_ref()[payload_start..],
),
now,
);

Expand Down Expand Up @@ -3545,6 +3528,27 @@ impl Connection {
pub fn plpmtu(&self) -> usize {
self.paths.primary().unwrap().borrow().plpmtu()
}

fn log_packet(&self, meta: packet::MetaData, now: Instant) {
if !log::log_enabled!(log::Level::Debug) {
return;
}

let mut s = String::new();
let mut d = Decoder::from(meta.payload());
while d.remaining() > 0 {
let Ok(f) = Frame::decode(&mut d) else {
s.push_str(" [broken]...");
break;
};
let x = f.dump();
if !x.is_empty() {
_ = write!(&mut s, "\n {} {}", meta.direction(), &x);
}
}
qdebug!("[{self}] {meta}{s}");
qlog::packet_io(&self.qlog, meta, now);
}
}

impl EventProvider for Connection {
Expand Down
122 changes: 122 additions & 0 deletions neqo-transport/src/packet/metadata.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Enable just this file for logging to just see packets.
// e.g. "RUST_LOG=neqo_transport::dump neqo-client ..."

use std::fmt::Display;

use neqo_common::IpTos;
use qlog::events::quic::PacketHeader;

use super::DecryptedPacket;
use crate::{
packet::{PacketNumber, PacketType},
path::PathRef,
};

#[derive(Clone, Copy)]
pub enum Direction {
Tx,
Rx,
}

impl Display for Direction {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Tx => write!(f, "TX ->"),
Self::Rx => write!(f, "-> RX"),
}
}
}

pub struct MetaData<'a> {
path: &'a PathRef,
direction: Direction,
packet_type: PacketType,
packet_number: PacketNumber,
tos: IpTos,
len: usize,
payload: &'a [u8],
}

impl MetaData<'_> {
pub fn new_in<'a>(
path: &'a PathRef,
tos: IpTos,
len: usize,
decrypted: &'a DecryptedPacket,
) -> MetaData<'a> {
MetaData {
path,
direction: Direction::Rx,
packet_type: decrypted.packet_type(),
packet_number: decrypted.pn(),
tos,
len,
payload: decrypted,
}
}

pub fn new_out<'a>(
path: &'a PathRef,
packet_type: PacketType,
packet_number: PacketNumber,
length: usize,
payload: &'a [u8],
) -> MetaData<'a> {
MetaData {
path,
direction: Direction::Tx,
packet_type,
packet_number,
tos: path.borrow().tos(),
len: length,
payload,
}
}

#[must_use]
pub const fn direction(&self) -> Direction {
self.direction
}

#[must_use]
pub const fn length(&self) -> usize {
self.len
}

#[must_use]
pub const fn payload(&self) -> &[u8] {
self.payload
}
}

impl From<MetaData<'_>> for PacketHeader {
fn from(val: MetaData<'_>) -> Self {
Self::with_type(
val.packet_type.into(),
Some(val.packet_number),
None,
None,
None,
)
}
}

impl Display for MetaData<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"pn={} type={:?} {} {:?} len {}",
self.packet_number,
self.packet_type,
self.path.borrow(),
self.tos,
self.len,
)
}
}
3 changes: 3 additions & 0 deletions neqo-transport/src/packet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ const SAMPLE_SIZE: usize = 16;
const SAMPLE_OFFSET: usize = 4;
const MAX_PACKET_NUMBER_LEN: usize = 4;

pub mod metadata;
mod retry;

pub use metadata::MetaData;

pub type PacketNumber = u64;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down
Loading

0 comments on commit 59ba66c

Please sign in to comment.