From 02ed9febab38c4ede7aa2bef71d6f2dd45f222eb Mon Sep 17 00:00:00 2001 From: borky-git Date: Mon, 21 Sep 2026 21:17:54 +0300 Subject: [PATCH 1/2] perf(multiwan): derive the split and the tracker state instead of shelling out ProbeMultiWAN ran two mwan3 shell scripts to learn things it already had the ingredients for. Both walk the whole rule set, and on ipq40xx-class hardware they cost, measured over five runs each: mwan3 interfaces 408 ms mwan3 policies 302 ms uci show mwan3 8 ms That is 710 ms spent every time somebody opens the Internet page, for a tracker verdict that is already one word per file under /var/run/mwan3/iface_state and a split that follows from the member weights. The state reading inside MwanActiveUplink becomes mwanLiveState, so the probe and the resolver share it, and two pure functions derive what the scripts reported: policyOfActiveRule names the policy a rule actually points at, and mwanShares works out the split the way mwan3 does - only the lowest metric group with something online carries traffic, and inside it the weights decide. A policy with nothing derivable falls back to 100% on the active uplink rather than reporting an empty split. Measured end to end on the same hardware, the whole probe goes from 1313 ms to 264 ms. parseMwanInterfaces and parseMwanPolicies are left in place: nothing calls them now, but they are tested parsers of mwan3's own output and removing them is a separate decision. --- internal/modules/multiwan.go | 120 +++++++++++++++++++++++++++++++---- 1 file changed, 107 insertions(+), 13 deletions(-) diff --git a/internal/modules/multiwan.go b/internal/modules/multiwan.go index 5c054a55..4dba1c3a 100644 --- a/internal/modules/multiwan.go +++ b/internal/modules/multiwan.go @@ -640,11 +640,27 @@ func ProbeMultiWAN() *MultiWanProbe { } running := installed && executor.ServiceRunning(mwanPkg) if running { - if out, err := exec.Command(mwanPkg, "interfaces").Output(); err == nil { - live = parseMwanInterfaces(string(out)) - } - if out, err := exec.Command(mwanPkg, "policies").Output(); err == nil { - policy, shares = parseMwanPolicies(string(out)) + // The tracker's verdict and the split come from its state files and + // the member weights, not from `mwan3 interfaces` and `mwan3 + // policies`. Those two shell scripts walk the whole rule set and + // cost the best part of a second between them on this class of + // hardware, which is a lot to spend every time somebody opens the + // page - and they only report what is derived here anyway. + if _, online, ok := mwanLiveState(); ok { + for name := range cfg.Ifaces { + state := "offline" + if online[name] { + state = "online" + } + live[name] = mwanLive{Online: state, Tracking: "active"} + } + if active := pickMwanActive(cfg, online); active != "" { + policy = policyOfActiveRule(cfg) + shares = mwanShares(cfg, online) + if len(shares) == 0 { + shares = map[string]int{active: 100} + } + } } } return buildMultiWanProbe(candidates, installed, installed && executor.ServiceEnabled(mwanPkg), @@ -1141,29 +1157,40 @@ func MwanActiveUplink() string { return mwanActive.name } mwanActive.at = time.Now() - mwanActive.name = "" + cfg, online, ok := mwanLiveState() + if !ok { + mwanActive.name = "" + return "" + } + mwanActive.name = pickMwanActive(cfg, online) + return mwanActive.name +} +// mwanLiveState is the cheap half of the picture: the config, which `uci` +// answers instantly, and the tracker's verdict on each link, which is one +// word per file under its run directory. Everything else mwan3 can tell us +// costs a shell script and the best part of a second — measured, on the +// class of hardware this runs on — so nothing on a polled path uses it. +func mwanLiveState() (mwanConfig, map[string]bool, bool) { if _, err := os.Stat(mwanStateDir); err != nil { - return "" + return mwanConfig{}, nil, false } show, err := exec.Command("uci", "show", mwanPkg).Output() if err != nil { - return "" + return mwanConfig{}, nil, false } - cfg := readMwanConfig(string(show)) - online := map[string]bool{} entries, err := os.ReadDir(mwanStateDir) if err != nil { - return "" + return mwanConfig{}, nil, false } + online := map[string]bool{} for _, e := range entries { b, err := os.ReadFile(mwanStateDir + "/" + e.Name()) if err == nil && strings.TrimSpace(string(b)) == "online" { online[e.Name()] = true } } - mwanActive.name = pickMwanActive(cfg, online) - return mwanActive.name + return readMwanConfig(string(show)), online, true } // pickMwanActive works out which member is carrying traffic from the config @@ -1202,3 +1229,70 @@ func pickMwanActive(cfg mwanConfig, online map[string]bool) string { } return best } + +// policyOfActiveRule names the policy a rule actually points at, which is +// what `mwan3 policies` would print as the one in force. +func policyOfActiveRule(cfg mwanConfig) string { + names := []string{} + for rule, policy := range cfg.RulePolicies { + if len(rule) > mwanMaxSectionLen || len(policy) > mwanMaxSectionLen { + continue + } + if cfg.All[policy] == "policy" { + names = append(names, policy) + } + } + if len(names) == 0 { + return "" + } + sort.Strings(names) + return names[0] +} + +// mwanShares is how traffic divides between the uplinks, worked out the way +// mwan3 does it rather than asked for: only the lowest metric group with +// something online carries anything, and inside it the split follows the +// weights. Deriving it keeps the number fresh on every push, where asking +// `mwan3 policies` would cost a third of a second each time. +func mwanShares(cfg mwanConfig, online map[string]bool) map[string]int { + type member struct { + iface string + weight int + } + lowest, group := 0, []member{} + for _, m := range cfg.Members { + if m.Interface == "" || !online[m.Interface] { + continue + } + switch { + case len(group) == 0 || m.Metric < lowest: + lowest, group = m.Metric, []member{{m.Interface, m.Weight}} + case m.Metric == lowest: + group = append(group, member{m.Interface, m.Weight}) + } + } + shares := map[string]int{} + total := 0 + for _, m := range group { + total += m.weight + } + if total == 0 { + return shares + } + // Heaviest first, so the rounding remainder lands there rather than + // leaving the column short of 100. + sort.Slice(group, func(i, j int) bool { + if group[i].weight != group[j].weight { + return group[i].weight > group[j].weight + } + return group[i].iface < group[j].iface + }) + assigned := 0 + for _, m := range group[1:] { + pct := m.weight * 100 / total + shares[m.iface] = pct + assigned += pct + } + shares[group[0].iface] = 100 - assigned + return shares +} From fbf335d93399cff096132c97b459a140de0ee7af Mon Sep 17 00:00:00 2001 From: borky-git Date: Sat, 26 Sep 2026 21:23:52 +0300 Subject: [PATCH 2/2] fix(multiwan): report an untracked uplink as unknown, not failed Reading the tracker's state files instead of running `mwan3 interfaces` turned every interface without a file into "offline", with tracking "active". An uplink mwan3 is not tracking - enabled '0' in its config, or not started - has no state file, and the script reports it as "unknown and tracking is down"; the UI shows that as standby, but "offline" as a failed link, so a link nobody monitors turned red. The probe now reports what the script would: an interface that is disabled in the mwan3 config or has no state file is unknown and not tracked, and a tracked one gets the state the tracker recorded (online, offline, connecting, ...) with tracking active. Tracking is therefore real again, derived from whether mwan3 tracks the link. policyOfActiveRule also says in its comment what it returns with more than one policy in use: the alphabetically first one a rule points at, which is exact for the single-policy configurations NetGrip writes. --- internal/modules/multiwan.go | 64 ++++++++++++++++++++++++------- internal/modules/multiwan_test.go | 33 ++++++++++++++++ 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/internal/modules/multiwan.go b/internal/modules/multiwan.go index 4dba1c3a..ebe584a7 100644 --- a/internal/modules/multiwan.go +++ b/internal/modules/multiwan.go @@ -646,14 +646,11 @@ func ProbeMultiWAN() *MultiWanProbe { // cost the best part of a second between them on this class of // hardware, which is a lot to spend every time somebody opens the // page - and they only report what is derived here anyway. - if _, online, ok := mwanLiveState(); ok { - for name := range cfg.Ifaces { - state := "offline" - if online[name] { - state = "online" - } - live[name] = mwanLive{Online: state, Tracking: "active"} + if _, states, ok := mwanLiveState(); ok { + for name, iface := range cfg.Ifaces { + live[name] = mwanLiveOf(name, iface, states) } + online := mwanOnline(states) if active := pickMwanActive(cfg, online); active != "" { policy = policyOfActiveRule(cfg) shares = mwanShares(cfg, online) @@ -1157,12 +1154,12 @@ func MwanActiveUplink() string { return mwanActive.name } mwanActive.at = time.Now() - cfg, online, ok := mwanLiveState() + cfg, states, ok := mwanLiveState() if !ok { mwanActive.name = "" return "" } - mwanActive.name = pickMwanActive(cfg, online) + mwanActive.name = pickMwanActive(cfg, mwanOnline(states)) return mwanActive.name } @@ -1171,7 +1168,11 @@ func MwanActiveUplink() string { // word per file under its run directory. Everything else mwan3 can tell us // costs a shell script and the best part of a second — measured, on the // class of hardware this runs on — so nothing on a polled path uses it. -func mwanLiveState() (mwanConfig, map[string]bool, bool) { +// +// The states map holds each state file's word as mwan3 wrote it (online, +// offline, connecting, ...). An interface mwan3 is not tracking - disabled in +// its config, or not started - has no file and no entry. +func mwanLiveState() (mwanConfig, map[string]string, bool) { if _, err := os.Stat(mwanStateDir); err != nil { return mwanConfig{}, nil, false } @@ -1183,14 +1184,42 @@ func mwanLiveState() (mwanConfig, map[string]bool, bool) { if err != nil { return mwanConfig{}, nil, false } - online := map[string]bool{} + states := map[string]string{} for _, e := range entries { b, err := os.ReadFile(mwanStateDir + "/" + e.Name()) - if err == nil && strings.TrimSpace(string(b)) == "online" { - online[e.Name()] = true + if err != nil { + continue + } + if st := strings.TrimSpace(string(b)); st != "" { + states[e.Name()] = st } } - return readMwanConfig(string(show)), online, true + return readMwanConfig(string(show)), states, true +} + +// mwanOnline is the set of interfaces the tracker has online. +func mwanOnline(states map[string]string) map[string]bool { + online := map[string]bool{} + for name, st := range states { + if st == "online" { + online[name] = true + } + } + return online +} + +// mwanLiveOf is what `mwan3 interfaces` would say about name, from its state +// file alone. An interface mwan3 does not track - disabled in the mwan3 +// config, or with no state file - is "unknown" and its tracking "down", as +// the script prints it: reading it as offline would paint a link nobody is +// monitoring as failed. One that is tracked reports the state the tracker +// recorded. +func mwanLiveOf(name string, iface mwanIface, states map[string]string) mwanLive { + st, tracked := states[name] + if !iface.Enabled || !tracked { + return mwanLive{Online: "unknown", Tracking: "down"} + } + return mwanLive{Online: st, Tracking: "active"} } // pickMwanActive works out which member is carrying traffic from the config @@ -1232,6 +1261,13 @@ func pickMwanActive(cfg mwanConfig, online map[string]bool) string { // policyOfActiveRule names the policy a rule actually points at, which is // what `mwan3 policies` would print as the one in force. +// +// mwan3 policies are per rule, so "the active policy" is a simplification: +// with several rules pointing at different policies this returns the +// alphabetically first one any rule uses. That is exact for the +// configurations NetGrip writes, which steer everything through a single +// policy; a hand-made multi-policy setup gets one of its policies, not the +// whole picture. func policyOfActiveRule(cfg mwanConfig) string { names := []string{} for rule, policy := range cfg.RulePolicies { diff --git a/internal/modules/multiwan_test.go b/internal/modules/multiwan_test.go index b573c174..958ed650 100644 --- a/internal/modules/multiwan_test.go +++ b/internal/modules/multiwan_test.go @@ -941,3 +941,36 @@ mwan3.` + mwanPolicyName + `=policy t.Fatal("a rule that does not pin must not be reported as sticky") } } + +// What each link reports without running `mwan3 interfaces`: the same words +// the script prints. A link mwan3 is not tracking - disabled in its config, +// or with no state file - is unknown and not tracked, never offline: the UI +// paints "offline" as a failed link. +func TestMwanLiveOfReportsWhatTheTrackerKnows(t *testing.T) { + states := map[string]string{"rds": "online", "lte": "offline", "wg": "connecting"} + for _, tc := range []struct { + name string + iface mwanIface + want mwanLive + }{ + {"rds", mwanIface{Enabled: true}, mwanLive{Online: "online", Tracking: "active"}}, + {"lte", mwanIface{Enabled: true}, mwanLive{Online: "offline", Tracking: "active"}}, + {"wg", mwanIface{Enabled: true}, mwanLive{Online: "connecting", Tracking: "active"}}, + // Present in the config and in network, but enabled '0' in mwan3, + // so no state file: `mwan3 interfaces` says "unknown and tracking is + // down". + {"wan", mwanIface{Enabled: false}, mwanLive{Online: "unknown", Tracking: "down"}}, + // Enabled, but the tracker has not written a file for it (yet). + {"spare", mwanIface{Enabled: true}, mwanLive{Online: "unknown", Tracking: "down"}}, + // Disabled in the config wins over a stale file left behind. + {"rds", mwanIface{Enabled: false}, mwanLive{Online: "unknown", Tracking: "down"}}, + } { + if got := mwanLiveOf(tc.name, tc.iface, states); got != tc.want { + t.Errorf("%s (enabled=%v): %+v, want %+v", tc.name, tc.iface.Enabled, got, tc.want) + } + } + online := mwanOnline(states) + if len(online) != 1 || !online["rds"] { + t.Errorf("online set: %v, want only rds", online) + } +}