diff --git a/cmd/sriov/main.go b/cmd/sriov/main.go index 10e8bd0f4..93784e051 100644 --- a/cmd/sriov/main.go +++ b/cmd/sriov/main.go @@ -42,7 +42,6 @@ func getEnvArgs(envArgsString string) (*envArgs, error) { } func cmdAdd(args *skel.CmdArgs) error { - var macAddr string netConf, err := config.LoadConf(args.StdinData) if err != nil { return fmt.Errorf("SRIOV-CNI failed to load netconf: %v", err) @@ -102,14 +101,15 @@ func cmdAdd(args *skel.CmdArgs) error { }} if !netConf.DPDKMode { - macAddr, err = sm.SetupVF(netConf, args.IfName, netns) + err = sm.SetupVF(netConf, args.IfName, netns) if err != nil { return fmt.Errorf("failed to set up pod interface %q from the device %q: %v", args.IfName, netConf.Master, err) } - result.Interfaces[0].Mac = macAddr } + result.Interfaces[0].Mac = config.GetMacAddressForResult(netConf) + // run the IPAM plugin if netConf.IPAM.Type != "" { var r types.Result diff --git a/pkg/config/config.go b/pkg/config/config.go index b2b6f706e..668bcea73 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -136,3 +136,21 @@ func LoadConfFromCache(args *skel.CmdArgs) (*sriovtypes.NetConf, string, error) return netConf, cRefPath, nil } + +// GetMacAddressForResult return the mac address we should report to the CNI call return object +// if the device is on kernel mode we report that one back +// if not we check the administrative mac address on the PF +// if it is set and is not zero, report it. +func GetMacAddressForResult(netConf *sriovtypes.NetConf) string { + if netConf.MAC != "" { + return netConf.MAC + } + if !netConf.DPDKMode { + return netConf.OrigVfState.EffectiveMAC + } + if netConf.OrigVfState.AdminMAC != "00:00:00:00:00:00" { + return netConf.OrigVfState.AdminMAC + } + + return "" +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 28de55b46..2140acede 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -2,6 +2,7 @@ package config import ( "github.com/containernetworking/plugins/pkg/testutils" + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/types" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -118,4 +119,49 @@ var _ = Describe("Config", func() { Expect(err).To(HaveOccurred()) }) }) + Context("Checking GetMacAddressForResult function", func() { + It("Should return the mac address requested by the user", func() { + netconf := &types.NetConf{ + MAC: "MAC", + OrigVfState: types.VfState{ + EffectiveMAC: "EffectiveMAC", + AdminMAC: "AdminMAC", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("MAC")) + }) + It("Should return the EffectiveMAC mac address if the user didn't request and the the driver is not DPDK", func() { + netconf := &types.NetConf{ + DPDKMode: false, + OrigVfState: types.VfState{ + EffectiveMAC: "EffectiveMAC", + AdminMAC: "AdminMAC", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("EffectiveMAC")) + }) + It("Should return the AdminMAC mac address if the user didn't request and the the driver is DPDK", func() { + netconf := &types.NetConf{ + DPDKMode: true, + OrigVfState: types.VfState{ + EffectiveMAC: "EffectiveMAC", + AdminMAC: "AdminMAC", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("AdminMAC")) + }) + It("Should return empty string if the user didn't request the the driver is DPDK and adminMac is 0", func() { + netconf := &types.NetConf{ + DPDKMode: true, + OrigVfState: types.VfState{ + AdminMAC: "00:00:00:00:00:00", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("")) + }) + }) }) diff --git a/pkg/sriov/sriov.go b/pkg/sriov/sriov.go index dbd007ab0..4d0eb8a6d 100644 --- a/pkg/sriov/sriov.go +++ b/pkg/sriov/sriov.go @@ -36,7 +36,7 @@ func (p *pciUtilsImpl) EnableArpAndNdiscNotify(ifName string) error { // Manager provides interface invoke sriov nic related operations type Manager interface { - SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) (string, error) + SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error ReleaseVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error ResetVFConfig(conf *sriovtypes.NetConf) error ApplyVFConfig(conf *sriovtypes.NetConf) error @@ -57,12 +57,12 @@ func NewSriovManager() Manager { } // SetupVF sets up a VF in Pod netns -func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) (string, error) { +func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error { linkName := conf.OrigVfState.HostIFName linkObj, err := s.nLink.LinkByName(linkName) if err != nil { - return "", fmt.Errorf("error getting VF netdevice with name %s", linkName) + return fmt.Errorf("error getting VF netdevice with name %s", linkName) } // tempName used as intermediary name to avoid name conflicts @@ -70,30 +70,27 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns // 1. Set link down if err := s.nLink.LinkSetDown(linkObj); err != nil { - return "", fmt.Errorf("failed to down vf device %q: %v", linkName, err) + return fmt.Errorf("failed to down vf device %q: %v", linkName, err) } // 2. Set temp name if err := s.nLink.LinkSetName(linkObj, tempName); err != nil { - return "", fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName) + return fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName) } - macAddress := linkObj.Attrs().HardwareAddr.String() + // Save the original effective MAC address before overriding it + conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String() // 3. Set MAC address if conf.MAC != "" { - // Save the original effective MAC address before overriding it - conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String() - err = utils.SetVFEffectiveMAC(s.nLink, tempName, conf.MAC) if err != nil { - return "", fmt.Errorf("failed to set netlink MAC address to %s: %v", conf.MAC, err) + return fmt.Errorf("failed to set netlink MAC address to %s: %v", conf.MAC, err) } - macAddress = conf.MAC } // 4. Change netns if err := s.nLink.LinkSetNsFd(linkObj, int(netns.Fd())); err != nil { - return "", fmt.Errorf("failed to move IF %s to netns: %q", tempName, err) + return fmt.Errorf("failed to move IF %s to netns: %q", tempName, err) } if err := netns.Do(func(_ ns.NetNS) error { @@ -113,11 +110,11 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns return nil }); err != nil { - return "", fmt.Errorf("error setting up interface in container namespace: %q", err) + return fmt.Errorf("error setting up interface in container namespace: %q", err) } conf.ContIFNames = podifName - return macAddress, nil + return nil } // ReleaseVF reset a VF from Pod netns and return it to init netns @@ -284,6 +281,7 @@ func (s *sriovManager) FillOriginalVfInfo(conf *sriovtypes.NetConf) error { return fmt.Errorf("failed to find vf %d", conf.VFID) } conf.OrigVfState.FillFromVfInfo(vfState) + return err } diff --git a/pkg/sriov/sriov_test.go b/pkg/sriov/sriov_test.go index 73b0eacce..8c5eff783 100644 --- a/pkg/sriov/sriov_test.go +++ b/pkg/sriov/sriov_test.go @@ -73,9 +73,9 @@ var _ = Describe("Sriov", func() { mocked.On("LinkSetVfVlanQos", mock.Anything, mock.AnythingOfType("int"), mock.AnythingOfType("int"), mock.AnythingOfType("int")).Return(nil) mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil) sm := sriovManager{nLink: mocked, utils: mockedPciUtils} - macAddr, err := sm.SetupVF(netconf, podifName, targetNetNS) + err = sm.SetupVF(netconf, podifName, targetNetNS) Expect(err).NotTo(HaveOccurred()) - Expect(macAddr).To(Equal("6e:16:06:0e:b7:e9")) + Expect(netconf.OrigVfState.EffectiveMAC).To(Equal("6e:16:06:0e:b7:e9")) }) It("Setting VF's MAC address", func() { var targetNetNS ns.NetNS @@ -116,9 +116,8 @@ var _ = Describe("Sriov", func() { mocked.On("LinkSetUp", fakeLink).Return(nil) mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil) sm := sriovManager{nLink: mocked, utils: mockedPciUtils} - macAddr, err := sm.SetupVF(netconf, podifName, targetNetNS) + err = sm.SetupVF(netconf, podifName, targetNetNS) Expect(err).NotTo(HaveOccurred()) - Expect(macAddr).To(Equal(netconf.MAC)) mocked.AssertExpectations(t) }) }) diff --git a/pkg/types/types.go b/pkg/types/types.go index 75e08845c..76ff57be9 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -35,7 +35,7 @@ func (vs *VfState) FillFromVfInfo(info *netlink.VfInfo) { type NetConf struct { types.NetConf OrigVfState VfState // Stores the original VF state as it was prior to any operations done during cmdAdd flow - DPDKMode bool + DPDKMode bool `json:"-"` Master string MAC string Vlan *int `json:"vlan"`