[PATCH v1 1/1] usertools/devbind: allow changing UID/GID for VFIO

[PATCH v1 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Anatoly Burakov]

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.burakov@intel.com>
---
 usertools/dpdk-devbind.py | 41 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 078e8c387b..37e2b9972d 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -8,6 +8,8 @@
 import subprocess
 import argparse
 import platform
+import grp
+import pwd
 
 from glob import glob
 from os.path import exists, basename
@@ -107,6 +109,8 @@
 b_flag = None
 status_flag = False
 force_flag = False
+vfio_uid = ""
+vfio_gid = ""
 args = []
 
 
@@ -462,6 +466,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
+        os.chown(dev_path, _uid, _gid)
+    except OSError as err:
+        sys.exit(f"Error: failed to read IOMMU group for {dev_id}: {err}")
+
+
 def unbind_all(dev_list, force=False):
     """Unbind method, takes a list of device locations"""
 
@@ -482,7 +502,7 @@ def unbind_all(dev_list, force=False):
         unbind_one(d, force)
 
 
-def bind_all(dev_list, driver, force=False):
+def bind_all(dev_list, driver, uid, gid, force=False):
     """Bind method, takes a list of device locations"""
     global devices
 
@@ -510,6 +530,9 @@ def bind_all(dev_list, driver, force=False):
 
     for d in dev_list:
         bind_one(d, driver, force)
+        # if we're binding to vfio-pci, set the IOMMU user/group ownership if one was specified
+        if driver == "vfio-pci" and (uid or gid):
+            own_one(d, uid, gid)
 
     # 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
@@ -662,6 +685,8 @@ def parse_args():
     global status_dev
     global force_flag
     global args
+    global vfio_uid
+    global vfio_gid
 
     parser = argparse.ArgumentParser(
         description='Utility to bind and unbind devices from Linux kernel',
@@ -707,6 +732,12 @@ def parse_args():
         '--unbind',
         action='store_true',
         help="Unbind a device (equivalent to \"-b none\")")
+    parser.add_argument(
+        "-U", "--uid", help="For VFIO, specify the UID to set IOMMU group ownership"
+    )
+    parser.add_argument(
+        "-G", "--gid", help="For VFIO, specify the GID to set IOMMU group ownership"
+    )
     parser.add_argument(
         '--force',
         action='store_true',
@@ -737,6 +768,10 @@ def parse_args():
         b_flag = opt.bind
     elif opt.unbind:
         b_flag = "none"
+    if opt.uid:
+        vfio_uid = opt.uid
+    if opt.gid:
+        vfio_gid = opt.gid
     args = opt.devices
 
     if not b_flag and not status_flag:
@@ -764,11 +799,13 @@ def do_arg_actions():
     global status_flag
     global force_flag
     global args
+    global vfio_uid
+    global vfio_gid
 
     if b_flag in ["none", "None"]:
         unbind_all(args, force_flag)
     elif b_flag is not None:
-        bind_all(args, b_flag, force_flag)
+        bind_all(args, b_flag, vfio_uid, vfio_gid, force_flag)
     if status_flag:
         if b_flag is not None:
             clear_data()
-- 
2.43.5

Re: [PATCH v1 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Burakov, Anatoly]

On 9/2/2024 3:57 PM, Anatoly Burakov wrote:
> 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.burakov@intel.com>
> ---

<snip>

>   
> +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
> +        os.chown(dev_path, _uid, _gid)
> +    except OSError as err:
> +        sys.exit(f"Error: failed to read IOMMU group for {dev_id}: {err}")

On another thought, perhaps sys.exit() here is a bit too drastic... Will 
replace with error message in v2

-- 
Thanks,
Anatoly

[PATCH v2 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Anatoly Burakov]

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.burakov@intel.com>
---

Notes:
    v1 -> v2:
    - Replaced hard exit with an error printout

 usertools/dpdk-devbind.py | 41 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index f2a2a9a12f..496d0e90e8 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -8,6 +8,8 @@
 import subprocess
 import argparse
 import platform
+import grp
+import pwd
 
 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 = ""
 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
+        os.chown(dev_path, _uid, _gid)
+    except OSError as err:
+        print(f"Error: failed to read IOMMU group for {dev_id}: {err}")
+
+
 def unbind_all(dev_list, force=False):
     """Unbind method, takes a list of device locations"""
 
@@ -512,7 +532,7 @@ def check_noiommu_mode():
     print("Warning: enabling unsafe no IOMMU mode for VFIO drivers")
 
 
-def bind_all(dev_list, driver, force=False):
+def bind_all(dev_list, driver, uid, gid, force=False):
     """Bind method, takes a list of device locations"""
     global devices
 
@@ -544,6 +564,9 @@ def bind_all(dev_list, driver, force=False):
 
     for d in dev_list:
         bind_one(d, driver, force)
+        # if we're binding to vfio-pci, set the IOMMU user/group ownership if one was specified
+        if driver == "vfio-pci" and (uid or gid):
+            own_one(d, uid, gid)
 
     # 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"
+    )
+    parser.add_argument(
+        "-G", "--gid", help="For VFIO, specify the GID to set IOMMU group ownership"
+    )
     parser.add_argument(
         '--force',
         action='store_true',
@@ -778,6 +809,10 @@ def parse_args():
         b_flag = opt.bind
     elif opt.unbind:
         b_flag = "none"
+    if opt.uid:
+        vfio_uid = opt.uid
+    if opt.gid:
+        vfio_gid = opt.gid
     args = opt.devices
 
     if not b_flag and not status_flag:
@@ -805,11 +840,13 @@ def do_arg_actions():
     global status_flag
     global force_flag
     global args
+    global vfio_uid
+    global vfio_gid
 
     if b_flag in ["none", "None"]:
         unbind_all(args, force_flag)
     elif b_flag is not None:
-        bind_all(args, b_flag, force_flag)
+        bind_all(args, b_flag, vfio_uid, vfio_gid, force_flag)
     if status_flag:
         if b_flag is not None:
             clear_data()
-- 
2.43.5

Re: [PATCH v2 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Bruce Richardson]

On Tue, Nov 26, 2024 at 03:02:38PM +0000, Anatoly Burakov wrote:
> 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.burakov@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v2 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Robin Jarry]

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

>  usertools/dpdk-devbind.py | 41 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> index f2a2a9a12f..496d0e90e8 100755
> --- a/usertools/dpdk-devbind.py
> +++ b/usertools/dpdk-devbind.py
> @@ -8,6 +8,8 @@
>  import subprocess
>  import argparse
>  import platform
> +import grp
> +import pwd

We may already be past this but could you try to sort these imports 
alphabetically?

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

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

> +        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.

> +
> +
>  def unbind_all(dev_list, force=False):
>      """Unbind method, takes a list of device locations"""
>  
> @@ -512,7 +532,7 @@ def check_noiommu_mode():
>      print("Warning: enabling unsafe no IOMMU mode for VFIO drivers")
>  
>  
> -def bind_all(dev_list, driver, force=False):
> +def bind_all(dev_list, driver, uid, gid, force=False):

Not required. These are global variables.

>      """Bind method, takes a list of device locations"""
>      global devices
>  
> @@ -544,6 +564,9 @@ def bind_all(dev_list, driver, force=False):
>  
>      for d in dev_list:
>          bind_one(d, driver, force)
> +        # if we're binding to vfio-pci, set the IOMMU user/group ownership if one was specified
> +        if driver == "vfio-pci" and (uid or gid):

           if driver == "vfio-pci" and (vfio_uid != -1 or vfio_gid != -1):

> +            own_one(d, uid, gid)

It will be better to store the chmod code path here:

               iommu_grp = os.path.join("/sys/bus/pci/devices", d, "iommu_group")
               iommu_grp = os.path.basename(os.readlink(iommu_grp))
               os.chown(os.path.join("/dev/vfio", iommu_grp), vfio_uid, vfio_gid)

>  
>      # 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,

> +    )
> +    parser.add_argument(
> +        "-G", "--gid", help="For VFIO, specify the GID to set IOMMU group ownership"

In order to fail early if an invalid group name is passed, add these two 
lines:

           type=lambda g: grp.getgrnam(g).gr_gid,
           default=-1,

> +    )
>      parser.add_argument(
>          '--force',
>          action='store_true',
> @@ -778,6 +809,10 @@ def parse_args():
>          b_flag = opt.bind
>      elif opt.unbind:
>          b_flag = "none"
> +    if opt.uid:
> +        vfio_uid = opt.uid
> +    if opt.gid:
> +        vfio_gid = opt.gid

No need for ifs here, the default values are set to -1 which means: "use 
default" for os.chmod().

       vfio_uid = opt.uid
       vfio_gid = opt.gid

>      args = opt.devices
>  
>      if not b_flag and not status_flag:
> @@ -805,11 +840,13 @@ def do_arg_actions():
>      global status_flag
>      global force_flag
>      global args
> +    global vfio_uid
> +    global vfio_gid

The global keyword is not required here.

>  
>      if b_flag in ["none", "None"]:
>          unbind_all(args, force_flag)
>      elif b_flag is not None:
> -        bind_all(args, b_flag, force_flag)
> +        bind_all(args, b_flag, vfio_uid, vfio_gid, force_flag)

Not required. These are global variables.

>      if status_flag:
>          if b_flag is not None:
>              clear_data()
> -- 
> 2.43.5

Thanks.

Re: [PATCH v2 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Burakov, Anatoly]

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

[PATCH v3 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Anatoly Burakov]

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.burakov@intel.com>
---

Notes:
    v2 -> v3:
    - Replaced error printout back to hard exit
    - Reworked UID/GID validation to be at command line parsing
    - Simplified chown code
    
    v1 -> v2:
    - Replaced hard exit with an error printout

 usertools/dpdk-devbind.py | 41 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index f2a2a9a12f..ed1ef0cabc 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -3,11 +3,13 @@
 # Copyright(c) 2010-2014 Intel Corporation
 #
 
-import sys
-import os
-import subprocess
 import argparse
+import grp
+import os
 import platform
+import pwd
+import subprocess
+import sys
 
 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 = -1
+vfio_gid = -1
 args = []
 
 
@@ -544,6 +548,19 @@ def bind_all(dev_list, driver, force=False):
 
     for d in dev_list:
         bind_one(d, driver, force)
+        # if we're binding to vfio-pci, set the IOMMU user/group ownership if one was specified
+        if driver == "vfio-pci" and (vfio_uid != -1 or vfio_gid != -1):
+            # find IOMMU group for a particular PCI device
+            iommu_grp_base_path = os.path.join("/sys/bus/pci/devices", d, "iommu_group")
+            # extract the IOMMU group number
+            iommu_grp = os.path.basename(os.readlink(iommu_grp_base_path))
+            # find VFIO device correspondiong to this IOMMU group
+            dev_path = os.path.join("/dev/vfio", iommu_grp)
+            # set ownership
+            try:
+                os.chown(dev_path, vfio_uid, vfio_gid)
+            except OSError as err:
+                sys.exit(f"Error: failed to set IOMMU group ownership for {d}: {err}")
 
     # 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 +714,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 +765,20 @@ 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",
+        type=lambda u: pwd.getpwnam(u).pw_uid,
+        default=-1,
+    )
+    parser.add_argument(
+        "-G",
+        "--gid",
+        help="For VFIO, specify the GID to set IOMMU group ownership",
+        type=lambda g: grp.getgrnam(g).gr_gid,
+        default=-1,
+    )
     parser.add_argument(
         '--force',
         action='store_true',
@@ -778,6 +811,8 @@ def parse_args():
         b_flag = opt.bind
     elif opt.unbind:
         b_flag = "none"
+    vfio_uid = opt.uid
+    vfio_gid = opt.gid
     args = opt.devices
 
     if not b_flag and not status_flag:
-- 
2.43.5

Re: [PATCH v3 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Robin Jarry]

Anatoly Burakov, Nov 27, 2024 at 10:13:
> 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.burakov@intel.com>
> ---

Reviewed-by: Robin Jarry <rjarry@redhat.com>

Re: [PATCH v3 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Thomas Monjalon]

27/11/2024 10:13, Anatoly Burakov:
> 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.burakov@intel.com>
> ---
> 
> Notes:
>     v2 -> v3:
>     - Replaced error printout back to hard exit
>     - Reworked UID/GID validation to be at command line parsing
>     - Simplified chown code
>     
>     v1 -> v2:
>     - Replaced hard exit with an error printout
> 
>  usertools/dpdk-devbind.py | 41 ++++++++++++++++++++++++++++++++++++---

Please could you update the documentation?
	doc/guides/tools/devbind.rst

[PATCH v4 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Anatoly Burakov]

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.burakov@intel.com>
---

Notes:
    v3 -> v4:
    - Added documentation
    
    v2 -> v3:
    - Replaced error printout back to hard exit
    - Reworked UID/GID validation to be at command line parsing
    - Simplified chown code
    
    v1 -> v2:
    - Replaced hard exit with an error printout

 doc/guides/tools/devbind.rst |  6 ++++++
 usertools/dpdk-devbind.py    | 41 +++++++++++++++++++++++++++++++++---
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/doc/guides/tools/devbind.rst b/doc/guides/tools/devbind.rst
index df4f3505ac..841615570f 100644
--- a/doc/guides/tools/devbind.rst
+++ b/doc/guides/tools/devbind.rst
@@ -56,6 +56,12 @@ OPTIONS
         WARNING: This can lead to loss of network connection and should be used
         with caution.
 
+* ``--uid uid, --gid gid``
+
+      By default, devices which are bound to VFIO will be owned by ``root``.
+      Use this flag to change ownership to the specified user and group, so that
+      devices bound to VFIO would be usable by unprivileged users.
+
 
 .. warning::
 
diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index f2a2a9a12f..ed1ef0cabc 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -3,11 +3,13 @@
 # Copyright(c) 2010-2014 Intel Corporation
 #
 
-import sys
-import os
-import subprocess
 import argparse
+import grp
+import os
 import platform
+import pwd
+import subprocess
+import sys
 
 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 = -1
+vfio_gid = -1
 args = []
 
 
@@ -544,6 +548,19 @@ def bind_all(dev_list, driver, force=False):
 
     for d in dev_list:
         bind_one(d, driver, force)
+        # if we're binding to vfio-pci, set the IOMMU user/group ownership if one was specified
+        if driver == "vfio-pci" and (vfio_uid != -1 or vfio_gid != -1):
+            # find IOMMU group for a particular PCI device
+            iommu_grp_base_path = os.path.join("/sys/bus/pci/devices", d, "iommu_group")
+            # extract the IOMMU group number
+            iommu_grp = os.path.basename(os.readlink(iommu_grp_base_path))
+            # find VFIO device correspondiong to this IOMMU group
+            dev_path = os.path.join("/dev/vfio", iommu_grp)
+            # set ownership
+            try:
+                os.chown(dev_path, vfio_uid, vfio_gid)
+            except OSError as err:
+                sys.exit(f"Error: failed to set IOMMU group ownership for {d}: {err}")
 
     # 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 +714,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 +765,20 @@ 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",
+        type=lambda u: pwd.getpwnam(u).pw_uid,
+        default=-1,
+    )
+    parser.add_argument(
+        "-G",
+        "--gid",
+        help="For VFIO, specify the GID to set IOMMU group ownership",
+        type=lambda g: grp.getgrnam(g).gr_gid,
+        default=-1,
+    )
     parser.add_argument(
         '--force',
         action='store_true',
@@ -778,6 +811,8 @@ def parse_args():
         b_flag = opt.bind
     elif opt.unbind:
         b_flag = "none"
+    vfio_uid = opt.uid
+    vfio_gid = opt.gid
     args = opt.devices
 
     if not b_flag and not status_flag:
-- 
2.43.5

Re: [PATCH v4 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Burakov, Anatoly]

On 12/2/2024 10:31 AM, Anatoly Burakov wrote:
> 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.burakov@intel.com>
> ---

Missed

Reviewed-by: Robin Jarry <rjarry@redhat.com>

-- 
Thanks,
Anatoly

Re: [PATCH v4 1/1] usertools/devbind: allow changing UID/GID for VFIO

[Burakov, Anatoly]

On 12/2/2024 10:31 AM, Anatoly Burakov wrote:
> 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.burakov@intel.com>
> ---

This functionality is also included in my devbind rewrite:

https://patches.dpdk.org/project/dpdk/list/?series=34098

-- 
Thanks,
Anatoly