From e148de825d14649b29462a28f204380449237d60 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Wed, 25 Sep 2024 14:15:08 +0200 Subject: [PATCH 1/3] add route to local table for non loopback devices Signed-off-by: Lukas Hoehl --- internal/netif/netif.go | 56 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/internal/netif/netif.go b/internal/netif/netif.go index e35b3700..a178ab6c 100644 --- a/internal/netif/netif.go +++ b/internal/netif/netif.go @@ -12,7 +12,19 @@ import ( "k8s.io/klog/v2" ) +// copied from golang.org/x/sys/unix constants +const ( + // copied from RT_TABLE_LOCAL + // Also visible in /etc/iproute2/rt_tables file on linux hosts + localRoutingTableId int = 0xff + // copied from RT_SCOPE_HOST + // Also visible in /etc/iproute2/rt_scopes file on linux hosts + hostScopeId int = 0xfe +) + type Handle interface { + RouteAdd(route *netlink.Route) error + RouteDel(route *netlink.Route) error AddrAdd(link netlink.Link, addr *netlink.Addr) error AddrDel(link netlink.Link, addr *netlink.Addr) error LinkByName(name string) (netlink.Link, error) @@ -81,14 +93,28 @@ func (m *netifManagerDefault) EnsureIPAddress() error { klog.V(6).Infof("Got interface %+v", l) if err := m.AddrAdd(l, m.addr); err != nil { - if os.IsExist(err) { - klog.V(4).Infof("Address %q already exists. Skipping", m.addr.String()) - return nil + if !os.IsExist(err) { + return xerrors.Errorf("could not add IPV4 addresses %v", err) } - - return xerrors.Errorf("could not add IPV4 addresses %v", err) + klog.V(4).Infof("Address %q already exists.", m.addr.String()) } + // loopback device adds new ip addresses to the local routing table by default. + // If we are using a different interface, we need to do the updates ourself + if l.Attrs().Name != "lo" { + route := &netlink.Route{ + LinkIndex: l.Attrs().Index, + Table: localRoutingTableId, + Dst: m.addr.IPNet, + Scope: netlink.Scope(hostScopeId), + } + if err = m.RouteAdd(route); err != nil { + if !os.IsExist(err) { + return xerrors.Errorf("could not add route for %s to interface %s:\n%v", m.addr, m.devName, err) + } + klog.V(4).Infof("Route for %q already exists", m.addr.String()) + } + } klog.Infof("Successfully added %q to %q", m.addr.String(), m.devName) return nil @@ -105,6 +131,16 @@ func (m *netifManagerDefault) RemoveIPAddress() error { klog.V(6).Infof("Got interface %+v", l) + if m.devName != "lo" { + route := localRoute(l.Attrs().Index, m.addr) + if err := m.RouteDel(route); err != nil { + if !os.IsNotExist(err) { + return xerrors.Errorf("could not remove route for %s from interface %s:\n%v", m.addr, m.devName, err) + } + klog.V(4).Infof("Route for %q already removed. Skipping", m.addr.String()) + } + } + if err := m.AddrDel(l, m.addr); err != nil { if os.IsNotExist(err) { klog.V(4).Infof("Address %q already removed. Skipping", m.addr.String()) @@ -119,6 +155,16 @@ func (m *netifManagerDefault) RemoveIPAddress() error { return nil } +func localRoute(linkIndex int, addr *netlink.Addr) *netlink.Route { + return &netlink.Route{ + LinkIndex: linkIndex, + Table: localRoutingTableId, + Dst: addr.IPNet, + Src: addr.IP, + Scope: netlink.Scope(hostScopeId), + } +} + func (m *netifManagerDefault) CleanupDevice() error { link, err := netlink.LinkByName(m.devName) if err != nil { From b19fff0faf74b92518e8e753da9eefbbc0524dad Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Wed, 25 Sep 2024 14:15:11 +0200 Subject: [PATCH 2/3] tests Signed-off-by: Lukas Hoehl --- internal/netif/mocks_test.go | 42 ++++++++++++++++++++++++++++++ internal/netif/netif_suite_test.go | 29 +++++++++++++++++++-- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/internal/netif/mocks_test.go b/internal/netif/mocks_test.go index 4bedade9..5861e40a 100644 --- a/internal/netif/mocks_test.go +++ b/internal/netif/mocks_test.go @@ -124,6 +124,34 @@ func (mr *MockHandleMockRecorder) LinkSetUp(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LinkSetUp", reflect.TypeOf((*MockHandle)(nil).LinkSetUp), arg0) } +// RouteAdd mocks base method. +func (m *MockHandle) RouteAdd(route *netlink.Route) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RouteAdd", route) + ret0, _ := ret[0].(error) + return ret0 +} + +// RouteAdd indicates an expected call of RouteAdd. +func (mr *MockHandleMockRecorder) RouteAdd(route any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RouteAdd", reflect.TypeOf((*MockHandle)(nil).RouteAdd), route) +} + +// RouteDel mocks base method. +func (m *MockHandle) RouteDel(route *netlink.Route) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RouteDel", route) + ret0, _ := ret[0].(error) + return ret0 +} + +// RouteDel indicates an expected call of RouteDel. +func (mr *MockHandleMockRecorder) RouteDel(route any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RouteDel", reflect.TypeOf((*MockHandle)(nil).RouteDel), route) +} + // MockManager is a mock of Manager interface. type MockManager struct { ctrl *gomock.Controller @@ -147,6 +175,20 @@ func (m *MockManager) EXPECT() *MockManagerMockRecorder { return m.recorder } +// CleanupDevice mocks base method. +func (m *MockManager) CleanupDevice() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CleanupDevice") + ret0, _ := ret[0].(error) + return ret0 +} + +// CleanupDevice indicates an expected call of CleanupDevice. +func (mr *MockManagerMockRecorder) CleanupDevice() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanupDevice", reflect.TypeOf((*MockManager)(nil).CleanupDevice)) +} + // EnsureIPAddress mocks base method. func (m *MockManager) EnsureIPAddress() error { m.ctrl.T.Helper() diff --git a/internal/netif/netif_suite_test.go b/internal/netif/netif_suite_test.go index f6d52366..74fb772e 100644 --- a/internal/netif/netif_suite_test.go +++ b/internal/netif/netif_suite_test.go @@ -106,6 +106,11 @@ var _ = Describe("Manager", func() { }) It("should return error when deleting ip address", func() { + mh.EXPECT(). + RouteDel(gomock.Any()). + Return(nil). + Times(1) + mh.EXPECT(). AddrDel(gomock.Eq(dummy), gomock.Eq(addr)). Return(fmt.Errorf("err")). @@ -116,6 +121,10 @@ var _ = Describe("Manager", func() { }) It("should return already removed error", func() { + mh.EXPECT(). + RouteDel(gomock.Any()). + Return(nil). + Times(1) mh.EXPECT(). AddrDel(gomock.Eq(dummy), gomock.Eq(addr)). // Return(syscall.EEXIST). @@ -132,6 +141,11 @@ var _ = Describe("Manager", func() { Return(nil). Times(1) + mh.EXPECT(). + RouteDel(gomock.Any()). + Return(nil). + Times(1) + err := manager.RemoveIPAddress() Expect(err).ToNot(HaveOccurred()) }) @@ -195,6 +209,11 @@ var _ = Describe("Manager", func() { Return(syscall.EEXIST). Times(1) + mh.EXPECT(). + RouteAdd(gomock.Any()). + Return(nil). + Times(1) + err := manager.EnsureIPAddress() Expect(err).ToNot(HaveOccurred()) }) @@ -239,7 +258,10 @@ var _ = Describe("Manager", func() { AddrAdd(gomock.Eq(dummy), gomock.Eq(addr)). Return(syscall.EEXIST). Times(1) - + mh.EXPECT(). + RouteAdd(gomock.Any()). + Return(nil). + Times(1) err := manager.EnsureIPAddress() Expect(err).ToNot(HaveOccurred()) }) @@ -249,7 +271,10 @@ var _ = Describe("Manager", func() { AddrAdd(gomock.Eq(dummy), gomock.Eq(addr)). Return(nil). Times(1) - + mh.EXPECT(). + RouteAdd(gomock.Any()). + Return(nil). + Times(1) err := manager.EnsureIPAddress() Expect(err).ToNot(HaveOccurred()) }) From a76b0b70a3d36ddf12d30483f0810870b75bdf0e Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Wed, 25 Sep 2024 15:15:34 +0200 Subject: [PATCH 3/3] add local type to route Signed-off-by: Lukas Hoehl --- internal/netif/netif.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/internal/netif/netif.go b/internal/netif/netif.go index a178ab6c..c0ec5b0e 100644 --- a/internal/netif/netif.go +++ b/internal/netif/netif.go @@ -8,18 +8,16 @@ import ( "os" "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" "golang.org/x/xerrors" "k8s.io/klog/v2" ) -// copied from golang.org/x/sys/unix constants const ( - // copied from RT_TABLE_LOCAL // Also visible in /etc/iproute2/rt_tables file on linux hosts - localRoutingTableId int = 0xff - // copied from RT_SCOPE_HOST + localRoutingTableId int = unix.RT_TABLE_LOCAL // Also visible in /etc/iproute2/rt_scopes file on linux hosts - hostScopeId int = 0xfe + hostScopeId int = unix.RT_SCOPE_HOST ) type Handle interface { @@ -102,12 +100,7 @@ func (m *netifManagerDefault) EnsureIPAddress() error { // loopback device adds new ip addresses to the local routing table by default. // If we are using a different interface, we need to do the updates ourself if l.Attrs().Name != "lo" { - route := &netlink.Route{ - LinkIndex: l.Attrs().Index, - Table: localRoutingTableId, - Dst: m.addr.IPNet, - Scope: netlink.Scope(hostScopeId), - } + route := localRoute(l.Attrs().Index, m.addr) if err = m.RouteAdd(route); err != nil { if !os.IsExist(err) { return xerrors.Errorf("could not add route for %s to interface %s:\n%v", m.addr, m.devName, err) @@ -161,6 +154,7 @@ func localRoute(linkIndex int, addr *netlink.Addr) *netlink.Route { Table: localRoutingTableId, Dst: addr.IPNet, Src: addr.IP, + Type: unix.RTN_LOCAL, Scope: netlink.Scope(hostScopeId), } }