diff --git a/pkg/config/config.go b/pkg/config/config.go index 361fbbba8..e15fa3829 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -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 diff --git a/pkg/sriov/sriov.go b/pkg/sriov/sriov.go index 646aec7d2..cc0e6a8b8 100644 --- a/pkg/sriov/sriov.go +++ b/pkg/sriov/sriov.go @@ -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 @@ -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 diff --git a/pkg/sriov/sriov_test.go b/pkg/sriov/sriov_test.go index 96d0dd73b..dd0541c41 100644 --- a/pkg/sriov/sriov_test.go +++ b/pkg/sriov/sriov_test.go @@ -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) @@ -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 @@ -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())