From ca12d49b41ff354166ceeb089c254e7b5b43cd8d Mon Sep 17 00:00:00 2001 From: Artur Korzeniewski Date: Thu, 30 Mar 2023 13:19:54 +0200 Subject: [PATCH] Add routes propagation for VRF plugin Up until now, if previous plugin assigned routes to interface, movement of this interface to new VRF cause routes to be deleted. This patch adds funtionality to VRF plugin to save the routes before interface is assgined to VRF, and then re-apply all saved routes to new VRF. Signed-off-by: Artur Korzeniewski --- plugins/meta/vrf/vrf.go | 25 ++++++ plugins/meta/vrf/vrf_test.go | 168 ++++++++++++++++++++++++++++++++++- 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/plugins/meta/vrf/vrf.go b/plugins/meta/vrf/vrf.go index f265c071f..bee2a9bff 100644 --- a/plugins/meta/vrf/vrf.go +++ b/plugins/meta/vrf/vrf.go @@ -104,6 +104,19 @@ func addInterface(vrf *netlink.Vrf, intf string) error { if err != nil { return fmt.Errorf("failed getting ipv6 addresses for %s", intf) } + + // Save all routes that are not local and connected, before setting master, + // because otherwise those routes will be deleted after interface is moved. + filter := &netlink.Route{ + LinkIndex: i.Attrs().Index, + Scope: netlink.SCOPE_UNIVERSE, // Exclude local and connected routes + } + filterMask := netlink.RT_FILTER_IIF | netlink.RT_FILTER_SCOPE // Filter based on link index and scope + routes, err := netlink.RouteListFiltered(netlink.FAMILY_ALL, filter, filterMask) + if err != nil { + return fmt.Errorf("failed getting all routes for %s", intf) + } + err = netlink.LinkSetMaster(i, vrf) if err != nil { return fmt.Errorf("could not set vrf %s as master of %s: %v", vrf.Name, intf, err) @@ -130,6 +143,18 @@ CONTINUE: } } + // Apply all saved routes for the interface that was moved to the VRF + for _, route := range routes { + r := route + // Modify original table to vrf one, + r.Table = int(vrf.Table) + // equivalent of 'ip route replace
table '. + err = netlink.RouteReplace(&r) + if err != nil { + return fmt.Errorf("could not add route '%s': %v", r, err) + } + } + return nil } diff --git a/plugins/meta/vrf/vrf_test.go b/plugins/meta/vrf/vrf_test.go index 896298b11..9a8266770 100644 --- a/plugins/meta/vrf/vrf_test.go +++ b/plugins/meta/vrf/vrf_test.go @@ -17,10 +17,13 @@ package main import ( "encoding/json" "fmt" + "net" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -107,7 +110,7 @@ var _ = Describe("vrf plugin", func() { }, }) Expect(err).NotTo(HaveOccurred()) - _, err = netlink.LinkByName(IF0Name) + _, err = netlink.LinkByName(IF1Name) Expect(err).NotTo(HaveOccurred()) return nil }) @@ -177,6 +180,102 @@ var _ = Describe("vrf plugin", func() { Expect(err).NotTo(HaveOccurred()) }) + It("adds the interface and custom routing to new VRF", func() { + conf := configWithRouteFor("test", IF0Name, VRF0Name, "10.0.0.2/24", "10.10.10.0/24") + + By("Setting custom routing first", func() { + err := targetNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + ipv4, err := types.ParseCIDR("10.0.0.2/24") + Expect(err).NotTo(HaveOccurred()) + Expect(ipv4).NotTo(BeNil()) + + _, routev4, err := net.ParseCIDR("10.10.10.0/24") + Expect(err).NotTo(HaveOccurred()) + + ipv6, err := types.ParseCIDR("abcd:1234:ffff::cdde/64") + Expect(err).NotTo(HaveOccurred()) + Expect(ipv6).NotTo(BeNil()) + + _, routev6, err := net.ParseCIDR("1111:dddd::/80") + Expect(err).NotTo(HaveOccurred()) + Expect(routev6).NotTo(BeNil()) + + link, err := netlink.LinkByName(IF0Name) + Expect(err).NotTo(HaveOccurred()) + + // Add IP addresses for network reachability + netlink.AddrAdd(link, &netlink.Addr{IPNet: ipv4}) + netlink.AddrAdd(link, &netlink.Addr{IPNet: ipv6}) + + ipAddrs, err := netlink.AddrList(link, netlink.FAMILY_V4) + Expect(err).NotTo(HaveOccurred()) + // Check if address was assigned properly + Expect(ipAddrs[0].IP.String()).To(Equal("10.0.0.2")) + + // Set interface UP, otherwise local route to 10.0.0.0/24 is not present + err = netlink.LinkSetUp(link) + Expect(err).NotTo(HaveOccurred()) + + // Add additional route to 10.10.10.0/24 via 10.0.0.1 gateway + r := netlink.Route{ + LinkIndex: link.Attrs().Index, + Src: ipv4.IP, + Dst: routev4, + Gw: net.ParseIP("10.0.0.1"), + } + err = netlink.RouteAdd(&r) + Expect(err).NotTo(HaveOccurred()) + + r6 := netlink.Route{ + LinkIndex: link.Attrs().Index, + Src: ipv6.IP, + Dst: routev6, + Gw: net.ParseIP("abcd:1234:ffff::1"), + } + err = netlink.RouteAdd(&r6) + Expect(err).NotTo(HaveOccurred()) + + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: targetNS.Path(), + IfName: IF0Name, + StdinData: conf, + } + + err := originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + r, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + + result, err := current.GetResult(r) + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Interfaces).To(HaveLen(1)) + Expect(result.Interfaces[0].Name).To(Equal(IF0Name)) + Expect(result.Routes).To(HaveLen(1)) + Expect(result.Routes[0].Dst.IP.String()).To(Equal("10.10.10.0")) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + err = targetNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + checkInterfaceOnVRF(VRF0Name, IF0Name) + checkRoutesOnVRF(VRF0Name, IF0Name, "10.10.10.0/24", "1111:dddd::/80") + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) + It("fails if the interface already has a master set", func() { conf := configFor("test", IF0Name, VRF0Name, "10.0.0.2/24") @@ -690,6 +789,35 @@ func configWithTableFor(name, intf, vrf, ip string, tableID int) []byte { return []byte(conf) } +func configWithRouteFor(name, intf, vrf, ip, route string) []byte { + conf := fmt.Sprintf(`{ + "name": "%s", + "type": "vrf", + "cniVersion": "0.3.1", + "vrfName": "%s", + "prevResult": { + "interfaces": [ + {"name": "%s", "sandbox":"netns"} + ], + "ips": [ + { + "version": "4", + "address": "%s", + "gateway": "10.0.0.1", + "interface": 0 + } + ], + "routes": [ + { + "dst": "%s", + "gw": "10.0.0.1" + } + ] + } + }`, name, vrf, intf, ip, route) + return []byte(conf) +} + func checkInterfaceOnVRF(vrfName, intfName string) { vrf, err := netlink.LinkByName(vrfName) Expect(err).NotTo(HaveOccurred()) @@ -702,3 +830,41 @@ func checkInterfaceOnVRF(vrfName, intfName string) { Expect(err).NotTo(HaveOccurred()) Expect(master.Attrs().Name).To(Equal(vrfName)) } + +func checkRoutesOnVRF(vrfName, intfName string, routesToCheck ...string) { + vrf, err := netlink.LinkByName(vrfName) + Expect(err).NotTo(HaveOccurred()) + Expect(vrf).To(BeAssignableToTypeOf(&netlink.Vrf{})) + + link, err := netlink.LinkByName(intfName) + Expect(err).NotTo(HaveOccurred()) + + err = netlink.LinkSetUp(link) + Expect(err).NotTo(HaveOccurred()) + + ipAddrs, err := netlink.AddrList(link, netlink.FAMILY_V4) + Expect(err).NotTo(HaveOccurred()) + Expect(ipAddrs).To(HaveLen(1)) + Expect(ipAddrs[0].IP.String()).To(Equal("10.0.0.2")) + + // Need to read all tables, so cannot use RouteList + routeFilter := &netlink.Route{ + LinkIndex: link.Attrs().Index, + Table: unix.RT_TABLE_UNSPEC, + } + + routes, err := netlink.RouteListFiltered(netlink.FAMILY_ALL, + routeFilter, + netlink.RT_FILTER_OIF|netlink.RT_FILTER_TABLE) + Expect(err).NotTo(HaveOccurred()) + + routesRead := []string{} + for _, route := range routes { + routesRead = append(routesRead, route.String()) + Expect(uint32(route.Table)).To(Equal(vrf.(*netlink.Vrf).Table)) + } + routesStr := strings.Join(routesRead, "\n") + for _, route := range routesToCheck { + Expect(routesStr).To(ContainSubstring(route)) + } +}