Skip to content

Commit

Permalink
Allow to use network port with VLAN-subinterfaces for untagged traffic
Browse files Browse the repository at this point in the history
With this enhancement, user will be able to use VLAN parent interface
as an L3 endpoint and access the untagged network either for EVE
management or from an application via Local network instance.

Some changes were required to allow having SystemAdapter assigned
to a network port (physical or bond) which is at the same time
referenced by a VLAN sub-interface. Previously, this was not allowed
and resulted in parsing errors published to the controller.

In terms of the Linux network configuration, not many changes were
required. We simply allow assigning static IP or running DHCP client
even for VLAN parent interface.
The only scenario which is not allowed, is to bridge network port and
at the same time use it with VLAN sub-interfaces. This means that
accessing untagged network from a Switch NI is not allowed when the port
is already used with VLAN subinterfaces.

Signed-off-by: Milan Lenco <[email protected]>
  • Loading branch information
milan-zededa committed Nov 20, 2024
1 parent 2db86b8 commit 954d37d
Show file tree
Hide file tree
Showing 14 changed files with 751 additions and 40 deletions.
2 changes: 2 additions & 0 deletions docs/DEVICE-CONNECTIVITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ EVE supports:
- VLAN sub-interfaces with a LAG as the parent
- VLAN filtering for switch network instances. This is actually a feature of the Switch NI,
VLAN network adapter is not used in this case.
- Separating VLANs from untagged network traffic: VLAN sub-interfaces enable access to tagged
VLANs, while the parent port can serve as an L3 endpoint for the untagged segment of the network

Both VLAN and LAG adapters can be used as ports for Local network instance and for EVE
management traffic.
Expand Down
Binary file modified docs/images/eve-vlans-and-lags.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
288 changes: 288 additions & 0 deletions docs/images/eve-vlans-and-lags.xml

Large diffs are not rendered by default.

60 changes: 58 additions & 2 deletions pkg/pillar/cmd/zedagent/parseconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -825,12 +825,50 @@ func parseSystemAdapterConfig(getconfigCtx *getconfigContext, config *zconfig.Ed
portConfig.Key = "bootstrap" // Instead of "zedagent".
}
var newPorts []*types.NetworkPortConfig
logicalLabelToAdapterName := make(map[string]string)
for _, sysAdapter := range sysAdapters {
if sysAdapter.LowerLayerName != "" {
logicalLabelToAdapterName[sysAdapter.LowerLayerName] = sysAdapter.Name
} else {
logicalLabelToAdapterName[sysAdapter.Name] = sysAdapter.Name
}
}
for _, sysAdapter := range sysAdapters {
ports, err := parseOneSystemAdapterConfig(getconfigCtx, sysAdapter, version)
if err != nil {
portConfig.RecordFailure(err.Error())
}
newPorts = append(newPorts, ports...)
for _, port := range ports {
if port.Logicallabel != sysAdapter.Name {
// If a referenced lower-layer port has its own SystemAdapter assigned,
// skip it here and let it be parsed by its own dedicated
// parseOneSystemAdapterConfig call.
_, hasAdapter := logicalLabelToAdapterName[port.Logicallabel]
if hasAdapter {
continue
}
}
newPorts = append(newPorts, port)
}
}
// Make sure that references to lower-layer ports with a SystemAdapter assigned
// use the SystemAdapter.Name, which differs from the LogicalLabel of the port
// when SystemAdapter.Name differs from SystemAdapter.LowerLayerName.
for _, port := range newPorts {
vlanParent := port.L2LinkConfig.VLAN.ParentPort
if vlanParent != "" {
adapterName := logicalLabelToAdapterName[vlanParent]
if adapterName != "" {
port.L2LinkConfig.VLAN.ParentPort = adapterName
}
}
for i := range port.L2LinkConfig.Bond.AggregatedPorts {
aggregatedPort := port.L2LinkConfig.Bond.AggregatedPorts[i]
adapterName := logicalLabelToAdapterName[aggregatedPort]
if adapterName != "" {
port.L2LinkConfig.Bond.AggregatedPorts[i] = adapterName
}
}
}
validateAndAssignNetPorts(portConfig, newPorts)

Expand Down Expand Up @@ -925,6 +963,16 @@ func validateAndAssignNetPorts(dpc *types.DevicePortConfig, newPorts []*types.Ne
port2.RecordFailure(errStr)
break
}
if port.IfName == "" &&
port.PCIAddr == port2.PCIAddr && port.USBAddr == port2.USBAddr {
errStr := fmt.Sprintf(
"Port collides with another port with the same physical address (%s, %s)",
port.PCIAddr, port.USBAddr)
log.Error(errStr)
port.RecordFailure(errStr)
port2.RecordFailure(errStr)
break
}
}
if skip {
continue
Expand Down Expand Up @@ -984,6 +1032,14 @@ func validateAndAssignNetPorts(dpc *types.DevicePortConfig, newPorts []*types.Ne
port.RecordFailure(errStr)
continue
}
if len(l2Refs.bondMasters) > 0 && port.IsL3Port {
errStr := fmt.Sprintf(
"Port %s aggregated by bond (%s) cannot be used with IP configuration",
port.Logicallabel, l2Refs.bondMasters[0].Logicallabel)
log.Error(errStr)
port.RecordFailure(errStr)
continue
}
for i, vlanSubIntf := range l2Refs.vlanSubIntfs {
for j := 0; j < i; j++ {
if vlanSubIntf.VLAN.ID == l2Refs.vlanSubIntfs[j].VLAN.ID {
Expand All @@ -1003,7 +1059,7 @@ func validateAndAssignNetPorts(dpc *types.DevicePortConfig, newPorts []*types.Ne
for len(propagateFrom) > 0 {
var propagateFromNext []*types.NetworkPortConfig
for _, port := range propagateFrom {
if port.IsL3Port || !port.HasError() {
if !port.HasError() {
continue
}
l2Refs := invertedRefs[port.Logicallabel]
Expand Down
195 changes: 186 additions & 9 deletions pkg/pillar/cmd/zedagent/parseconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package zedagent
import (
"crypto/sha256"
"encoding/hex"
"fmt"
"os"
"path/filepath"
"sort"
Expand Down Expand Up @@ -362,6 +363,40 @@ func TestParseVlans(t *testing.T) {
g.Expect(port.L2LinkConfig.L2Type).To(Equal(types.L2LinkTypeNone))
g.Expect(port.WirelessCfg.WType).To(Equal(types.WirelessTypeNone))

// With VLAN sub-interfaces configured, the parent interface can be used
// for the untagged traffic.
config.SystemAdapterList = append(config.SystemAdapterList, &zconfig.SystemAdapter{
Name: "adapter-shopfloor-untagged",
Uplink: false,
NetworkUUID: network1UUID,
LowerLayerName: "shopfloor",
Cost: 5,
})
parseSystemAdapterConfig(getconfigCtx, config, fromController, true)
portConfig, err = getconfigCtx.pubDevicePortConfig.Get("zedagent")
g.Expect(err).To(BeNil())
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.HasError()).To(BeFalse())
// The number of ports has not changed, just the shopfloor ethernet port was
// elevated to L3.
g.Expect(dpc.Ports).To(HaveLen(3))
sortDPCPorts(&dpc)
port = dpc.Ports[0]
g.Expect(port.Logicallabel).To(Equal("adapter-shopfloor-untagged"))
g.Expect(port.Phylabel).To(Equal("ethernet0"))
g.Expect(port.IsL3Port).To(BeTrue()) // This changed from false to true
g.Expect(port.IfName).To(Equal("eth0"))
g.Expect(port.NetworkUUID.String()).To(Equal(network1UUID))
g.Expect(port.Cost).To(BeEquivalentTo(5))
g.Expect(port.DhcpConfig.Type).To(BeEquivalentTo(types.NetworkTypeIPv4))
g.Expect(port.DhcpConfig.Dhcp).To(Equal(types.DhcpTypeClient))
g.Expect(port.L2LinkConfig.L2Type).To(Equal(types.L2LinkTypeNone))
g.Expect(port.WirelessCfg.WType).To(Equal(types.WirelessTypeNone))
port = dpc.Ports[1]
g.Expect(port.Logicallabel).To(Equal("adapter-shopfloor-vlan100"))
port = dpc.Ports[2]
g.Expect(port.Logicallabel).To(Equal("adapter-shopfloor-vlan200"))

// Add adapter for "warehouse-vlan100"
config.SystemAdapterList = append(config.SystemAdapterList, &zconfig.SystemAdapter{
Name: "adapter-warehouse-vlan100",
Expand All @@ -372,7 +407,6 @@ func TestParseVlans(t *testing.T) {
Addr: "192.168.1.150",
})
parseSystemAdapterConfig(getconfigCtx, config, fromController, true)

portConfig, err = getconfigCtx.pubDevicePortConfig.Get("zedagent")
g.Expect(err).To(BeNil())
dpc = portConfig.(types.DevicePortConfig)
Expand All @@ -381,7 +415,7 @@ func TestParseVlans(t *testing.T) {
g.Expect(dpc.Ports).To(HaveLen(5))
sortDPCPorts(&dpc)
// VLAN warehouse.100
port = dpc.Ports[2]
port = dpc.Ports[3]
g.Expect(port.Logicallabel).To(Equal("adapter-warehouse-vlan100"))
g.Expect(port.Phylabel).To(BeEmpty())
g.Expect(port.IsL3Port).To(BeTrue())
Expand Down Expand Up @@ -530,6 +564,24 @@ func TestParseBonds(t *testing.T) {
g.Expect(port.DhcpConfig.Dhcp).To(Equal(types.DhcpTypeNOOP))
g.Expect(port.L2LinkConfig.L2Type).To(Equal(types.L2LinkTypeNone))
g.Expect(port.WirelessCfg.WType).To(Equal(types.WirelessTypeNone))

// It is not allowed to use physical port with IP if it is under a bond.
config.SystemAdapterList = append(config.SystemAdapterList, &zconfig.SystemAdapter{
Name: "shopfloor0",
Uplink: true,
NetworkUUID: networkUUID,
})
parseSystemAdapterConfig(getconfigCtx, config, fromController, true)
portConfig, err = getconfigCtx.pubDevicePortConfig.Get("zedagent")
g.Expect(err).To(BeNil())
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.Ports).To(HaveLen(3))
sortDPCPorts(&dpc)
// underlying physical "shopfloor0" adapter
port = dpc.Ports[1]
g.Expect(port.HasError()).To(BeTrue())
g.Expect(port.LastError).To(Equal(
"Port shopfloor0 aggregated by bond (adapter-shopfloor) cannot be used with IP configuration"))
}

func TestParseVlansOverBonds(t *testing.T) {
Expand Down Expand Up @@ -717,6 +769,72 @@ func TestParseVlansOverBonds(t *testing.T) {
g.Expect(port.DhcpConfig.Dhcp).To(Equal(types.DhcpTypeNOOP))
g.Expect(port.L2LinkConfig.L2Type).To(Equal(types.L2LinkTypeNone))
g.Expect(port.WirelessCfg.WType).To(Equal(types.WirelessTypeNone))

// With VLAN sub-interfaces configured, the parent interface can be used
// for the untagged traffic.
config.SystemAdapterList = append(config.SystemAdapterList, &zconfig.SystemAdapter{
Name: "adapter-shopfloor-untagged",
Uplink: false,
NetworkUUID: network1UUID,
LowerLayerName: "bond-shopfloor",
Cost: 2,
})
parseSystemAdapterConfig(getconfigCtx, config, fromController, true)
portConfig, err = getconfigCtx.pubDevicePortConfig.Get("zedagent")
g.Expect(err).To(BeNil())
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.HasError()).To(BeFalse())
// The number of ports has not changed, just the shopfloor bond was elevated to L3.
g.Expect(dpc.Ports).To(HaveLen(5))
sortDPCPorts(&dpc)
port = dpc.Ports[0]
g.Expect(port.Logicallabel).To(Equal("adapter-shopfloor-untagged"))
g.Expect(port.Phylabel).To(BeEmpty())
g.Expect(port.IsL3Port).To(BeTrue())
g.Expect(port.IsMgmt).To(BeFalse())
g.Expect(port.IfName).To(Equal("bond"))
g.Expect(port.NetworkUUID.String()).To(Equal(network1UUID))
g.Expect(port.Cost).To(BeEquivalentTo(2))
g.Expect(port.DhcpConfig.Type).To(BeEquivalentTo(types.NetworkTypeIPv4))
g.Expect(port.DhcpConfig.Dhcp).To(Equal(types.DhcpTypeClient))
g.Expect(port.L2LinkConfig.L2Type).To(Equal(types.L2LinkTypeBond))
g.Expect(port.L2LinkConfig.Bond.AggregatedPorts).To(Equal([]string{"shopfloor1", "shopfloor0"}))
g.Expect(port.L2LinkConfig.Bond.Mode).To(Equal(types.BondModeActiveBackup))
g.Expect(port.L2LinkConfig.Bond.MIIMonitor.Enabled).To(BeTrue())
g.Expect(port.L2LinkConfig.Bond.MIIMonitor.Interval).To(BeEquivalentTo(400))
g.Expect(port.L2LinkConfig.Bond.MIIMonitor.UpDelay).To(BeEquivalentTo(800))
g.Expect(port.L2LinkConfig.Bond.MIIMonitor.DownDelay).To(BeEquivalentTo(1200))
g.Expect(port.L2LinkConfig.Bond.ARPMonitor.Enabled).To(BeFalse())
g.Expect(port.WirelessCfg.WType).To(Equal(types.WirelessTypeNone))
port = dpc.Ports[1]
g.Expect(port.Logicallabel).To(Equal("adapter-shopfloor-vlan100"))
port = dpc.Ports[2]
g.Expect(port.Logicallabel).To(Equal("adapter-shopfloor-vlan200"))
port = dpc.Ports[3]
g.Expect(port.Logicallabel).To(Equal("shopfloor0"))
port = dpc.Ports[4]
g.Expect(port.Logicallabel).To(Equal("shopfloor1"))

// It is not allowed to use physical port with IP if it is under a bond.
config.SystemAdapterList = append(config.SystemAdapterList, &zconfig.SystemAdapter{
Name: "adapter-ethernet0",
Uplink: true,
LowerLayerName: "shopfloor0",
NetworkUUID: network1UUID,
})
parseSystemAdapterConfig(getconfigCtx, config, fromController, true)
portConfig, err = getconfigCtx.pubDevicePortConfig.Get("zedagent")
g.Expect(err).To(BeNil())
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.Ports).To(HaveLen(5))
sortDPCPorts(&dpc)
port = dpc.Ports[0]
fmt.Printf("%+v\n", dpc)
g.Expect(port.Logicallabel).To(Equal("adapter-ethernet0"))
g.Expect(port.HasError()).To(BeTrue())
g.Expect(port.LastError).To(Equal(
"Port adapter-ethernet0 aggregated by bond (adapter-shopfloor-untagged) cannot be used with IP configuration"))

}

func TestInvalidLowerLayerReferences(t *testing.T) {
Expand Down Expand Up @@ -799,9 +917,9 @@ func TestInvalidLowerLayerReferences(t *testing.T) {
dpc := portConfig.(types.DevicePortConfig)
g.Expect(dpc.HasError()).To(BeTrue())
g.Expect(getPortError(&dpc, "adapter1")).
To(ContainSubstring("Port collides with another port"))
To(ContainSubstring("Port collides with another port with the same interface name"))
g.Expect(getPortError(&dpc, "adapter2")).
To(ContainSubstring("Port collides with another port"))
To(ContainSubstring("Port collides with another port with the same interface name"))
g.Expect(dpc.Ports).To(HaveLen(2))

// fix:
Expand Down Expand Up @@ -889,7 +1007,7 @@ func TestInvalidLowerLayerReferences(t *testing.T) {
g.Expect(dpc.HasError()).To(BeFalse())
g.Expect(dpc.Ports).To(HaveLen(3))

// Scenario 4: interface referenced by both a system adapter and a L2 object
// Scenario 4: interface referenced by both a system adapter and a bond
config = &zconfig.EdgeDevConfig{
Bonds: []*zconfig.BondAdapter{
{
Expand Down Expand Up @@ -921,10 +1039,8 @@ func TestInvalidLowerLayerReferences(t *testing.T) {
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.HasError()).To(BeTrue())
g.Expect(getPortError(&dpc, "adapter-warehouse")).
To(ContainSubstring("Port collides with another port"))
g.Expect(getPortError(&dpc, "adapter-bond-shopfloor")).
To(ContainSubstring("Port collides with another port"))
g.Expect(dpc.Ports).To(HaveLen(4))
To(Equal("Port adapter-warehouse aggregated by bond (adapter-bond-shopfloor) cannot be used with IP configuration"))
g.Expect(dpc.Ports).To(HaveLen(3))

// fix:
config.Bonds[0].LowerLayerNames = []string{"shopfloor"}
Expand Down Expand Up @@ -1142,6 +1258,67 @@ func TestInvalidLowerLayerReferences(t *testing.T) {
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.HasError()).To(BeFalse())
g.Expect(dpc.Ports).To(HaveLen(2))

// Scenario 10: System adapters referencing the same underlying port by physical addresses
// Note that we allow only wwan ports to be defined without interface name.
config = &zconfig.EdgeDevConfig{
DeviceIoList: []*zconfig.PhysicalIO{
{
Ptype: zcommon.PhyIoType_PhyIoNetWWAN,
Phylabel: "ethernet0",
Logicallabel: "shopfloor",
Phyaddrs: map[string]string{
"pcilong": "0000:f4:00.0",
},
Usage: zcommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps,
},
{
Ptype: zcommon.PhyIoType_PhyIoNetWWAN,
Phylabel: "ethernet1",
Logicallabel: "warehouse",
Phyaddrs: map[string]string{
"pcilong": "0000:05:00.0",
},
Usage: zcommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps,
},
},
SystemAdapterList: []*zconfig.SystemAdapter{
{
Name: "adapter1",
Uplink: true,
NetworkUUID: network1UUID,
LowerLayerName: "shopfloor",
Cost: 10,
},
{
Name: "adapter2",
Uplink: true,
NetworkUUID: network2UUID,
LowerLayerName: "shopfloor",
Cost: 20,
},
},
}
parseDeviceIoListConfig(getconfigCtx, config)
parseSystemAdapterConfig(getconfigCtx, config, fromController, true)
portConfig, err = getconfigCtx.pubDevicePortConfig.Get("zedagent")
g.Expect(err).To(BeNil())
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.HasError()).To(BeTrue())
g.Expect(getPortError(&dpc, "adapter1")).
To(ContainSubstring("Port collides with another port with the same physical address"))
g.Expect(getPortError(&dpc, "adapter2")).
To(ContainSubstring("Port collides with another port with the same physical address"))
g.Expect(dpc.Ports).To(HaveLen(2))

// fix:
config.SystemAdapterList[1].LowerLayerName = "warehouse"
parseSystemAdapterConfig(getconfigCtx, config, fromController, true)
portConfig, err = getconfigCtx.pubDevicePortConfig.Get("zedagent")
g.Expect(err).To(BeNil())
dpc = portConfig.(types.DevicePortConfig)
g.Expect(dpc.HasError()).To(BeFalse())
g.Expect(dpc.Ports).To(HaveLen(2))
}

func TestParseSRIOV(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/pillar/cmd/zedrouter/networkinstance.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,14 @@ func (z *zedrouter) updateNIPorts(niConfig types.NetworkInstanceConfig,
port.Logicallabel))
continue
}
if z.deviceNetworkStatus.IsPortUsedAsVlanParent(port.Logicallabel) {
// It is not supported/valid to bridge port which has VLAN
// sub-interfaces configured.
errorMsgs = append(errorMsgs,
fmt.Sprintf("port %s with VLAN sub-interfaces cannot be used "+
"in Switch Network Instance", port.Logicallabel))
continue
}
if len(newPorts) > 1 && port.Dhcp != types.DhcpTypeNone {
errorMsgs = append(errorMsgs,
fmt.Sprintf(
Expand Down
3 changes: 3 additions & 0 deletions pkg/pillar/dpcreconciler/genericitems/netio.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ const (
IOUsageL3Adapter
// IOUsageVlanParent : network IO is used as VLAN parent interface.
IOUsageVlanParent
// IOUsageVlanParentAndL3Adapter : network IO is used as a VLAN parent interface
// and at the same time as an L3 endpoint (for untagged traffic).
IOUsageVlanParentAndL3Adapter
// IOUsageBondAggrIf : network IO is aggregated by Bond interface.
IOUsageBondAggrIf
)
Expand Down
Loading

0 comments on commit 954d37d

Please sign in to comment.