Added vlan tag to the bridge cni plugin.

With the VLAN filter, the Linux bridge acts more like a real switch, Allow to tag and untag
vlan id's on every interface connected to the bridge.

This PR also creates a veth interface for the bridge vlan interface on L3 configuration.

Related to https://developers.redhat.com/blog/2017/09/14/vlan-filter-support-on-bridge/ post.

Note: This feature was introduced in Linux kernel 3.8 and was added to RHEL in version 7.0.

plugins/main/bridge/README.md

@@ -52,3 +52,8 @@ If the bridge is missing, the plugin will create one on first use and, if gatewa
 * `hairpinMode` (boolean, optional): set hairpin mode for interfaces on the bridge. Defaults to false.
 * `ipam` (dictionary, required): IPAM configuration to be used for this network. For L2-only network, create empty dictionary.
 * `promiscMode` (boolean, optional): set promiscuous mode on the bridge. Defaults to false.
+* `vlan` (int, optional): assign VLAN tag. Defaults to none.
+
+*Note:* The VLAN parameter configures the VLAN tag on the host end of the veth and also enables the vlan_filtering feature on the bridge interface.
+
+*Note:* To configure uplink for L2 network you need to allow the vlan on the uplink interface by using the following command ``` bridge vlan add vid VLAN_ID dev DEV```.

plugins/main/bridge/bridge.go

@@ -52,6 +52,7 @@ type NetConf struct {
 	MTU          int    `json:"mtu"`
 	HairpinMode  bool   `json:"hairpinMode"`
 	PromiscMode  bool   `json:"promiscMode"`
+	Vlan         int    `json:"vlan"`
 }
 
 type gwInfo struct {
@@ -144,7 +145,7 @@ func calcGateways(result *current.Result, n *NetConf) (*gwInfo, *gwInfo, error)
 	return gwsV4, gwsV6, nil
 }
 
-func ensureBridgeAddr(br *netlink.Bridge, family int, ipn *net.IPNet, forceAddress bool) error {
+func ensureAddr(br netlink.Link, family int, ipn *net.IPNet, forceAddress bool) error {
 	addrs, err := netlink.AddrList(br, family)
 	if err != nil && err != syscall.ENOENT {
 		return fmt.Errorf("could not get list of IP addresses: %v", err)
@@ -164,34 +165,34 @@ func ensureBridgeAddr(br *netlink.Bridge, family int, ipn *net.IPNet, forceAddre
 		// forceAddress is true, otherwise throw an error.
 		if family == netlink.FAMILY_V4 || a.IPNet.Contains(ipn.IP) || ipn.Contains(a.IPNet.IP) {
 			if forceAddress {
-				if err = deleteBridgeAddr(br, a.IPNet); err != nil {
+				if err = deleteAddr(br, a.IPNet); err != nil {
 					return err
 				}
 			} else {
-				return fmt.Errorf("%q already has an IP address different from %v", br.Name, ipnStr)
+				return fmt.Errorf("%q already has an IP address different from %v", br.Attrs().Name, ipnStr)
 			}
 		}
 	}
 
 	addr := &netlink.Addr{IPNet: ipn, Label: ""}
 	if err := netlink.AddrAdd(br, addr); err != nil {
-		return fmt.Errorf("could not add IP address to %q: %v", br.Name, err)
+		return fmt.Errorf("could not add IP address to %q: %v", br.Attrs().Name, err)
 	}
 
 	// Set the bridge's MAC to itself. Otherwise, the bridge will take the
 	// lowest-numbered mac on the bridge, and will change as ifs churn
-	if err := netlink.LinkSetHardwareAddr(br, br.HardwareAddr); err != nil {
+	if err := netlink.LinkSetHardwareAddr(br, br.Attrs().HardwareAddr); err != nil {
 		return fmt.Errorf("could not set bridge's mac: %v", err)
 	}
 
 	return nil
 }
 
-func deleteBridgeAddr(br *netlink.Bridge, ipn *net.IPNet) error {
+func deleteAddr(br netlink.Link, ipn *net.IPNet) error {
 	addr := &netlink.Addr{IPNet: ipn, Label: ""}
 
 	if err := netlink.AddrDel(br, addr); err != nil {
-		return fmt.Errorf("could not remove IP address from %q: %v", br.Name, err)
+		return fmt.Errorf("could not remove IP address from %q: %v", br.Attrs().Name, err)
 	}
 
 	return nil
@@ -209,7 +210,7 @@ func bridgeByName(name string) (*netlink.Bridge, error) {
 	return br, nil
 }
 
-func ensureBridge(brName string, mtu int, promiscMode bool) (*netlink.Bridge, error) {
+func ensureBridge(brName string, mtu int, promiscMode, vlanFiltering bool) (*netlink.Bridge, error) {
 	br := &netlink.Bridge{
 		LinkAttrs: netlink.LinkAttrs{
 			Name: brName,
@@ -220,6 +221,7 @@ func ensureBridge(brName string, mtu int, promiscMode bool) (*netlink.Bridge, er
 			// default packet limit
 			TxQLen: -1,
 		},
+		VlanFiltering: &vlanFiltering,
 	}
 
 	err := netlink.LinkAdd(br)
@@ -247,7 +249,35 @@ func ensureBridge(brName string, mtu int, promiscMode bool) (*netlink.Bridge, er
 	return br, nil
 }
 
-func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool) (*current.Interface, *current.Interface, error) {
+func ensureVlanInterface(br *netlink.Bridge, vlanId int) (netlink.Link, error) {
+	name := fmt.Sprintf("%s.%d", br.Name, vlanId)
+
+	brGatewayVeth, err := netlink.LinkByName(name)
+	if err != nil {
+		if err.Error() != "Link not found" {
+			return nil, fmt.Errorf("failed to find interface %q: %v", name, err)
+		}
+
+		hostNS, err := ns.GetCurrentNS()
+		if err != nil {
+			return nil, fmt.Errorf("faild to find host namespace: %v", err)
+		}
+
+		_, brGatewayIface, err := setupVeth(hostNS, br, name, br.MTU, false, vlanId)
+		if err != nil {
+			return nil, fmt.Errorf("faild to create vlan gateway %q: %v", name, err)
+		}
+
+		brGatewayVeth, err = netlink.LinkByName(brGatewayIface.Name)
+		if err != nil {
+			return nil, fmt.Errorf("failed to lookup %q: %v", brGatewayIface.Name, err)
+		}
+	}
+
+	return brGatewayVeth, nil
+}
+
+func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool, vlanID int) (*current.Interface, *current.Interface, error) {
 	contIface := &current.Interface{}
 	hostIface := &current.Interface{}
 
@@ -284,6 +314,13 @@ func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairp
 		return nil, nil, fmt.Errorf("failed to setup hairpin mode for %v: %v", hostVeth.Attrs().Name, err)
 	}
 
+	if vlanID != 0 {
+		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)
+		}
+	}
+
 	return hostIface, contIface, nil
 }
 
@@ -293,8 +330,12 @@ func calcGatewayIP(ipn *net.IPNet) net.IP {
 }
 
 func setupBridge(n *NetConf) (*netlink.Bridge, *current.Interface, error) {
+	vlanFiltering := false
+	if n.Vlan != 0 {
+		vlanFiltering = true
+	}
 	// create bridge if necessary
-	br, err := ensureBridge(n.BrName, n.MTU, n.PromiscMode)
+	br, err := ensureBridge(n.BrName, n.MTU, n.PromiscMode, vlanFiltering)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to create bridge %q: %v", n.BrName, err)
 	}
@@ -355,7 +396,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 	}
 	defer netns.Close()
 
-	hostInterface, containerInterface, err := setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode)
+	hostInterface, containerInterface, err := setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode, n.Vlan)
 	if err != nil {
 		return err
 	}
@@ -435,16 +476,34 @@ func cmdAdd(args *skel.CmdArgs) error {
 
 		if n.IsGW {
 			var firstV4Addr net.IP
+			var vlanInterface *current.Interface
 			// Set the IP address(es) on the bridge and enable forwarding
 			for _, gws := range []*gwInfo{gwsV4, gwsV6} {
 				for _, gw := range gws.gws {
 					if gw.IP.To4() != nil && firstV4Addr == nil {
 						firstV4Addr = gw.IP
 					}
+					if n.Vlan != 0 {
+						vlanIface, err := ensureVlanInterface(br, n.Vlan)
+						if err != nil {
+							return fmt.Errorf("failed to create vlan interface: %v", err)
+						}
 
-					err = ensureBridgeAddr(br, gws.family, &gw, n.ForceAddress)
+						if vlanInterface == nil {
-					if err != nil {
+							vlanInterface = &current.Interface{Name: vlanIface.Attrs().Name,
-						return fmt.Errorf("failed to set bridge addr: %v", err)
+								Mac: vlanIface.Attrs().HardwareAddr.String()}
+							result.Interfaces = append(result.Interfaces, vlanInterface)
+						}
+
+						err = ensureAddr(vlanIface, gws.family, &gw, n.ForceAddress)
+						if err != nil {
+							return fmt.Errorf("failed to set vlan interface for bridge with addr: %v", err)
+						}
+					} else {
+						err = ensureAddr(br, gws.family, &gw, n.ForceAddress)
+						if err != nil {
+							return fmt.Errorf("failed to set bridge addr: %v", err)
+						}
 					}
 				}
 

plugins/main/bridge/bridge_test.go

@@ -17,6 +17,7 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"github.com/vishvananda/netlink/nl"
 	"io/ioutil"
 	"net"
 	"os"
@@ -37,8 +38,9 @@ import (
 )
 
 const (
-	BRNAME = "bridge0"
+	BRNAME     = "bridge0"
-	IFNAME = "eth0"
+	BRNAMEVLAN = "bridge0.100"
+	IFNAME     = "eth0"
 )
 
 type Net struct {
@@ -67,6 +69,7 @@ type testCase struct {
 	isGW       bool
 	isLayer2   bool
 	expGWCIDRs []string // Expected gateway addresses in CIDR form
+	vlan       int
 }
 
 // Range definition for each entry in the ranges list
@@ -96,9 +99,12 @@ const (
 	"cniVersion": "%s",
 	"name": "testConfig",
 	"type": "bridge",
-	"bridge": "%s",`
+	"bridge": "%s"`
 
-	netDefault = `
+	vlan = `,
+	"vlan": %d`
+
+	netDefault = `,
 	"isDefaultGateway": true,
 	"ipMasq": false`
 
@@ -138,6 +144,10 @@ const (
 // for a test case.
 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.isLayer2 {
 		conf += netDefault
 		if tc.subnet != "" || tc.ranges != nil {
@@ -154,7 +164,7 @@ func (tc testCase) netConfJSON(dataDir string) string {
 			conf += ipamEndStr
 		}
 	} else {
-		conf += `
+		conf += `,
 	"ipam": {}`
 	}
 	return "{" + conf + "\n}"
@@ -253,6 +263,38 @@ func delBridgeAddrs(testNS ns.NetNS) {
 			}
 		}
 
+		br, err = netlink.LinkByName(BRNAMEVLAN)
+		if err == nil {
+			addrs, err = netlink.AddrList(br, netlink.FAMILY_ALL)
+			Expect(err).NotTo(HaveOccurred())
+			for _, addr := range addrs {
+				if !addr.IP.IsLinkLocalUnicast() {
+					err = netlink.AddrDel(br, &addr)
+					Expect(err).NotTo(HaveOccurred())
+				}
+			}
+		}
+
+		return nil
+	})
+	Expect(err).NotTo(HaveOccurred())
+}
+
+func delVlanAddrs(testNS ns.NetNS, vlan int) {
+	err := testNS.Do(func(ns.NetNS) error {
+		defer GinkgoRecover()
+
+		vlanLink, err := netlink.LinkByName(fmt.Sprintf("%s.%d", BRNAME, vlan))
+		Expect(err).NotTo(HaveOccurred())
+		addrs, err := netlink.AddrList(vlanLink, netlink.FAMILY_ALL)
+		Expect(err).NotTo(HaveOccurred())
+		for _, addr := range addrs {
+			if !addr.IP.IsLinkLocalUnicast() {
+				err = netlink.AddrDel(vlanLink, &addr)
+				Expect(err).NotTo(HaveOccurred())
+			}
+		}
+
 		return nil
 	})
 	Expect(err).NotTo(HaveOccurred())
@@ -283,6 +325,16 @@ func countIPAMIPs(path string) (int, error) {
 	return count, nil
 }
 
+func checkVlan(vlanId int, bridgeVlanInfo []*nl.BridgeVlanInfo) bool {
+	for _, vlan := range bridgeVlanInfo {
+		if vlan.Vid == uint16(vlanId) {
+			return true
+		}
+	}
+
+	return false
+}
+
 type cmdAddDelTester interface {
 	setNS(testNS ns.NetNS, targetNS ns.NetNS)
 	cmdAddTest(tc testCase, dataDir string) (*current.Result, error)
@@ -588,7 +640,12 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (*current.Resu
 		result, err = current.GetResult(r)
 		Expect(err).NotTo(HaveOccurred())
 
-		Expect(len(result.Interfaces)).To(Equal(3))
+		if !tc.isLayer2 && tc.vlan != 0 {
+			Expect(len(result.Interfaces)).To(Equal(4))
+		} else {
+			Expect(len(result.Interfaces)).To(Equal(3))
+		}
+
 		Expect(result.Interfaces[0].Name).To(Equal(BRNAME))
 		Expect(result.Interfaces[0].Mac).To(HaveLen(17))
 
@@ -607,8 +664,38 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (*current.Resu
 		Expect(link.Attrs().HardwareAddr.String()).To(Equal(result.Interfaces[0].Mac))
 		bridgeMAC := link.Attrs().HardwareAddr.String()
 
+		var vlanLink netlink.Link
+		if !tc.isLayer2 && tc.vlan != 0 {
+			// Make sure vlan link exists
+			vlanLink, err = netlink.LinkByName(fmt.Sprintf("%s.%d", BRNAME, tc.vlan))
+			Expect(err).NotTo(HaveOccurred())
+			Expect(vlanLink.Attrs().Name).To(Equal(fmt.Sprintf("%s.%d", BRNAME, tc.vlan)))
+			Expect(vlanLink).To(BeAssignableToTypeOf(&netlink.Veth{}))
+
+			// Check the bridge dot vlan interface have the vlan tag
+			peerLink, err := netlink.LinkByIndex(vlanLink.Attrs().Index - 1)
+			Expect(err).NotTo(HaveOccurred())
+			interfaceMap, err := netlink.BridgeVlanList()
+			Expect(err).NotTo(HaveOccurred())
+			vlans, isExist := interfaceMap[int32(peerLink.Attrs().Index)]
+			Expect(isExist).To(BeTrue())
+			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+		}
+
+		// Check the bridge vlan filtering equals true
+		if tc.vlan != 0 {
+			Expect(*link.(*netlink.Bridge).VlanFiltering).To(Equal(true))
+		} else {
+			Expect(*link.(*netlink.Bridge).VlanFiltering).To(Equal(false))
+		}
+
 		// Ensure bridge has expected gateway address(es)
-		addrs, err := netlink.AddrList(link, netlink.FAMILY_ALL)
+		var addrs []netlink.Addr
+		if tc.vlan == 0 {
+			addrs, err = netlink.AddrList(link, netlink.FAMILY_ALL)
+		} else {
+			addrs, err = netlink.AddrList(vlanLink, netlink.FAMILY_ALL)
+		}
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(addrs)).To(BeNumerically(">", 0))
 		for _, cidr := range tc.expGWCIDRs {
@@ -630,18 +717,31 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (*current.Resu
 		// Check for the veth link in the main namespace
 		links, err := netlink.LinkList()
 		Expect(err).NotTo(HaveOccurred())
-		Expect(len(links)).To(Equal(3)) // Bridge, veth, and loopback
+		if !tc.isLayer2 && tc.vlan != 0 {
+			Expect(len(links)).To(Equal(5)) // Bridge, Bridge vlan veth, veth, and loopback
+		} else {
+			Expect(len(links)).To(Equal(3)) // Bridge, veth, and loopback
+		}
 
 		link, err = netlink.LinkByName(result.Interfaces[1].Name)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{}))
 		tester.vethName = result.Interfaces[1].Name
 
+		// check vlan exist on the veth interface
+		if tc.vlan != 0 {
+			interfaceMap, err := netlink.BridgeVlanList()
+			Expect(err).NotTo(HaveOccurred())
+			vlans, isExist := interfaceMap[int32(link.Attrs().Index)]
+			Expect(isExist).To(BeTrue())
+			Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
+		}
+
 		// Check that the bridge has a different mac from the veth
 		// If not, it means the bridge has an unstable mac and will change
 		// as ifs are added and removed
 		// this check is not relevant for a layer 2 bridge
-		if !tc.isLayer2 {
+		if !tc.isLayer2 && tc.vlan == 0 {
 			Expect(link.Attrs().HardwareAddr.String()).NotTo(Equal(bridgeMAC))
 		}
 
@@ -986,6 +1086,10 @@ func cmdAddDelCheckTest(testNS ns.NetNS, tc testCase, dataDir string) {
 
 	// Clean up bridge addresses for next test case
 	delBridgeAddrs(testNS)
+
+	if tc.vlan != 0 && !tc.isLayer2 {
+		delVlanAddrs(testNS, tc.vlan)
+	}
 }
 
 var _ = Describe("bridge Operations", func() {
@@ -1115,6 +1219,63 @@ var _ = Describe("bridge Operations", func() {
 		cmdAddDelTest(originalNS, tc, dataDir)
 	})
 
+	It("configures and deconfigures a l2 bridge with vlan id 100 using ADD/DEL for 0.3.1 config", func() {
+		tc := testCase{cniVersion: "0.3.0", isLayer2: true, vlan: 100}
+		cmdAddDelTest(originalNS, tc, dataDir)
+	})
+
+	It("configures and deconfigures a l2 bridge with vlan id 100 using ADD/DEL for 0.3.1 config", func() {
+		tc := testCase{cniVersion: "0.3.1", isLayer2: true, vlan: 100}
+		cmdAddDelTest(originalNS, tc, dataDir)
+	})
+
+	It("configures and deconfigures a bridge, veth with default route and vlanID 100 with ADD/DEL for 0.3.0 config", func() {
+		testCases := []testCase{
+			{
+				// IPv4 only
+				subnet:     "10.1.2.0/24",
+				expGWCIDRs: []string{"10.1.2.1/24"},
+				vlan:       100,
+			},
+			{
+				// IPv6 only
+				subnet:     "2001:db8::0/64",
+				expGWCIDRs: []string{"2001:db8::1/64"},
+				vlan:       100,
+			},
+			{
+				// Dual-Stack
+				ranges: []rangeInfo{
+					{subnet: "192.168.0.0/24"},
+					{subnet: "fd00::0/64"},
+				},
+				expGWCIDRs: []string{
+					"192.168.0.1/24",
+					"fd00::1/64",
+				},
+				vlan: 100,
+			},
+			{
+				// 3 Subnets (1 IPv4 and 2 IPv6 subnets)
+				ranges: []rangeInfo{
+					{subnet: "192.168.0.0/24"},
+					{subnet: "fd00::0/64"},
+					{subnet: "2001:db8::0/64"},
+				},
+				expGWCIDRs: []string{
+					"192.168.0.1/24",
+					"fd00::1/64",
+					"2001:db8::1/64",
+				},
+				vlan: 100,
+			},
+		}
+		for _, tc := range testCases {
+			tc.cniVersion = "0.3.0"
+			cmdAddDelTest(originalNS, tc, dataDir)
+		}
+	})
+
 	It("configures and deconfigures a bridge and veth with default route with ADD/DEL for 0.3.1 config", func() {
 		testCases := []testCase{
 			{
@@ -1263,13 +1424,13 @@ var _ = Describe("bridge Operations", func() {
 				Expect(conf.ForceAddress).To(Equal(false))
 
 				// Set first address on bridge
-				err = ensureBridgeAddr(bridge, family, &gwnFirst, conf.ForceAddress)
+				err = ensureAddr(bridge, family, &gwnFirst, conf.ForceAddress)
 				Expect(err).NotTo(HaveOccurred())
 				checkBridgeIPs(tc.gwCIDRFirst, "")
 
 				// Attempt to set the second address on the bridge
 				// with ForceAddress set to false.
-				err = ensureBridgeAddr(bridge, family, &gwnSecond, false)
+				err = ensureAddr(bridge, family, &gwnSecond, false)
 				if family == netlink.FAMILY_V4 || subnetsOverlap {
 					// IPv4 or overlapping IPv6 subnets:
 					// Expect an error, and address should remain the same
@@ -1285,7 +1446,7 @@ var _ = Describe("bridge Operations", func() {
 
 				// Set the second address on the bridge
 				// with ForceAddress set to true.
-				err = ensureBridgeAddr(bridge, family, &gwnSecond, true)
+				err = ensureAddr(bridge, family, &gwnSecond, true)
 				Expect(err).NotTo(HaveOccurred())
 				if family == netlink.FAMILY_V4 || subnetsOverlap {
 					// IPv4 or overlapping IPv6 subnets: