diff --git a/plugins/meta/sbr/main.go b/plugins/meta/sbr/main.go index affaf835..7929952a 100644 --- a/plugins/meta/sbr/main.go +++ b/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. -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 := firstTableID +// getNextTableID picks the first free table id from a giveen candidate id +func getNextTableID(rules []netlink.Rule, routes []netlink.Route, candidateID int) int { + table := candidateID 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 diff --git a/plugins/meta/sbr/sbr_linux_test.go b/plugins/meta/sbr/sbr_linux_test.go index 967bcbb1..3327609f 100644 --- a/plugins/meta/sbr/sbr_linux_test.go +++ b/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 := `{