Merge pull request #875 from mlguerrero12/adddefaultvlanparam

Add parameter to disable default vlan

plugins/main/bridge/bridge.go

@@ -46,17 +46,18 @@ const defaultBrName = "cni0"
 
 type NetConf struct {
 	types.NetConf
-	BrName       string `json:"bridge"`
+	BrName              string `json:"bridge"`
-	IsGW         bool   `json:"isGateway"`
+	IsGW                bool   `json:"isGateway"`
-	IsDefaultGW  bool   `json:"isDefaultGateway"`
+	IsDefaultGW         bool   `json:"isDefaultGateway"`
-	ForceAddress bool   `json:"forceAddress"`
+	ForceAddress        bool   `json:"forceAddress"`
-	IPMasq       bool   `json:"ipMasq"`
+	IPMasq              bool   `json:"ipMasq"`
-	MTU          int    `json:"mtu"`
+	MTU                 int    `json:"mtu"`
-	HairpinMode  bool   `json:"hairpinMode"`
+	HairpinMode         bool   `json:"hairpinMode"`
-	PromiscMode  bool   `json:"promiscMode"`
+	PromiscMode         bool   `json:"promiscMode"`
-	Vlan         int    `json:"vlan"`
+	Vlan                int    `json:"vlan"`
-	MacSpoofChk  bool   `json:"macspoofchk,omitempty"`
+	PreserveDefaultVlan bool   `json:"preserveDefaultVlan"`
-	EnableDad    bool   `json:"enabledad,omitempty"`
+	MacSpoofChk         bool   `json:"macspoofchk,omitempty"`
+	EnableDad           bool   `json:"enabledad,omitempty"`
 
 	Args struct {
 		Cni BridgeArgs `json:"cni,omitempty"`
@@ -94,6 +95,8 @@ func init() {
 func loadNetConf(bytes []byte, envArgs string) (*NetConf, string, error) {
 	n := &NetConf{
 		BrName: defaultBrName,
+		// Set default value equal to true to maintain existing behavior.
+		PreserveDefaultVlan: true,
 	}
 	if err := json.Unmarshal(bytes, n); err != nil {
 		return nil, "", fmt.Errorf("failed to load netconf: %v", err)
@@ -299,7 +302,7 @@ func ensureBridge(brName string, mtu int, promiscMode, vlanFiltering bool) (*net
 	return br, nil
 }
 
-func ensureVlanInterface(br *netlink.Bridge, vlanID int) (netlink.Link, error) {
+func ensureVlanInterface(br *netlink.Bridge, vlanID int, preserveDefaultVlan bool) (netlink.Link, error) {
 	name := fmt.Sprintf("%s.%d", br.Name, vlanID)
 
 	brGatewayVeth, err := netlink.LinkByName(name)
@@ -313,7 +316,7 @@ func ensureVlanInterface(br *netlink.Bridge, vlanID int) (netlink.Link, error) {
 			return nil, fmt.Errorf("faild to find host namespace: %v", err)
 		}
 
-		_, brGatewayIface, err := setupVeth(hostNS, br, name, br.MTU, false, vlanID, "")
+		_, brGatewayIface, err := setupVeth(hostNS, br, name, br.MTU, false, vlanID, preserveDefaultVlan, "")
 		if err != nil {
 			return nil, fmt.Errorf("faild to create vlan gateway %q: %v", name, err)
 		}
@@ -332,7 +335,7 @@ func ensureVlanInterface(br *netlink.Bridge, vlanID int) (netlink.Link, error) {
 	return brGatewayVeth, nil
 }
 
-func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool, vlanID int, mac string) (*current.Interface, *current.Interface, error) {
+func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool, vlanID int, preserveDefaultVlan bool, mac string) (*current.Interface, *current.Interface, error) {
 	contIface := &current.Interface{}
 	hostIface := &current.Interface{}
 
@@ -370,6 +373,13 @@ func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairp
 	}
 
 	if vlanID != 0 {
+		if !preserveDefaultVlan {
+			err = removeDefaultVlan(hostVeth)
+			if err != nil {
+				return nil, nil, fmt.Errorf("failed to remove default vlan on interface %q: %v", hostIface.Name, err)
+			}
+		}
+
 		err = netlink.BridgeVlanAdd(hostVeth, uint16(vlanID), true, true, false, true)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to setup vlan tag on interface %q: %v", hostIface.Name, err)
@@ -379,6 +389,25 @@ func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairp
 	return hostIface, contIface, nil
 }
 
+func removeDefaultVlan(hostVeth netlink.Link) error {
+	vlanInfo, err := netlink.BridgeVlanList()
+	if err != nil {
+		return err
+	}
+
+	brVlanInfo, ok := vlanInfo[int32(hostVeth.Attrs().Index)]
+	if ok {
+		for _, info := range brVlanInfo {
+			err = netlink.BridgeVlanDel(hostVeth, info.Vid, false, false, false, true)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func calcGatewayIP(ipn *net.IPNet) net.IP {
 	nid := ipn.IP.Mask(ipn.Mask)
 	return ip.NextIP(nid)
@@ -434,7 +463,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 	}
 	defer netns.Close()
 
-	hostInterface, containerInterface, err := setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode, n.Vlan, n.mac)
+	hostInterface, containerInterface, err := setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode, n.Vlan, n.PreserveDefaultVlan, n.mac)
 	if err != nil {
 		return err
 	}
@@ -523,7 +552,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 						firstV4Addr = gw.IP
 					}
 					if n.Vlan != 0 {
-						vlanIface, err := ensureVlanInterface(br, n.Vlan)
+						vlanIface, err := ensureVlanInterface(br, n.Vlan, n.PreserveDefaultVlan)
 						if err != nil {
 							return fmt.Errorf("failed to create vlan interface: %v", err)
 						}

plugins/main/bridge/bridge_test.go

@@ -65,21 +65,22 @@ type Net struct {
 // testCase defines the CNI network configuration and the expected
 // bridge addresses for a test case.
 type testCase struct {
-	cniVersion  string      // CNI Version
+	cniVersion        string      // CNI Version
-	subnet      string      // Single subnet config: Subnet CIDR
+	subnet            string      // Single subnet config: Subnet CIDR
-	gateway     string      // Single subnet config: Gateway
+	gateway           string      // Single subnet config: Gateway
-	ranges      []rangeInfo // Ranges list (multiple subnets config)
+	ranges            []rangeInfo // Ranges list (multiple subnets config)
-	resolvConf  string      // host-local resolvConf file path
+	resolvConf        string      // host-local resolvConf file path
-	isGW        bool
+	isGW              bool
-	isLayer2    bool
+	isLayer2          bool
-	expGWCIDRs  []string // Expected gateway addresses in CIDR form
+	expGWCIDRs        []string // Expected gateway addresses in CIDR form
-	vlan        int
+	vlan              int
-	ipMasq      bool
+	removeDefaultVlan bool
-	macspoofchk bool
+	ipMasq            bool
-	AddErr020   string
+	macspoofchk       bool
-	DelErr020   string
+	AddErr020         string
-	AddErr010   string
+	DelErr020         string
-	DelErr010   string
+	AddErr010         string
+	DelErr010         string
 
 	envArgs       string // CNI_ARGS
 	runtimeConfig struct {
@@ -129,6 +130,9 @@ const (
 	vlan = `,
 	"vlan": %d`
 
+	preserveDefaultVlan = `,
+	"preserveDefaultVlan": false`
+
 	netDefault = `,
 	"isDefaultGateway": true`
 
@@ -191,6 +195,10 @@ func (tc testCase) netConfJSON(dataDir string) string {
 	conf := fmt.Sprintf(netConfStr, tc.cniVersion, BRNAME)
 	if tc.vlan != 0 {
 		conf += fmt.Sprintf(vlan, tc.vlan)
+
+		if tc.removeDefaultVlan {
+			conf += preserveDefaultVlan
+		}
 	}
 	if tc.ipMasq {
 		conf += tc.ipMasqConfig()
@@ -527,6 +535,9 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			vlans, isExist := interfaceMap[int32(peerLink.Attrs().Index)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		// Check the bridge vlan filtering equals true
@@ -582,6 +593,9 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			vlans, isExist := interfaceMap[int32(link.Attrs().Index)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		// Check that the bridge has a different mac from the veth
@@ -832,6 +846,9 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			vlans, isExist := interfaceMap[int32(peerLink.Attrs().Index)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		// Check the bridge vlan filtering equals true
@@ -887,6 +904,9 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			vlans, isExist := interfaceMap[int32(link.Attrs().Index)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		// Check that the bridge has a different mac from the veth
@@ -1132,6 +1152,9 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			vlans, isExist := interfaceMap[int32(peerLink.Attrs().Index)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		// Check the bridge vlan filtering equals true
@@ -1187,6 +1210,9 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			vlans, isExist := interfaceMap[int32(link.Attrs().Index)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		// Check that the bridge has a different mac from the veth
@@ -1358,6 +1384,9 @@ func (tester *testerV01xOr02x) cmdAddTest(tc testCase, dataDir string) (types.Re
 			vlans, isExist := interfaceMap[int32(peerLink.Attrs().Index)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		// Check the bridge vlan filtering equals true
@@ -1480,6 +1509,9 @@ func (tester *testerV01xOr02x) cmdAddTest(tc testCase, dataDir string) (types.Re
 			vlans, isExist := hostNSVlanMap[int32(peerIndex)]
 			Expect(isExist).To(BeTrue())
 			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+			if tc.removeDefaultVlan {
+				Expect(vlans).To(HaveLen(1))
+			}
 		}
 
 		return nil
@@ -1772,6 +1804,18 @@ var _ = Describe("bridge Operations", func() {
 			cmdAddDelTest(originalNS, targetNS, tc, dataDir)
 		})
 
+		It(fmt.Sprintf("[%s] configures and deconfigures a l2 bridge with vlan id 100 and no default vlan using ADD/DEL", ver), func() {
+			tc := testCase{
+				cniVersion:        ver,
+				isLayer2:          true,
+				vlan:              100,
+				removeDefaultVlan: true,
+				AddErr020:         "cannot convert: no valid IP addresses",
+				AddErr010:         "cannot convert: no valid IP addresses",
+			}
+			cmdAddDelTest(originalNS, targetNS, tc, dataDir)
+		})
+
 		for i, tc := range []testCase{
 			{
 				// IPv4 only
@@ -1822,6 +1866,11 @@ var _ = Describe("bridge Operations", func() {
 				tc.cniVersion = ver
 				cmdAddDelTest(originalNS, targetNS, tc, dataDir)
 			})
+			It(fmt.Sprintf("[%s] (%d) configures and deconfigures a bridge, veth with default route and vlanID 100 and no default vlan with ADD/DEL", ver, i), func() {
+				tc.cniVersion = ver
+				tc.removeDefaultVlan = true
+				cmdAddDelTest(originalNS, targetNS, tc, dataDir)
+			})
 		}
 
 		for i, tc := range []testCase{