[sbr]: Use different tableID for every ipCfg

Check tableID not in use for every ipCfg

       This allows SBR plugin to accommodate for multi-ip interfaces

       Fixes #581

Signed-off-by: Anurag Dwivedi <anuragensemble1@gmail.com>

plugins/meta/sbr/main.go

@@ -176,23 +176,9 @@ func cmdAdd(args *skel.CmdArgs) error {
 	return types.PrintResult(conf.PrevResult, conf.CNIVersion)
 }
 
-// doRoutes does all the work to set up routes and rules during an add.
+// getNextTableID picks the first free table id from a giveen candidate id
-func doRoutes(ipCfgs []*current.IPConfig, origRoutes []*types.Route, iface string) error {
+func getNextTableID(rules []netlink.Rule, routes []netlink.Route, candidateID int) int {
-	// Get a list of rules and routes ready.
+	table := candidateID
-	rules, err := netlink.RuleList(netlink.FAMILY_ALL)
-	if err != nil {
-		return fmt.Errorf("Failed to list all rules: %v", err)
-	}
-
-	routes, err := netlink.RouteList(nil, netlink.FAMILY_ALL)
-	if err != nil {
-		return fmt.Errorf("Failed to list all routes: %v", err)
-	}
-
-	// Pick a table ID to use. We pick the first table ID from firstTableID
-	// on that has no existing rules mapping to it and no existing routes in
-	// it.
-	table := firstTableID
 	for {
 		foundExisting := false
 		for _, rule := range rules {
@@ -215,7 +201,26 @@ func doRoutes(ipCfgs []*current.IPConfig, origRoutes []*types.Route, iface strin
 			break
 		}
 	}
+	return table
+}
 
+// doRoutes does all the work to set up routes and rules during an add.
+func doRoutes(ipCfgs []*current.IPConfig, origRoutes []*types.Route, iface string) error {
+	// Get a list of rules and routes ready.
+	rules, err := netlink.RuleList(netlink.FAMILY_ALL)
+	if err != nil {
+		return fmt.Errorf("Failed to list all rules: %v", err)
+	}
+
+	routes, err := netlink.RouteList(nil, netlink.FAMILY_ALL)
+	if err != nil {
+		return fmt.Errorf("Failed to list all routes: %v", err)
+	}
+
+	// Pick a table ID to use. We pick the first table ID from firstTableID
+	// on that has no existing rules mapping to it and no existing routes in
+	// it.
+	table := getNextTableID(rules, routes, firstTableID)
 	log.Printf("First unreferenced table: %d", table)
 
 	link, err := netlink.LinkByName(iface)
@@ -287,17 +292,17 @@ func doRoutes(ipCfgs []*current.IPConfig, origRoutes []*types.Route, iface strin
 		// to the table we want them in.
 		for _, r := range routes {
 			if ipCfg.Address.Contains(r.Src) || ipCfg.Address.Contains(r.Gw) ||
-					(r.Src == nil && r.Gw == nil) {
+				(r.Src == nil && r.Gw == nil) {
 				// (r.Src == nil && r.Gw == nil) is inferred as a generic route
 				log.Printf("Copying route %s from table %d to %d",
 					r.String(), r.Table, table)
 
 				r.Table = table
-		
+
 				// Reset the route flags since if it is dynamically created,
 				// adding it to the new table will fail with "invalid argument"
 				r.Flags = 0
-		
+
 				// We use route replace in case the route already exists, which
 				// is possible for the default gateway we added above.
 				err = netlink.RouteReplace(&r)
@@ -309,8 +314,9 @@ func doRoutes(ipCfgs []*current.IPConfig, origRoutes []*types.Route, iface strin
 
 		// Use a different table for each ipCfg
 		table++
+		table = getNextTableID(rules, routes, table)
 	}
-	
+
 	// Delete all the interface routes in the default routing table, which were
 	// copied to source based routing tables.
 	// Not deleting them while copying to accommodate for multiple ipCfgs from

plugins/meta/sbr/sbr_linux_test.go

@@ -439,50 +439,50 @@ var _ = Describe("sbr test", func() {
 			IfName:      ifname,
 			StdinData:   []byte(conf),
 		}
-		
+
 		preStatus := createDefaultStatus()
-		
+
 		// Add the second IP on net1 interface
 		preStatus.Devices[1].Addrs = append(preStatus.Devices[1].Addrs,
 			net.IPNet{
-				IP: net.IPv4(192, 168, 101, 209),
+				IP:   net.IPv4(192, 168, 101, 209),
 				Mask: net.IPv4Mask(255, 255, 255, 0),
 			})
-		
+
 		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())
-		
+
 		// Check results. We expect all the routes on net1 to have moved to
 		// table 100 except for local routes (table 255); a new default gateway
 		// route to have been created; and 2 rules to exist.
 		expNet1 := oldStatus.Devices[0]
 		expEth0 := oldStatus.Devices[1]
-		
+
 		for i := range expNet1.Routes {
 			if expNet1.Routes[i].Table != 255 {
 				if expNet1.Routes[i].Src.String() == "192.168.101.209" {
 					// All 192.168.101.x routes expected in table 101
 					expNet1.Routes[i].Table = 101
-				} else if (expNet1.Routes[i].Src == nil && expNet1.Routes[i].Gw == nil) {
+				} else if expNet1.Routes[i].Src == nil && expNet1.Routes[i].Gw == nil {
 					// Generic Link Local Addresses assigned. They should exist in all
 					// route tables
 					expNet1.Routes[i].Table = 100
 					expNet1.Routes = append(expNet1.Routes,
 						netlink.Route{
-							Dst: expNet1.Routes[i].Dst,
+							Dst:       expNet1.Routes[i].Dst,
 							Table:     101,
 							LinkIndex: expNet1.Routes[i].LinkIndex})
 				} else {
-					// All 192.168.1.x routes expected in tabele 100
+					// All 192.168.1.x routes expected in table 100
 					expNet1.Routes[i].Table = 100
 				}
 			}
@@ -492,30 +492,30 @@ var _ = Describe("sbr test", func() {
 				Gw:        net.IPv4(192, 168, 1, 1),
 				Table:     100,
 				LinkIndex: expNet1.Routes[0].LinkIndex})
-		
+
 		expNet1.Routes = append(expNet1.Routes,
 			netlink.Route{
 				Gw:        net.IPv4(192, 168, 101, 1),
 				Table:     101,
 				LinkIndex: expNet1.Routes[0].LinkIndex})
-		
+
-		// 2 Rules will ve created for each IP address. (100, 101)
+		// 2 Rules will be created for each IP address. (100, 101)
 		Expect(len(newStatus.Rules)).To(Equal(2))
-				
+
 		// First entry corresponds to last table
 		Expect(newStatus.Rules[0].Table).To(Equal(101))
 		Expect(newStatus.Rules[0].Src.String()).To(Equal("192.168.101.209/32"))
-		
+
 		// Second entry corresponds to first table (100)
 		Expect(newStatus.Rules[1].Table).To(Equal(100))
 		Expect(newStatus.Rules[1].Src.String()).To(Equal("192.168.1.209/32"))
-		
+
 		devNet1 := newStatus.Devices[0]
 		devEth0 := newStatus.Devices[1]
 		Expect(equalRoutes(expNet1.Routes, devNet1.Routes)).To(BeTrue())
 		Expect(equalRoutes(expEth0.Routes, devEth0.Routes)).To(BeTrue())
-		
+
-	})	
+	})
 
 	It("fails with CNI spec versions that don't support plugin chaining", func() {
 		conf := `{