Skip to content

Commit

Permalink
Create instance of IB interface in the host object
Browse files Browse the repository at this point in the history 
Signed-off-by: amaslennikov <[email protected]>
  • Loading branch information
@almaslennikov
almaslennikov committed Jul 23, 2024
1 parent 0022c10 commit 2886ca6
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 39 deletions.
6 changes: 5 additions & 1 deletion pkg/helper/host.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ func NewHostHelpers(utilsHelper utils.CmdInterface,
func NewDefaultHostHelpers() (HostHelpersInterface, error) {
utilsHelper := utils.New()
mlxHelper := mlx.New(utilsHelper)
hostManager := host.NewHostManager(utilsHelper)
hostManager, err := host.NewHostManager(utilsHelper)
if err != nil {
log.Log.Error(err, "failed to create host manager")
return nil, err
}
storeManager, err := store.NewManager()
if err != nil {
log.Log.Error(err, "failed to create store manager")
Expand Down
14 changes: 14 additions & 0 deletions pkg/helper/mock/mock_helper.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 20 additions & 17 deletions pkg/host/internal/sriov/sriov.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,16 @@ type interfaceToConfigure struct {
}

type sriov struct {
utilsHelper utils.CmdInterface
kernelHelper types.KernelInterface
networkHelper types.NetworkInterface
udevHelper types.UdevInterface
vdpaHelper types.VdpaInterface
utilsHelper utils.CmdInterface
kernelHelper types.KernelInterface
networkHelper types.NetworkInterface
udevHelper types.UdevInterface
vdpaHelper types.VdpaInterface
infinibandHelper types.InfinibandInterface
netlinkLib netlinkPkg.NetlinkLib
dputilsLib dputilsPkg.DPUtilsLib
sriovnetLib sriovnetPkg.SriovnetLib
ghwLib ghwPkg.GHWLib
netlinkLib netlinkPkg.NetlinkLib
dputilsLib dputilsPkg.DPUtilsLib
sriovnetLib sriovnetPkg.SriovnetLib
ghwLib ghwPkg.GHWLib
}

func New(utilsHelper utils.CmdInterface,
Expand All @@ -56,15 +56,15 @@ func New(utilsHelper utils.CmdInterface,
sriovnetLib sriovnetPkg.SriovnetLib,
ghwLib ghwPkg.GHWLib) types.SriovInterface {
return &sriov{utilsHelper: utilsHelper,
kernelHelper: kernelHelper,
networkHelper: networkHelper,
udevHelper: udevHelper,
vdpaHelper: vdpaHelper,
kernelHelper: kernelHelper,
networkHelper: networkHelper,
udevHelper: udevHelper,
vdpaHelper: vdpaHelper,
infinibandHelper: infinibandHelper,
netlinkLib: netlinkLib,
dputilsLib: dputilsLib,
sriovnetLib: sriovnetLib,
ghwLib: ghwLib,
netlinkLib: netlinkLib,
dputilsLib: dputilsLib,
sriovnetLib: sriovnetLib,
ghwLib: ghwLib,
}
}

Expand Down Expand Up @@ -476,6 +476,9 @@ func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error {
if err := s.infinibandHelper.ConfigureVfGUID(addr, iface.PciAddress, vfID, pfLink); err != nil {
return err
}
if err := s.kernelHelper.Unbind(iface.PciAddress); err != nil {

This comment has been minimized.

Sign in to view

Copy link
@popsiclexu

popsiclexu Oct 30, 2024

Hi there,

I wanted to understand the reasoning behind unbinding the driver in this context. After performing the unbind operation, we encountered an issue where we could no longer retrieve the default driver for the virtual function (VF). This caused a problem in our setup. Could you please explain why the unbind is necessary and if there’s a way to address this issue?

Thanks for any insights you can provide!

This comment has been minimized.

Sign in to view

Copy link
@adrianchiris

adrianchiris Oct 30, 2024

Collaborator

unbind (and later on bind) is need for applying GUID configuration for the VF. currently to my knowledge there is no way in kernel to do that without unbind/bind.
return err
}
} else {
vfLink, err := s.VFIsReady(addr)
if err != nil {
Expand Down
22 changes: 6 additions & 16 deletions pkg/host/internal/sriov/sriov_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ var _ = Describe("SRIOV", func() {
ghwLibMock *ghwMockPkg.MockGHWLib
hostMock *hostMockPkg.MockHostManagerInterface
storeManagerMode *hostStoreMockPkg.MockManagerInterface

testCtrl *gomock.Controller
testCtrl *gomock.Controller

testError = fmt.Errorf("test")
)
Expand All @@ -50,7 +49,7 @@ var _ = Describe("SRIOV", func() {
hostMock = hostMockPkg.NewMockHostManagerInterface(testCtrl)
storeManagerMode = hostStoreMockPkg.NewMockManagerInterface(testCtrl)

s = New(nil, hostMock, hostMock, hostMock, hostMock, nil, netlinkLibMock, dputilsLibMock, sriovnetLibMock, ghwLibMock)
s = New(nil, hostMock, hostMock, hostMock, hostMock, hostMock, netlinkLibMock, dputilsLibMock, sriovnetLibMock, ghwLibMock)
})

AfterEach(func() {
Expand Down Expand Up @@ -227,7 +226,6 @@ var _ = Describe("SRIOV", func() {
pfLinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{Flags: 0, EncapType: "ether"})
netlinkLibMock.EXPECT().IsLinkAdminStateUp(pfLinkMock).Return(false)
netlinkLibMock.EXPECT().LinkSetUp(pfLinkMock).Return(nil)
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).Times(0)

dputilsLibMock.EXPECT().GetVFID("0000:d8:00.2").Return(0, nil).Times(2)
hostMock.EXPECT().HasDriver("0000:d8:00.2").Return(false, "")
Expand Down Expand Up @@ -296,16 +294,14 @@ var _ = Describe("SRIOV", func() {
netlinkLibMock.EXPECT().IsLinkAdminStateUp(pfLinkMock).Return(false)
netlinkLibMock.EXPECT().LinkSetUp(pfLinkMock).Return(nil)

dputilsLibMock.EXPECT().GetVFID("0000:d8:00.2").Return(0, nil).Times(2)
hostMock.EXPECT().Unbind("0000:d8:00.2").Return(nil)
dputilsLibMock.EXPECT().GetVFID("0000:d8:00.2").Return(0, nil).Times(1)
hostMock.EXPECT().HasDriver("0000:d8:00.2").Return(true, "test").Times(2)
hostMock.EXPECT().UnbindDriverIfNeeded("0000:d8:00.2", true).Return(nil)
hostMock.EXPECT().BindDefaultDriver("0000:d8:00.2").Return(nil)
hostMock.EXPECT().SetNetdevMTU("0000:d8:00.2", 2000).Return(nil)
vf0LinkMock := netlinkMockPkg.NewMockLink(testCtrl)
netlinkLibMock.EXPECT().LinkSetVfNodeGUID(vf0LinkMock, 0, gomock.Any()).Return(nil)
netlinkLibMock.EXPECT().LinkSetVfPortGUID(vf0LinkMock, 0, gomock.Any()).Return(nil)
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).AnyTimes()
hostMock.EXPECT().ConfigureVfGUID(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)

hostMock.EXPECT().Unbind(gomock.Any()).Return(nil).Times(1)

storeManagerMode.EXPECT().SaveLastPfAppliedStatus(gomock.Any()).Return(nil)

Expand Down Expand Up @@ -372,7 +368,6 @@ var _ = Describe("SRIOV", func() {
hostMock.EXPECT().AddVfRepresentorUdevRule("0000:d8:00.0", "enp216s0f0np0", "7cfe90ff2cc0", "p0").Return(nil)
hostMock.EXPECT().CreateVDPADevice("0000:d8:00.2", "vhost_vdpa")
hostMock.EXPECT().LoadUdevRules().Return(nil)
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).AnyTimes()

storeManagerMode.EXPECT().SaveLastPfAppliedStatus(gomock.Any()).Return(nil)

Expand Down Expand Up @@ -400,7 +395,6 @@ var _ = Describe("SRIOV", func() {

It("externally managed - wrong VF count", func() {
dputilsLibMock.EXPECT().GetVFconfigured("0000:d8:00.0").Return(0)
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).AnyTimes()

Expect(s.ConfigSriovInterfaces(storeManagerMode,
[]sriovnetworkv1.Interface{{
Expand All @@ -426,7 +420,6 @@ var _ = Describe("SRIOV", func() {
netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return(
&netlink.DevlinkDevice{Attrs: netlink.DevlinkDevAttrs{Eswitch: netlink.DevlinkDevEswitchAttr{Mode: "legacy"}}},
nil)
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).AnyTimes()

hostMock.EXPECT().GetNetdevMTU("0000:d8:00.0")
Expect(s.ConfigSriovInterfaces(storeManagerMode,
Expand Down Expand Up @@ -463,7 +456,6 @@ var _ = Describe("SRIOV", func() {
netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return(
&netlink.DevlinkDevice{Attrs: netlink.DevlinkDevAttrs{Eswitch: netlink.DevlinkDevEswitchAttr{Mode: "legacy"}}},
nil)
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).AnyTimes()
hostMock.EXPECT().RemoveDisableNMUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().RemovePersistPFNameUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().RemoveVfRepresentorUdevRule("0000:d8:00.0").Return(nil)
Expand All @@ -482,7 +474,6 @@ var _ = Describe("SRIOV", func() {
helpers.GinkgoAssertFileContentsEquals("/sys/bus/pci/devices/0000:d8:00.0/sriov_numvfs", "0")
})
It("reset device - skip external", func() {
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).AnyTimes()
storeManagerMode.EXPECT().LoadPfsStatus("0000:d8:00.0").Return(&sriovnetworkv1.Interface{
Name: "enp216s0f0np0",
PciAddress: "0000:d8:00.0",
Expand Down Expand Up @@ -511,7 +502,6 @@ var _ = Describe("SRIOV", func() {
netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return(
&netlink.DevlinkDevice{Attrs: netlink.DevlinkDevAttrs{Eswitch: netlink.DevlinkDevEswitchAttr{Mode: "legacy"}}},
nil)
netlinkLibMock.EXPECT().LinkList().Return(nil, nil).AnyTimes()
hostMock.EXPECT().RemoveDisableNMUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().RemovePersistPFNameUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().RemoveVfRepresentorUdevRule("0000:d8:00.0").Return(nil)
Expand Down
14 changes: 11 additions & 3 deletions pkg/host/manager.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package host

import (
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/infiniband"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/kernel"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/dputils"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/ethtool"
Expand All @@ -26,6 +27,7 @@ type HostManagerInterface interface {
types.UdevInterface
types.SriovInterface
types.VdpaInterface
types.InfinibandInterface
}

type hostManager struct {
Expand All @@ -36,9 +38,10 @@ type hostManager struct {
types.UdevInterface
types.SriovInterface
types.VdpaInterface
types.InfinibandInterface
}

func NewHostManager(utilsInterface utils.CmdInterface) HostManagerInterface {
func NewHostManager(utilsInterface utils.CmdInterface) (HostManagerInterface, error) {
dpUtils := dputils.New()
netlinkLib := netlink.New()
ethtoolLib := ethtool.New()
Expand All @@ -49,7 +52,11 @@ func NewHostManager(utilsInterface utils.CmdInterface) HostManagerInterface {
sv := service.New(utilsInterface)
u := udev.New(utilsInterface)
v := vdpa.New(k, netlinkLib)
sr := sriov.New(utilsInterface, k, n, u, v, netlinkLib, dpUtils, sriovnetLib, ghwLib)
ib, err := infiniband.New(netlinkLib, k, n)
if err != nil {
return nil, err
}
sr := sriov.New(utilsInterface, k, n, u, v, ib, netlinkLib, dpUtils, sriovnetLib, ghwLib)

return &hostManager{
utilsInterface,
Expand All @@ -59,5 +66,6 @@ func NewHostManager(utilsInterface utils.CmdInterface) HostManagerInterface {
u,
sr,
v,
}
ib,
}, nil
}
14 changes: 14 additions & 0 deletions pkg/host/mock/mock_host.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion pkg/platforms/platforms.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package platforms

import (
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/openshift"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/openstack"
Expand All @@ -24,7 +26,11 @@ func NewDefaultPlatformHelper() (Interface, error) {
return nil, err
}
utilsHelper := utils.New()
hostManager := host.NewHostManager(utilsHelper)
hostManager, err := host.NewHostManager(utilsHelper)
if err != nil {
log.Log.Error(err, "failed to create host manager")
return nil, err
}
openstackContext := openstack.New(hostManager)

return &platformHelper{
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/k8s/k8s_plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ var _ = Describe("K8s plugin", func() {
testCtrl = gomock.NewController(GinkgoT())

hostHelper = mock_helper.NewMockHostHelpersInterface(testCtrl)
realHostMgr := host.NewHostManager(hostHelper)
realHostMgr, _ := host.NewHostManager(hostHelper)

// proxy some functions to real host manager to simplify testing and to additionally validate manifests
for _, f := range []string{
Expand Down

0 comments on commit 2886ca6

Please sign in to comment.