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

Author: behzad-mir

cni/network/network.go

@@ -1071,13 +1071,24 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
 		epInfos, err = plugin.nm.GetEndpointState(networkID, args.ContainerID)
 		// if stateless CNI fail to get the endpoint from CNS for any reason other than Endpoint Not found or CNS connection failure
 		// return a retriable error so the container runtime will retry this DEL later
-		// the implementation of this function returns nil if the endpoint doens't exist, so
+		// the implementation of this function returns nil if the endpoint doesn't exist, so
 		// we don't have to check that here
 		if err != nil {
 			switch {
 			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 in the absence of the endpoint state.
+				// 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.
+				// This workaround mitigates the issue by removing the secondary NIC from the pod netns when CNS is unreachable during DEL to provide the endpoint state.
+				if err = plugin.nm.RemoveSecondaryEndpointFromPodNetNS(args.IfName, args.Netns); err != nil {
+					logger.Error("Failed to remove secondary endpoint from pod netns", zap.String("netns", args.Netns), zap.Error(err))
+					return plugin.RetriableError(fmt.Errorf("failed to remove secondary endpoint from pod netns: %w", err))
+				}
 			case errors.Is(err, network.ErrEndpointStateNotFound):
 				logger.Info("Endpoint Not found", zap.String("containerID", args.ContainerID), zap.Error(err))
 				return nil
@@ -1088,15 +1099,15 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
 		} else {
 			for i, epInfo := range epInfos {
 				logger.Info("Found endpoint to delete", zap.String("IfName", epInfo.IfName), zap.String("EndpointID", epInfo.EndpointID), zap.Any("NICType", epInfo.NICType))
-				epInfos[i].NetNsPath = args.Netns // in case DelegatedNIC need to be moved to host namespace
+				epInfos[i].NetNsPath = args.Netns // This is needed in SSwiftV2 scenario to move DelegatedNIC to host namespace
 			}
 		}
 	} else {
 		epInfos = plugin.nm.GetEndpointInfosFromContainerID(args.ContainerID)
 	}
 
 	// for Stateful CNI when the endpoint is not created, but the ips are already allocated (only works if single network, single infra)
-	// this block is applied to stateless CNI only if there was a connection failure in previous block
+	// this block is applied to stateless CNI only if there was a connection failure in previous block and asynchronous delete by CNS will remover the endpoint from state file
 	if len(epInfos) == 0 {
 		endpointID := plugin.nm.GetEndpointID(args.ContainerID, args.IfName)
 		if !nwCfg.MultiTenancy {
@@ -1122,7 +1133,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
 		if err = plugin.nm.DeleteEndpoint(epInfo.NetworkID, epInfo.EndpointID, epInfo); err != nil {
 			// An error will not be returned if the endpoint is not found
 			// return a retriable error so the container runtime will retry this DEL later
-			// the implementation of this function returns nil if the endpoint doens't exist, so
+			// the implementation of this function returns nil if the endpoint doesn't exist, so
 			// we don't have to check that here
 			return plugin.RetriableError(fmt.Errorf("failed to delete endpoint: %w", err))
 		}

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(nsc NamespaceClientInterface) error {
+	secondaryepClient := NewSecondaryEndpointClient(nil, 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(_ 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,14 @@ func generateCNSIPInfoMap(eps []*endpoint) map[string]*restserver.IPInfo {
 
 	return ifNametoIPInfoMap
 }
+
+// RemoveSecondaryEndpointFromPodNetNS removes the secondary endpoint from the pod netns
+func (nm *networkManager) RemoveSecondaryEndpointFromPodNetNS(ifName, 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.nsClient)
+	return err
+}

network/manager_mock.go

@@ -221,3 +221,7 @@ func (nm *MockNetworkManager) GetEndpointInfosFromContainerID(containerID string
 func (nm *MockNetworkManager) GetEndpointState(_, _ string) ([]*EndpointInfo, error) {
 	return []*EndpointInfo{}, nil
 }
+
+func (nm *MockNetworkManager) RemoveSecondaryEndpointFromPodNetNS(_, _ string) error {
+	return nil
+}

network/secondary_endpoint_client_linux.go

@@ -13,6 +13,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,13 +191,13 @@ 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 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 Stateful 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 delegated NIC
 	for iface := range ep.SecondaryInterfaces {
 		if err := client.moveInterfaceToHostNetns(iface, vmns); err != nil {
 			logger.Error("Failed to move interface", zap.String("IfName", iface), zap.Error(newErrorSecondaryEndpointClient(err)))
@@ -216,3 +217,61 @@ func (client *SecondaryEndpointClient) moveInterfaceToHostNetns(ifName string, v
 	}
 	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, 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)))
+		}
+	}()
+	// Use the existing netlink interface to list links
+	links, err := vishnetlink.LinkList()
+	if err != nil {
+		return newErrorSecondaryEndpointClient(err)
+	}
+
+	ifnames := make([]string, 0, len(links))
+	for _, l := range links {
+		ifnames = append(ifnames, l.Attrs().Name)
+	}
+	logger.Info("Found interfaces in netns that needs to be moved back to host", zap.Any("interfaces", ifnames))
+	// For stateless cni linux, check if delegated vmnic type, and if so, delete using this *endpoint* struct's ifname
+	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
+}