mirror of
https://github.com/jmcorgan/fips.git
synced 2026-09-14 00:45:08 +00:00
Merge master into next, carrying the show links counter fix
show links and its control-socket snapshot now report the traffic counters of the peer bound to each link, instead of counters on the link record that nothing ever wrote, so the query agrees with show peers for the same link_id. The code applies unchanged. The changelog conflicted only on position: the entry is filed under next's Unreleased Fixed section, ahead of its Packaging subsection, without master's Changed heading that followed it.
This commit is contained in:
@@ -712,6 +712,21 @@ with v0.5.x or earlier peers.
|
||||
platforms with the connected-socket fast path); elsewhere the heartbeat alone
|
||||
carries the new address.
|
||||
|
||||
#### 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.
|
||||
|
||||
#### Packaging
|
||||
|
||||
- The Linux `.deb` and the systemd tarball now install and run on Debian 12 and
|
||||
|
||||
+47
-1
@@ -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<Value> = 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(),
|
||||
@@ -3520,6 +3524,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
|
||||
|
||||
+17
-1
@@ -2463,10 +2463,14 @@ impl Node {
|
||||
.collect();
|
||||
|
||||
// --- links (show_links) ---
|
||||
let counters = self.link_counters();
|
||||
let link_rows: Vec<snap::LinkRow> = 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(),
|
||||
@@ -2988,6 +2992,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<LinkId, &crate::transport::LinkStats> {
|
||||
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
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user