Skip to content

Commit c885d62

Browse files
author
aghaiand
committed
Added ability for specifying an explicit vlan mode to be applied to port being attached
Signed-off-by: David Aghaian <[email protected]>
1 parent a4385e4 commit c885d62

File tree

5 files changed

+127
-30
lines changed

5 files changed

+127
-30
lines changed

‎docs/cni-plugin.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ Another example with a port which has an interface of type system:
5656
* `mtu` (integer, optional): MTU.
5757
* `trunk` (optional): List of VLAN ID's and/or ranges of accepted VLAN
5858
ID's.
59+
* `vlan_mode` (optional): VLAN mode to set on attached port. Allowed values are [native-untagged,native-tagged,trunk,access,dot1q-tunnel]
5960
* `ofport_request` (integer, optional): request a static OpenFlow port number in range 1 to 65,279
6061
* `interface_type` (string, optional): type of the interface belongs to ports. if value is "", ovs will use default interface of type 'internal'
6162
* `configuration_path` (optional): configuration file containing ovsdb

‎pkg/ovsdb/ovsdb.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -828,9 +828,12 @@ func createPortOperation(intfName, contNetnsPath, contIfaceName string, vlanTag
828828

829829
port["vlan_mode"] = portType
830830
var err error
831-
if portType == "access" {
831+
832+
if portType != "trunk" && vlanTag != 0 {
832833
port["tag"] = vlanTag
833-
} else if len(trunks) > 0 {
834+
}
835+
836+
if len(trunks) > 0 {
834837
port["trunks"], err = ovsdb.NewOvsSet(trunks)
835838
if err != nil {
836839
return ovsdb.UUID{}, nil, err

‎pkg/plugin/plugin.go‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -243,20 +243,33 @@ func CmdAdd(args *skel.CmdArgs) error {
243243

244244
var vlanTagNum uint = 0
245245
trunks := make([]uint, 0)
246-
portType := "access"
247-
if netconf.VlanTag == nil || len(netconf.Trunk) > 0 {
248-
portType = "trunk"
249-
if len(netconf.Trunk) > 0 {
250-
trunkVlanIds, err := splitVlanIds(netconf.Trunk)
251-
if err != nil {
252-
return err
253-
}
254-
trunks = append(trunks, trunkVlanIds...)
255-
}
256-
} else if netconf.VlanTag != nil {
246+
247+
portType := ""
248+
if netconf.VlanMode != "" {
249+
portType = netconf.VlanMode
250+
}
251+
252+
if netconf.VlanTag != nil {
257253
vlanTagNum = *netconf.VlanTag
258254
}
259255

256+
if len(netconf.Trunk) > 0 {
257+
trunkVlanIds, err := splitVlanIds(netconf.Trunk)
258+
if err != nil {
259+
return err
260+
}
261+
trunks = append(trunks, trunkVlanIds...)
262+
}
263+
// Assuming an explicit override has not been specified for port type, if tag is nil or trunks are specified set port type to trunk
264+
if (netconf.VlanTag == nil || len(netconf.Trunk) > 0) && portType == "" {
265+
portType = "trunk"
266+
}
267+
268+
// Assuming portType has not been set at any of the previous checks, fallback to access
269+
if portType == "" {
270+
portType = "access"
271+
}
272+
260273
bridgeName, err := getBridgeName(netconf.BrName, ovnPort)
261274
if err != nil {
262275
return err
@@ -804,9 +817,6 @@ func validateOvs(args *skel.CmdArgs, netconf *types.NetConf, hostIfname string)
804817
if *tag != *netconf.VlanTag {
805818
return fmt.Errorf("vlan tag mismatch. ovs=%d,netconf=%d", *tag, *netconf.VlanTag)
806819
}
807-
if vlanMode != "access" {
808-
return fmt.Errorf("vlan mode mismatch. expected=access,real=%s", vlanMode)
809-
}
810820
}
811821

812822
// check trunk
@@ -827,11 +837,18 @@ func validateOvs(args *skel.CmdArgs, netconf *types.NetConf, hostIfname string)
827837
return fmt.Errorf("trunk mismatch. ovs=%v,netconf=%v", trunk, netconfTrunks)
828838
}
829839
}
840+
}
830841

831-
if vlanMode != "trunk" {
832-
return fmt.Errorf("vlan mode mismatch. expected=trunk,real=%s", vlanMode)
842+
// check vlan mode
843+
if netconf.VlanMode != "" {
844+
if vlanMode == "" {
845+
return fmt.Errorf("vlan mode mismatch. ovs=nil,netconf=%s", netconf.VlanMode)
833846
}
834-
}
847+
if vlanMode != netconf.VlanMode {
848+
return fmt.Errorf("vlan mode mismatch. ovs=%s,netconf=%s", vlanMode, netconf.VlanMode)
849+
}
850+
} else {
835851

852+
}
836853
return nil
837854
}

‎pkg/plugin/plugin_test.go‎

Lines changed: 86 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"encoding/json"
2020
"errors"
2121
"fmt"
22-
"github.com/k8snetworkplumbingwg/ovs-cni/pkg/config"
2322
"math/rand"
2423
"net"
2524
"os/exec"
@@ -28,6 +27,8 @@ import (
2827
"syscall"
2928
"time"
3029

30+
"github.com/k8snetworkplumbingwg/ovs-cni/pkg/config"
31+
3132
"github.com/containernetworking/cni/pkg/skel"
3233
cnitypes "github.com/containernetworking/cni/pkg/types"
3334
types040 "github.com/containernetworking/cni/pkg/types/040"
@@ -96,6 +97,8 @@ const mtu = 1456
9697
const defaultMTU = 1500
9798
const IFNAME = "eth0"
9899
const systemType = "system"
100+
const defaultVlanMode = "access"
101+
const vlanMode = "native-untagged"
99102

100103
var _ = BeforeSuite(func() {
101104
output, err := exec.Command("ovs-vsctl", "show").CombinedOutput()
@@ -330,7 +333,7 @@ var testFunc = func(version string) {
330333
Expect(len(brPorts)).To(Equal(0))
331334
}
332335

333-
testAdd := func(conf string, setVlan, setMtu bool, Trunk string, targetNs ns.NetNS) (string, cnitypes.Result) {
336+
testAdd := func(conf string, setVlan, setMtu, setVLanMode bool, Trunk string, targetNs ns.NetNS) (string, cnitypes.Result) {
334337
args := &skel.CmdArgs{
335338
ContainerID: "dummy",
336339
Netns: targetNs.Path(),
@@ -378,6 +381,19 @@ var testFunc = func(version string) {
378381
Expect(portVlan).To(Equal("[]"))
379382
}
380383

384+
By("Checking that port the VLAN Mode matches expected state")
385+
portVlanMode, err := getPortAttribute(hostIface.Name, "vlan_mode")
386+
Expect(err).NotTo(HaveOccurred())
387+
if setVLanMode {
388+
Expect(portVlanMode).To(Equal(vlanMode))
389+
} else {
390+
if !setVlan || Trunk != "" {
391+
Expect(portVlanMode).To(Equal("trunk"))
392+
} else {
393+
Expect(portVlanMode).To(Equal(defaultVlanMode))
394+
}
395+
}
396+
381397
if setMtu {
382398
Expect(hostLink.Attrs().MTU).To(Equal(mtu))
383399
} else {
@@ -478,7 +494,7 @@ var testFunc = func(version string) {
478494
defer func() {
479495
closeNS(targetNs)
480496
}()
481-
hostIfName, result := testAdd(conf, true, false, "", targetNs)
497+
hostIfName, result := testAdd(conf, true, false, false, "", targetNs)
482498
testCheck(conf, result, targetNs)
483499
testDel(conf, hostIfName, targetNs, true)
484500
})
@@ -495,7 +511,7 @@ var testFunc = func(version string) {
495511
defer func() {
496512
closeNS(targetNs)
497513
}()
498-
hostIfName, result := testAdd(conf, false, false, "", targetNs)
514+
hostIfName, result := testAdd(conf, false, false, false, "", targetNs)
499515
testCheck(conf, result, targetNs)
500516
testDel(conf, hostIfName, targetNs, true)
501517
})
@@ -513,7 +529,7 @@ var testFunc = func(version string) {
513529
defer func() {
514530
closeNS(targetNs)
515531
}()
516-
hostIfName, result := testAdd(conf, false, false, "[10, 11, 12, 15, 17, 18]", targetNs)
532+
hostIfName, result := testAdd(conf, false, false, false, "[10, 11, 12, 15, 17, 18]", targetNs)
517533
testCheck(conf, result, targetNs)
518534
testDel(conf, hostIfName, targetNs, true)
519535
})
@@ -531,7 +547,7 @@ var testFunc = func(version string) {
531547
defer func() {
532548
closeNS(targetNs)
533549
}()
534-
hostIfName, result := testAdd(conf, false, false, "[10, 11, 12, 15, 17, 18]", targetNs)
550+
hostIfName, result := testAdd(conf, false, false, false, "[10, 11, 12, 15, 17, 18]", targetNs)
535551
testCheck(conf, result, targetNs)
536552
testDel(conf, hostIfName, targetNs, true)
537553
})
@@ -609,7 +625,7 @@ var testFunc = func(version string) {
609625
defer func() {
610626
closeNS(targetNs)
611627
}()
612-
hostIfName, result := testAdd(conf, false, true, "", targetNs)
628+
hostIfName, result := testAdd(conf, false, true, false, "", targetNs)
613629
testCheck(conf, result, targetNs)
614630
testDel(conf, hostIfName, targetNs, true)
615631
})
@@ -628,7 +644,7 @@ var testFunc = func(version string) {
628644
_ = targetNs.Close()
629645
_ = testutils.UnmountNS(targetNs)
630646
}()
631-
hostIfName, result := testAdd(conf, false, false, "", targetNs)
647+
hostIfName, result := testAdd(conf, false, false, false, "", targetNs)
632648
testCheck(conf, result, targetNs)
633649
Expect(targetNs.Close()).To(Succeed())
634650
Expect(testutils.UnmountNS(targetNs)).To(Succeed())
@@ -647,7 +663,7 @@ var testFunc = func(version string) {
647663
defer func() {
648664
closeNS(targetNs)
649665
}()
650-
hostIfName, result := testAdd(conf, false, false, "", targetNs)
666+
hostIfName, result := testAdd(conf, false, false, false, "", targetNs)
651667
testCheck(conf, result, targetNs)
652668
err := targetNs.Do(func(ns.NetNS) error {
653669
defer GinkgoRecover()
@@ -779,7 +795,7 @@ var testFunc = func(version string) {
779795
defer func() {
780796
closeNS(targetNs)
781797
}()
782-
hostIfName, result := testAdd(conf, false, false, "", targetNs)
798+
hostIfName, result := testAdd(conf, false, false, false, "", targetNs)
783799
testCheck(conf, result, targetNs)
784800
testDel(conf, hostIfName, targetNs, true)
785801
})
@@ -796,7 +812,7 @@ var testFunc = func(version string) {
796812
defer func() {
797813
closeNS(targetNs)
798814
}()
799-
hostIfName, result := testAdd(conf, false, false, "", targetNs)
815+
hostIfName, result := testAdd(conf, false, false, false, "", targetNs)
800816
testCheck(conf, result, targetNs)
801817
testDel(conf, hostIfName, targetNs, true)
802818
})
@@ -904,6 +920,65 @@ var testFunc = func(version string) {
904920
ContainSubstring(secondHostIface.Name), "OVS port with healthy interface should have been kept")
905921
})
906922
})
923+
924+
Context("with vlan mode set explicitly", func() {
925+
conf := fmt.Sprintf(`{
926+
"cniVersion": "%s",
927+
"name": "mynet",
928+
"type": "ovs",
929+
"bridge": "%s",
930+
"vlan_mode":"%s"
931+
932+
}`, version, bridgeName, vlanMode)
933+
It("should successfully complete ADD, CHECK and DEL commands", func() {
934+
targetNs := newNS()
935+
defer func() {
936+
closeNS(targetNs)
937+
}()
938+
hostIfName, result := testAdd(conf, false, false, true, "", targetNs)
939+
testCheck(conf, result, targetNs)
940+
testDel(conf, hostIfName, targetNs, true)
941+
})
942+
})
943+
Context("with vlan mode not set explicitly", func() {
944+
conf := fmt.Sprintf(`{
945+
"cniVersion": "%s",
946+
"name": "mynet",
947+
"type": "ovs",
948+
"bridge": "%s"
949+
950+
}`, version, bridgeName)
951+
It("should successfully complete ADD, CHECK and DEL commands", func() {
952+
targetNs := newNS()
953+
defer func() {
954+
closeNS(targetNs)
955+
}()
956+
hostIfName, result := testAdd(conf, false, false, false, "", targetNs)
957+
testCheck(conf, result, targetNs)
958+
testDel(conf, hostIfName, targetNs, true)
959+
})
960+
})
961+
Context("with vlan mode set explicitly along with tag and trunk", func() {
962+
conf := fmt.Sprintf(`{
963+
"cniVersion": "%s",
964+
"name": "mynet",
965+
"type": "ovs",
966+
"bridge": "%s",
967+
"vlan": %d,
968+
"trunk": [ {"minID": 10, "maxID": 12}, {"id": 15}, {"minID": 17, "maxID": 18} ],
969+
"vlan_mode": "%s"
970+
971+
}`, version, bridgeName, vlanID, vlanMode)
972+
It("should successfully complete ADD, CHECK and DEL commands", func() {
973+
targetNs := newNS()
974+
defer func() {
975+
closeNS(targetNs)
976+
}()
977+
hostIfName, result := testAdd(conf, true, false, true, "", targetNs)
978+
testCheck(conf, result, targetNs)
979+
testDel(conf, hostIfName, targetNs, true)
980+
})
981+
})
907982
})
908983
}
909984

‎pkg/types/types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type NetConf struct {
3232
BrName string `json:"bridge,omitempty"`
3333
VlanTag *uint `json:"vlan"`
3434
MTU int `json:"mtu"`
35+
VlanMode string `json:"vlan_mode,omitempty"`
3536
Trunk []*Trunk `json:"trunk,omitempty"`
3637
DeviceID string `json:"deviceID"` // PCI address of a VF in valid sysfs format
3738
OfportRequest uint `json:"ofport_request"` // OpenFlow port number in range 1 to 65,279

0 commit comments

Comments
 (0)