Skip to content

Commit

Permalink
Add support for automatic bridge selection in HW offload mode
Browse files Browse the repository at this point in the history
If deviceID argument is passed it can be used for automatic
bridge selection: VF deviceID > PF > Bond(Optional) > Bridge

Signed-off-by: Yury Kulazhenkov <[email protected]>
  • Loading branch information
ykulazhenkov committed Mar 1, 2024
1 parent 70e52f7 commit 34f16be
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 36 deletions.
8 changes: 7 additions & 1 deletion docs/cni-plugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ Another example with a port which has an interface of type system:

* `name` (string, required): the name of the network.
* `type` (string, required): "ovs".
* `bridge` (string, required): name of the bridge to use.
* `bridge` (string, optional): name of the bridge to use, can be omitted if `ovnPort` is set in CNI_ARGS, or if `deviceID` is set
* `deviceID` (string, optional): PCI address of a Virtual Function in valid sysfs format to use in HW offloading mode. This value is usually set by Multus.
* `vlan` (integer, optional): VLAN ID of attached port. Trunk port if not
specified.
* `mtu` (integer, optional): MTU.
Expand All @@ -61,6 +62,11 @@ Another example with a port which has an interface of type system:
* `configuration_path` (optional): configuration file containing ovsdb
socket file path, etc.


_*Note:* if `deviceID` is provided, then it is possible to omit `bridge` argument. Bridge will be automatically selected by the CNI plugin by following
the chain: Virtual Function PCI address (provided in `deviceID` argument) > Physical Function > Bond interface
(optional, if Physical Function is part of a bond interface) > ovs bridge_

### Flatfile Configuation

There is one option for flat file configuration:
Expand Down
4 changes: 4 additions & 0 deletions docs/ovs-offload.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ spec:
}'
```
_*Note:* it is possible to omit `bridge` argument. Bridge will be automatically selected by the CNI plugin by following
the chain: Virtual Function PCI address (injected by Multus to `deviceID` parameter) > Physical Function > Bond interface
(optional, if Physical Function is part of a bond interface) > ovs bridge_

Now deploy a pod with the following config to attach VF into container and its representor net device
attached with ovs bridge `br-snic0`.

Expand Down
27 changes: 27 additions & 0 deletions pkg/ovsdb/ovsdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ func connectToOvsDb(ovsSocket string) (client.Client, error) {
func NewOvsDriver(ovsSocket string) (*OvsDriver, error) {
ovsDriver := new(OvsDriver)

if ovsSocket == "" {
ovsSocket = "unix:/var/run/openvswitch/db.sock"
}

ovsDB, err := connectToOvsDb(ovsSocket)
if err != nil {
return nil, fmt.Errorf("failed to connect to ovsdb error: %v", err)
Expand Down Expand Up @@ -615,6 +619,29 @@ func (ovsd *OvsDriver) IsBridgePresent(bridgeName string) (bool, error) {
return true, nil
}

// FindBridgeByInterface returns name of the bridge that contains provided interface
func (ovsd *OvsDriver) FindBridgeByInterface(ifaceName string) (string, error) {
iface, err := ovsd.findByCondition("Interface",
ovsdb.NewCondition("name", ovsdb.ConditionEqual, ifaceName),
[]string{"name", "_uuid"})
if err != nil {
return "", fmt.Errorf("failed to find interface: %v", err)
}
port, err := ovsd.findByCondition("Port",
ovsdb.NewCondition("interfaces", ovsdb.ConditionIncludes, iface["_uuid"]),
[]string{"name", "_uuid"})
if err != nil {
return "", fmt.Errorf("failed to find port: %v", err)
}
bridge, err := ovsd.findByCondition("Bridge",
ovsdb.NewCondition("ports", ovsdb.ConditionIncludes, port["_uuid"]),
[]string{"name"})
if err != nil {
return "", fmt.Errorf("failed to find bridge: %v", err)
}
return fmt.Sprintf("%v", bridge["name"]), nil
}

// GetOvsPortForContIface Return ovs port name for an container interface
func (ovsd *OvsDriver) GetOvsPortForContIface(contIface, contNetnsPath string) (string, bool, error) {
searchMap := map[string]string{
Expand Down
98 changes: 63 additions & 35 deletions pkg/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,21 @@ func assignMacToLink(link netlink.Link, mac net.HardwareAddr, name string) error
return nil
}

func getBridgeName(bridgeName, ovnPort string) (string, error) {
func getBridgeName(driver *ovsdb.OvsDriver, bridgeName, ovnPort, deviceID string) (string, error) {
if bridgeName != "" {
return bridgeName, nil
} else if bridgeName == "" && ovnPort != "" {
return "br-int", nil
} else if deviceID != "" {
uplink, err := sriov.GetBridgeUplinkNameByDeviceID(deviceID)
if err != nil {
return "", fmt.Errorf("failed to get bridge name - failed to resolve uplink name: %v", err)
}
bridgeName, err = driver.FindBridgeByInterface(uplink)
if err != nil {
return "", fmt.Errorf("failed to get bridge name - failed to find bridge name by uplink name: %v", err)
}
return bridgeName, nil
}

return "", fmt.Errorf("failed to get bridge name")
Expand Down Expand Up @@ -256,19 +266,27 @@ func CmdAdd(args *skel.CmdArgs) error {
} else if netconf.VlanTag != nil {
vlanTagNum = *netconf.VlanTag
}

bridgeName, err := getBridgeName(netconf.BrName, ovnPort)
ovsDriver, err := ovsdb.NewOvsDriver(netconf.SocketFile)
if err != nil {
return err
}
bridgeName, err := getBridgeName(ovsDriver, netconf.BrName, ovnPort, netconf.DeviceID)
if err != nil {
return err
}
// save discovered bridge name to the netconf struct to make
// sure it is save in the cache.
// we need to cache discovered bridge name to make sure that we will
// use the right bridge name in CmdDel
netconf.BrName = bridgeName

ovsDriver, err := ovsdb.NewOvsBridgeDriver(bridgeName, netconf.SocketFile)
ovsBridgeDriver, err := ovsdb.NewOvsBridgeDriver(bridgeName, netconf.SocketFile)
if err != nil {
return err
}

// removes all ports whose interfaces have an error
if err := cleanPorts(ovsDriver); err != nil {
if err := cleanPorts(ovsBridgeDriver); err != nil {
return err
}

Expand Down Expand Up @@ -305,19 +323,19 @@ func CmdAdd(args *skel.CmdArgs) error {
}
}

if err = attachIfaceToBridge(ovsDriver, hostIface.Name, contIface.Name, netconf.OfportRequest, vlanTagNum, trunks, portType, netconf.InterfaceType, args.Netns, ovnPort); err != nil {
if err = attachIfaceToBridge(ovsBridgeDriver, hostIface.Name, contIface.Name, netconf.OfportRequest, vlanTagNum, trunks, portType, netconf.InterfaceType, args.Netns, ovnPort); err != nil {
return err
}
defer func() {
if err != nil {
// Unlike veth pair, OVS port will not be automatically removed
// if the following IPAM configuration fails and netns gets removed.
portName, portFound, err := getOvsPortForContIface(ovsDriver, args.IfName, args.Netns)
portName, portFound, err := getOvsPortForContIface(ovsBridgeDriver, args.IfName, args.Netns)
if err != nil {
log.Printf("Failed best-effort cleanup: %v", err)
}
if portFound {
if err := removeOvsPort(ovsDriver, portName); err != nil {
if err := removeOvsPort(ovsBridgeDriver, portName); err != nil {
log.Printf("Failed best-effort cleanup: %v", err)
}
}
Expand Down Expand Up @@ -364,7 +382,7 @@ func CmdAdd(args *skel.CmdArgs) error {

// wait until OF port link state becomes up. This is needed to make
// gratuitous arp for args.IfName to be sent over ovs bridge
err = waitLinkUp(ovsDriver, hostIface.Name, netconf.LinkStateCheckRetries, netconf.LinkStateCheckInterval)
err = waitLinkUp(ovsBridgeDriver, hostIface.Name, netconf.LinkStateCheckRetries, netconf.LinkStateCheckInterval)
if err != nil {
return err
}
Expand Down Expand Up @@ -502,13 +520,16 @@ func CmdDel(args *skel.CmdArgs) error {
if envArgs != nil {
ovnPort = string(envArgs.OvnPort)
}

bridgeName, err := getBridgeName(cache.Netconf.BrName, ovnPort)
ovsDriver, err := ovsdb.NewOvsDriver(cache.Netconf.SocketFile)
if err != nil {
return err
}
bridgeName, err := getBridgeName(ovsDriver, cache.Netconf.BrName, ovnPort, cache.Netconf.DeviceID)
if err != nil {
return err
}

ovsDriver, err := ovsdb.NewOvsBridgeDriver(bridgeName, cache.Netconf.SocketFile)
ovsBridgeDriver, err := ovsdb.NewOvsBridgeDriver(bridgeName, cache.Netconf.SocketFile)
if err != nil {
return err
}
Expand All @@ -530,7 +551,7 @@ func CmdDel(args *skel.CmdArgs) error {
if rep, err = sriov.GetNetRepresentor(cache.Netconf.DeviceID); err != nil {
return err
}
if err = removeOvsPort(ovsDriver, rep); err != nil {
if err = removeOvsPort(ovsBridgeDriver, rep); err != nil {
// Don't throw err as delete can be called multiple times because of error in ResetVF and ovs
// port is already deleted in a previous invocation.
log.Printf("Error: %v\n", err)
Expand All @@ -540,7 +561,7 @@ func CmdDel(args *skel.CmdArgs) error {
}
} else {
// In accordance with the spec we clean up as many resources as possible.
if err := cleanPorts(ovsDriver); err != nil {
if err := cleanPorts(ovsBridgeDriver); err != nil {
return err
}
}
Expand All @@ -550,15 +571,15 @@ func CmdDel(args *skel.CmdArgs) error {
// Unlike veth pair, OVS port will not be automatically removed when
// container namespace is gone. Find port matching DEL arguments and remove
// it explicitly.
portName, portFound, err := getOvsPortForContIface(ovsDriver, args.IfName, args.Netns)
portName, portFound, err := getOvsPortForContIface(ovsBridgeDriver, args.IfName, args.Netns)
if err != nil {
return fmt.Errorf("Failed to obtain OVS port for given connection: %v", err)
}

// Do not return an error if the port was not found, it may have been
// already removed by someone.
if portFound {
if err := removeOvsPort(ovsDriver, portName); err != nil {
if err := removeOvsPort(ovsBridgeDriver, portName); err != nil {
return err
}
}
Expand Down Expand Up @@ -589,7 +610,7 @@ func CmdDel(args *skel.CmdArgs) error {
}

// removes all ports whose interfaces have an error
if err := cleanPorts(ovsDriver); err != nil {
if err := cleanPorts(ovsBridgeDriver); err != nil {
return err
}

Expand All @@ -613,12 +634,33 @@ func CmdCheck(args *skel.CmdArgs) error {
}
}

envArgs, err := getEnvArgs(args.Args)
if err != nil {
return err
}
var ovnPort string
if envArgs != nil {
ovnPort = string(envArgs.OvnPort)
}
ovsDriver, err := ovsdb.NewOvsDriver(netconf.SocketFile)
if err != nil {
return err
}
// cached config may contain bridge name which were automatically
// discovered in CmdAdd, we need to re-discover the bridge name before we validating the cache
bridgeName, err := getBridgeName(ovsDriver, netconf.BrName, ovnPort, netconf.DeviceID)
if err != nil {
return err
}
netconf.BrName = bridgeName

// check cache
cRef := config.GetCRef(args.ContainerID, args.IfName)
cache, err := config.LoadConfFromCache(cRef)
if err != nil {
return err
}

if err := validateCache(cache, netconf); err != nil {
return err
}
Expand Down Expand Up @@ -752,42 +794,28 @@ func validateInterface(intf current.Interface, isHost bool) error {
}

func validateOvs(args *skel.CmdArgs, netconf *types.NetConf, hostIfname string) error {
envArgs, err := getEnvArgs(args.Args)
if err != nil {
return err
}
var ovnPort string
if envArgs != nil {
ovnPort = string(envArgs.OvnPort)
}

bridgeName, err := getBridgeName(netconf.BrName, ovnPort)
ovsBridgeDriver, err := ovsdb.NewOvsBridgeDriver(netconf.BrName, netconf.SocketFile)
if err != nil {
return err
}

ovsDriver, err := ovsdb.NewOvsBridgeDriver(bridgeName, netconf.SocketFile)
if err != nil {
return err
}

found, err := ovsDriver.IsBridgePresent(netconf.BrName)
found, err := ovsBridgeDriver.IsBridgePresent(netconf.BrName)
if err != nil {
return err
}
if !found {
return fmt.Errorf("Error: bridge %s is not found in OVS", netconf.BrName)
}

ifaces, err := ovsDriver.FindInterfacesWithError()
ifaces, err := ovsBridgeDriver.FindInterfacesWithError()
if err != nil {
return err
}
if len(ifaces) > 0 {
return fmt.Errorf("Error: There are some interfaces in error state: %v", ifaces)
}

vlanMode, tag, trunk, err := ovsDriver.GetOFPortVlanState(hostIfname)
vlanMode, tag, trunk, err := ovsBridgeDriver.GetOFPortVlanState(hostIfname)
if err != nil {
return fmt.Errorf("Error: Failed to retrieve port %s state: %v", hostIfname, err)
}
Expand Down
36 changes: 36 additions & 0 deletions pkg/sriov/sriov.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,42 @@ func IsOvsHardwareOffloadEnabled(deviceID string) bool {
return deviceID != ""
}

// GetBridgeUplinkNameByDeviceID tries to automatically resolve uplink interface name
// for provided VF deviceID by following the sequence:
// VF pci address > PF pci address > Bond (optional, if PF is part of a bond)
func GetBridgeUplinkNameByDeviceID(deviceID string) (string, error) {
pfName, err := sriovnet.GetUplinkRepresentor(deviceID)
if err != nil {
return "", err
}
pfLink, err := netlink.LinkByName(pfName)
if err != nil {
return "", fmt.Errorf("failed to get link info for uplink %s: %v", pfName, err)
}
linkToCheck := pfLink
for {
var parent netlink.Link
parent, err = getParentInterface(linkToCheck)
if err != nil {
return "", err
}
if parent.Type() == "openvswitch" {
break
}
linkToCheck = parent
}

return linkToCheck.Attrs().Name, nil
}

// getParentInterface returns a parent interface for the link if it exists
func getParentInterface(link netlink.Link) (netlink.Link, error) {
if link.Attrs().MasterIndex == 0 {
return nil, fmt.Errorf("link %s has no parent interface", link.Attrs().Name)
}
return netlink.LinkByIndex(link.Attrs().MasterIndex)
}

// GetNetRepresentor retrieves network representor device for smartvf
func GetNetRepresentor(deviceID string) (string, error) {
// get Uplink netdevice. The uplink is basically the PF name of the deviceID (smart VF).
Expand Down

0 comments on commit 34f16be

Please sign in to comment.