Merge pull request #277 from mlguerrero12/supportvlanproto

Support vlan Proto

docs/configuration-reference.md

@@ -10,6 +10,7 @@ The SR-IOV CNI configures networks through a CNI spec configuration object. In a
 * `deviceID` (string, required): A valid pci address of an SRIOV NIC's VF. e.g. "0000:03:02.3"
 * `vlan` (int, optional): VLAN ID to assign for the VF. Value must be in the range 0-4094 (0 for disabled, 1-4094 for valid VLAN IDs).
 * `vlanQoS` (int, optional): VLAN QoS to assign for the VF. Value must be in the range 0-7. This option requires `vlan` field to be set to a non-zero value. Otherwise, the error will be returned.
+* `vlanProto` (string, optional): VLAN protocol to assign for the VF. Allowed values: "802.1ad", "802.1q" (default).
 * `mac` (string, optional): MAC address to assign for the VF
 * `spoofchk` (string, optional): turn packet spoof checking on or off for the VF
 * `trust` (string, optional): turn trust setting on or off for the VF
@@ -27,9 +28,10 @@ An SR-IOV CNI config with each field filled out looks like:
     "name": "sriov-dpdk",
     "type": "sriovi-net",
     "deviceID": "0000:03:02.0",
-    "vlan": 1000,
     "mac": "CA:FE:C0:FF:EE:00",
+    "vlan": 1000,
     "vlanQoS": 4,
+    "vlanProto": "802.1ad",
     "min_tx_rate": 100,
     "max_tx_rate": 200,
     "spoofchk": "off",

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.9.2
 	github.com/onsi/gomega v1.27.5
 	github.com/stretchr/testify v1.6.1
-	github.com/vishvananda/netlink v1.2.1-beta.2
+	github.com/vishvananda/netlink v1.2.1-beta.2.0.20230905152006-63484bbf69f8
 	golang.org/x/net v0.8.0
 )
 
@@ -22,8 +22,8 @@ require (
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/safchain/ethtool v0.2.0 // indirect
 	github.com/stretchr/objx v0.1.0 // indirect
-	github.com/vishvananda/netns v0.0.2 // indirect
-	golang.org/x/sys v0.6.0 // indirect
+	github.com/vishvananda/netns v0.0.4 // indirect
+	golang.org/x/sys v0.12.0 // indirect
 	golang.org/x/text v0.8.0 // indirect
 	golang.org/x/tools v0.7.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

go.sum

@@ -39,11 +39,11 @@ github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/vishvananda/netlink v1.2.1-beta.2 h1:Llsql0lnQEbHj0I1OuKyp8otXp0r3q0mPkuhwHfStVs=
-github.com/vishvananda/netlink v1.2.1-beta.2/go.mod h1:twkDnbuQxJYemMlGd4JFIcuhgX83tXhKS2B/PRMpOho=
+github.com/vishvananda/netlink v1.2.1-beta.2.0.20230905152006-63484bbf69f8 h1:xxfANIR2aBrzBufGZZkbIg0N4V1AEmw0hKWlWv8eMYM=
+github.com/vishvananda/netlink v1.2.1-beta.2.0.20230905152006-63484bbf69f8/go.mod h1:whJevzBpTrid75eZy99s3DqCmy05NfibNaF2Ol5Ox5A=
 github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae/go.mod h1:DD4vA1DwXk04H54A1oHXtwZmA0grkVMdPxx/VGLCah0=
-github.com/vishvananda/netns v0.0.2 h1:Cn05BRLm+iRP/DZxyVSsfVyrzgjDbwHwkVt38qvXnNI=
-github.com/vishvananda/netns v0.0.2/go.mod h1:yitZXdAVI+yPFSb4QUe+VW3vOVl4PZPNcBgbPxAtJxw=
+github.com/vishvananda/netns v0.0.4 h1:Oeaw1EM2JMxD51g9uhtC0D7erkIjgmj8+JZc26m1YX8=
+github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZlaprSwhNNJM=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
@@ -62,16 +62,16 @@ golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200217220822-9197077df867/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o=
+golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=

pkg/config/config.go

@@ -69,28 +69,38 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) {
 		return nil, fmt.Errorf("LoadConf(): the VF %s does not have a interface name or a dpdk driver", n.DeviceID)
 	}
 
-	if n.Vlan != nil {
-		// validate vlan id range
-		if *n.Vlan < 0 || *n.Vlan > 4094 {
-			return nil, fmt.Errorf("LoadConf(): vlan id %d invalid: value must be in the range 0-4094", *n.Vlan)
-		}
+	if n.Vlan == nil {
+		vlan := 0
+		n.Vlan = &vlan
 	}
 
-	if n.VlanQoS != nil {
-		// validate that VLAN QoS is in the 0-7 range
-		if *n.VlanQoS < 0 || *n.VlanQoS > 7 {
-			return nil, fmt.Errorf("LoadConf(): vlan QoS PCP %d invalid: value must be in the range 0-7", *n.VlanQoS)
-		}
+	// validate vlan id range
+	if *n.Vlan < 0 || *n.Vlan > 4094 {
+		return nil, fmt.Errorf("LoadConf(): vlan id %d invalid: value must be in the range 0-4094", *n.Vlan)
+	}
+
+	if *n.Vlan == 0 && (n.VlanQoS != nil || n.VlanProto != nil) {
+		return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan Qos and/or Proto")
+	}
+
+	if n.VlanQoS == nil {
+		qos := 0
+		n.VlanQoS = &qos
+	}
+
+	// validate that VLAN QoS is in the 0-7 range
+	if *n.VlanQoS < 0 || *n.VlanQoS > 7 {
+		return nil, fmt.Errorf("LoadConf(): vlan QoS PCP %d invalid: value must be in the range 0-7", *n.VlanQoS)
 	}
 
-	// validate that vlan id is set if vlan qos is set
-	if n.VlanQoS != nil && n.Vlan == nil {
-		return nil, fmt.Errorf(("LoadConf(): vlan id must be configured to set vlan QoS"))
+	if n.VlanProto == nil {
+		proto := sriovtypes.Proto8021q
+		n.VlanProto = &proto
 	}
 
-	// validate non-zero value for vlan id if vlan qos is set to a non-zero value
-	if (n.VlanQoS != nil && *n.VlanQoS != 0) && *n.Vlan == 0 {
-		return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan QoS to a non-zero value")
+	*n.VlanProto = strings.ToLower(*n.VlanProto)
+	if *n.VlanProto != sriovtypes.Proto8021ad && *n.VlanProto != sriovtypes.Proto8021q {
+		return nil, fmt.Errorf("LoadConf(): vlan Proto %s invalid: value must be '802.1Q' or '802.1ad'", *n.VlanProto)
 	}
 
 	// validate that link state is one of supported values

pkg/config/config_test.go

@@ -1,12 +1,15 @@
 package config
 
 import (
+	"fmt"
+	"os"
+
 	"github.com/containernetworking/plugins/pkg/testutils"
-	"github.com/k8snetworkplumbingwg/sriov-cni/pkg/types"
-	"github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"os"
+
+	"github.com/k8snetworkplumbingwg/sriov-cni/pkg/types"
+	"github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils"
 )
 
 var _ = Describe("Config", func() {
@@ -66,6 +69,55 @@ var _ = Describe("Config", func() {
 			Expect(err).To(HaveOccurred())
 		})
 
+		validVlanID := 100
+		zeroVlanID := 0
+		invalidVlanID := 5000
+		validQoS := 1
+		invalidQoS := 10
+		valid8021qProto := "802.1Q"
+		valid8021adProto := "802.1ad"
+		invalidProto := "802"
+		DescribeTable("Vlan ID, QoS and Proto",
+			func(vlanID *int, vlanQoS *int, vlanProto *string, failure bool) {
+				s := `{
+        "name": "mynet",
+        "type": "sriov",
+        "deviceID": "0000:af:06.1",
+        "vf": 0`
+				if vlanID != nil {
+					s = fmt.Sprintf(`%s,
+        "vlan": %d`, s, *vlanID)
+				}
+				if vlanQoS != nil {
+					s = fmt.Sprintf(`%s,
+        "vlanQoS": %d`, s, *vlanQoS)
+				}
+				if vlanProto != nil {
+					s = fmt.Sprintf(`%s,
+        "vlanProto": "%s"`, s, *vlanProto)
+				}
+				s = fmt.Sprintf(`%s
+                        }`, s)
+				conf := []byte(s)
+				_, err := LoadConf(conf)
+				if failure {
+					Expect(err).To(HaveOccurred())
+				} else {
+					Expect(err).ToNot(HaveOccurred())
+				}
+			},
+			Entry("valid vlan ID", &validVlanID, nil, nil, false),
+			Entry("invalid vlan ID", &invalidVlanID, nil, nil, true),
+			Entry("vlan ID equal to zero and QoS set", &zeroVlanID, &validQoS, nil, true),
+			Entry("vlan ID equal to zero and Proto set", &zeroVlanID, nil, &valid8021qProto, true),
+			Entry("invalid QoS", &validVlanID, &invalidQoS, nil, true),
+			Entry("invalid Proto", &validVlanID, nil, &invalidProto, true),
+			Entry("valid 802.1q Proto", &validVlanID, nil, &valid8021qProto, false),
+			Entry("valid 802.1ad Proto", &validVlanID, nil, &valid8021adProto, false),
+			Entry("no vlan ID and QoS set", nil, &validQoS, nil, true),
+			Entry("no vlan ID and Proto set", nil, nil, &valid8021adProto, true),
+		)
+
 		It("Assuming device is allocated", func() {
 			conf := []byte(`{
         "name": "mynet",

pkg/sriov/sriov.go

@@ -175,21 +175,8 @@ func (s *sriovManager) ApplyVFConfig(conf *sriovtypes.NetConf) error {
 		return fmt.Errorf("failed to lookup master %q: %v", conf.Master, err)
 	}
 	// 1. Set vlan
-	if conf.Vlan == nil {
-		vlan := new(int)
-		*vlan = 0
-		conf.Vlan = vlan
-	}
-	// set vlan qos if present in the config
-	if conf.VlanQoS != nil {
-		if err = s.nLink.LinkSetVfVlanQos(pfLink, conf.VFID, *conf.Vlan, *conf.VlanQoS); err != nil {
-			return fmt.Errorf("failed to set vf %d vlan configuration: %v", conf.VFID, err)
-		}
-	} else {
-		// set vlan id field only
-		if err = s.nLink.LinkSetVfVlan(pfLink, conf.VFID, *conf.Vlan); err != nil {
-			return fmt.Errorf("failed to set vf %d vlan: %v", conf.VFID, err)
-		}
+	if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, *conf.Vlan, *conf.VlanQoS, sriovtypes.VlanProtoInt[*conf.VlanProto]); err != nil {
+		return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %s: %v", conf.VFID, *conf.Vlan, *conf.VlanQoS, *conf.VlanProto, err)
 	}
 
 	// 2. Set mac address
@@ -287,15 +274,13 @@ func (s *sriovManager) ResetVFConfig(conf *sriovtypes.NetConf) error {
 		return fmt.Errorf("failed to lookup master %q: %v", conf.Master, err)
 	}
 
-	// Restore VLAN
-	if conf.Vlan != nil {
-		if conf.VlanQoS != nil {
-			if err = s.nLink.LinkSetVfVlanQos(pfLink, conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS); err != nil {
-				return fmt.Errorf("failed to restore vf %d vlan: %v", conf.VFID, err)
-			}
-		} else if err = s.nLink.LinkSetVfVlan(pfLink, conf.VFID, conf.OrigVfState.Vlan); err != nil {
-			return fmt.Errorf("failed to restore vf %d vlan: %v", conf.VFID, err)
-		}
+	// Set 802.1q as default in case cache config does not have a value for vlan proto.
+	if conf.OrigVfState.VlanProto == 0 {
+		conf.OrigVfState.VlanProto = sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]
+	}
+
+	if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto); err != nil {
+		return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %d: %v", conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto, err)
 	}
 
 	// Restore spoofchk

pkg/sriov/sriov_test.go

@@ -296,6 +296,7 @@ var _ = Describe("Sriov", func() {
 			fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink"}}
 
 			mocked.On("LinkByName", netconf.Master).Return(fakeLink, nil)
+			mocked.On("LinkSetVfVlanQosProto", fakeLink, netconf.VFID, netconf.OrigVfState.Vlan, netconf.OrigVfState.VlanQoS, sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]).Return(nil)
 			sm := sriovManager{nLink: mocked}
 			err := sm.ResetVFConfig(netconf)
 			Expect(err).NotTo(HaveOccurred())
@@ -347,7 +348,7 @@ var _ = Describe("Sriov", func() {
 			}}}
 
 			mocked.On("LinkByName", netconf.Master).Return(fakeLink, nil)
-			mocked.On("LinkSetVfVlanQos", fakeLink, netconf.VFID, netconf.OrigVfState.Vlan, netconf.OrigVfState.VlanQoS).Return(nil)
+			mocked.On("LinkSetVfVlanQosProto", fakeLink, netconf.VFID, netconf.OrigVfState.Vlan, netconf.OrigVfState.VlanQoS, sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]).Return(nil)
 			mocked.On("LinkSetVfSpoofchk", fakeLink, netconf.VFID, netconf.OrigVfState.SpoofChk).Return(nil)
 			mocked.On("LinkSetVfHardwareAddr", fakeLink, netconf.VFID, origMac).Return(nil)
 			mocked.On("LinkSetVfTrust", fakeLink, netconf.VFID, false).Return(nil)

pkg/types/types.go

@@ -5,6 +5,13 @@ import (
 	"github.com/vishvananda/netlink"
 )
 
+const (
+	Proto8021q  = "802.1q"
+	Proto8021ad = "802.1ad"
+)
+
+var VlanProtoInt = map[string]int{Proto8021q: 33024, Proto8021ad: 34984}
+
 // VfState represents the state of the VF
 type VfState struct {
 	HostIFName   string
@@ -14,6 +21,7 @@ type VfState struct {
 	EffectiveMAC string
 	Vlan         int
 	VlanQoS      int
+	VlanProto    int
 	MinTxRate    int
 	MaxTxRate    int
 	LinkState    uint32
@@ -27,6 +35,7 @@ func (vs *VfState) FillFromVfInfo(info *netlink.VfInfo) {
 	vs.MinTxRate = int(info.MinTxRate)
 	vs.Vlan = info.Vlan
 	vs.VlanQoS = info.Qos
+	vs.VlanProto = info.VlanProto
 	vs.SpoofChk = info.Spoofchk
 	vs.Trust = info.Trust != 0
 }
@@ -38,9 +47,10 @@ type NetConf struct {
 	DPDKMode      bool    `json:"-"`
 	Master        string
 	MAC           string
-	Vlan          *int   `json:"vlan"`
-	VlanQoS       *int   `json:"vlanQoS"`
-	DeviceID      string `json:"deviceID"` // PCI address of a VF in valid sysfs format
+	Vlan          *int    `json:"vlan"`
+	VlanQoS       *int    `json:"vlanQoS"`
+	VlanProto     *string `json:"vlanProto"` // 802.1ad|802.1q
+	DeviceID      string  `json:"deviceID"`  // PCI address of a VF in valid sysfs format
 	VFID          int
 	MinTxRate     *int   `json:"min_tx_rate"`          // Mbps, 0 = disable rate limiting
 	MaxTxRate     *int   `json:"max_tx_rate"`          // Mbps, 0 = disable rate limiting

pkg/utils/netlink_manager.go

@@ -11,8 +11,7 @@ import (
 // NetlinkManager is an interface to mock nelink library
 type NetlinkManager interface {
 	LinkByName(string) (netlink.Link, error)
-	LinkSetVfVlan(netlink.Link, int, int) error
-	LinkSetVfVlanQos(netlink.Link, int, int, int) error
+	LinkSetVfVlanQosProto(netlink.Link, int, int, int, int) error
 	LinkSetVfHardwareAddr(netlink.Link, int, net.HardwareAddr) error
 	LinkSetHardwareAddr(netlink.Link, net.HardwareAddr) error
 	LinkSetUp(netlink.Link) error
@@ -35,14 +34,9 @@ func (n *MyNetlink) LinkByName(name string) (netlink.Link, error) {
 	return netlink.LinkByName(name)
 }
 
-// LinkSetVfVlan using NetlinkManager
-func (n *MyNetlink) LinkSetVfVlan(link netlink.Link, vf, vlan int) error {
-	return netlink.LinkSetVfVlan(link, vf, vlan)
-}
-
-// LinkSetVfVlanQos sets VLAN ID and QoS field for given VF using NetlinkManager
-func (n *MyNetlink) LinkSetVfVlanQos(link netlink.Link, vf, vlan, qos int) error {
-	return netlink.LinkSetVfVlanQos(link, vf, vlan, qos)
+// LinkSetVfVlanQosProto sets VLAN ID, QoS and Proto field for given VF using NetlinkManager
+func (n *MyNetlink) LinkSetVfVlanQosProto(link netlink.Link, vf, vlan, qos, proto int) error {
+	return netlink.LinkSetVfVlanQosProto(link, vf, vlan, qos, proto)
 }
 
 // LinkSetVfHardwareAddr using NetlinkManager