Skip to content

Commit

Permalink
Add function EnableHwTcOffload to the network pkg
Browse files Browse the repository at this point in the history
This function ensures that hw-tc-offload
feature is enabled for the interface if the interface
supports it.

relies on github.com/safchain/ethtool lib

Signed-off-by: Yury Kulazhenkov <[email protected]>
  • Loading branch information
ykulazhenkov committed Feb 26, 2024
1 parent 0d51338 commit d00c856
Show file tree
Hide file tree
Showing 19 changed files with 1,688 additions and 3 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ require (
github.com/openshift/client-go v0.0.0-20220831193253-4950ae70c8ea
github.com/openshift/machine-config-operator v0.0.1-0.20230118083703-fc27a2bdaa85
github.com/pkg/errors v0.9.1
github.com/safchain/ethtool v0.3.0
github.com/spf13/cobra v1.7.0
github.com/stretchr/testify v1.8.4
github.com/vishvananda/netlink v1.2.1-beta.2.0.20240221172127-ec7bcb248e94
Expand Down
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,8 @@ github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjR
github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog=
github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/safchain/ethtool v0.3.0 h1:gimQJpsI6sc1yIqP/y8GYgiXn/NjgvpM0RNoWLVVmP0=
github.com/safchain/ethtool v0.3.0/go.mod h1:SA9BwrgyAqNo7M+uaL6IYbxpm5wk3L7Mm6ocLW+CJUs=
github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
github.com/shopspring/decimal v1.2.0 h1:abSATXmQEYyShuxI4/vyW3tV1MrKAJzCZ/0zLUXYbsQ=
Expand Down Expand Up @@ -625,6 +627,7 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y=
golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
Expand Down
14 changes: 14 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.

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

import (
"github.com/safchain/ethtool"
)

func New() EthtoolLib {
return &libWrapper{}
}

//go:generate ../../../../../bin/mockgen -destination mock/mock_ethtool.go -source ethtool.go
type EthtoolLib interface {
// Features retrieves features of the given interface name.
Features(ifaceName string) (map[string]bool, error)
// FeatureNames shows supported features by their name.
FeatureNames(ifaceName string) (map[string]uint, error)
// Change requests a change in the given device's features.
Change(ifaceName string, config map[string]bool) error
}

type libWrapper struct{}

// Features retrieves features of the given interface name.
func (w *libWrapper) Features(ifaceName string) (map[string]bool, error) {
e, err := ethtool.NewEthtool()
if err != nil {
return nil, err
}
defer e.Close()
return e.Features(ifaceName)
}

// FeatureNames shows supported features by their name.
func (w *libWrapper) FeatureNames(ifaceName string) (map[string]uint, error) {
e, err := ethtool.NewEthtool()
if err != nil {
return nil, err
}
defer e.Close()
return e.FeatureNames(ifaceName)
}

// Change requests a change in the given device's features.
func (w *libWrapper) Change(ifaceName string, config map[string]bool) error {
e, err := ethtool.NewEthtool()
if err != nil {
return err
}
defer e.Close()
return e.Change(ifaceName, config)
}
78 changes: 78 additions & 0 deletions pkg/host/internal/lib/ethtool/mock/mock_ethtool.go

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

45 changes: 44 additions & 1 deletion pkg/host/internal/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
dputilsPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/dputils"
ethtoolPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/ethtool"
netlinkPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/netlink"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
Expand All @@ -24,13 +25,15 @@ type network struct {
utilsHelper utils.CmdInterface
dputilsLib dputilsPkg.DPUtilsLib
netlinkLib netlinkPkg.NetlinkLib
ethtoolLib ethtoolPkg.EthtoolLib
}

func New(utilsHelper utils.CmdInterface, dputilsLib dputilsPkg.DPUtilsLib, netlinkLib netlinkPkg.NetlinkLib) types.NetworkInterface {
func New(utilsHelper utils.CmdInterface, dputilsLib dputilsPkg.DPUtilsLib, netlinkLib netlinkPkg.NetlinkLib, ethtoolLib ethtoolPkg.EthtoolLib) types.NetworkInterface {
return &network{
utilsHelper: utilsHelper,
dputilsLib: dputilsLib,
netlinkLib: netlinkLib,
ethtoolLib: ethtoolLib,
}
}

Expand Down Expand Up @@ -286,3 +289,43 @@ func (n *network) SetDevlinkDeviceParam(pciAddr, paramName, value string) error
}
return nil
}

// EnableHwTcOffload makes sure that hw-tc-offload feature is enabled if device supports it
func (n *network) EnableHwTcOffload(ifaceName string) error {
log.Log.V(2).Info("EnableHwTcOffload(): enable offloading", "device", ifaceName)
hwTcOffloadFeatureName := "hw-tc-offload"

knownFeatures, err := n.ethtoolLib.FeatureNames(ifaceName)
if err != nil {
log.Log.Error(err, "EnableHwTcOffload(): can't list supported features", "device", ifaceName)
return err
}
if _, isKnown := knownFeatures[hwTcOffloadFeatureName]; !isKnown {
log.Log.V(2).Info("EnableHwTcOffload(): can't enable feature, feature is not supported", "device", ifaceName)
return nil
}
currentFeaturesState, err := n.ethtoolLib.Features(ifaceName)
if err != nil {
log.Log.Error(err, "EnableHwTcOffload(): can't read features state for device", "device", ifaceName)
return err
}
if currentFeaturesState[hwTcOffloadFeatureName] {
log.Log.V(2).Info("EnableHwTcOffload(): already enabled", "device", ifaceName)
return nil
}
if err := n.ethtoolLib.Change(ifaceName, map[string]bool{hwTcOffloadFeatureName: true}); err != nil {
log.Log.Error(err, "EnableHwTcOffload(): can't set feature for device", "device", ifaceName)
return err
}
updatedFeaturesState, err := n.ethtoolLib.Features(ifaceName)
if err != nil {
log.Log.Error(err, "EnableHwTcOffload(): can't read features state for device", "device", ifaceName)
return err
}
if updatedFeaturesState[hwTcOffloadFeatureName] {
log.Log.V(2).Info("EnableHwTcOffload(): feature enabled", "device", ifaceName)
return nil
}
log.Log.V(2).Info("EnableHwTcOffload(): feature is still disabled, not supported by device", "device", ifaceName)
return nil
}
52 changes: 51 additions & 1 deletion pkg/host/internal/network/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

hostMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/helper/mock"
dputilsMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/dputils/mock"
ethtoolMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/ethtool/mock"
netlinkMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/netlink/mock"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types"
)
Expand All @@ -29,6 +30,7 @@ var _ = Describe("Network", func() {
var (
n types.NetworkInterface
netlinkLibMock *netlinkMockPkg.MockNetlinkLib
ethtoolLibMock *ethtoolMockPkg.MockEthtoolLib
dputilsLibMock *dputilsMockPkg.MockDPUtilsLib
hostMock *hostMockPkg.MockHostHelpersInterface

Expand All @@ -38,10 +40,11 @@ var _ = Describe("Network", func() {
BeforeEach(func() {
testCtrl = gomock.NewController(GinkgoT())
netlinkLibMock = netlinkMockPkg.NewMockNetlinkLib(testCtrl)
ethtoolLibMock = ethtoolMockPkg.NewMockEthtoolLib(testCtrl)
dputilsLibMock = dputilsMockPkg.NewMockDPUtilsLib(testCtrl)
hostMock = hostMockPkg.NewMockHostHelpersInterface(testCtrl)

n = New(hostMock, dputilsLibMock, netlinkLibMock)
n = New(hostMock, dputilsLibMock, netlinkLibMock, ethtoolLibMock)
})

AfterEach(func() {
Expand Down Expand Up @@ -152,4 +155,51 @@ var _ = Describe("Network", func() {
Expect(err).To(HaveOccurred())
})
})
Context("EnableHwTcOffload", func() {
It("Enabled", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(map[string]uint{"hw-tc-offload": 42}, nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(map[string]bool{"hw-tc-offload": false}, nil)
ethtoolLibMock.EXPECT().Change("enp216s0f0np0", map[string]bool{"hw-tc-offload": true}).Return(nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(map[string]bool{"hw-tc-offload": true}, nil)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).NotTo(HaveOccurred())
})
It("Feature unknown", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(map[string]uint{}, nil)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).NotTo(HaveOccurred())
})
It("Already enabled", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(map[string]uint{"hw-tc-offload": 42}, nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(map[string]bool{"hw-tc-offload": true}, nil)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).NotTo(HaveOccurred())
})
It("not supported", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(map[string]uint{"hw-tc-offload": 42}, nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(map[string]bool{"hw-tc-offload": false}, nil)
ethtoolLibMock.EXPECT().Change("enp216s0f0np0", map[string]bool{"hw-tc-offload": true}).Return(nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(map[string]bool{"hw-tc-offload": false}, nil)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).NotTo(HaveOccurred())
})
It("fail - can't list supported", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(nil, testErr)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).To(MatchError(testErr))
})
It("fail - can't get features", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(map[string]uint{"hw-tc-offload": 42}, nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(nil, testErr)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).To(MatchError(testErr))
})
It("fail - can't change features", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(map[string]uint{"hw-tc-offload": 42}, nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(map[string]bool{"hw-tc-offload": false}, nil)
ethtoolLibMock.EXPECT().Change("enp216s0f0np0", map[string]bool{"hw-tc-offload": true}).Return(testErr)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).To(MatchError(testErr))
})
It("fail - can't reread features", func() {
ethtoolLibMock.EXPECT().FeatureNames("enp216s0f0np0").Return(map[string]uint{"hw-tc-offload": 42}, nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(map[string]bool{"hw-tc-offload": false}, nil)
ethtoolLibMock.EXPECT().Change("enp216s0f0np0", map[string]bool{"hw-tc-offload": true}).Return(nil)
ethtoolLibMock.EXPECT().Features("enp216s0f0np0").Return(nil, testErr)
Expect(n.EnableHwTcOffload("enp216s0f0np0")).To(MatchError(testErr))
})
})
})
4 changes: 3 additions & 1 deletion pkg/host/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package host
import (
"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"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/lib/netlink"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/network"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/service"
Expand Down Expand Up @@ -38,8 +39,9 @@ type hostManager struct {
func NewHostManager(utilsInterface utils.CmdInterface) HostManagerInterface {
dpUtils := dputils.New()
netlinkLib := netlink.New()
ethtoolLib := ethtool.New()
k := kernel.New(utilsInterface)
n := network.New(utilsInterface, dpUtils, netlinkLib)
n := network.New(utilsInterface, dpUtils, netlinkLib, ethtoolLib)
sv := service.New(utilsInterface)
u := udev.New(utilsInterface)
sr := sriov.New(utilsInterface, k, n, u, netlinkLib, dpUtils)
Expand Down
14 changes: 14 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.

2 changes: 2 additions & 0 deletions pkg/host/types/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ type NetworkInterface interface {
// as a string. Automatically set CMODE for the parameter and converts the value to the right
// type before submitting it.
SetDevlinkDeviceParam(pciAddr, paramName, value string) error
// EnableHwTcOffload make sure that hw-tc-offload feature is enabled if device supports it
EnableHwTcOffload(ifaceName string) error
}

type ServiceInterface interface {
Expand Down
Loading

0 comments on commit d00c856

Please sign in to comment.