Revert "Add support for allmulticast flag"

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",

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"
 }
 ```

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)

pkg/config/config_test.go

@@ -65,44 +65,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",

pkg/sriov/sriov.go

@@ -106,21 +106,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)
 		}
@@ -171,19 +157,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)

pkg/sriov/sriov_test.go

@@ -121,44 +121,6 @@ var _ = Describe("Sriov", func() {
 			Expect(macAddr).To(Equal(netconf.MAC))
 			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() {
@@ -278,37 +240,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 (

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"`

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)