Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for allmulticast flag #268

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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, enabled allmulticast mode and enabled trust mode. These parameters are commonly set in more advanced SR-IOV VF based networks.

```json
{
Expand All @@ -115,6 +115,7 @@ 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",
Expand Down
2 changes: 2 additions & 0 deletions docs/configuration-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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.
Expand All @@ -34,6 +35,7 @@ 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"
}
```
Expand Down
19 changes: 19 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,25 @@ 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)
Expand Down
38 changes: 38 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,44 @@ var _ = Describe("Config", func() {
Expect(err).To(HaveOccurred())
})

It("Assuming correct config file - all multicast", func() {
mlguerrero12 marked this conversation as resolved.
Show resolved Hide resolved
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",
Expand Down
29 changes: 28 additions & 1 deletion pkg/sriov/sriov.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,21 @@ 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. Bring IF up in Pod netns
// 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
if err := s.nLink.LinkSetUp(linkObj); err != nil {
return fmt.Errorf("error bringing interface up in container ns: %q", err)
}
Expand Down Expand Up @@ -157,6 +171,19 @@ 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)
Expand Down
69 changes: 69 additions & 0 deletions pkg/sriov/sriov_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,44 @@ 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() {
Expand Down Expand Up @@ -240,6 +278,37 @@ 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 (
Expand Down
12 changes: 7 additions & 5 deletions pkg/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type VfState struct {
MinTxRate int
MaxTxRate int
LinkState uint32
AllMulti bool
adrianchiris marked this conversation as resolved.
Show resolved Hide resolved
}

// FillFromVfInfo - Fill attributes according to the provided netlink.VfInfo struct
Expand All @@ -43,11 +44,12 @@ 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
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
RuntimeConfig struct {
Mac string `json:"mac,omitempty"`
} `json:"runtimeConfig,omitempty"`
Expand Down
35 changes: 33 additions & 2 deletions pkg/utils/mocks/netlink_manager_mock.go

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

12 changes: 12 additions & 0 deletions pkg/utils/netlink_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ 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
Expand All @@ -35,6 +37,16 @@ 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)
Expand Down