fix race condition in link manager

main.go

@@ -33,10 +33,8 @@ func Main() {
 	go runWatchdog()
 	go confirmCurrentSystem()
 
-	initNative(systemVersionLocal, appVersionLocal)
-
-	// initialize display
 	initDisplay()
+	initNative(systemVersionLocal, appVersionLocal)
 
 	http.DefaultClient.Timeout = 1 * time.Minute
 

pkg/nmlite/interface.go

@@ -106,6 +106,9 @@ func NewInterfaceManager(ctx context.Context, ifaceName string, config *types.Ne
 
 // Start starts managing the interface
 func (im *InterfaceManager) Start() error {
+	im.stateMu.Lock()
+	defer im.stateMu.Unlock()
+
 	im.logger.Info().Msg("starting interface manager")
 
 	// Start monitoring interface state
@@ -113,6 +116,22 @@ func (im *InterfaceManager) Start() error {
 	go im.monitorInterfaceState()
 
 	nl := getNetlinkManager()
+
+	// Set the link state
+	linkState, err := nl.GetLinkByName(im.ifaceName)
+	if err != nil {
+		return fmt.Errorf("failed to get interface: %w", err)
+	}
+	im.linkState = linkState
+
+	// Bring the interface up
+	_, linkUpErr := nl.EnsureInterfaceUpWithTimeout(
+		im.ctx,
+		im.linkState,
+		30*time.Second,
+	)
+
+	// Set callback after the interface is up
 	nl.AddStateChangeCallback(im.ifaceName, link.StateChangeCallback{
 		Async: true,
 		Func: func(link *link.Link) {
@@ -120,10 +139,14 @@ func (im *InterfaceManager) Start() error {
 		},
 	})
 
-	// Apply initial configuration
-	if err := im.applyConfiguration(); err != nil {
-		im.logger.Error().Err(err).Msg("failed to apply initial configuration")
-		return err
+	if linkUpErr != nil {
+		im.logger.Error().Err(linkUpErr).Msg("failed to bring interface up, continuing anyway")
+	} else {
+		// Apply initial configuration
+		if err := im.applyConfiguration(); err != nil {
+			im.logger.Error().Err(err).Msg("failed to apply initial configuration")
+			return err
+		}
 	}
 
 	im.logger.Info().Msg("interface manager started")
@@ -451,12 +474,18 @@ func (im *InterfaceManager) applyIPv6SLAAC() error {
 	}
 
 	netlinkMgr := getNetlinkManager()
+
+	// Ensure interface is up
+	if err := netlinkMgr.EnsureInterfaceUp(l); err != nil {
+		return fmt.Errorf("failed to bring interface up: %w", err)
+	}
+
 	if err := netlinkMgr.RemoveNonLinkLocalIPv6Addresses(l); err != nil {
 		return fmt.Errorf("failed to remove non-link-local IPv6 addresses: %w", err)
 	}
 
 	if err := im.SendRouterSolicitation(); err != nil {
-		return fmt.Errorf("failed to send router solicitation: %w", err)
+		im.logger.Error().Err(err).Msg("failed to send router solicitation, continuing anyway")
 	}
 
 	// Enable SLAAC
@@ -561,6 +590,10 @@ func (im *InterfaceManager) SendRouterSolicitation() error {
 		return fmt.Errorf("failed to get interface: %w", err)
 	}
 
+	if l.Attrs().OperState != netlink.OperUp {
+		return fmt.Errorf("interface %s is not up", im.ifaceName)
+	}
+
 	iface := l.Interface()
 	if iface == nil {
 		return fmt.Errorf("failed to get net.Interface for %s", im.ifaceName)
@@ -572,7 +605,6 @@ func (im *InterfaceManager) SendRouterSolicitation() error {
 	}
 
 	c, _, err := ndp.Listen(iface, ndp.LinkLocal)
-	defer c.Close()
 	if err != nil {
 		return fmt.Errorf("failed to create NDP listener on %s: %w", im.ifaceName, err)
 	}
@@ -585,10 +617,12 @@ func (im *InterfaceManager) SendRouterSolicitation() error {
 	targetAddr := netip.MustParseAddr("ff02::2")
 
 	if err := c.WriteTo(m, nil, targetAddr); err != nil {
+		c.Close()
 		return fmt.Errorf("failed to write to %s: %w", targetAddr.String(), err)
 	}
 
 	im.logger.Info().Msg("router solicitation sent")
+	c.Close()
 
 	return nil
 }

pkg/nmlite/jetdhcpc/dhcp4.go

@@ -1,6 +1,8 @@
 package jetdhcpc
 
 import (
+	"fmt"
+
 	"github.com/insomniacslk/dhcp/dhcpv4"
 	"github.com/insomniacslk/dhcp/dhcpv4/nclient4"
 	"github.com/vishvananda/netlink"
@@ -46,7 +48,11 @@ func (c *Client) requestLease4(iface netlink.Link) (*Lease, error) {
 		return nil, err
 	}
 
-	l.Info().Msgf("DHCPv4 lease acquired: %s", lease.ACK.Summary())
+	if lease == nil || lease.ACK == nil {
+		return nil, fmt.Errorf("failed to acquire DHCPv4 lease")
+	}
+
+	summaryStructured(lease.ACK, &l).Info().Msgf("DHCPv4 lease acquired: %s", lease.ACK.String())
 
 	return fromNclient4Lease(lease, ifname), nil
 }

pkg/nmlite/jetdhcpc/logging.go

@@ -21,11 +21,35 @@ func (s dhcpLogger) Printf(format string, v ...interface{}) {
 
 // PrintMessage prints a DHCP message in the short format via predefined Printfer
 func (s dhcpLogger) PrintMessage(prefix string, message *dhcpv4.DHCPv4) {
-	s.l.Info().Str("prefix", prefix).Str("message", message.String()).Msg("DHCP message")
+	s.l.Info().Msgf("%s: %s", prefix, message.String())
+}
+
+func summaryStructured(d *dhcpv4.DHCPv4, l *zerolog.Logger) *zerolog.Logger {
+	logger := l.With().
+		Str("opCode", d.OpCode.String()).
+		Str("hwType", d.HWType.String()).
+		Int("hopCount", int(d.HopCount)).
+		Str("transactionID", d.TransactionID.String()).
+		Int("numSeconds", int(d.NumSeconds)).
+		Str("flagsString", d.FlagsToString()).
+		Int("flags", int(d.Flags)).
+		Str("clientIP", d.ClientIPAddr.String()).
+		Str("yourIP", d.YourIPAddr.String()).
+		Str("serverIP", d.ServerIPAddr.String()).
+		Str("gatewayIP", d.GatewayIPAddr.String()).
+		Str("clientMAC", d.ClientHWAddr.String()).
+		Str("serverHostname", d.ServerHostName).
+		Str("bootFileName", d.BootFileName).
+		Str("options", d.Options.Summary(nil)).
+		Logger()
+	return &logger
 }
 
 func (c *Client) getDHCP4Logger(ifname string) nclient4.ClientOpt {
-	logger := c.l.With().Str("interface", ifname).Logger()
+	logger := c.l.With().
+		Str("interface", ifname).
+		Str("source", "dhcp4").
+		Logger()
 
 	return nclient4.WithLogger(dhcpLogger{
 		l: &logger,

pkg/nmlite/link/manager.go

@@ -163,22 +163,40 @@ func (nm *NetlinkManager) EnsureInterfaceUpWithTimeout(ctx context.Context, ifac
 		}
 
 		state := link.Attrs().OperState
+
+		l = l.With().
+			Int("attempt", attempt).
+			Dur("duration", time.Since(start)).
+			Str("state", state.String()).
+			Logger()
 		if state == netlink.OperUp || state == netlink.OperUnknown {
+			if attempt > 0 {
+				l.Info().Int("attempt", attempt-1).Msg("interface is up")
+			}
 			return link, nil
 		}
 
-		l.Info().Str("state", state.String()).Msg("bringing up interface")
+		l.Info().Msg("bringing up interface")
 
+		// bring up the interface
 		if err = nm.LinkSetUp(link); err != nil {
 			l.Error().Err(err).Msg("interface can't make it up")
 		}
 
-		l = l.With().Int("attempt", attempt).Dur("duration", time.Since(start)).Logger()
-
-		if attempt > 0 {
-			l.Info().Msg("interface up")
+		// refresh the link attributes
+		if err = link.Refresh(); err != nil {
+			l.Error().Err(err).Msg("failed to refresh link attributes")
 		}
 
+		// check the state again
+		state = link.Attrs().OperState
+		l = l.With().Str("new_state", state.String()).Logger()
+		if state == netlink.OperUp {
+			l.Info().Msg("interface is up")
+			return link, nil
+		}
+		l.Warn().Msg("interface is still down, retrying")
+
 		select {
 		case <-time.After(500 * time.Millisecond):
 			attempt++
@@ -381,24 +399,29 @@ func (nm *NetlinkManager) ReconcileLink(link *Link, expected []*types.IPAddress)
 		}
 
 		ipCidr := addr.Address.IP.String() + "/" + addr.Address.Mask.String()
+		ipNet := &net.IPNet{
+			IP:   addr.Address.IP,
+			Mask: addr.Address.Mask,
+		}
+
+		l := nm.logger.With().Str("address", ipNet.String()).Logger()
 		if ok := existingAddrs[ipCidr]; !ok {
-			ipNet := &net.IPNet{
-				IP:   addr.Address.IP,
-				Mask: addr.Address.Mask,
-			}
+			l.Trace().Msg("adding address")
 
 			if err := nm.AddrAdd(link, &netlink.Addr{IPNet: ipNet}); err != nil {
 				return fmt.Errorf("failed to add address %s: %w", ipCidr, err)
 			}
 
-			nm.logger.Info().Str("address", ipCidr).Msg("added address")
+			l.Info().Msg("address added")
 		}
 
 		if addr.Gateway != nil {
-			nm.logger.Trace().Str("address", ipCidr).Str("gateway", addr.Gateway.String()).Msg("adding default route for address")
+			gl := l.With().Str("gateway", addr.Gateway.String()).Logger()
+			gl.Trace().Msg("adding default route")
 			if err := nm.AddDefaultRoute(link, addr.Gateway, family); err != nil {
 				return fmt.Errorf("failed to add default route for address %s: %w", ipCidr, err)
 			}
+			gl.Info().Msg("default route added")
 		}
 	}
 

pkg/nmlite/link/netlink.go

@@ -35,11 +35,14 @@ var (
 // Link is a wrapper around netlink.Link
 type Link struct {
 	netlink.Link
+	mu sync.Mutex
 }
 
-// Refresh refreshes the link
-func (l *Link) Refresh() error {
-	linkName := l.Link.Attrs().Name
+// All lock actions should be done in external functions
+// and the internal functions should not be called directly
+
+func (l *Link) refresh() error {
+	linkName := l.ifName()
 	link, err := netlink.LinkByName(linkName)
 	if err != nil {
 		return err
@@ -51,13 +54,44 @@ func (l *Link) Refresh() error {
 	return nil
 }
 
+func (l *Link) attrs() *netlink.LinkAttrs {
+	return l.Link.Attrs()
+}
+
+func (l *Link) ifName() string {
+	attrs := l.attrs()
+	if attrs.Name == "" {
+		return ""
+	}
+	return attrs.Name
+}
+
+// Refresh refreshes the link
+func (l *Link) Refresh() error {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	return l.refresh()
+}
+
+// Attrs returns the attributes of the link
+func (l *Link) Attrs() *netlink.LinkAttrs {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	return l.attrs()
+}
+
 // Interface returns the interface of the link
 func (l *Link) Interface() *net.Interface {
-	attrs := l.Attrs()
-	if attrs.Name == "" {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	ifname := l.ifName()
+	if ifname == "" {
 		return nil
 	}
-	iface, err := net.InterfaceByName(attrs.Name)
+	iface, err := net.InterfaceByName(ifname)
 	if err != nil {
 		return nil
 	}
@@ -66,20 +100,22 @@ func (l *Link) Interface() *net.Interface {
 
 // HardwareAddr returns the hardware address of the link
 func (l *Link) HardwareAddr() net.HardwareAddr {
-	attrs := l.Attrs()
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	attrs := l.attrs()
 	if attrs.HardwareAddr == nil {
 		return nil
 	}
 	return attrs.HardwareAddr
 }
 
-// Attrs returns the attributes of the link
-func (l *Link) Attrs() *netlink.LinkAttrs {
-	return l.Link.Attrs()
-}
-
+// AddrList returns the addresses of the link
 func (l *Link) AddrList(family int) ([]netlink.Addr, error) {
-	return netlink.AddrList(l, family)
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	return netlink.AddrList(l.Link, family)
 }
 
 func (l *Link) IsSame(other *Link) bool {