fix: update networking stack after Equnix Metal testing

This PR contains multiple fixes to the networking controllers and
logging improvements for easier debugging:

* `LinkConfigController` now correctly merges duplicate link definitions
in the machine configuration
* `LinkConfigController` correctly enslaves bond interfaces even if
they're not mentioned explicitly in the config
* bond slaves are no longer brought down forcefully, but they're brought
down before being enslaved (and brought up once they're enslaved)
* route sync code ignores flags which are not managed by Talos

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>

internal/app/machined/pkg/controllers/network/link_config.go

@@ -162,6 +162,12 @@ func (ctrl *LinkConfigController) Run(ctx context.Context, r controller.Runtime,
 		if cfgProvider != nil {
 			for _, device := range cfgProvider.Machine().Network().Devices() {
 				configuredLinks[device.Interface()] = struct{}{}
+
+				if device.Bond() != nil {
+					for _, link := range device.Bond().Interfaces() {
+						configuredLinks[link] = struct{}{}
+					}
+				}
 			}
 		}
 
@@ -262,7 +268,7 @@ func (ctrl *LinkConfigController) parseCmdline(logger *zap.Logger) (network.Link
 }
 
 //nolint:gocyclo
-func (ctrl *LinkConfigController) parseMachineConfiguration(logger *zap.Logger, cfgProvider talosconfig.Provider) (links []network.LinkSpecSpec) {
+func (ctrl *LinkConfigController) parseMachineConfiguration(logger *zap.Logger, cfgProvider talosconfig.Provider) []network.LinkSpecSpec {
 	// scan for the bonds
 	bondedLinks := map[string]string{} // mapping physical interface -> bond interface
 
@@ -276,7 +282,7 @@ func (ctrl *LinkConfigController) parseMachineConfiguration(logger *zap.Logger,
 		}
 
 		for _, linkName := range device.Bond().Interfaces() {
-			if _, exists := bondedLinks[linkName]; exists {
+			if bondName, exists := bondedLinks[linkName]; exists && bondName != device.Interface() {
 				logger.Sugar().Warnf("link %q is included into more than two bonds", linkName)
 			}
 
@@ -284,50 +290,70 @@ func (ctrl *LinkConfigController) parseMachineConfiguration(logger *zap.Logger,
 		}
 	}
 
+	linkMap := map[string]*network.LinkSpecSpec{}
+
 	for _, device := range cfgProvider.Machine().Network().Devices() {
 		if device.Ignore() {
 			continue
 		}
 
-		link := network.LinkSpecSpec{
-			Name:        device.Interface(),
-			MTU:         uint32(device.MTU()),
-			Up:          true,
-			ConfigLayer: network.ConfigMachineConfiguration,
+		if _, exists := linkMap[device.Interface()]; !exists {
+			linkMap[device.Interface()] = &network.LinkSpecSpec{
+				Name:        device.Interface(),
+				Up:          true,
+				ConfigLayer: network.ConfigMachineConfiguration,
+			}
 		}
 
-		if bondName := bondedLinks[device.Interface()]; bondName != "" {
-			bondSlave(&link, bondName)
+		if device.MTU() != 0 {
+			linkMap[device.Interface()].MTU = uint32(device.MTU())
 		}
 
 		if device.Bond() != nil {
-			if err := bondMaster(&link, device.Bond()); err != nil {
+			if err := bondMaster(linkMap[device.Interface()], device.Bond()); err != nil {
 				logger.Error("error parsing bond config", zap.Error(err))
 			}
 		}
 
 		if device.WireguardConfig() != nil {
-			if err := wireguardLink(&link, device.WireguardConfig()); err != nil {
+			if err := wireguardLink(linkMap[device.Interface()], device.WireguardConfig()); err != nil {
 				logger.Error("error parsing wireguard config", zap.Error(err))
 			}
 		}
 
 		if device.Dummy() {
-			dummyLink(&link)
+			dummyLink(linkMap[device.Interface()])
 		}
 
 		for _, vlan := range device.Vlans() {
-			links = append(links, vlanLink(device.Interface(), vlan))
+			vlanSpec := vlanLink(device.Interface(), vlan)
+
+			linkMap[vlanSpec.Name] = &vlanSpec
+		}
+	}
+
+	for slaveName, bondName := range bondedLinks {
+		if _, exists := linkMap[slaveName]; !exists {
+			linkMap[slaveName] = &network.LinkSpecSpec{
+				Name:        slaveName,
+				Up:          true,
+				ConfigLayer: network.ConfigMachineConfiguration,
+			}
 		}
 
-		links = append(links, link)
+		bondSlave(linkMap[slaveName], bondName)
+	}
+
+	links := make([]network.LinkSpecSpec, 0, len(linkMap))
+
+	for _, link := range linkMap {
+		links = append(links, *link)
 	}
 
 	return links
 }
 
 func bondSlave(link *network.LinkSpecSpec, bondName string) {
-	link.Up = false
 	link.MasterName = bondName
 }
 

internal/app/machined/pkg/controllers/network/link_config_test.go

@@ -191,6 +191,10 @@ func (suite *LinkConfigSuite) TestMachineConfiguration() {
 						DeviceInterface: "eth1",
 						DeviceCIDR:      "192.168.0.24/28",
 					},
+					{
+						DeviceInterface: "eth1",
+						DeviceMTU:       9001,
+					},
 					{
 						DeviceIgnore:    true,
 						DeviceInterface: "eth2",
@@ -199,9 +203,6 @@ func (suite *LinkConfigSuite) TestMachineConfiguration() {
 					{
 						DeviceInterface: "eth2",
 					},
-					{
-						DeviceInterface: "eth3",
-					},
 					{
 						DeviceInterface: "bond0",
 						DeviceBond: &v1alpha1.Bond{
@@ -262,6 +263,12 @@ func (suite *LinkConfigSuite) TestMachineConfiguration() {
 				case "eth0", "eth1":
 					suite.Assert().True(r.TypedSpec().Up)
 					suite.Assert().False(r.TypedSpec().Logical)
+
+					if r.TypedSpec().Name == "eth0" {
+						suite.Assert().EqualValues(0, r.TypedSpec().MTU)
+					} else {
+						suite.Assert().EqualValues(9001, r.TypedSpec().MTU)
+					}
 				case "eth0.24", "eth0.48":
 					suite.Assert().True(r.TypedSpec().Up)
 					suite.Assert().True(r.TypedSpec().Logical)
@@ -276,7 +283,7 @@ func (suite *LinkConfigSuite) TestMachineConfiguration() {
 						suite.Assert().EqualValues(48, r.TypedSpec().VLAN.VID)
 					}
 				case "eth2", "eth3":
-					suite.Assert().False(r.TypedSpec().Up)
+					suite.Assert().True(r.TypedSpec().Up)
 					suite.Assert().False(r.TypedSpec().Logical)
 					suite.Assert().Equal("bond0", r.TypedSpec().MasterName)
 				case "bond0":
@@ -316,7 +323,7 @@ func (suite *LinkConfigSuite) TestDefaultUp() {
 		Cmdline: procfs.NewCmdline("talos.network.interface.ignore=eth2"),
 	}))
 
-	for _, link := range []string{"eth0", "eth1", "eth2"} {
+	for _, link := range []string{"eth0", "eth1", "eth2", "eth3", "eth4"} {
 		linkStatus := network.NewLinkStatus(network.NamespaceName, link)
 		linkStatus.TypedSpec().Type = nethelpers.LinkEther
 		linkStatus.TypedSpec().LinkState = true
@@ -345,6 +352,15 @@ func (suite *LinkConfigSuite) TestDefaultUp() {
 							},
 						},
 					},
+					{
+						DeviceInterface: "bond0",
+						DeviceBond: &v1alpha1.Bond{
+							BondInterfaces: []string{
+								"eth3",
+								"eth4",
+							},
+						},
+					},
 				},
 			},
 		},
@@ -378,6 +394,8 @@ func (suite *LinkConfigSuite) TestDefaultUp() {
 			return suite.assertNoLinks([]string{
 				"default/eth0",
 				"default/eth2",
+				"default/eth3",
+				"default/eth4",
 			})
 		}))
 }

internal/app/machined/pkg/controllers/network/link_merge.go

@@ -111,6 +111,8 @@ func (ctrl *LinkMergeController) Run(ctx context.Context, r controller.Runtime,
 			}); err != nil {
 				return fmt.Errorf("error updating resource: %w", err)
 			}
+
+			logger.Debug("merged link spec", zap.String("id", id), zap.Any("spec", link))
 		}
 
 		// list link for cleanup

internal/app/machined/pkg/controllers/network/link_spec.go

@@ -403,7 +403,7 @@ func (ctrl *LinkSpecController) syncLink(ctx context.Context, r controller.Runti
 		}
 
 		// sync UP flag
-		existingUp := existing.Attributes.OperationalState == rtnetlink.OperStateUnknown || existing.Attributes.OperationalState == rtnetlink.OperStateUp
+		existingUp := existing.Flags&unix.IFF_UP == unix.IFF_UP
 		if existingUp != link.TypedSpec().Up {
 			flags := uint32(0)
 
@@ -456,6 +456,7 @@ func (ctrl *LinkSpecController) syncLink(ctx context.Context, r controller.Runti
 				Family: existing.Family,
 				Type:   existing.Type,
 				Index:  existing.Index,
+				Change: unix.IFF_UP,
 				Attributes: &rtnetlink.LinkAttributes{
 					Master: pointer.ToUint32(masterIndex),
 				},

internal/app/machined/pkg/controllers/network/link_spec_test.go

@@ -339,7 +339,7 @@ func (suite *LinkSpecSuite) TestBond() {
 		Name:        dummy0Name,
 		Type:        nethelpers.LinkEther,
 		Kind:        "dummy",
-		Up:          false,
+		Up:          true,
 		Logical:     true,
 		MasterName:  bondName,
 		ConfigLayer: network.ConfigDefault,
@@ -351,7 +351,7 @@ func (suite *LinkSpecSuite) TestBond() {
 		Name:        dummy1Name,
 		Type:        nethelpers.LinkEther,
 		Kind:        "dummy",
-		Up:          false,
+		Up:          true,
 		Logical:     true,
 		MasterName:  bondName,
 		ConfigLayer: network.ConfigDefault,
@@ -374,8 +374,8 @@ func (suite *LinkSpecSuite) TestBond() {
 				case dummy0Name, dummy1Name:
 					suite.Assert().Equal("dummy", r.TypedSpec().Kind)
 
-					if r.TypedSpec().OperationalState != nethelpers.OperStateDown {
-						return retry.ExpectedErrorf("link is not down: %s", r.TypedSpec().OperationalState)
+					if r.TypedSpec().OperationalState != nethelpers.OperStateUnknown {
+						return retry.ExpectedErrorf("link is not up: %s", r.TypedSpec().OperationalState)
 					}
 
 					if r.TypedSpec().MasterIndex == 0 {

internal/app/machined/pkg/controllers/network/operator_config.go

@@ -117,6 +117,12 @@ func (ctrl *OperatorConfigController) Run(ctx context.Context, r controller.Runt
 					continue
 				}
 
+				if device.Bond() != nil {
+					for _, link := range device.Bond().Interfaces() {
+						configuredInterfaces[link] = struct{}{}
+					}
+				}
+
 				if device.DHCP() && device.DHCPOptions().IPv4() {
 					routeMetric := device.DHCPOptions().RouteMetric()
 					if routeMetric == 0 {

internal/app/machined/pkg/controllers/network/resolver_merge.go

@@ -3,6 +3,8 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 // Package network provides controllers which manage network resources.
+//
+//nolint:dupl
 package network
 
 import (

internal/app/machined/pkg/controllers/network/resolver_spec.go

@@ -87,6 +87,14 @@ func (ctrl *ResolverSpecController) Run(ctx context.Context, r controller.Runtim
 					return fmt.Errorf("error removing finalizer: %w", err)
 				}
 			case resource.PhaseRunning:
+				resolvers := make([]string, len(spec.TypedSpec().DNSServers))
+
+				for i := range resolvers {
+					resolvers[i] = spec.TypedSpec().DNSServers[i].String()
+				}
+
+				logger.Info("setting resolvers", zap.Strings("resolvers", resolvers))
+
 				if err = r.Modify(ctx, network.NewResolverStatus(network.NamespaceName, spec.Metadata().ID()), func(r resource.Resource) error {
 					status := r.(*network.ResolverStatus) //nolint:forcetypeassert,errcheck
 

internal/app/machined/pkg/controllers/network/route_spec.go

@@ -149,11 +149,20 @@ func (ctrl *RouteSpecController) syncRoute(ctx context.Context, r controller.Run
 	linkIndex := resolveLinkName(links, route.TypedSpec().OutLinkName)
 
 	destinationStr := route.TypedSpec().Destination.String()
-
 	if route.TypedSpec().Destination.IsZero() {
 		destinationStr = "default"
 	}
 
+	sourceStr := route.TypedSpec().Source.String()
+	if route.TypedSpec().Source.IsZero() {
+		sourceStr = ""
+	}
+
+	gatewayStr := route.TypedSpec().Gateway.String()
+	if route.TypedSpec().Gateway.IsZero() {
+		gatewayStr = ""
+	}
+
 	switch route.Metadata().Phase() {
 	case resource.PhaseTearingDown:
 		for _, existing := range findRoutes(routes, route.TypedSpec().Destination, route.TypedSpec().Gateway, route.TypedSpec().Table) {
@@ -162,7 +171,12 @@ func (ctrl *RouteSpecController) syncRoute(ctx context.Context, r controller.Run
 				return fmt.Errorf("error removing route: %w", err)
 			}
 
-			logger.Sugar().Infof("removed route to %s via %s (link %q)", destinationStr, route.TypedSpec().Gateway, route.TypedSpec().OutLinkName)
+			logger.Info("deleted route",
+				zap.String("destination", destinationStr),
+				zap.String("gateway", gatewayStr),
+				zap.Stringer("table", route.TypedSpec().Table),
+				zap.String("link", route.TypedSpec().OutLinkName),
+			)
 		}
 
 		// now remove finalizer as address was deleted
@@ -178,11 +192,10 @@ func (ctrl *RouteSpecController) syncRoute(ctx context.Context, r controller.Run
 		matchFound := false
 
 		for _, existing := range findRoutes(routes, route.TypedSpec().Destination, route.TypedSpec().Gateway, route.TypedSpec().Table) {
-			// check if existing matches the spec: if it does, skip update
-			if existing.Scope == uint8(route.TypedSpec().Scope) && existing.Flags == uint32(route.TypedSpec().Flags) &&
-				existing.Protocol == uint8(route.TypedSpec().Protocol) && existing.Flags == uint32(route.TypedSpec().Flags) &&
+			// check if existing route matches the spec: if it does, skip update
+			if existing.Scope == uint8(route.TypedSpec().Scope) && nethelpers.RouteFlags(existing.Flags).Equal(route.TypedSpec().Flags) &&
+				existing.Protocol == uint8(route.TypedSpec().Protocol) &&
 				existing.Attributes.OutIface == linkIndex && existing.Attributes.Priority == route.TypedSpec().Priority &&
-				existing.Attributes.Table == uint32(route.TypedSpec().Table) &&
 				(route.TypedSpec().Source.IsZero() ||
 					existing.Attributes.Src.Equal(route.TypedSpec().Source.IP.IPAddr().IP)) {
 				matchFound = true
@@ -190,12 +203,29 @@ func (ctrl *RouteSpecController) syncRoute(ctx context.Context, r controller.Run
 				continue
 			}
 
-			// delete route, it doesn't match the spec
+			// delete the route, it doesn't match the spec
 			if err := conn.Route.Delete(existing); err != nil {
 				return fmt.Errorf("error removing route: %w", err)
 			}
 
-			logger.Sugar().Infof("removed route to %s via %s (link %q)", destinationStr, route.TypedSpec().Gateway, route.TypedSpec().OutLinkName)
+			logger.Debug("removed route due to mismatch",
+				zap.String("destination", destinationStr),
+				zap.String("gateway", gatewayStr),
+				zap.Stringer("table", route.TypedSpec().Table),
+				zap.String("link", route.TypedSpec().OutLinkName),
+				zap.Stringer("old_scope", nethelpers.Scope(existing.Scope)),
+				zap.Stringer("new_scope", route.TypedSpec().Scope),
+				zap.Stringer("old_flags", nethelpers.RouteFlags(existing.Flags)),
+				zap.Stringer("new_flags", route.TypedSpec().Flags),
+				zap.Stringer("old_protocol", nethelpers.RouteProtocol(existing.Protocol)),
+				zap.Stringer("new_protocol", route.TypedSpec().Protocol),
+				zap.Uint32("old_link_index", existing.Attributes.OutIface),
+				zap.Uint32("new_link_index", linkIndex),
+				zap.Uint32("old_priority", existing.Attributes.Priority),
+				zap.Uint32("new_priority", route.TypedSpec().Priority),
+				zap.Stringer("old_source", existing.Attributes.Src),
+				zap.String("new_source", sourceStr),
+			)
 		}
 
 		if matchFound {
@@ -225,7 +255,12 @@ func (ctrl *RouteSpecController) syncRoute(ctx context.Context, r controller.Run
 			return fmt.Errorf("error adding route: %w, message %+v", err, *msg)
 		}
 
-		logger.Sugar().Infof("created route to %s via %s (link %q)", destinationStr, route.TypedSpec().Gateway, route.TypedSpec().OutLinkName)
+		logger.Info("created route",
+			zap.String("destination", destinationStr),
+			zap.String("gateway", gatewayStr),
+			zap.Stringer("table", route.TypedSpec().Table),
+			zap.String("link", route.TypedSpec().OutLinkName),
+		)
 	}
 
 	return nil

internal/app/machined/pkg/controllers/network/timeserver_merge.go

@@ -3,6 +3,8 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 // Package network provides controllers which manage network resources.
+//
+//nolint:dupl
 package network
 
 import (

pkg/machinery/nethelpers/routeflag_string_linux.go

@@ -12,6 +12,10 @@ func _() {
 	_ = x[RouteCloned-512]
 	_ = x[RouteEqualize-1024]
 	_ = x[RoutePrefix-2048]
+	_ = x[RouteLookupTable-4096]
+	_ = x[RouteFIBMatch-8192]
+	_ = x[RouteOffload-16384]
+	_ = x[RouteTrap-32768]
 }
 
 const (
@@ -19,6 +23,10 @@ const (
 	_RouteFlag_name_1 = "cloned"
 	_RouteFlag_name_2 = "equalize"
 	_RouteFlag_name_3 = "prefix"
+	_RouteFlag_name_4 = "lookup_table"
+	_RouteFlag_name_5 = "fib_match"
+	_RouteFlag_name_6 = "offload"
+	_RouteFlag_name_7 = "trap"
 )
 
 func (i RouteFlag) String() string {
@@ -31,6 +39,14 @@ func (i RouteFlag) String() string {
 		return _RouteFlag_name_2
 	case i == 2048:
 		return _RouteFlag_name_3
+	case i == 4096:
+		return _RouteFlag_name_4
+	case i == 8192:
+		return _RouteFlag_name_5
+	case i == 16384:
+		return _RouteFlag_name_6
+	case i == 32768:
+		return _RouteFlag_name_7
 	default:
 		return "RouteFlag(" + strconv.FormatInt(int64(i), 10) + ")"
 	}

pkg/machinery/nethelpers/routeflags_linux.go

@@ -18,7 +18,7 @@ type RouteFlags uint32
 func (flags RouteFlags) String() string {
 	var values []string
 
-	for flag := RouteNotify; flag <= RoutePrefix; flag <<= 1 {
+	for flag := RouteNotify; flag <= RouteTrap; flag <<= 1 {
 		if (RouteFlag(flags) & flag) == flag {
 			values = append(values, flag.String())
 		}
@@ -27,6 +27,11 @@ func (flags RouteFlags) String() string {
 	return strings.Join(values, ",")
 }
 
+// Equal tests for RouteFlags equality ignoring flags not managed by this implementation.
+func (flags RouteFlags) Equal(other RouteFlags) bool {
+	return (flags & RouteFlags(RouteFlagsMask)) == (other & RouteFlags(RouteFlagsMask))
+}
+
 // MarshalYAML implements yaml.Marshaler.
 func (flags RouteFlags) MarshalYAML() (interface{}, error) {
 	return flags.String(), nil
@@ -37,8 +42,15 @@ type RouteFlag uint32
 
 // RouteFlag constants.
 const (
-	RouteNotify   RouteFlag = unix.RTM_F_NOTIFY   // notify
-	RouteCloned   RouteFlag = unix.RTM_F_CLONED   // cloned
-	RouteEqualize RouteFlag = unix.RTM_F_EQUALIZE // equalize
-	RoutePrefix   RouteFlag = unix.RTM_F_PREFIX   // prefix
+	RouteNotify      RouteFlag = unix.RTM_F_NOTIFY       // notify
+	RouteCloned      RouteFlag = unix.RTM_F_CLONED       // cloned
+	RouteEqualize    RouteFlag = unix.RTM_F_EQUALIZE     // equalize
+	RoutePrefix      RouteFlag = unix.RTM_F_PREFIX       // prefix
+	RouteLookupTable RouteFlag = unix.RTM_F_LOOKUP_TABLE // lookup_table
+	RouteFIBMatch    RouteFlag = unix.RTM_F_FIB_MATCH    // fib_match
+	RouteOffload     RouteFlag = unix.RTM_F_OFFLOAD      // offload
+	RouteTrap        RouteFlag = unix.RTM_F_TRAP         // trap
 )
+
+// RouteFlagsMask is a supported set of flags to manage.
+const RouteFlagsMask = RouteNotify | RouteCloned | RouteEqualize | RoutePrefix

pkg/resources/network/link_spec.go

@@ -63,6 +63,8 @@ var (
 )
 
 // Merge with other, overwriting fields from other if set.
+//
+//nolint:gocyclo
 func (spec *LinkSpecSpec) Merge(other *LinkSpecSpec) error {
 	if spec.Logical != other.Logical {
 		return fmt.Errorf("mismatch on logical for %q: %v != %v", spec.Name, spec.Logical, other.Logical)
@@ -84,6 +86,14 @@ func (spec *LinkSpecSpec) Merge(other *LinkSpecSpec) error {
 		spec.Type = 0
 	}
 
+	if other.ParentName != "" {
+		spec.ParentName = other.ParentName
+	}
+
+	if other.MasterName != "" {
+		spec.MasterName = other.MasterName
+	}
+
 	if other.VLAN != zeroVLAN {
 		spec.VLAN = other.VLAN
 	}