Skip to content

Commit

Permalink
kernel: Set arguments based on CPU architecture
Browse files Browse the repository at this point in the history
Kernel arguments like `intel_iommu=on` does not have sense
on AMD or ARM systems and some user might complain about
their presence, though they are likely to be harmless.

Also, on ARM systems the `iommu.passthrough` parameter is the
one to use [1].

Improve `GHWLib` to bridge CPU information from the library.
Add `CpuInfoProviderInterface` and inject it into the GenericPlugin
to implement the per CPU vendor logic.

[1] https://github.com/torvalds/linux/blob/master/Documentation/admin-guide/kernel-parameters.txt#L2343

Signed-off-by: Andrea Panattoni <[email protected]>
  • Loading branch information
zeeke committed Oct 25, 2024
1 parent 68b6c02 commit b26db1b
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 12 deletions.
1 change: 1 addition & 0 deletions pkg/consts/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ const (
KernelArgPciRealloc = "pci=realloc"
KernelArgIntelIommu = "intel_iommu=on"
KernelArgIommuPt = "iommu=pt"
KernelArgIommuPassthrough = "iommu.passthrough=1"

// Feature gates
// ParallelNicConfigFeatureGate: allow to configure nics in parallel
Expand Down
15 changes: 15 additions & 0 deletions pkg/helper/mock/mock_helper.go

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

42 changes: 42 additions & 0 deletions pkg/host/internal/cpu/cpu.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package cpu

import (
"fmt"

ghwPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/ghw"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types"
)


type cpuInfoProvider struct {
ghwLib ghwPkg.GHWLib
}

func New(ghwLib ghwPkg.GHWLib) *cpuInfoProvider {
return &cpuInfoProvider{
ghwLib: ghwLib,
}
}


func (c *cpuInfoProvider) GetCPUVendor() (types.CPUVendor, error) {
cpuInfo, err := c.ghwLib.CPU()
if err != nil {
return -1, fmt.Errorf("can't retrieve the CPU vendor: %w", err)
}

if len(cpuInfo.Processors) == 0 {
return -1, fmt.Errorf("wrong CPU information retrieved: %v", cpuInfo)
}

switch (cpuInfo.Processors[0].Vendor) {
case "GenuineIntel":
return types.CPUVendorIntel, nil
case "AuthenticAMD":
return types.CPUVendorAMD, nil
case "ARM":
return types.CPUVendorARM, nil
}

return -1, fmt.Errorf("unknown CPU vendor: %s", cpuInfo.Processors[0].Vendor)
}
27 changes: 27 additions & 0 deletions pkg/host/internal/cpu/cpu_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package cpu

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"go.uber.org/zap/zapcore"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)



var _ = Describe("Cpu", func() {
// TODO
})


func TestCpu(t *testing.T) {
log.SetLogger(zap.New(
zap.WriteTo(GinkgoWriter),
zap.Level(zapcore.Level(-2)),
zap.UseDevMode(true)))
RegisterFailHandler(Fail)
RunSpecs(t, "Package CPU Suite")
}
8 changes: 8 additions & 0 deletions pkg/host/internal/lib/ghw/ghw.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ghw

import (
"github.com/jaypipes/ghw"
"github.com/jaypipes/ghw/pkg/cpu"
)

func New() GHWLib {
Expand All @@ -12,6 +13,9 @@ func New() GHWLib {
type GHWLib interface {
// PCI returns a pointer to an Info that provide methods to access info about devices
PCI() (Info, error)

// CPU returns a pointer to an Info that provide methods to access info about devices
CPU() (*cpu.Info, error)
}

// Info interface provide methods to access info about devices
Expand All @@ -27,3 +31,7 @@ type libWrapper struct{}
func (w *libWrapper) PCI() (Info, error) {
return ghw.PCI()
}

func (w *libWrapper) CPU() (*cpu.Info, error) {
return ghw.CPU()
}
16 changes: 16 additions & 0 deletions pkg/host/internal/lib/ghw/mock/mock_ghw.go

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

5 changes: 5 additions & 0 deletions pkg/host/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package host

import (
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/bridge"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/cpu"
"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"
Expand Down Expand Up @@ -30,6 +31,7 @@ type HostManagerInterface interface {
types.VdpaInterface
types.InfinibandInterface
types.BridgeInterface
types.CpuInfoProviderInterface
}

type hostManager struct {
Expand All @@ -42,6 +44,7 @@ type hostManager struct {
types.VdpaInterface
types.InfinibandInterface
types.BridgeInterface
types.CpuInfoProviderInterface
}

func NewHostManager(utilsInterface utils.CmdInterface) (HostManagerInterface, error) {
Expand All @@ -61,6 +64,7 @@ func NewHostManager(utilsInterface utils.CmdInterface) (HostManagerInterface, er
}
br := bridge.New()
sr := sriov.New(utilsInterface, k, n, u, v, ib, netlinkLib, dpUtils, sriovnetLib, ghwLib, br)
cpuInfoProvider := cpu.New(ghwLib)
return &hostManager{
utilsInterface,
k,
Expand All @@ -71,5 +75,6 @@ func NewHostManager(utilsInterface utils.CmdInterface) (HostManagerInterface, er
v,
ib,
br,
cpuInfoProvider,
}, nil
}
15 changes: 15 additions & 0 deletions pkg/host/mock/mock_host.go

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

13 changes: 13 additions & 0 deletions pkg/host/types/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,16 @@ type InfinibandInterface interface {
// ConfigureVfGUID configures and sets a GUID for an IB VF device
ConfigureVfGUID(vfAddr string, pfAddr string, vfID int, pfLink netlink.Link) error
}

type CPUVendor int

const (
CPUVendorIntel CPUVendor = iota
CPUVendorAMD
CPUVendorARM
)

type CpuInfoProviderInterface interface {
// Retrieve the CPU vendor of the current system
GetCPUVendor() (CPUVendor, error)
}
29 changes: 27 additions & 2 deletions pkg/plugins/generic/generic_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/helper"
hostTypes "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types"
plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
Expand Down Expand Up @@ -419,9 +420,33 @@ func (p *GenericPlugin) shouldConfigureBridges() bool {

func (p *GenericPlugin) addVfioDesiredKernelArg(state *sriovnetworkv1.SriovNetworkNodeState) {
driverState := p.DriverStateMap[Vfio]

kernelArgFnByCPUVendor := map[hostTypes.CPUVendor]func() {
hostTypes.CPUVendorIntel: func() {
p.addToDesiredKernelArgs(consts.KernelArgIntelIommu)
p.addToDesiredKernelArgs(consts.KernelArgIommuPt)
},
hostTypes.CPUVendorAMD: func() {
p.addToDesiredKernelArgs(consts.KernelArgIommuPt)
},
hostTypes.CPUVendorARM: func() {
p.addToDesiredKernelArgs(consts.KernelArgIommuPassthrough)
},
}



if !driverState.DriverLoaded && driverState.NeedDriverFunc(state, driverState) {
p.addToDesiredKernelArgs(consts.KernelArgIntelIommu)
p.addToDesiredKernelArgs(consts.KernelArgIommuPt)
cpuVendor, err := p.helpers.GetCPUVendor()
if err != nil {
log.Log.Error(err, "can't get CPU vendor, falling back to Intel")
cpuVendor = hostTypes.CPUVendorIntel
}

addKernelArgFn := kernelArgFnByCPUVendor[cpuVendor]
if addKernelArgFn != nil {
addKernelArgFn()
}
}
}

Expand Down
48 changes: 38 additions & 10 deletions pkg/plugins/generic/generic_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
mock_helper "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/helper/mock"
hostTypes "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types"
plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
)
Expand Down Expand Up @@ -850,8 +851,10 @@ var _ = Describe("Generic plugin", func() {
Expect(changed).To(BeTrue())
})

It("should detect changes on status due to missing kernel args", func() {
networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{

Context("Kernel Args", func() {

vfioNetworkNodeState := &sriovnetworkv1.SriovNetworkNodeState{
Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{
Interfaces: sriovnetworkv1.Interfaces{{
PciAddress: "0000:00:00.0",
Expand Down Expand Up @@ -896,16 +899,41 @@ var _ = Describe("Generic plugin", func() {
},
}

// Load required kernel args.
genericPlugin.(*GenericPlugin).addVfioDesiredKernelArg(networkNodeState)
It("should detect changes on status due to missing kernel args", func() {
hostHelper.EXPECT().GetCPUVendor().Return(hostTypes.CPUVendorIntel, nil)

hostHelper.EXPECT().GetCurrentKernelArgs().Return("", nil)
hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIntelIommu).Return(false)
hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIommuPt).Return(false)
// Load required kernel args.
genericPlugin.(*GenericPlugin).addVfioDesiredKernelArg(vfioNetworkNodeState)

changed, err := genericPlugin.CheckStatusChanges(networkNodeState)
Expect(err).ToNot(HaveOccurred())
Expect(changed).To(BeTrue())
Expect(genericPlugin.(*GenericPlugin).DesiredKernelArgs).To(Equal(map[string]bool{
consts.KernelArgIntelIommu: false,
consts.KernelArgIommuPt: false,
}))

hostHelper.EXPECT().GetCurrentKernelArgs().Return("", nil)
hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIntelIommu).Return(false)
hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIommuPt).Return(false)

changed, err := genericPlugin.CheckStatusChanges(vfioNetworkNodeState)
Expect(err).ToNot(HaveOccurred())
Expect(changed).To(BeTrue())
})

It("should set the correct kernel args on AMD CPUs", func() {
hostHelper.EXPECT().GetCPUVendor().Return(hostTypes.CPUVendorAMD, nil)
genericPlugin.(*GenericPlugin).addVfioDesiredKernelArg(vfioNetworkNodeState)
Expect(genericPlugin.(*GenericPlugin).DesiredKernelArgs).To(Equal(map[string]bool{
consts.KernelArgIommuPt: false,
}))
})

It("should set the correct kernel args on ARM CPUs", func() {
hostHelper.EXPECT().GetCPUVendor().Return(hostTypes.CPUVendorARM, nil)
genericPlugin.(*GenericPlugin).addVfioDesiredKernelArg(vfioNetworkNodeState)
Expect(genericPlugin.(*GenericPlugin).DesiredKernelArgs).To(Equal(map[string]bool{
consts.KernelArgIommuPassthrough: false,
}))
})
})

It("should load vfio_pci driver", func() {
Expand Down

0 comments on commit b26db1b

Please sign in to comment.