Merge branch 'master' into revert-268-allmultisupport

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

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 ""
+}

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(""))
+		})
+	})
 })

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,43 +57,40 @@ 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
 	tempName := fmt.Sprintf("%s%d", "temp_", linkObj.Attrs().Index)
 
 	// 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
 }
 

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)
 		})
 	})

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"`