Skip to content

Commit

Permalink
Fix VLANs and LAGs
Browse files Browse the repository at this point in the history
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 <milan@zededa.com>
  • Loading branch information
milan-zededa committed Jan 5, 2024
1 parent 03b8972 commit 10b6230
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 82 deletions.
5 changes: 4 additions & 1 deletion pkg/pillar/cmd/diag/diag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Check warning on line 877 in pkg/pillar/cmd/diag/diag.go

View check run for this annotation

Codecov / codecov/patch

pkg/pillar/cmd/diag/diag.go#L877

Added line #L877 was not covered by tests
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

Check warning on line 885 in pkg/pillar/cmd/diag/diag.go

View check run for this annotation

Codecov / codecov/patch

pkg/pillar/cmd/diag/diag.go#L884-L885

Added lines #L884 - L885 were not covered by tests
}
// Print usefully formatted info based on which
// fields are set and Dhcp type; proxy info order
ifname := port.IfName
Expand Down
23 changes: 0 additions & 23 deletions pkg/pillar/cmd/domainmgr/domainmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down
10 changes: 10 additions & 0 deletions pkg/pillar/cmd/zedagent/parseconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion pkg/pillar/devicenetwork/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
9 changes: 8 additions & 1 deletion pkg/pillar/dpcmanager/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,21 @@ 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)
}
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 {
Expand All @@ -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 {
Expand Down
24 changes: 14 additions & 10 deletions pkg/pillar/types/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||

Check warning on line 215 in pkg/pillar/types/dns.go

View check run for this annotation

Codecov / codecov/patch

pkg/pillar/types/dns.go#L215

Added line #L215 was not covered by tests
p1.Cost != p2.Cost {
return false
}
Expand Down Expand Up @@ -365,50 +366,50 @@ 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
// port cost ignoring ports with failures.
// 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)

Check warning on line 384 in pkg/pillar/types/dns.go

View check run for this annotation

Codecov / codecov/patch

pkg/pillar/types/dns.go#L384

Added line #L384 was not covered by tests
}

// 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)

Check warning on line 402 in pkg/pillar/types/dns.go

View check run for this annotation

Codecov / codecov/patch

pkg/pillar/types/dns.go#L402

Added line #L402 was not covered by tests
}

// 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 {
Expand All @@ -419,6 +420,9 @@ func getPortsImpl(dns DeviceNetworkStatus, rotation int,
!us.IsMgmt {
continue
}
if l3Only && !us.IsL3Port {
continue
}
if dropFailed && us.HasError() {
continue
}
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 10b6230

Please sign in to comment.