From 10b62309ff455a4fc902fd52c3c7e4aa0a7cda25 Mon Sep 17 00:00:00 2001 From: Milan Lenco Date: Thu, 4 Jan 2024 17:55:03 +0100 Subject: [PATCH] Fix VLANs and LAGs Support for VLANs and LAGs has been added to EVE quite some time ago, but it has not been retested since. As we are soon planning to productize this feature, this commit contains a set of patches for regressions found while testing EVE with VLAN and LAG configurations. Patches are mostly about skipping L2-only ports and not trying to use them for connectivity tests and HTTP requests. Signed-off-by: Milan Lenco --- pkg/pillar/cmd/diag/diag.go | 5 +- pkg/pillar/cmd/domainmgr/domainmgr.go | 23 ---- pkg/pillar/cmd/zedagent/parseconfig.go | 10 ++ pkg/pillar/devicenetwork/dns.go | 2 +- pkg/pillar/dpcmanager/dns.go | 9 +- pkg/pillar/types/dns.go | 24 +++-- pkg/pillar/types/zedroutertypes_test.go | 134 ++++++++++++++++-------- pkg/pillar/zedcloud/send.go | 10 +- 8 files changed, 135 insertions(+), 82 deletions(-) diff --git a/pkg/pillar/cmd/diag/diag.go b/pkg/pillar/cmd/diag/diag.go index 3d15239f6c..3d5b3164b5 100644 --- a/pkg/pillar/cmd/diag/diag.go +++ b/pkg/pillar/cmd/diag/diag.go @@ -874,13 +874,16 @@ func printOutput(ctx *diagContext, caller string) { ctx.DeviceNetworkStatus.State.String()) } - numPorts := len(ctx.DeviceNetworkStatus.Ports) + numPorts := len(types.GetAllPortsSortedCost(*ctx.DeviceNetworkStatus, true, 0)) mgmtPorts := 0 passPorts := 0 numMgmtPorts := len(types.GetMgmtPortsAny(*ctx.DeviceNetworkStatus, 0)) ctx.ph.Print("INFO: Have %d total ports. %d ports should be connected to EV controller\n", numPorts, numMgmtPorts) for _, port := range ctx.DeviceNetworkStatus.Ports { + if !port.IsL3Port { + continue + } // Print usefully formatted info based on which // fields are set and Dhcp type; proxy info order ifname := port.IfName diff --git a/pkg/pillar/cmd/domainmgr/domainmgr.go b/pkg/pillar/cmd/domainmgr/domainmgr.go index 041dfe41d9..a525f3b77b 100644 --- a/pkg/pillar/cmd/domainmgr/domainmgr.go +++ b/pkg/pillar/cmd/domainmgr/domainmgr.go @@ -1684,29 +1684,6 @@ func doActivateTail(ctx *domainContext, status *types.DomainStatus, status.UUIDandVersion, status.DisplayName) } -// VLAN filtering could have been enabled on the bridge immediately after -// it's creation in zedrouter. For some strange reason netlink -// throws back error stating that the device is busy if enabling the vlan -// filtering is tried immediately after bridge creation. -// We can either loop retrying in zedrouter or enable it when needed in domainmgr -func enableVlanFiltering(bridgeName string) error { - bridge, err := netlink.LinkByName(bridgeName) - if err != nil { - log.Errorf("enableVlanFiltering: LinkByName failed for %s: %s", bridgeName, err) - return err - } - if !*bridge.(*netlink.Bridge).VlanFiltering { - // Enable VLAN filtering on bridge - if err := netlink.BridgeSetVlanFiltering(bridge, true); err != nil { - err = fmt.Errorf("enableVlanFiltering on %s failed: %w", - bridgeName, err) - log.Error(err) - return err - } - } - return nil -} - // shutdown and wait for the domain to go away; if that fails destroy and wait func doInactivate(ctx *domainContext, status *types.DomainStatus, impatient bool) { diff --git a/pkg/pillar/cmd/zedagent/parseconfig.go b/pkg/pillar/cmd/zedagent/parseconfig.go index 136467fff4..79a5a9fe16 100644 --- a/pkg/pillar/cmd/zedagent/parseconfig.go +++ b/pkg/pillar/cmd/zedagent/parseconfig.go @@ -1396,6 +1396,16 @@ func parseBonds(getconfigCtx *getconfigContext, config *zconfig.EdgeDevConfig) b log.Errorf("parseBonds: %s", errStr) portConfig.RecordFailure(errStr) } + // Attempt to create bond with interface name "bond0" returns "File exists", + // even if there is no such interface: + // $ ip link add bond0 type bond + // RTNETLINK answers: File exists + // This is very strange because this does not happen for example on Ubuntu. + if portConfig.IfName == "bond0" { + errStr := "interface name \"bond0\" is reserved" + log.Errorf("parseBonds: %s", errStr) + portConfig.RecordFailure(errStr) + } // bond parameters portConfig.Bond.Mode = types.BondMode(bondConfig.GetBondMode()) diff --git a/pkg/pillar/devicenetwork/dns.go b/pkg/pillar/devicenetwork/dns.go index 2ea18573d4..658074a14c 100644 --- a/pkg/pillar/devicenetwork/dns.go +++ b/pkg/pillar/devicenetwork/dns.go @@ -111,7 +111,7 @@ func ResolveWithPortsLambda(domain string, var wg sync.WaitGroup for _, port := range dns.Ports { - if port.Cost > 0 { + if !port.IsL3Port || port.Cost > 0 { continue } diff --git a/pkg/pillar/dpcmanager/dns.go b/pkg/pillar/dpcmanager/dns.go index 198e76b53d..20f2735a00 100644 --- a/pkg/pillar/dpcmanager/dns.go +++ b/pkg/pillar/dpcmanager/dns.go @@ -125,6 +125,13 @@ func (m *DpcManager) updateDNS() { ipAddrs = nil } m.deviceNetStatus.Ports[ix].MacAddr = macAddr + + // Below this point we collect L3-specific info for the port. + if !port.IsL3Port { + m.deviceNetStatus.Ports[ix].AddrInfoList = nil + continue + } + m.deviceNetStatus.Ports[ix].AddrInfoList = make([]types.AddrInfo, len(ipAddrs)) if len(ipAddrs) == 0 { m.Log.Functionf("updateDNS: interface %s has NO IP addresses", port.IfName) @@ -132,6 +139,7 @@ func (m *DpcManager) updateDNS() { for i, addr := range ipAddrs { m.deviceNetStatus.Ports[ix].AddrInfoList[i].Addr = addr.IP } + // Get DNS etc info from dhcpcd. Updates DomainName and DNSServers. err = m.getDHCPInfo(&m.deviceNetStatus.Ports[ix]) if err != nil && dpc.State != types.DPCStateAsyncWait { @@ -141,7 +149,6 @@ func (m *DpcManager) updateDNS() { if err != nil && dpc.State != types.DPCStateAsyncWait { m.Log.Error(err) } - // Get used default routers aka gateways from kernel gws, err := m.NetworkMonitor.GetInterfaceDefaultGWs(ifindex) if err != nil { diff --git a/pkg/pillar/types/dns.go b/pkg/pillar/types/dns.go index 1978a3ae64..efb0658f53 100644 --- a/pkg/pillar/types/dns.go +++ b/pkg/pillar/types/dns.go @@ -212,6 +212,7 @@ func (status DeviceNetworkStatus) MostlyEqual(status2 DeviceNetworkStatus) bool p1.Logicallabel != p2.Logicallabel || p1.Alias != p2.Alias || p1.IsMgmt != p2.IsMgmt || + p1.IsL3Port != p2.IsL3Port || p1.Cost != p2.Cost { return false } @@ -365,14 +366,14 @@ func rotate(arr []string, amount int) []string { // rotation causes rotation/round-robin within each cost func GetMgmtPortsSortedCost(dns DeviceNetworkStatus, rotation int) []string { return getPortsSortedCostImpl(dns, rotation, - PortCostMax, true, false) + PortCostMax, true, true, false) } // GetAllPortsSortedCost returns all ports (management and app shared) sorted by port cost. // Rotation causes rotation/round-robin within each cost. -func GetAllPortsSortedCost(dns DeviceNetworkStatus, rotation int) []string { +func GetAllPortsSortedCost(dns DeviceNetworkStatus, l3Only bool, rotation int) []string { return getPortsSortedCostImpl(dns, rotation, - PortCostMax, false, false) + PortCostMax, l3Only, false, false) } // GetMgmtPortsSortedCostWithoutFailed returns all management ports sorted by @@ -380,35 +381,35 @@ func GetAllPortsSortedCost(dns DeviceNetworkStatus, rotation int) []string { // rotation causes rotation/round-robin within each cost func GetMgmtPortsSortedCostWithoutFailed(dns DeviceNetworkStatus, rotation int) []string { return getPortsSortedCostImpl(dns, rotation, - PortCostMax, true, true) + PortCostMax, true, true, true) } // getPortsSortedCostImpl returns all ports sorted by port cost // up to and including the maxCost func getPortsSortedCostImpl(dns DeviceNetworkStatus, rotation int, maxCost uint8, - mgmtOnly, dropFailed bool) []string { + l3Only, mgmtOnly, dropFailed bool) []string { ifnameList := []string{} costList := getPortCostListImpl(dns, maxCost) for _, cost := range costList { ifnameList = append(ifnameList, - getPortsImpl(dns, rotation, true, cost, mgmtOnly, dropFailed)...) + getPortsImpl(dns, rotation, true, cost, l3Only, mgmtOnly, dropFailed)...) } return ifnameList } // GetMgmtPortsAny returns all management ports func GetMgmtPortsAny(dns DeviceNetworkStatus, rotation int) []string { - return getPortsImpl(dns, rotation, false, 0, true, false) + return getPortsImpl(dns, rotation, false, 0, true, true, false) } // GetMgmtPortsByCost returns all management ports with a given port cost func GetMgmtPortsByCost(dns DeviceNetworkStatus, cost uint8) []string { - return getPortsImpl(dns, 0, true, cost, true, false) + return getPortsImpl(dns, 0, true, cost, true, true, false) } // Returns the IfNames. func getPortsImpl(dns DeviceNetworkStatus, rotation int, - matchCost bool, cost uint8, mgmtOnly, dropFailed bool) []string { + matchCost bool, cost uint8, l3Only, mgmtOnly, dropFailed bool) []string { ifnameList := make([]string, 0, len(dns.Ports)) for _, us := range dns.Ports { @@ -419,6 +420,9 @@ func getPortsImpl(dns DeviceNetworkStatus, rotation int, !us.IsMgmt { continue } + if l3Only && !us.IsL3Port { + continue + } if dropFailed && us.HasError() { continue } @@ -636,7 +640,7 @@ func getLocalAddrListImpl(dns DeviceNetworkStatus, if ifname == "" { // Get interfaces in cost order ifnameList = getPortsSortedCostImpl(dns, 0, - maxCost, true, false) + maxCost, true, true, false) // If we are looking across all interfaces, then We ignore errors // since we get them if there are no addresses on a ports ignoreErrors = true diff --git a/pkg/pillar/types/zedroutertypes_test.go b/pkg/pillar/types/zedroutertypes_test.go index 581f81e862..db7d6d8c28 100755 --- a/pkg/pillar/types/zedroutertypes_test.go +++ b/pkg/pillar/types/zedroutertypes_test.go @@ -457,6 +457,7 @@ func TestGetPortCostList(t *testing.T) { func TestGetMgmtPortsSortedCost(t *testing.T) { testMatrix := map[string]struct { deviceNetworkStatus DeviceNetworkStatus + l3Only bool rotate int expectedMgmtValue []string expectedAllValue []string @@ -466,8 +467,9 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: true, - Cost: 0}, + IsMgmt: true, + IsL3Port: true, + Cost: 0}, }, }, expectedMgmtValue: []string{"port1"}, @@ -478,8 +480,9 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: true, - Cost: 0}, + IsMgmt: true, + IsL3Port: true, + Cost: 0}, }, }, rotate: 14, @@ -496,8 +499,9 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: false, - Cost: 0}, + IsMgmt: false, + IsL3Port: true, + Cost: 0}, }, }, rotate: 14, @@ -509,17 +513,21 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: true, - Cost: 17}, + IsMgmt: true, + IsL3Port: true, + Cost: 17}, {IfName: "port2", - IsMgmt: true, - Cost: 1}, + IsMgmt: true, + IsL3Port: true, + Cost: 1}, {IfName: "port3", - IsMgmt: true, - Cost: 17}, + IsMgmt: true, + IsL3Port: true, + Cost: 17}, {IfName: "port4", - IsMgmt: true, - Cost: 1}, + IsMgmt: true, + IsL3Port: true, + Cost: 1}, }, }, expectedMgmtValue: []string{"port2", "port4", "port1", "port3"}, @@ -530,17 +538,21 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: true, - Cost: 17}, + IsMgmt: true, + IsL3Port: true, + Cost: 17}, {IfName: "port2", - IsMgmt: true, - Cost: 1}, + IsMgmt: true, + IsL3Port: true, + Cost: 1}, {IfName: "port3", - IsMgmt: true, - Cost: 17}, + IsMgmt: true, + IsL3Port: true, + Cost: 17}, {IfName: "port4", - IsMgmt: true, - Cost: 1}, + IsMgmt: true, + IsL3Port: true, + Cost: 1}, }, }, rotate: 1, @@ -552,17 +564,21 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: false, - Cost: 17}, + IsMgmt: false, + IsL3Port: true, + Cost: 17}, {IfName: "port2", - IsMgmt: false, - Cost: 1}, + IsMgmt: false, + IsL3Port: true, + Cost: 1}, {IfName: "port3", - IsMgmt: true, - Cost: 17}, + IsMgmt: true, + IsL3Port: true, + Cost: 17}, {IfName: "port4", - IsMgmt: true, - Cost: 1}, + IsMgmt: true, + IsL3Port: true, + Cost: 1}, }, }, expectedMgmtValue: []string{"port4", "port3"}, @@ -573,14 +589,17 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: true, - Cost: 2}, + IsMgmt: true, + IsL3Port: true, + Cost: 2}, {IfName: "port2", - IsMgmt: true, - Cost: 1}, + IsMgmt: true, + IsL3Port: true, + Cost: 1}, {IfName: "port3", - IsMgmt: true, - Cost: 0}, + IsMgmt: true, + IsL3Port: true, + Cost: 0}, }, }, expectedMgmtValue: []string{"port3", "port2", "port1"}, @@ -591,25 +610,54 @@ func TestGetMgmtPortsSortedCost(t *testing.T) { Version: DPCIsMgmt, Ports: []NetworkPortStatus{ {IfName: "port1", - IsMgmt: true, - Cost: 2}, + IsMgmt: true, + IsL3Port: true, + Cost: 2}, {IfName: "port2", - IsMgmt: false, - Cost: 1}, + IsMgmt: false, + IsL3Port: true, + Cost: 1}, {IfName: "port3", - IsMgmt: true, - Cost: 0}, + IsMgmt: true, + IsL3Port: true, + Cost: 0}, }, }, expectedMgmtValue: []string{"port3", "port1"}, expectedAllValue: []string{"port3", "port2", "port1"}, }, + "Test L3-only": { + deviceNetworkStatus: DeviceNetworkStatus{ + Version: DPCIsMgmt, + Ports: []NetworkPortStatus{ + {IfName: "port1", + IsMgmt: true, + IsL3Port: true, + Cost: 17}, + {IfName: "port2", + IsMgmt: false, + IsL3Port: false, + Cost: 1}, + {IfName: "port3", + IsMgmt: false, + IsL3Port: false, + Cost: 17}, + {IfName: "port4", + IsMgmt: false, + IsL3Port: true, + Cost: 1}, + }, + }, + l3Only: true, + expectedMgmtValue: []string{"port1"}, + expectedAllValue: []string{"port4", "port1"}, + }, } for testname, test := range testMatrix { t.Logf("Running test case %s", testname) value := GetMgmtPortsSortedCost(test.deviceNetworkStatus, test.rotate) assert.Equal(t, test.expectedMgmtValue, value) - value = GetAllPortsSortedCost(test.deviceNetworkStatus, test.rotate) + value = GetAllPortsSortedCost(test.deviceNetworkStatus, test.l3Only, test.rotate) assert.Equal(t, test.expectedAllValue, value) } } diff --git a/pkg/pillar/zedcloud/send.go b/pkg/pillar/zedcloud/send.go index 73a6a47059..4740dd3ff0 100644 --- a/pkg/pillar/zedcloud/send.go +++ b/pkg/pillar/zedcloud/send.go @@ -324,7 +324,9 @@ func VerifyAllIntf(ctx *ZedCloudContext, url string, requiredSuccessCount uint, // that was needed to achieve requiredSuccessCount. var workingMgmtCost uint8 - intfs := types.GetAllPortsSortedCost(*ctx.DeviceNetworkStatus, iteration) + // A small set of verification checks is run even for L2-only interfaces + // (link presence, admin status, etc.) + intfs := types.GetAllPortsSortedCost(*ctx.DeviceNetworkStatus, false, iteration) ctxWork, cancel := GetContextForAllIntfFunctions(ctx) defer cancel() @@ -380,8 +382,10 @@ func VerifyAllIntf(ctx *ZedCloudContext, url string, requiredSuccessCount uint, var noAddrErr *types.IPAddrNotAvailError if errors.As(err, &noAddrErr) { // Interface link exists and is UP but does not have any IP address assigned. - // This is expected with app-shared interface and DhcpTypeNone. - if !portStatus.IsMgmt && portStatus.Dhcp == types.DhcpTypeNone { + // This is expected for L2-only interfaces and also for app-shared interfaces + // configured with DhcpTypeNone. + if !portStatus.IsL3Port || + (!portStatus.IsMgmt && portStatus.Dhcp == types.DhcpTypeNone) { verifyRV.IntfStatusMap.RecordSuccess(intf) continue }