Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APP-7257] Support older NM (>1.30.0) and hardware that cannot scan in hotspot mode #55

Merged
merged 4 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ fetch_config() {
fi
}

# Verifies that NetworkManager is 1.42 or newer.
# Verifies that NetworkManager is 1.30 or newer.
check_nm_version() {
which nmcli >/dev/null 2>&1 || return 1

Expand All @@ -77,7 +77,7 @@ check_nm_version() {
NM_VERSION_MAJOR=$(echo $NM_VERSION | cut -d. -f1)
NM_VERSION_MINOR=$(echo $NM_VERSION | cut -d. -f2)

if [ $NM_VERSION_MAJOR -ge 1 ] && [ $NM_VERSION_MINOR -ge 42 ]; then
if [ $NM_VERSION_MAJOR -ge 1 ] && [ $NM_VERSION_MINOR -ge 30 ]; then
return 0
fi

Expand Down Expand Up @@ -139,7 +139,7 @@ enable_networkmanager() {
systemctl is-enabled NetworkManager && check_nm_version && return

echo
echo "Viam provides a wifi management and device provisioning service. To use it, NetworkManager 1.42 (or newer) must be installed and active."
echo "Viam provides a wifi management and device provisioning service. To use it, NetworkManager 1.30 (or newer) must be installed and active."

if check_nm_version || is_bullseye; then
# We can automate this.
Expand Down Expand Up @@ -177,7 +177,7 @@ enable_networkmanager() {
fi

if systemctl cat NetworkManager >/dev/null; then
systemctl enable --now NetworkManager || (echo "Failed to active NetworkManager" && return 1)
systemctl enable --now NetworkManager || (echo "Failed to activate NetworkManager" && return 1)
systemctl disable dhcpcd
else
return 1
Expand Down
34 changes: 19 additions & 15 deletions subsystems/provisioning/definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,18 @@ const (

var (
DefaultConf = Config{
Manufacturer: "viam",
Model: "custom",
FragmentID: "",
HotspotPrefix: "viam-setup",
HotspotPassword: "viamsetup",
DisableDNSRedirect: false,
RoamingMode: false,
OfflineTimeout: Timeout(time.Minute * 2),
UserTimeout: Timeout(time.Minute * 5),
FallbackTimeout: Timeout(time.Minute * 10),
Networks: []NetworkConfig{},
Manufacturer: "viam",
Model: "custom",
FragmentID: "",
HotspotPrefix: "viam-setup",
HotspotPassword: "viamsetup",
DisableDNSRedirect: false,
RoamingMode: false,
OfflineTimeout: Timeout(time.Minute * 2),
UserTimeout: Timeout(time.Minute * 5),
FallbackTimeout: Timeout(time.Minute * 10),
DeviceRebootAfterOfflineMinutes: Timeout(0),
Networks: []NetworkConfig{},
}

// Can be overwritten via cli arguments.
Expand All @@ -63,6 +64,7 @@ var (
ErrConnCheckDisabled = errors.New("NetworkManager connectivity checking disabled by user, network management will be unavailable")
ErrNoActiveConnectionFound = errors.New("no active connection found")
scanLoopDelay = time.Second * 15
scanTimeout = time.Second * 30
connectTimeout = time.Second * 50 // longer than the 45 second timeout in NetworkManager
)

Expand Down Expand Up @@ -298,9 +300,11 @@ func ConfigFromJSON(defaultConf Config, jsonBytes []byte) (*Config, error) {
}

if conf.DeviceRebootAfterOfflineMinutes != 0 &&
conf.DeviceRebootAfterOfflineMinutes < conf.OfflineTimeout ||
conf.DeviceRebootAfterOfflineMinutes < conf.UserTimeout {
return &conf, errw.Errorf("device_reboot_after_offline_minutes cannot be less than offline_timeout or user_timeout")
(conf.DeviceRebootAfterOfflineMinutes < conf.OfflineTimeout || conf.DeviceRebootAfterOfflineMinutes < conf.UserTimeout) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me WAY too long to figure out this was the issue. One pair of missing parenthesis! So easy to miss!

badOffline := conf.DeviceRebootAfterOfflineMinutes
conf.DeviceRebootAfterOfflineMinutes = defaultConf.DeviceRebootAfterOfflineMinutes
return &conf, errw.Errorf("device_reboot_after_offline_minutes (%s) cannot be less than offline_timeout (%s) or user_timeout (%s)",
time.Duration(badOffline), time.Duration(conf.OfflineTimeout), time.Duration(conf.UserTimeout))
}

return &conf, nil
Expand Down Expand Up @@ -382,7 +386,7 @@ type Config struct {
DeviceRebootAfterOfflineMinutes Timeout `json:"device_reboot_after_offline_minutes"`
}

// Timeout allows parsing golang-style durations (1h20m30s) OR seconds-as-float from/to json.
// Timeout allows parsing golang-style durations (1h20m30s) OR minutes-as-float from/to json.
type Timeout time.Duration

func (t Timeout) MarshalJSON() ([]byte, error) {
Expand Down
16 changes: 11 additions & 5 deletions subsystems/provisioning/networkmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ func (w *Provisioning) warnIfMultiplePrimaryNetworks() {
func (w *Provisioning) getVisibleNetworks() []NetworkInfo {
var visible []NetworkInfo
for _, nw := range w.netState.Networks() {
if nw.lastSeen.After(time.Now().Add(time.Minute*-1)) && !nw.isHotspot {
// note this does NOT use VisibleNetworkTimeout (like getCandidates does)
recentlySeen := nw.lastSeen.After(w.connState.getProvisioningChange().Add(time.Duration(w.Config().OfflineTimeout * -2)))

if !nw.isHotspot && recentlySeen {
visible = append(visible, nw.getInfo())
}
}
Expand Down Expand Up @@ -510,7 +513,7 @@ func (w *Provisioning) getCandidates(ifName string) []string {
continue
}
// ssid seen within the past minute
visible := nw.lastSeen.After(time.Now().Add(time.Minute * -1))
visible := nw.lastSeen.After(time.Now().Add(VisibleNetworkTimeout * -1))

// ssid has a connection known to network manager
configured := nw.conn != nil
Expand Down Expand Up @@ -555,6 +558,9 @@ func (w *Provisioning) backgroundLoop(ctx context.Context, scanChan chan<- bool)
if err := w.networkScan(ctx); err != nil {
w.logger.Error(err)
}
if err := w.updateKnownConnections(ctx); err != nil {
w.logger.Error(err)
}
if err := w.checkConnections(); err != nil {
w.logger.Error(err)
}
Expand Down Expand Up @@ -656,9 +662,9 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
}
}
case <-scanChan:
case <-time.After(scanLoopDelay * 4):
case <-time.After((scanLoopDelay + scanTimeout) * 2):
// safety fallback if something hangs
w.logger.Warn("wifi scan has not completed for %s", scanLoopDelay*5)
w.logger.Warnf("wifi scan has not completed for %s", (scanLoopDelay+scanTimeout)*2)
}

w.mainLoopHealth.MarkGood()
Expand Down Expand Up @@ -741,7 +747,7 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
offlineRebootTimeout := w.cfg.DeviceRebootAfterOfflineMinutes > 0 &&
lastConnectivity.Before(now.Add(time.Duration(w.cfg.DeviceRebootAfterOfflineMinutes)*-1))
if offlineRebootTimeout {
w.logger.Infof("device has been offline for more than %s minutes, rebooting", w.cfg.DeviceRebootAfterOfflineMinutes)
w.logger.Infof("device has been offline for more than %s, rebooting", time.Duration(w.cfg.DeviceRebootAfterOfflineMinutes))
cmd := exec.Command("systemctl", "reboot")
output, err := cmd.CombinedOutput()
if err != nil {
Expand Down
39 changes: 39 additions & 0 deletions subsystems/provisioning/networkstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ type networkState struct {
// the wifi interface to default to when no interface is specified
hotspotInterface string

// these variables track and disable the scan-in-hotspot functionality
scanFailCount uint
noScanInHotspot bool

// key is ssid@interface for wifi, ex: TestNetwork@wlan0
// interface may be "any" for no interface set, ex: TestNetwork@any
// wired networks are just interface, ex: eth0
Expand Down Expand Up @@ -43,6 +47,41 @@ func NewNetworkState(logger logging.Logger) *networkState {
}
}

func (n *networkState) NoScanInHotspot() bool {
n.mu.Lock()
defer n.mu.Unlock()
return n.noScanInHotspot
}

func (n *networkState) SetNoScanInHotspot(noScan bool) {
n.mu.Lock()
defer n.mu.Unlock()
n.noScanInHotspot = noScan
}

func (n *networkState) IncrementFailScan() {
n.mu.Lock()
defer n.mu.Unlock()
n.scanFailCount++
if n.scanFailCount >= 3 {
n.noScanInHotspot = true
n.logger.Warn("Device hardware/software does not appear to support wifi scanning while hotspot is active. " +
"Further scanning will be disabled while in hotspot mode. Relying on fallback timeout to exit hotspot mode and allow rescans.")
}
}

func (n *networkState) FailScan() uint {
n.mu.Lock()
defer n.mu.Unlock()
return n.scanFailCount
}

func (n *networkState) ResetFailScan() {
n.mu.Lock()
defer n.mu.Unlock()
n.scanFailCount = 0
}

func (n *networkState) SetHotspotInterface(iface string) {
n.mu.Lock()
defer n.mu.Unlock()
Expand Down
43 changes: 25 additions & 18 deletions subsystems/provisioning/provisioning.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package provisioning

import (
"context"
"errors"
"net/http"
"reflect"
"strings"
Expand Down Expand Up @@ -92,22 +93,18 @@ func NewProvisioning(ctx context.Context, logger logging.Logger, updateConf *age
}

func (w *Provisioning) getNM() (gnm.NetworkManager, error) {
nmErr := errw.New("NetworkManager does not appear to be responding as expected. " +
"Please ensure NetworkManger >= v1.42 is installed and enabled. Disabling agent-provisioning until next restart.")
wifiErr := errw.New("No WiFi devices available. Disabling agent-provisioning until next restart.")

nm, err := gnm.NewNetworkManager()
if err != nil {
w.noNM = true
w.logger.Error(err)
return nil, nmErr
return nil, ErrNM
}

ver, err := nm.GetPropertyVersion()
if err != nil {
w.noNM = true
w.logger.Error(err)
return nil, nmErr
return nil, ErrNM
}

w.logger.Infof("Found NetworkManager version: %s", ver)
Expand All @@ -116,24 +113,28 @@ func (w *Provisioning) getNM() (gnm.NetworkManager, error) {
if err != nil {
w.noNM = true
w.logger.Error(err)
return nil, nmErr
return nil, ErrNM
}

if !sv.GreaterThanEqual(semver.MustParse("1.42.0")) {
if !sv.GreaterThanEqual(semver.MustParse("1.30.0")) {
w.noNM = true
return nil, nmErr
return nil, ErrNM
}

flags, err := nm.GetPropertyRadioFlags()
if err != nil {
w.noNM = true
w.logger.Error(err)
return nil, wifiErr
}
// Bail out here early if we can't find a wifi radio
// Older versions will bail out during initDevices() if scan fails to find a wifi interface
if sv.GreaterThanEqual(semver.MustParse("1.38.0")) {
flags, err := nm.GetPropertyRadioFlags()
if err != nil {
w.noNM = true
w.logger.Error(err)
return nil, ErrNoWifi
}

if flags&gnm.NmRadioFlagsWlanAvailable != gnm.NmRadioFlagsWlanAvailable {
w.noNM = true
return nil, wifiErr
if flags&gnm.NmRadioFlagsWlanAvailable != gnm.NmRadioFlagsWlanAvailable {
w.noNM = true
return nil, ErrNoWifi
}
}

return nm, nil
Expand Down Expand Up @@ -177,13 +178,19 @@ func (w *Provisioning) init(ctx context.Context) error {
}

if err := w.initDevices(); err != nil {
if errors.Is(err, ErrNoWifi) {
w.noNM = true
}
return err
}

w.checkConfigured()
if err := w.networkScan(ctx); err != nil {
w.logger.Error(err)
}
if err := w.updateKnownConnections(ctx); err != nil {
w.logger.Error(err)
}

w.warnIfMultiplePrimaryNetworks()

Expand Down
28 changes: 24 additions & 4 deletions subsystems/provisioning/scanning.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,18 @@ import (
errw "github.com/pkg/errors"
)

var (
ErrScanTimeout = errw.New("wifi scanning timed out")

// how long is a scanned network "visible" for candidate selection?
VisibleNetworkTimeout = time.Minute
)

func (w *Provisioning) networkScan(ctx context.Context) error {
if w.connState.getProvisioning() && w.netState.NoScanInHotspot() {
return nil
}

wifiDev := w.netState.WifiDevice(w.Config().HotspotInterface)
if wifiDev == nil {
return errw.Errorf("cannot find hotspot interface: %s", w.Config().HotspotInterface)
Expand All @@ -27,18 +38,27 @@ func (w *Provisioning) networkScan(ctx context.Context) error {
return errw.Wrap(err, "scanning wifi")
}

var lastScan int64
scanDeadline := time.Now().Add(scanTimeout)
for {
lastScan, err = wifiDev.GetPropertyLastScan()
lastScan, err := wifiDev.GetPropertyLastScan()
if err != nil {
return errw.Wrap(err, "scanning wifi")
}
if lastScan > prevScan {
if w.connState.getProvisioning() {
w.netState.ResetFailScan()
}
break
}
if !w.bgLoopHealth.Sleep(ctx, time.Second) {
return nil
}
if time.Now().After(scanDeadline) {
if w.connState.getProvisioning() {
w.netState.IncrementFailScan()
}
return ErrScanTimeout
}
}

wifiList, err := wifiDev.GetAccessPoints()
Expand Down Expand Up @@ -109,14 +129,14 @@ func (w *Provisioning) networkScan(ctx context.Context) error {
}
nw.mu.Lock()
// if a network isn't visible, reset the times so we'll retry if it comes back
if nw.lastSeen.Before(time.Now().Add(time.Minute * -1)) {
if nw.lastSeen.Before(time.Now().Add(VisibleNetworkTimeout * -1)) {
nw.firstSeen = time.Time{}
nw.lastTried = time.Time{}
}
nw.mu.Unlock()
}

return w.updateKnownConnections(ctx)
return nil
}

func parseWPAFlags(apFlags, wpaFlags, rsnFlags uint32) string {
Expand Down
8 changes: 7 additions & 1 deletion subsystems/provisioning/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import (
errw "github.com/pkg/errors"
)

var (
ErrNM = errw.New("NetworkManager does not appear to be responding as expected. " +
"Please ensure NetworkManger >= v1.30 is installed and enabled. Disabling agent-provisioning until next restart.")
ErrNoWifi = errw.New("No WiFi devices available. Disabling agent-provisioning until next restart.")
)

func (w *Provisioning) writeDNSMasq() error {
DNSMasqContents := DNSMasqContentsRedirect
if w.cfg.DisableDNSRedirect {
Expand Down Expand Up @@ -137,7 +143,7 @@ func (w *Provisioning) initDevices() error {
}

if w.cfg.HotspotInterface == "" {
return errors.New("cannot find wifi device for provisioning/hotspot")
return ErrNoWifi
}

return nil
Expand Down
Loading