Skip to content

Commit

Permalink
Merge pull request #875 from mlguerrero12/adddefaultvlanparam
Browse files Browse the repository at this point in the history
Add parameter to disable default vlan
  • Loading branch information
squeed committed Apr 17, 2023
2 parents 71aa710 + 821982d commit 9f1f9a5
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 31 deletions.
61 changes: 45 additions & 16 deletions plugins/main/bridge/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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{}

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down
79 changes: 64 additions & 15 deletions plugins/main/bridge/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -129,6 +130,9 @@ const (
vlan = `,
"vlan": %d`

preserveDefaultVlan = `,
"preserveDefaultVlan": false`

netDefault = `,
"isDefaultGateway": true`

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit 9f1f9a5

Please sign in to comment.