diff --git a/src/sessiond/server.rs b/src/sessiond/server.rs index 31ff859..2627569 100644 --- a/src/sessiond/server.rs +++ b/src/sessiond/server.rs @@ -420,24 +420,52 @@ fn apply_link_preference(shared: &Arc, bearer: crate::sessiond::bearer:: Bearer::Wifi => METRIC_PREFERRED, Bearer::Cellular => METRIC_DEMOTED, }; - for conn in active_connections_of_type("802-11-wireless") { - let _ = std::process::Command::new("nmcli") - .args(["connection", "modify", &conn, "ipv4.route-metric", metric]) - .status(); + let conns = active_connections_of_type("802-11-wireless"); + let mut failed: Vec = Vec::new(); + for conn in &conns { + let ok = std::process::Command::new("nmcli") + .args(["connection", "modify", conn, "ipv4.route-metric", metric]) + .status() + .map(|s| s.success()) + .unwrap_or(false); + if !ok { + failed.push(conn.clone()); + } } // Reapply rather than up/down. Cycling the connection is what made the old // gate's corrections into NM events that re-entered it; `reapply` changes // the live config without a state transition, so nothing observing NM sees // a link flap. - let _ = std::process::Command::new("nmcli") + let reapplied = std::process::Command::new("nmcli") .args(["device", "reapply", "wlan0"]) - .status(); + .status() + .map(|s| s.success()) + .unwrap_or(false); - shared.lock().device_state.record_error( - "bearer", - "prefer-link", - &format!("preferred {} (wifi route-metric {metric})", bearer.as_str()), - ); + let mut d = shared.lock(); + if failed.is_empty() && reapplied { + // A success is a decision, not an error. Recording it as an error put + // the daemon's own working actions in the trail's error channel, which + // makes the one signal a reader scans for useless. + d.device_state.record_decision( + "bearer-applied", + serde_json::json!({ + "bearer": bearer.as_str(), + "wifi_route_metric": metric, + "connections": conns, + }), + "wifi route metric set and reapplied", + ); + } else { + d.device_state.record_error( + "bearer", + "prefer-link", + &format!( + "nmcli failed (modify: {failed:?}, reapply wlan0: {reapplied}) — \ + the metric may not match the decision" + ), + ); + } } /// List active connection names of a given `connection.type`. @@ -475,14 +503,29 @@ fn pin_tunnel_underlay(shared: &Arc, bearer: crate::sessiond::bearer::Be // `replace` rather than `add`: this runs on every settled change, and an // `add` that collides leaves the old pin in place, which is the stale // route the whole exercise is trying not to inherit. - let _ = std::process::Command::new("ip") + let ok = std::process::Command::new("ip") .args(["route", "replace", &format!("{endpoint}/32"), "dev", dev]) - .status(); - shared.lock().device_state.record_error( - "bearer", - "pin-tunnel", - &format!("pinned {endpoint} via {dev}"), - ); + .status() + .map(|s| s.success()) + .unwrap_or(false); + let mut d = shared.lock(); + if ok { + d.device_state.record_decision( + "bearer-tunnel-pinned", + serde_json::json!({ "endpoint": endpoint, "dev": dev }), + "tunnel endpoint pinned to the chosen underlay", + ); + } else { + // Expected without CAP_NET_ADMIN. Worth an error rather than silence: + // the tunnel still works over the default route, but it is no longer + // pinned to the link the machine chose, and a reader should know the + // difference between "pinned" and "left to the default". + d.device_state.record_error( + "bearer", + "pin-tunnel", + &format!("could not pin {endpoint} via {dev}; tunnel follows the default route"), + ); + } } /// The WireGuard peer's endpoint address, as an IP.