Merge https://github.com/k8snetworkplumbingwg/sriov-cni:master into main

Author: SchSeba

pkg/sriov/sriov.go

@@ -66,82 +66,100 @@ func NewSriovManager() Manager {
 // SetupVF sets up a VF in Pod netns
 func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error {
 	linkName := conf.OrigVfState.HostIFName
+	// Save the original NS in case we need to restore it
+	// after an error occurs
+	initns, err := ns.GetCurrentNS()
+	if err != nil {
+		return fmt.Errorf("failed to get current NS: %v", err)
+	}
+	tempNS, err := ns.TempNetNS()
+	if err != nil {
+		return fmt.Errorf("failed to create tempNS: %v", err)
+	}
+
+	defer func() {
+		if cerr := tempNS.Close(); cerr != nil {
+			logging.Warning("failed to close temporary netns; VF may remain accessible in temp namespace",
+				"func", "SetupVF",
+				"linkName", linkName,
+				"error", cerr)
+		}
+	}()
 
 	linkObj, err := s.nLink.LinkByName(linkName)
 	if err != nil {
-		return fmt.Errorf("error getting VF netdevice with name %s", linkName)
+		return fmt.Errorf("error: %v. Failed to get VF netdevice with name %s", err, linkName)
 	}
 
 	// Save the original effective MAC address before overriding it
 	conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String()
 
-	// tempName used as intermediary name to avoid name conflicts
-	tempName := fmt.Sprintf("%s%d", "temp_", linkObj.Attrs().Index)
-
-	// 1. Set link down
-	logging.Debug("1. Set link down",
+	// 1.Move interface to tempNS
+	logging.Debug("1. Move the interface to tempNS",
 		"func", "SetupVF",
 		"linkObj", linkObj)
-	if err := s.nLink.LinkSetDown(linkObj); err != nil {
-		return fmt.Errorf("failed to down vf device %q: %v", linkName, err)
-	}
-
-	// 2. Set temp name
-	logging.Debug("2. Set temp name",
-		"func", "SetupVF",
-		"linkObj", linkObj,
-		"tempName", tempName)
-	if err := s.nLink.LinkSetName(linkObj, tempName); err != nil {
-		return fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName)
+	if err = s.nLink.LinkSetNsFd(linkObj, int(tempNS.Fd())); err != nil {
+		return fmt.Errorf("failed to move %q to tempNS: %v", linkName, err)
 	}
+	err = tempNS.Do(func(linkNS ns.NetNS) error {
+		// lookup the device in tempNS (index might have changed)
+		tempNSLinkObj, err := s.nLink.LinkByName(linkName)
+		if err != nil {
+			return fmt.Errorf("failed to find %q in tempNS: %v", linkName, err)
+		}
+		// Rename the interface to pod interface name
+		if err = s.nLink.LinkSetName(tempNSLinkObj, podifName); err != nil {
+			return fmt.Errorf("failed to rename host device %q to %q: %v", linkName, podifName, err)
+		}
 
-	// 3. Remove alt name from the nic
-	logging.Debug("3. Remove interface original name from alt names",
-		"func", "SetupVF",
-		"linkObj", linkObj,
-		"OriginalLinkName", linkName,
-		"tempName", tempName)
-	linkObj, err = s.nLink.LinkByName(tempName)
-	if err != nil {
-		return fmt.Errorf("error getting VF netdevice with name %s: %v", tempName, err)
-	}
-	for _, altName := range linkObj.Attrs().AltNames {
-		if altName == linkName {
-			if err := s.nLink.LinkDelAltName(linkObj, linkName); err != nil {
-				return fmt.Errorf("error removing VF altname %s: %v", linkName, err)
+		// 3. Remove alt name from the nic
+		logging.Debug("3. Remove interface original name from alt names",
+			"func", "SetupVF",
+			"tempNSObj", tempNSLinkObj,
+			"OriginalLinkName", linkName)
+		for _, altName := range tempNSLinkObj.Attrs().AltNames {
+			if altName == linkName {
+				if err = s.nLink.LinkDelAltName(tempNSLinkObj, linkName); err != nil {
+					return fmt.Errorf("error removing VF altname %s: %v", linkName, err)
+				}
 			}
 		}
-	}
 
-	// 4. Change netns
-	logging.Debug("4. Change netns",
-		"func", "SetupVF",
-		"linkObj", linkObj,
-		"netns.Fd()", int(netns.Fd()))
-	if err := s.nLink.LinkSetNsFd(linkObj, int(netns.Fd())); err != nil {
-		return fmt.Errorf("failed to move IF %s to netns: %q", tempName, err)
+		// 4. Change netns
+		logging.Debug("4. Change netns",
+			"func", "SetupVF",
+			"tempNSObj", tempNSLinkObj,
+			"netns.Fd()", int(netns.Fd()))
+		if err = s.nLink.LinkSetNsFd(tempNSLinkObj, int(netns.Fd())); err != nil {
+			return fmt.Errorf("failed to move IF %s to netns: %w", podifName, err)
+		}
+		return nil
+	})
+	if err != nil {
+		logging.Error("Move the interface back to initNS because of ", "error", err)
+		renameAndMoveLinkErr := s.renameAndMoveLink(tempNS, initns, []string{podifName, linkName}, linkName)
+		if renameAndMoveLinkErr != nil {
+			return fmt.Errorf("setupVF failed: %v; rollback failed: %v", err, renameAndMoveLinkErr)
+		}
+		return fmt.Errorf("setupVF failed: %v", err)
 	}
 
-	if err := netns.Do(func(_ ns.NetNS) error {
-		// 5. Set Pod IF name
-		logging.Debug("5. Set Pod IF name",
-			"func", "SetupVF",
-			"linkObj", linkObj,
-			"podifName", podifName)
-		if err := s.nLink.LinkSetName(linkObj, podifName); err != nil {
-			return fmt.Errorf("error setting container interface name %s for %s", linkName, tempName)
+	err = netns.Do(func(_ ns.NetNS) error {
+		netNSLinkObj, err := s.nLink.LinkByName(podifName)
+		if err != nil {
+			return fmt.Errorf("error: %v. Failed to get VF netdevice with name %s", err, podifName)
 		}
 
-		// 6. Enable IPv4 ARP notify and IPv6 Network Discovery notify
+		// 5. Enable IPv4 ARP notify and IPv6 Network Discovery notify
 		// Error is ignored here because enabling this feature is only a performance enhancement.
-		logging.Debug("6. Enable IPv4 ARP notify and IPv6 Network Discovery notify",
+		logging.Debug("5. Enable IPv4 ARP notify and IPv6 Network Discovery notify",
 			"func", "SetupVF",
 			"podifName", podifName)
 		_ = s.utils.EnableArpAndNdiscNotify(podifName)
 
-		// 7. Set MAC address
+		// 6. Set MAC address
 		if conf.MAC != "" {
-			logging.Debug("7. Set MAC address",
+			logging.Debug("6. Set MAC address",
 				"func", "SetupVF",
 				"s.nLink", s.nLink,
 				"podifName", podifName,
@@ -152,31 +170,79 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns
 			}
 		}
 
-		logging.Debug("8. Enable Optimistic DAD for IPv6 addresses", "func", "SetupVF",
-			"linkObj", linkObj)
+		logging.Debug("7. Enable Optimistic DAD for IPv6 addresses", "func", "SetupVF",
+			"linkObj", netNSLinkObj)
 		_ = s.utils.EnableOptimisticDad(podifName)
 
-		// 9. Bring IF up in Pod netns
-		logging.Debug("9. Bring IF up in Pod netns",
+		// 8. Bring IF up in Pod netns
+		logging.Debug("8. Bring IF up in Pod netns",
 			"func", "SetupVF",
-			"linkObj", linkObj)
-		if err := s.nLink.LinkSetUp(linkObj); err != nil {
+			"linkObj", netNSLinkObj)
+		if err = s.nLink.LinkSetUp(netNSLinkObj); err != nil {
 			return fmt.Errorf("error bringing interface up in container ns: %q", err)
 		}
 
 		return nil
-	}); err != nil {
+	})
+	if err != nil {
 		return fmt.Errorf("error setting up interface in container namespace: %q", err)
 	}
 
 	// Copy the MTU value to a new variable
 	// and use it as a pointer
 	vfMTU := linkObj.Attrs().MTU
 	conf.MTU = &vfMTU
-
 	return nil
 }
 
+// renameAndMoveLink finds a link by iterating over possible names, renames it if needed, and moves it to target namespace
+func (s *sriovManager) renameAndMoveLink(source, target ns.NetNS, possibleIfNames []string, targetIfname string) error {
+	return source.Do(func(_ ns.NetNS) error {
+		var linkObj netlink.Link
+		var currentName string
+		var err error
+
+		// Iterate over possible interface names to find the link
+		for _, name := range possibleIfNames {
+			linkObj, err = s.nLink.LinkByName(name)
+			if err == nil {
+				currentName = name
+				logging.Debug("Found interface",
+					"func", "renameAndMoveLink",
+					"currentName", currentName)
+				break
+			}
+		}
+
+		if linkObj == nil {
+			return fmt.Errorf("failed to find interface with any of the possible names: %v", possibleIfNames)
+		}
+
+		// Rename the interface if its current name is different from target name
+		if currentName != targetIfname {
+			logging.Debug("Renaming interface",
+				"func", "renameAndMoveLink",
+				"from", currentName,
+				"to", targetIfname)
+			if err = s.nLink.LinkSetName(linkObj, targetIfname); err != nil {
+				logging.Warning("LinkSetName failed when trying to rename", "error", err)
+				return fmt.Errorf("failed to rename interface from %q to %q: %v", currentName, targetIfname, err)
+			}
+		}
+
+		// Move interface to target namespace
+		logging.Debug("Moving interface to target namespace",
+			"func", "renameAndMoveLink",
+			"ifname", targetIfname)
+		if err = s.nLink.LinkSetNsFd(linkObj, int(target.Fd())); err != nil {
+			logging.Warning("LinkSetNsFd failed when trying to move to target namespace", "error", err)
+			return fmt.Errorf("failed to move interface %q to target namespace: %v", targetIfname, err)
+		}
+
+		return nil
+	})
+}
+
 // ReleaseVF reset a VF from Pod netns and return it to init netns
 func (s *sriovManager) ReleaseVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error {
 	initns, err := ns.GetCurrentNS()
@@ -273,7 +339,6 @@ func (s *sriovManager) ApplyVFConfig(conf *sriovtypes.NetConf) error {
 			return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %s: %v", conf.VFID, *conf.Vlan, *conf.VlanQoS, *conf.VlanProto, err)
 		}
 	}
-
 	// 2. Set mac address
 	if conf.MAC != "" {
 		// when we restore the original hardware mac address we may get a device or resource busy. so we introduce retry