SBR: option to pass the table id (#1088)

* Use of Table ID in IPAM

Signed-off-by: Lionel Jouin <[email protected]>

* SBR: option to pass the table id

Using the option to set the table number in the SBR meta plugin will
create a policy route for each IP added for the interface returned by
the main plugin.
Unlike the default behavior, the routes will not be moved to the table.
The default behavior of the SBR plugin is kept if the table id is not set.

Signed-off-by: Lionel Jouin <[email protected]>

---------

Signed-off-by: Lionel Jouin <[email protected]>

pkg/ipam/ipam_linux.go

@@ -119,8 +119,12 @@ func ConfigureIface(ifName string, res *current.Result) error {
 			Gw:        gw,
 		}
 
+		if r.Table != nil {
+			route.Table = *r.Table
+		}
+
 		if err = netlink.RouteAddEcmp(&route); err != nil {
-			return fmt.Errorf("failed to add route '%v via %v dev %v': %v", r.Dst, gw, ifName, err)
+			return fmt.Errorf("failed to add route '%v via %v dev %v (Table: %d)': %v", r.Dst, gw, ifName, route.Table, err)
 		}
 	}
 

pkg/ipam/ipam_linux_test.go

@@ -44,6 +44,7 @@ var _ = Describe("ConfigureIface", func() {
 	var ipv4, ipv6, routev4, routev6 *net.IPNet
 	var ipgw4, ipgw6, routegwv4, routegwv6 net.IP
 	var result *current.Result
+	var routeTable int
 
 	BeforeEach(func() {
 		// Create a new NetNS so we don't modify the host
@@ -93,6 +94,8 @@ var _ = Describe("ConfigureIface", func() {
 		ipgw6 = net.ParseIP("abcd:1234:ffff::1")
 		Expect(ipgw6).NotTo(BeNil())
 
+		routeTable := 5000
+
 		result = &current.Result{
 			Interfaces: []*current.Interface{
 				{
@@ -121,6 +124,7 @@ var _ = Describe("ConfigureIface", func() {
 			Routes: []*types.Route{
 				{Dst: *routev4, GW: routegwv4},
 				{Dst: *routev6, GW: routegwv6},
+				{Dst: *routev4, GW: routegwv4, Table: &routeTable},
 			},
 		}
 	})
@@ -201,7 +205,7 @@ var _ = Describe("ConfigureIface", func() {
 			routes, err := netlink.RouteList(link, 0)
 			Expect(err).NotTo(HaveOccurred())
 
-			var v4found, v6found bool
+			var v4found, v6found, v4Tablefound bool
 			for _, route := range routes {
 				isv4 := route.Dst.IP.To4() != nil
 				if isv4 && ipNetEqual(route.Dst, routev4) && route.Gw.Equal(ipgw4) {
@@ -218,6 +222,29 @@ var _ = Describe("ConfigureIface", func() {
 			Expect(v4found).To(BeTrue())
 			Expect(v6found).To(BeTrue())
 
+			// Need to read all tables, so cannot use RouteList
+			routeFilter := &netlink.Route{
+				Table: routeTable,
+			}
+
+			routes, err = netlink.RouteListFiltered(netlink.FAMILY_ALL,
+				routeFilter,
+				netlink.RT_FILTER_TABLE)
+			Expect(err).NotTo(HaveOccurred())
+
+			for _, route := range routes {
+				isv4 := route.Dst.IP.To4() != nil
+				if isv4 && ipNetEqual(route.Dst, routev4) && route.Gw.Equal(ipgw4) {
+					v4Tablefound = true
+				}
+
+				if v4Tablefound {
+					break
+				}
+			}
+
+			Expect(v4Tablefound).To(BeTrue())
+
 			return nil
 		})
 		Expect(err).NotTo(HaveOccurred())

plugins/meta/sbr/main.go

@@ -47,6 +47,7 @@ type PluginConf struct {
 	PrevResult    *current.Result         `json:"-"`
 
 	// Add plugin-specific flags here
+	Table *int `json:"table,omitempty"`
 }
 
 // Wrapper that does a lock before and unlock after operations to serialise
@@ -163,6 +164,9 @@ func cmdAdd(args *skel.CmdArgs) error {
 
 	// Do the actual work.
 	err = withLockAndNetNS(args.Netns, func(_ ns.NetNS) error {
+		if conf.Table != nil {
+			return doRoutesWithTable(ipCfgs, *conf.Table)
+		}
 		return doRoutes(ipCfgs, args.IfName)
 	})
 	if err != nil {
@@ -330,31 +334,73 @@ func doRoutes(ipCfgs []*current.IPConfig, iface string) error {
 	return nil
 }
 
+func doRoutesWithTable(ipCfgs []*current.IPConfig, table int) error {
+	for _, ipCfg := range ipCfgs {
+		log.Printf("Set rule for source %s", ipCfg.String())
+		rule := netlink.NewRule()
+		rule.Table = table
+
+		// Source must be restricted to a single IP, not a full subnet
+		var src net.IPNet
+		src.IP = ipCfg.Address.IP
+		if src.IP.To4() != nil {
+			src.Mask = net.CIDRMask(32, 32)
+		} else {
+			src.Mask = net.CIDRMask(128, 128)
+		}
+
+		log.Printf("Source to use %s", src.String())
+		rule.Src = &src
+
+		if err := netlink.RuleAdd(rule); err != nil {
+			return fmt.Errorf("failed to add rule: %v", err)
+		}
+	}
+
+	return nil
+}
+
 // cmdDel is called for DELETE requests
 func cmdDel(args *skel.CmdArgs) error {
 	// We care a bit about config because it sets log level.
-	_, err := parseConfig(args.StdinData)
+	conf, err := parseConfig(args.StdinData)
 	if err != nil {
 		return err
 	}
 
 	log.Printf("Cleaning up SBR for %s", args.IfName)
 	err = withLockAndNetNS(args.Netns, func(_ ns.NetNS) error {
-		return tidyRules(args.IfName)
+		return tidyRules(args.IfName, conf.Table)
 	})
 
 	return err
 }
 
 // Tidy up the rules for the deleted interface
-func tidyRules(iface string) error {
+func tidyRules(iface string, table *int) error {
 	// We keep on going on rule deletion error, but return the last failure.
 	var errReturn error
-
-	rules, err := netlink.RuleList(netlink.FAMILY_ALL)
-	if err != nil {
-		log.Printf("Failed to list all rules to tidy: %v", err)
-		return fmt.Errorf("Failed to list all rules to tidy: %v", err)
+	var err error
+	var rules []netlink.Rule
+
+	if table != nil {
+		rules, err = netlink.RuleListFiltered(
+			netlink.FAMILY_ALL,
+			&netlink.Rule{
+				Table: *table,
+			},
+			netlink.RT_FILTER_TABLE,
+		)
+		if err != nil {
+			log.Printf("Failed to list rules of table %d to tidy: %v", *table, err)
+			return fmt.Errorf("failed to list rules of table %d to tidy: %v", *table, err)
+		}
+	} else {
+		rules, err = netlink.RuleList(netlink.FAMILY_ALL)
+		if err != nil {
+			log.Printf("Failed to list all rules to tidy: %v", err)
+			return fmt.Errorf("Failed to list all rules to tidy: %v", err)
+		}
 	}
 
 	link, err := netlink.LinkByName(iface)

plugins/meta/sbr/sbr_linux_test.go

@@ -543,4 +543,81 @@ var _ = Describe("sbr test", func() {
 		_, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
 		Expect(err).To(MatchError("This plugin must be called as chained plugin"))
 	})
+
+	It("Works with Table ID", func() {
+		ifname := "net1"
+		tableID := 5000
+		conf := `{
+	"cniVersion": "0.3.0",
+	"name": "cni-plugin-sbr-test",
+	"type": "sbr",
+	"table": %d,
+	"prevResult": {
+		"cniVersion": "0.3.0",
+		"interfaces": [
+			{
+				"name": "%s",
+				"sandbox": "%s"
+			}
+		],
+		"ips": [
+			{
+				"address": "192.168.1.209/24",
+				"interface": 0
+			},
+			{
+				"address": "192.168.101.209/24",
+				"interface": 0
+			}
+		],
+		"routes": []
+	}
+}`
+		conf = fmt.Sprintf(conf, tableID, ifname, targetNs.Path())
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       targetNs.Path(),
+			IfName:      ifname,
+			StdinData:   []byte(conf),
+		}
+
+		preStatus := createDefaultStatus()
+
+		err := setup(targetNs, preStatus)
+		Expect(err).NotTo(HaveOccurred())
+
+		oldStatus, err := readback(targetNs, []string{"net1", "eth0"})
+		Expect(err).NotTo(HaveOccurred())
+
+		_, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
+		Expect(err).NotTo(HaveOccurred())
+
+		newStatus, err := readback(targetNs, []string{"net1", "eth0"})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Routes have not been moved.
+		Expect(newStatus).To(Equal(oldStatus))
+
+		// Fetch all rules for the requested table ID.
+		var rules []netlink.Rule
+		err = targetNs.Do(func(_ ns.NetNS) error {
+			var err error
+			rules, err = netlink.RuleListFiltered(
+				netlink.FAMILY_ALL, &netlink.Rule{
+					Table: tableID,
+				},
+				netlink.RT_FILTER_TABLE,
+			)
+			return err
+		})
+
+		Expect(err).NotTo(HaveOccurred())
+		Expect(rules).To(HaveLen(2))
+
+		// Both IPs have been added as source based routes with requested table ID.
+		Expect(rules[0].Table).To(Equal(tableID))
+		Expect(rules[0].Src.String()).To(Equal("192.168.101.209/32"))
+		Expect(rules[1].Table).To(Equal(tableID))
+		Expect(rules[1].Src.String()).To(Equal("192.168.1.209/32"))
+	})
 })