Skip to content

Commit

Permalink
Merge pull request k8snetworkplumbingwg#296 from mlguerrero12/skipvla…
Browse files Browse the repository at this point in the history 
…nnil

Skip setting vlan when parameter is not present
  • Loading branch information
@SchSeba
SchSeba authored May 9, 2024
2 parents 58ffdd3 + 7f6a746 commit 20c67e0
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 37 deletions.
67 changes: 37 additions & 30 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,43 +86,50 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) {
}

if n.Vlan == nil {
vlan := 0
n.Vlan = &vlan
}
// validate non-nil value for vlan qos
if n.VlanQoS != nil {
return nil, fmt.Errorf("LoadConf(): vlan id must be configured to set vlan QoS to a non-nil value")
}

// validate vlan id range
if *n.Vlan < 0 || *n.Vlan > 4094 {
return nil, fmt.Errorf("LoadConf(): vlan id %d invalid: value must be in the range 0-4094", *n.Vlan)
}
// validate non-nil value for vlan proto
if n.VlanProto != nil {
return nil, fmt.Errorf("LoadConf(): vlan id must be configured to set vlan proto to a non-nil value")
}
} else {
// validate vlan id range
if *n.Vlan < 0 || *n.Vlan > 4094 {
return nil, fmt.Errorf("LoadConf(): vlan id %d invalid: value must be in the range 0-4094", *n.Vlan)
}

if n.VlanQoS == nil {
qos := 0
n.VlanQoS = &qos
}
if n.VlanQoS == nil {
qos := 0
n.VlanQoS = &qos
}

// validate that VLAN QoS is in the 0-7 range
if *n.VlanQoS < 0 || *n.VlanQoS > 7 {
return nil, fmt.Errorf("LoadConf(): vlan QoS PCP %d invalid: value must be in the range 0-7", *n.VlanQoS)
}
// validate that VLAN QoS is in the 0-7 range
if *n.VlanQoS < 0 || *n.VlanQoS > 7 {
return nil, fmt.Errorf("LoadConf(): vlan QoS PCP %d invalid: value must be in the range 0-7", *n.VlanQoS)
}

// validate non-zero value for vlan id if vlan qos is set to a non-zero value
if *n.VlanQoS != 0 && *n.Vlan == 0 {
return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan QoS to a non-zero value")
}
// validate non-zero value for vlan id if vlan qos is set to a non-zero value
if *n.VlanQoS != 0 && *n.Vlan == 0 {
return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan QoS to a non-zero value")
}

if n.VlanProto == nil {
proto := sriovtypes.Proto8021q
n.VlanProto = &proto
}
if n.VlanProto == nil {
proto := sriovtypes.Proto8021q
n.VlanProto = &proto
}

*n.VlanProto = strings.ToLower(*n.VlanProto)
if *n.VlanProto != sriovtypes.Proto8021ad && *n.VlanProto != sriovtypes.Proto8021q {
return nil, fmt.Errorf("LoadConf(): vlan Proto %s invalid: value must be '802.1Q' or '802.1ad'", *n.VlanProto)
}
*n.VlanProto = strings.ToLower(*n.VlanProto)
if *n.VlanProto != sriovtypes.Proto8021ad && *n.VlanProto != sriovtypes.Proto8021q {
return nil, fmt.Errorf("LoadConf(): vlan Proto %s invalid: value must be '802.1Q' or '802.1ad'", *n.VlanProto)
}

// validate non-zero value for vlan id if vlan proto is set to 802.1ad
if *n.VlanProto == sriovtypes.Proto8021ad && *n.Vlan == 0 {
return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan proto 802.1ad")
// validate non-zero value for vlan id if vlan proto is set to 802.1ad
if *n.VlanProto == sriovtypes.Proto8021ad && *n.Vlan == 0 {
return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan proto 802.1ad")
}
}

// validate that link state is one of supported values
Expand Down
12 changes: 8 additions & 4 deletions pkg/sriov/sriov.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,10 @@ func (s *sriovManager) ApplyVFConfig(conf *sriovtypes.NetConf) error {
return fmt.Errorf("failed to lookup master %q: %v", conf.Master, err)
}
// 1. Set vlan
if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, *conf.Vlan, *conf.VlanQoS, sriovtypes.VlanProtoInt[*conf.VlanProto]); err != nil {
return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %s: %v", conf.VFID, *conf.Vlan, *conf.VlanQoS, *conf.VlanProto, err)
if conf.Vlan != nil {
if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, *conf.Vlan, *conf.VlanQoS, sriovtypes.VlanProtoInt[*conf.VlanProto]); err != nil {
return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %s: %v", conf.VFID, *conf.Vlan, *conf.VlanQoS, *conf.VlanProto, err)
}
}

// 2. Set mac address
Expand Down Expand Up @@ -355,8 +357,10 @@ func (s *sriovManager) ResetVFConfig(conf *sriovtypes.NetConf) error {
conf.OrigVfState.VlanProto = sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]
}

if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto); err != nil {
return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %d: %v", conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto, err)
if conf.Vlan != nil {
if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto); err != nil {
return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %d: %v", conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto, err)
}
}

// Restore spoofchk
Expand Down
59 changes: 56 additions & 3 deletions pkg/sriov/sriov_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ var _ = Describe("Sriov", func() {
mocked.On("LinkSetName", fakeLink, mock.Anything).Return(nil)
mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil)
mocked.On("LinkSetUp", fakeLink).Return(nil)
mocked.On("LinkSetVfVlan", mock.Anything, mock.AnythingOfType("int"), mock.AnythingOfType("int")).Return(nil)
mocked.On("LinkSetVfVlanQos", mock.Anything, mock.AnythingOfType("int"), mock.AnythingOfType("int"), mock.AnythingOfType("int")).Return(nil)
mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil)
sm := sriovManager{nLink: mocked, utils: mockedPciUtils}
err = sm.SetupVF(netconf, podifName, targetNetNS)
Expand Down Expand Up @@ -237,6 +235,62 @@ var _ = Describe("Sriov", func() {
mocked.AssertExpectations(t)
})
})
Context("Checking ApplyVFConfig function", func() {
var (
netconf *sriovtypes.NetConf
mocked *mocks_utils.NetlinkManager
fakeLink *utils.FakeLink
)

BeforeEach(func() {
netconf = &sriovtypes.NetConf{
Master: "enp175s0f1",
VFID: 0,
}
mocked = &mocks_utils.NetlinkManager{}
fakeLink = &utils.FakeLink{}
})

It("should not call functions to configure the VF when config has no optional parameters", func() {
mocked.On("LinkByName", mock.AnythingOfType("string")).Return(fakeLink, nil)
sm := sriovManager{nLink: mocked}
err := sm.ApplyVFConfig(netconf)
Expect(err).NotTo(HaveOccurred())
})

It("should call functions to configure the VF when config has optional parameters", func() {
vlan := 100
netconf.Vlan = &vlan
qos := 0
netconf.VlanQoS = &qos
vlanProto := "802.1q"
netconf.VlanProto = &vlanProto

hwaddr, err := net.ParseMAC("aa:f3:8d:65:1b:d4")
Expect(err).NotTo(HaveOccurred())

maxTxRate := 4000
minTxRate := 1000
netconf.MaxTxRate = &maxTxRate
netconf.MinTxRate = &minTxRate

netconf.SpoofChk = "on"
netconf.Trust = "on"
netconf.LinkState = "enable"

mocked.On("LinkByName", mock.AnythingOfType("string")).Return(fakeLink, nil)
mocked.On("LinkSetVfVlanQosProto", fakeLink, netconf.VFID, *netconf.Vlan, *netconf.VlanQoS, sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]).Return(nil)
mocked.On("LinkSetVfHardwareAddr", fakeLink, netconf.VFID, hwaddr).Return(nil)
mocked.On("LinkSetVfRate", fakeLink, netconf.VFID, *netconf.MinTxRate, *netconf.MaxTxRate).Return(nil)
mocked.On("LinkSetVfSpoofchk", fakeLink, netconf.VFID, true).Return(nil)
mocked.On("LinkSetVfTrust", fakeLink, netconf.VFID, true).Return(nil)
mocked.On("LinkSetVfState", fakeLink, netconf.VFID, netlink.VF_LINK_STATE_ENABLE).Return(nil)

sm := sriovManager{nLink: mocked}
err = sm.ApplyVFConfig(netconf)
Expect(err).NotTo(HaveOccurred())
})
})
Context("Checking ReleaseVF function", func() {
var (
podifName string
Expand Down Expand Up @@ -412,7 +466,6 @@ var _ = Describe("Sriov", func() {
fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink"}}

mocked.On("LinkByName", netconf.Master).Return(fakeLink, nil)
mocked.On("LinkSetVfVlanQosProto", fakeLink, netconf.VFID, netconf.OrigVfState.Vlan, netconf.OrigVfState.VlanQoS, sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]).Return(nil)
sm := sriovManager{nLink: mocked}
err := sm.ResetVFConfig(netconf)
Expect(err).NotTo(HaveOccurred())
Expand Down

0 comments on commit 20c67e0

Please sign in to comment.