From 34f16be956114fe07a139f11f3e870e0c3bb2b3a Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Mon, 12 Feb 2024 16:25:31 +0200 Subject: [PATCH] Add support for automatic bridge selection in HW offload mode If deviceID argument is passed it can be used for automatic bridge selection: VF deviceID > PF > Bond(Optional) > Bridge Signed-off-by: Yury Kulazhenkov --- docs/cni-plugin.md | 8 +++- docs/ovs-offload.md | 4 ++ pkg/ovsdb/ovsdb.go | 27 ++++++++++++ pkg/plugin/plugin.go | 98 ++++++++++++++++++++++++++++---------------- pkg/sriov/sriov.go | 36 ++++++++++++++++ 5 files changed, 137 insertions(+), 36 deletions(-) diff --git a/docs/cni-plugin.md b/docs/cni-plugin.md index 7536001d..467a06fd 100644 --- a/docs/cni-plugin.md +++ b/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: diff --git a/docs/ovs-offload.md b/docs/ovs-offload.md index 7ea83ca6..242a10ec 100644 --- a/docs/ovs-offload.md +++ b/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`. diff --git a/pkg/ovsdb/ovsdb.go b/pkg/ovsdb/ovsdb.go index 767fc3a4..3f4ec711 100644 --- a/pkg/ovsdb/ovsdb.go +++ b/pkg/ovsdb/ovsdb.go @@ -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) @@ -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{ diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 5d173a71..e8df415e 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -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") @@ -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 } @@ -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) } } @@ -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 } @@ -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 } @@ -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) @@ -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 } } @@ -550,7 +571,7 @@ 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) } @@ -558,7 +579,7 @@ func CmdDel(args *skel.CmdArgs) error { // 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 +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 } @@ -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 } @@ -752,26 +794,12 @@ 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 } @@ -779,7 +807,7 @@ func validateOvs(args *skel.CmdArgs, netconf *types.NetConf, hostIfname string) 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 } @@ -787,7 +815,7 @@ func validateOvs(args *skel.CmdArgs, netconf *types.NetConf, hostIfname string) 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) } diff --git a/pkg/sriov/sriov.go b/pkg/sriov/sriov.go index 3afbc2e7..ce2985df 100644 --- a/pkg/sriov/sriov.go +++ b/pkg/sriov/sriov.go @@ -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).