Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to specify vlan_mode in the network configuration #261

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Ability to specify vlan_mode in the network configuration #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/cni-plugin.md
View file
Open in desktop
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
7 changes: 5 additions & 2 deletions pkg/ovsdb/ovsdb.go
View file
Open in desktop
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 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
Expand Down
51 changes: 34 additions & 17 deletions pkg/plugin/plugin.go
View file
Open in desktop
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 {

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails the linter check

return nil
}
97 changes: 86 additions & 11 deletions pkg/plugin/plugin_test.go
View file
Open in desktop
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
Open in desktop
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