From 2cbad1316dc67ce2cc85ef3e67d2580675ff1bfb Mon Sep 17 00:00:00 2001 From: boebu Date: Wed, 6 Aug 2025 16:11:01 +0200 Subject: [PATCH] bridge-cni: add uplinkInterface parameter Allow bridge-cni to configure the bridge's uplink interface in L2-only vlan configuration with additional VLANs as defined by the CNI. Signed-off-by: boebu --- plugins/main/bridge/bridge.go | 30 +++++ plugins/main/bridge/bridge_test.go | 204 ++++++++++++++++++++++++++++- 2 files changed, 228 insertions(+), 6 deletions(-) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index 0b5a157c0..92d08ca8b 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -63,6 +63,7 @@ type NetConf struct { EnableDad bool `json:"enabledad,omitempty"` DisableContainerInterface bool `json:"disableContainerInterface,omitempty"` PortIsolation bool `json:"portIsolation,omitempty"` + UplinkInterface string `json:"uplinkInterface,omitempty"` Args struct { Cni BridgeArgs `json:"cni,omitempty"` @@ -525,6 +526,29 @@ func setupBridge(n *NetConf) (*netlink.Bridge, *current.Interface, error) { }, nil } +func ensureUplinkVlan(n *NetConf) error { + uplinkIf, err := netlinksafe.LinkByName(n.UplinkInterface) + if err != nil { + return fmt.Errorf("failed to find uplink interface %q: %v", n.UplinkInterface, err) + } + + if n.Vlan != 0 { + err = netlink.BridgeVlanAdd(uplinkIf, uint16(n.Vlan), false, false, false, false) + if err != nil { + return fmt.Errorf("failed to setup vlan tag on interface %q: %v", uplinkIf, err) + } + } + + for _, vlan := range n.vlans { + err = netlink.BridgeVlanAdd(uplinkIf, uint16(vlan), false, false, false, false) + if err != nil { + return fmt.Errorf("failed to setup vlan tag on interface %q: %w", uplinkIf, err) + } + } + + return nil +} + func enableIPForward(family int) error { if family == netlink.FAMILY_V4 { return ip.EnableIP4Forward() @@ -570,6 +594,12 @@ func cmdAdd(args *skel.CmdArgs) error { return err } + if n.UplinkInterface != "" { + if err = ensureUplinkVlan(n); err != nil { + return err + } + } + // Assume L2 interface only result := ¤t.Result{ CNIVersion: current.ImplementedSpecVersion, diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index c9e68e2f3..d823ecb13 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -84,6 +84,7 @@ type testCase struct { macspoofchk bool disableContIface bool portIsolation bool + uplinkInterface string AddErr020 string DelErr020 string @@ -112,6 +113,12 @@ type rangeInfo struct { gateway string } +type BridgeFixture struct { + NS ns.NetNS + BridgeName string + UplinkName string +} + // netConf() creates a NetConf structure for a test case. func (tc testCase) netConf() *NetConf { return &NetConf{ @@ -167,6 +174,9 @@ const ( portIsolation = `, "portIsolation": true` + uplinkInterface = `, + "uplinkInterface": "%s"` + ipamStartStr = `, "ipam": { "type": "host-local"` @@ -275,6 +285,10 @@ func (tc testCase) netConfJSON(dataDir string) string { conf += portIsolation } + if tc.uplinkInterface != "" { + conf += fmt.Sprintf(uplinkInterface, tc.uplinkInterface) + } + if !tc.isLayer2 { conf += netDefault if tc.subnet != "" || tc.ranges != nil { @@ -467,6 +481,91 @@ func ipVersion(ip net.IP) string { return "6" } +func NewUplinkBridgeFixture(netns ns.NetNS, bridgeName string) (*BridgeFixture, error) { + uplinkName := "dummy0" + + err := netns.Do(func(ns.NetNS) error { + link := &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Name: uplinkName, + }, + } + if err := netlink.LinkAdd(link); err != nil { + return fmt.Errorf("add dummy: %w", err) + } + if err := netlink.LinkSetUp(link); err != nil { + return fmt.Errorf("up dummy: %w", err) + } + return nil + }) + if err != nil { + return nil, err + } + + // Create bridge + err = netns.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}} + if err := netlink.LinkAdd(bridge); err != nil { + return fmt.Errorf("add bridge: %w", err) + } + if err := netlink.BridgeSetVlanFiltering(bridge, true); err != nil { + return fmt.Errorf("vlan filtering: %w", err) + } + if err := netlink.LinkSetUp(bridge); err != nil { + return fmt.Errorf("up bridge: %w", err) + } + return nil + }) + if err != nil { + _ = deleteLink(netns, uplinkName) + return nil, err + } + + // Attach dummy to bridge + err = netns.Do(func(ns.NetNS) error { + link, err := netlinksafe.LinkByName(uplinkName) + if err != nil { + return fmt.Errorf("lookup dummy: %w", err) + } + bridge, err := netlinksafe.LinkByName(bridgeName) + if err != nil { + return fmt.Errorf("lookup bridge: %w", err) + } + return netlink.LinkSetMaster(link, bridge.(*netlink.Bridge)) + }) + if err != nil { + _ = deleteLink(netns, uplinkName) + _ = deleteLink(netns, bridgeName) + return nil, err + } + + return &BridgeFixture{ + NS: netns, + BridgeName: bridgeName, + UplinkName: uplinkName, + }, nil +} + +func (f *BridgeFixture) Teardown() { + _ = f.NS.Do(func(ns.NetNS) error { + _ = deleteLink(f.NS, f.UplinkName) + _ = deleteLink(f.NS, f.BridgeName) + return nil + }) +} + +func deleteLink(netns ns.NetNS, name string) error { + return netns.Do(func(ns.NetNS) error { + link, err := netlinksafe.LinkByName(name) + if err != nil { + return nil // Already deleted or never existed + } + return netlink.LinkDel(link) + }) +} + func countIPAMIPs(path string) (int, error) { count := 0 entries, err := os.ReadDir(path) @@ -647,9 +746,12 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result, // Check for the veth link in the main namespace links, err := netlinksafe.LinkList() Expect(err).NotTo(HaveOccurred()) - if !tc.isLayer2 && tc.vlan != 0 { + switch { + case !tc.isLayer2 && tc.vlan != 0: Expect(links).To(HaveLen(5)) // Bridge, Bridge vlan veth, veth, and loopback - } else { + case (tc.vlan != 0 || len(tc.vlanTrunk) > 0) && tc.uplinkInterface != "": + Expect(links).To(HaveLen(4)) // Bridge, veth, uplink interface and loopback + default: Expect(links).To(HaveLen(3)) // Bridge, veth, and loopback } @@ -999,9 +1101,12 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result, // Check for the veth link in the main namespace links, err := netlinksafe.LinkList() Expect(err).NotTo(HaveOccurred()) - if !tc.isLayer2 && tc.vlan != 0 { + switch { + case !tc.isLayer2 && tc.vlan != 0: Expect(links).To(HaveLen(5)) // Bridge, Bridge vlan veth, veth, and loopback - } else { + case (tc.vlan != 0 || len(tc.vlanTrunk) > 0) && tc.uplinkInterface != "": + Expect(links).To(HaveLen(4)) // Bridge, veth, uplink interface and loopback + default: Expect(links).To(HaveLen(3)) // Bridge, veth, and loopback } @@ -1048,6 +1153,30 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result, Expect(checkVlan(nativeVlan, vlans)).To(BeTrue()) } + // Check if uplink interface has vlans + if tc.uplinkInterface != "" { + uplink, err := netlinksafe.LinkByName(tc.uplinkInterface) + Expect(err).NotTo(HaveOccurred()) + Expect(uplink).To(BeAssignableToTypeOf(&netlink.Dummy{})) + interfaceMap, err := netlinksafe.BridgeVlanList() + Expect(err).NotTo(HaveOccurred()) + vlans, isExist := interfaceMap[int32(link.Attrs().Index)] + Expect(isExist).To(BeTrue()) + if tc.vlan != 0 { + Expect(checkVlan(tc.vlan, vlans)).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 @@ -1340,9 +1469,12 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) (types.Result, // Check for the veth link in the main namespace links, err := netlinksafe.LinkList() Expect(err).NotTo(HaveOccurred()) - if !tc.isLayer2 && tc.vlan != 0 { + switch { + case !tc.isLayer2 && tc.vlan != 0: Expect(links).To(HaveLen(5)) // Bridge, Bridge vlan veth, veth, and loopback - } else { + case (tc.vlan != 0 || len(tc.vlanTrunk) > 0) && tc.uplinkInterface != "": + Expect(links).To(HaveLen(4)) // Bridge, veth, uplink interface and loopback + default: Expect(links).To(HaveLen(3)) // Bridge, veth, and loopback } @@ -2092,6 +2224,66 @@ var _ = Describe("bridge Operations", func() { cmdAddDelTest(originalNS, targetNS, tc, dataDir) }) + It(fmt.Sprintf("[%s] configures and deconfigures a bridge with vlan id 100 and uplink interface with ADD/DEL", ver), func() { + fixture, err := NewUplinkBridgeFixture(originalNS, BRNAME) + Expect(err).NotTo(HaveOccurred()) + defer fixture.Teardown() + tc := testCase{ + cniVersion: ver, + isLayer2: true, + vlan: 100, + uplinkInterface: fixture.UplinkName, + 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 bridge with vlanTrunk and uplink interface using ADD/DEL", ver), func() { + fixture, err := NewUplinkBridgeFixture(originalNS, BRNAME) + Expect(err).NotTo(HaveOccurred()) + defer fixture.Teardown() + id, minID, maxID := 101, 200, 210 + tc := testCase{ + cniVersion: ver, + isLayer2: true, + uplinkInterface: fixture.UplinkName, + 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 bridge with vlan and vlanTrunk and uplink interface using ADD/DEL", ver), func() { + fixture, err := NewUplinkBridgeFixture(originalNS, BRNAME) + Expect(err).NotTo(HaveOccurred()) + defer fixture.Teardown() + id, minID, maxID := 101, 200, 210 + tc := testCase{ + cniVersion: ver, + isLayer2: true, + uplinkInterface: fixture.UplinkName, + 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