Enhance LinuxCollector to support detecting multiple app VIF IPs

Application VIF may have multiple IP address assigned. They can be
either assigned directly on the same interface, or used on separate
VLAN sub-interfaces which share the parent interface MAC address.

LinuxCollector should detect and publish all of them, instead of
flapping between them and generating many IP address change
notifications, which trigger a flood of NI and App info messages
published to the controller.

For DHCP assigned IP addresses, we use lease time to determine if
a previously detected IP address is still valid. For statically
assigned IP, we expect to see at least one associated ARP packet
every 10 minutes. Otherwise, we consider the IP address to be removed
or simply not used anymore.

This commit also enhances LinuxCollector to support enabling or
disabling ARP snooping in runtime, without requiring to recreate all
switch network instances or rebooting device.

Signed-off-by: Milan Lenco <milan@zededa.com>
(cherry picked from commit 5b2acf4)

docs/CONFIG-PROPERTIES.md

@@ -51,7 +51,7 @@
 | netdump.topic.maxcount | integer | 10 | maximum number of netdumps that can be published for each topic. The oldest netdump is unpublished should a new netdump exceed the limit.
 | netdump.downloader.with.pcap | boolean | false | include packet captures inside netdumps for download requests. However, even if enabled, TCP segments carrying non-empty payload (i.e. content which is being downloaded) are excluded and the overall PCAP size is limited to 64MB. |
 | netdump.downloader.http.with.fieldvalue | boolean | false | include HTTP header field values in captured network traces for download requests (beware: may contain secrets, such as datastore credentials). |
-| network.switch.enable.arpsnoop | boolean | true | enable ARP Snooping on switch Network Instance, may need a device reboot to take effect |
+| network.switch.enable.arpsnoop | boolean | true | enable ARP Snooping on switch Network Instances |
 | wwan.query.visible.providers | bool | false | enable to periodically (once per hour) query the set of visible cellular service providers and publish them under WirelessStatus (for every modem) |
 | network.local.legacy.mac.address | bool | false | enables legacy MAC address generation for local network instances for those EVE nodes where changing MAC addresses in applications will lead to incorrect network configuration |
 

pkg/pillar/cmd/zedagent/handleconfig.go

@@ -1201,8 +1201,22 @@ func updateLocalServerMap(getconfigCtx *getconfigContext, localServerURL string)
 				continue
 			}
 			if localServerIP != nil {
-				// check if the defined IP of localServer equals the allocated IP of the app
-				if adapterStatus.AllocatedIPv4Addr.Equal(localServerIP) {
+				// Check if the defined IP of localServer equals one of the IPs
+				// allocated to the app.
+				var matchesApp bool
+				for _, ip := range adapterStatus.AssignedAddresses.IPv4Addrs {
+					if ip.Address.Equal(localServerIP) {
+						matchesApp = true
+						break
+					}
+				}
+				for _, ip := range adapterStatus.AssignedAddresses.IPv6Addrs {
+					if ip.Address.Equal(localServerIP) {
+						matchesApp = true
+						break
+					}
+				}
+				if matchesApp {
 					srvAddr := localServerAddr{
 						localServerAddr: localServerURL,
 						bridgeIP:        adapterStatus.BridgeIPAddr,

pkg/pillar/cmd/zedagent/handlemetrics.go

@@ -1104,16 +1104,18 @@ func PublishAppInfoToZedCloud(ctx *zedagentContext, uuid string,
 		for _, ifname := range ifNames {
 			networkInfo := new(info.ZInfoNetwork)
 			networkInfo.LocalName = *proto.String(ifname)
-			ipv4Addr, ipv6Addrs, allocated, macAddr, ipAddrMismatch :=
-				getAppIP(ctx, aiStatus, ifname)
-			if ipv4Addr != nil {
-				networkInfo.IPAddrs = append(networkInfo.IPAddrs, ipv4Addr.String())
+			addrs, hasIPv4Addr, macAddr, ipAddrMismatch :=
+				getAppIPs(ctx, aiStatus, ifname)
+			for _, ipv4Addr := range addrs.IPv4Addrs {
+				networkInfo.IPAddrs = append(networkInfo.IPAddrs,
+					ipv4Addr.Address.String())
 			}
-			for _, ipv6Addr := range ipv6Addrs {
-				networkInfo.IPAddrs = append(networkInfo.IPAddrs, ipv6Addr.String())
+			for _, ipv6Addr := range addrs.IPv6Addrs {
+				networkInfo.IPAddrs = append(networkInfo.IPAddrs,
+					ipv6Addr.Address.String())
 			}
 			networkInfo.MacAddr = *proto.String(macAddr.String())
-			networkInfo.Ipv4Up = allocated
+			networkInfo.Ipv4Up = hasIPv4Addr
 			networkInfo.IpAddrMisMatch = ipAddrMismatch
 			name := appIfnameToName(aiStatus, ifname)
 			log.Tracef("app %s/%s localName %s devName %s",
@@ -1559,21 +1561,22 @@ func sendMetricsProtobuf(ctx *getconfigContext,
 
 // Use the ifname/vifname to find the AppNetAdapter status
 // and from there the (ip, allocated, mac) addresses for the app
-func getAppIP(ctx *zedagentContext, aiStatus *types.AppInstanceStatus,
-	vifname string) (net.IP, []net.IP, bool, net.HardwareAddr, bool) {
+func getAppIPs(ctx *zedagentContext, aiStatus *types.AppInstanceStatus,
+	vifname string) (types.AssignedAddrs, bool, net.HardwareAddr, bool) {
 
 	log.Tracef("getAppIP(%s, %s)", aiStatus.Key(), vifname)
 	for _, adapterStatus := range aiStatus.AppNetAdapters {
 		if adapterStatus.VifUsed != vifname {
 			continue
 		}
-		log.Tracef("getAppIP(%s, %s) found AppIP v4: %s, v6: %s, ipv4 assigned %v mac %s",
-			aiStatus.Key(), vifname, adapterStatus.AllocatedIPv4Addr,
-			adapterStatus.AllocatedIPv6List, adapterStatus.IPv4Assigned, adapterStatus.Mac)
-		return adapterStatus.AllocatedIPv4Addr, adapterStatus.AllocatedIPv6List, adapterStatus.IPv4Assigned,
+		log.Tracef("getAppIP(%s, %s) found AppIPs v4: %v, v6: %v, ipv4 assigned %v mac %s",
+			aiStatus.Key(), vifname, adapterStatus.AssignedAddresses.IPv4Addrs,
+			adapterStatus.AssignedAddresses.IPv6Addrs, adapterStatus.IPv4Assigned,
+			adapterStatus.Mac)
+		return adapterStatus.AssignedAddresses, adapterStatus.IPv4Assigned,
 			adapterStatus.Mac, adapterStatus.IPAddrMisMatch
 	}
-	return nil, nil, false, nil, false
+	return types.AssignedAddrs{}, false, nil, false
 }
 
 func createVolumeInstanceMetrics(ctx *zedagentContext, reportMetrics *metrics.ZMetricMsg) {

pkg/pillar/cmd/zedagent/handlenetworkinstance.go

@@ -8,7 +8,6 @@ package zedagent
 import (
 	"bytes"
 	"fmt"
-	"net"
 	"time"
 
 	"github.com/golang/protobuf/ptypes/timestamp"
@@ -108,11 +107,13 @@ func prepareAndPublishNetworkInstanceInfoMsg(ctx *zedagentContext,
 		for mac, addrs := range status.IPAssignments {
 			assignment := new(zinfo.ZmetIPAssignmentEntry)
 			assignment.MacAddress = mac
-			if !addrs.IPv4Addr.Equal(net.IP{}) {
-				assignment.IpAddress = append(assignment.IpAddress, addrs.IPv4Addr.String())
+			for _, assignedIP := range addrs.IPv4Addrs {
+				assignment.IpAddress = append(assignment.IpAddress,
+					assignedIP.Address.String())
 			}
-			for _, ip := range addrs.IPv6Addrs {
-				assignment.IpAddress = append(assignment.IpAddress, ip.String())
+			for _, assignedIP := range addrs.IPv6Addrs {
+				assignment.IpAddress = append(assignment.IpAddress,
+					assignedIP.Address.String())
 			}
 			info.IpAssignments = append(info.IpAssignments, assignment)
 		}

pkg/pillar/cmd/zedrouter/appnetwork.go

@@ -62,7 +62,8 @@ func (z *zedrouter) updateVIFsForStateCollecting(
 		}
 		_, vifs, err := z.getArgsForNIStateCollecting(network)
 		if err == nil {
-			err = z.niStateCollector.UpdateCollectingForNI(*netConfig, vifs)
+			err = z.niStateCollector.UpdateCollectingForNI(*netConfig, vifs,
+				z.enableArpSnooping)
 		}
 		if err != nil {
 			z.log.Error(err)
@@ -167,10 +168,7 @@ func (z *zedrouter) updateNIStatusAfterAppNetworkActivate(status *types.AppNetwo
 		netInstStatus.AddVif(z.log, adapterStatus.Vif, adapterStatus.Mac,
 			status.UUIDandVersion.UUID)
 		netInstStatus.IPAssignments[adapterStatus.Mac.String()] =
-			types.AssignedAddrs{
-				IPv4Addr:  adapterStatus.AllocatedIPv4Addr,
-				IPv6Addrs: adapterStatus.AllocatedIPv6List,
-			}
+			adapterStatus.AssignedAddresses
 		z.publishNetworkInstanceStatus(netInstStatus)
 	}
 }

pkg/pillar/cmd/zedrouter/ipam.go

@@ -120,13 +120,15 @@ func (z *zedrouter) lookupOrAllocateIPv4ForVIF(niStatus *types.NetworkInstanceSt
 	// Lookup to see if it is already allocated.
 	if ipAddr == nil {
 		addrs := niStatus.IPAssignments[adapterStatus.Mac.String()]
-		if !isEmptyIP(addrs.IPv4Addr) {
-			z.log.Functionf("lookupOrAllocateIPv4(NI:%v, app:%v): found IP %v for MAC %v",
-				networkID, appID, addrs.IPv4Addr, adapterStatus.Mac)
-			ipAddr = addrs.IPv4Addr
+		ipAddr = addrs.GetInternallyLeasedIPv4Addr()
+		if ipAddr != nil {
+			z.log.Functionf("lookupOrAllocateIPv4(NI:%v, app:%v): "+
+				"found EVE-allocated IP %v for MAC %v", networkID, appID, ipAddr,
+				adapterStatus.Mac)
 		}
 	}
 
+	var newlyAllocated bool
 	if ipAddr == nil {
 		// Allocate IP address dynamically.
 		// Get the app number for the AppNetAdapter entry.
@@ -140,6 +142,7 @@ func (z *zedrouter) lookupOrAllocateIPv4ForVIF(niStatus *types.NetworkInstanceSt
 		}
 		// Pick an IP address from the subnet.
 		ipAddr = netutils.AddToIP(niStatus.DhcpRange.Start, appNum)
+		newlyAllocated = true
 		// Check if the address falls into the Dhcp Range.
 		if !niStatus.DhcpRange.Contains(ipAddr) {
 			err := fmt.Errorf("no free IP addresses in DHCP range(%v, %v)",
@@ -162,12 +165,19 @@ func (z *zedrouter) lookupOrAllocateIPv4ForVIF(niStatus *types.NetworkInstanceSt
 		}
 	}
 	// Later will be overwritten with addresses received from nistate.Collector,
-	// which snoops DHCP traffic and watches DNS server leases to learn the *actual*
+	// which snoops DHCP traffic and watches DHCP server leases to learn the *actual*
 	// IP address assignments.
-	addrs := niStatus.IPAssignments[adapterStatus.Mac.String()] // preserve IPv6 addresses
-	addrs.IPv4Addr = ipAddr
-	niStatus.IPAssignments[adapterStatus.Mac.String()] = addrs
-	z.publishNetworkInstanceStatus(niStatus)
+	if newlyAllocated {
+		// Preserve other IPv4 and IPv6 addresses.
+		addrs := niStatus.IPAssignments[adapterStatus.Mac.String()]
+		addrs.IPv4Addrs = append(addrs.IPv4Addrs,
+			types.AssignedAddr{
+				Address:    ipAddr,
+				AssignedBy: types.AddressSourceInternalDHCP,
+			})
+		niStatus.IPAssignments[adapterStatus.Mac.String()] = addrs
+		z.publishNetworkInstanceStatus(niStatus)
+	}
 	z.log.Functionf("lookupOrAllocateIPv4(NI:%v, app:%v): allocated IP %v for MAC %v",
 		networkID, appID, ipAddr, adapterStatus.Mac)
 	return ipAddr, nil
@@ -188,16 +198,16 @@ func (z *zedrouter) recordAssignedIPsToAdapterStatus(adapter *types.AppNetAdapte
 		z.removeAssignedIPsFromAdapterStatus(adapter)
 		return
 	}
-	adapter.AllocatedIPv4Addr = vifAddrs.IPv4Addr
-	if !isEmptyIP(adapter.AppIPAddr) &&
-		!adapter.AppIPAddr.Equal(adapter.AllocatedIPv4Addr) {
-		// Config and status do not match.
-		adapter.IPAddrMisMatch = true
-	} else {
-		adapter.IPAddrMisMatch = false
+	adapter.AssignedAddresses = vifAddrs.AssignedAddrs
+	adapter.IPAddrMisMatch = false
+	if !isEmptyIP(adapter.AppIPAddr) {
+		leasedIP := adapter.AssignedAddresses.GetInternallyLeasedIPv4Addr()
+		if !adapter.AppIPAddr.Equal(leasedIP) {
+			// Config and status do not match.
+			adapter.IPAddrMisMatch = true
+		}
 	}
-	adapter.AllocatedIPv6List = vifAddrs.IPv6Addrs
-	adapter.IPv4Assigned = !isEmptyIP(vifAddrs.IPv4Addr)
+	adapter.IPv4Assigned = len(vifAddrs.IPv4Addrs) > 0
 }
 
 func (z *zedrouter) removeAssignedIPsFromAppNetStatus(status *types.AppNetworkStatus) {
@@ -208,8 +218,8 @@ func (z *zedrouter) removeAssignedIPsFromAppNetStatus(status *types.AppNetworkSt
 }
 
 func (z *zedrouter) removeAssignedIPsFromAdapterStatus(adapterStatus *types.AppNetAdapterStatus) {
-	adapterStatus.AllocatedIPv6List = nil
-	adapterStatus.AllocatedIPv4Addr = nil
+	adapterStatus.AssignedAddresses.IPv4Addrs = nil
+	adapterStatus.AssignedAddresses.IPv6Addrs = nil
 	adapterStatus.IPAddrMisMatch = false
 	adapterStatus.IPv4Assigned = false
 }

pkg/pillar/cmd/zedrouter/networkinstance.go

@@ -230,7 +230,7 @@ func (z *zedrouter) doUpdateActivatedNetworkInstance(config types.NetworkInstanc
 	z.processNIReconcileStatus(niRecStatus, status)
 	_, vifs, err := z.getArgsForNIStateCollecting(config.UUID)
 	if err == nil {
-		err = z.niStateCollector.UpdateCollectingForNI(config, vifs)
+		err = z.niStateCollector.UpdateCollectingForNI(config, vifs, z.enableArpSnooping)
 	}
 	if err != nil {
 		z.log.Error(err)

pkg/pillar/cmd/zedrouter/pubsubhandlers.go

@@ -50,7 +50,33 @@ func (z *zedrouter) handleGlobalConfigImpl(ctxArg interface{}, key string,
 			}
 			z.metricInterval = metricInterval
 		}
-		z.enableArpSnooping = gcp.GlobalValueBool(types.EnableARPSnoop)
+		enableArpSnooping := gcp.GlobalValueBool(types.EnableARPSnoop)
+		if z.enableArpSnooping != enableArpSnooping {
+			z.enableArpSnooping = enableArpSnooping
+			// Start/Stop ARP snooping in every activated Switch NI.
+			for _, item := range z.pubNetworkInstanceStatus.GetAll() {
+				niStatus := item.(types.NetworkInstanceStatus)
+				if !niStatus.Activated {
+					continue
+				}
+				if niStatus.Type != types.NetworkInstanceTypeSwitch {
+					// ARP snooping is only used in Switch NIs.
+					continue
+				}
+				niConfig := z.lookupNetworkInstanceConfig(niStatus.Key())
+				if niConfig == nil {
+					continue
+				}
+				_, vifs, err := z.getArgsForNIStateCollecting(niConfig.UUID)
+				if err == nil {
+					err = z.niStateCollector.UpdateCollectingForNI(
+						*niConfig, vifs, z.enableArpSnooping)
+				}
+				if err != nil {
+					z.log.Error(err)
+				}
+			}
+		}
 		z.localLegacyMACAddr = gcp.GlobalValueBool(types.NetworkLocalLegacyMACAddress)
 		z.niReconciler.ApplyUpdatedGCP(z.runCtx, *gcp)
 	}
@@ -222,7 +248,13 @@ func (z *zedrouter) handleNetworkInstanceCreate(ctxArg interface{}, key string,
 
 	// Set bridge IP address.
 	if status.Gateway != nil {
-		addrs := types.AssignedAddrs{IPv4Addr: status.Gateway}
+		addrs := types.AssignedAddrs{
+			IPv4Addrs: []types.AssignedAddr{
+				{
+					Address:    status.Gateway,
+					AssignedBy: types.AddressSourceEVEInternal,
+				}},
+		}
 		status.IPAssignments[status.BridgeMac.String()] = addrs
 		status.BridgeIPAddr = status.Gateway
 	}
@@ -358,8 +390,14 @@ func (z *zedrouter) handleNetworkInstanceModify(ctxArg interface{}, key string,
 	if status.BridgeMac != nil {
 		delete(status.IPAssignments, status.BridgeMac.String())
 	}
-	if status.Gateway != nil {
-		addrs := types.AssignedAddrs{IPv4Addr: status.Gateway}
+	if status.Gateway != nil && status.BridgeMac != nil {
+		addrs := types.AssignedAddrs{
+			IPv4Addrs: []types.AssignedAddr{
+				{
+					Address:    status.Gateway,
+					AssignedBy: types.AddressSourceEVEInternal,
+				}},
+		}
 		status.IPAssignments[status.BridgeMac.String()] = addrs
 		status.BridgeIPAddr = status.Gateway
 	}

pkg/pillar/cmd/zedrouter/zedrouter.go

@@ -459,10 +459,7 @@ func (z *zedrouter) run(ctx context.Context) (err error) {
 						niKey, vif.NetAdapterName)
 					continue
 				}
-				netStatus.IPAssignments[mac] = types.AssignedAddrs{
-					IPv4Addr:  newAddrs.IPv4Addr,
-					IPv6Addrs: newAddrs.IPv6Addrs,
-				}
+				netStatus.IPAssignments[mac] = newAddrs.AssignedAddrs
 				z.publishNetworkInstanceStatus(netStatus)
 				appKey := vif.App.String()
 				appStatus := z.lookupAppNetworkStatus(appKey)
@@ -1000,11 +997,13 @@ func (z *zedrouter) lookupNetworkInstanceStatusByAppIP(
 	for _, st := range items {
 		status := st.(types.NetworkInstanceStatus)
 		for _, addrs := range status.IPAssignments {
-			if ip.Equal(addrs.IPv4Addr) {
-				return &status
+			for _, assignedIP := range addrs.IPv4Addrs {
+				if ip.Equal(assignedIP.Address) {
+					return &status
+				}
 			}
-			for _, nip := range addrs.IPv6Addrs {
-				if ip.Equal(nip) {
+			for _, assignedIP := range addrs.IPv6Addrs {
+				if ip.Equal(assignedIP.Address) {
 					return &status
 				}
 			}
@@ -1187,8 +1186,15 @@ func (z *zedrouter) lookupAppNetworkStatusByAppIP(ip net.IP) *types.AppNetworkSt
 	for _, st := range items {
 		status := st.(types.AppNetworkStatus)
 		for _, adapterStatus := range status.AppNetAdapterList {
-			if adapterStatus.AllocatedIPv4Addr.Equal(ip) {
-				return &status
+			for _, adapterIP := range adapterStatus.AssignedAddresses.IPv4Addrs {
+				if adapterIP.Address.Equal(ip) {
+					return &status
+				}
+			}
+			for _, adapterIP := range adapterStatus.AssignedAddresses.IPv6Addrs {
+				if adapterIP.Address.Equal(ip) {
+					return &status
+				}
 			}
 		}
 	}