diff --git a/docs/cni-plugin.md b/docs/cni-plugin.md index 7536001d6..e6d9e0dcb 100644 --- a/docs/cni-plugin.md +++ b/docs/cni-plugin.md @@ -56,6 +56,7 @@ Another example with a port which has an interface of type system: * `mtu` (integer, optional): MTU. * `trunk` (optional): List of VLAN ID's and/or ranges of accepted VLAN ID's. +* `vlan_mode` (optional): VLAN mode to set on attached port. Allowed values are [native-untagged,native-tagged,trunk,access,dot1q-tunnel] * `ofport_request` (integer, optional): request a static OpenFlow port number in range 1 to 65,279 * `interface_type` (string, optional): type of the interface belongs to ports. if value is "", ovs will use default interface of type 'internal' * `configuration_path` (optional): configuration file containing ovsdb diff --git a/pkg/ovsdb/ovsdb.go b/pkg/ovsdb/ovsdb.go index 767fc3a41..9ca097590 100644 --- a/pkg/ovsdb/ovsdb.go +++ b/pkg/ovsdb/ovsdb.go @@ -828,9 +828,12 @@ func createPortOperation(intfName, contNetnsPath, contIfaceName string, vlanTag port["vlan_mode"] = portType var err error - if portType == "access" { + + if portType != "trunk" && vlanTag != 0 { port["tag"] = vlanTag - } else if len(trunks) > 0 { + } + + if len(trunks) > 0 { port["trunks"], err = ovsdb.NewOvsSet(trunks) if err != nil { return ovsdb.UUID{}, nil, err diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 5d173a715..a5ff36bdb 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -243,20 +243,33 @@ func CmdAdd(args *skel.CmdArgs) error { var vlanTagNum uint = 0 trunks := make([]uint, 0) - portType := "access" - if netconf.VlanTag == nil || len(netconf.Trunk) > 0 { - portType = "trunk" - if len(netconf.Trunk) > 0 { - trunkVlanIds, err := splitVlanIds(netconf.Trunk) - if err != nil { - return err - } - trunks = append(trunks, trunkVlanIds...) - } - } else if netconf.VlanTag != nil { + + portType := "" + if netconf.VlanMode != "" { + portType = netconf.VlanMode + } + + if netconf.VlanTag != nil { vlanTagNum = *netconf.VlanTag } + if len(netconf.Trunk) > 0 { + trunkVlanIds, err := splitVlanIds(netconf.Trunk) + if err != nil { + return err + } + trunks = append(trunks, trunkVlanIds...) + } + // Assuming an explicit override has not been specified for port type, if tag is nil or trunks are specified set port type to trunk + if (netconf.VlanTag == nil || len(netconf.Trunk) > 0) && portType == "" { + portType = "trunk" + } + + // Assuming portType has not been set at any of the previous checks, fallback to access + if portType == "" { + portType = "access" + } + bridgeName, err := getBridgeName(netconf.BrName, ovnPort) if err != nil { return err @@ -804,9 +817,6 @@ func validateOvs(args *skel.CmdArgs, netconf *types.NetConf, hostIfname string) if *tag != *netconf.VlanTag { return fmt.Errorf("vlan tag mismatch. ovs=%d,netconf=%d", *tag, *netconf.VlanTag) } - if vlanMode != "access" { - return fmt.Errorf("vlan mode mismatch. expected=access,real=%s", vlanMode) - } } // check trunk @@ -827,11 +837,18 @@ func validateOvs(args *skel.CmdArgs, netconf *types.NetConf, hostIfname string) return fmt.Errorf("trunk mismatch. ovs=%v,netconf=%v", trunk, netconfTrunks) } } + } - if vlanMode != "trunk" { - return fmt.Errorf("vlan mode mismatch. expected=trunk,real=%s", vlanMode) + // check vlan mode + if netconf.VlanMode != "" { + if vlanMode == "" { + return fmt.Errorf("vlan mode mismatch. ovs=nil,netconf=%s", netconf.VlanMode) } - } + if vlanMode != netconf.VlanMode { + return fmt.Errorf("vlan mode mismatch. ovs=%s,netconf=%s", vlanMode, netconf.VlanMode) + } + } else { + } return nil } diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index 3cf85e85b..f7e8e0565 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -19,7 +19,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/k8snetworkplumbingwg/ovs-cni/pkg/config" "math/rand" "net" "os/exec" @@ -28,6 +27,8 @@ import ( "syscall" "time" + "github.com/k8snetworkplumbingwg/ovs-cni/pkg/config" + "github.com/containernetworking/cni/pkg/skel" cnitypes "github.com/containernetworking/cni/pkg/types" types040 "github.com/containernetworking/cni/pkg/types/040" @@ -96,6 +97,8 @@ const mtu = 1456 const defaultMTU = 1500 const IFNAME = "eth0" const systemType = "system" +const defaultVlanMode = "access" +const vlanMode = "native-untagged" var _ = BeforeSuite(func() { output, err := exec.Command("ovs-vsctl", "show").CombinedOutput() @@ -330,7 +333,7 @@ var testFunc = func(version string) { Expect(len(brPorts)).To(Equal(0)) } - testAdd := func(conf string, setVlan, setMtu bool, Trunk string, targetNs ns.NetNS) (string, cnitypes.Result) { + testAdd := func(conf string, setVlan, setMtu, setVLanMode bool, Trunk string, targetNs ns.NetNS) (string, cnitypes.Result) { args := &skel.CmdArgs{ ContainerID: "dummy", Netns: targetNs.Path(), @@ -378,6 +381,19 @@ var testFunc = func(version string) { Expect(portVlan).To(Equal("[]")) } + By("Checking that port the VLAN Mode matches expected state") + portVlanMode, err := getPortAttribute(hostIface.Name, "vlan_mode") + Expect(err).NotTo(HaveOccurred()) + if setVLanMode { + Expect(portVlanMode).To(Equal(vlanMode)) + } else { + if !setVlan || Trunk != "" { + Expect(portVlanMode).To(Equal("trunk")) + } else { + Expect(portVlanMode).To(Equal(defaultVlanMode)) + } + } + if setMtu { Expect(hostLink.Attrs().MTU).To(Equal(mtu)) } else { @@ -478,7 +494,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, true, false, "", targetNs) + hostIfName, result := testAdd(conf, true, false, false, "", targetNs) testCheck(conf, result, targetNs) testDel(conf, hostIfName, targetNs, true) }) @@ -495,7 +511,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, false, false, "", targetNs) + hostIfName, result := testAdd(conf, false, false, false, "", targetNs) testCheck(conf, result, targetNs) testDel(conf, hostIfName, targetNs, true) }) @@ -513,7 +529,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, false, false, "[10, 11, 12, 15, 17, 18]", targetNs) + hostIfName, result := testAdd(conf, false, false, false, "[10, 11, 12, 15, 17, 18]", targetNs) testCheck(conf, result, targetNs) testDel(conf, hostIfName, targetNs, true) }) @@ -531,7 +547,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, false, false, "[10, 11, 12, 15, 17, 18]", targetNs) + hostIfName, result := testAdd(conf, false, false, false, "[10, 11, 12, 15, 17, 18]", targetNs) testCheck(conf, result, targetNs) testDel(conf, hostIfName, targetNs, true) }) @@ -609,7 +625,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, false, true, "", targetNs) + hostIfName, result := testAdd(conf, false, true, false, "", targetNs) testCheck(conf, result, targetNs) testDel(conf, hostIfName, targetNs, true) }) @@ -628,7 +644,7 @@ var testFunc = func(version string) { _ = targetNs.Close() _ = testutils.UnmountNS(targetNs) }() - hostIfName, result := testAdd(conf, false, false, "", targetNs) + hostIfName, result := testAdd(conf, false, false, false, "", targetNs) testCheck(conf, result, targetNs) Expect(targetNs.Close()).To(Succeed()) Expect(testutils.UnmountNS(targetNs)).To(Succeed()) @@ -647,7 +663,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, false, false, "", targetNs) + hostIfName, result := testAdd(conf, false, false, false, "", targetNs) testCheck(conf, result, targetNs) err := targetNs.Do(func(ns.NetNS) error { defer GinkgoRecover() @@ -779,7 +795,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, false, false, "", targetNs) + hostIfName, result := testAdd(conf, false, false, false, "", targetNs) testCheck(conf, result, targetNs) testDel(conf, hostIfName, targetNs, true) }) @@ -796,7 +812,7 @@ var testFunc = func(version string) { defer func() { closeNS(targetNs) }() - hostIfName, result := testAdd(conf, false, false, "", targetNs) + hostIfName, result := testAdd(conf, false, false, false, "", targetNs) testCheck(conf, result, targetNs) testDel(conf, hostIfName, targetNs, true) }) @@ -904,6 +920,65 @@ var testFunc = func(version string) { ContainSubstring(secondHostIface.Name), "OVS port with healthy interface should have been kept") }) }) + + Context("with vlan mode set explicitly", func() { + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "ovs", + "bridge": "%s", + "vlan_mode":"%s" + + }`, version, bridgeName, vlanMode) + It("should successfully complete ADD, CHECK and DEL commands", func() { + targetNs := newNS() + defer func() { + closeNS(targetNs) + }() + hostIfName, result := testAdd(conf, false, false, true, "", targetNs) + testCheck(conf, result, targetNs) + testDel(conf, hostIfName, targetNs, true) + }) + }) + Context("with vlan mode not set explicitly", func() { + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "ovs", + "bridge": "%s" + + }`, version, bridgeName) + It("should successfully complete ADD, CHECK and DEL commands", func() { + targetNs := newNS() + defer func() { + closeNS(targetNs) + }() + hostIfName, result := testAdd(conf, false, false, false, "", targetNs) + testCheck(conf, result, targetNs) + testDel(conf, hostIfName, targetNs, true) + }) + }) + Context("with vlan mode set explicitly along with tag and trunk", func() { + conf := fmt.Sprintf(`{ + "cniVersion": "%s", + "name": "mynet", + "type": "ovs", + "bridge": "%s", + "vlan": %d, + "trunk": [ {"minID": 10, "maxID": 12}, {"id": 15}, {"minID": 17, "maxID": 18} ], + "vlan_mode": "%s" + + }`, version, bridgeName, vlanID, vlanMode) + It("should successfully complete ADD, CHECK and DEL commands", func() { + targetNs := newNS() + defer func() { + closeNS(targetNs) + }() + hostIfName, result := testAdd(conf, true, false, true, "", targetNs) + testCheck(conf, result, targetNs) + testDel(conf, hostIfName, targetNs, true) + }) + }) }) } diff --git a/pkg/types/types.go b/pkg/types/types.go index 6e168115c..85676fc4d 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -32,6 +32,7 @@ type NetConf struct { BrName string `json:"bridge,omitempty"` VlanTag *uint `json:"vlan"` MTU int `json:"mtu"` + VlanMode string `json:"vlan_mode,omitempty"` Trunk []*Trunk `json:"trunk,omitempty"` DeviceID string `json:"deviceID"` // PCI address of a VF in valid sysfs format OfportRequest uint `json:"ofport_request"` // OpenFlow port number in range 1 to 65,279