domainmgr: match device ports by PCI, not just interface name

pkg/pillar/cmd/domainmgr/domainmgr.go

@@ -99,7 +99,7 @@ var currentTTY = 0
 func isPort(ctx *domainContext, ifname string) bool {
 	ctx.dnsLock.Lock()
 	defer ctx.dnsLock.Unlock()
-	return types.IsPort(ctx.deviceNetworkStatus, ifname)
+	return types.IsPort(ctx.deviceNetworkStatus, ifname, "")
 }
 
 // Information for handleCreate/Modify/Delete
@@ -3573,11 +3573,11 @@ func handlePhysicalIOAdapterListImpl(ctxArg interface{}, key string,
 			ib := types.IoBundleFromPhyAdapter(log, phyAdapter)
 			// Fill in PCIlong, macaddr, unique
 			_, err := checkAndFillIoBundle(ib)
+			var missing []string
 			if err != nil {
-				ib.Error.Append(err)
-			} else {
-				ib.Error.Clear()
+				missing = []string{err.Error()}
 			}
+			ib.Error.SetSourceErrors(types.ErrIoBundleMissingDevice{}, false, false, missing)
 			// We assume AddOrUpdateIoBundle will preserve any
 			// existing IsPort/IsPCIBack/UsedByUUID
 			aa.AddOrUpdateIoBundle(log, *ib)
@@ -3643,11 +3643,11 @@ func handlePhysicalIOAdapterListImpl(ctxArg interface{}, key string,
 		ib := types.IoBundleFromPhyAdapter(log, phyAdapter)
 		// Fill in PCIlong, macaddr, unique
 		_, err := checkAndFillIoBundle(ib)
+		var missing []string
 		if err != nil {
-			ib.Error.Append(err)
-		} else {
-			ib.Error.Clear()
+			missing = []string{err.Error()}
 		}
+		ib.Error.SetSourceErrors(types.ErrIoBundleMissingDevice{}, false, false, missing)
 		currentIbPtr := aa.LookupIoBundlePhylabel(phyAdapter.Phylabel)
 		if currentIbPtr == nil || currentIbPtr.HasAdapterChanged(log, phyAdapter) {
 
@@ -3850,6 +3850,17 @@ func updatePortAndPciBackIoBundle(ctx *domainContext, ib *types.IoBundle) (chang
 		list = append(list, ib)
 	}
 
+	// The group being processed, and the members the controller's model actually
+	// listed in it, so members EVE adds via ExpandControllers can be flagged.
+	reqGroup := ib.AssignmentGroup
+	origGroup := make(map[*types.IoBundle]bool, len(list))
+	for _, m := range list {
+		origGroup[m] = true
+	}
+	// Per-bundle advisories for model inconsistencies worked around below;
+	// recorded as warnings on ib.Error so the controller is informed.
+	modelWarnings := map[*types.IoBundle][]string{}
+
 	keepInHostUsbControllers := usbControllersWithoutPCIReserve(ctx.assignableAdapters.IoBundleList)
 
 	// Is any member a network port?
@@ -3866,9 +3877,42 @@ 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) && ib.Type.IsNet() {
+		if !origGroup[ib] {
+			// EVE pulled this member into the group (ExpandControllers) because
+			// it shares a PCI controller with a member the controller listed.
+			modelWarnings[ib] = append(modelWarnings[ib], fmt.Sprintf(
+				"adapter %s (logicallabel %s, ifname %q, PCI %s) was added to "+
+					"assignment group %q because it shares a PCI controller with a "+
+					"group member, though the controller's model did not list it there",
+				ib.Phylabel, ib.Logicallabel, ib.Ifname, ib.PciLong, reqGroup))
+		}
+		switch {
+		case ib.Type.IsNet() &&
+			types.IsPort(ctx.deviceNetworkStatus, ib.Ifname, ib.PciLong):
+			// Match by PCI as well as ifname: recognize an in-use port even
+			// when the kernel name differs from the model (ethN vs enpNsN), so
+			// it is not wrongly reserved to pciback.
 			isPort = true
 			keepInHost = true
+			if ib.PciLong != "" &&
+				!types.IsPort(ctx.deviceNetworkStatus, ib.Ifname, "") {
+				// Matched by PCI only — model ifname doesn't match the kernel.
+				modelWarnings[ib] = append(modelWarnings[ib], fmt.Sprintf(
+					"adapter %s (logicallabel %s, model ifname %q, PCI %s) does not "+
+						"match the kernel-assigned interface name; matched to the "+
+						"in-use network port by PCI address and kept in the host",
+					ib.Phylabel, ib.Logicallabel, ib.Ifname, ib.PciLong))
+			}
+		case ib.PciLong != "" && !ib.Type.IsNet() &&
+			types.IsPort(ctx.deviceNetworkStatus, "", ib.PciLong):
+			// Model types this device as non-network, but its PCI backs a live
+			// network port; keep it in the host rather than reserve to pciback.
+			keepInHost = true
+			modelWarnings[ib] = append(modelWarnings[ib], fmt.Sprintf(
+				"adapter %s (logicallabel %s, ifname %q, PCI %s) is modeled as "+
+					"non-network type %d but that PCI address is in use as a network "+
+					"port; kept in the host instead of assigning it to pciback",
+				ib.Phylabel, ib.Logicallabel, ib.Ifname, ib.PciLong, ib.Type))
 		}
 		if ib.Type == types.IoNetWLAN || ib.Type == types.IoNetWWAN {
 			// Do not put unused wireless devices (unassigned and not associated with any network) into pciback,
@@ -3916,11 +3960,17 @@ func updatePortAndPciBackIoBundle(ctx *domainContext, ib *types.IoBundle) (chang
 		}
 		changed, err := updatePortAndPciBackIoMember(ctx, ib, isPort, keepInHost)
 		anyChanged = anyChanged || changed
+		// Reconcile only this loop's own error sources; leave others
+		// (collision, assignment-group, missing-device, rename) to their owners.
+		var pcibackErr []string
 		if err != nil {
-			ib.Error.Append(err)
+			pcibackErr = []string{err.Error()}
 			log.Error(err)
-		} else {
-			ib.Error.Clear()
+		}
+		ib.Error.SetSourceErrors(types.ErrIoBundlePcibackOp{}, false, false, pcibackErr)
+		ib.Error.SetSourceErrors(types.ErrIoBundleModelInconsistency{}, true, false, modelWarnings[ib])
+		for _, w := range modelWarnings[ib] {
+			log.Warn(w)
 		}
 	}
 	return anyChanged

pkg/pillar/cmd/domainmgr/domainmgr_test.go

@@ -652,3 +652,52 @@ func TestConfigEnableUsbUpdatePortAndPciBackIoBundle(t *testing.T) {
 		}
 	}
 }
+
+// TestMistypedNetworkPortKeptInHost covers a device model that assigns a device
+// a non-network type (here a GPU) even though its PCI address is in fact backing
+// a network port. The device must be kept in the host rather than reserved to
+// pciback, which would unbind the live port.
+func TestMistypedNetworkPortKeptInHost(t *testing.T) {
+	const pciLong = "0000:06:00.0"
+	assignableAdapters := types.AssignableAdapters{
+		IoBundleList: []types.IoBundle{
+			{
+				Phylabel:        "mislabeled",
+				Logicallabel:    "mislabeled",
+				Type:            types.IoHDMI,
+				AssignmentGroup: "1",
+				PciLong:         pciLong,
+			},
+		},
+	}
+	ctx := &domainContext{
+		assignableAdapters: &assignableAdapters,
+		deviceNetworkStatus: types.DeviceNetworkStatus{
+			Ports: []types.NetworkPortStatus{
+				{IfName: "eth0", PciLong: pciLong},
+			},
+		},
+	}
+	ib := &types.IoBundle{AssignmentGroup: "1"}
+
+	updatePortAndPciBackIoBundle(ctx, ib)
+
+	for _, b := range ctx.assignableAdapters.IoBundleList {
+		if b.Phylabel != "mislabeled" {
+			continue
+		}
+		if !b.KeepInHost {
+			t.Fatalf("IoBundle %+v should be kept in host: its PCI address is in "+
+				"use as a network port", b)
+		}
+		// The model/hardware inconsistency must be reported to the controller as
+		// an advisory warning (not a hard error).
+		if b.Error.Empty() || !b.Error.IsOnlyWarnings() {
+			t.Fatalf("IoBundle %+v should carry a warning about the type mismatch, got %q",
+				b, b.Error.String())
+		}
+		if !strings.Contains(b.Error.String(), pciLong) {
+			t.Fatalf("warning should identify the PCI address %s, got %q", pciLong, b.Error.String())
+		}
+	}
+}

pkg/pillar/cmd/zedagent/reportinfo.go

@@ -535,12 +535,18 @@ func PublishDeviceInfoToZedCloud(ctx *zedagentContext, dest destinationBitset) {
 		} else if ib.KeepInHost {
 			reportAA.UsedByBaseOS = true
 		}
-		if !ib.Error.Empty() {
+		// Combine all group members' errors into the single ZioBundle error slot.
+		aggErr := types.AggregateIoBundleGroupErrors(list)
+		if !aggErr.Empty {
 			errInfo := new(info.ErrorInfo)
-			errInfo.Description = ib.Error.String()
-			errInfo.Severity = info.Severity_SEVERITY_ERROR
-			if !ib.Error.ErrorTime().IsZero() {
-				errInfo.Timestamp = timestamppb.New(ib.Error.ErrorTime())
+			errInfo.Description = aggErr.Description
+			if aggErr.OnlyWarnings {
+				errInfo.Severity = info.Severity_SEVERITY_WARNING
+			} else {
+				errInfo.Severity = info.Severity_SEVERITY_ERROR
+			}
+			if !aggErr.ErrorTime.IsZero() {
+				errInfo.Timestamp = timestamppb.New(aggErr.ErrorTime)
 			}
 			reportAA.Err = errInfo
 		}
@@ -893,6 +899,11 @@ func encodeNetworkPortStatus(ctx *zedagentContext,
 	devicePort.Err = encodeTestResults(port.TestResults)
 	if ioBundle != nil {
 		devicePort.Usage = ioBundle.Usage
+		// A port that is also an assignable adapter can carry model /
+		// pciback errors on its IoBundle in addition to any connectivity
+		// error from network testing. Fold those into the port error so
+		// they surface in the port view, not only under assignableAdapters.
+		mergeIoBundleErrIntoPort(devicePort.Err, ioBundle)
 	}
 	devicePort.Cost = uint32(port.Cost)
 	devicePort.IsMgmt = port.IsMgmt
@@ -1008,6 +1019,39 @@ func encodeNetworkPortStatus(ctx *zedagentContext,
 	return devicePort
 }
 
+// mergeIoBundleErrIntoPort folds an assignable-adapter port's IoBundle error
+// (model inconsistency, rename, pciback op, ...) into the port ErrorInfo, which
+// otherwise carries only the network TestResults error. Descriptions are
+// concatenated and the reported severity is the more severe of the two, so a
+// model error is never masked by a healthy network result and vice versa.
+func mergeIoBundleErrIntoPort(errInfo *info.ErrorInfo, ioBundle *types.IoBundle) {
+	aggErr := types.AggregateIoBundleGroupErrors([]*types.IoBundle{ioBundle})
+	if aggErr.Empty {
+		return
+	}
+	bundleSeverity := info.Severity_SEVERITY_ERROR
+	if aggErr.OnlyWarnings {
+		bundleSeverity = info.Severity_SEVERITY_WARNING
+	}
+	if errInfo.Description == "" {
+		// Port carries no network error; adopt the IoBundle error wholesale.
+		errInfo.Description = aggErr.Description
+		errInfo.Severity = bundleSeverity
+		if !aggErr.ErrorTime.IsZero() {
+			errInfo.Timestamp = timestamppb.New(aggErr.ErrorTime)
+		}
+		return
+	}
+	errInfo.Description += "; " + aggErr.Description
+	if errInfo.Severity < bundleSeverity {
+		errInfo.Severity = bundleSeverity
+	}
+	if !aggErr.ErrorTime.IsZero() &&
+		(errInfo.Timestamp == nil || aggErr.ErrorTime.After(errInfo.Timestamp.AsTime())) {
+		errInfo.Timestamp = timestamppb.New(aggErr.ErrorTime)
+	}
+}
+
 func encodeBondStatus(bs *types.BondStatus) *info.BondStatus {
 	pbBond := &info.BondStatus{
 		Mode:         evecommon.BondMode(bs.Mode),

pkg/pillar/dpcmanager/dns.go

@@ -51,6 +51,12 @@ func (m *DpcManager) updateDNS() {
 		m.deviceNetStatus.Ports[ix].IfName = port.IfName
 		m.deviceNetStatus.Ports[ix].Phylabel = port.Phylabel
 		m.deviceNetStatus.Ports[ix].Logicallabel = port.Logicallabel
+		// Record the PCI address backing this port so consumers (e.g. domainmgr
+		// deciding whether to reserve a device to pciback) can match a port to
+		// a physical device by stable PCI identity rather than by the
+		// kernel-assigned interface name. Seed from the config; the live value
+		// resolved from the interface below takes precedence when available.
+		m.deviceNetStatus.Ports[ix].PciLong = port.PCIAddr
 		m.deviceNetStatus.Ports[ix].SharedLabels = port.SharedLabels
 		m.deviceNetStatus.Ports[ix].Alias = port.Alias
 		m.deviceNetStatus.Ports[ix].IsMgmt = port.IsMgmt
@@ -90,6 +96,11 @@ func (m *DpcManager) updateDNS() {
 			wwanNetStatus := m.wwanStatus.GetNetworkStatus(port.Logicallabel)
 			if wwanNetStatus != nil {
 				m.deviceNetStatus.Ports[ix].WirelessStatus.Cellular = *wwanNetStatus
+				// A modem's PCI can't be derived from its interface; use the
+				// address the wwan microservice resolved.
+				if wwanNetStatus.PhysAddrs.PCI != "" {
+					m.deviceNetStatus.Ports[ix].PciLong = wwanNetStatus.PhysAddrs.PCI
+				}
 			}
 		}
 		// Do not try to get state data for interface which is in PCIback.
@@ -121,6 +132,14 @@ func (m *DpcManager) updateDNS() {
 			}
 			continue
 		}
+		// Prefer the PCI resolved from the live interface: it reflects the
+		// device actually backing the port, regardless of the model's ifname.
+		// Cellular is excluded (see above; IfNameToPci can't resolve a modem).
+		if port.WirelessCfg.WType != types.WirelessTypeCellular {
+			if pciLong, _, err := types.IfNameToPciAndUsbAddr(m.Log, port.IfName); err == nil {
+				m.deviceNetStatus.Ports[ix].PciLong = pciLong
+			}
+		}
 		ifAttrs, err := m.NetworkMonitor.GetInterfaceAttrs(ifindex)
 		if err != nil {
 			m.Log.Warnf(

pkg/pillar/dpcmanager/dpcmanager_test.go

@@ -964,6 +964,83 @@ func makeAA(intfs selectedIntfs) types.AssignableAdapters {
 	return aa
 }
 
+// TestPortPciLongInDNS verifies that a port's PCI address is recorded in the
+// device network status. The interface name is chosen so it cannot resolve to
+// a real PCI device on the test host, which exercises the fallback to the PCI
+// address carried in the port config. This PCI identity is what lets domainmgr
+// recognize a live port even when its kernel-assigned name differs from the
+// model name, instead of reserving the device to pciback.
+func TestPortPciLongInDNS(test *testing.T) {
+	t := initTest(test)
+
+	const ifname = "nictest0"
+	const pciAddr = "0000:06:00.0"
+
+	mockIf := netmonitor.MockInterface{
+		Attrs: netmonitor.IfAttrs{
+			IfIndex:       42,
+			IfName:        ifname,
+			IfType:        "device",
+			WithBroadcast: true,
+			AdminUp:       true,
+			LowerUp:       true,
+		},
+		IPAddrs: []*net.IPNet{ipAddress("192.168.77.5/24")},
+		HwAddr:  macAddress("02:00:00:00:00:77"),
+	}
+	networkMonitor.AddOrUpdateInterface(mockIf)
+
+	dpcManager.UpdateGCP(globalConfig())
+
+	aa := types.AssignableAdapters{
+		Initialized: true,
+		IoBundleList: []types.IoBundle{
+			{
+				Type:         types.IoNetEth,
+				Phylabel:     ifname,
+				Logicallabel: "mock-nic",
+				Usage:        evecommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps,
+				Ifname:       ifname,
+				PciLong:      pciAddr,
+				MacAddr:      mockIf.HwAddr.String(),
+				IsPort:       true,
+			},
+		},
+	}
+	dpc := types.DevicePortConfig{
+		Version:      types.DPCIsMgmt,
+		Key:          "zedagent",
+		TimePriority: time.Now(),
+		Ports: []types.NetworkPortConfig{
+			{
+				IfName:       ifname,
+				Phylabel:     ifname,
+				Logicallabel: "mock-nic",
+				PCIAddr:      pciAddr,
+				IsMgmt:       true,
+				IsL3Port:     true,
+				DhcpConfig: types.DhcpConfig{
+					Dhcp: types.DhcpTypeClient,
+					Type: types.NetworkTypeIPv4,
+				},
+				ConfigSource: types.PortConfigSource{
+					Origin: types.NetworkConfigOriginController,
+				},
+			},
+		},
+	}
+	dpcManager.UpdateAA(aa)
+	dpcManager.AddDPC(dpc)
+
+	t.Eventually(func() string {
+		dns := getDNS()
+		if len(dns.Ports) == 0 {
+			return ""
+		}
+		return dns.Ports[0].PciLong
+	}).Should(Equal(pciAddr))
+}
+
 func TestSingleDPC(test *testing.T) {
 	t := initTest(test)
 	t.Expect(dpcManager.GetDNS().DPCKey).To(BeEmpty())