Fix checks for vlan parameters

- Add check to not allow non default values of qos and proto when vlan is zero. - Remove check when vlan is zero and qos and proto are not in config since default values are now set. Signed-off-by: Marcelo Guerrero <[email protected]>
k8snetworkplumbingwg · Oct 5, 2023 · f76881b · f76881b
1 parent f0e62c3
commit f76881b
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 8 deletions.
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -79,10 +79,6 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) {
 		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
@@ -93,6 +89,11 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) {
 		return nil, fmt.Errorf("LoadConf(): vlan QoS PCP %d invalid: value must be in the range 0-7", *n.VlanQoS)
 	}
 
+	// validate non-zero value for vlan id if vlan qos is set to a non-zero value
+	if *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")
+	}
+
 	if n.VlanProto == nil {
 		proto := sriovtypes.Proto8021q
 		n.VlanProto = &proto
@@ -103,6 +104,11 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) {
 		return nil, fmt.Errorf("LoadConf(): vlan Proto %s invalid: value must be '802.1Q' or '802.1ad'", *n.VlanProto)
 	}
 
+	// validate non-zero value for vlan id if vlan proto is set to 802.1ad
+	if *n.VlanProto == sriovtypes.Proto8021ad && *n.Vlan == 0 {
+		return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan proto 802.1ad")
+	}
+
 	// validate that link state is one of supported values
 	if n.LinkState != "" && n.LinkState != "auto" && n.LinkState != "enable" && n.LinkState != "disable" {
 		return nil, fmt.Errorf("LoadConf(): invalid link_state value: %s", n.LinkState)

diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
@@ -73,6 +73,7 @@ var _ = Describe("Config", func() {
 		zeroVlanID := 0
 		invalidVlanID := 5000
 		validQoS := 1
+		zeroQoS := 0
 		invalidQoS := 10
 		valid8021qProto := "802.1Q"
 		valid8021adProto := "802.1ad"
@@ -108,14 +109,15 @@ var _ = Describe("Config", func() {
 			},
 			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("vlan ID equal to zero and non-zero QoS set", &zeroVlanID, &validQoS, nil, true),
+			Entry("vlan ID equal to zero and 802.1ad Proto set", &zeroVlanID, nil, &valid8021adProto, 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),
+			Entry("no vlan ID and non-zero QoS set", nil, &validQoS, nil, true),
+			Entry("no vlan ID and 802.1ad Proto set", nil, nil, &valid8021adProto, true),
+			Entry("default values for vlan, qos and proto", &zeroVlanID, &zeroQoS, &valid8021qProto, false),
 		)
 
 		It("Assuming device is allocated", func() {