diff --git a/CHANGELOG.md b/CHANGELOG.md index 27d5d19..6da7dde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **Separate send/recv reporting in bidir tests** (issue #56) — `--bidir` now reports per-direction bytes and throughput in the summary instead of just the combined total, which was useless on asymmetric links. Plain text shows `Send: X Recv: Y (Total: Z)`; JSON adds `bytes_sent`, `bytes_received`, `throughput_send_mbps`, `throughput_recv_mbps`; CSV gets four new columns; TUI shows `↑ X / ↓ Y` in the throughput panel. Unidirectional tests are unchanged (the existing `bytes_total`/`throughput_mbps` is already the single-direction number). + ### Fixed - **Fast, accurate TCP teardown** (issue #54) — replaced the blocking `shutdown()` drain on the send path with `SO_LINGER=0` on Linux, so cancel and natural end-of-test no longer wait for bufferbloated send buffers to ACK through rate-limited paths. Fixes the "Timed out waiting 2s for N data streams to stop" warning matttbe reported with `-P 4 --mptcp -t 1sec`. - **Sender-side byte-count accuracy** — `stats.bytes_sent` is now clamped to `tcpi_bytes_acked` before abortive close, removing a quiet ~5-10% overcount where the send-buffer tail discarded by RST was being reported as transferred. Download and bidir tests are the primary beneficiaries. diff --git a/ROADMAP.md b/ROADMAP.md index c444d92..5ac0666 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -50,7 +50,7 @@ - [x] **CSV output format** (`--csv`) - frequently requested iperf2 feature never added to iperf3 - [x] **JSON streaming output** (`--json-stream`) - iperf3 3.18 added this, one JSON object per line - [x] **Quiet mode** (`-q`) - suppress interval output, show only summary -- [ ] **Separate send/recv in bidir summary** (issue #56) — currently `--bidir` reports combined `bytes_total` and `throughput_mbps`. For asymmetric links this collapses two different numbers into one; users have to re-run with and without `-R`. Expose split `bytes_sent`/`bytes_received` + per-direction throughput in plain/JSON/CSV/TUI outputs while keeping the combined total for backward compat +- [x] **Separate send/recv in bidir summary** (issue #56) — `--bidir` now reports per-direction bytes and throughput in plain text, JSON, CSV, and TUI outputs. Combined total kept for backward compat ### Usability - [x] **Server max duration** (`--max-duration`) - limit test length server-side diff --git a/benches/throughput.rs b/benches/throughput.rs index dafe591..01af8b5 100644 --- a/benches/throughput.rs +++ b/benches/throughput.rs @@ -165,6 +165,10 @@ fn bench_protocol_serialize_interval(c: &mut Criterion) { lost: None, rtt_us: None, cwnd: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, }, }; @@ -195,6 +199,10 @@ fn bench_protocol_deserialize_interval(c: &mut Criterion) { lost: None, rtt_us: None, cwnd: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, }, }; let json = msg.serialize().unwrap(); @@ -213,6 +221,10 @@ fn bench_protocol_serialize_result(c: &mut Criterion) { streams: vec![], tcp_info: None, udp_stats: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, }; let msg = ControlMessage::Result(result); diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 67f68d7..e3832e5 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -161,7 +161,19 @@ xfr --bidir # Bidirectional xfr --bidir -P 2 # Bidir with 2 streams each direction ``` -Reports separate statistics for upload and download. +Plain text summary shows per-direction bytes and throughput plus the +combined total: + +``` + Transfer: Send: 5.00 GB Recv: 7.00 GB (Total: 12.00 GB) + Throughput: Send: 40.0 Gbps Recv: 56.0 Gbps (Total: 96.0 Gbps) +``` + +JSON adds `bytes_sent`, `bytes_received`, `throughput_send_mbps`, and +`throughput_recv_mbps` fields; CSV gets four matching columns; the TUI +shows `↑` / `↓` throughput in the stats panel. Unidirectional tests +continue to report only the combined number (which equals the +single-direction throughput). ## Direction Control diff --git a/src/client.rs b/src/client.rs index 9a73836..ed93aa8 100644 --- a/src/client.rs +++ b/src/client.rs @@ -128,6 +128,12 @@ pub struct TestProgress { pub cwnd: Option, /// Cumulative retransmits from local TCP_INFO (sender-side, for upload/bidir) pub total_retransmits: Option, + /// Per-direction interval bytes (bidirectional tests only, from client's + /// perspective). `bytes_sent` is what the client sent this interval. + pub bytes_sent: Option, + pub bytes_received: Option, + pub throughput_send_mbps: Option, + pub throughput_recv_mbps: Option, } pub struct Client { @@ -531,6 +537,10 @@ impl Client { cwnd, total_retransmits, streams, + bytes_sent: aggregate.bytes_sent, + bytes_received: aggregate.bytes_received, + throughput_send_mbps: aggregate.throughput_send_mbps, + throughput_recv_mbps: aggregate.throughput_recv_mbps, }) .await; } @@ -1324,6 +1334,10 @@ impl Client { cwnd: aggregate.cwnd, total_retransmits: None, streams, + bytes_sent: aggregate.bytes_sent, + bytes_received: aggregate.bytes_received, + throughput_send_mbps: aggregate.throughput_send_mbps, + throughput_recv_mbps: aggregate.throughput_recv_mbps, }) .await; } diff --git a/src/diff.rs b/src/diff.rs index 5c08aa6..5c70521 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -157,6 +157,10 @@ mod tests { bytes_acked: None, }), udp_stats: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, } } diff --git a/src/main.rs b/src/main.rs index 1ec9678..611d5f7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1295,6 +1295,10 @@ fn build_fallback_result(cumulative_bytes: u64, elapsed_ms: u64) -> xfr::protoco streams: vec![], tcp_info: None, udp_stats: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, } } @@ -2097,6 +2101,10 @@ mod tests { rtt_us: None, cwnd: None, total_retransmits, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, } } diff --git a/src/output/csv.rs b/src/output/csv.rs index b1a9df2..66283b1 100644 --- a/src/output/csv.rs +++ b/src/output/csv.rs @@ -6,12 +6,19 @@ use crate::protocol::{TestResult, TimestampFormat}; pub fn output_csv(result: &TestResult) -> String { let mut output = String::new(); - // Header - output.push_str("test_id,duration_secs,transfer_bytes,throughput_mbps,retransmits,jitter_ms,lost,lost_percent\n"); + // Header. bytes_sent / bytes_received / throughput_send_mbps / + // throughput_recv_mbps are populated only for bidirectional tests; + // unidirectional tests leave those columns empty. + output.push_str( + "test_id,duration_secs,transfer_bytes,throughput_mbps,retransmits,jitter_ms,lost,lost_percent,bytes_sent,bytes_received,throughput_send_mbps,throughput_recv_mbps\n", + ); + + let fmt_u64 = |v: Option| v.map(|n| n.to_string()).unwrap_or_default(); + let fmt_f64 = |v: Option| v.map(|n| format!("{:.2}", n)).unwrap_or_default(); // Summary row output.push_str(&format!( - "{},{:.2},{},{:.2},{},{:.2},{},{:.2}\n", + "{},{:.2},{},{:.2},{},{:.2},{},{:.2},{},{},{},{}\n", result.id, result.duration_ms as f64 / 1000.0, result.bytes_total, @@ -28,6 +35,10 @@ pub fn output_csv(result: &TestResult) -> String { .as_ref() .map(|u| u.lost_percent) .unwrap_or(0.0), + fmt_u64(result.bytes_sent), + fmt_u64(result.bytes_received), + fmt_f64(result.throughput_send_mbps), + fmt_f64(result.throughput_recv_mbps), )); output diff --git a/src/output/plain.rs b/src/output/plain.rs index 7e9a1cb..1737590 100644 --- a/src/output/plain.rs +++ b/src/output/plain.rs @@ -17,14 +17,38 @@ pub fn output_plain(result: &TestResult, mptcp: bool) -> String { " Duration: {:.2}s\n", result.duration_ms as f64 / 1000.0 )); - output.push_str(&format!( - " Transfer: {}\n", - bytes_to_human(result.bytes_total) - )); - output.push_str(&format!( - " Throughput: {}\n", - mbps_to_human(result.throughput_mbps) - )); + + // Bidirectional tests: show per-direction bytes/throughput alongside the total. + // Unidirectional tests report only the combined number (which equals the + // single-direction throughput anyway). + if let (Some(sent), Some(recv), Some(ts_send), Some(ts_recv)) = ( + result.bytes_sent, + result.bytes_received, + result.throughput_send_mbps, + result.throughput_recv_mbps, + ) { + output.push_str(&format!( + " Transfer: Send: {} Recv: {} (Total: {})\n", + bytes_to_human(sent), + bytes_to_human(recv), + bytes_to_human(result.bytes_total) + )); + output.push_str(&format!( + " Throughput: Send: {} Recv: {} (Total: {})\n", + mbps_to_human(ts_send), + mbps_to_human(ts_recv), + mbps_to_human(result.throughput_mbps) + )); + } else { + output.push_str(&format!( + " Transfer: {}\n", + bytes_to_human(result.bytes_total) + )); + output.push_str(&format!( + " Throughput: {}\n", + mbps_to_human(result.throughput_mbps) + )); + } output.push('\n'); if let Some(ref tcp_info) = result.tcp_info { @@ -158,6 +182,10 @@ mod tests { bytes_acked: None, }), udp_stats: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, } } @@ -174,6 +202,34 @@ mod tests { assert!(output.contains("Sender TCP Info (initial subflow):\n")); } + #[test] + fn test_output_plain_bidir_shows_split() { + // Bidirectional result populates the split fields — output should render + // Send/Recv/Total lines so asymmetric throughput is visible. + let mut result = make_result_with_tcp_info(); + result.bytes_sent = Some(5_000_000_000); + result.bytes_received = Some(7_000_000_000); + result.throughput_send_mbps = Some(40_000.0); + result.throughput_recv_mbps = Some(56_000.0); + result.bytes_total = 12_000_000_000; + result.throughput_mbps = 96_000.0; + + let output = output_plain(&result, false); + assert!(output.contains("Send:"), "expected split 'Send:' line"); + assert!(output.contains("Recv:"), "expected split 'Recv:' line"); + assert!(output.contains("(Total:"), "expected combined total"); + } + + #[test] + fn test_output_plain_unidir_no_split() { + // Unidirectional result leaves Options None — output stays single-line. + let output = output_plain(&make_result_with_tcp_info(), false); + assert!( + !output.contains("Send:") && !output.contains("Recv:"), + "unidir output must not show split lines" + ); + } + #[test] fn test_interval_plain_tcp_shows_rtx_rtt() { let output = output_interval_plain( diff --git a/src/protocol.rs b/src/protocol.rs index 069bffc..a62f5dd 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -211,6 +211,19 @@ pub struct AggregateInterval { pub rtt_us: Option, #[serde(skip_serializing_if = "Option::is_none")] pub cwnd: Option, + /// Bidirectional test: bytes sent by the reporting side during this interval. + /// Populated only for bidir tests; None for unidirectional (where `bytes` is authoritative). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bytes_sent: Option, + /// Bidirectional test: bytes received by the reporting side during this interval. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bytes_received: Option, + /// Bidirectional test: per-direction throughput (upload) in Mbps. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub throughput_send_mbps: Option, + /// Bidirectional test: per-direction throughput (download) in Mbps. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub throughput_recv_mbps: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -224,6 +237,19 @@ pub struct TestResult { pub tcp_info: Option, #[serde(skip_serializing_if = "Option::is_none")] pub udp_stats: Option, + /// Bidirectional test: total bytes sent by the reporting side. + /// Populated only for bidir tests; None for unidirectional (where `bytes_total` is authoritative). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bytes_sent: Option, + /// Bidirectional test: total bytes received by the reporting side. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bytes_received: Option, + /// Bidirectional test: per-direction throughput (upload) in Mbps. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub throughput_send_mbps: Option, + /// Bidirectional test: per-direction throughput (download) in Mbps. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub throughput_recv_mbps: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/serve.rs b/src/serve.rs index 4c1658d..896969a 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -1339,6 +1339,42 @@ async fn handle_test_request( } } +/// Compute per-direction byte/throughput totals (from the CLIENT's perspective) +/// when the test is bidirectional. For unidirectional tests returns all None — +/// the existing `bytes_total` and `throughput_mbps` fields already carry the +/// single-direction number. +/// +/// Direction convention: `TestResult.bytes_sent` means "bytes the *client* sent" +/// (its upload direction). Server-local counters are reversed from that view +/// — what the server sent went to the client (the client's downloads), and +/// what the server received came from the client (the client's uploads). So +/// we flip when emitting. +fn directional_totals( + direction: Direction, + stats: &TestStats, + duration_ms: u64, +) -> (Option, Option, Option, Option) { + if direction != Direction::Bidir { + return (None, None, None, None); + } + // Swap: server-sent == client-received, server-received == client-sent. + let client_bytes_sent = stats.total_bytes_received(); + let client_bytes_received = stats.total_bytes_sent(); + let mbps = |bytes: u64| { + if duration_ms == 0 { + 0.0 + } else { + (bytes as f64 * 8.0) / (duration_ms as f64 / 1000.0) / 1_000_000.0 + } + }; + ( + Some(client_bytes_sent), + Some(client_bytes_received), + Some(mbps(client_bytes_sent)), + Some(mbps(client_bytes_received)), + ) +} + /// Run a QUIC bandwidth test #[allow(clippy::too_many_arguments)] async fn run_quic_test( @@ -1500,7 +1536,7 @@ async fn run_quic_test( .zip(intervals.iter()) .map(|(s, i)| s.to_interval(i)) .collect(); - let aggregate = stats.to_aggregate(&intervals); + let aggregate = stats.to_aggregate_with_direction(&intervals, direction == Direction::Bidir); let interval_msg = ControlMessage::Interval { id: id.to_string(), @@ -1596,6 +1632,10 @@ async fn run_quic_test( .map(|s| s.to_result(duration_ms)) .collect(); + // Bidir: split up/down reporting for asymmetric links (issue #56). + let (bytes_sent, bytes_received, throughput_send_mbps, throughput_recv_mbps) = + directional_totals(direction, &stats, duration_ms); + let result = ControlMessage::Result(TestResult { id: id.to_string(), bytes_total, @@ -1604,6 +1644,10 @@ async fn run_quic_test( streams: stream_results, tcp_info: None, udp_stats: None, + bytes_sent, + bytes_received, + throughput_send_mbps, + throughput_recv_mbps, }); ctrl_send @@ -1862,7 +1906,7 @@ async fn run_test( .zip(intervals.iter()) .map(|(s, i)| s.to_interval(i)) .collect(); - let aggregate = stats.to_aggregate(&intervals); + let aggregate = stats.to_aggregate_with_direction(&intervals, direction == Direction::Bidir); let interval_msg = ControlMessage::Interval { id: id.to_string(), @@ -1984,6 +2028,11 @@ async fn run_test( .map(|s| s.to_result(duration_ms)) .collect(); + // For bidir tests, expose per-direction totals so clients can distinguish + // upload from download throughput on asymmetric links (issue #56). + let (bytes_sent, bytes_received, throughput_send_mbps, throughput_recv_mbps) = + directional_totals(direction, &stats, duration_ms); + let result = ControlMessage::Result(TestResult { id: id.to_string(), bytes_total, @@ -1992,6 +2041,10 @@ async fn run_test( streams: stream_results, tcp_info: stats.get_tcp_info(), udp_stats: stats.aggregate_udp_stats(), + bytes_sent, + bytes_received, + throughput_send_mbps, + throughput_recv_mbps, }); // Send result FIRST so the client isn't blocked by slow post-processing diff --git a/src/stats.rs b/src/stats.rs index 0777776..b214590 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -18,6 +18,11 @@ pub struct StreamStats { pub retransmits: AtomicU64, pub interval_retransmits_total: AtomicU64, pub last_bytes: AtomicU64, + // Per-direction `last_bytes` mirrors for bidir interval deltas. When the + // test is unidirectional, one of these stays at 0 and `last_bytes` is + // sufficient on its own. + pub last_bytes_sent: AtomicU64, + pub last_bytes_received: AtomicU64, pub last_tcp_retransmits: AtomicU64, // UDP stats (updated live by receiver) pub udp_jitter_us: AtomicU64, // Jitter in microseconds (convert to ms when reading) @@ -40,6 +45,10 @@ pub struct IntervalStats { pub lost: u64, pub rtt_us: Option, pub cwnd: Option, + /// Per-direction interval deltas. Zero for unidirectional tests (the + /// single-direction count is already in `bytes`). + pub bytes_sent: u64, + pub bytes_received: u64, } impl StreamStats { @@ -53,6 +62,8 @@ impl StreamStats { retransmits: AtomicU64::new(0), interval_retransmits_total: AtomicU64::new(0), last_bytes: AtomicU64::new(0), + last_bytes_sent: AtomicU64::new(0), + last_bytes_received: AtomicU64::new(0), last_tcp_retransmits: AtomicU64::new(0), udp_jitter_us: AtomicU64::new(0), udp_lost: AtomicU64::new(0), @@ -148,6 +159,14 @@ impl StreamStats { let last_bytes = self.last_bytes.swap(total_bytes, Ordering::Relaxed); let interval_bytes = total_bytes.saturating_sub(last_bytes); + // Per-direction deltas for bidir interval reporting. + let total_sent = self.bytes_sent.load(Ordering::Relaxed); + let total_recv = self.bytes_received.load(Ordering::Relaxed); + let last_sent = self.last_bytes_sent.swap(total_sent, Ordering::Relaxed); + let last_recv = self.last_bytes_received.swap(total_recv, Ordering::Relaxed); + let interval_bytes_sent = total_sent.saturating_sub(last_sent); + let interval_bytes_received = total_recv.saturating_sub(last_recv); + let elapsed = now.duration_since(self.start_time); let intervals = self.intervals.lock(); let interval_duration = if let Some(last) = intervals.back() { @@ -194,6 +213,8 @@ impl StreamStats { lost: interval_lost, rtt_us: tcp_info.as_ref().map(|t| t.rtt_us), cwnd: tcp_info.as_ref().map(|t| t.cwnd), + bytes_sent: interval_bytes_sent, + bytes_received: interval_bytes_received, }; let mut intervals = self.intervals.lock(); @@ -321,11 +342,49 @@ impl TestStats { } pub fn to_aggregate(&self, intervals: &[IntervalStats]) -> AggregateInterval { + self.to_aggregate_with_direction(intervals, false) + } + + /// Build an `AggregateInterval` for the given per-stream interval snapshots. + /// When `is_bidir` is true, also populates the per-direction split fields + /// (swapped from the server's view so the client reads its own direction: + /// `bytes_sent` = bytes the client sent = what the server received, etc.). + pub fn to_aggregate_with_direction( + &self, + intervals: &[IntervalStats], + is_bidir: bool, + ) -> AggregateInterval { let total_bytes: u64 = intervals.iter().map(|i| i.bytes).sum(); let total_throughput: f64 = intervals.iter().map(|i| i.throughput_mbps).sum(); let total_retransmits: u64 = intervals.iter().map(|i| i.retransmits).sum(); let total_lost: u64 = intervals.iter().map(|i| i.lost).sum(); + // Per-direction interval deltas from the reporting side, swapped for + // the client's perspective when this is bidirectional. + let (split_bytes_sent, split_bytes_received, split_ts_send, split_ts_recv) = if is_bidir { + // Server-local `bytes_sent` is what the server sent — from the + // client's perspective that's `bytes_received`. Swap. + let server_sent: u64 = intervals.iter().map(|i| i.bytes_sent).sum(); + let server_recv: u64 = intervals.iter().map(|i| i.bytes_received).sum(); + let client_sent = server_recv; + let client_recv = server_sent; + // Per-direction throughput, using the same interval window the + // aggregate `throughput_mbps` was computed against. If total_bytes + // is nonzero, distribute total_throughput proportionally; otherwise + // both are zero. + let (ts, tr) = if total_bytes > 0 { + ( + total_throughput * (client_sent as f64) / (total_bytes as f64), + total_throughput * (client_recv as f64) / (total_bytes as f64), + ) + } else { + (0.0, 0.0) + }; + (Some(client_sent), Some(client_recv), Some(ts), Some(tr)) + } else { + (None, None, None, None) + }; + // Average jitter across streams that have jitter data let jitter_values: Vec = intervals .iter() @@ -369,6 +428,10 @@ impl TestStats { }, rtt_us: avg_rtt, cwnd: total_cwnd, + bytes_sent: split_bytes_sent, + bytes_received: split_bytes_received, + throughput_send_mbps: split_ts_send, + throughput_recv_mbps: split_ts_recv, } } @@ -376,6 +439,23 @@ impl TestStats { self.streams.iter().map(|s| s.total_bytes()).sum() } + /// Bytes this side has sent, summed across all streams. Used to separate + /// upload vs download accounting in bidirectional tests. + pub fn total_bytes_sent(&self) -> u64 { + self.streams + .iter() + .map(|s| s.bytes_sent.load(Ordering::Relaxed)) + .sum() + } + + /// Bytes this side has received, summed across all streams. + pub fn total_bytes_received(&self) -> u64 { + self.streams + .iter() + .map(|s| s.bytes_received.load(Ordering::Relaxed)) + .sum() + } + pub fn add_tcp_info(&self, info: TcpInfoSnapshot) { self.tcp_info.lock().push(info); } @@ -570,6 +650,20 @@ mod tests { assert_eq!(bytes_to_human(1024 * 1024 * 1024), "1.00 GB"); } + #[test] + fn test_total_bytes_sent_and_received_split() { + let stats = TestStats::new("test".to_string(), 2); + stats.streams[0].add_bytes_sent(100); + stats.streams[0].add_bytes_received(50); + stats.streams[1].add_bytes_sent(200); + stats.streams[1].add_bytes_received(75); + + assert_eq!(stats.total_bytes_sent(), 300); + assert_eq!(stats.total_bytes_received(), 125); + // Existing total_bytes() still includes both. + assert_eq!(stats.total_bytes(), 425); + } + #[test] fn test_mbps_to_human() { assert_eq!(mbps_to_human(500.0), "500.0 Mbps"); diff --git a/src/tui/app.rs b/src/tui/app.rs index 8a6df3f..2ab1c57 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -51,6 +51,13 @@ pub struct App { pub throughput_history: VecDeque, pub streams: Vec, + // Bidirectional split stats (populated from AggregateInterval/TestResult + // when the server reports per-direction totals). Zero for unidirectional tests. + pub bidir_bytes_sent: u64, + pub bidir_bytes_received: u64, + pub throughput_send_mbps: f64, + pub throughput_recv_mbps: f64, + pub total_retransmits: u64, pub rtt_us: u32, pub cwnd: u32, @@ -116,6 +123,10 @@ impl App { total_bytes: 0, current_throughput_mbps: 0.0, throughput_history: VecDeque::with_capacity(SPARKLINE_HISTORY), + bidir_bytes_sent: 0, + bidir_bytes_received: 0, + throughput_send_mbps: 0.0, + throughput_recv_mbps: 0.0, streams: (0..streams) .map(|id| StreamData { id, @@ -254,6 +265,21 @@ impl App { self.total_bytes = progress.total_bytes; self.current_throughput_mbps = progress.throughput_mbps; + // Bidirectional split: when the server reports per-direction counts + // (all four Some), surface them to the UI so the live ↑/↓ panel works + // during the test rather than only after it finishes. + if let (Some(sent), Some(recv), Some(ts), Some(tr)) = ( + progress.bytes_sent, + progress.bytes_received, + progress.throughput_send_mbps, + progress.throughput_recv_mbps, + ) { + self.bidir_bytes_sent += sent; + self.bidir_bytes_received += recv; + self.throughput_send_mbps = ts; + self.throughput_recv_mbps = tr; + } + // Update sparkline history self.throughput_history.push_back(progress.throughput_mbps); if self.throughput_history.len() > SPARKLINE_HISTORY { @@ -364,6 +390,20 @@ impl App { self.udp_packets_sent = udp_stats.packets_sent; self.udp_packets_lost = udp_stats.lost; } + // Bidirectional split stats: populated only by servers that know to + // report them. Older servers leave these None, in which case we keep + // the zero defaults and the UI falls back to the combined throughput. + if let (Some(sent), Some(recv), Some(ts), Some(tr)) = ( + result.bytes_sent, + result.bytes_received, + result.throughput_send_mbps, + result.throughput_recv_mbps, + ) { + self.bidir_bytes_sent = sent; + self.bidir_bytes_received = recv; + self.throughput_send_mbps = ts; + self.throughput_recv_mbps = tr; + } self.result = Some(result); self.log(format!( "Test completed. Avg: {:.0} Mbps.", diff --git a/src/tui/settings.rs b/src/tui/settings.rs index 7b270fe..a1550c6 100644 --- a/src/tui/settings.rs +++ b/src/tui/settings.rs @@ -363,6 +363,7 @@ pub enum SettingsAction { /// Result of TUI loop - either exit or restart with new params #[derive(Debug, Clone)] +#[allow(clippy::large_enum_variant)] // Exit carries the full TestResult; Restart is rare. pub enum TuiLoopResult { Exit { result: Option, diff --git a/src/tui/ui.rs b/src/tui/ui.rs index 97ad1d6..18f35db 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -222,17 +222,36 @@ fn draw_realtime_stats(frame: &mut Frame, app: &App, theme: &Theme, area: Rect) ]) .split(chunks[0]); - // Current throughput value (big number) - let throughput_str = mbps_to_human(app.current_throughput_mbps); - let throughput_display = Paragraph::new(vec![ - Line::from(""), - Line::from(Span::styled( - throughput_str, - Style::default() - .fg(theme.graph_primary) - .add_modifier(Modifier::BOLD), - )), - ]); + // Current throughput value. For bidir tests with per-direction numbers + // available, show "↑ X / ↓ Y" split; otherwise show the combined value. + let throughput_display = if app.direction == crate::protocol::Direction::Bidir + && (app.throughput_send_mbps > 0.0 || app.throughput_recv_mbps > 0.0) + { + Paragraph::new(vec![ + Line::from(Span::styled( + format!("↑ {}", mbps_to_human(app.throughput_send_mbps)), + Style::default() + .fg(theme.graph_primary) + .add_modifier(Modifier::BOLD), + )), + Line::from(Span::styled( + format!("↓ {}", mbps_to_human(app.throughput_recv_mbps)), + Style::default() + .fg(theme.graph_primary) + .add_modifier(Modifier::BOLD), + )), + ]) + } else { + Paragraph::new(vec![ + Line::from(""), + Line::from(Span::styled( + mbps_to_human(app.current_throughput_mbps), + Style::default() + .fg(theme.graph_primary) + .add_modifier(Modifier::BOLD), + )), + ]) + }; frame.render_widget(throughput_display, sparkline_chunks[0]); // Sparkline showing throughput history diff --git a/tests/integration.rs b/tests/integration.rs index 4d0d7fc..bf5c83e 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -71,6 +71,17 @@ async fn test_tcp_single_stream() { // Server reports bytes based on what it tracked, which may be 0 if stats aren't linked // The test passes if we got a valid result structure back assert!(result.duration_ms > 0, "Should have duration"); + + // Unidirectional tests should NOT populate the per-direction split — those + // fields are reserved for bidir where the combined total would be misleading. + assert!( + result.bytes_sent.is_none(), + "unidir result should not populate bytes_sent" + ); + assert!( + result.bytes_received.is_none(), + "unidir result should not populate bytes_received" + ); } #[tokio::test] @@ -213,6 +224,24 @@ async fn test_tcp_bidir() { let result = result.unwrap(); assert!(result.duration_ms > 0, "Should have duration"); + + // Bidir tests should populate the per-direction fields (issue #56). + assert!( + result.bytes_sent.is_some(), + "bidir result should have bytes_sent populated" + ); + assert!( + result.bytes_received.is_some(), + "bidir result should have bytes_received populated" + ); + assert!( + result.throughput_send_mbps.is_some(), + "bidir result should have throughput_send_mbps populated" + ); + assert!( + result.throughput_recv_mbps.is_some(), + "bidir result should have throughput_recv_mbps populated" + ); } #[tokio::test] diff --git a/tests/protocol.rs b/tests/protocol.rs index b2f8d38..232ba1c 100644 --- a/tests/protocol.rs +++ b/tests/protocol.rs @@ -142,6 +142,10 @@ fn test_interval_message() { lost: None, rtt_us: Some(1217), cwnd: Some(98303), + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, }, }; @@ -226,6 +230,10 @@ fn test_result_message() { bytes_acked: None, }), udp_stats: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, }; let msg = ControlMessage::Result(result); @@ -266,6 +274,10 @@ fn test_large_result_message_exceeds_old_8k_guard_but_fits_64k() { bytes_acked: None, }), udp_stats: None, + bytes_sent: None, + bytes_received: None, + throughput_send_mbps: None, + throughput_recv_mbps: None, }; let msg = ControlMessage::Result(result);