diff --git a/CHANGELOG.md b/CHANGELOG.md index d074ff81..ede84835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 interval instead. That retry interval gates only a peer whose last attempt failed, so it cannot clamp a `heartbeat_interval_secs` configured below it. +#### Control socket + +- `show_links` (`fipsctl show links`) now reports the traffic a link has + carried. Its `packets_sent`, `packets_recv`, `bytes_sent`, `bytes_recv` and + `last_recv_ms` were read from counters on the link record that nothing on + the data plane ever wrote, so every link reported zero however much traffic + it carried, while `show_peers` counted the same traffic on the peer. A link + bound to an authenticated peer now reports that peer's counters, so the two + queries agree for the same `link_id`; a link still in handshake has no peer + yet and still reports zero. The counters follow the peer across address + changes, while the row's `transport_id` and `remote_addr` stay those the + link was created with. The counters cover authenticated link frames only, so + they are not expected to match the transport totals in `show_transports`. + The response shape is unchanged. + ### Changed - The lockfile moves `chacha20` from 0.10.1 to 0.10.2, because 0.10.1 is yanked. diff --git a/src/control/queries.rs b/src/control/queries.rs index 83fe847a..5768e480 100644 --- a/src/control/queries.rs +++ b/src/control/queries.rs @@ -567,10 +567,14 @@ pub(crate) fn show_peers_from_handle(handle: &super::read_handle::ControlReadHan /// `show_links` — Active links. pub fn show_links(node: &Node) -> Value { + let counters = node.link_counters(); let links: Vec = node .links() .map(|link| { - let stats = link.stats(); + let stats = counters + .get(&link.link_id()) + .copied() + .unwrap_or_else(|| link.stats()); json!({ "link_id": link.link_id().as_u64(), "transport_id": link.transport_id().as_u32(), @@ -3419,6 +3423,48 @@ mod tests { ); } + /// A link with no authenticated peer yet (one still in handshake) keeps its + /// row in `show_links`, with zero traffic counters, on both the on-loop and + /// the snapshot render. Guards against dropping unbound rows. + #[test] + fn show_links_keeps_a_link_with_no_bound_peer_and_reports_zero_counters() { + use crate::transport::{Link, LinkDirection, LinkId, TransportAddr, TransportId}; + + let mut node = build_test_node(); + let link_id = LinkId::new(7); + node.add_link(Link::connectionless( + link_id, + TransportId::new(1), + TransportAddr::from_string("127.0.0.1:2121"), + LinkDirection::Outbound, + Duration::from_millis(50), + )) + .expect("an empty node has room for a link"); + + let links = show_links(&node); + let rows = links["links"].as_array().expect("links array"); + assert_eq!(rows.len(), 1, "the unbound link must be listed"); + let row = &rows[0]; + assert_eq!(row["link_id"], link_id.as_u64()); + for key in [ + "packets_sent", + "packets_recv", + "bytes_sent", + "bytes_recv", + "last_recv_ms", + ] { + assert_eq!(row["stats"][key], 0, "{key} for a link with no peer"); + } + + node.record_stats_history(); + let handle = node.control_read_handle(); + assert_eq!( + render(show_links(&node)), + render(show_links_from_handle(&handle)), + "off-loop show_links must match on-loop output for an unbound link" + ); + } + // ---- native datagram API coverage ------------------------------------ /// Freshness + fidelity: after a `record_stats_history()` tick (the native diff --git a/src/node/mod.rs b/src/node/mod.rs index ec93cb04..ca4f2877 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -2277,10 +2277,14 @@ impl Node { .collect(); // --- links (show_links) --- + let counters = self.link_counters(); let link_rows: Vec = self .links() .map(|link| { - let stats = link.stats(); + let stats = counters + .get(&link.link_id()) + .copied() + .unwrap_or_else(|| link.stats()); snap::LinkRow { link_id: link.link_id().as_u64(), transport_id: link.transport_id().as_u32(), @@ -2728,6 +2732,18 @@ impl Node { self.links.values() } + /// Traffic counters for each link bound to an active peer, keyed by link. + /// + /// The data plane counts a link's authenticated traffic on the peer that + /// owns it (`ActivePeer::link_stats_mut`), not on the `Link` record, so a + /// link still in handshake has no entry here. + pub(crate) fn link_counters(&self) -> HashMap { + self.peers + .values() + .map(|p| (p.link_id(), p.link_stats())) + .collect() + } + // === Connection Management (Handshake Phase) === /// Whether `link_id` has a pending handshake, read through the control diff --git a/src/node/tests/control.rs b/src/node/tests/control.rs index cb986521..4bdf9fb8 100644 --- a/src/node/tests/control.rs +++ b/src/node/tests/control.rs @@ -311,3 +311,113 @@ async fn test_api_disconnect_unknown_peer_changes_nothing() { assert_eq!(node.peer_machines.len(), machines_before); assert_eq!(node.links.len(), links_before); } + +/// The `show_links` row whose `link_id` is `link_id`. +fn link_row(links: &serde_json::Value, link_id: LinkId) -> &serde_json::Value { + links["links"] + .as_array() + .expect("show_links returns a links array") + .iter() + .find(|row| row["link_id"] == link_id.as_u64()) + .expect("show_links lists the link bound to the peer") +} + +/// Check one node's `show_links` row for the link it shares with `peer_idx` +/// against the counters the data plane kept on that peer, and return the row. +fn assert_link_row_matches_peer( + nodes: &[TestNode], + node_idx: usize, + peer_idx: usize, +) -> serde_json::Value { + let peer_addr = *nodes[peer_idx].node.node_addr(); + let peer = nodes[node_idx] + .node + .get_peer(&peer_addr) + .expect("the tree test establishes the peer"); + let link_id = peer.link_id(); + let expected = peer.link_stats().clone(); + + // Without traffic every counter is zero on both copies and the comparison + // below would pass whether or not show_links reads the right one. + assert!(expected.packets_sent > 0, "node {node_idx} sent no frames"); + assert!( + expected.packets_recv > 0, + "node {node_idx} received no frames" + ); + assert!(expected.bytes_sent > 0, "node {node_idx} sent no bytes"); + assert!(expected.bytes_recv > 0, "node {node_idx} received no bytes"); + assert!( + expected.last_recv_ms > 0, + "node {node_idx} stamped no receive time" + ); + + let links = crate::control::queries::show_links(&nodes[node_idx].node); + let row = link_row(&links, link_id).clone(); + let stats = &row["stats"]; + assert_eq!( + stats["packets_sent"], expected.packets_sent, + "node {node_idx}" + ); + assert_eq!( + stats["packets_recv"], expected.packets_recv, + "node {node_idx}" + ); + assert_eq!(stats["bytes_sent"], expected.bytes_sent, "node {node_idx}"); + assert_eq!(stats["bytes_recv"], expected.bytes_recv, "node {node_idx}"); + assert_eq!( + stats["last_recv_ms"], expected.last_recv_ms, + "node {node_idx}" + ); + row +} + +/// `show_links` reports the traffic a link has carried, not zero: for a link +/// bound to an authenticated peer its counters are the ones the data plane +/// keeps on that peer, on both ends of the link, and the tick-published +/// snapshot render agrees with the on-loop render. +#[tokio::test] +async fn show_links_reports_the_traffic_counters_of_the_peer_bound_to_each_link() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + + let row0 = assert_link_row_matches_peer(&nodes, 0, 1); + + // The off-loop render comes from the snapshot published on the tick. + nodes[0].node.record_stats_history(); + let handle = nodes[0].node.control_read_handle(); + let on_loop = crate::control::queries::show_links(&nodes[0].node); + let off_loop = crate::control::queries::show_links_from_handle(&handle); + assert_eq!( + off_loop, on_loop, + "the snapshot render of show_links must match the on-loop render" + ); + let link_id = nodes[0] + .node + .get_peer(nodes[1].node.node_addr()) + .expect("node 0 still has node 1") + .link_id(); + let off_row = link_row(&off_loop, link_id); + assert!( + off_row["stats"]["packets_recv"].as_u64().unwrap_or(0) > 0, + "the snapshot render must carry the link's receive count, got {off_row}" + ); + + let row1 = assert_link_row_matches_peer(&nodes, 1, 0); + + // A check that does not come from the same node's peer copy: every frame + // node 1 counted as received from node 0 was counted as sent by node 0. + // Loopback is lossless, but a frame sent before node 1 promoted node 0 is + // counted by the sender only, so the bound is not an equality. + let sent0 = row0["stats"]["packets_sent"] + .as_u64() + .expect("packets_sent is a number"); + let recv1 = row1["stats"]["packets_recv"] + .as_u64() + .expect("packets_recv is a number"); + assert!(recv1 > 0, "node 1's link row shows no frames received"); + assert!( + recv1 <= sent0, + "node 1's link row counts {recv1} frames received, more than the {sent0} node 0 sent" + ); + + cleanup_nodes(&mut nodes).await; +}