Skip to content

Commit a394b27

Browse files
committed
Fix SFE/conntrack race and HTB quantum warnings on IPQ9574 (#506)
* Flush SFE before conntrack deletes to fix double-free race on IPQ9574 When wansteer deletes conntrack entries (failover, config reload, reconcile, shutdown), SFE may still hold offloaded fast-path references. SFE then tries to clean up a stale path and hits a double-free, causing "sfe_ipv4_remove_connection: Connection has been removed already" kernel errors. Fix: flush SFE cache (IPv4 + IPv6) before every conntrack deletion. The order SFE delete -> conntrack delete prevents the race. flushSFE() writes to /sys/sfe_ipv4/flush and /sys/sfe_ipv6/flush, silently skipping on platforms without SFE. Covered paths: - flushConntrackForMark (health failover, config reload, shutdown) - Reconcile cycle (drift-detected rule rebuild) * Replace misleading SFE sysfs test with idempotency test TestFlushSFE_WritesToSysfs didn't actually call flushSFE() - it just tested os.WriteFile on temp files. Replace with an idempotency test that verifies multiple flushSFE calls are safe (relevant for flushAllSteeredConntrack which calls it per-WAN). * Change HTB minimum class rate from 64bit to 100kbit in Adaptive SQM rate 64bit causes the kernel to log "HTB: quantum of class XXXXX is small" warnings every time qdiscs are reapplied (every 5 minutes), generating ~21K warnings per 3 hours in kern.log. This adds unnecessary eMMC write pressure on the UniFi Cloud Gateway Fiber. 100kbit is still effectively zero guaranteed rate (classes borrow via ceil), but produces a quantum of 1250 bytes with r2q 10, well above the warning threshold. Also update the filter to match both 64bit (stock UniFi) and 100Kbit (after our update) as best-effort class markers.
1 parent 156f740 commit a394b27

4 files changed

Lines changed: 54 additions & 4 deletions

File tree

src/NetworkOptimizer.Sqm/ScriptGenerator.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -568,8 +568,9 @@ local mem
568568
prio=$(echo ""$line"" | grep -o ""prio [0-9]*"" | awk '{print $2}')
569569
rate=$(echo ""$line"" | grep -o ""rate [0-9]*[a-zA-Z]*"" | awk '{print $2}')
570570
571-
# Skip classes with rate > 64bit (UniFi-configured classes)
572-
if [ ""$rate"" != ""64bit"" ]; then
571+
# Skip classes with a real guaranteed rate (UniFi-configured classes).
572+
# Match 64bit (stock UniFi) and 100Kbit (after our update) as best-effort markers.
573+
if [ ""$rate"" != ""64bit"" ] && [ ""$rate"" != ""100Kbit"" ]; then
573574
continue
574575
fi
575576
@@ -585,9 +586,9 @@ tc qdisc change dev $device parent $classid handle $leaf_qdisc fq_codel limit $f
585586
fi
586587
587588
if [ -n ""$prio"" ]; then
588-
tc class change dev $device parent 1:1 classid $classid htb rate 64bit ceil ${new_rate}Mbit burst ${burst}b cburst ${burst}b prio $prio
589+
tc class change dev $device parent 1:1 classid $classid htb rate 100kbit ceil ${new_rate}Mbit burst ${burst}b cburst ${burst}b prio $prio
589590
else
590-
tc class change dev $device parent 1:1 classid $classid htb rate 64bit ceil ${new_rate}Mbit burst ${burst}b cburst ${burst}b
591+
tc class change dev $device parent 1:1 classid $classid htb rate 100kbit ceil ${new_rate}Mbit burst ${burst}b cburst ${burst}b
591592
fi
592593
fi
593594
done

src/wansteer/main.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ func main() {
132132
default:
133133
slog.Info("shutdown signal received", "signal", sig)
134134
removeRules()
135+
// SFE flush happens inside flushAllSteeredConntrack via flushConntrackForMark
135136
flushAllSteeredConntrack(cfg)
136137
// Write final status
137138
status := buildStatus(cfg, startedAt, lastReconcile, reconcileCount, health)
@@ -152,6 +153,10 @@ func main() {
152153
"actual_rules", actual,
153154
"jump_present", jumpOk,
154155
)
156+
// Flush SFE before rule changes: when rules are rebuilt,
157+
// connections may shift WANs and SFE's offloaded paths
158+
// become stale. Flushing prevents the double-free race.
159+
flushSFE()
155160
unhealthy := health.unhealthyWANs()
156161
if err := reapplyRules(cfg, unhealthy); err != nil {
157162
slog.Error("reconciliation failed", "error", err)

src/wansteer/rules.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"log/slog"
66
"math"
7+
"os"
78
"os/exec"
89
"strings"
910
)
@@ -329,10 +330,34 @@ func activeTargetWANs(cfg *Config) map[string]bool {
329330
return targets
330331
}
331332

333+
// flushSFE flushes the Shortcut Forwarding Engine cache for both IPv4 and IPv6.
334+
// On Qualcomm IPQ9574 (UniFi Cloud Gateway Fiber), SFE offloads connections into
335+
// a kernel fast path. If conntrack deletes an entry before SFE releases it, SFE
336+
// hits a double-free race that causes "sfe_ipv4_remove_connection: Connection has
337+
// been removed already" kernel errors. Always call this BEFORE deleting conntrack
338+
// entries.
339+
func flushSFE() {
340+
for _, path := range []string{"/sys/sfe_ipv4/flush", "/sys/sfe_ipv6/flush"} {
341+
if err := os.WriteFile(path, []byte("1"), 0644); err != nil {
342+
// Not an error: SFE may not be present on all platforms
343+
slog.Debug("sfe flush skipped", "path", path, "error", err)
344+
} else {
345+
slog.Debug("sfe cache flushed", "path", path)
346+
}
347+
}
348+
}
349+
332350
// flushConntrackForMark deletes all conntrack entries with the given WAN fwmark.
333351
// This forces existing connections to be re-routed through the default WAN
334352
// when their assigned WAN goes down.
353+
// Order is critical: SFE flush must happen before conntrack delete to avoid
354+
// the SFE double-free race on IPQ9574.
335355
func flushConntrackForMark(fwmark string) {
356+
// Flush SFE first: SFE must release offloaded connections before conntrack
357+
// deletes the tracking entry, otherwise SFE tries to clean up a path whose
358+
// conntrack entry is already gone.
359+
flushSFE()
360+
336361
// conntrack -D -m <mark> deletes entries matching the mark
337362
// The mark includes the WAN bits in 0x7e0000, so we match on those
338363
err := run("conntrack", "-D", "-m", fwmark)

src/wansteer/wansteer_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,3 +639,22 @@ func TestBuildStatus_PopulatesFields(t *testing.T) {
639639
t.Errorf("expected 2 WAN health entries, got %d", len(status.WANHealth))
640640
}
641641
}
642+
643+
// ---------------------------------------------------------------------------
644+
// SFE flush
645+
// ---------------------------------------------------------------------------
646+
647+
func TestFlushSFE_NoSysfs(t *testing.T) {
648+
// flushSFE should not panic or error when /sys/sfe_ipv4/flush doesn't exist.
649+
// On non-gateway hosts (dev machines, CI), the sysfs paths won't exist and
650+
// the function should silently skip them.
651+
flushSFE() // must not panic
652+
}
653+
654+
func TestFlushSFE_Idempotent(t *testing.T) {
655+
// Calling flushSFE multiple times should be safe (e.g. when
656+
// flushAllSteeredConntrack calls flushConntrackForMark per-WAN).
657+
flushSFE()
658+
flushSFE()
659+
// No panic, no error - global SFE flush is idempotent
660+
}

0 commit comments

Comments
 (0)