CA-412146 Filter out VF when scan #6528
Conversation
|
The method at https://github.com/xapi-project/xen-api/blob/v25.22.0/ocaml/xapi/xapi_pci_helpers.ml#L68-L69 appears to use a different path, but after resolving the symlink, both methods actually refer to the same location. |
changlei-li
force-pushed
the
private/changleli/CA-412146
branch
from
9367f2f to
ff83be0
Compare
ocaml/networkd/lib/network_utils.ml
Outdated
| in | ||
| let is_phys = not (is_vif || is_vf) in | ||
| if not is_phys then | ||
| debug "%s is not physical: is_vif=%B, is_vf=%B" name is_vif is_vf ; |
There was a problem hiding this comment.
Is this needed, I'm afraid some callers my call the function in high frequency, adding a lot of loglines which are not useful.
There was a problem hiding this comment.
Hmm, I intended to record the device is filtered out by this check leading to not create PIF for it. I haven't seen too many (Just pif-scan and some few items) in my self-test but I'm not sure.
However, it should be OK not to log, keeping the same with previous behaviours.
SR-IOV (Single Root I/O Virtualization) is a technology that allows a single physical PCI Express (PCIe) device, such as a network adapter, to be shared efficiently among multiple virtual machines (VMs) or containers. It achieves this by creating Virtual Functions (VFs) that act as lightweight PCIe functions, each assigned to a VM, while the Physical Function (PF) remains responsible for managing the device. Add check in Sysfs.is_physical - check if there is "physfn" in the device dir to filter out VF, then XAPI will not create PIF object for VF during scan. Signed-off-by: Changlei Li <[email protected]>
changlei-li
force-pushed
the
private/changleli/CA-412146
branch
from
ff83be0 to
7762a50
Compare
| @@ -1547,7 +1555,7 @@ module Ovs = struct | |||
| let vif_arg = | |||
| let existing_vifs = | |||
| List.filter | |||
| (fun iface -> not (Sysfs.is_physical iface)) | |||
| (fun iface -> try Sysfs.is_vif iface with _ -> false) | |||
There was a problem hiding this comment.
I searched all the callers of is_physical, this caller actually wants to check is_vif. So, I split the functions out to use is_vif here.
| try | ||
| let devpath = getpath name "device" in | ||
| let driver_link = Unix.readlink (devpath ^ "/driver") in | ||
| (* filter out symlinks under device/driver which look like | ||
| /../../../devices/xen-backend/vif- *) | ||
| not | ||
| (List.mem "xen-backend" | ||
| (Astring.String.cuts ~empty:false ~sep:"/" driver_link) | ||
| ) | ||
| List.mem "xen-backend" | ||
| (Astring.String.cuts ~empty:false ~sep:"/" driver_link) | ||
| with _ -> | ||
| raise (Network_error (Internal_error "Unable to read driver link")) |
There was a problem hiding this comment.
Non of the users of is_vifcare about the raised exception, so there's no need to waste time creating a backtrace.
| try | |
| let devpath = getpath name "device" in | |
| let driver_link = Unix.readlink (devpath ^ "/driver") in | |
| (* filter out symlinks under device/driver which look like | |
| /../../../devices/xen-backend/vif- *) | |
| not | |
| (List.mem "xen-backend" | |
| (Astring.String.cuts ~empty:false ~sep:"/" driver_link) | |
| ) | |
| List.mem "xen-backend" | |
| (Astring.String.cuts ~empty:false ~sep:"/" driver_link) | |
| with _ -> | |
| raise (Network_error (Internal_error "Unable to read driver link")) | |
| try | |
| let driver_link = Unix.readlink (devpath ^ "/driver") in | |
| (* filter out symlinks under device/driver which look like | |
| /../../../devices/xen-backend/vif- *) | |
| List.mem "xen-backend" | |
| (Astring.String.cuts ~empty:false ~sep:"/" driver_link) | |
| with _ -> | |
| false |
There was a problem hiding this comment.
Let me think about it. For is_physical, if a device reads driver error, then it should not be a physical device either. In your suggestion, is_vif returns false, is_physical returns true in this scenario. It's different with original behaviour https://github.com/xapi-project/xen-api/blob/v25.22.0/ocaml/networkd/lib/network_utils.ml#L184-L194
| with _ -> false | ||
|
|
||
| let is_physical name = try not (is_vif name || is_vf name) with _ -> false |
There was a problem hiding this comment.
| let is_physical name = try not (is_vif name || is_vf name) with _ -> false | |
| let is_physical name = not (is_vif name || is_vf name) |
| let existing_vifs = | ||
| List.filter | ||
| (fun iface -> not (Sysfs.is_physical iface)) | ||
| (fun iface -> try Sysfs.is_vif iface with _ -> false) | ||
| (bridge_to_interfaces name) | ||
| in |
There was a problem hiding this comment.
| let existing_vifs = | |
| List.filter | |
| (fun iface -> not (Sysfs.is_physical iface)) | |
| (fun iface -> try Sysfs.is_vif iface with _ -> false) | |
| (bridge_to_interfaces name) | |
| in | |
| let existing_vifs = List.filter Sysfs.is_vif (bridge_to_interfaces name) in |
SR-IOV (Single Root I/O Virtualization) is a technology that allows a single physical PCI Express (PCIe) device, such as a network adapter, to be shared efficiently among multiple virtual machines (VMs) or containers. It achieves this by creating Virtual Functions (VFs) that act as lightweight PCIe functions, each assigned to a VM, while the Physical Function (PF) remains responsible for managing the device.
Add check in Sysfs.is_physical - check if there is "physfn" in the device dir to filter out VF, then XAPI will not create PIF object for VF during scan.