From 288e856fdbf9eba4c4565bcfe694598ebbc278ca Mon Sep 17 00:00:00 2001 From: MengMengCode <227010654+MengMengCode@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:30:22 +0800 Subject: [PATCH] Stabilize OpenWrt modem startup and upgrades --- cmd/vocat/instance_lock_linux.go | 27 ++++++++--- cmd/vocat/instance_lock_linux_test.go | 9 ++-- cmd/vocat/main.go | 66 +++++++++++++++++++++++++-- cmd/vocat/vowifi_startup_test.go | 56 +++++++++++++++++++++++ scripts/install.sh | 25 ++++++++-- 5 files changed, 162 insertions(+), 21 deletions(-) create mode 100644 cmd/vocat/vowifi_startup_test.go diff --git a/cmd/vocat/instance_lock_linux.go b/cmd/vocat/instance_lock_linux.go index 7a6ee43..56a6bc4 100644 --- a/cmd/vocat/instance_lock_linux.go +++ b/cmd/vocat/instance_lock_linux.go @@ -12,19 +12,32 @@ import ( ) func lockServerInstance(databasePath string) (*os.File, error) { - directory := filepath.Dir(databasePath) - if err := os.MkdirAll(directory, 0o755); err != nil { - return nil, fmt.Errorf("create data directory for instance lock: %w", err) + // The modem, PC/SC reader, XFRM policies and listener are host resources, + // not database resources. Lock per OS user so a diagnostic instance using a + // different VOCAT_DATABASE_PATH cannot silently steal the same AT port from + // the managed service. Prefer /run because systemd's PrivateTmp would + // otherwise hide the managed service's lock from a manually started process. + // The UID-specific directory still permits intentionally isolated users to + // operate independently; development hosts without writable /run fall back + // to TempDir. + uid := os.Geteuid() + directory := filepath.Join("/run", fmt.Sprintf("vocat-%d", uid)) + if uid == 0 { + directory = "/run/vocat" } - path := filepath.Join(directory, ".vocat.lock") - file, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) + if err := os.MkdirAll(directory, 0o700); err != nil { + directory = os.TempDir() + } + path := filepath.Join(directory, "vocat-server.lock") + fd, err := unix.Open(path, unix.O_CREAT|unix.O_RDWR|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0o600) if err != nil { return nil, fmt.Errorf("open server instance lock: %w", err) } - if err := unix.Flock(int(file.Fd()), unix.LOCK_EX|unix.LOCK_NB); err != nil { + file := os.NewFile(uintptr(fd), path) + if err := unix.Flock(fd, unix.LOCK_EX|unix.LOCK_NB); err != nil { _ = file.Close() if errors.Is(err, unix.EWOULDBLOCK) || errors.Is(err, unix.EAGAIN) { - return nil, fmt.Errorf("another vocat server is already using database %s", databasePath) + return nil, errors.New("another vocat server already controls this host's modem resources") } return nil, fmt.Errorf("lock server instance: %w", err) } diff --git a/cmd/vocat/instance_lock_linux_test.go b/cmd/vocat/instance_lock_linux_test.go index ab09079..c9cd245 100644 --- a/cmd/vocat/instance_lock_linux_test.go +++ b/cmd/vocat/instance_lock_linux_test.go @@ -9,17 +9,18 @@ import ( ) func TestServerInstanceLockRejectsSecondProcess(t *testing.T) { - database := filepath.Join(t.TempDir(), "vocat.db") - first, err := lockServerInstance(database) + firstDatabase := filepath.Join(t.TempDir(), "vocat.db") + first, err := lockServerInstance(firstDatabase) if err != nil { t.Fatal(err) } defer first.Close() - second, err := lockServerInstance(database) + secondDatabase := filepath.Join(t.TempDir(), "other.db") + second, err := lockServerInstance(secondDatabase) if second != nil { second.Close() } - if err == nil || !strings.Contains(err.Error(), "already using database") { + if err == nil || !strings.Contains(err.Error(), "already controls this host") { t.Fatalf("second lock error = %v", err) } } diff --git a/cmd/vocat/main.go b/cmd/vocat/main.go index b59569d..adf94d3 100644 --- a/cmd/vocat/main.go +++ b/cmd/vocat/main.go @@ -622,12 +622,18 @@ func configureVoWiFiRuntime( } if deviceConfig.VoWiFiEnabled { if entry, mapErr := mapper.Get(deviceConfig.ID); mapErr == nil { - flightContext, cancelFlight := context.WithTimeout(ctx, 10*time.Second) - _, flightErr := deviceManager.SetFlight(flightContext, entry.ID, true) - cancelFlight() + flightErr := protectVoWiFiStartupRadio(ctx, deviceManager, entry.ID) if flightErr != nil { - _ = manager.Close(context.Background()) - return nil, fmt.Errorf("protect device %q before VoWiFi startup: %w", deviceConfig.ID, flightErr) + // A modem can be temporarily unavailable while OpenWrt/procd is + // restarting the service (notably after loading XFRM modules). Do + // not take the Web/API service down with it: the orchestrator below + // remains fail-closed and its runtime manager retries until CFUN=4 + // can be established. + logger.Warn( + "VoWiFi startup radio protection deferred to automatic retry", + "device_id", deviceConfig.ID, + "error", flightErr, + ) } } if _, err := manager.RequestEnabled(deviceConfig.ID, true); err != nil { @@ -639,6 +645,56 @@ func configureVoWiFiRuntime( return manager, nil } +const ( + vowifiStartupRadioAttempts = 3 + vowifiStartupRadioDelay = time.Second +) + +type flightModeSetter interface { + SetFlight(context.Context, string, bool) (device.FlightResult, error) +} + +func protectVoWiFiStartupRadio(ctx context.Context, manager flightModeSetter, physicalID string) error { + return protectVoWiFiStartupRadioWithRetry( + ctx, + manager, + physicalID, + vowifiStartupRadioAttempts, + vowifiStartupRadioDelay, + ) +} + +func protectVoWiFiStartupRadioWithRetry( + ctx context.Context, + manager flightModeSetter, + physicalID string, + attempts int, + delay time.Duration, +) error { + var lastErr error + for attempt := 0; attempt < attempts; attempt++ { + flightContext, cancel := context.WithTimeout(ctx, 10*time.Second) + _, lastErr = manager.SetFlight(flightContext, physicalID, true) + cancel() + if lastErr == nil { + return nil + } + if attempt+1 == attempts { + break + } + timer := time.NewTimer(delay) + select { + case <-ctx.Done(): + if !timer.Stop() { + <-timer.C + } + return errors.Join(lastErr, ctx.Err()) + case <-timer.C: + } + } + return lastErr +} + type vowifiDeviceAdapter interface { vowifi.SIMIdentityReader vowifi.AKAProvider diff --git a/cmd/vocat/vowifi_startup_test.go b/cmd/vocat/vowifi_startup_test.go new file mode 100644 index 0000000..5346c76 --- /dev/null +++ b/cmd/vocat/vowifi_startup_test.go @@ -0,0 +1,56 @@ +package main + +import ( + "context" + "errors" + "testing" + + "vocat/internal/device" +) + +type startupFlightSetter struct { + errors []error + calls int + id string +} + +func (setter *startupFlightSetter) SetFlight( + _ context.Context, + id string, + enabled bool, +) (device.FlightResult, error) { + setter.calls++ + setter.id = id + if !enabled { + return device.FlightResult{}, errors.New("expected flight mode to be enabled") + } + if setter.calls <= len(setter.errors) { + return device.FlightResult{}, setter.errors[setter.calls-1] + } + return device.FlightResult{CurrentMode: 4, FlightMode: true, RadioOff: true}, nil +} + +func TestProtectVoWiFiStartupRadioRetriesTransientFailure(t *testing.T) { + transient := errors.New("modem is reopening") + setter := &startupFlightSetter{errors: []error{transient, transient}} + if err := protectVoWiFiStartupRadioWithRetry( + context.Background(), setter, "quectel-1", 3, 0, + ); err != nil { + t.Fatalf("protect startup radio: %v", err) + } + if setter.calls != 3 || setter.id != "quectel-1" { + t.Fatalf("SetFlight calls = %d, id = %q", setter.calls, setter.id) + } +} + +func TestProtectVoWiFiStartupRadioReturnsLastFailure(t *testing.T) { + first := errors.New("first") + last := errors.New("last") + setter := &startupFlightSetter{errors: []error{first, last}} + err := protectVoWiFiStartupRadioWithRetry( + context.Background(), setter, "quectel-1", 2, 0, + ) + if !errors.Is(err, last) || setter.calls != 2 { + t.Fatalf("protect startup radio = %v after %d calls", err, setter.calls) + } +} diff --git a/scripts/install.sh b/scripts/install.sh index 75bc277..5eba151 100644 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -337,6 +337,8 @@ TimeoutStartSec=30s # HTTP, VoWiFi, and modem cleanup have bounded shutdown contexts totalling up # to 30 seconds. Leave a small margin before systemd resorts to SIGKILL. TimeoutStopSec=40s +RuntimeDirectory=vocat +RuntimeDirectoryMode=0755 AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW @@ -409,11 +411,24 @@ enable_and_start() { if [ -x "$OPENWRT_INIT_PATH" ] && { [ -x /sbin/procd ] || [ -x /sbin/ubusd ]; }; then "$OPENWRT_INIT_PATH" enable if "$OPENWRT_INIT_PATH" restart; then - sleep 2 - if "$OPENWRT_INIT_PATH" running; then - rm -f "${BINARY_PATH}.bak" - return - fi + # Modems may need several seconds to release and reopen their AT + # port after procd stops the previous process. Require consecutive + # healthy observations so a short-lived respawn is not mistaken for + # a successful upgrade. + local attempt stable + stable=0 + for attempt in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30; do + sleep 1 + if "$OPENWRT_INIT_PATH" running; then + stable=$((stable + 1)) + if [ "$stable" -ge 3 ]; then + rm -f "${BINARY_PATH}.bak" + return + fi + else + stable=0 + fi + done fi if [ -e "${BINARY_PATH}.bak" ]; then cp -a "${BINARY_PATH}.bak" "$BINARY_PATH"