ioBundle->NetworkPortStatus matching tightening

Check all three identifiers: ifname, usbaddr, pciaddr Some io types can have an empty string in one of these fields. Expand matching conditions to ensure ioBundles are not incorrectly matched to a different NetworkPort. Signed-off-by: Andrew Durbin <[email protected]>
lf-edge · Dec 10, 2024 · a68769d · a68769d
1 parent e872c81
commit a68769d
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 32 deletions.
diff --git a/pkg/pillar/cmd/diag/diag.go b/pkg/pillar/cmd/diag/diag.go
@@ -887,9 +887,9 @@ func printOutput(ctx *diagContext, caller string) {
 		// Print usefully formatted info based on which
 		// fields are set and Dhcp type; proxy info order
 		ifname := port.IfName
-		isMgmt := types.IsMgmtPort(*ctx.DeviceNetworkStatus, ifname)
+		isMgmt := types.IsMgmtPort(*ctx.DeviceNetworkStatus, ifname, port.USBAddr, port.USBProd, port.PCIAddr)
 		cost := types.GetPortCost(*ctx.DeviceNetworkStatus,
-			ifname)
+			ifname, port.USBAddr, port.USBProd, port.PCIAddr)
 		if isMgmt {
 			mgmtPorts++
 		}

diff --git a/pkg/pillar/cmd/domainmgr/domainmgr.go b/pkg/pillar/cmd/domainmgr/domainmgr.go
@@ -19,7 +19,6 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/containerd/cgroups"
@@ -68,21 +67,12 @@ const (
 // Really a constant
 var nilUUID = uuid.UUID{}
 
-func isPort(ctx *domainContext, ifname string) bool {
-	ctx.dnsLock.Lock()
-	defer ctx.dnsLock.Unlock()
-	return types.IsPort(ctx.deviceNetworkStatus, ifname)
-}
-
 // Information for handleCreate/Modify/Delete
 type domainContext struct {
 	agentbase.AgentBase
-	ps *pubsub.PubSub
-	// The isPort function is called by different goroutines
-	// hence we serialize the calls on a mutex.
+	ps                     *pubsub.PubSub
 	decryptCipherContext   cipher.DecryptCipherContext
 	deviceNetworkStatus    types.DeviceNetworkStatus
-	dnsLock                sync.Mutex
 	assignableAdapters     *types.AssignableAdapters
 	DNSinitialized         bool // Received DeviceNetworkStatus
 	subDeviceNetworkStatus pubsub.Subscription
@@ -3154,7 +3144,7 @@ func updatePortAndPciBackIoBundle(ctx *domainContext, ib *types.IoBundle) (chang
 	// EVE controller doesn't know it
 	list = aa.ExpandControllers(log, list, hyper.PCISameController)
 	for _, ib := range list {
-		if types.IsPort(ctx.deviceNetworkStatus, ib.Ifname) {
+		if types.IsPort(ctx.deviceNetworkStatus, ib.Ifname, ib.UsbAddr, ib.UsbProduct, ib.PciLong) {
 			isPort = true
 			keepInHost = true
 		}

diff --git a/pkg/pillar/cmd/wstunnelclient/wstunnelclient.go b/pkg/pillar/cmd/wstunnelclient/wstunnelclient.go
@@ -387,7 +387,7 @@ func scanAIConfigs(ctx *wstunnelclientContext) {
 	deviceNetworkStatus := ctx.dnsContext.deviceNetworkStatus
 	for _, port := range deviceNetworkStatus.Ports {
 		ifname := port.IfName
-		if !types.IsMgmtPort(*deviceNetworkStatus, ifname) {
+		if !types.IsMgmtPort(*deviceNetworkStatus, ifname, port.USBAddr, port.USBProd, port.PCIAddr) {
 			log.Tracef("Skipping connection using non-mangement intf %s\n",
 				ifname)
 			continue

diff --git a/pkg/pillar/cmd/zedagent/parseconfig.go b/pkg/pillar/cmd/zedagent/parseconfig.go
@@ -1138,6 +1138,7 @@ func propagatePhyioAttrsToPort(port *types.NetworkPortConfig, phyio *types.Physi
 	port.Phylabel = phyio.Phylabel
 	port.IfName = phyio.Phyaddr.Ifname
 	port.USBAddr = phyio.Phyaddr.UsbAddr
+	port.USBProd = phyio.Phyaddr.UsbProduct
 	port.PCIAddr = phyio.Phyaddr.PciLong
 	if port.IfName == "" {
 		// Inside device model, network adapter may be referenced by PCI or USB address

diff --git a/pkg/pillar/dpcmanager/dns.go b/pkg/pillar/dpcmanager/dns.go
@@ -47,6 +47,9 @@ func (m *DpcManager) updateDNS() {
 	m.deviceNetStatus.Ports = make([]types.NetworkPortStatus, len(dpc.Ports))
 	for ix, port := range dpc.Ports {
 		m.deviceNetStatus.Ports[ix].IfName = port.IfName
+		m.deviceNetStatus.Ports[ix].USBAddr = port.USBAddr
+		m.deviceNetStatus.Ports[ix].USBProd = port.USBProd
+		m.deviceNetStatus.Ports[ix].PCIAddr = port.PCIAddr
 		m.deviceNetStatus.Ports[ix].Phylabel = port.Phylabel
 		m.deviceNetStatus.Ports[ix].Logicallabel = port.Logicallabel
 		m.deviceNetStatus.Ports[ix].SharedLabels = port.SharedLabels

diff --git a/pkg/pillar/types/dns.go b/pkg/pillar/types/dns.go
@@ -31,6 +31,9 @@ type DeviceNetworkStatus struct {
 
 type NetworkPortStatus struct {
 	IfName       string
+	USBAddr      string
+	USBProd      string
+	PCIAddr      string
 	Phylabel     string // Physical name set by controller/model
 	Logicallabel string
 	// Unlike the logicallabel, which is defined in the device model and unique
@@ -682,31 +685,38 @@ func getLocalAddrListImpl(dns DeviceNetworkStatus,
 }
 
 // Check if an interface name is a port owned by nim
-func IsPort(dns DeviceNetworkStatus, ifname string) bool {
+func IsPort(dns DeviceNetworkStatus, ifname string, usbaddr string, usbprod string, pciaddr string) bool {
 	for _, us := range dns.Ports {
-		if us.IfName != ifname {
+		if ifname != "" && us.IfName != ifname {
 			continue
 		}
-		return true
-	}
-	return false
-}
-
-// IsL3Port checks if an interface name belongs to a port with SystemAdapter attached.
-func IsL3Port(dns DeviceNetworkStatus, ifname string) bool {
-	for _, us := range dns.Ports {
-		if us.IfName != ifname {
+		if us.USBAddr != usbaddr {
+			continue
+		}
+		if us.USBProd != usbprod {
 			continue
 		}
-		return us.IsL3Port
+		if us.PCIAddr != pciaddr {
+			continue
+		}
+		return true
 	}
 	return false
 }
 
 // Check if a physical label or ifname is a management port
-func IsMgmtPort(dns DeviceNetworkStatus, ifname string) bool {
+func IsMgmtPort(dns DeviceNetworkStatus, ifname string, usbaddr string, usbprod string, pciaddr string) bool {
 	for _, us := range dns.Ports {
-		if us.IfName != ifname {
+		if ifname != "" && us.IfName != ifname {
+			continue
+		}
+		if us.USBAddr != usbaddr {
+			continue
+		}
+		if us.USBProd != usbprod {
+			continue
+		}
+		if us.PCIAddr != pciaddr {
 			continue
 		}
 		if dns.Version >= DPCIsMgmt &&
@@ -720,9 +730,18 @@ func IsMgmtPort(dns DeviceNetworkStatus, ifname string) bool {
 
 // GetPortCost returns the port cost
 // Returns 0 if the ifname does not exist.
-func GetPortCost(dns DeviceNetworkStatus, ifname string) uint8 {
+func GetPortCost(dns DeviceNetworkStatus, ifname string, usbaddr string, usbprod string, pciaddr string) uint8 {
 	for _, us := range dns.Ports {
-		if us.IfName != ifname {
+		if ifname != "" && us.IfName != ifname {
+			continue
+		}
+		if us.USBAddr != usbaddr {
+			continue
+		}
+		if us.USBProd != usbprod {
+			continue
+		}
+		if us.PCIAddr != pciaddr {
 			continue
 		}
 		return us.Cost
@@ -732,7 +751,7 @@ func GetPortCost(dns DeviceNetworkStatus, ifname string) uint8 {
 
 func GetPort(dns DeviceNetworkStatus, ifname string) *NetworkPortStatus {
 	for _, us := range dns.Ports {
-		if us.IfName != ifname {
+		if ifname != "" && us.IfName != ifname {
 			continue
 		}
 		if dns.Version < DPCIsMgmt {

diff --git a/pkg/pillar/types/dpc.go b/pkg/pillar/types/dpc.go
@@ -442,6 +442,7 @@ func (config *DevicePortConfig) MostlyEqual(config2 *DevicePortConfig) bool {
 		if p1.IfName != p2.IfName ||
 			p1.PCIAddr != p2.PCIAddr ||
 			p1.USBAddr != p2.USBAddr ||
+			p1.USBProd != p2.USBProd ||
 			p1.Phylabel != p2.Phylabel ||
 			p1.Logicallabel != p2.Logicallabel ||
 			!generics.EqualSets(p1.SharedLabels, p2.SharedLabels) ||
@@ -567,6 +568,7 @@ func (config *DevicePortConfig) IsAnyPortInPciBack(
 type NetworkPortConfig struct {
 	IfName       string
 	USBAddr      string
+	USBProd      string
 	PCIAddr      string
 	Phylabel     string // Physical name set by controller/model
 	Logicallabel string // SystemAdapter's name which is logical label in phyio