diff --git a/cmd/main.go b/cmd/main.go index db7b4b2..3fbbe83 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -112,7 +112,7 @@ func deleteResources(args *skel.CmdArgs, netConf *evpngwtypes.NetConf) error { }() // Release VF from Pods namespace and rename it to the original name - err = sm.ReleaseVF(netConf, netns, args.Netns) + err = sm.ReleaseVF(netConf, netns) if err != nil { return fmt.Errorf("deleteResources() error releasing VF: %q", err) } diff --git a/pkg/sriov/sriov.go b/pkg/sriov/sriov.go index 4a90ac5..736b8dd 100644 --- a/pkg/sriov/sriov.go +++ b/pkg/sriov/sriov.go @@ -18,6 +18,7 @@ import ( evpngwtypes "github.com/opiproject/opi-gateway-evpn-cni/pkg/types" "github.com/opiproject/opi-gateway-evpn-cni/pkg/utils" + "github.com/vishvananda/netlink" ) type pciUtils interface { @@ -48,7 +49,7 @@ func (p *pciUtilsImpl) EnableArpAndNdiscNotify(ifName string) error { // Manager provides interface invoke sriov nic related operations type Manager interface { SetupVF(conf *evpngwtypes.NetConf, podifName string, netns ns.NetNS) (string, error) - ReleaseVF(conf *evpngwtypes.NetConf, netns ns.NetNS, netNSPath string) error + ReleaseVF(conf *evpngwtypes.NetConf, netns ns.NetNS) error ResetVFConfig(conf *evpngwtypes.NetConf) error ResetVF(conf *evpngwtypes.NetConf) error ApplyVFConfig(conf *evpngwtypes.NetConf) error @@ -73,6 +74,7 @@ func (s *sriovManager) SetupVF(conf *evpngwtypes.NetConf, podifName string, netn ctx := context.Background() linkName := conf.OrigVfState.HostIFName + conf.TotalIntfNames = append(conf.TotalIntfNames, linkName) linkObj, err := s.nLink.LinkByName(ctx, linkName) if err != nil { @@ -92,6 +94,8 @@ func (s *sriovManager) SetupVF(conf *evpngwtypes.NetConf, podifName string, netn return "", fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName) } + conf.TotalIntfNames = append(conf.TotalIntfNames, tempName) + macAddress := linkObj.Attrs().HardwareAddr.String() if conf.MAC != "" { fmt.Printf("SetupVF(): MAC address configuration functionality is not supported currently") @@ -139,6 +143,8 @@ func (s *sriovManager) SetupVF(conf *evpngwtypes.NetConf, podifName string, netn return fmt.Errorf("error setting container interface name %s for %s", linkName, tempName) } + conf.TotalIntfNames = append(conf.TotalIntfNames, podifName) + // 6. Enable IPv4 ARP notify and IPv6 Network Discovery notify // Error is ignored here because enabling this feature is only a performance enhancement. _ = s.utils.EnableArpAndNdiscNotify(podifName) @@ -157,7 +163,9 @@ func (s *sriovManager) SetupVF(conf *evpngwtypes.NetConf, podifName string, netn } // ReleaseVF reset a VF from Pod netns and return it to init netns -func (s *sriovManager) ReleaseVF(conf *evpngwtypes.NetConf, netns ns.NetNS, netNSPath string) error { +func (s *sriovManager) ReleaseVF(conf *evpngwtypes.NetConf, netns ns.NetNS) error { + var linkObj netlink.Link + var podifName string ctx := context.Background() initns, err := ns.GetCurrentNS() @@ -165,24 +173,29 @@ func (s *sriovManager) ReleaseVF(conf *evpngwtypes.NetConf, netns ns.NetNS, netN return fmt.Errorf("ReleaseVF(): failed to get init netns: %v", err) } - // get VF netdevice from PCI that is attached to container. This is executed on the host namespace accessing - // the containers filesystem through the /proc/ path on the host. - vfNetdevices, err := utils.GetContainerNetDevFromPci(netNSPath, conf.DeviceID) - if err != nil { - return fmt.Errorf("ReleaseVF(): failed to get VF netdevice from PCI %s : %v", conf.DeviceID, err) - } - - if len(vfNetdevices) == 0 { - // The VF has not been found in the Container namespace so no point to continue + if len(conf.TotalIntfNames) == 0 { + // No SetupVF action has been executed as the slice is empty. + // There is no point to continue further return nil } - podifName := vfNetdevices[0] return netns.Do(func(_ ns.NetNS) error { // get VF device - linkObj, err := s.nLink.LinkByName(ctx, podifName) - if err != nil { - return fmt.Errorf("ReleaseVF(): failed to get netlink device with name %s: %q", podifName, err) + for _, ifName := range conf.TotalIntfNames { + tempLinkObj, err := s.nLink.LinkByName(ctx, ifName) + if err != nil { + continue + } + linkObj = tempLinkObj + podifName = ifName + break + } + + if linkObj == nil { + // The VF has not been found in the container's namespace so no point to continue. + // This is according to the idempotent logic where if something is not found + // then is considered that is not an error. + return nil } // shutdown VF device @@ -448,6 +461,12 @@ func (s *sriovManager) ResetVF(conf *evpngwtypes.NetConf) error { } // shutdown VF device + // Although this call is executed successfully + // sometimes the VF is not going down. That makes + // the system to report a "resource is busy" error when + // we try to rename the interface further below. + // This error is not fatal and the interface gets renamed correctly + // so we will not do anything to address the issue currently. if err = s.nLink.LinkSetDown(ctx, linkObj); err != nil { return fmt.Errorf("ResetVF(): failed to set link %s down: %q", curNetVFName, err) } diff --git a/pkg/types/types.go b/pkg/types/types.go index 4f3b078..f9094a9 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -41,8 +41,9 @@ type NetConf struct { LogicalBridge string `json:"logical_bridge,omitempty"` LogicalBridges []string `json:"logical_bridges,omitempty"` PortType string - BridgePortName string // Stores the "name" of the created Bridge Port - DeviceID string `json:"deviceID"` // PCI address of a VF in valid sysfs format + BridgePortName string // Stores the "name" of the created Bridge Port + TotalIntfNames []string // Stores all the possible "names" of the interface + DeviceID string `json:"deviceID"` // PCI address of a VF in valid sysfs format VFID int MinTxRate *int `json:"min_tx_rate"` // Mbps, 0 = disable rate limiting (XPU Not supported) MaxTxRate *int `json:"max_tx_rate"` // Mbps, 0 = disable rate limiting (XPU Not supported)