From 8679b04d33ebbbac9a4fbdd501d409ac6e7d4373 Mon Sep 17 00:00:00 2001 From: Ran Gu Date: Wed, 5 Jun 2024 10:51:28 +0800 Subject: [PATCH] Remove unexpected AltName after rename interface (#6321) Fix #6301 Signed-off-by: gran Co-authored-by: Lan --- pkg/agent/util/net_linux.go | 23 ++++++++++++++ pkg/agent/util/net_linux_test.go | 27 +++++++++++++++++ pkg/agent/util/netlink/netlink_linux.go | 4 +++ .../netlink/testing/mock_netlink_linux.go | 30 ++++++++++++++++++- 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/pkg/agent/util/net_linux.go b/pkg/agent/util/net_linux.go index 80459d737c1..b7b763e6d7e 100644 --- a/pkg/agent/util/net_linux.go +++ b/pkg/agent/util/net_linux.go @@ -292,6 +292,15 @@ func RenameInterface(from, to string) error { if pollErr != nil { return fmt.Errorf("failed to rename host interface name %s to %s", from, to) } + // Fix for the issue https://github.com/antrea-io/antrea/issues/6301. + // In some new Linux versions which support AltName, if the only valid altname of the interface is the same as the + // interface name, it would be left empty when the name is occupied by the interface name; after we rename the + // interface name to another value, the altname of the interface would be set to the original interface name by the + // system. + // This altname must be removed as we need to reserve the name for an OVS internal port. + if err := removeInterfaceAltName(to, from); err != nil { + return fmt.Errorf("failed to remove AltName %s on interface %s: %w", from, to, err) + } return nil } @@ -379,3 +388,17 @@ func renameHostInterface(oriName string, newName string) error { } return nil } + +// removeInterfaceAltName removes altName on interface with provided name. altName not found will return nil. +func removeInterfaceAltName(name string, altName string) error { + link, err := netlinkUtil.LinkByName(name) + if err != nil { + return err + } + for _, existAltName := range link.Attrs().AltNames { + if existAltName == altName { + return netlinkUtil.LinkDelAltName(link, altName) + } + } + return nil +} diff --git a/pkg/agent/util/net_linux_test.go b/pkg/agent/util/net_linux_test.go index 472e53dc74a..eff99a266c2 100644 --- a/pkg/agent/util/net_linux_test.go +++ b/pkg/agent/util/net_linux_test.go @@ -36,6 +36,7 @@ type mockLink struct { name string masterIndex int hardwareAddr net.HardwareAddr + altNames []string } func (l mockLink) Attrs() *netlink.LinkAttrs { @@ -44,6 +45,7 @@ func (l mockLink) Attrs() *netlink.LinkAttrs { Name: l.name, MasterIndex: l.masterIndex, HardwareAddr: l.hardwareAddr, + AltNames: l.altNames, } } @@ -557,6 +559,7 @@ func TestGetInterfaceConfig(t *testing.T) { func TestRenameInterface(t *testing.T) { renameFailErr := fmt.Errorf("failed to rename host interface name test1 to test2") + removeAltNameFailErr := fmt.Errorf("failed to remove AltName test1 on interface test2: %w", testInvalidErr) tests := []struct { name string expectedCalls func(mockNetlink *netlinktest.MockInterfaceMockRecorder) @@ -569,6 +572,18 @@ func TestRenameInterface(t *testing.T) { mockNetlink.LinkSetDown(mockLink{name: "test1"}).Return(nil) mockNetlink.LinkSetName(mockLink{name: "test1"}, "test2").Return(nil) mockNetlink.LinkSetUp(mockLink{name: "test1"}).Return(nil) + mockNetlink.LinkByName("test2").Return(mockLink{name: "test2", altNames: []string{}}, nil) + }, + }, + { + name: "Rename Interface Success with AltName", + expectedCalls: func(mockNetlink *netlinktest.MockInterfaceMockRecorder) { + mockNetlink.LinkByName("test1").Return(mockLink{name: "test1"}, nil) + mockNetlink.LinkSetDown(mockLink{name: "test1"}).Return(nil) + mockNetlink.LinkSetName(mockLink{name: "test1"}, "test2").Return(nil) + mockNetlink.LinkSetUp(mockLink{name: "test1"}).Return(nil) + mockNetlink.LinkByName("test2").Return(mockLink{name: "test2", altNames: []string{"test1"}}, nil) + mockNetlink.LinkDelAltName(mockLink{name: "test2", altNames: []string{"test1"}}, "test1").Return(nil) }, }, { @@ -605,6 +620,18 @@ func TestRenameInterface(t *testing.T) { }, wantErr: renameFailErr, }, + { + name: "Remove AltName Err", + expectedCalls: func(mockNetlink *netlinktest.MockInterfaceMockRecorder) { + mockNetlink.LinkByName("test1").Return(mockLink{name: "test1"}, nil) + mockNetlink.LinkSetDown(mockLink{name: "test1"}).Return(nil) + mockNetlink.LinkSetName(mockLink{name: "test1"}, "test2").Return(nil) + mockNetlink.LinkSetUp(mockLink{name: "test1"}).Return(nil) + mockNetlink.LinkByName("test2").Return(mockLink{name: "test2", altNames: []string{"test1"}}, nil) + mockNetlink.LinkDelAltName(mockLink{name: "test2", altNames: []string{"test1"}}, "test1").Return(testInvalidErr) + }, + wantErr: removeAltNameFailErr, + }, } for _, tc := range tests { diff --git a/pkg/agent/util/netlink/netlink_linux.go b/pkg/agent/util/netlink/netlink_linux.go index 253f0dd1ddc..ffe56ca02fb 100644 --- a/pkg/agent/util/netlink/netlink_linux.go +++ b/pkg/agent/util/netlink/netlink_linux.go @@ -64,6 +64,10 @@ type Interface interface { LinkSetName(link netlink.Link, name string) error + LinkAddAltName(link netlink.Link, name string) error + + LinkDelAltName(link netlink.Link, name string) error + LinkSetUp(link netlink.Link) error ConntrackDeleteFilter(table netlink.ConntrackTableType, family netlink.InetFamily, filter netlink.CustomConntrackFilter) (uint, error) diff --git a/pkg/agent/util/netlink/testing/mock_netlink_linux.go b/pkg/agent/util/netlink/testing/mock_netlink_linux.go index f61b0276b5c..e8353d9f8c1 100644 --- a/pkg/agent/util/netlink/testing/mock_netlink_linux.go +++ b/pkg/agent/util/netlink/testing/mock_netlink_linux.go @@ -1,4 +1,4 @@ -// Copyright 2023 Antrea Authors +// Copyright 2024 Antrea Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -126,6 +126,20 @@ func (mr *MockInterfaceMockRecorder) ConntrackDeleteFilter(arg0, arg1, arg2 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConntrackDeleteFilter", reflect.TypeOf((*MockInterface)(nil).ConntrackDeleteFilter), arg0, arg1, arg2) } +// LinkAddAltName mocks base method. +func (m *MockInterface) LinkAddAltName(arg0 netlink.Link, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LinkAddAltName", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// LinkAddAltName indicates an expected call of LinkAddAltName. +func (mr *MockInterfaceMockRecorder) LinkAddAltName(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LinkAddAltName", reflect.TypeOf((*MockInterface)(nil).LinkAddAltName), arg0, arg1) +} + // LinkByIndex mocks base method. func (m *MockInterface) LinkByIndex(arg0 int) (netlink.Link, error) { m.ctrl.T.Helper() @@ -156,6 +170,20 @@ func (mr *MockInterfaceMockRecorder) LinkByName(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LinkByName", reflect.TypeOf((*MockInterface)(nil).LinkByName), arg0) } +// LinkDelAltName mocks base method. +func (m *MockInterface) LinkDelAltName(arg0 netlink.Link, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LinkDelAltName", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// LinkDelAltName indicates an expected call of LinkDelAltName. +func (mr *MockInterfaceMockRecorder) LinkDelAltName(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LinkDelAltName", reflect.TypeOf((*MockInterface)(nil).LinkDelAltName), arg0, arg1) +} + // LinkSetDown mocks base method. func (m *MockInterface) LinkSetDown(arg0 netlink.Link) error { m.ctrl.T.Helper()