Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bridge: add vlan trunk support #689

Closed
wants to merge 1 commit into from
Closed

bridge: add vlan trunk support #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 96 additions & 16 deletions plugins/main/bridge/bridge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"net"
"os"
"runtime"
"sort"
"syscall"
"time"

Expand All @@ -46,17 +47,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"`
VlanTrunk []*VlanTrunk `json:"vlanTrunk,omitempty"`
MacSpoofChk bool `json:"macspoofchk,omitempty"`
EnableDad bool `json:"enabledad,omitempty"`

Args struct {
Cni BridgeArgs `json:"cni,omitempty"`
Expand All @@ -65,7 +67,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 {
Expand Down Expand Up @@ -101,6 +110,11 @@ 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
if n.vlans, err = collectVlanTrunk(n.VlanTrunk); err != nil {
Comment on lines +113 to +114
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be more consistent with the rest of the code to use the scoped variable and not define err?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will still need to copy the value back to n.vlans later.
So I will consider to keep current code

// fail to parsing
return nil, "", err
}

if envArgs != "" {
e := MacEnvArgs{}
Expand All @@ -124,6 +138,59 @@ 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/main/pkg/plugin/plugin.go
tjjh89017 marked this conversation as resolved.
Show resolved Hide resolved
func collectVlanTrunk(vlanTrunk []*VlanTrunk) ([]int, error) {
if vlanTrunk == nil {
return nil, nil
}

vlanMap := make(map[int]bool)
for _, item := range vlanTrunk {
var minID int = 0
var maxID int = 0
var ID int = 0
if item.MinID != nil {
minID = *item.MinID
if minID < 0 || minID > 4094 {
return nil, errors.New("incorrect trunk minID parameter")
}
}
if item.MaxID != nil {
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")
}
}
if minID > 0 && maxID > 0 {
for v := minID; v <= maxID; v++ {
vlanMap[v] = true
}
}

// single vid
if item.ID != nil {
ID = *item.ID
if ID < 0 || ID > 4094 {
return nil, errors.New("incorrect trunk id parameter")
}
vlanMap[ID] = true
}
}

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
Expand Down Expand Up @@ -314,7 +381,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, nil, "")
if err != nil {
return nil, fmt.Errorf("faild to create vlan gateway %q: %v", name, err)
}
Expand All @@ -333,7 +400,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, vlans []int, mac string) (*current.Interface, *current.Interface, error) {
contIface := &current.Interface{}
hostIface := &current.Interface{}

Expand Down Expand Up @@ -377,6 +444,15 @@ func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairp
}
}

if vlans != nil {
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
}

Expand All @@ -387,7 +463,7 @@ func calcGatewayIP(ipn *net.IPNet) net.IP {

func setupBridge(n *NetConf) (*netlink.Bridge, *current.Interface, error) {
vlanFiltering := false
if n.Vlan != 0 {
if n.Vlan != 0 || n.VlanTrunk != nil {
vlanFiltering = true
}
// create bridge if necessary
Expand Down Expand Up @@ -427,6 +503,10 @@ func cmdAdd(args *skel.CmdArgs) error {
return fmt.Errorf("cannot set hairpin mode and promiscuous mode at the same time.")
}

if n.vlans != nil && len(n.vlans) > 0 && isLayer3 {
return fmt.Errorf("cannot set vlanTrunk and IPAM at the same time.")
}

br, brInterface, err := setupBridge(n)
if err != nil {
return err
Expand All @@ -438,7 +518,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.vlans, n.mac)
if err != nil {
return err
}
Expand Down
118 changes: 115 additions & 3 deletions plugins/main/bridge/bridge_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type testCase struct {
isLayer2 bool
expGWCIDRs []string // Expected gateway addresses in CIDR form
vlan int
vlanTrunk []*VlanTrunk
ipMasq bool
macspoofchk bool
AddErr020 string
Expand Down Expand Up @@ -129,6 +130,23 @@ const (
vlan = `,
"vlan": %d`

vlanTrunkStartStr = `,
"vlanTrunk": [`

vlanTrunk = `
{
"id": %d
}`

vlanTrunkRange = `
{
"minID": %d,
"maxID": %d
}`

vlanTrunkEndStr = `
]`

netDefault = `,
"isDefaultGateway": true`

Expand Down Expand Up @@ -189,6 +207,23 @@ func (tc testCase) netConfJSON(dataDir string) string {
if tc.vlan != 0 {
conf += fmt.Sprintf(vlan, tc.vlan)
}

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()
}
Expand Down Expand Up @@ -503,7 +538,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(Equal(true))
} else {
Expect(*link.(*netlink.Bridge).VlanFiltering).To(Equal(false))
Expand Down Expand Up @@ -557,6 +592,25 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result,
Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
}

// 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 _, vlan_entry := range tc.vlanTrunk {
if vlan_entry.ID != nil {
Expect(checkVlan(*vlan_entry.ID, vlans)).To(BeTrue())
}
if vlan_entry.MinID != nil && vlan_entry.MaxID != nil {
for vid := *vlan_entry.MinID; vid <= *vlan_entry.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
Expand Down Expand Up @@ -803,7 +857,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(Equal(true))
} else {
Expect(*link.(*netlink.Bridge).VlanFiltering).To(Equal(false))
Expand Down Expand Up @@ -857,6 +911,25 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result,
Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
}

// 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 _, vlan_entry := range tc.vlanTrunk {
if vlan_entry.ID != nil {
Expect(checkVlan(*vlan_entry.ID, vlans)).To(BeTrue())
}
if vlan_entry.MinID != nil && vlan_entry.MaxID != nil {
for vid := *vlan_entry.MinID; vid <= *vlan_entry.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
Expand Down Expand Up @@ -1103,7 +1176,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(Equal(true))
} else {
Expect(*link.(*netlink.Bridge).VlanFiltering).To(Equal(false))
Expand Down Expand Up @@ -1157,6 +1230,25 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (types.Result,
Expect(checkVlan(tc.vlan, vlans)).To(BeTrue())
}

// 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 _, vlan_entry := range tc.vlanTrunk {
if vlan_entry.ID != nil {
Expect(checkVlan(*vlan_entry.ID, vlans)).To(BeTrue())
}
if vlan_entry.MinID != nil && vlan_entry.MaxID != nil {
for vid := *vlan_entry.MinID; vid <= *vlan_entry.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
Expand Down Expand Up @@ -1736,6 +1828,26 @@ 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,
vlan: 100,
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)
})

for i, tc := range []testCase{
{
// IPv4 only
Expand Down