[PATCH v2] usertools: add check for IOMMU support in dpdk-devbind

[PATCH v2] usertools: add check for IOMMU support in dpdk-devbind

[Fidaullah Noonari]

binding with vfio driver, when IOMMU is disabled, causes program to crash.
this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
disabled, it changes vfio into unsafe noiommu mode and prints warning
message.

Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
---
 usertools/dpdk-devbind.py | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index ace4627218..91fa4c7620 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -96,6 +96,7 @@
 b_flag = None
 status_flag = False
 force_flag = False
+noiommu_flag = False
 args = []
 
 
@@ -466,9 +467,30 @@ def unbind_all(dev_list, force=False):
         unbind_one(d, force)
 
 
+def check_iommu():
+    """Check if IOMMU is enabled on system"""
+    if len(os.listdir("/sys/class/iommu")) != 0:
+        return True
+    return False
+
+
+def enable_noiommu_mode():
+    """Enables the noiommu mode for vfio drivers"""
+    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
+    try:
+        f = open(filename, "w")
+    except OSError as err:
+        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
+                % (filename, err))
+    f.write("1")
+    f.close()
+    print("Warning: enabling unsafe no IOMMU mode for vfio drivers")
+
+
 def bind_all(dev_list, driver, force=False):
     """Bind method, takes a list of device locations"""
     global devices
+    global noiommu_flag
 
     # a common user error is to forget to specify the driver the devices need to
     # be bound to. check if the driver is a valid device, and if it is, show
@@ -492,6 +514,14 @@ def bind_all(dev_list, driver, force=False):
     except ValueError as ex:
         sys.exit(ex)
 
+    # check for IOMMU support
+    if not check_iommu():
+        if noiommu_flag:
+            enable_noiommu_mode()
+        elif driver == "vfio-pci":
+            print("Warning: IOMMU support is disabled")
+            print("Info: use --noiommu-mode for binding in noiommu mode")
+
     for d in dev_list:
         bind_one(d, driver, force)
 
@@ -634,6 +664,7 @@ def parse_args():
     global status_flag
     global status_dev
     global force_flag
+    global noiommu_flag
     global args
 
     parser = argparse.ArgumentParser(
@@ -687,6 +718,12 @@ def parse_args():
 Override restriction on binding devices in use by Linux"
 WARNING: This can lead to loss of network connection and should be used with caution.
 """)
+    parser.add_argument(
+        '--noiommu-mode',
+        action='store_true',
+        help="""
+if IOMMU is not available, Enables no IOMMU mode for vfio drivers.
+        """)
     parser.add_argument(
         'devices',
         metavar='DEVICE',
@@ -706,6 +743,8 @@ def parse_args():
         status_dev = "all"
     if opt.force:
         force_flag = True
+    if opt.noiommu_mode:
+        noiommu_flag = True
     if opt.bind:
         b_flag = opt.bind
     elif opt.unbind:
-- 
2.25.1

Re: [PATCH v2] usertools: add check for IOMMU support in dpdk-devbind

[Bruce Richardson]

On Tue, Mar 15, 2022 at 11:26:52AM +0500, Fidaullah Noonari wrote:
> binding with vfio driver, when IOMMU is disabled, causes program to crash.
> this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
> disabled, it changes vfio into unsafe noiommu mode and prints warning
> message.
> 
> Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> ---

Hi,

patch generally looks good, but some more feedback inline below. The main
thing missing is that we should probably also check the current value of
the vfio noiommu flag and take that into account when printing warnings or
errors.

/Bruce

>  usertools/dpdk-devbind.py | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> index ace4627218..91fa4c7620 100755
> --- a/usertools/dpdk-devbind.py
> +++ b/usertools/dpdk-devbind.py
> @@ -96,6 +96,7 @@
>  b_flag = None
>  status_flag = False
>  force_flag = False
> +noiommu_flag = False
>  args = []
>  
>  
> @@ -466,9 +467,30 @@ def unbind_all(dev_list, force=False):
>          unbind_one(d, force)
>  
>  
> +def check_iommu():
> +    """Check if IOMMU is enabled on system"""
> +    if len(os.listdir("/sys/class/iommu")) != 0:
> +        return True
> +    return False

I'd suggest renaming the function for clarity, since "check_iommu" is
a little unclear as to what the return value is i.e. false could mean you
can't check, rather than it doesn't have one. "has_iommu()" might be
clearer.

Can probably be shortened to one-line body:

    def has_iommu():
        """..."""
        return len(os.listdir("/sys/class/iommu")) > 0

> +
> +
> +def enable_noiommu_mode():
> +    """Enables the noiommu mode for vfio drivers"""
> +    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
> +    try:
> +        f = open(filename, "w")
> +    except OSError as err:
> +        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
> +                % (filename, err))
> +    f.write("1")
> +    f.close()
> +    print("Warning: enabling unsafe no IOMMU mode for vfio drivers")
> +
> +
>  def bind_all(dev_list, driver, force=False):
>      """Bind method, takes a list of device locations"""
>      global devices
> +    global noiommu_flag
>  
>      # a common user error is to forget to specify the driver the devices need to
>      # be bound to. check if the driver is a valid device, and if it is, show
> @@ -492,6 +514,14 @@ def bind_all(dev_list, driver, force=False):
>      except ValueError as ex:
>          sys.exit(ex)
>  
> +    # check for IOMMU support
> +    if not check_iommu():
> +        if noiommu_flag:
> +            enable_noiommu_mode()
> +        elif driver == "vfio-pci":
> +            print("Warning: IOMMU support is disabled")
> +            print("Info: use --noiommu-mode for binding in noiommu mode")
> +

Rather than just warning here, I think this needs a little extra check to
see if no-iommu mode is already enabled. If so, just continue, if it is
not, then the warning should be an error and no binding should be
attempted.

>      for d in dev_list:
>          bind_one(d, driver, force)
>  
> @@ -634,6 +664,7 @@ def parse_args():
>      global status_flag
>      global status_dev
>      global force_flag
> +    global noiommu_flag
>      global args
>  
>      parser = argparse.ArgumentParser(
> @@ -687,6 +718,12 @@ def parse_args():
>  Override restriction on binding devices in use by Linux"
>  WARNING: This can lead to loss of network connection and should be used with caution.
>  """)
> +    parser.add_argument(
> +        '--noiommu-mode',
> +        action='store_true',
> +        help="""
> +if IOMMU is not available, Enables no IOMMU mode for vfio drivers.
> +        """)
>      parser.add_argument(
>          'devices',
>          metavar='DEVICE',
> @@ -706,6 +743,8 @@ def parse_args():
>          status_dev = "all"
>      if opt.force:
>          force_flag = True
> +    if opt.noiommu_mode:
> +        noiommu_flag = True
>      if opt.bind:
>          b_flag = opt.bind
>      elif opt.unbind:
> -- 
> 2.25.1
> 

[PATCH v3] usertools: add check for IOMMU support in dpdk-devbind

[Fidaullah Noonari]

binding with vfio driver, when IOMMU is disabled, causes program to crash.
this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
disabled, it changes vfio into unsafe noiommu mode and prints warning
message.

Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
---
 usertools/dpdk-devbind.py | 47 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index ace4627218..e2181efac8 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -96,6 +96,7 @@
 b_flag = None
 status_flag = False
 force_flag = False
+noiommu_flag = False
 args = []
 
 
@@ -466,6 +467,39 @@ def unbind_all(dev_list, force=False):
         unbind_one(d, force)
 
 
+def has_iommu():
+    """Check if IOMMU is enabled on system"""
+    return len(os.listdir("/sys/class/iommu")) > 0
+
+
+def check_noiommu_mode():
+    """checks and enables the noiommu mode for vfio drivers"""
+    global noiommu_flag
+    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
+
+    try:
+        with open(filename,"r") as f:
+            if f.read(1) == "1":
+                return
+    except OSError as err:
+        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
+                % (filename, err))
+
+    if not noiommu_flag:
+        print("Error: failed to bind vfio-pci - IOMMU support is disabled")
+        print("Info: use --noiommu-mode for binding in noiommu mode")
+        sys.exit()
+
+    try:
+        with open(filename, "w") as f:
+            f.write("1")
+            f.close()
+    except OSError as err:
+        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
+                % (filename, err))
+    print("Warning: enabling unsafe no IOMMU mode for vfio drivers")
+
+
 def bind_all(dev_list, driver, force=False):
     """Bind method, takes a list of device locations"""
     global devices
@@ -492,6 +526,10 @@ def bind_all(dev_list, driver, force=False):
     except ValueError as ex:
         sys.exit(ex)
 
+    # check for IOMMU support
+    if driver == "vfio-pci" and not has_iommu():
+        check_noiommu_mode()
+
     for d in dev_list:
         bind_one(d, driver, force)
 
@@ -634,6 +672,7 @@ def parse_args():
     global status_flag
     global status_dev
     global force_flag
+    global noiommu_flag
     global args
 
     parser = argparse.ArgumentParser(
@@ -687,6 +726,12 @@ def parse_args():
 Override restriction on binding devices in use by Linux"
 WARNING: This can lead to loss of network connection and should be used with caution.
 """)
+    parser.add_argument(
+        '--noiommu-mode',
+        action='store_true',
+        help="""
+if IOMMU is not available, Enables no IOMMU mode for vfio drivers.
+        """)
     parser.add_argument(
         'devices',
         metavar='DEVICE',
@@ -706,6 +751,8 @@ def parse_args():
         status_dev = "all"
     if opt.force:
         force_flag = True
+    if opt.noiommu_mode:
+        noiommu_flag = True
     if opt.bind:
         b_flag = opt.bind
     elif opt.unbind:
-- 
2.25.1

Re: [PATCH v3] usertools: add check for IOMMU support in dpdk-devbind

[Burakov, Anatoly]

On 21-Mar-22 12:27 PM, Fidaullah Noonari wrote:
> binding with vfio driver, when IOMMU is disabled, causes program to crash.
> this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
> disabled, it changes vfio into unsafe noiommu mode and prints warning
> message.
> 
> Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> ---

Hi,

> +def check_noiommu_mode():
> +    """checks and enables the noiommu mode for vfio drivers"""
> +    global noiommu_flag
> +    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
> +
> +    try:
> +        with open(filename,"r") as f:
> +            if f.read(1) == "1":
> +                return
> +    except OSError as err:
> +        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
> +                % (filename, err))

Nitpick, but maybe use f-strings instead of old-school string interpolation?

> +
> +    if not noiommu_flag:
> +        print("Error: failed to bind vfio-pci - IOMMU support is disabled")
> +        print("Info: use --noiommu-mode for binding in noiommu mode")
> +        sys.exit()
> +
> +    try:
> +        with open(filename, "w") as f:
> +            f.write("1")
> +            f.close()
> +    except OSError as err:
> +        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
> +                % (filename, err))

Same as above.

Otherwise LGTM,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [PATCH v3] usertools: add check for IOMMU support in dpdk-devbind

[Bruce Richardson]

On Thu, Mar 31, 2022 at 03:37:40PM +0100, Burakov, Anatoly wrote:
> On 21-Mar-22 12:27 PM, Fidaullah Noonari wrote:
> > binding with vfio driver, when IOMMU is disabled, causes program to crash.
> > this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
> > disabled, it changes vfio into unsafe noiommu mode and prints warning
> > message.
> > 
> > Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> > ---
> 
> Hi,
> 
> > +def check_noiommu_mode():
> > +    """checks and enables the noiommu mode for vfio drivers"""
> > +    global noiommu_flag
> > +    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
> > +
> > +    try:
> > +        with open(filename,"r") as f:
> > +            if f.read(1) == "1":
> > +                return
> > +    except OSError as err:
> > +        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
> > +                % (filename, err))
> 
> Nitpick, but maybe use f-strings instead of old-school string interpolation?
> 

Do we still not need to support some distros that don't have Python earlier
than 3.6, where this support was added? 

Re: [PATCH v3] usertools: add check for IOMMU support in dpdk-devbind

[Thomas Monjalon]

31/03/2022 16:44, Bruce Richardson:
> On Thu, Mar 31, 2022 at 03:37:40PM +0100, Burakov, Anatoly wrote:
> > On 21-Mar-22 12:27 PM, Fidaullah Noonari wrote:
> > > binding with vfio driver, when IOMMU is disabled, causes program to crash.
> > > this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
> > > disabled, it changes vfio into unsafe noiommu mode and prints warning
> > > message.
> > > 
> > > Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> > > ---
> > 
> > Hi,
> > 
> > > +def check_noiommu_mode():
> > > +    """checks and enables the noiommu mode for vfio drivers"""
> > > +    global noiommu_flag
> > > +    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
> > > +
> > > +    try:
> > > +        with open(filename,"r") as f:
> > > +            if f.read(1) == "1":
> > > +                return
> > > +    except OSError as err:
> > > +        sys.exit("Error: failed to enable unsafe noiommu mode - Cannot open %s: %s"
> > > +                % (filename, err))
> > 
> > Nitpick, but maybe use f-strings instead of old-school string interpolation?
> > 
> 
> Do we still not need to support some distros that don't have Python earlier
> than 3.6, where this support was added? 

The requirements have been upgraded to Python 3.6 with the new pmdinfo.

Are we waiting for a new version?

[PATCH v4] usertools: add check for IOMMU support in dpdk-devbind

[Fidaullah Noonari]

binding with vfio driver, when IOMMU is disabled, causes program to crash.
this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
disabled, it changes vfio into unsafe noiommu mode and prints warning
message.

Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
---
 usertools/dpdk-devbind.py | 45 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 4d9c1be666..d875ff23d6 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -100,6 +100,7 @@
 b_flag = None
 status_flag = False
 force_flag = False
+noiommu_flag = False
 args = []
 
 
@@ -470,6 +471,37 @@ def unbind_all(dev_list, force=False):
         unbind_one(d, force)
 
 
+def has_iommu():
+    """Check if IOMMU is enabled on system"""
+    return len(os.listdir("/sys/class/iommu")) > 0
+
+
+def check_noiommu_mode():
+    """checks and enables the noiommu mode for vfio drivers"""
+    global noiommu_flag
+    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
+
+    try:
+        with open(filename,"r") as f:
+            if f.read(1) == "1":
+                return
+    except OSError as err:
+        sys.exit(f"Error: failed to enable unsafe noiommu mode - Cannot open {filename}: {err}")
+
+    if not noiommu_flag:
+        print("Error: failed to bind vfio-pci - IOMMU support is disabled")
+        print("Info: use --noiommu-mode for binding in noiommu mode")
+        sys.exit()
+
+    try:
+        with open(filename, "w") as f:
+            f.write("1")
+            f.close()
+    except OSError as err:
+        sys.exit(f"Error: failed to enable unsafe noiommu mode - Cannot open {filename}: {err}")
+    print("Warning: enabling unsafe no IOMMU mode for vfio drivers")
+
+
 def bind_all(dev_list, driver, force=False):
     """Bind method, takes a list of device locations"""
     global devices
@@ -496,6 +528,10 @@ def bind_all(dev_list, driver, force=False):
     except ValueError as ex:
         sys.exit(ex)
 
+    # check for IOMMU support
+    if driver == "vfio-pci" and not has_iommu():
+        check_noiommu_mode()
+
     for d in dev_list:
         bind_one(d, driver, force)
 
@@ -638,6 +674,7 @@ def parse_args():
     global status_flag
     global status_dev
     global force_flag
+    global noiommu_flag
     global args
 
     parser = argparse.ArgumentParser(
@@ -691,6 +728,12 @@ def parse_args():
 Override restriction on binding devices in use by Linux"
 WARNING: This can lead to loss of network connection and should be used with caution.
 """)
+    parser.add_argument(
+        '--noiommu-mode',
+        action='store_true',
+        help="""
+if IOMMU is not available, Enables no IOMMU mode for vfio drivers.
+        """)
     parser.add_argument(
         'devices',
         metavar='DEVICE',
@@ -710,6 +753,8 @@ def parse_args():
         status_dev = "all"
     if opt.force:
         force_flag = True
+    if opt.noiommu_mode:
+        noiommu_flag = True
     if opt.bind:
         b_flag = opt.bind
     elif opt.unbind:
-- 
2.25.1

Re: [PATCH v3] usertools: add check for IOMMU support in dpdk-devbind

[Stephen Hemminger]

On Mon, 21 Mar 2022 17:27:27 +0500
Fidaullah Noonari <fidaullah.noonari@emumba.com> wrote:

> binding with vfio driver, when IOMMU is disabled, causes program to crash.
> this patch adds a flag for noiommmu-mode. when this is set, if IOMMU is
> disabled, it changes vfio into unsafe noiommu mode and prints warning
> message.
> 
> Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> ---

Minor indentation issues reported by flake8 python checker:

./usertools/dpdk-devbind.py:489:27: E231 missing whitespace after ','
./usertools/dpdk-devbind.py:494:17: E128 continuation line under-indented for visual indent
./usertools/dpdk-devbind.py:507:17: E128 continuation line under-indented for visual indent

Re: [PATCH v3] usertools: add check for IOMMU support in dpdk-devbind

[Stephen Hemminger]

On Mon, 21 Mar 2022 17:27:27 +0500
Fidaullah Noonari <fidaullah.noonari@emumba.com> wrote:

> +
> +def check_noiommu_mode():
> +    """checks and enables the noiommu mode for vfio drivers"""
> +    global noiommu_flag
> +    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
> +
> +    try:
> +        with open(filename,"r") as f:
> +            if f.read(1) == "1":
> +                return

Note: flake8 reports missing comma after filename in the open() statement.

Overall
Acked-by: Stephen Hemminger <stephen@networkplumber.org>