Skip to content

Commit

Permalink
getDefaultGatewayInterfaceByFamily: custom filter for MultiHop
Browse files Browse the repository at this point in the history
Due to the way how IPv6 multipath is implemented, it is not possible
to filter by ifindex as the ifindex for multihop routes will always be
0. Instead, implement a custom filter.

Reported-at: https://issues.redhat.com/browse/OCPBUGS-1318
Signed-off-by: Andreas Karis <ak.karis@gmail.com>
  • Loading branch information
andreaskaris committed Sep 20, 2022
1 parent 0590d6a commit 980694d
Show file tree
Hide file tree
Showing 2 changed files with 221 additions and 4 deletions.
49 changes: 45 additions & 4 deletions go-controller/pkg/node/helper_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func getDefaultGatewayInterfaceByFamily(family int, gwIface string) (string, net
// filter the default route to obtain the gateway
filter := &netlink.Route{Dst: nil}
mask := netlink.RT_FILTER_DST
gwIfIdx := 0
// gw interface provided
if len(gwIface) > 0 {
link, err := netlink.LinkByName(gwIface)
Expand All @@ -67,16 +68,15 @@ func getDefaultGatewayInterfaceByFamily(family int, gwIface string) (string, net
if link.Attrs() == nil {
return "", nil, fmt.Errorf("no attributes found for link: %#v", link)
}
gwIfIdx := link.Attrs().Index
gwIfIdx = link.Attrs().Index
klog.Infof("Provided gateway interface %q, found as index: %d", gwIface, gwIfIdx)
filter.LinkIndex = gwIfIdx
mask = mask | netlink.RT_FILTER_OIF
}

routes, err := netlink.RouteListFiltered(family, filter, mask)
routeList, err := netlink.RouteListFiltered(family, filter, mask)
if err != nil {
return "", nil, errors.Wrapf(err, "failed to get routing table in node")
}
routes := filterRoutesByIfIndex(routeList, gwIfIdx)
// use the first valid default gateway
for _, r := range routes {
// no multipath
Expand Down Expand Up @@ -148,3 +148,44 @@ func getIntfName(gatewayIntf string) (string, error) {
}
return intfName, nil
}

// filterRoutesByIfIndex is a helper function that will sieve the provided routes and check
// if they match the provided index. This used to be implemented with netlink.RT_FILTER_OIF,
// however the problem is that this filtered out MultiPath IPv6 routes which have a LinkIndex of 0.
// filterRoutesByIfIndex will return:
// a) if ifidx <= 0: routesUnfiltered
// b) else: a filtered list of routes
// The filter works as follows:
// i) return routes with LinkIndex == ifIdx
// ii) return routes with LinkIndex == 0 and where *all* MultiPaths have LinkIndex == ifIdx
// That also means: If a MultiPath route points out different interfaces, then skip that route and
// do not return it.
func filterRoutesByIfIndex(routesUnfiltered []netlink.Route, ifIdx int) []netlink.Route {
if ifIdx <= 0 {
return routesUnfiltered
}
var routes []netlink.Route
for _, r := range routesUnfiltered {
if r.LinkIndex == ifIdx ||
multipathRouteMatchesIfIndex(r, ifIdx) {
routes = append(routes, r)
}
}
return routes
}

// multipathRouteMatchesIfIndex is a helper for filterRoutesByIfIndex.
func multipathRouteMatchesIfIndex(r netlink.Route, ifIdx int) bool {
if r.LinkIndex != 0 {
return false
}
if len(r.MultiPath) == 0 {
return false
}
for _, mr := range r.MultiPath {
if mr.LinkIndex != ifIdx {
return false
}
}
return true
}
176 changes: 176 additions & 0 deletions go-controller/pkg/node/helper_linux_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package node

import (
"reflect"
"testing"

"github.com/vishvananda/netlink"
)

func TestFilterRoutesByIfIndex(t *testing.T) {
tcs := []struct {
routesUnfiltered []netlink.Route
gwIfIdx int
routesFiltered []netlink.Route
}{
// tc0 - baseline.
{
routesUnfiltered: []netlink.Route{},
gwIfIdx: 0,
routesFiltered: []netlink.Route{},
},
// tc1 - do not filter.
{
routesUnfiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
{
LinkIndex: 2,
Gw: []byte{2, 2, 2, 2},
},
{
LinkIndex: 1,
Gw: []byte{3, 3, 3, 3},
},
},
gwIfIdx: 0,
routesFiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
{
LinkIndex: 2,
Gw: []byte{2, 2, 2, 2},
},
{
LinkIndex: 1,
Gw: []byte{3, 3, 3, 3},
},
},
},
// tc2 - filter by link index.
{
routesUnfiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
{
LinkIndex: 2,
Gw: []byte{2, 2, 2, 2},
},
{
LinkIndex: 1,
Gw: []byte{3, 3, 3, 3},
},
},
gwIfIdx: 1,
routesFiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
{
LinkIndex: 1,
Gw: []byte{3, 3, 3, 3},
},
},
},
// tc3 - filter by MultiPath index.
{
routesUnfiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
{
LinkIndex: 2,
Gw: []byte{2, 2, 2, 2},
},
{
LinkIndex: 0,
MultiPath: []*netlink.NexthopInfo{
{
LinkIndex: 1,
Hops: 0,
Gw: []byte{3, 3, 3, 3},
},
{
LinkIndex: 1,
Hops: 0,
Gw: []byte{4, 4, 4, 4},
},
},
},
},
gwIfIdx: 1,
routesFiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
{
LinkIndex: 0,
MultiPath: []*netlink.NexthopInfo{
{
LinkIndex: 1,
Hops: 0,
Gw: []byte{3, 3, 3, 3},
},
{
LinkIndex: 1,
Hops: 0,
Gw: []byte{4, 4, 4, 4},
},
},
},
},
},
// tc4 - filter by MultiPath index - MultiPath with multiple interfaces.
{
routesUnfiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
{
LinkIndex: 2,
Gw: []byte{2, 2, 2, 2},
},
{
LinkIndex: 0,
MultiPath: []*netlink.NexthopInfo{
{
LinkIndex: 1,
Hops: 0,
Gw: []byte{3, 3, 3, 3},
},
{
LinkIndex: 2,
Hops: 0,
Gw: []byte{4, 4, 4, 4},
},
},
},
},
gwIfIdx: 1,
routesFiltered: []netlink.Route{
{
LinkIndex: 1,
Gw: []byte{1, 1, 1, 1},
},
},
},
}

for i, tc := range tcs {
routesFiltered := filterRoutesByIfIndex(tc.routesUnfiltered, tc.gwIfIdx)
if !reflect.DeepEqual(tc.routesFiltered, routesFiltered) {
t.Fatalf("TestFilterRoutesByIfIndex(%d): Filtering '%v' by index '%d' should have yielded '%v' but got '%v'",
i, tc.routesUnfiltered, tc.gwIfIdx, tc.routesFiltered, routesFiltered)
}
}
}

0 comments on commit 980694d

Please sign in to comment.