On 11/26/2024 5:15 PM, Robin Jarry wrote:
Hi Anatoly,
Anatoly Burakov, Nov 26, 2024 at 16:02:
Currently, when binding a device to VFIO, the UID/GID for the device will
always stay as system default (`root`). Yet, when running DPDK as non-
root
user, one has to change the UID/GID of the device to match the user's
UID/GID to use the device.
This patch adds an option to `dpdk-devbind.py` to change the UID/GID of
the device when binding it to VFIO.
Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
---
Notes:
v1 -> v2:
- Replaced hard exit with an error printout
Sorry I had missed that particular detail.
I don't think this should only print a warning. Otherwise, the user has
no way to detect if the operation failed.
Sure, I'll change it back.
from glob import glob
from os.path import exists, basename
@@ -108,6 +110,8 @@
status_flag = False
force_flag = False
noiommu_flag = False
+vfio_uid = ""
+vfio_gid = ""
These are supposed to be integers. Initialize them to -1.
Actually, the pwd.getpwnam() accepts strings not integers, but yeah,
technically these are supposed to be integers. I'll change that.
args = []
@@ -463,6 +467,22 @@ def bind_one(dev_id, driver, force):
% (dev_id, filename, err))
+def own_one(dev_id, uid, gid):
+ """Set the IOMMU group ownership for a device"""
+ # find IOMMU group for a particular device
+ iommu_grp_base_path = os.path.join("/sys/bus/pci/devices",
dev_id, "iommu_group")
+ try:
+ iommu_grp = os.path.basename(os.readlink(iommu_grp_base_path))
+ # we found IOMMU group, now find the device
+ dev_path = os.path.join("/dev/vfio", iommu_grp)
+ # set the ownership
+ _uid = pwd.getpwnam(uid).pw_uid if uid else -1
+ _gid = grp.getgrnam(gid).gr_gid if gid else -1
The validity of these values should be checked when parsing command line
arguments.
Sure, I'll move this check somewhere close to init.
+ os.chown(dev_path, _uid, _gid)
+ except OSError as err:
+ print(f"Error: failed to read IOMMU group for {dev_id}: {err}")
Remove the try/except block and let the error bubble up the stack. This
probably does not require a dedicated function. Moreover, the name
own_one() is ambiguous.
We do the same thing for other errors (e.g. in bind_one) so I'm not sure
if we want to let it bubble up the stack - we don't catch any exceptions
anywhere up the stack. Current implementation, however deficient from
error handling point of view, is consistent with the rest of the script.
# For kernels < 3.15 when binding devices to a generic driver
# (i.e. one that doesn't have a PCI ID table) using new_id, some
devices
@@ -697,6 +720,8 @@ def parse_args():
global force_flag
global noiommu_flag
global args
+ global vfio_uid
+ global vfio_gid
parser = argparse.ArgumentParser(
description='Utility to bind and unbind devices from Linux
kernel',
@@ -746,6 +771,12 @@ def parse_args():
'--noiommu-mode',
action='store_true',
help="If IOMMU is not available, enable no IOMMU mode for
VFIO drivers")
+ parser.add_argument(
+ "-U", "--uid", help="For VFIO, specify the UID to set IOMMU
group ownership"
In order to fail early if an invalid user name is passed, add these two
lines:
type=lambda u: pwd.getpwnam(u).pw_uid,
default=-1,
Guido doesn't like lambdas :D
--
Thanks,
Anatoly
