From bf47e9aa1b7e961e11d57325c275b610d4f1919f Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Wed, 21 Nov 2018 17:57:10 +0200 Subject: [PATCH] Allow L2 connection for the bridge cni plugin. --- plugins/main/bridge/README.md | 13 ++- plugins/main/bridge/bridge.go | 182 +++++++++++++++-------------- plugins/main/bridge/bridge_test.go | 48 +++++--- 3 files changed, 144 insertions(+), 99 deletions(-) diff --git a/plugins/main/bridge/README.md b/plugins/main/bridge/README.md index 07eac591..75f8ede0 100644 --- a/plugins/main/bridge/README.md +++ b/plugins/main/bridge/README.md @@ -28,6 +28,17 @@ If the bridge is missing, the plugin will create one on first use and, if gatewa } ``` +## Example L2-only configuration +``` +{ + "cniVersion": "0.3.1", + "name": "mynet", + "type": "bridge", + "bridge": "mynet0", + "ipam": {} +} +``` + ## Network configuration reference * `name` (string, required): the name of the network. @@ -39,5 +50,5 @@ If the bridge is missing, the plugin will create one on first use and, if gatewa * `ipMasq` (boolean, optional): set up IP Masquerade on the host for traffic originating from this network and destined outside of it. Defaults to false. * `mtu` (integer, optional): explicitly set MTU to the specified value. Defaults to the value chosen by the kernel. * `hairpinMode` (boolean, optional): set hairpin mode for interfaces on the bridge. Defaults to false. -* `ipam` (dictionary, required): IPAM configuration to be used for this network. +* `ipam` (dictionary, required): IPAM configuration to be used for this network. For L2-only network, create empty dictionary. * `promiscMode` (boolean, optional): set promiscuous mode on the bridge. Defaults to false. diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index 2f14e603..b66f81f0 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -334,6 +334,8 @@ func cmdAdd(args *skel.CmdArgs) error { return err } + isLayer3 := n.IPAM.Type != "" + if n.IsDefaultGW { n.IsGW = true } @@ -358,104 +360,110 @@ func cmdAdd(args *skel.CmdArgs) error { return err } - // run the IPAM plugin and get back the config to apply - r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) - if err != nil { - return err - } + // Assume L2 interface only + result := ¤t.Result{CNIVersion: cniVersion, Interfaces: []*current.Interface{brInterface, hostInterface, containerInterface}} - // release IP in case of failure - defer func() { - if !success { - os.Setenv("CNI_COMMAND", "DEL") - ipam.ExecDel(n.IPAM.Type, args.StdinData) - os.Setenv("CNI_COMMAND", "ADD") - } - }() - - // Convert whatever the IPAM result was into the current Result type - result, err := current.NewResultFromResult(r) - if err != nil { - return err - } - - if len(result.IPs) == 0 { - return errors.New("IPAM plugin returned missing IP config") - } - - result.Interfaces = []*current.Interface{brInterface, hostInterface, containerInterface} - - // Gather gateway information for each IP family - gwsV4, gwsV6, err := calcGateways(result, n) - if err != nil { - return err - } - - // Configure the container hardware address and IP address(es) - if err := netns.Do(func(_ ns.NetNS) error { - contVeth, err := net.InterfaceByName(args.IfName) + if isLayer3 { + // run the IPAM plugin and get back the config to apply + r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) if err != nil { return err } - // Disable IPv6 DAD just in case hairpin mode is enabled on the - // bridge. Hairpin mode causes echos of neighbor solicitation - // packets, which causes DAD failures. - for _, ipc := range result.IPs { - if ipc.Version == "6" && (n.HairpinMode || n.PromiscMode) { - if err := disableIPV6DAD(args.IfName); err != nil { - return err - } - break + // release IP in case of failure + defer func() { + if !success { + os.Setenv("CNI_COMMAND", "DEL") + ipam.ExecDel(n.IPAM.Type, args.StdinData) + os.Setenv("CNI_COMMAND", "ADD") } - } + }() - // Add the IP to the interface - if err := ipam.ConfigureIface(args.IfName, result); err != nil { + // Convert whatever the IPAM result was into the current Result type + ipamResult, err := current.NewResultFromResult(r) + if err != nil { return err } - // Send a gratuitous arp - for _, ipc := range result.IPs { - if ipc.Version == "4" { - _ = arping.GratuitousArpOverIface(ipc.Address.IP, *contVeth) - } + result.IPs = ipamResult.IPs + result.Routes = ipamResult.Routes + + if len(result.IPs) == 0 { + return errors.New("IPAM plugin returned missing IP config") } - return nil - }); err != nil { - return err - } - if n.IsGW { - var firstV4Addr net.IP - // Set the IP address(es) on the bridge and enable forwarding - for _, gws := range []*gwInfo{gwsV4, gwsV6} { - for _, gw := range gws.gws { - if gw.IP.To4() != nil && firstV4Addr == nil { - firstV4Addr = gw.IP - } - - err = ensureBridgeAddr(br, gws.family, &gw, n.ForceAddress) - if err != nil { - return fmt.Errorf("failed to set bridge addr: %v", err) - } - } - - if gws.gws != nil { - if err = enableIPForward(gws.family); err != nil { - return fmt.Errorf("failed to enable forwarding: %v", err) - } - } + // Gather gateway information for each IP family + gwsV4, gwsV6, err := calcGateways(result, n) + if err != nil { + return err } - } - if n.IPMasq { - chain := utils.FormatChainName(n.Name, args.ContainerID) - comment := utils.FormatComment(n.Name, args.ContainerID) - for _, ipc := range result.IPs { - if err = ip.SetupIPMasq(ip.Network(&ipc.Address), chain, comment); err != nil { + // Configure the container hardware address and IP address(es) + if err := netns.Do(func(_ ns.NetNS) error { + contVeth, err := net.InterfaceByName(args.IfName) + if err != nil { return err } + + // Disable IPv6 DAD just in case hairpin mode is enabled on the + // bridge. Hairpin mode causes echos of neighbor solicitation + // packets, which causes DAD failures. + for _, ipc := range result.IPs { + if ipc.Version == "6" && (n.HairpinMode || n.PromiscMode) { + if err := disableIPV6DAD(args.IfName); err != nil { + return err + } + break + } + } + + // Add the IP to the interface + if err := ipam.ConfigureIface(args.IfName, result); err != nil { + return err + } + + // Send a gratuitous arp + for _, ipc := range result.IPs { + if ipc.Version == "4" { + _ = arping.GratuitousArpOverIface(ipc.Address.IP, *contVeth) + } + } + return nil + }); err != nil { + return err + } + + if n.IsGW { + var firstV4Addr net.IP + // Set the IP address(es) on the bridge and enable forwarding + for _, gws := range []*gwInfo{gwsV4, gwsV6} { + for _, gw := range gws.gws { + if gw.IP.To4() != nil && firstV4Addr == nil { + firstV4Addr = gw.IP + } + + err = ensureBridgeAddr(br, gws.family, &gw, n.ForceAddress) + if err != nil { + return fmt.Errorf("failed to set bridge addr: %v", err) + } + } + + if gws.gws != nil { + if err = enableIPForward(gws.family); err != nil { + return fmt.Errorf("failed to enable forwarding: %v", err) + } + } + } + } + + if n.IPMasq { + chain := utils.FormatChainName(n.Name, args.ContainerID) + comment := utils.FormatComment(n.Name, args.ContainerID) + for _, ipc := range result.IPs { + if err = ip.SetupIPMasq(ip.Network(&ipc.Address), chain, comment); err != nil { + return err + } + } } } @@ -485,8 +493,12 @@ func cmdDel(args *skel.CmdArgs) error { return err } - if err := ipam.ExecDel(n.IPAM.Type, args.StdinData); err != nil { - return err + isLayer3 := n.IPAM.Type != "" + + if isLayer3 { + if err := ipam.ExecDel(n.IPAM.Type, args.StdinData); err != nil { + return err + } } if args.Netns == "" { @@ -510,7 +522,7 @@ func cmdDel(args *skel.CmdArgs) error { return err } - if n.IPMasq { + if isLayer3 && n.IPMasq { chain := utils.FormatChainName(n.Name, args.ContainerID) comment := utils.FormatComment(n.Name, args.ContainerID) for _, ipn := range ipnets { diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index aa1f8300..420773bf 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -47,6 +47,7 @@ type testCase struct { gateway string // Single subnet config: Gateway ranges []rangeInfo // Ranges list (multiple subnets config) isGW bool + isLayer2 bool expGWCIDRs []string // Expected gateway addresses in CIDR form } @@ -77,7 +78,9 @@ const ( "cniVersion": "%s", "name": "testConfig", "type": "bridge", - "bridge": "%s", + "bridge": "%s",` + + netDefault = ` "isDefaultGateway": true, "ipMasq": false` @@ -117,18 +120,24 @@ const ( // for a test case. func (tc testCase) netConfJSON(dataDir string) string { conf := fmt.Sprintf(netConfStr, tc.cniVersion, BRNAME) - if tc.subnet != "" || tc.ranges != nil { - conf += ipamStartStr - if dataDir != "" { - conf += fmt.Sprintf(ipamDataDirStr, dataDir) + if !tc.isLayer2 { + conf += netDefault + if tc.subnet != "" || tc.ranges != nil { + conf += ipamStartStr + if dataDir != "" { + conf += fmt.Sprintf(ipamDataDirStr, dataDir) + } + if tc.subnet != "" { + conf += tc.subnetConfig() + } + if tc.ranges != nil { + conf += tc.rangesConfig() + } + conf += ipamEndStr } - if tc.subnet != "" { - conf += tc.subnetConfig() - } - if tc.ranges != nil { - conf += tc.rangesConfig() - } - conf += ipamEndStr + } else { + conf += ` + "ipam": {}` } return "{" + conf + "\n}" } @@ -336,7 +345,10 @@ func (tester *testerV03x) cmdAddTest(tc testCase, dataDir string) { // 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 - Expect(link.Attrs().HardwareAddr.String()).NotTo(Equal(bridgeMAC)) + // this check is not relevant for a layer 2 bridge + if !tc.isLayer2 { + Expect(link.Attrs().HardwareAddr.String()).NotTo(Equal(bridgeMAC)) + } return nil }) @@ -700,6 +712,16 @@ var _ = Describe("bridge Operations", func() { } }) + It("configures and deconfigures a l2 bridge and veth with ADD/DEL for 0.3.1 config", func() { + tc := testCase{cniVersion: "0.3.0", isLayer2: true} + cmdAddDelTest(originalNS, tc, dataDir) + }) + + It("configures and deconfigures a l2 bridge and veth with ADD/DEL for 0.3.1 config", func() { + tc := testCase{cniVersion: "0.3.1", isLayer2: true} + cmdAddDelTest(originalNS, tc, dataDir) + }) + It("configures and deconfigures a bridge and veth with default route with ADD/DEL for 0.3.1 config", func() { testCases := []testCase{ {