Skip to content

Commit

Permalink
Add support for allmulticast flag
Browse files Browse the repository at this point in the history
This allows users to set the allmulticast mode for a VF.

Signed-off-by: Marcelo Guerrero <marguerr@redhat.com>
  • Loading branch information
mlguerrero12 committed May 31, 2023
1 parent 80e5fcc commit 9bcc0f8
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 9 deletions.
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. 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
11 changes: 11 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ 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 that allMulti is one of supported values
if n.AllMulti != "" && 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() {
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
30 changes: 29 additions & 1 deletion pkg/sriov/sriov.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns
return "", fmt.Errorf("error getting VF netdevice with name %s", linkName)
}

conf.OrigVfState.AllMulti = linkObj.Attrs().Allmulti != 0

// tempName used as intermediary name to avoid name conflicts
tempName := fmt.Sprintf("%s%d", "temp_", linkObj.Attrs().Index)

Expand Down Expand Up @@ -126,7 +128,20 @@ 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 != "" {
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 @@ -181,6 +196,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
6 changes: 6 additions & 0 deletions pkg/sriov/sriov_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ var _ = Describe("Sriov", func() {
HardwareAddr: fakeMac,
}}

// Enable allmulticast flag
netconf.AllMulti = "on"

mocked.On("LinkByName", mock.AnythingOfType("string")).Return(fakeLink, nil)
mocked.On("LinkSetAllmulticastOn", 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)
Expand Down Expand Up @@ -190,6 +194,7 @@ var _ = Describe("Sriov", func() {
HostIFName: "enp175s6",
EffectiveMAC: "c6:c8:7f:1f:21:90",
},
AllMulti: "off",
}
})
It("Restores Effective MAC address when provided in netconf", func() {
Expand All @@ -205,6 +210,7 @@ var _ = Describe("Sriov", func() {
fakeLink := &FakeLink{netlink.LinkAttrs{Index: 1000, Name: "dummylink"}}

mocked.On("LinkByName", netconf.ContIFNames).Return(fakeLink, nil)
mocked.On("LinkSetAllmulticastOff", 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)
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
}

// 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

0 comments on commit 9bcc0f8

Please sign in to comment.