Add support for automatic bridge selection in HW offload mode (#301)

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

Author: ykulazhenkov

docs/cni-plugin.md

@@ -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.
@@ -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:

docs/ovs-offload.md

@@ -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`.
 

pkg/ovsdb/ovsdb.go

@@ -91,6 +91,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)
@@ -619,6 +623,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 %s: %v", ifaceName, 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 %s: %v", ifaceName, 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 for %s: %v", ifaceName, 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{

pkg/plugin/plugin.go

@@ -142,11 +142,27 @@ 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 != "" {
+		possibleUplinkNames, err := sriov.GetBridgeUplinkNameByDeviceID(deviceID)
+		if err != nil {
+			return "", fmt.Errorf("failed to get bridge name - failed to resolve uplink name: %v", err)
+		}
+		var errList []error
+		for _, uplinkName := range possibleUplinkNames {
+			bridgeName, err = driver.FindBridgeByInterface(uplinkName)
+			if err != nil {
+				errList = append(errList,
+					fmt.Errorf("failed to get bridge name - failed to find bridge name by uplink name %s: %v", uplinkName, err))
+				continue
+			}
+			return bridgeName, nil
+		}
+		return "", fmt.Errorf("failed to find bridge by uplink names %v: %v", possibleUplinkNames, errList)
 	}
 
 	return "", fmt.Errorf("failed to get bridge name")
@@ -256,19 +272,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
 	}
 
@@ -305,19 +329,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)
 				}
 			}
@@ -364,7 +388,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
 		}
@@ -502,13 +526,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
 	}
@@ -530,7 +557,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)
@@ -540,7 +567,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
 			}
 		}
@@ -550,15 +577,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
 		}
 	}
@@ -589,7 +616,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
 	}
 
@@ -614,12 +641,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
 	}
@@ -754,42 +802,28 @@ func validateInterface(intf current.Interface, isHost bool, hwOffload bool) erro
 }
 
 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)
 	}

pkg/sriov/sriov.go

@@ -67,6 +67,72 @@ 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)
+// return list of candidate names
+func GetBridgeUplinkNameByDeviceID(deviceID string) ([]string, error) {
+	pfName, err := sriovnet.GetUplinkRepresentor(deviceID)
+	if err != nil {
+		return nil, err
+	}
+	pfLink, err := netlink.LinkByName(pfName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get link info for uplink %s: %v", pfName, err)
+	}
+	bond, err := getBondInterface(pfLink)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get parent link for uplink %s: %v", pfName, err)
+	}
+	if bond == nil {
+		// PF has no parent bond, return only PF name
+		return []string{pfLink.Attrs().Name}, nil
+	}
+	// for some OVS datapathes, to use bond configuration it is required to attach primary PF (usually first one) to the ovs instead of the bond interface.
+	// Example:
+	// 		- Bond interface bond0 (contains PF0 + PF1)
+	//		- OVS bridge br0 (only PF0 is attached)
+	//		- VF representors from PF0 and PF1 can be attached to OVS bridge br0, traffic will be offloaded and sent through bond0
+	//
+	// to support autobridge selection for VFs from the PF1 (which is part of the bond, but not directly attached to the ovs),
+	// we need to add other interfaces that are part of the bond as candidates, for PF1 candidates list will be: [bond0, PF0, PF1]
+	bondMembers, err := getBondMembers(bond)
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve list of bond members for bond %s, uplink %s: %v", bond.Attrs().Name, pfName, err)
+	}
+	return bondMembers, nil
+}
+
+// getBondInterface returns a parent bond interface for the link if it exists
+func getBondInterface(link netlink.Link) (netlink.Link, error) {
+	if link.Attrs().MasterIndex == 0 {
+		return nil, nil
+	}
+	bondLink, err := netlink.LinkByIndex(link.Attrs().MasterIndex)
+	if err != nil {
+		return nil, err
+	}
+	if bondLink.Type() != "bond" {
+		return nil, nil
+	}
+	return bondLink, nil
+}
+
+// getBondMembers returns list with name of the bond and all bond members
+func getBondMembers(bond netlink.Link) ([]string, error) {
+	allLinks, err := netlink.LinkList()
+	if err != nil {
+		return nil, err
+	}
+	result := []string{bond.Attrs().Name}
+	for _, link := range allLinks {
+		if link.Attrs().MasterIndex == bond.Attrs().Index {
+			result = append(result, link.Attrs().Name)
+		}
+	}
+	return result, nil
+}
+
 // 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).