From 090af7db9a275cccb8759a903710580a8e4801ca Mon Sep 17 00:00:00 2001 From: Date Huang Date: Thu, 6 Jan 2022 15:34:11 +0800 Subject: [PATCH] bridge: add vlan trunk support add vlan trunk support for veth vlan trunk only support L2 only mode without any IPAM refer ovs-cni design https://github.com/k8snetworkplumbingwg/ovs-cni/blob/main/pkg/plugin/plugin.go design: origin "vlan" option will be PVID or untagged vlan for the network. "vlanTrunk" will setup tagged vlan for veth. entry type: `{ "id": 100 }` will specify only tagged vlan 100 `{ "minID": 100, "maxID": 120 }` will specify tagged vlan from 100 to 120 (include 100 and 120) vlanTrunk is a list of above entry type, so you can use this to add tagged vlan `[ { "id": 100 }, { "minID": 1000, "maxID": 2000 } ]` complete config will be like this { "cniVersion": "0.3.1", "name": "mynet", "type": "bridge", "bridge": "mynet0", "vlan": 100, "vlanTrunk": [ { "id": 101 }, { "minID": 1000, "maxID": 2000 }, { "minID": 3000, "maxID": 4000 } ], "ipam": {} } Signed-off-by: Date Huang --- plugins/main/bridge/bridge.go | 132 ++++++++++++++++++---- plugins/main/bridge/bridge_test.go | 172 ++++++++++++++++++++++++++++- 2 files changed, 278 insertions(+), 26 deletions(-) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index b8af4043..f4e61c72 100644 --- a/plugins/main/bridge/bridge.go +++ b/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"` - IsGW bool `json:"isGateway"` - IsDefaultGW bool `json:"isDefaultGateway"` - ForceAddress bool `json:"forceAddress"` - IPMasq bool `json:"ipMasq"` - MTU int `json:"mtu"` - HairpinMode bool `json:"hairpinMode"` - PromiscMode bool `json:"promiscMode"` - Vlan int `json:"vlan"` - PreserveDefaultVlan bool `json:"preserveDefaultVlan"` - MacSpoofChk bool `json:"macspoofchk,omitempty"` - EnableDad bool `json:"enabledad,omitempty"` + BrName string `json:"bridge"` + IsGW bool `json:"isGateway"` + IsDefaultGW bool `json:"isDefaultGateway"` + ForceAddress bool `json:"forceAddress"` + IPMasq bool `json:"ipMasq"` + MTU int `json:"mtu"` + HairpinMode bool `json:"hairpinMode"` + PromiscMode bool `json:"promiscMode"` + Vlan int `json:"vlan"` + VlanTrunk []*VlanTrunk `json:"vlanTrunk,omitempty"` + PreserveDefaultVlan bool `json:"preserveDefaultVlan"` + 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 := ¤t.Interface{} hostIface := ¤t.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 !preserveDefaultVlan { - err = removeDefaultVlan(hostVeth) - if err != nil { - return nil, nil, fmt.Errorf("failed to remove default vlan on interface %q: %v", hostIface.Name, err) - } + if (vlanID != 0 || len(vlans) > 0) && !preserveDefaultVlan { + err = removeDefaultVlan(hostVeth) + if err != nil { + 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 } diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 78fc0d70..74b82ce3 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/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,