diff --git a/README.md b/README.md index d1f28aadd..6e8429500 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ This is the minimum configuration for a working kernel driver interface using an ``` #### Extended kernel driver config -This configuration sets a number of extra parameters that may be key for SR-IOV networks including a vlan tag, disabled spoof checking, enabled allmulticast mode and enabled trust mode. These parameters are commonly set in more advanced SR-IOV VF based networks. +This configuration sets a number of extra parameters that may be key for SR-IOV networks including a vlan tag, disabled spoof checking and enabled trust mode. These parameters are commonly set in more advanced SR-IOV VF based networks. ```json { @@ -115,7 +115,6 @@ This configuration sets a number of extra parameters that may be key for SR-IOV "vlan": 1000, "spoofchk": "off", "trust": "on", - "all_multicast": "on", "ipam": { "type": "host-local", "subnet": "10.56.217.0/24", diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index 9fee4bce1..aebc9eb60 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -13,7 +13,6 @@ The SR-IOV CNI configures networks through a CNI spec configuration object. In a * `mac` (string, optional): MAC address to assign for the VF * `spoofchk` (string, optional): turn packet spoof checking on or off for the VF * `trust` (string, optional): turn trust setting on or off for the VF -* `all_multicast` (string, optional): turn allmulticast setting on or off for the VF network device. Trust setting must be enabled when this setting is on. * `link_state` (string, optional): enforce link state for the VF. Allowed values: auto, enable, disable. Note that driver support may differ for this feature. For example, `i40e` is known to work but `igb` doesn't. * `min_tx_rate` (int, optional): change the allowed minimum transmit bandwidth, in Mbps, for the VF. Setting this to 0 disables rate limiting. The min_tx_rate value should be <= max_tx_rate. Support of this feature depends on NICs and drivers. * `max_tx_rate` (int, optional): change the allowed maximum transmit bandwidth, in Mbps, for the VF. @@ -35,7 +34,6 @@ An SR-IOV CNI config with each field filled out looks like: "max_tx_rate": 200, "spoofchk": "off", "trust": "on", - "all_multicast": "on", "link_state": "enable" } ``` diff --git a/pkg/config/config.go b/pkg/config/config.go index 3c9a74304..668bcea73 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -93,25 +93,6 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) { return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan QoS to a non-zero value") } - // validate allMulti parameter - if n.AllMulti != "" { - // Can't be set when VF has a dpdk driver - if n.DPDKMode { - return nil, fmt.Errorf("LoadConf(): all_multicast cannot be set when VF has a dpdk driver") - } - - // Verify that the value is supported - if n.AllMulti != "on" && n.AllMulti != "off" { - return nil, fmt.Errorf("LoadConf(): invalid all_multicast value: %s", n.AllMulti) - } - - // validate that both, trust and allMulti are enabled - // only trusted VFs can enter the all-multicast RX mode - if n.AllMulti == "on" && n.Trust != "on" { - return nil, fmt.Errorf("LoadConf(): trust must be enabled to set all_multicast: %s", n.AllMulti) - } - } - // validate that link state is one of supported values if n.LinkState != "" && n.LinkState != "auto" && n.LinkState != "enable" && n.LinkState != "disable" { return nil, fmt.Errorf("LoadConf(): invalid link_state value: %s", n.LinkState) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 911cee7f2..2140acede 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -66,44 +66,6 @@ var _ = Describe("Config", func() { Expect(err).To(HaveOccurred()) }) - It("Assuming correct config file - all multicast", func() { - conf := []byte(`{ - "name": "mynet", - "type": "sriov", - "deviceID": "0000:af:06.1", - "vf": 0, - "all_multicast": "on", - "trust": "on" - }`) - _, err := LoadConf(conf) - Expect(err).ToNot(HaveOccurred()) - }) - - It("Assuming incorrect all_multicast - trust not enabled", func() { - conf := []byte(`{ - "name": "mynet", - "type": "sriov", - "deviceID": "0000:af:06.1", - "vf": 0, - "all_multicast": "on", - "trust": "off" - }`) - _, err := LoadConf(conf) - Expect(err).To(HaveOccurred()) - }) - - It("Assuming incorrect all_multicast - incorrect value", func() { - conf := []byte(`{ - "name": "mynet", - "type": "sriov", - "deviceID": "0000:af:06.1", - "vf": 0, - "all_multicast": "sriov" - }`) - _, err := LoadConf(conf) - Expect(err).To(HaveOccurred()) - }) - It("Assuming device is allocated", func() { conf := []byte(`{ "name": "mynet", diff --git a/pkg/sriov/sriov.go b/pkg/sriov/sriov.go index c4998e417..4d0eb8a6d 100644 --- a/pkg/sriov/sriov.go +++ b/pkg/sriov/sriov.go @@ -103,21 +103,7 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns // Error is ignored here because enabling this feature is only a performance enhancement. _ = s.utils.EnableArpAndNdiscNotify(podifName) - // 7. Set allmulticast flag - if conf.AllMulti != "" { - conf.OrigVfState.AllMulti = linkObj.Attrs().Allmulti != 0 - if conf.AllMulti == "on" { - if err := s.nLink.LinkSetAllmulticastOn(linkObj); err != nil { - return fmt.Errorf("error setting allmulticast %s: %v", conf.AllMulti, err) - } - } else { - if err := s.nLink.LinkSetAllmulticastOff(linkObj); err != nil { - return fmt.Errorf("error setting allmulticast %s: %v", conf.AllMulti, err) - } - } - } - - // 8. Bring IF up in Pod netns + // 7. Bring IF up in Pod netns if err := s.nLink.LinkSetUp(linkObj); err != nil { return fmt.Errorf("error bringing interface up in container ns: %q", err) } @@ -168,19 +154,6 @@ func (s *sriovManager) ReleaseVF(conf *sriovtypes.NetConf, podifName string, net } } - // reset allmulticast flag - if conf.AllMulti != "" { - if conf.OrigVfState.AllMulti { - if err := s.nLink.LinkSetAllmulticastOn(linkObj); err != nil { - return fmt.Errorf("error setting allmulticast %s: %v", conf.AllMulti, err) - } - } else { - if err := s.nLink.LinkSetAllmulticastOff(linkObj); err != nil { - return fmt.Errorf("error setting allmulticast %s: %v", conf.AllMulti, err) - } - } - } - // move VF device to init netns if err = s.nLink.LinkSetNsFd(linkObj, int(initns.Fd())); err != nil { return fmt.Errorf("failed to move interface %s to init netns: %v", conf.OrigVfState.HostIFName, err) diff --git a/pkg/sriov/sriov_test.go b/pkg/sriov/sriov_test.go index 4b863d6da..8c5eff783 100644 --- a/pkg/sriov/sriov_test.go +++ b/pkg/sriov/sriov_test.go @@ -120,44 +120,6 @@ var _ = Describe("Sriov", func() { Expect(err).NotTo(HaveOccurred()) mocked.AssertExpectations(t) }) - - DescribeTable("Setting all multicast", func(value, mockFunc string) { - var targetNetNS ns.NetNS - targetNetNS, err := testutils.NewNS() - defer func() { - if targetNetNS != nil { - targetNetNS.Close() - } - }() - Expect(err).NotTo(HaveOccurred()) - - mocked := &mocks_utils.NetlinkManager{} - mockedPciUtils := &mocks.PciUtils{} - - fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ - Index: 1000, - Name: "dummylink", - }} - - netconf.AllMulti = value - - mocked.On("LinkByName", mock.AnythingOfType("string")).Return(fakeLink, nil) - mocked.On(mockFunc, mock.Anything).Return(nil) - mocked.On("LinkSetDown", fakeLink).Return(nil) - 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) - Expect(err).NotTo(HaveOccurred()) - }, - Entry("Enabling all multicast", "on", "LinkSetAllmulticastOn"), - Entry("Disabling all multicast", "off", "LinkSetAllmulticastOff"), - ) }) Context("Checking ReleaseVF function", func() { @@ -277,37 +239,6 @@ var _ = Describe("Sriov", func() { Expect(err).NotTo(HaveOccurred()) mocked.AssertExpectations(t) }) - - DescribeTable("restores all multicast when provided in netconf", func(value, mockFunc string) { - var targetNetNS ns.NetNS - targetNetNS, err := testutils.NewNS() - defer func() { - if targetNetNS != nil { - targetNetNS.Close() - } - }() - Expect(err).NotTo(HaveOccurred()) - - fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} - mocked := &mocks_utils.NetlinkManager{} - - netconf.OrigVfState.AllMulti = value != "on" - netconf.AllMulti = value - - mocked.On("LinkByName", netconf.ContIFNames).Return(fakeLink, nil) - mocked.On(mockFunc, mock.Anything).Return(nil) - mocked.On("LinkSetDown", fakeLink).Return(nil) - mocked.On("LinkSetName", fakeLink, netconf.OrigVfState.HostIFName).Return(nil) - mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil) - - sm := sriovManager{nLink: mocked} - err = sm.ReleaseVF(netconf, podifName, targetNetNS) - Expect(err).NotTo(HaveOccurred()) - mocked.AssertExpectations(t) - }, - Entry("Restoring all multicast off", "on", "LinkSetAllmulticastOff"), - Entry("Restoring all multicast on", "off", "LinkSetAllmulticastOn"), - ) }) Context("Checking FillOriginalVfInfo function", func() { var ( diff --git a/pkg/types/types.go b/pkg/types/types.go index 7ea517470..76ff57be9 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -17,7 +17,6 @@ type VfState struct { MinTxRate int MaxTxRate int LinkState uint32 - AllMulti bool } // FillFromVfInfo - Fill attributes according to the provided netlink.VfInfo struct @@ -44,12 +43,11 @@ type NetConf struct { DeviceID string `json:"deviceID"` // PCI address of a VF in valid sysfs format VFID int ContIFNames string // VF names after in the container; used during deletion - MinTxRate *int `json:"min_tx_rate"` // Mbps, 0 = disable rate limiting - MaxTxRate *int `json:"max_tx_rate"` // Mbps, 0 = disable rate limiting - SpoofChk string `json:"spoofchk,omitempty"` // on|off - Trust string `json:"trust,omitempty"` // on|off - LinkState string `json:"link_state,omitempty"` // auto|enable|disable - AllMulti string `json:"all_multicast,omitempty"` // on|off + MinTxRate *int `json:"min_tx_rate"` // Mbps, 0 = disable rate limiting + MaxTxRate *int `json:"max_tx_rate"` // Mbps, 0 = disable rate limiting + SpoofChk string `json:"spoofchk,omitempty"` // on|off + Trust string `json:"trust,omitempty"` // on|off + LinkState string `json:"link_state,omitempty"` // auto|enable|disable RuntimeConfig struct { Mac string `json:"mac,omitempty"` } `json:"runtimeConfig,omitempty"` diff --git a/pkg/utils/mocks/netlink_manager_mock.go b/pkg/utils/mocks/netlink_manager_mock.go index 363dbf7cb..891a4e9d8 100644 --- a/pkg/utils/mocks/netlink_manager_mock.go +++ b/pkg/utils/mocks/netlink_manager_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.20.0. DO NOT EDIT. +// Code generated by mockery v2.14.0. DO NOT EDIT. package mocks @@ -20,10 +20,6 @@ func (_m *NetlinkManager) LinkByName(_a0 string) (netlink.Link, error) { ret := _m.Called(_a0) var r0 netlink.Link - var r1 error - if rf, ok := ret.Get(0).(func(string) (netlink.Link, error)); ok { - return rf(_a0) - } if rf, ok := ret.Get(0).(func(string) netlink.Link); ok { r0 = rf(_a0) } else { @@ -32,6 +28,7 @@ func (_m *NetlinkManager) LinkByName(_a0 string) (netlink.Link, error) { } } + var r1 error if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(_a0) } else { @@ -41,34 +38,6 @@ func (_m *NetlinkManager) LinkByName(_a0 string) (netlink.Link, error) { return r0, r1 } -// LinkSetAllmulticastOff provides a mock function with given fields: _a0 -func (_m *NetlinkManager) LinkSetAllmulticastOff(_a0 netlink.Link) error { - ret := _m.Called(_a0) - - var r0 error - if rf, ok := ret.Get(0).(func(netlink.Link) error); ok { - r0 = rf(_a0) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// LinkSetAllmulticastOn provides a mock function with given fields: _a0 -func (_m *NetlinkManager) LinkSetAllmulticastOn(_a0 netlink.Link) error { - ret := _m.Called(_a0) - - var r0 error - if rf, ok := ret.Get(0).(func(netlink.Link) error); ok { - r0 = rf(_a0) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // LinkSetDown provides a mock function with given fields: _a0 func (_m *NetlinkManager) LinkSetDown(_a0 netlink.Link) error { ret := _m.Called(_a0) diff --git a/pkg/utils/netlink_manager.go b/pkg/utils/netlink_manager.go index 5db2877cc..cf681eee0 100644 --- a/pkg/utils/netlink_manager.go +++ b/pkg/utils/netlink_manager.go @@ -11,8 +11,6 @@ import ( // NetlinkManager is an interface to mock nelink library type NetlinkManager interface { LinkByName(string) (netlink.Link, error) - LinkSetAllmulticastOn(netlink.Link) error - LinkSetAllmulticastOff(netlink.Link) error LinkSetVfVlan(netlink.Link, int, int) error LinkSetVfVlanQos(netlink.Link, int, int, int) error LinkSetVfHardwareAddr(netlink.Link, int, net.HardwareAddr) error @@ -37,16 +35,6 @@ func (n *MyNetlink) LinkByName(name string) (netlink.Link, error) { return netlink.LinkByName(name) } -// LinkSetAllmulticastOn sets allmulticast on -func (n *MyNetlink) LinkSetAllmulticastOn(link netlink.Link) error { - return netlink.LinkSetAllmulticastOn(link) -} - -// LinkSetAllmulticastOff sets allmulticast off -func (n *MyNetlink) LinkSetAllmulticastOff(link netlink.Link) error { - return netlink.LinkSetAllmulticastOff(link) -} - // LinkSetVfVlan using NetlinkManager func (n *MyNetlink) LinkSetVfVlan(link netlink.Link, vf, vlan int) error { return netlink.LinkSetVfVlan(link, vf, vlan)