[v2] usertools/devbind: fix binding for built-in 1kernel drivers

Message ID 20201118025854.13455-1-yongxin.liu@windriver.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] usertools/devbind: fix binding for built-in 1kernel drivers |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing warning Testing issues
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Liu, Yongxin Nov. 18, 2020, 2:58 a.m. UTC
  In commit 681a67288655 ("usertools: check if module is loaded before
binding"), script will exit if no driver is found in /sys/module/.

However, for built-in kernel driver, /sys/module/MODULENAME only
shows up if it has a version or at least one parameter. Take ixgbe
for example, after kernel commit 34a2a3b83e2c ("net/intel: remove
driver versions from Intel drivers"), and if ixgbe is built directly
into kernel, there is no ixgbe folder in /sys/module. So the devbind
script should not exit.

Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com>
---

v2: 
 - fix git commit description style in commit log
 - fix typo spelling

---
 usertools/dpdk-devbind.py | 4 ----
 1 file changed, 4 deletions(-)
  

Comments

Anatoly Burakov Nov. 18, 2020, 4:27 p.m. UTC | #1
On 18-Nov-20 2:58 AM, Yongxin Liu wrote:
> In commit 681a67288655 ("usertools: check if module is loaded before
> binding"), script will exit if no driver is found in /sys/module/.
> 
> However, for built-in kernel driver, /sys/module/MODULENAME only
> shows up if it has a version or at least one parameter. Take ixgbe
> for example, after kernel commit 34a2a3b83e2c ("net/intel: remove
> driver versions from Intel drivers"), and if ixgbe is built directly
> into kernel, there is no ixgbe folder in /sys/module. So the devbind
> script should not exit.
> 
> Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com>
> ---
> 
> v2:
>   - fix git commit description style in commit log
>   - fix typo spelling
> 
> ---
>   usertools/dpdk-devbind.py | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> index 99112b7ab..f3c0d9814 100755
> --- a/usertools/dpdk-devbind.py
> +++ b/usertools/dpdk-devbind.py
> @@ -530,10 +530,6 @@ def bind_all(dev_list, driver, force=False):
>           # driver generated error - it's not a valid device ID, so all is well
>           pass
>   
> -    # check if we're attempting to bind to a driver that isn't loaded
> -    if not module_is_loaded(driver.replace('-','_')):
> -        sys.exit("Error: Driver '%s' is not loaded." % driver)
> -

I believe there is a way to check if module is built-in, can't we use 
that? We could keep a list of built-in modules of interest that we can 
get from:

cat /lib/modules/$(uname -r)/modules.builtin

It's a bit more changes, but this is better than just removing the error 
check.

>       try:
>           dev_list = map(dev_id_from_dev_name, dev_list)
>       except ValueError as ex:
>
  
Liu, Yongxin Nov. 19, 2020, 7:24 a.m. UTC | #2
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Thursday, November 19, 2020 00:28
> To: Liu, Yongxin <Yongxin.Liu@windriver.com>; dev@dpdk.org;
> thomas@monjalon.net
> Subject: Re: [dpdk-dev] [PATCH v2] usertools/devbind: fix binding for
> built-in 1kernel drivers
> 
> 
> On 18-Nov-20 2:58 AM, Yongxin Liu wrote:
> > In commit 681a67288655 ("usertools: check if module is loaded before
> > binding"), script will exit if no driver is found in /sys/module/.
> >
> > However, for built-in kernel driver, /sys/module/MODULENAME only shows
> > up if it has a version or at least one parameter. Take ixgbe for
> > example, after kernel commit 34a2a3b83e2c ("net/intel: remove driver
> > versions from Intel drivers"), and if ixgbe is built directly into
> > kernel, there is no ixgbe folder in /sys/module. So the devbind script
> > should not exit.
> >
> > Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com>
> > ---
> >
> > v2:
> >   - fix git commit description style in commit log
> >   - fix typo spelling
> >
> > ---
> >   usertools/dpdk-devbind.py | 4 ----
> >   1 file changed, 4 deletions(-)
> >
> > diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> > index 99112b7ab..f3c0d9814 100755
> > --- a/usertools/dpdk-devbind.py
> > +++ b/usertools/dpdk-devbind.py
> > @@ -530,10 +530,6 @@ def bind_all(dev_list, driver, force=False):
> >           # driver generated error - it's not a valid device ID, so all
> is well
> >           pass
> >
> > -    # check if we're attempting to bind to a driver that isn't loaded
> > -    if not module_is_loaded(driver.replace('-','_')):
> > -        sys.exit("Error: Driver '%s' is not loaded." % driver)
> > -
> 
> I believe there is a way to check if module is built-in, can't we use that?
> We could keep a list of built-in modules of interest that we can get from:
> 
> cat /lib/modules/$(uname -r)/modules.builtin
> 
> It's a bit more changes, but this is better than just removing the error
> check.

Thanks Anatoly for your advice.
I have sent v3. Please review.

/Yongxin

> 
> >       try:
> >           dev_list = map(dev_id_from_dev_name, dev_list)
> >       except ValueError as ex:
> >
> 
> 
> --
> Thanks,
> Anatoly
  

Patch

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 99112b7ab..f3c0d9814 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -530,10 +530,6 @@  def bind_all(dev_list, driver, force=False):
         # driver generated error - it's not a valid device ID, so all is well
         pass
 
-    # check if we're attempting to bind to a driver that isn't loaded
-    if not module_is_loaded(driver.replace('-','_')):
-        sys.exit("Error: Driver '%s' is not loaded." % driver)
-
     try:
         dev_list = map(dev_id_from_dev_name, dev_list)
     except ValueError as ex: