From 40cdb4f662e87e685a209351fec959919360c30b Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Thu, 8 Jun 2023 09:12:46 +0000 Subject: [PATCH] fix(netlink): `RouteList` list routes from all tables - Do not filter by link anymore - IPv6 detection simplified --- cmd/gluetun/main.go | 3 +- internal/netlink/ipv6.go | 39 ++++++++++------------- internal/netlink/route.go | 14 +++++--- internal/netlink/route_unspecified.go | 2 +- internal/routing/default.go | 2 +- internal/routing/local.go | 2 +- internal/routing/mocks_test.go | 8 ++--- internal/routing/routing.go | 3 +- internal/routing/vpn.go | 4 +-- internal/vpn/interfaces.go | 3 +- internal/wireguard/netlinker.go | 3 +- internal/wireguard/netlinker_mock_test.go | 8 ++--- 12 files changed, 43 insertions(+), 48 deletions(-) diff --git a/cmd/gluetun/main.go b/cmd/gluetun/main.go index 54103160..bfc12a8a 100644 --- a/cmd/gluetun/main.go +++ b/cmd/gluetun/main.go @@ -535,8 +535,7 @@ type Addresser interface { } type Router interface { - RouteList(link *netlink.Link, family int) ( - routes []netlink.Route, err error) + RouteList(family int) (routes []netlink.Route, err error) RouteAdd(route netlink.Route) error RouteDel(route netlink.Route) error RouteReplace(route netlink.Route) error diff --git a/internal/netlink/ipv6.go b/internal/netlink/ipv6.go index 4af1a55d..07c51aff 100644 --- a/internal/netlink/ipv6.go +++ b/internal/netlink/ipv6.go @@ -5,35 +5,28 @@ import ( ) func (n *NetLink) IsIPv6Supported() (supported bool, err error) { - links, err := n.LinkList() + routes, err := n.RouteList(FamilyV6) if err != nil { - return false, fmt.Errorf("listing links: %w", err) + return false, fmt.Errorf("listing IPv6 routes: %w", err) } - var totalRoutes uint - for _, link := range links { - link := link - routes, err := n.RouteList(&link, FamilyV6) - if err != nil { - return false, fmt.Errorf("listing IPv6 routes for link %s: %w", - link.Name, err) - } - - // Check each route for IPv6 due to Podman bug listing IPv4 routes - // as IPv6 routes at container start, see: - // https://github.com/qdm12/gluetun/issues/1241#issuecomment-1333405949 - for _, route := range routes { - sourceIsIPv6 := route.Src.IsValid() && route.Src.Is6() - destinationIsIPv6 := route.Dst.IsValid() && route.Dst.Addr().Is6() - if sourceIsIPv6 || destinationIsIPv6 { - n.debugLogger.Debugf("IPv6 is supported by link %s", link.Name) - return true, nil + // Check each route for IPv6 due to Podman bug listing IPv4 routes + // as IPv6 routes at container start, see: + // https://github.com/qdm12/gluetun/issues/1241#issuecomment-1333405949 + for _, route := range routes { + sourceIsIPv6 := route.Src.IsValid() && route.Src.Is6() + destinationIsIPv6 := route.Dst.IsValid() && route.Dst.Addr().Is6() + if sourceIsIPv6 || destinationIsIPv6 { + link, err := n.LinkByIndex(route.LinkIndex) + if err != nil { + return false, fmt.Errorf("finding IPv6 supported link: %w", err) } - totalRoutes++ + n.debugLogger.Debugf("IPv6 is supported by link %s", link.Name) + return true, nil } } - n.debugLogger.Debugf("IPv6 is not supported after searching %d links and %d routes", - len(links), totalRoutes) + n.debugLogger.Debugf("IPv6 is not supported after searching %d routes", + len(routes)) return false, nil } diff --git a/internal/netlink/route.go b/internal/netlink/route.go index e7153412..8016b09b 100644 --- a/internal/netlink/route.go +++ b/internal/netlink/route.go @@ -6,10 +6,16 @@ import ( "github.com/vishvananda/netlink" ) -func (n *NetLink) RouteList(link *Link, family int) ( - routes []Route, err error) { - netlinkLink := linkToNetlinkLink(link) - netlinkRoutes, err := netlink.RouteList(netlinkLink, family) +func (n *NetLink) RouteList(family int) (routes []Route, err error) { + // We set the filter to netlink.RT_FILTER_TABLE so that + // routes from all tables are listed, as long as the filter + // table is set to 0. + const filterMask = netlink.RT_FILTER_TABLE + // The filter is not left to `nil` otherwise non-main tables + // are ignored. + filter := &netlink.Route{} + + netlinkRoutes, err := netlink.RouteListFiltered(family, filter, filterMask) if err != nil { return nil, err } diff --git a/internal/netlink/route_unspecified.go b/internal/netlink/route_unspecified.go index 8696dcb2..14816b01 100644 --- a/internal/netlink/route_unspecified.go +++ b/internal/netlink/route_unspecified.go @@ -2,7 +2,7 @@ package netlink -func (n *NetLink) RouteList(link *Link, family int) ( +func (n *NetLink) RouteList(family int) ( routes []Route, err error) { panic("not implemented") } diff --git a/internal/routing/default.go b/internal/routing/default.go index d550ec7d..8ae77001 100644 --- a/internal/routing/default.go +++ b/internal/routing/default.go @@ -25,7 +25,7 @@ func (d DefaultRoute) String() string { } func (r *Routing) DefaultRoutes() (defaultRoutes []DefaultRoute, err error) { - routes, err := r.netLinker.RouteList(nil, netlink.FamilyAll) + routes, err := r.netLinker.RouteList(netlink.FamilyAll) if err != nil { return nil, fmt.Errorf("listing routes: %w", err) } diff --git a/internal/routing/local.go b/internal/routing/local.go index 075ca2a7..b9a5876c 100644 --- a/internal/routing/local.go +++ b/internal/routing/local.go @@ -41,7 +41,7 @@ func (r *Routing) LocalNetworks() (localNetworks []LocalNetwork, err error) { return localNetworks, fmt.Errorf("%w: in %d links", ErrLinkLocalNotFound, len(links)) } - routes, err := r.netLinker.RouteList(nil, netlink.FamilyAll) + routes, err := r.netLinker.RouteList(netlink.FamilyAll) if err != nil { return localNetworks, fmt.Errorf("listing routes: %w", err) } diff --git a/internal/routing/mocks_test.go b/internal/routing/mocks_test.go index 90f41438..534ea9b0 100644 --- a/internal/routing/mocks_test.go +++ b/internal/routing/mocks_test.go @@ -210,18 +210,18 @@ func (mr *MockNetLinkerMockRecorder) RouteDel(arg0 interface{}) *gomock.Call { } // RouteList mocks base method. -func (m *MockNetLinker) RouteList(arg0 *netlink.Link, arg1 int) ([]netlink.Route, error) { +func (m *MockNetLinker) RouteList(arg0 int) ([]netlink.Route, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RouteList", arg0, arg1) + ret := m.ctrl.Call(m, "RouteList", arg0) ret0, _ := ret[0].([]netlink.Route) ret1, _ := ret[1].(error) return ret0, ret1 } // RouteList indicates an expected call of RouteList. -func (mr *MockNetLinkerMockRecorder) RouteList(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockNetLinkerMockRecorder) RouteList(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RouteList", reflect.TypeOf((*MockNetLinker)(nil).RouteList), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RouteList", reflect.TypeOf((*MockNetLinker)(nil).RouteList), arg0) } // RouteReplace mocks base method. diff --git a/internal/routing/routing.go b/internal/routing/routing.go index 03eb313d..d7e6a1d9 100644 --- a/internal/routing/routing.go +++ b/internal/routing/routing.go @@ -22,8 +22,7 @@ type Addresser interface { } type Router interface { - RouteList(link *netlink.Link, family int) ( - routes []netlink.Route, err error) + RouteList(family int) (routes []netlink.Route, err error) RouteAdd(route netlink.Route) error RouteDel(route netlink.Route) error RouteReplace(route netlink.Route) error diff --git a/internal/routing/vpn.go b/internal/routing/vpn.go index 86fdff5f..8432c852 100644 --- a/internal/routing/vpn.go +++ b/internal/routing/vpn.go @@ -14,7 +14,7 @@ var ( ) func (r *Routing) VPNDestinationIP() (ip netip.Addr, err error) { - routes, err := r.netLinker.RouteList(nil, netlink.FamilyAll) + routes, err := r.netLinker.RouteList(netlink.FamilyAll) if err != nil { return ip, fmt.Errorf("listing routes: %w", err) } @@ -42,7 +42,7 @@ func (r *Routing) VPNDestinationIP() (ip netip.Addr, err error) { } func (r *Routing) VPNLocalGatewayIP(vpnIntf string) (ip netip.Addr, err error) { - routes, err := r.netLinker.RouteList(nil, netlink.FamilyAll) + routes, err := r.netLinker.RouteList(netlink.FamilyAll) if err != nil { return ip, fmt.Errorf("listing routes: %w", err) } diff --git a/internal/vpn/interfaces.go b/internal/vpn/interfaces.go index d52e2297..3d05aac2 100644 --- a/internal/vpn/interfaces.go +++ b/internal/vpn/interfaces.go @@ -50,8 +50,7 @@ type NetLinker interface { } type Router interface { - RouteList(link *netlink.Link, family int) ( - routes []netlink.Route, err error) + RouteList(family int) (routes []netlink.Route, err error) RouteAdd(route netlink.Route) error } diff --git a/internal/wireguard/netlinker.go b/internal/wireguard/netlinker.go index 02070015..6b077016 100644 --- a/internal/wireguard/netlinker.go +++ b/internal/wireguard/netlinker.go @@ -13,8 +13,7 @@ type NetLinker interface { } type Router interface { - RouteList(link *netlink.Link, family int) ( - routes []netlink.Route, err error) + RouteList(family int) (routes []netlink.Route, err error) RouteAdd(route netlink.Route) error } diff --git a/internal/wireguard/netlinker_mock_test.go b/internal/wireguard/netlinker_mock_test.go index 4e8199a4..124a659b 100644 --- a/internal/wireguard/netlinker_mock_test.go +++ b/internal/wireguard/netlinker_mock_test.go @@ -166,18 +166,18 @@ func (mr *MockNetLinkerMockRecorder) RouteAdd(arg0 interface{}) *gomock.Call { } // RouteList mocks base method. -func (m *MockNetLinker) RouteList(arg0 *netlink.Link, arg1 int) ([]netlink.Route, error) { +func (m *MockNetLinker) RouteList(arg0 int) ([]netlink.Route, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RouteList", arg0, arg1) + ret := m.ctrl.Call(m, "RouteList", arg0) ret0, _ := ret[0].([]netlink.Route) ret1, _ := ret[1].(error) return ret0, ret1 } // RouteList indicates an expected call of RouteList. -func (mr *MockNetLinkerMockRecorder) RouteList(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockNetLinkerMockRecorder) RouteList(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RouteList", reflect.TypeOf((*MockNetLinker)(nil).RouteList), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RouteList", reflect.TypeOf((*MockNetLinker)(nil).RouteList), arg0) } // RuleAdd mocks base method.