From 821982da1c9b96870b2459d1cb972d6e5dc0a1d2 Mon Sep 17 00:00:00 2001 From: Marcelo Guerrero Viveros Date: Fri, 31 Mar 2023 18:19:16 +0200 Subject: [PATCH] Add parameter to disable default vlan This new parameter allows users to remove the default vlan Fixes: #667 Signed-off-by: Marcelo Guerrero Viveros --- plugins/main/bridge/bridge.go | 61 +++++++++++++++++------ plugins/main/bridge/bridge_test.go | 79 ++++++++++++++++++++++++------ 2 files changed, 109 insertions(+), 31 deletions(-) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index c4c33730..b8af4043 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -46,17 +46,18 @@ 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"` - 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"` + PreserveDefaultVlan bool `json:"preserveDefaultVlan"` + 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 := ¤t.Interface{} hostIface := ¤t.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) } diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 296ee237..78fc0d70 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/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 - subnet string // Single subnet config: Subnet CIDR - gateway string // Single subnet config: Gateway - ranges []rangeInfo // Ranges list (multiple subnets config) - resolvConf string // host-local resolvConf file path - isGW bool - isLayer2 bool - expGWCIDRs []string // Expected gateway addresses in CIDR form - vlan int - ipMasq bool - macspoofchk bool - AddErr020 string - DelErr020 string - AddErr010 string - DelErr010 string + cniVersion string // CNI Version + subnet string // Single subnet config: Subnet CIDR + gateway string // Single subnet config: Gateway + ranges []rangeInfo // Ranges list (multiple subnets config) + resolvConf string // host-local resolvConf file path + isGW bool + isLayer2 bool + expGWCIDRs []string // Expected gateway addresses in CIDR form + vlan int + removeDefaultVlan bool + ipMasq bool + macspoofchk bool + AddErr020 string + DelErr020 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{