From d49a1787b8b9b27e429837de9a3ce6eef0b804ff Mon Sep 17 00:00:00 2001 From: Milan Lenco Date: Fri, 24 May 2024 17:46:25 +0200 Subject: [PATCH] Do not try to use invalid network config Even if zedagent detects invalid network config and reports DPC with error, NIM may still try to use it, run connection tests and overwrite the error with another or even with nil. Instead, NIM should skip mgmt ports with invalid config and preserve the error (incl. across reboots). Similarly, DPC reconciler should not try to apply invalid config into the network stack. Additionally, diag was extended to report when port config is not valid. Lastly, readability of some parsing error messages was improved. Signed-off-by: Milan Lenco (cherry picked from commit 2beb07e16585fd8d16aad4cfe04be01aeb534abc) --- pkg/pillar/cmd/diag/diag.go | 19 ++- pkg/pillar/cmd/zedagent/parseconfig.go | 121 ++++++++------------ pkg/pillar/cmd/zedagent/parseconfig_test.go | 2 +- pkg/pillar/cmd/zedrouter/networkinstance.go | 7 +- pkg/pillar/conntester/zedcloud.go | 3 + pkg/pillar/dpcmanager/dns.go | 1 + pkg/pillar/dpcmanager/dpc.go | 11 +- pkg/pillar/dpcreconciler/linux.go | 21 ++-- pkg/pillar/types/dns.go | 20 ++-- pkg/pillar/types/dpc.go | 18 ++- pkg/pillar/uplinkprober/uplinkprober.go | 4 +- pkg/pillar/zedcloud/send.go | 5 + 12 files changed, 127 insertions(+), 105 deletions(-) diff --git a/pkg/pillar/cmd/diag/diag.go b/pkg/pillar/cmd/diag/diag.go index 3d5b3164b5..538d373e2d 100644 --- a/pkg/pillar/cmd/diag/diag.go +++ b/pkg/pillar/cmd/diag/diag.go @@ -894,6 +894,10 @@ func printOutput(ctx *diagContext, caller string) { mgmtPorts++ } + var isValidStr string + if port.InvalidConfig { + isValidStr = " (invalid config)" + } typeStr := "use: app-shared " if isMgmt { if priority == types.PortCostMin { @@ -921,8 +925,8 @@ func printOutput(ctx *diagContext, caller string) { ipCount++ noGeo := ipinfo.IPInfo{} if dpcSuccess { - ctx.ph.Print("INFO: Port %s: %s%s%s%s\n", - ifname, macStr, linkStr, typeStr, ai.Addr) + ctx.ph.Print("INFO: Port %s%s: %s%s%s%s\n", + ifname, isValidStr, macStr, linkStr, typeStr, ai.Addr) } else if ai.Geo == noGeo { ctx.ph.Print("INFO: %s: IP address %s not geolocated\n", ifname, ai.Addr) @@ -932,8 +936,8 @@ func printOutput(ctx *diagContext, caller string) { } } if ipCount == 0 { - ctx.ph.Print("INFO: Port %s: %s%s%sNo IP address\n", - ifname, macStr, linkStr, typeStr) + ctx.ph.Print("INFO: Port %s%s: %s%s%sNo IP address\n", + ifname, isValidStr, macStr, linkStr, typeStr) } // Skip details if we are connected to controller. Count @@ -963,6 +967,13 @@ func printOutput(ctx *diagContext, caller string) { ifname, port.NtpServer.String()) } printProxy(ctx, port, ifname) + if port.HasError() { + if port.InvalidConfig { + ctx.ph.Print("ERROR: %s: invalid config: %s\n", ifname, port.LastError) + } else { + ctx.ph.Print("ERROR: %s: has error: %s\n", ifname, port.LastError) + } + } if !isMgmt { ctx.ph.Print("INFO: %s: not intended for EV controller; skipping those tests\n", diff --git a/pkg/pillar/cmd/zedagent/parseconfig.go b/pkg/pillar/cmd/zedagent/parseconfig.go index e1c80e002e..69b275e72d 100644 --- a/pkg/pillar/cmd/zedagent/parseconfig.go +++ b/pkg/pillar/cmd/zedagent/parseconfig.go @@ -466,8 +466,9 @@ func publishNetworkInstanceConfig(ctx *getconfigContext, Activate: apiConfigEntry.Activate, PropagateConnRoutes: apiConfigEntry.PropagateConnectedRoutes, } + uuidStr := networkInstanceConfig.UUID.String() log.Functionf("publishNetworkInstanceConfig: processing %s %s type %d activate %v", - networkInstanceConfig.UUID.String(), networkInstanceConfig.DisplayName, + uuidStr, networkInstanceConfig.DisplayName, networkInstanceConfig.Type, networkInstanceConfig.Activate) if apiConfigEntry.Port != nil { @@ -480,7 +481,7 @@ func publishNetworkInstanceConfig(ctx *getconfigContext, // network instances if networkInstanceConfig.IpType != types.AddressTypeNone { log.Errorf("Switch network instance %s %s with invalid IpType %d should be %d", - networkInstanceConfig.UUID.String(), + uuidStr, networkInstanceConfig.DisplayName, networkInstanceConfig.IpType, types.AddressTypeNone) @@ -494,9 +495,8 @@ func publishNetworkInstanceConfig(ctx *getconfigContext, if networkInstanceConfig.IpType != types.AddressTypeNone { err := parseIpspec(apiConfigEntry.Ip, &networkInstanceConfig) if err != nil { - errStr := fmt.Sprintf("Network Instance %s parameter parse failed: %s", - networkInstanceConfig.Key(), err) - log.Error(errStr) + errStr := fmt.Sprintf("Invalid IP configuration: %s", err) + log.Errorf("publishNetworkInstanceConfig (%s): %s", uuidStr, errStr) networkInstanceConfig.SetErrorNow(errStr) // Proceed to send error back to controller } @@ -505,10 +505,9 @@ func publishNetworkInstanceConfig(ctx *getconfigContext, for _, route := range apiConfigEntry.StaticRoutes { err := parseStaticRoute(route, &networkInstanceConfig) if err != nil { - err = fmt.Errorf("network Instance %s IP route %v parsing failed: %w", - networkInstanceConfig.Key(), route, err) - log.Error(err) - networkInstanceConfig.SetErrorNow(err.Error()) + errStr := fmt.Sprintf("Invalid IP route (%v): %v", route, err) + log.Errorf("publishNetworkInstanceConfig (%s): %s", uuidStr, errStr) + networkInstanceConfig.SetErrorNow(errStr) // Proceed to send error back to controller } } @@ -757,7 +756,7 @@ func parseSystemAdapterConfig(getconfigCtx *getconfigContext, config *zconfig.Ed // Check if all management ports have errors // Propagate any parse errors for all ports to the DPC // since controller expects LastError and LastFailed for the DPC - ok := false + hasValidMgmtPort := false mgmtCount := 0 errStr := "" for _, p := range portConfig.Ports { @@ -766,12 +765,12 @@ func parseSystemAdapterConfig(getconfigCtx *getconfigContext, config *zconfig.Ed } mgmtCount++ if !p.HasError() { - ok = true + hasValidMgmtPort = true break } errStr += p.LastError + "\n" } - if !ok { + if !hasValidMgmtPort { if errStr == "" && portConfig.HasError() { errStr = portConfig.LastError } @@ -948,6 +947,9 @@ func validateAndAssignNetPorts(dpc *types.DevicePortConfig, newPorts []*types.Ne // 4. Assign all non-duplicate, validated ports. for _, port := range validatedPorts { + if port.HasError() { + port.InvalidConfig = true + } dpc.Ports = append(dpc.Ports, *port) } } @@ -1165,9 +1167,8 @@ func parseOneSystemAdapterConfig(getconfigCtx *getconfigContext, if sysAdapter.Addr != "" { ip = net.ParseIP(sysAdapter.Addr) if ip == nil { - errStr := fmt.Sprintf("Device Config Error. Port %s has Bad "+ - "SysAdapter.Addr %s. The IP address is ignored. Please fix the "+ - "device configuration.", sysAdapter.Name, sysAdapter.Addr) + errStr := fmt.Sprintf("Port %s configured with invalid IP address (%s)", + sysAdapter.Name, sysAdapter.Addr) log.Errorf("parseSystemAdapterConfig: %s", errStr) port.RecordFailure(errStr) // IP will not be set below @@ -1182,9 +1183,7 @@ func parseOneSystemAdapterConfig(getconfigCtx *getconfigContext, networkXObject, err := getconfigCtx.pubNetworkXObjectConfig.Get(sysAdapter.NetworkUUID) if err != nil { // XXX when do we retry looking for the networkXObject? - errStr := fmt.Sprintf("Device Config Error. Port %s configured with "+ - "UNKNOWN Network UUID (%s). Err: %s. Please fix the "+ - "device configuration.", + errStr := fmt.Sprintf("Port %s configured with unknown Network UUID (%s): %v", port.Logicallabel, sysAdapter.NetworkUUID, err) log.Errorf("parseSystemAdapterConfig: %s", errStr) port.RecordFailure(errStr) @@ -1194,9 +1193,8 @@ func parseOneSystemAdapterConfig(getconfigCtx *getconfigContext, port.Type = net.Type network = &net if network.HasError() { - errStr := fmt.Sprintf("Port %s configured with a network "+ - "(UUID: %s) which has an error (%s).", - port.Logicallabel, port.NetworkUUID, network.Error) + errStr := fmt.Sprintf("Network %s assigned to port %s has invalid config: %v", + port.NetworkUUID, port.Logicallabel, network.Error) log.Errorf("parseSystemAdapterConfig: %s", errStr) port.RecordFailure(errStr) } @@ -1217,10 +1215,9 @@ func parseOneSystemAdapterConfig(getconfigCtx *getconfigContext, port.Dhcp = network.Dhcp switch port.Dhcp { case types.DhcpTypeStatic: - if port.AddrSubnet == "" { - errStr := fmt.Sprintf("Port %s Configured as DhcpTypeStatic but "+ - "missing subnet address. SysAdapter - Name: %s, Addr:%s", - port.Logicallabel, sysAdapter.Name, sysAdapter.Addr) + if sysAdapter.Addr == "" { + errStr := fmt.Sprintf("Port %s configured with static IP config "+ + "but IP address is not defined", port.Logicallabel) log.Errorf("parseSystemAdapterConfig: %s", errStr) port.RecordFailure(errStr) } @@ -1248,8 +1245,8 @@ func parseOneSystemAdapterConfig(getconfigCtx *getconfigContext, } } } else if isMgmt { - errStr := fmt.Sprintf("Port %s Configured as Management port without "+ - "configuring a Network. Network is required for Management ports", + errStr := fmt.Sprintf("Port %s configured as Management port but without "+ + "Network assigned. Network is required for Management ports", port.Logicallabel) log.Errorf("parseSystemAdapterConfig: %s", errStr) port.RecordFailure(errStr) @@ -1789,9 +1786,8 @@ func parseOneNetworkXObjectConfig(ctx *getconfigContext, netEnt *zconfig.Network config.Type = types.NetworkType(netEnt.Type) id, err := uuid.FromString(netEnt.Id) if err != nil { - errStr := fmt.Sprintf("parseOneNetworkXObjectConfig: Malformed UUID ignored: %s", - err) - log.Error(errStr) + errStr := fmt.Sprintf("Malformed UUID ignored: %s", err) + log.Errorf("parseOneNetworkXObjectConfig (%s): %s", config.Key(), errStr) config.SetErrorNow(errStr) return config } @@ -1849,17 +1845,15 @@ func parseOneNetworkXObjectConfig(ctx *getconfigContext, netEnt *zconfig.Network switch config.Type { case types.NetworkTypeIPv4, types.NetworkTypeIPV6: if ipspec == nil { - errStr := fmt.Sprintf("parseOneNetworkXObjectConfig: Missing ipspec for %s in %v", - config.Key(), netEnt) - log.Error(errStr) + errStr := "Missing IP configuration" + log.Errorf("parseOneNetworkXObjectConfig (%s): %s", config.Key(), errStr) config.SetErrorNow(errStr) return config } err := parseIpspecNetworkXObject(ipspec, config) if err != nil { - errStr := fmt.Sprintf("Network parameter parse for %s failed: %s", - config.Key(), err) - log.Error(errStr) + errStr := fmt.Sprintf("Invalid IP configuration: %s", err) + log.Errorf("parseOneNetworkXObjectConfig (%s): %s", config.Key(), errStr) config.SetErrorNow(errStr) return config } @@ -1870,18 +1864,16 @@ func parseOneNetworkXObjectConfig(ctx *getconfigContext, netEnt *zconfig.Network config.Key(), ipspec) err := parseIpspecNetworkXObject(ipspec, config) if err != nil { - errStr := fmt.Sprintf("Network parameter parse for %s failed: %s", - config.Key(), err) - log.Error(errStr) + errStr := fmt.Sprintf("Invalid IP configuration: %s", err) + log.Errorf("parseOneNetworkXObjectConfig (%s): %s", config.Key(), errStr) config.SetErrorNow(errStr) return config } } default: - errStr := fmt.Sprintf("parseOneNetworkXObjectConfig: Unknown NetworkConfig type %d for %s in %v; ignored", - config.Type, id.String(), netEnt) - log.Error(errStr) + errStr := fmt.Sprintf("Unknown Network type (%d)", config.Type) + log.Errorf("parseOneNetworkXObjectConfig (%s): %s", config.Key(), errStr) config.SetErrorNow(errStr) return config } @@ -1901,9 +1893,8 @@ func parseOneNetworkXObjectConfig(ctx *getconfigContext, netEnt *zconfig.Network if ip != nil { ips = append(ips, ip) } else { - errStr := fmt.Sprintf("parseOneNetworkXObjectConfig: bad dnsEntry %s for %s", - strAddr, config.Key()) - log.Error(errStr) + errStr := fmt.Sprintf("Invalid DNS entry address (%s)", strAddr) + log.Errorf("parseOneNetworkXObjectConfig (%s): %s", config.Key(), errStr) config.SetErrorNow(errStr) return config } @@ -2038,43 +2029,37 @@ func parseIpspecNetworkXObject(ipspec *zconfig.Ipspec, config *types.NetworkXObj if s := ipspec.GetSubnet(); s != "" { _, subnet, err := net.ParseCIDR(s) if err != nil { - return errors.New(fmt.Sprintf("bad subnet %s: %s", - s, err)) + return fmt.Errorf("invalid subnet (%s): %w", s, err) } config.Subnet = *subnet } if g := ipspec.GetGateway(); g != "" { config.Gateway = net.ParseIP(g) if config.Gateway == nil { - return errors.New(fmt.Sprintf("bad gateway IP %s", - g)) + return fmt.Errorf("invalid gateway IP (%s)", g) } } if n := ipspec.GetNtp(); n != "" { config.NTPServer = net.ParseIP(n) if config.NTPServer == nil { - return errors.New(fmt.Sprintf("bad ntp IP %s", - n)) + return fmt.Errorf("invalid NTP IP (%s)", n) } } for _, dsStr := range ipspec.GetDns() { ds := net.ParseIP(dsStr) if ds == nil { - return errors.New(fmt.Sprintf("bad dns IP %s", - dsStr)) + return fmt.Errorf("invalid DNS IP (%s)", dsStr) } config.DNSServers = append(config.DNSServers, ds) } if dr := ipspec.GetDhcpRange(); dr != nil && dr.GetStart() != "" { start := net.ParseIP(dr.GetStart()) if start == nil { - return errors.New(fmt.Sprintf("bad start IP %s", - dr.GetStart())) + return fmt.Errorf("invalid DHCP range start IP (%s)", dr.GetStart()) } end := net.ParseIP(dr.GetEnd()) if end == nil && dr.GetEnd() != "" { - return errors.New(fmt.Sprintf("bad end IP %s", - dr.GetEnd())) + return fmt.Errorf("invalid DHCP range end IP (%s)", dr.GetEnd()) } config.DhcpRange.Start = start config.DhcpRange.End = end @@ -2090,16 +2075,14 @@ func parseIpspec(ipspec *zconfig.Ipspec, if n := ipspec.GetNtp(); n != "" { config.NtpServer = net.ParseIP(n) if config.NtpServer == nil { - return errors.New(fmt.Sprintf("bad ntp IP %s", - n)) + return fmt.Errorf("invalid NTP IP (%s)", n) } } // Parse Dns Servers for _, dsStr := range ipspec.GetDns() { ds := net.ParseIP(dsStr) if ds == nil { - return errors.New(fmt.Sprintf("bad dns IP %s", - dsStr)) + return fmt.Errorf("invalid DNS IP (%s)", dsStr) } config.DnsServers = append(config.DnsServers, ds) } @@ -2107,8 +2090,7 @@ func parseIpspec(ipspec *zconfig.Ipspec, if s := ipspec.GetSubnet(); s != "" { _, subnet, err := net.ParseCIDR(s) if err != nil { - return errors.New(fmt.Sprintf("bad subnet %s: %s", - s, err)) + return fmt.Errorf("invalid subnet (%s): %w", s, err) } config.Subnet = *subnet } @@ -2116,21 +2098,18 @@ func parseIpspec(ipspec *zconfig.Ipspec, if g := ipspec.GetGateway(); g != "" { config.Gateway = net.ParseIP(g) if config.Gateway == nil { - return errors.New(fmt.Sprintf("bad gateway IP %s", - g)) + return fmt.Errorf("invalid gateway IP (%s)", g) } } // Parse DhcpRange if dr := ipspec.GetDhcpRange(); dr != nil && dr.GetStart() != "" { start := net.ParseIP(dr.GetStart()) if start == nil { - return errors.New(fmt.Sprintf("bad start IP %s", - dr.GetStart())) + return fmt.Errorf("invalid DHCP range start IP (%s)", dr.GetStart()) } end := net.ParseIP(dr.GetEnd()) if end == nil && dr.GetEnd() != "" { - return errors.New(fmt.Sprintf("bad end IP %s", - dr.GetEnd())) + return fmt.Errorf("invalid DHCP range end IP (%s)", dr.GetEnd()) } config.DhcpRange.Start = start config.DhcpRange.End = end @@ -2138,8 +2117,8 @@ func parseIpspec(ipspec *zconfig.Ipspec, addrCount := netutils.GetIPAddrCountOnSubnet(config.Subnet) if addrCount < types.MinSubnetSize { - return fmt.Errorf("network(%s), Subnet too small(%d)", - config.Key(), addrCount) + return fmt.Errorf("subnet is too small (only %d available IP addresses, need %d)", + addrCount, types.MinSubnetSize) } // if not set, take some default diff --git a/pkg/pillar/cmd/zedagent/parseconfig_test.go b/pkg/pillar/cmd/zedagent/parseconfig_test.go index 670b4f37bd..9b76a2b189 100644 --- a/pkg/pillar/cmd/zedagent/parseconfig_test.go +++ b/pkg/pillar/cmd/zedagent/parseconfig_test.go @@ -195,7 +195,7 @@ func TestDPCWithError(t *testing.T) { g.Expect(dpc.LastFailed).ToNot(BeZero()) g.Expect(dpc.LastSucceeded).To(BeZero()) g.Expect(getPortError(&dpc, "adapter-shopfloor")). - To(ContainSubstring("UNKNOWN Network UUID")) + To(ContainSubstring("Port adapter-shopfloor configured with unknown Network UUID")) g.Expect(dpc.Ports).To(HaveLen(1)) port := dpc.Ports[0] g.Expect(port.Logicallabel).To(Equal("adapter-shopfloor")) diff --git a/pkg/pillar/cmd/zedrouter/networkinstance.go b/pkg/pillar/cmd/zedrouter/networkinstance.go index dfaff8eac1..eb47848fa3 100644 --- a/pkg/pillar/cmd/zedrouter/networkinstance.go +++ b/pkg/pillar/cmd/zedrouter/networkinstance.go @@ -118,7 +118,12 @@ func (z *zedrouter) setSelectedUplink(uplinkLogicalLabel string, // Wait for DPC update return true, err case 1: - // OK + if ports[0].InvalidConfig { + return false, fmt.Errorf("port %s has invalid config: %s", ports[0].Logicallabel, + ports[0].LastError) + } + // Selected port is OK + break default: err = fmt.Errorf("label of selected uplink matches multiple ports (%v)", ports) return false, err diff --git a/pkg/pillar/conntester/zedcloud.go b/pkg/pillar/conntester/zedcloud.go index 5e2150105e..b9b206153e 100644 --- a/pkg/pillar/conntester/zedcloud.go +++ b/pkg/pillar/conntester/zedcloud.go @@ -94,6 +94,9 @@ func (t *ZedcloudConnectivityTester) TestConnectivity(dns types.DeviceNetworkSta } zedcloudCtx.TlsConfig = tlsConfig for ix := range dns.Ports { + if dns.Ports[ix].InvalidConfig { + continue + } ifName := dns.Ports[ix].IfName err = devicenetwork.CheckAndGetNetworkProxy(t.Log, &dns, ifName, t.Metrics) if err != nil { diff --git a/pkg/pillar/dpcmanager/dns.go b/pkg/pillar/dpcmanager/dns.go index 1c9d2e8654..7a1d203612 100644 --- a/pkg/pillar/dpcmanager/dns.go +++ b/pkg/pillar/dpcmanager/dns.go @@ -69,6 +69,7 @@ func (m *DpcManager) updateDNS() { // Prefer errors recorded by DPC verification. // New errors are recorded from this function only when there is none yet // (HasError() == false). + m.deviceNetStatus.Ports[ix].InvalidConfig = port.InvalidConfig m.deviceNetStatus.Ports[ix].TestResults = port.TestResults m.deviceNetStatus.Ports[ix].WirelessStatus.WType = port.WirelessCfg.WType // If this is a cellular network connectivity, add status information diff --git a/pkg/pillar/dpcmanager/dpc.go b/pkg/pillar/dpcmanager/dpc.go index 062f5c7693..9e7145e5e6 100644 --- a/pkg/pillar/dpcmanager/dpc.go +++ b/pkg/pillar/dpcmanager/dpc.go @@ -23,7 +23,7 @@ func (m *DpcManager) currentDPC() *types.DevicePortConfig { func (m *DpcManager) doAddDPC(ctx context.Context, dpc types.DevicePortConfig) { m.setDiscoveredWwanIfNames(&dpc) - mgmtCount := dpc.CountMgmtPorts() + mgmtCount := dpc.CountMgmtPorts(false) if mgmtCount == 0 { // This DPC will be ignored when we check IsDPCUsable which // is called from IsDPCTestable and IsDPCUntested. @@ -233,12 +233,15 @@ func (m *DpcManager) ingestDPCList() (dpclPresentAtBoot bool) { for _, portConfig := range storedDpcl.PortConfigList { // Sanitize port labels and IsL3Port flag. portConfig.DoSanitize(m.Log, false, false, "", true, true) - // Clear the errors from before reboot and start fresh. + // Clear runtime errors (not config validation errors) from before reboot + // and start fresh. for i := 0; i < len(portConfig.Ports); i++ { portPtr := &portConfig.Ports[i] - portPtr.Clear() + if !portPtr.InvalidConfig { + portPtr.Clear() + } } - if portConfig.CountMgmtPorts() == 0 { + if portConfig.CountMgmtPorts(false) == 0 { m.Log.Warnf("Stored DevicePortConfig key %s has no management ports; ignored", portConfig.Key) continue diff --git a/pkg/pillar/dpcreconciler/linux.go b/pkg/pillar/dpcreconciler/linux.go index c58430eeff..4bb82e79bd 100644 --- a/pkg/pillar/dpcreconciler/linux.go +++ b/pkg/pillar/dpcreconciler/linux.go @@ -720,7 +720,7 @@ func (r *LinuxDpcReconciler) updateCurrentAdapterAddrs( sgPath := dg.NewSubGraphPath(L3SG, AdaptersSG, AdapterAddrsSG) currentAddrs := dg.New(dg.InitArgs{Name: AdapterAddrsSG}) for _, port := range dpc.Ports { - if !port.IsL3Port || port.IfName == "" { + if !port.IsL3Port || port.IfName == "" || port.InvalidConfig { continue } ifIndex, found, err := r.NetworkMonitor.GetInterfaceIndex(port.IfName) @@ -770,7 +770,7 @@ func (r *LinuxDpcReconciler) updateCurrentRoutes(dpc types.DevicePortConfig) (ch } } for _, port := range dpc.Ports { - if port.IfName == "" { + if port.IfName == "" || port.InvalidConfig { continue } ifIndex, found, err := r.NetworkMonitor.GetInterfaceIndex(port.IfName) @@ -863,7 +863,7 @@ func (r *LinuxDpcReconciler) getIntendedGlobalCfg(dpc types.DevicePortConfig) dg // Intended content of /etc/resolv.conf dnsServers := make(map[string][]net.IP) for _, port := range dpc.Ports { - if !port.IsMgmt || port.IfName == "" { + if !port.IsMgmt || port.IfName == "" || port.InvalidConfig { continue } ifIndex, found, err := r.NetworkMonitor.GetInterfaceIndex(port.IfName) @@ -914,7 +914,7 @@ func (r *LinuxDpcReconciler) getIntendedLogicalIO(dpc types.DevicePortConfig) dg } intendedIO := dg.New(graphArgs) for _, port := range dpc.Ports { - if port.IfName == "" { + if port.IfName == "" || port.InvalidConfig { continue } switch port.L2Type { @@ -1001,7 +1001,7 @@ func (r *LinuxDpcReconciler) getIntendedAdapters(dpc types.DevicePortConfig) dg. } intendedAdapters := dg.New(graphArgs) for _, port := range dpc.Ports { - if !port.IsL3Port || port.IfName == "" { + if !port.IsL3Port || port.IfName == "" || port.InvalidConfig { continue } adapter := linux.Adapter{ @@ -1049,7 +1049,7 @@ func (r *LinuxDpcReconciler) getIntendedSrcIPRules(dpc types.DevicePortConfig) d } intendedRules := dg.New(graphArgs) for _, port := range dpc.Ports { - if port.IfName == "" { + if port.IfName == "" || port.InvalidConfig { continue } ifIndex, found, err := r.NetworkMonitor.GetInterfaceIndex(port.IfName) @@ -1108,7 +1108,7 @@ func (r *LinuxDpcReconciler) getIntendedRoutes(dpc types.DevicePortConfig) dg.Gr } } for _, port := range dpc.Ports { - if port.IfName == "" { + if port.IfName == "" || port.InvalidConfig { continue } ifIndex, found, err := r.NetworkMonitor.GetInterfaceIndex(port.IfName) @@ -1178,7 +1178,7 @@ type portAddr struct { func (r *LinuxDpcReconciler) groupPortAddrs(dpc types.DevicePortConfig) map[string][]portAddr { arpGroups := map[string][]portAddr{} for _, port := range dpc.Ports { - if port.IfName == "" { + if port.IfName == "" || port.InvalidConfig { continue } ifIndex, found, err := r.NetworkMonitor.GetInterfaceIndex(port.IfName) @@ -1371,6 +1371,9 @@ func (r *LinuxDpcReconciler) getIntendedWwanConfig(dpc types.DevicePortConfig, Networks: []types.WwanNetworkConfig{}, } for _, port := range dpc.Ports { + if port.InvalidConfig { + continue + } if port.WirelessCfg.WType != types.WirelessTypeCellular { continue } @@ -1787,7 +1790,7 @@ func (r *LinuxDpcReconciler) getIntendedACLs( // i.e. these rules are below protoMarkV4Rules/protoMarkV6Rules var dropMarkRules []iptables.Rule for _, port := range dpc.Ports { - if port.IfName == "" { + if port.IfName == "" || port.InvalidConfig { continue } dropIngressRule := iptables.Rule{ diff --git a/pkg/pillar/types/dns.go b/pkg/pillar/types/dns.go index b900dd0ddc..38cb6b2a87 100644 --- a/pkg/pillar/types/dns.go +++ b/pkg/pillar/types/dns.go @@ -29,12 +29,16 @@ type DeviceNetworkStatus struct { } type NetworkPortStatus struct { - IfName string - Phylabel string // Physical name set by controller/model - Logicallabel string - Alias string // From SystemAdapter's alias - IsMgmt bool // Used to talk to controller - IsL3Port bool // True if port is applicable to operate on the network layer + IfName string + Phylabel string // Physical name set by controller/model + Logicallabel string + Alias string // From SystemAdapter's alias + IsMgmt bool // Used to talk to controller + IsL3Port bool // True if port is applicable to operate on the network layer + // InvalidConfig is used to flag port config which failed parsing or (static) validation + // checks, such as: malformed IP address, undefined required field, IP address not inside + // the subnet, etc. + InvalidConfig bool Cost uint8 Dhcp DhcpType Type NetworkType // IPv4 or IPv6 or Dual stack @@ -210,7 +214,9 @@ func (status DeviceNetworkStatus) MostlyEqual(status2 DeviceNetworkStatus) bool p1.Alias != p2.Alias || p1.IsMgmt != p2.IsMgmt || p1.IsL3Port != p2.IsL3Port || - p1.Cost != p2.Cost { + p1.InvalidConfig != p2.InvalidConfig || + p1.Cost != p2.Cost || + p1.MTU != p2.MTU { return false } if p1.Dhcp != p2.Dhcp || diff --git a/pkg/pillar/types/dpc.go b/pkg/pillar/types/dpc.go index bd109f8d22..e034ab74f5 100644 --- a/pkg/pillar/types/dpc.go +++ b/pkg/pillar/types/dpc.go @@ -355,10 +355,12 @@ func (config *DevicePortConfig) DoSanitize(log *base.LogObject, // CountMgmtPorts returns the number of management ports // Exclude any broken ones with Dhcp = DhcpTypeNone -func (config *DevicePortConfig) CountMgmtPorts() int { +// Optionally exclude mgmt ports with invalid config +func (config *DevicePortConfig) CountMgmtPorts(onlyValidConfig bool) int { count := 0 for _, port := range config.Ports { - if port.IsMgmt && port.Dhcp != DhcpTypeNone { + if port.IsMgmt && port.Dhcp != DhcpTypeNone && + !(onlyValidConfig && port.InvalidConfig) { count++ } } @@ -434,7 +436,7 @@ func (config DevicePortConfig) IsDPCUntested() bool { // IsDPCUsable - checks whether something is invalid; no management IP // addresses means it isn't usable hence we return false if none. func (config DevicePortConfig) IsDPCUsable() bool { - mgmtCount := config.CountMgmtPorts() + mgmtCount := config.CountMgmtPorts(true) return mgmtCount > 0 } @@ -511,9 +513,13 @@ type NetworkPortConfig struct { Alias string // From SystemAdapter's alias // NetworkUUID - UUID of the Network Object configured for the port. NetworkUUID uuid.UUID - IsMgmt bool // Used to talk to controller - IsL3Port bool // True if port is applicable to operate on the network layer - Cost uint8 // Zero is free + IsMgmt bool // Used to talk to controller + IsL3Port bool // True if port is applicable to operate on the network layer + // InvalidConfig is used to flag port config which failed parsing or (static) validation + // checks, such as: malformed IP address, undefined required field, IP address not inside + // the subnet, etc. + InvalidConfig bool + Cost uint8 // Zero is free DhcpConfig ProxyConfig L2LinkConfig diff --git a/pkg/pillar/uplinkprober/uplinkprober.go b/pkg/pillar/uplinkprober/uplinkprober.go index 44e511ea6b..d41e6fc56a 100644 --- a/pkg/pillar/uplinkprober/uplinkprober.go +++ b/pkg/pillar/uplinkprober/uplinkprober.go @@ -438,7 +438,7 @@ func (p *UplinkProber) applyPendingDNS(pendingDNS types.DeviceNetworkStatus) { continue } port := ports[0] - if !port.IsMgmt { + if !port.IsMgmt || port.InvalidConfig { p.log.Noticef("UplinkProber: Removed %s from the list of probed uplinks", uplinkLL) delete(p.uplinkProbeStatus, uplinkLL) @@ -446,7 +446,7 @@ func (p *UplinkProber) applyPendingDNS(pendingDNS types.DeviceNetworkStatus) { } // Add ports that have just appeared and update existing. for _, port := range p.pendingDNS.Ports { - if !port.IsMgmt { + if !port.IsMgmt || port.InvalidConfig { continue } uplinkStatus, haveStatus := p.uplinkProbeStatus[port.Logicallabel] diff --git a/pkg/pillar/zedcloud/send.go b/pkg/pillar/zedcloud/send.go index 81d3176f36..d573f5f9cb 100644 --- a/pkg/pillar/zedcloud/send.go +++ b/pkg/pillar/zedcloud/send.go @@ -335,6 +335,11 @@ func VerifyAllIntf(ctx *ZedCloudContext, url string, requiredSuccessCount uint, // (aka dry-run). for _, intf := range intfs { portStatus := types.GetPort(*ctx.DeviceNetworkStatus, intf) + if portStatus.InvalidConfig { + // Do not try to test port with invalid config. + // Otherwise, the test would fail and the parsing error would get overwritten. + continue + } // If we have enough uplinks with cloud connectivity, then the remaining // interfaces (some of which might not be free) are verified using // only local checks, without generating any traffic.