From f23830a05c266cadfb4dadb28b757b6915f8f7e5 Mon Sep 17 00:00:00 2001 From: wenyingd Date: Wed, 13 Sep 2023 17:09:54 +0800 Subject: [PATCH] Do not delete IPv6 link-local route in reconciler In the existing code, the IPv6 link-local route on antrea-gw0 is deleted in route reconcile, which results in the IPv6 Neighbor Solicitation sent from Pod's link-local address is dropped on the Node by kenel reverse path filtering, and Pod would mark the antrea-gw0 as a "FAILED" neighbor. Then the Pod's accross Node traffic or the Pod-to-external traffic does not work as expected. This change includes, 1. Do not delete IPv6 link-local routes in the reconcile function, 2. Restore IPv6 link-local route in syncRoute function. Signed-off-by: wenyingd --- pkg/agent/route/route_linux.go | 52 ++++++++++++++++++++--------- pkg/agent/route/route_linux_test.go | 6 ++++ test/e2e/basic_test.go | 33 ++++++++++++------ 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/pkg/agent/route/route_linux.go b/pkg/agent/route/route_linux.go index f72ccf3bbe9..e4c6c4dfcec 100644 --- a/pkg/agent/route/route_linux.go +++ b/pkg/agent/route/route_linux.go @@ -86,6 +86,10 @@ var ( // globalVMAC is used in the IPv6 neighbor configuration to advertise ND solicitation for the IPv6 address of the // host gateway interface on other Nodes. globalVMAC, _ = net.ParseMAC("aa:bb:cc:dd:ee:ff") + + // The system auto-generated IPv6 link-local route always uses "fe80::/64" as the destination regardless of the + // interface's global address's mask. + _, llrCIDR, _ = net.ParseCIDR("fe80::/64") ) // Client takes care of routing container packets in host network, coordinating ip route, ip rule, iptables and ipset. @@ -225,22 +229,35 @@ func (c *Client) syncIPInfra() { klog.V(3).Info("Successfully synced iptables, ipset and route") } +type routeKey struct { + linkIndex int + dst string + gw string +} + func (c *Client) syncRoute() error { routeList, err := c.netlink.RouteList(nil, netlink.FAMILY_ALL) if err != nil { return err } - routeMap := make(map[string]*netlink.Route) + routeKeys := sets.New[routeKey]() for i := range routeList { r := &routeList[i] if r.Dst == nil { continue } - routeMap[r.Dst.String()] = r + routeKeys.Insert(routeKey{ + linkIndex: r.LinkIndex, + dst: r.Dst.String(), + gw: r.Gw.String(), + }) } restoreRoute := func(route *netlink.Route) bool { - r, ok := routeMap[route.Dst.String()] - if ok && routeEqual(route, r) { + if routeKeys.Has(routeKey{ + linkIndex: route.LinkIndex, + dst: route.Dst.String(), + gw: route.Gw.String(), + }) { return true } if err := c.netlink.RouteReplace(route); err != nil { @@ -277,12 +294,21 @@ func (c *Client) syncRoute() error { }) } if c.nodeConfig.PodIPv6CIDR != nil { + // Here we assume the IPv6 link-local address always exists on antrea-gw0 + // to avoid unexpected issues in the IPv6 forwarding. gwAutoconfRoutes = append(gwAutoconfRoutes, &netlink.Route{ LinkIndex: c.nodeConfig.GatewayConfig.LinkIndex, Dst: c.nodeConfig.PodIPv6CIDR, Src: c.nodeConfig.GatewayConfig.IPv6, Scope: netlink.SCOPE_LINK, - }) + }, + // Restore the IPv6 link-local route. + &netlink.Route{ + LinkIndex: c.nodeConfig.GatewayConfig.LinkIndex, + Dst: llrCIDR, + Scope: netlink.SCOPE_LINK, + }, + ) } for _, route := range gwAutoconfRoutes { restoreRoute(route) @@ -290,16 +316,6 @@ func (c *Client) syncRoute() error { return nil } -func routeEqual(x, y *netlink.Route) bool { - if x == nil || y == nil { - return false - } - return x.LinkIndex == y.LinkIndex && - x.Dst.IP.Equal(y.Dst.IP) && - bytes.Equal(x.Dst.Mask, y.Dst.Mask) && - x.Gw.Equal(y.Gw) -} - // syncIPSet ensures that the required ipset exists and it has the initial members. func (c *Client) syncIPSet() error { // In policy-only mode, Node Pod CIDR is undefined. @@ -891,6 +907,12 @@ func (c *Client) Reconcile(podCIDRs []string) error { if desiredPodCIDRs.Has(route.Dst.String()) { continue } + // The route to the IPv6 link-local CIDR is always auto-generated by the system along with + // a link-local address, which is not configured by Antrea and should therefore to be ignored + // in the "deletion" list. Such routes are useful in some cases, e.g., IPv6 NDP. + if route.Dst.IP.IsLinkLocalUnicast() && route.Dst.IP.To4() == nil { + continue + } // IPv6 doesn't support "on-link" route, routes to the peer IPv6 gateways need to // be added separately. So don't delete such routes. if desiredIPv6GWs.Has(route.Dst.IP.String()) { diff --git a/pkg/agent/route/route_linux_test.go b/pkg/agent/route/route_linux_test.go index 9b675304025..be179f22036 100644 --- a/pkg/agent/route/route_linux_test.go +++ b/pkg/agent/route/route_linux_test.go @@ -74,6 +74,11 @@ func TestSyncRoutes(t *testing.T) { Src: net.ParseIP("aabb:ccdd::1"), Scope: netlink.SCOPE_LINK, }) + mockNetlink.EXPECT().RouteReplace(&netlink.Route{ + LinkIndex: 10, + Dst: ip.MustParseCIDR("fe80::/64"), + Scope: netlink.SCOPE_LINK, + }) c := &Client{ netlink: mockNetlink, @@ -676,6 +681,7 @@ func TestReconcile(t *testing.T) { {Dst: ip.MustParseCIDR("2001:ab03:cd04:55ee:1001::1/128")}, // existing podCIDR, should not be deleted. {Dst: ip.MustParseCIDR("fc01::aabb:ccdd:eeff/128")}, // service route, should not be deleted. {Dst: ip.MustParseCIDR("2001:ab03:cd04:55ee:100b::/80")}, // non-existing podCIDR, should be deleted. + {Dst: ip.MustParseCIDR("fe80::/80")}, // link-local route, should not be deleted. }, nil) mockNetlink.EXPECT().RouteDel(&netlink.Route{Dst: ip.MustParseCIDR("192.168.11.0/24")}) mockNetlink.EXPECT().RouteDel(&netlink.Route{Dst: ip.MustParseCIDR("2001:ab03:cd04:55ee:100b::/80")}) diff --git a/test/e2e/basic_test.go b/test/e2e/basic_test.go index 40d91498b51..87adb8878a2 100644 --- a/test/e2e/basic_test.go +++ b/test/e2e/basic_test.go @@ -380,7 +380,8 @@ func testReconcileGatewayRoutesOnStartup(t *testing.T, data *TestData, isIPv6 bo t.Logf("Retrieving gateway routes on Node '%s'", nodeName) var routes []Route if err := wait.PollImmediate(defaultInterval, defaultTimeout, func() (found bool, err error) { - routes, _, err = getGatewayRoutes(t, data, antreaGWName, nodeName, isIPv6) + var llRoute *Route + routes, _, llRoute, err = getGatewayRoutes(t, data, antreaGWName, nodeName, isIPv6) if err != nil { return false, err } @@ -391,6 +392,9 @@ func testReconcileGatewayRoutesOnStartup(t *testing.T, data *TestData, isIPv6 bo } else if len(routes) > expectedRtNumMax { return false, fmt.Errorf("found too many gateway routes, expected %d but got %d", expectedRtNumMax, len(routes)) } + if isIPv6 && llRoute == nil { + return false, fmt.Errorf("IPv6 link-local route not found") + } return true, nil }); err == wait.ErrWaitTimeout { t.Fatalf("Not enough gateway routes after %v", defaultTimeout) @@ -410,8 +414,8 @@ func testReconcileGatewayRoutesOnStartup(t *testing.T, data *TestData, isIPv6 bo _, routeToAdd.routeCIDR, _ = net.ParseCIDR("99.99.99.0/24") routeToAdd.routeGW = net.ParseIP("99.99.99.1") } else { - _, routeToAdd.routeCIDR, _ = net.ParseCIDR("fe80::0/112") - routeToAdd.routeGW = net.ParseIP("fe80::1") + _, routeToAdd.routeCIDR, _ = net.ParseCIDR("fa80::/112") + routeToAdd.routeGW = net.ParseIP("fa80::1") } // We run the ip command from the antrea-agent container for delete / add since they need to @@ -474,10 +478,14 @@ func testReconcileGatewayRoutesOnStartup(t *testing.T, data *TestData, isIPv6 bo // We expect the agent to delete the extra route we added and add back the route we deleted t.Logf("Waiting for gateway routes to converge") if err := wait.Poll(defaultInterval, defaultTimeout, func() (bool, error) { - newRoutes, _, err := getGatewayRoutes(t, data, antreaGWName, nodeName, isIPv6) + var llRoute *Route + newRoutes, _, llRoute, err := getGatewayRoutes(t, data, antreaGWName, nodeName, isIPv6) if err != nil { return false, err } + if isIPv6 && llRoute == nil { + return false, fmt.Errorf("IPv6 link-local route not found") + } if len(newRoutes) != len(routes) { return false, nil } @@ -559,7 +567,7 @@ func testCleanStaleClusterIPRoutes(t *testing.T, data *TestData, isIPv6 bool) { } var routes []Route if err := wait.PollImmediate(defaultInterval, defaultTimeout, func() (bool, error) { - _, routes, err = getGatewayRoutes(t, data, antreaGWName, nodeName, isIPv6) + _, routes, _, err = getGatewayRoutes(t, data, antreaGWName, nodeName, isIPv6) if err != nil { t.Logf("Failed to get Service gateway routes: %v", err) return false, nil @@ -650,7 +658,7 @@ type Route struct { routeGW net.IP } -func getGatewayRoutes(t *testing.T, data *TestData, antreaGWName, nodeName string, isIPv6 bool) ([]Route, []Route, error) { +func getGatewayRoutes(t *testing.T, data *TestData, antreaGWName, nodeName string, isIPv6 bool) ([]Route, []Route, *Route, error) { var cmd []string virtualIP := config.VirtualServiceIPv4 mask := 32 @@ -664,15 +672,20 @@ func getGatewayRoutes(t *testing.T, data *TestData, antreaGWName, nodeName strin podName := getAntreaPodName(t, data, nodeName) stdout, stderr, err := data.RunCommandFromPod(antreaNamespace, podName, agentContainerName, cmd) if err != nil { - return nil, nil, fmt.Errorf("error when running ip command in Pod '%s': %v - stdout: %s - stderr: %s", podName, err, stdout, stderr) + return nil, nil, nil, fmt.Errorf("error when running ip command in Pod '%s': %v - stdout: %s - stderr: %s", podName, err, stdout, stderr) } var nodeRoutes, serviceRoutes []Route + var llRoute *Route re := regexp.MustCompile(`([^\s]+) via ([^\s]+)`) for _, line := range strings.Split(stdout, "\n") { var err error matches := re.FindStringSubmatch(line) if len(matches) == 0 { + if isIPv6 && strings.HasPrefix(line, "fe80::") { + llRoute = &Route{} + _, llRoute.routeCIDR, _ = net.ParseCIDR(strings.Split(line, " ")[0]) + } continue } if net.ParseIP(matches[1]) != nil { @@ -680,10 +693,10 @@ func getGatewayRoutes(t *testing.T, data *TestData, antreaGWName, nodeName strin } route := Route{} if _, route.routeCIDR, err = net.ParseCIDR(matches[1]); err != nil { - return nil, nil, fmt.Errorf("%s is not a valid net CIDR", matches[1]) + return nil, nil, nil, fmt.Errorf("%s is not a valid net CIDR", matches[1]) } if route.routeGW = net.ParseIP(matches[2]); route.routeGW == nil { - return nil, nil, fmt.Errorf("%s is not a valid IP", matches[2]) + return nil, nil, nil, fmt.Errorf("%s is not a valid IP", matches[2]) } if route.routeGW.Equal(virtualIP) { // If the route is added by AntreaProxy, append it to slice serviceRoutes. @@ -693,7 +706,7 @@ func getGatewayRoutes(t *testing.T, data *TestData, antreaGWName, nodeName strin nodeRoutes = append(nodeRoutes, route) } } - return nodeRoutes, serviceRoutes, nil + return nodeRoutes, serviceRoutes, llRoute, nil } // testDeletePreviousRoundFlowsOnStartup checks that when the Antrea agent is restarted, flows from