From b34402abd3d90f05e31ceaf2e9291d0b310a8977 Mon Sep 17 00:00:00 2001 From: Federico Paolinelli Date: Fri, 18 Sep 2020 11:27:46 +0200 Subject: [PATCH] VRF CNI: Add an optional table parameter. When specified from the user, the VRF will get assigned to the given tableid instead of having the CNI to choose for a free one. Signed-off-by: Federico Paolinelli --- plugins/meta/vrf/README.md | 5 ++- plugins/meta/vrf/main.go | 10 ++++- plugins/meta/vrf/vrf.go | 12 +++-- plugins/meta/vrf/vrf_test.go | 87 ++++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 7 deletions(-) diff --git a/plugins/meta/vrf/README.md b/plugins/meta/vrf/README.md index d2dc64cc..66342021 100644 --- a/plugins/meta/vrf/README.md +++ b/plugins/meta/vrf/README.md @@ -4,7 +4,7 @@ This plugin creates a [VRF](https://www.kernel.org/doc/Documentation/networking/vrf.txt) in the network namespace and assigns it the interface passed in the arguments. If the VRF is already present in the namespace, it only adds the interface to it. -As a table id is mandatory, the plugin generates a new one for each different VRF that is added to the namespace. +The table id is mandatory for VRF but optional in the CNI configuration. If not specified, the plugin generates a new one for each different VRF that is added to the namespace. It does not create any network interfaces and therefore does not bring connectivity by itself. It is only useful when used in addition to other plugins. @@ -50,4 +50,5 @@ The only configuration is the name of the VRF, as per the following example: The following [args conventions](https://github.com/containernetworking/cni/blob/master/CONVENTIONS.md#args-in-network-config) are supported: -* `vrfname` (string, optional): The name of the VRF to be created and to be set as master of the interface +* `vrfname` (string): The name of the VRF to be created and to be set as master of the interface +* `table` (int, optional): The route table to be associated to the created VRF. diff --git a/plugins/meta/vrf/main.go b/plugins/meta/vrf/main.go index e6f52c3f..1a0c179d 100644 --- a/plugins/meta/vrf/main.go +++ b/plugins/meta/vrf/main.go @@ -35,6 +35,8 @@ type VRFNetConf struct { // VRFName is the name of the vrf to add the interface to. VRFName string `json:"vrfname"` + // Table is the optional name of the routing table set for the vrf + Table uint32 `json:"table"` } func main() { @@ -54,8 +56,14 @@ func cmdAdd(args *skel.CmdArgs) error { err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { vrf, err := findVRF(conf.VRFName) + // If the user set a tableid and the vrf is already in the namespace + // we check if the tableid is the same one already assigned to the vrf. + if err == nil && conf.Table != 0 && vrf.Table != conf.Table { + return fmt.Errorf("VRF %s already exist with different routing table %d", conf.VRFName, vrf.Table) + } + if _, ok := err.(netlink.LinkNotFoundError); ok { - vrf, err = createVRF(conf.VRFName) + vrf, err = createVRF(conf.VRFName, conf.Table) } if err != nil { diff --git a/plugins/meta/vrf/vrf.go b/plugins/meta/vrf/vrf.go index 61a3d056..f265c071 100644 --- a/plugins/meta/vrf/vrf.go +++ b/plugins/meta/vrf/vrf.go @@ -35,15 +35,19 @@ func findVRF(name string) (*netlink.Vrf, error) { } // createVRF creates a new VRF and sets it up. -func createVRF(name string) (*netlink.Vrf, error) { +func createVRF(name string, tableID uint32) (*netlink.Vrf, error) { links, err := netlink.LinkList() if err != nil { return nil, fmt.Errorf("createVRF: Failed to find links %v", err) } - tableID, err := findFreeRoutingTableID(links) - if err != nil { - return nil, err + + if tableID == 0 { + tableID, err = findFreeRoutingTableID(links) + if err != nil { + return nil, err + } } + vrf := &netlink.Vrf{ LinkAttrs: netlink.LinkAttrs{ Name: name, diff --git a/plugins/meta/vrf/vrf_test.go b/plugins/meta/vrf/vrf_test.go index 2eb37e01..4b07886f 100644 --- a/plugins/meta/vrf/vrf_test.go +++ b/plugins/meta/vrf/vrf_test.go @@ -354,6 +354,69 @@ var _ = Describe("vrf plugin", func() { Entry("added to different vrfs with same ip IPV6", VRF0Name, VRF1Name, "2A00:0C98:2060:A000:0001:0000:1d1e:ca75/64", "2A00:0C98:2060:A000:0001:0000:1d1e:ca75/64"), ) + DescribeTable("handles tableid conflicts", + func(vrf0, vrf1 string, tableid0, tableid1 int, expectedError string) { + conf0 := configWithTableFor("test", IF0Name, vrf0, "10.0.0.2/24", tableid0) + conf1 := configWithTableFor("test1", IF1Name, vrf1, "10.0.0.2/24", tableid1) + + By("Adding the first interface to first vrf", func() { + err := originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: targetNS.Path(), + IfName: IF0Name, + StdinData: conf0, + } + _, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Checking that the first vrf has the right routing table", func() { + err := targetNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + l, err := netlink.LinkByName(vrf0) + Expect(err).NotTo(HaveOccurred()) + vrf := l.(*netlink.Vrf) + Expect(vrf.Table).To(Equal(uint32(tableid0))) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Adding the second interface to second vrf", func() { + err := originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: targetNS.Path(), + IfName: IF1Name, + StdinData: conf1, + } + _, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + return err + }) + if expectedError != "" { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(expectedError)) + return + } + Expect(err).NotTo(HaveOccurred()) + }) + }, + Entry("same vrf with same tableid", VRF0Name, VRF0Name, 1001, 1001, ""), + Entry("different vrf with same tableid", VRF0Name, VRF1Name, 1001, 1001, ""), + Entry("same vrf with different tableids", VRF0Name, VRF0Name, 1001, 1002, "already exist with different routing table"), + ) + It("removes the VRF only when the last interface is removed", func() { conf0 := configFor("test", IF0Name, VRF0Name, "10.0.0.2/24") conf1 := configFor("test1", IF1Name, VRF0Name, "10.0.0.2/24") @@ -605,6 +668,30 @@ func configFor(name, intf, vrf, ip string) []byte { return []byte(conf) } +func configWithTableFor(name, intf, vrf, ip string, tableID int) []byte { + conf := fmt.Sprintf(`{ + "name": "%s", + "type": "vrf", + "cniVersion": "0.3.1", + "vrfName": "%s", + "table": %d, + "prevResult": { + "interfaces": [ + {"name": "%s", "sandbox":"netns"} + ], + "ips": [ + { + "version": "4", + "address": "%s", + "gateway": "10.0.0.1", + "interface": 0 + } + ] + } + }`, name, vrf, tableID, intf, ip) + return []byte(conf) +} + func checkInterfaceOnVRF(vrfName, intfName string) { vrf, err := netlink.LinkByName(vrfName) Expect(err).NotTo(HaveOccurred())