Skip to content

Commit

Permalink
Add parameter to disable default vlan
Browse files Browse the repository at this point in the history 
This new parameter allows users to remove the default vlan

Fixes: #667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
  • Loading branch information
@mlguerrero12
mlguerrero12 committed Apr 5, 2023
1 parent 47a4319 commit 821982d
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
View file
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
View file
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 821982d

Please sign in to comment.