Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not delete IPv6 link-local route in reconciler #5483

Merged
merged 1 commit into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 37 additions & 15 deletions pkg/agent/route/route_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -277,29 +294,28 @@ 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,
},
tnqn marked this conversation as resolved.
Show resolved Hide resolved
)
}
for _, route := range gwAutoconfRoutes {
restoreRoute(route)
}
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.
Expand Down Expand Up @@ -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()) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/agent/route/route_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")})
Expand Down
33 changes: 23 additions & 10 deletions test/e2e/basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -664,26 +672,31 @@ 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 {
matches[1] = fmt.Sprintf("%s/%d", matches[1], mask)
}
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.
Expand All @@ -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
Expand Down
Loading