From 20cb85cd13f5502f48bb7b1818e50da4bfd5f3a0 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Wed, 1 Jan 2025 15:09:39 +0200 Subject: [PATCH] Save MTU in cache to reset on vf release we introduce this feature in the cni to handle cases where a pod that used the vf has net_admin and change the MTU on the VF. when that po is removed the VF will go back to the host and be available for a new pod to run but when that VF gets allocated to the pod it will remain with an unexpected MTU. Signed-off-by: Sebastian Sch --- pkg/sriov/sriov.go | 22 ++++++++++++++++++++++ pkg/sriov/sriov_test.go | 15 ++++++++++++++- pkg/types/types.go | 1 + pkg/utils/mocks/netlink_manager_mock.go | 18 ++++++++++++++++++ pkg/utils/netlink_manager.go | 6 ++++++ 5 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/sriov/sriov.go b/pkg/sriov/sriov.go index f5c57970f..5057f6384 100644 --- a/pkg/sriov/sriov.go +++ b/pkg/sriov/sriov.go @@ -225,6 +225,19 @@ func (s *sriovManager) ReleaseVF(conf *sriovtypes.NetConf, podifName string, net } } + // reset MTU for VF device until if the MTU was captured in the cache + if conf.OrigVfState.MTU != 0 { + logging.Debug("Reset VF device MTU", + "func", "ReleaseVF", + "linkObj", linkObj, + "conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName, + "conf.OrigVfState.MTU", conf.OrigVfState.MTU) + err = s.nLink.LinkSetMTU(linkObj, conf.OrigVfState.MTU) + if err != nil { + return fmt.Errorf("failed to reset MTU for link link %s: %q", conf.OrigVfState.HostIFName, err) + } + } + // move VF device to init netns logging.Debug("Move VF device to init netns", "func", "ReleaseVF", @@ -351,6 +364,15 @@ func (s *sriovManager) FillOriginalVfInfo(conf *sriovtypes.NetConf) error { } conf.OrigVfState.FillFromVfInfo(vfState) + // add also MTU to the vf info in the vf is we have an interface name + if conf.OrigVfState.HostIFName != "" { + vfLink, err := s.nLink.LinkByName(conf.OrigVfState.HostIFName) + if err != nil { + return fmt.Errorf("failed to lookup vf %q: %v", conf.OrigVfState.HostIFName, err) + } + conf.OrigVfState.MTU = vfLink.Attrs().MTU + } + return err } diff --git a/pkg/sriov/sriov_test.go b/pkg/sriov/sriov_test.go index fdd6497bd..b61ef8dad 100644 --- a/pkg/sriov/sriov_test.go +++ b/pkg/sriov/sriov_test.go @@ -38,6 +38,7 @@ var _ = Describe("Sriov", func() { VFID: 0, OrigVfState: sriovtypes.VfState{ HostIFName: "enp175s6", + MTU: 1500, }}, } t = GinkgoT() @@ -310,6 +311,7 @@ var _ = Describe("Sriov", func() { OrigVfState: sriovtypes.VfState{ HostIFName: "enp175s6", EffectiveMAC: "6e:16:06:0e:b7:e9", + MTU: 1500, }}, } }) @@ -331,6 +333,7 @@ var _ = Describe("Sriov", func() { mocked.On("LinkByName", podifName).Return(fakeLink, nil) mocked.On("LinkSetDown", fakeLink).Return(nil) mocked.On("LinkSetName", fakeLink, netconf.OrigVfState.HostIFName).Return(nil) + mocked.On("LinkSetMTU", fakeLink, 1500).Return(nil) mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil) sm := sriovManager{nLink: mocked} err = sm.ReleaseVF(netconf, podifName, targetNetNS) @@ -434,7 +437,7 @@ var _ = Describe("Sriov", func() { fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ Index: 1000, - Name: "dummylink", + Name: netconf.Name, HardwareAddr: fakeMac, Vfs: []netlink.VfInfo{ { @@ -443,10 +446,19 @@ var _ = Describe("Sriov", func() { }, }, }} + + fakeVFLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ + Index: 1001, + Name: netconf.OrigVfState.HostIFName, + MTU: 1500, + }} + mocked.On("LinkByName", netconf.Master).Return(fakeLink, nil) + mocked.On("LinkByName", netconf.OrigVfState.HostIFName).Return(fakeVFLink, nil) sm := sriovManager{nLink: mocked} err = sm.FillOriginalVfInfo(netconf) Expect(err).NotTo(HaveOccurred()) + Expect(netconf.OrigVfState.MTU).To(Equal(1500)) mocked.AssertExpectations(t) }) }) @@ -509,6 +521,7 @@ var _ = Describe("Sriov", func() { MinTxRate: 0, MaxTxRate: 0, LinkState: 2, // disable + MTU: 1500, }}, } }) diff --git a/pkg/types/types.go b/pkg/types/types.go index 07b30f83f..4cb44aab5 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -27,6 +27,7 @@ type VfState struct { MinTxRate int MaxTxRate int LinkState uint32 + MTU int } // FillFromVfInfo - Fill attributes according to the provided netlink.VfInfo struct diff --git a/pkg/utils/mocks/netlink_manager_mock.go b/pkg/utils/mocks/netlink_manager_mock.go index f77682ef0..cf6893008 100644 --- a/pkg/utils/mocks/netlink_manager_mock.go +++ b/pkg/utils/mocks/netlink_manager_mock.go @@ -83,6 +83,24 @@ func (_m *NetlinkManager) LinkSetHardwareAddr(_a0 netlink.Link, _a1 net.Hardware return r0 } +// LinkSetMTU provides a mock function with given fields: _a0, _a1 +func (_m *NetlinkManager) LinkSetMTU(_a0 netlink.Link, _a1 int) error { + ret := _m.Called(_a0, _a1) + + if len(ret) == 0 { + panic("no return value specified for LinkSetMTU") + } + + var r0 error + if rf, ok := ret.Get(0).(func(netlink.Link, int) error); ok { + r0 = rf(_a0, _a1) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // LinkSetName provides a mock function with given fields: _a0, _a1 func (_m *NetlinkManager) LinkSetName(_a0 netlink.Link, _a1 string) error { ret := _m.Called(_a0, _a1) diff --git a/pkg/utils/netlink_manager.go b/pkg/utils/netlink_manager.go index 165f0e7d2..c5ea823c1 100644 --- a/pkg/utils/netlink_manager.go +++ b/pkg/utils/netlink_manager.go @@ -22,6 +22,7 @@ type NetlinkManager interface { LinkSetVfSpoofchk(netlink.Link, int, bool) error LinkSetVfTrust(netlink.Link, int, bool) error LinkSetVfState(netlink.Link, int, uint32) error + LinkSetMTU(netlink.Link, int) error LinkDelAltName(netlink.Link, string) error } @@ -92,6 +93,11 @@ func (n *MyNetlink) LinkSetVfState(link netlink.Link, vf int, state uint32) erro return netlink.LinkSetVfState(link, vf, state) } +// LinkSetMTU using NetlinkManager +func (n *MyNetlink) LinkSetMTU(link netlink.Link, mtu int) error { + return netlink.LinkSetMTU(link, mtu) +} + // LinkDelAltName using NetlinkManager func (n *MyNetlink) LinkDelAltName(link netlink.Link, altName string) error { return netlink.LinkDelAltName(link, altName)