Fix: removing secondary interface in Asyc delete for SWiftV2 stateless CNI

Author: behzad-mir

cni/network/network.go

@@ -1078,6 +1078,13 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
 			case errors.Is(err, network.ErrConnectionFailure):
 				logger.Error("Failed to connect to CNS", zap.Error(err))
 				logger.Info("Endpoint will be deleted from state file asynchronously", zap.String("containerID", args.ContainerID))
+				// In SwiftV2 Linux stateless CNI mode, if the plugin cannot connect to CNS,
+				// we asynchronously remove the secondary (delegated) interface from the pod’s network namespace.
+				// This is necessary because leaving the delegated NIC in the pod netns can cause the kernel to block rtnetlink operations.
+				// When that happens, kubelet and containerd hang during sandbox creation or teardown.
+				// The delegated NIC (SR-IOV VF) used by SwiftV2 for multitenant pods remains tied to the pod namespace,
+				// triggering hot-unplug/re-register events and leaving the node in an unhealthy state.
+				plugin.nm.RemoveSecondaryEndpointFromPodNetNS(args.IfName, args.Netns)
 			case errors.Is(err, network.ErrEndpointStateNotFound):
 				logger.Info("Endpoint Not found", zap.String("containerID", args.ContainerID), zap.Error(err))
 				return nil

cns/restserver/restserver.go

@@ -132,7 +132,7 @@ type IPInfo struct {
 	HnsNetworkID  string      `json:",omitempty"`
 	HostVethName  string      `json:",omitempty"`
 	MacAddress    string      `json:",omitempty"`
-	NICType       cns.NICType `json:",omitempty"`
+	NICType       cns.NICType
 }
 
 type GetHTTPServiceDataResponse struct {

network/endpoint_linux.go

@@ -547,3 +547,12 @@ func getDefaultGateway(routes []RouteInfo) net.IP {
 func (epInfo *EndpointInfo) GetEndpointInfoByIPImpl(_ []net.IPNet, _ string) (*EndpointInfo, error) {
 	return epInfo, nil
 }
+
+// removeSecondaryEndpointFromPodNetNSImpl deletes an existing secondary endpoint from the pod network namespace.
+func (ep *endpoint) removeSecondaryEndpointFromPodNetNSImpl(nl netlink.NetlinkInterface, nsc NamespaceClientInterface) error {
+	secondaryepClient := NewSecondaryEndpointClient(nl, nil, nil, nsc, nil, ep)
+	if err := secondaryepClient.RemoveInterfacesfromNetnsPath(ep.IfName, ep.NetworkNameSpace); err != nil {
+		return err
+	}
+	return nil
+}

network/endpoint_windows.go

@@ -746,3 +746,8 @@ func getPnpDeviceState(instanceID string, plc platform.ExecClient) (string, stri
 	logger.Info("Retrieved device problem code", zap.String("code", devpkeyDeviceProblemCode))
 	return devpkeyDeviceIsPresent, devpkeyDeviceProblemCode, nil
 }
+
+// removeSecondaryEndpointFromPodNetNSImpl removes an existing secondary endpoint from the pod network namespace.
+func (ep *endpoint) removeSecondaryEndpointFromPodNetNSImpl(_ netlink.NetlinkInterface, _ NamespaceClientInterface) error {
+	return nil
+}

network/manager.go

@@ -122,6 +122,7 @@ type NetworkManager interface {
 	DeleteState(epInfos []*EndpointInfo) error
 	GetEndpointInfosFromContainerID(containerID string) []*EndpointInfo
 	GetEndpointState(networkID, containerID string) ([]*EndpointInfo, error)
+	RemoveSecondaryEndpointFromPodNetNS(ifName string, netns string) error
 }
 
 // Creates a new network manager.
@@ -860,3 +861,16 @@ func generateCNSIPInfoMap(eps []*endpoint) map[string]*restserver.IPInfo {
 
 	return ifNametoIPInfoMap
 }
+
+// RemoveSecondaryEndpointFromPodNetNS removes the secondary endpoint from the pod netns
+func (nm *networkManager) RemoveSecondaryEndpointFromPodNetNS(ifName string, netns string) error {
+	ep := &endpoint{
+		NetworkNameSpace: netns,
+		IfName:           ifName, // TODO: For stateless cni linux populate IfName here to use in deletion in secondary endpoint client
+	}
+
+	logger.Info("Removing Secondary Endpoint from", zap.String("NetworkNameSpace: ", netns))
+
+	err := ep.removeSecondaryEndpointFromPodNetNSImpl(nm.netlink, nm.nsClient)
+	return err
+}

network/secondary_endpoint_client_linux.go

@@ -2,6 +2,7 @@ package network
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"strings"
 	"time"
@@ -13,6 +14,7 @@ import (
 	"github.com/Azure/azure-container-networking/network/networkutils"
 	"github.com/Azure/azure-container-networking/platform"
 	"github.com/pkg/errors"
+	vishnetlink "github.com/vishvananda/netlink"
 	"go.uber.org/zap"
 	"k8s.io/kubernetes/pkg/kubelet"
 )
@@ -190,15 +192,15 @@ func (client *SecondaryEndpointClient) DeleteEndpoints(ep *endpoint) error {
 		}
 	}()
 	// For stateless cni linux, check if delegated vmnic type, and if so, delete using this *endpoint* struct's ifname
-	if ep.NICType == cns.DelegatedVMNIC {
-		if err := client.moveInterfaceToHostNetns(ep.IfName, vmns); err != nil {
+	if ep.NICType == cns.NodeNetworkInterfaceFrontendNIC {
+		if err := client.MoveInterfaceToHostNetns(ep.IfName, vmns); err != nil {
 			logger.Error("Failed to move interface", zap.String("IfName", ep.IfName), zap.Error(newErrorSecondaryEndpointClient(err)))
 		}
 	}
 	// For Statefull cni linux, Use SecondaryInterfaces map to move all interfaces to host netns
 	// TODO: SecondaryInterfaces map should be retired and only IfName field and NICType should be used to determine the delgated NIC
 	for iface := range ep.SecondaryInterfaces {
-		if err := client.moveInterfaceToHostNetns(iface, vmns); err != nil {
+		if err := client.MoveInterfaceToHostNetns(iface, vmns); err != nil {
 			logger.Error("Failed to move interface", zap.String("IfName", iface), zap.Error(newErrorSecondaryEndpointClient(err)))
 			continue
 		}
@@ -209,10 +211,74 @@ func (client *SecondaryEndpointClient) DeleteEndpoints(ep *endpoint) error {
 }
 
 // moveInterfaceToHostNetns moves the given interface to the host netns.
-func (client *SecondaryEndpointClient) moveInterfaceToHostNetns(ifName string, vmns int) error {
+func (client *SecondaryEndpointClient) MoveInterfaceToHostNetns(ifName string, vmns int) error {
 	logger.Info("Moving interface to host netns", zap.String("IfName", ifName))
 	if err := client.netlink.SetLinkNetNs(ifName, uintptr(vmns)); err != nil {
 		return newErrorSecondaryEndpointClient(err)
 	}
 	return nil
 }
+
+// removeInterfacesfromNetnsPath finds and removes all interfaces from the specified netns path except the infra and non-eth interfaces.
+func (client *SecondaryEndpointClient) RemoveInterfacesfromNetnsPath(infraInterfaceName string, netnspath string) error {
+	// Get VM namespace
+	vmns, err := netns.New().Get()
+	if err != nil {
+		return newErrorSecondaryEndpointClient(err)
+	}
+
+	// Open the network namespace.
+	logger.Info("Opening netns", zap.Any("NetNsPath", netnspath))
+	ns, err := client.nsClient.OpenNamespace(netnspath)
+	if err != nil {
+		return newErrorSecondaryEndpointClient(err)
+	}
+	defer ns.Close()
+
+	// Enter the container network namespace.
+	logger.Info("Entering netns", zap.Any("NetNsPath", netnspath))
+	if err := ns.Enter(); err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return nil
+		}
+
+		return newErrorSecondaryEndpointClient(err)
+	}
+
+	// Return to host network namespace.
+	defer func() {
+		logger.Info("Exiting netns", zap.Any("NetNsPath", netnspath))
+		if err := ns.Exit(); err != nil {
+			logger.Error("Failed to exit netns with", zap.Error(newErrorSecondaryEndpointClient(err)))
+		}
+	}()
+	// For stateless cni linux, check if delegated vmnic type, and if so, delete using this *endpoint* struct's ifname
+	ifnames, err := listIfNamesInNetns(ns)
+	for _, iface := range ifnames {
+		// skip the infra interface as well as non-eth interfaces
+		if iface == infraInterfaceName || !strings.Contains(iface, "eth") {
+			continue
+		}
+		if err := client.MoveInterfaceToHostNetns(iface, vmns); err != nil {
+			logger.Error("Failed to move interface", zap.String("IfName", iface), zap.Error(newErrorSecondaryEndpointClient(err)))
+		}
+	}
+
+	return nil
+}
+
+// ListIfNamesInNetns returns all interface names in the given netns path.
+func listIfNamesInNetns(ns NamespaceInterface) ([]string, error) {
+	// Use the existing netlink interface to list links
+	links, err := vishnetlink.LinkList()
+	if err != nil {
+		return nil, fmt.Errorf("failed to list links: %w", err)
+	}
+
+	names := make([]string, 0, len(links))
+	for _, l := range links {
+		names = append(names, l.Attrs().Name)
+	}
+	logger.Info("Found interfaces in netns that needs to be moved back to host", zap.Any("interfaces", names))
+	return names, nil
+}