sessiond: a working bearer action is a decision, not an error
Both executors recorded success via record_error, so the daemon's own working
actions landed in the trail's error channel - visible on the phone as
{"event":{"error":{"component":"bearer","error":"preferred wifi"}}}. That makes
the one signal a reader scans for useless.
They also discarded every exit status, so a failed nmcli was indistinguishable
from a successful one and the trail would claim a metric that was never set.
Both now check status: record_decision on success, record_error with what
actually failed otherwise. The pin failing is expected without CAP_NET_ADMIN
and says so - the tunnel still works, it just follows the default route
instead of the chosen link, and those are different states.
This commit is contained in:
parent
7e91de1956
commit
d2f51e93de
1 changed files with 61 additions and 18 deletions
|
|
@ -420,24 +420,52 @@ fn apply_link_preference(shared: &Arc<Shared>, 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<String> = 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<Shared>, 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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue