Merge pull request #829 from tjjh89017/bridge_vlan_trunk

bridge: add vlan trunk support

plugins/main/bridge/bridge.go

@@ -21,6 +21,7 @@ import (
 	"net"
 	"os"
 	"runtime"
+	"sort"
 	"syscall"
 	"time"
 
@@ -46,18 +47,19 @@ 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"`
-	PreserveDefaultVlan bool   `json:"preserveDefaultVlan"`
+	VlanTrunk           []*VlanTrunk `json:"vlanTrunk,omitempty"`
-	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"`
@@ -66,7 +68,14 @@ type NetConf struct {
 		Mac string `json:"mac,omitempty"`
 	} `json:"runtimeConfig,omitempty"`
 
-	mac string
+	mac   string
+	vlans []int
+}
+
+type VlanTrunk struct {
+	MinID *int `json:"minID,omitempty"`
+	MaxID *int `json:"maxID,omitempty"`
+	ID    *int `json:"id,omitempty"`
 }
 
 type BridgeArgs struct {
@@ -104,6 +113,17 @@ func loadNetConf(bytes []byte, envArgs string) (*NetConf, string, error) {
 	if n.Vlan < 0 || n.Vlan > 4094 {
 		return nil, "", fmt.Errorf("invalid VLAN ID %d (must be between 0 and 4094)", n.Vlan)
 	}
+	var err error
+	n.vlans, err = collectVlanTrunk(n.VlanTrunk)
+	if err != nil {
+		// fail to parsing
+		return nil, "", err
+	}
+
+	// Currently bridge CNI only support access port(untagged only) or trunk port(tagged only)
+	if n.Vlan > 0 && n.vlans != nil {
+		return nil, "", errors.New("cannot set vlan and vlanTrunk at the same time")
+	}
 
 	if envArgs != "" {
 		e := MacEnvArgs{}
@@ -127,6 +147,61 @@ func loadNetConf(bytes []byte, envArgs string) (*NetConf, string, error) {
 	return n, n.CNIVersion, nil
 }
 
+// This method is copied from https://github.com/k8snetworkplumbingwg/ovs-cni/blob/v0.27.2/pkg/plugin/plugin.go
+func collectVlanTrunk(vlanTrunk []*VlanTrunk) ([]int, error) {
+	if vlanTrunk == nil {
+		return nil, nil
+	}
+
+	vlanMap := make(map[int]struct{})
+	for _, item := range vlanTrunk {
+		var minID int
+		var maxID int
+		var ID int
+
+		switch {
+		case item.MinID != nil && item.MaxID != nil:
+			minID = *item.MinID
+			if minID <= 0 || minID > 4094 {
+				return nil, errors.New("incorrect trunk minID parameter")
+			}
+			maxID = *item.MaxID
+			if maxID <= 0 || maxID > 4094 {
+				return nil, errors.New("incorrect trunk maxID parameter")
+			}
+			if maxID < minID {
+				return nil, errors.New("minID is greater than maxID in trunk parameter")
+			}
+			for v := minID; v <= maxID; v++ {
+				vlanMap[v] = struct{}{}
+			}
+		case item.MinID == nil && item.MaxID != nil:
+			return nil, errors.New("minID and maxID should be configured simultaneously, minID is missing")
+		case item.MinID != nil && item.MaxID == nil:
+			return nil, errors.New("minID and maxID should be configured simultaneously, maxID is missing")
+		}
+
+		// single vid
+		if item.ID != nil {
+			ID = *item.ID
+			if ID <= 0 || ID > 4094 {
+				return nil, errors.New("incorrect trunk id parameter")
+			}
+			vlanMap[ID] = struct{}{}
+		}
+	}
+
+	if len(vlanMap) == 0 {
+		return nil, nil
+	}
+	vlans := make([]int, 0, len(vlanMap))
+	for k := range vlanMap {
+		vlans = append(vlans, k)
+	}
+	sort.Slice(vlans, func(i int, j int) bool { return vlans[i] < vlans[j] })
+	return vlans, nil
+}
+
 // calcGateways processes the results from the IPAM plugin and does the
 // following for each IP family:
 //   - Calculates and compiles a list of gateway addresses
@@ -316,7 +391,7 @@ func ensureVlanInterface(br *netlink.Bridge, vlanID int, preserveDefaultVlan boo
 			return nil, fmt.Errorf("faild to find host namespace: %v", err)
 		}
 
-		_, brGatewayIface, err := setupVeth(hostNS, br, name, br.MTU, false, vlanID, preserveDefaultVlan, "")
+		_, brGatewayIface, err := setupVeth(hostNS, br, name, br.MTU, false, vlanID, nil, preserveDefaultVlan, "")
 		if err != nil {
 			return nil, fmt.Errorf("faild to create vlan gateway %q: %v", name, err)
 		}
@@ -335,7 +410,7 @@ func ensureVlanInterface(br *netlink.Bridge, vlanID int, preserveDefaultVlan boo
 	return brGatewayVeth, nil
 }
 
-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) {
+func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool, vlanID int, vlans []int, preserveDefaultVlan bool, mac string) (*current.Interface, *current.Interface, error) {
 	contIface := &current.Interface{}
 	hostIface := &current.Interface{}
 
@@ -372,20 +447,28 @@ 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 {
+	if (vlanID != 0 || len(vlans) > 0) && !preserveDefaultVlan {
-		if !preserveDefaultVlan {
+		err = removeDefaultVlan(hostVeth)
-			err = removeDefaultVlan(hostVeth)
+		if err != nil {
-			if err != nil {
+			return nil, nil, fmt.Errorf("failed to remove default vlan on interface %q: %v", hostIface.Name, err)
-				return nil, nil, fmt.Errorf("failed to remove default vlan on interface %q: %v", hostIface.Name, err)
-			}
 		}
+	}
 
+	// Currently bridge CNI only support access port(untagged only) or trunk port(tagged only)
+	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)
 		}
 	}
 
+	for _, v := range vlans {
+		err = netlink.BridgeVlanAdd(hostVeth, uint16(v), false, false, false, true)
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to setup vlan tag on interface %q: %w", hostIface.Name, err)
+		}
+	}
+
 	return hostIface, contIface, nil
 }
 
@@ -414,7 +497,10 @@ func calcGatewayIP(ipn *net.IPNet) net.IP {
 }
 
 func setupBridge(n *NetConf) (*netlink.Bridge, *current.Interface, error) {
-	vlanFiltering := n.Vlan != 0
+	vlanFiltering := false
+	if n.Vlan != 0 || n.VlanTrunk != nil {
+		vlanFiltering = true
+	}
 	// create bridge if necessary
 	br, err := ensureBridge(n.BrName, n.MTU, n.PromiscMode, vlanFiltering)
 	if err != nil {
@@ -463,7 +549,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.PreserveDefaultVlan, n.mac)
+	hostInterface, containerInterface, err := setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode, n.Vlan, n.vlans, n.PreserveDefaultVlan, n.mac)
 	if err != nil {
 		return err
 	}

plugins/main/bridge/bridge_test.go

@@ -74,6 +74,7 @@ type testCase struct {
 	isLayer2          bool
 	expGWCIDRs        []string // Expected gateway addresses in CIDR form
 	vlan              int
+	vlanTrunk         []*VlanTrunk
 	removeDefaultVlan bool
 	ipMasq            bool
 	macspoofchk       bool
@@ -130,6 +131,23 @@ const (
 	vlan = `,
 	"vlan": %d`
 
+	vlanTrunkStartStr = `,
+	"vlanTrunk": [`
+
+	vlanTrunk = `
+	{
+		"id": %d
+	}`
+
+	vlanTrunkRange = `
+	{
+		"minID": %d,
+		"maxID": %d
+	}`
+
+	vlanTrunkEndStr = `
+	]`
+
 	preserveDefaultVlan = `,
 	"preserveDefaultVlan": false`
 
@@ -200,6 +218,23 @@ func (tc testCase) netConfJSON(dataDir string) string {
 			conf += preserveDefaultVlan
 		}
 	}
+
+	if tc.isLayer2 && tc.vlanTrunk != nil {
+		conf += vlanTrunkStartStr
+		for i, vlan := range tc.vlanTrunk {
+			if i > 0 {
+				conf += ","
+			}
+			if vlan.ID != nil {
+				conf += fmt.Sprintf(vlanTrunk, *vlan.ID)
+			}
+			if vlan.MinID != nil && vlan.MaxID != nil {
+				conf += fmt.Sprintf(vlanTrunkRange, *vlan.MinID, *vlan.MaxID)
+			}
+		}
+		conf += vlanTrunkEndStr
+	}
+
 	if tc.ipMasq {
 		conf += tc.ipMasqConfig()
 	}
@@ -541,7 +576,7 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 		}
 
 		// Check the bridge vlan filtering equals true
-		if tc.vlan != 0 {
+		if tc.vlan != 0 || tc.vlanTrunk != nil {
 			Expect(*link.(*netlink.Bridge).VlanFiltering).To(BeTrue())
 		} else {
 			Expect(*link.(*netlink.Bridge).VlanFiltering).To(BeFalse())
@@ -598,6 +633,25 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			}
 		}
 
+		// check VlanTrunks exist on the veth interface
+		if tc.vlanTrunk != nil {
+			interfaceMap, err := netlink.BridgeVlanList()
+			Expect(err).NotTo(HaveOccurred())
+			vlans, isExist := interfaceMap[int32(link.Attrs().Index)]
+			Expect(isExist).To(BeTrue())
+
+			for _, vlanEntry := range tc.vlanTrunk {
+				if vlanEntry.ID != nil {
+					Expect(checkVlan(*vlanEntry.ID, vlans)).To(BeTrue())
+				}
+				if vlanEntry.MinID != nil && vlanEntry.MaxID != nil {
+					for vid := *vlanEntry.MinID; vid <= *vlanEntry.MaxID; vid++ {
+						Expect(checkVlan(vid, 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
@@ -852,7 +906,7 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 		}
 
 		// Check the bridge vlan filtering equals true
-		if tc.vlan != 0 {
+		if tc.vlan != 0 || tc.vlanTrunk != nil {
 			Expect(*link.(*netlink.Bridge).VlanFiltering).To(BeTrue())
 		} else {
 			Expect(*link.(*netlink.Bridge).VlanFiltering).To(BeFalse())
@@ -909,6 +963,25 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			}
 		}
 
+		// check VlanTrunks exist on the veth interface
+		if tc.vlanTrunk != nil {
+			interfaceMap, err := netlink.BridgeVlanList()
+			Expect(err).NotTo(HaveOccurred())
+			vlans, isExist := interfaceMap[int32(link.Attrs().Index)]
+			Expect(isExist).To(BeTrue())
+
+			for _, vlanEntry := range tc.vlanTrunk {
+				if vlanEntry.ID != nil {
+					Expect(checkVlan(*vlanEntry.ID, vlans)).To(BeTrue())
+				}
+				if vlanEntry.MinID != nil && vlanEntry.MaxID != nil {
+					for vid := *vlanEntry.MinID; vid <= *vlanEntry.MaxID; vid++ {
+						Expect(checkVlan(vid, 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
@@ -1158,7 +1231,7 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 		}
 
 		// Check the bridge vlan filtering equals true
-		if tc.vlan != 0 {
+		if tc.vlan != 0 || tc.vlanTrunk != nil {
 			Expect(*link.(*netlink.Bridge).VlanFiltering).To(BeTrue())
 		} else {
 			Expect(*link.(*netlink.Bridge).VlanFiltering).To(BeFalse())
@@ -1215,6 +1288,25 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (types.Result,
 			}
 		}
 
+		// check VlanTrunks exist on the veth interface
+		if tc.vlanTrunk != nil {
+			interfaceMap, err := netlink.BridgeVlanList()
+			Expect(err).NotTo(HaveOccurred())
+			vlans, isExist := interfaceMap[int32(link.Attrs().Index)]
+			Expect(isExist).To(BeTrue())
+
+			for _, vlanEntry := range tc.vlanTrunk {
+				if vlanEntry.ID != nil {
+					Expect(checkVlan(*vlanEntry.ID, vlans)).To(BeTrue())
+				}
+				if vlanEntry.MinID != nil && vlanEntry.MaxID != nil {
+					for vid := *vlanEntry.MinID; vid <= *vlanEntry.MaxID; vid++ {
+						Expect(checkVlan(vid, 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
@@ -1672,6 +1764,61 @@ var _ = Describe("bridge Operations", func() {
 		Expect(testutils.UnmountNS(targetNS)).To(Succeed())
 	})
 
+	var (
+		correctID      int = 10
+		correctMinID   int = 100
+		correctMaxID   int = 105
+		incorrectMinID int = 1000
+		incorrectMaxID int = 100
+		overID         int = 5000
+		negativeID     int = -1
+	)
+
+	DescribeTable(
+		"collectVlanTrunk succeeds",
+		func(vlanTrunks []*VlanTrunk, expectedVIDs []int) {
+			Expect(collectVlanTrunk(vlanTrunks)).To(ConsistOf(expectedVIDs))
+		},
+		Entry("when provided an empty VLAN trunk configuration", []*VlanTrunk{}, nil),
+		Entry("when provided a VLAN trunk configuration with both min / max range", []*VlanTrunk{
+			{
+				MinID: &correctMinID,
+				MaxID: &correctMaxID,
+			},
+		}, []int{100, 101, 102, 103, 104, 105}),
+		Entry("when provided a VLAN trunk configuration with id only", []*VlanTrunk{
+			{
+				ID: &correctID,
+			},
+		}, []int{10}),
+		Entry("when provided a VLAN trunk configuration with id and range", []*VlanTrunk{
+			{
+				ID: &correctID,
+			},
+			{
+				MinID: &correctMinID,
+				MaxID: &correctMaxID,
+			},
+		}, []int{10, 100, 101, 102, 103, 104, 105}),
+	)
+
+	DescribeTable(
+		"collectVlanTrunk failed",
+		func(vlanTrunks []*VlanTrunk, expectedError error) {
+			_, err := collectVlanTrunk(vlanTrunks)
+			Expect(err).To(MatchError(expectedError))
+		},
+		Entry("when not passed the maxID", []*VlanTrunk{{MinID: &correctMinID}}, fmt.Errorf("minID and maxID should be configured simultaneously, maxID is missing")),
+		Entry("when not passed the minID", []*VlanTrunk{{MaxID: &correctMaxID}}, fmt.Errorf("minID and maxID should be configured simultaneously, minID is missing")),
+		Entry("when the minID is negative", []*VlanTrunk{{MinID: &negativeID, MaxID: &correctMaxID}}, fmt.Errorf("incorrect trunk minID parameter")),
+		Entry("when the minID is larger than 4094", []*VlanTrunk{{MinID: &overID, MaxID: &correctMaxID}}, fmt.Errorf("incorrect trunk minID parameter")),
+		Entry("when the maxID is larger than 4094", []*VlanTrunk{{MinID: &correctMinID, MaxID: &overID}}, fmt.Errorf("incorrect trunk maxID parameter")),
+		Entry("when the maxID is negative", []*VlanTrunk{{MinID: &correctMinID, MaxID: &overID}}, fmt.Errorf("incorrect trunk maxID parameter")),
+		Entry("when the ID is larger than 4094", []*VlanTrunk{{ID: &overID}}, fmt.Errorf("incorrect trunk id parameter")),
+		Entry("when the ID is negative", []*VlanTrunk{{ID: &negativeID}}, fmt.Errorf("incorrect trunk id parameter")),
+		Entry("when the maxID is smaller than minID", []*VlanTrunk{{MinID: &incorrectMinID, MaxID: &incorrectMaxID}}, fmt.Errorf("minID is greater than maxID in trunk parameter")),
+	)
+
 	for _, ver := range testutils.AllSpecVersions {
 		// Redefine ver inside for scope so real value is picked up by each dynamically defined It()
 		// See Gingkgo's "Patterns for dynamically generating tests" documentation.
@@ -1804,6 +1951,25 @@ var _ = Describe("bridge Operations", func() {
 			cmdAddDelTest(originalNS, targetNS, tc, dataDir)
 		})
 
+		// TODO find some way to put pointer
+		It(fmt.Sprintf("[%s] configures and deconfigures a l2 bridge with vlan id 100, vlanTrunk 101,200~210 using ADD/DEL", ver), func() {
+			id, minID, maxID := 101, 200, 210
+			tc := testCase{
+				cniVersion: ver,
+				isLayer2:   true,
+				vlanTrunk: []*VlanTrunk{
+					{ID: &id},
+					{
+						MinID: &minID,
+						MaxID: &maxID,
+					},
+				},
+				AddErr020: "cannot convert: no valid IP addresses",
+				AddErr010: "cannot convert: no valid IP addresses",
+			}
+			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,