Skip to content

Commit

Permalink
Added ability for specifying an explicit vlan mode to be applied to p…
Browse files Browse the repository at this point in the history 
…ort being attached

Signed-off-by: David Aghaian <[email protected]>
  • Loading branch information
aghaiand committed Jan 20, 2023
1 parent a4385e4 commit 6e0e9a1
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 30 deletions.
1 change: 1 addition & 0 deletions docs/cni-plugin.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 16 additions & 2 deletions pkg/ovsdb/ovsdb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -828,9 +828,12 @@ func createPortOperation(intfName, contNetnsPath, contIfaceName string, vlanTag

port["vlan_mode"] = portType
var err error
if portType == "access" {

if validTypeForTag(portType) && 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
Expand Down Expand Up @@ -1133,3 +1136,14 @@ func convertToArray(elem interface{}) ([]interface{}, error) {
}
return nil, errors.New("unknown type")
}

// utility function to verify whether current port type allows for tags to be set
func validTypeForTag(portType string) bool {
validTypes := []string{"access", "native-tagged", "native-untagged", "dot1q-tunnel"}
for _, t := range validTypes {
if t == portType {
return true
}
}
return false
}
51 changes: 34 additions & 17 deletions pkg/plugin/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
97 changes: 86 additions & 11 deletions pkg/plugin/plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/k8snetworkplumbingwg/ovs-cni/pkg/config"
"math/rand"
"net"
"os/exec"
Expand All @@ -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"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})
Expand All @@ -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)
})
Expand All @@ -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)
})
Expand All @@ -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)
})
Expand Down Expand Up @@ -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)
})
Expand All @@ -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())
Expand All @@ -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()
Expand Down Expand Up @@ -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)
})
Expand All @@ -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)
})
Expand Down Expand Up @@ -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)
})
})
})
}

Expand Down
1 change: 1 addition & 0 deletions pkg/types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 6e0e9a1

Please sign in to comment.