[dpdk-dev] [PATCH v2 1/2] tools: Add support for handling built-in kernel modules

[dpdk-dev] [PATCH v2 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently dpdk_nic_bind.py detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
---
 tools/dpdk_nic_bind.py | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
index f02454e..cb1ac80 100755
--- a/tools/dpdk_nic_bind.py
+++ b/tools/dpdk_nic_bind.py
@@ -33,7 +33,7 @@
 #
 
 import sys, os, getopt, subprocess
-from os.path import exists, abspath, dirname, basename
+from os.path import exists, abspath, dirname, basename, join
 
 
 # The PCI device class for ETHERNET devices
@@ -173,6 +173,26 @@ def check_modules():
             elif line.replace("_", "-").startswith(mod["Name"]):
                 mod["Found"] = True
 
+    try:
+        # Get list of syfs modules, some of them might be builtin and merge with mods
+        sysfs_path = '/sys/module/'
+
+        # Get the list of directories in sysfs_path
+        sysfs_mods = [os.path.join(sysfs_path,o) for o in os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]
+
+        # Extract the last element of '/sys/module/abc' in the array
+        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
+
+        # special case for vfio_pci (module is named vfio-pci,
+        # but its .ko is named vfio_pci)
+        sysfs_mods = map(lambda a: a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
+
+        for mod in mods:
+            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
+                mod["Found"] = True
+    except:
+        pass
+
     # check if we have at least one loaded module
     if True not in [mod["Found"] for mod in mods] and b_flag is not None:
         if b_flag in dpdk_drivers:
-- 
2.5.0

[dpdk-dev] [PATCH v2 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently rte_eal_check_module() detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 635ec36..6cab906 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -52,6 +52,8 @@
 #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
 #include <sys/io.h>
 #endif
+#include <sys/types.h>
+#include <sys/stat.h>
 
 #include <rte_common.h>
 #include <rte_debug.h>
@@ -902,7 +904,10 @@ int
 rte_eal_check_module(const char *module_name)
 {
 	char mod_name[30]; /* Any module names can be longer than 30 bytes? */
+	char sysfs_mod_name[PATH_MAX];
+	struct stat st;
 	int ret = 0;
+	int rv;
 	int n;
 
 	if (NULL == module_name)
@@ -918,9 +923,23 @@ rte_eal_check_module(const char *module_name)
 		n = fscanf(fd, "%29s %*[^\n]", mod_name);
 		if ((n == 1) && !strcmp(mod_name, module_name)) {
 			ret = 1;
-			break;
+			goto finish;
 		}
 	}
+	RTE_LOG(DEBUG, EAL, "Module %s not found in /proc/modules",
+	        module_name);
+
+	/* A module might be builtin, try sysfs */
+	snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
+	if ((rv = stat(sysfs_mod_name, &st)) == 0) {
+		ret = 1;
+		goto finish;
+	}
+
+	RTE_LOG(DEBUG, EAL, "Open %s failed! error %i (%s)\n",
+	        sysfs_mod_name, errno, strerror(errno));
+
+finish:
 	fclose(fd);
 
 	return ret;
-- 
2.5.0

Re: [dpdk-dev] [PATCH v2 2/2] eal/linux: Add support for handling built-in kernel modules

[David Marchand]

Hello Kamil,

On Mon, Dec 7, 2015 at 5:57 PM, Kamil Rytarowski <
Kamil.Rytarowski@caviumnetworks.com> wrote:

> Currently rte_eal_check_module() detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
>
> Add support for checking built-in modules with parsing the sysfs files
>
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> ---
>  lib/librte_eal/linuxapp/eal/eal.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 635ec36..6cab906 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -52,6 +52,8 @@
>  #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
>  #include <sys/io.h>
>  #endif
> +#include <sys/types.h>
> +#include <sys/stat.h>
>
>  #include <rte_common.h>
>  #include <rte_debug.h>
> @@ -902,7 +904,10 @@ int
>  rte_eal_check_module(const char *module_name)
>  {
>         char mod_name[30]; /* Any module names can be longer than 30
> bytes? */
> +       char sysfs_mod_name[PATH_MAX];
> +       struct stat st;
>         int ret = 0;
> +       int rv;
>         int n;
>
>         if (NULL == module_name)
> @@ -918,9 +923,23 @@ rte_eal_check_module(const char *module_name)
>                 n = fscanf(fd, "%29s %*[^\n]", mod_name);
>                 if ((n == 1) && !strcmp(mod_name, module_name)) {
>                         ret = 1;
> -                       break;
> +                       goto finish;
>                 }
>         }
> +       RTE_LOG(DEBUG, EAL, "Module %s not found in /proc/modules",
> +               module_name);
> +
> +       /* A module might be builtin, try sysfs */
> +       snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
> +       if ((rv = stat(sysfs_mod_name, &st)) == 0) {
> +               ret = 1;
> +               goto finish;
> +       }
> +
> +       RTE_LOG(DEBUG, EAL, "Open %s failed! error %i (%s)\n",
> +               sysfs_mod_name, errno, strerror(errno));
> +
> +finish:
>         fclose(fd);
>
>         return ret;
>

Well, in the end, won't all modules end up in /sys/module ?
So, I would say we can get rid of /proc/modules parsing.

The only thing that bothers me is this comment in the kernel documentation :


/sys/module/MODULENAME
                The name of the module that is in the kernel.
This
                module name will always show up if the module is loaded as
a
                dynamic module.  If it is built directly into the kernel,
it
                will only show up if it has a version or at least
one

parameter.


                Note: The conditions of creation in the built-in case are
not
                by design and may be removed in the
future.

But, at the moment, I suppose we are fine.


-- 
David Marchand

[dpdk-dev] [PATCH v3 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently dpdk_nic_bind.py detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files.

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
---
 tools/dpdk_nic_bind.py | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
index f02454e..0318598 100755
--- a/tools/dpdk_nic_bind.py
+++ b/tools/dpdk_nic_bind.py
@@ -33,7 +33,7 @@
 #
 
 import sys, os, getopt, subprocess
-from os.path import exists, abspath, dirname, basename
+from os.path import exists, abspath, dirname, basename, join
 
 
 # The PCI device class for ETHERNET devices
@@ -156,22 +156,29 @@ def check_modules():
     '''Checks that igb_uio is loaded'''
     global dpdk_drivers
 
-    fd = file("/proc/modules")
-    loaded_mods = fd.readlines()
-    fd.close()
-
     # list of supported modules
     mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
 
     # first check if module is loaded
-    for line in loaded_mods:
+    try:
+        # Get list of syfs modules, some of them might be builtin and merge with mods
+        sysfs_path = '/sys/module/'
+
+        # Get the list of directories in sysfs_path
+        sysfs_mods = [os.path.join(sysfs_path,o) for o in os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]
+
+        # Extract the last element of '/sys/module/abc' in the array
+        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
+
+        # special case for vfio_pci (module is named vfio-pci,
+        # but its .ko is named vfio_pci)
+        sysfs_mods = map(lambda a: a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
+
         for mod in mods:
-            if line.startswith(mod["Name"]):
-                mod["Found"] = True
-            # special case for vfio_pci (module is named vfio-pci,
-            # but its .ko is named vfio_pci)
-            elif line.replace("_", "-").startswith(mod["Name"]):
+            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
                 mod["Found"] = True
+    except:
+        pass
 
     # check if we have at least one loaded module
     if True not in [mod["Found"] for mod in mods] and b_flag is not None:
-- 
2.5.0

[dpdk-dev] [PATCH v3 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently rte_eal_check_module() detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 635ec36..539188f 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -52,6 +52,8 @@
 #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
 #include <sys/io.h>
 #endif
+#include <sys/types.h>
+#include <sys/stat.h>
 
 #include <rte_common.h>
 #include <rte_debug.h>
@@ -901,27 +903,27 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_check_module(const char *module_name)
 {
-	char mod_name[30]; /* Any module names can be longer than 30 bytes? */
-	int ret = 0;
-	int n;
+	char sysfs_mod_name[PATH_MAX];
+	struct stat st;
 
 	if (NULL == module_name)
 		return -1;
 
-	FILE *fd = fopen("/proc/modules", "r");
-	if (NULL == fd) {
-		RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
-			" error %i (%s)\n", errno, strerror(errno));
+	/* Check if there is sysfs mounted */
+	if (stat("/sys/module", &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "Open /sys/module failed: %s\n",
+			strerror(errno));
 		return -1;
 	}
-	while (!feof(fd)) {
-		n = fscanf(fd, "%29s %*[^\n]", mod_name);
-		if ((n == 1) && !strcmp(mod_name, module_name)) {
-			ret = 1;
-			break;
-		}
+
+	/* A module might be built-in, therefore try sysfs */
+	snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
+	if (stat(sysfs_mod_name, &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "Open %s failed! error %i (%s)\n",
+			sysfs_mod_name, errno, strerror(errno));
+		return 0;
 	}
-	fclose(fd);
 
-	return ret;
+	/* Module has been found */
+	return 1;
 }
-- 
2.5.0

Re: [dpdk-dev] [PATCH v3 2/2] eal/linux: Add support for handling built-in kernel modules

[David Marchand]

On Mon, Dec 7, 2015 at 7:36 PM, Kamil Rytarowski <
Kamil.Rytarowski@caviumnetworks.com> wrote:

> Currently rte_eal_check_module() detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
>
> Add support for checking built-in modules with parsing the sysfs files
>
> This commit obsoletes the /proc/modules parsing approach.
>
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> ---
>  lib/librte_eal/linuxapp/eal/eal.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 635ec36..539188f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -52,6 +52,8 @@
>  #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
>  #include <sys/io.h>
>  #endif
> +#include <sys/types.h>
> +#include <sys/stat.h>
>
>  #include <rte_common.h>
>  #include <rte_debug.h>
> @@ -901,27 +903,27 @@ int rte_eal_has_hugepages(void)
>  int
>  rte_eal_check_module(const char *module_name)
>  {
> -       char mod_name[30]; /* Any module names can be longer than 30
> bytes? */
> -       int ret = 0;
> -       int n;
> +       char sysfs_mod_name[PATH_MAX];
> +       struct stat st;
>
>         if (NULL == module_name)
>                 return -1;
>
> -       FILE *fd = fopen("/proc/modules", "r");
> -       if (NULL == fd) {
> -               RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
> -                       " error %i (%s)\n", errno, strerror(errno));
> +       /* Check if there is sysfs mounted */
> +       if (stat("/sys/module", &st) != 0) {
> +               RTE_LOG(DEBUG, EAL, "Open /sys/module failed: %s\n",
> +                       strerror(errno));
>                 return -1;
>         }
>

Not sure making a difference between /sys/module and /sys/module/XXX
presence really helps, but this is the same philosophy as how it was done
before...
So, ok let's go with this.



> -       while (!feof(fd)) {
> -               n = fscanf(fd, "%29s %*[^\n]", mod_name);
> -               if ((n == 1) && !strcmp(mod_name, module_name)) {
> -                       ret = 1;
> -                       break;
> -               }
> +
> +       /* A module might be built-in, therefore try sysfs */
> +       snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
> +       if (stat(sysfs_mod_name, &st) != 0) {
> +               RTE_LOG(DEBUG, EAL, "Open %s failed! error %i (%s)\n",
> +                       sysfs_mod_name, errno, strerror(errno));
> +               return 0;
>         }
>

Please, add a check on snprintf return value.
Even if this can't happen, if snprintf fails, stat would access a truncated
or uninitialized (when < 0) path.

With this fixed, you can add my ack.


Thanks.

-- 
David Marchand

Re: [dpdk-dev] [PATCH v3 2/2] eal/linux: Add support for handling built-in kernel modules

[Stephen Hemminger]

On Mon, 7 Dec 2015 19:36:05 +0100
Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com> wrote:

> +	/* Check if there is sysfs mounted */
> +	if (stat("/sys/module", &st) != 0) {
> +		RTE_LOG(DEBUG, EAL, "Open /sys/module failed: %s\n",
> +			strerror(errno));
>  		return -1;
>  	}

This check is useless.
If /sys/module does not exist then /sys/module/XXX won't exist either.

Re: [dpdk-dev] [PATCH v3 2/2] eal/linux: Add support for handling built-in kernel modules

[Panu Matilainen]

On 12/07/2015 10:55 PM, Stephen Hemminger wrote:
> On Mon, 7 Dec 2015 19:36:05 +0100
> Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com> wrote:
>
>> +	/* Check if there is sysfs mounted */
>> +	if (stat("/sys/module", &st) != 0) {
>> +		RTE_LOG(DEBUG, EAL, "Open /sys/module failed: %s\n",
>> +			strerror(errno));
>>   		return -1;
>>   	}
>
> This check is useless.
> If /sys/module does not exist then /sys/module/XXX won't exist either.

Yes, but non-mounted sysfs is an error whereas /sys/module/XXX is merely 
an existence test, and the current sole caller in pci_vfio_enable() even 
bothers checking for the difference. So its perhaps a bit academic but 
its not incorrect.

At any rate, the debug messages are incorrect/misleading. It's certainly 
not trying to *open* these directories so it should not claim to do so.

	- Panu -

Re: [dpdk-dev] [PATCH v3 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 08.12.2015 o 08:25, Panu Matilainen pisze:
> On 12/07/2015 10:55 PM, Stephen Hemminger wrote:
>> On Mon, 7 Dec 2015 19:36:05 +0100
>> Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com> wrote:
>>
>>> +    /* Check if there is sysfs mounted */
>>> +    if (stat("/sys/module", &st) != 0) {
>>> +        RTE_LOG(DEBUG, EAL, "Open /sys/module failed: %s\n",
>>> +            strerror(errno));
>>>           return -1;
>>>       }
>>
>> This check is useless.
>> If /sys/module does not exist then /sys/module/XXX won't exist either.
>
> Yes, but non-mounted sysfs is an error whereas /sys/module/XXX is 
> merely an existence test, and the current sole caller in 
> pci_vfio_enable() even bothers checking for the difference. So its 
> perhaps a bit academic but its not incorrect.
>
> At any rate, the debug messages are incorrect/misleading. It's 
> certainly not trying to *open* these directories so it should not 
> claim to do so.
>

Yes, this check is to determine whether there is sysfs mounted. It's 
different than checking if there is a module loaded.

This seems academical, but it retains the original behavior.

I will try to improve the logging.

>     - Panu -
>
>
>

[dpdk-dev] [PATCH v4 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently dpdk_nic_bind.py detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files.

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 tools/dpdk_nic_bind.py | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
index f02454e..e161062 100755
--- a/tools/dpdk_nic_bind.py
+++ b/tools/dpdk_nic_bind.py
@@ -156,22 +156,29 @@ def check_modules():
     '''Checks that igb_uio is loaded'''
     global dpdk_drivers
 
-    fd = file("/proc/modules")
-    loaded_mods = fd.readlines()
-    fd.close()
-
     # list of supported modules
     mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
 
     # first check if module is loaded
-    for line in loaded_mods:
+    try:
+        # Get list of syfs modules, some of them might be builtin and merge with mods
+        sysfs_path = '/sys/module/'
+
+        # Get the list of directories in sysfs_path
+        sysfs_mods = [os.path.join(sysfs_path,o) for o in os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]
+
+        # Extract the last element of '/sys/module/abc' in the array
+        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
+
+        # special case for vfio_pci (module is named vfio-pci,
+        # but its .ko is named vfio_pci)
+        sysfs_mods = map(lambda a: a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
+
         for mod in mods:
-            if line.startswith(mod["Name"]):
-                mod["Found"] = True
-            # special case for vfio_pci (module is named vfio-pci,
-            # but its .ko is named vfio_pci)
-            elif line.replace("_", "-").startswith(mod["Name"]):
+            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
                 mod["Found"] = True
+    except:
+        pass
 
     # check if we have at least one loaded module
     if True not in [mod["Found"] for mod in mods] and b_flag is not None:
-- 
1.9.1

[dpdk-dev] [PATCH v4 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently rte_eal_check_module() detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 635ec36..92482a0 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -901,27 +901,33 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_check_module(const char *module_name)
 {
-	char mod_name[30]; /* Any module names can be longer than 30 bytes? */
-	int ret = 0;
+	char sysfs_mod_name[PATH_MAX];
+	struct stat st;
 	int n;
 
 	if (NULL == module_name)
 		return -1;
 
-	FILE *fd = fopen("/proc/modules", "r");
-	if (NULL == fd) {
-		RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
-			" error %i (%s)\n", errno, strerror(errno));
+	/* Check if there is sysfs mounted */
+	if (stat("/sys/module", &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
+			errno, strerror(errno));
 		return -1;
 	}
-	while (!feof(fd)) {
-		n = fscanf(fd, "%29s %*[^\n]", mod_name);
-		if ((n == 1) && !strcmp(mod_name, module_name)) {
-			ret = 1;
-			break;
-		}
+
+	/* A module might be built-in, therefore try sysfs */
+	n = snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
+	if (n < 0 || n > PATH_MAX) {
+		RTE_LOG(DEBUG, EAL, "Could not format module path\n");
+		return -1;
 	}
-	fclose(fd);
 
-	return ret;
+	if (stat(sysfs_mod_name, &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "Open %s failed! error %i (%s)\n",
+		        sysfs_mod_name, errno, strerror(errno));
+		return 0;
+	}
+
+	/* Module has been found */
+	return 1;
 }
-- 
1.9.1

Re: [dpdk-dev] [PATCH v4 2/2] eal/linux: Add support for handling built-in kernel modules

[Panu Matilainen]

On 12/08/2015 05:33 PM, Kamil Rytarowski wrote:
> Currently rte_eal_check_module() detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
>
> Add support for checking built-in modules with parsing the sysfs files
>
> This commit obsoletes the /proc/modules parsing approach.
>
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>   lib/librte_eal/linuxapp/eal/eal.c | 34 ++++++++++++++++++++--------------
>   1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 635ec36..92482a0 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -901,27 +901,33 @@ int rte_eal_has_hugepages(void)
>   int
>   rte_eal_check_module(const char *module_name)
>   {
> -	char mod_name[30]; /* Any module names can be longer than 30 bytes? */
> -	int ret = 0;
> +	char sysfs_mod_name[PATH_MAX];
> +	struct stat st;
>   	int n;
>
>   	if (NULL == module_name)
>   		return -1;
>
> -	FILE *fd = fopen("/proc/modules", "r");
> -	if (NULL == fd) {
> -		RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
> -			" error %i (%s)\n", errno, strerror(errno));
> +	/* Check if there is sysfs mounted */
> +	if (stat("/sys/module", &st) != 0) {
> +		RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
> +			errno, strerror(errno));
>   		return -1;
>   	}
> -	while (!feof(fd)) {
> -		n = fscanf(fd, "%29s %*[^\n]", mod_name);
> -		if ((n == 1) && !strcmp(mod_name, module_name)) {
> -			ret = 1;
> -			break;
> -		}
> +
> +	/* A module might be built-in, therefore try sysfs */
> +	n = snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
> +	if (n < 0 || n > PATH_MAX) {
> +		RTE_LOG(DEBUG, EAL, "Could not format module path\n");
> +		return -1;
>   	}
> -	fclose(fd);
>
> -	return ret;
> +	if (stat(sysfs_mod_name, &st) != 0) {
> +		RTE_LOG(DEBUG, EAL, "Open %s failed! error %i (%s)\n",
> +		        sysfs_mod_name, errno, strerror(errno));
> +		return 0;
> +	}

Like with /sys/module, its not trying to *open* sysfs_mod_name directory 
either so it shouldn't claim to do so.

I did use plural on purpose when I said "the debug messages are 
incorrect/misleading. It's certainly not trying to *open* these 
directories so it should not claim to do so" in my previous mail :)

- Panu -

[dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently dpdk_nic_bind.py detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files.

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 tools/dpdk_nic_bind.py | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
index f02454e..e161062 100755
--- a/tools/dpdk_nic_bind.py
+++ b/tools/dpdk_nic_bind.py
@@ -156,22 +156,29 @@ def check_modules():
     '''Checks that igb_uio is loaded'''
     global dpdk_drivers
 
-    fd = file("/proc/modules")
-    loaded_mods = fd.readlines()
-    fd.close()
-
     # list of supported modules
     mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
 
     # first check if module is loaded
-    for line in loaded_mods:
+    try:
+        # Get list of syfs modules, some of them might be builtin and merge with mods
+        sysfs_path = '/sys/module/'
+
+        # Get the list of directories in sysfs_path
+        sysfs_mods = [os.path.join(sysfs_path,o) for o in os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]
+
+        # Extract the last element of '/sys/module/abc' in the array
+        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
+
+        # special case for vfio_pci (module is named vfio-pci,
+        # but its .ko is named vfio_pci)
+        sysfs_mods = map(lambda a: a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
+
         for mod in mods:
-            if line.startswith(mod["Name"]):
-                mod["Found"] = True
-            # special case for vfio_pci (module is named vfio-pci,
-            # but its .ko is named vfio_pci)
-            elif line.replace("_", "-").startswith(mod["Name"]):
+            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
                 mod["Found"] = True
+    except:
+        pass
 
     # check if we have at least one loaded module
     if True not in [mod["Found"] for mod in mods] and b_flag is not None:
-- 
2.6.3

[dpdk-dev] [PATCH v5 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Currently rte_eal_check_module() detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 635ec36..21a4a32 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -901,27 +901,33 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_check_module(const char *module_name)
 {
-	char mod_name[30]; /* Any module names can be longer than 30 bytes? */
-	int ret = 0;
+	char sysfs_mod_name[PATH_MAX];
+	struct stat st;
 	int n;
 
 	if (NULL == module_name)
 		return -1;
 
-	FILE *fd = fopen("/proc/modules", "r");
-	if (NULL == fd) {
-		RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
-			" error %i (%s)\n", errno, strerror(errno));
+	/* Check if there is sysfs mounted */
+	if (stat("/sys/module", &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
+			errno, strerror(errno));
 		return -1;
 	}
-	while (!feof(fd)) {
-		n = fscanf(fd, "%29s %*[^\n]", mod_name);
-		if ((n == 1) && !strcmp(mod_name, module_name)) {
-			ret = 1;
-			break;
-		}
+
+	/* A module might be built-in, therefore try sysfs */
+	n = snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
+	if (n < 0 || n > PATH_MAX) {
+		RTE_LOG(DEBUG, EAL, "Could not format module path\n");
+		return -1;
 	}
-	fclose(fd);
 
-	return ret;
+	if (stat(sysfs_mod_name, &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "Module %s not found! error %i (%s)\n",
+		        sysfs_mod_name, errno, strerror(errno));
+		return 0;
+	}
+
+	/* Module has been found */
+	return 1;
 }
-- 
2.6.3

Re: [dpdk-dev] [PATCH v4 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 09.12.2015 o 08:30, Panu Matilainen pisze:
> On 12/08/2015 05:33 PM, Kamil Rytarowski wrote:
>> Currently rte_eal_check_module() detects Linux kernel modules via 
>> reading
>> /proc/modules. Built-in ones aren't listed there and therefore they 
>> are not
>> being found by the script.
>>
>> Add support for checking built-in modules with parsing the sysfs files
>>
>> This commit obsoletes the /proc/modules parsing approach.
>>
>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>> Signed-off-by: David Marchand <david.marchand@6wind.com>
>> ---
>>   lib/librte_eal/linuxapp/eal/eal.c | 34 
>> ++++++++++++++++++++--------------
>>   1 file changed, 20 insertions(+), 14 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
>> b/lib/librte_eal/linuxapp/eal/eal.c
>> index 635ec36..92482a0 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -901,27 +901,33 @@ int rte_eal_has_hugepages(void)
>>   int
>>   rte_eal_check_module(const char *module_name)
>>   {
>> -    char mod_name[30]; /* Any module names can be longer than 30 
>> bytes? */
>> -    int ret = 0;
>> +    char sysfs_mod_name[PATH_MAX];
>> +    struct stat st;
>>       int n;
>>
>>       if (NULL == module_name)
>>           return -1;
>>
>> -    FILE *fd = fopen("/proc/modules", "r");
>> -    if (NULL == fd) {
>> -        RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
>> -            " error %i (%s)\n", errno, strerror(errno));
>> +    /* Check if there is sysfs mounted */
>> +    if (stat("/sys/module", &st) != 0) {
>> +        RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
>> +            errno, strerror(errno));
>>           return -1;
>>       }
>> -    while (!feof(fd)) {
>> -        n = fscanf(fd, "%29s %*[^\n]", mod_name);
>> -        if ((n == 1) && !strcmp(mod_name, module_name)) {
>> -            ret = 1;
>> -            break;
>> -        }
>> +
>> +    /* A module might be built-in, therefore try sysfs */
>> +    n = snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", 
>> module_name);
>> +    if (n < 0 || n > PATH_MAX) {
>> +        RTE_LOG(DEBUG, EAL, "Could not format module path\n");
>> +        return -1;
>>       }
>> -    fclose(fd);
>>
>> -    return ret;
>> +    if (stat(sysfs_mod_name, &st) != 0) {
>> +        RTE_LOG(DEBUG, EAL, "Open %s failed! error %i (%s)\n",
>> +                sysfs_mod_name, errno, strerror(errno));
>> +        return 0;
>> +    }
>
> Like with /sys/module, its not trying to *open* sysfs_mod_name 
> directory either so it shouldn't claim to do so.
>
> I did use plural on purpose when I said "the debug messages are 
> incorrect/misleading. It's certainly not trying to *open* these 
> directories so it should not claim to do so" in my previous mail :)
>
> - Panu -
>

Should be good now!

Thank you.

Re: [dpdk-dev] [PATCH v5 2/2] eal/linux: Add support for handling built-in kernel modules

[Stephen Hemminger]

On Wed, 9 Dec 2015 14:19:58 +0100
Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com> wrote:

> +	/* Check if there is sysfs mounted */
> +	if (stat("/sys/module", &st) != 0) {
> +		RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
> +			errno, strerror(errno));
>  		return -1;
>  	}

This check is redundant. Remove it.
If the later "/sys/module/foo" would fail if /sys/module was not present.

> -	while (!feof(fd)) {
> -		n = fscanf(fd, "%29s %*[^\n]", mod_name);
> -		if ((n == 1) && !strcmp(mod_name, module_name)) {
> -			ret = 1;
> -			break;
> -		}
> +
> +	/* A module might be built-in, therefore try sysfs */
> +	n = snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);

Re: [dpdk-dev] [PATCH v5 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 09.12.2015 o 17:38, Stephen Hemminger pisze:
> On Wed, 9 Dec 2015 14:19:58 +0100
> Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com> wrote:
>
>> +	/* Check if there is sysfs mounted */
>> +	if (stat("/sys/module", &st) != 0) {
>> +		RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
>> +			errno, strerror(errno));
>>   		return -1;
>>   	}
> This check is redundant. Remove it.
> If the later "/sys/module/foo" would fail if /sys/module was not present.
This check isn't redundant. It checks different thing (sysfs mounted vs 
module loaded) and formulates different error message.

Please see pci_vfio_enable():

         module_vfio_type1 = rte_eal_check_module("vfio_iommu_type1");

         /* return error directly */
         if (module_vfio_type1 == -1) {
                 RTE_LOG(INFO, EAL, "Could not get loaded module 
details!\n");
                 return -1;
         }

         /* return 0 if VFIO modules not loaded */
         if (module_vfio_type1 == 0) {
                 RTE_LOG(INFO, EAL, "VFIO modules not all loaded, "
                         "skip VFIO support...\n");
                 return 0;
         }

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

ping?

W dniu 09.12.2015 o 14:19, Kamil Rytarowski pisze:
> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
>
> Add support for checking built-in modules with parsing the sysfs files.
>
> This commit obsoletes the /proc/modules parsing approach.
>
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>   tools/dpdk_nic_bind.py | 27 +++++++++++++++++----------
>   1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
> index f02454e..e161062 100755
> --- a/tools/dpdk_nic_bind.py
> +++ b/tools/dpdk_nic_bind.py
> @@ -156,22 +156,29 @@ def check_modules():
>       '''Checks that igb_uio is loaded'''
>       global dpdk_drivers
>   
> -    fd = file("/proc/modules")
> -    loaded_mods = fd.readlines()
> -    fd.close()
> -
>       # list of supported modules
>       mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
>   
>       # first check if module is loaded
> -    for line in loaded_mods:
> +    try:
> +        # Get list of syfs modules, some of them might be builtin and merge with mods
> +        sysfs_path = '/sys/module/'
> +
> +        # Get the list of directories in sysfs_path
> +        sysfs_mods = [os.path.join(sysfs_path,o) for o in os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]
> +
> +        # Extract the last element of '/sys/module/abc' in the array
> +        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
> +
> +        # special case for vfio_pci (module is named vfio-pci,
> +        # but its .ko is named vfio_pci)
> +        sysfs_mods = map(lambda a: a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
> +
>           for mod in mods:
> -            if line.startswith(mod["Name"]):
> -                mod["Found"] = True
> -            # special case for vfio_pci (module is named vfio-pci,
> -            # but its .ko is named vfio_pci)
> -            elif line.replace("_", "-").startswith(mod["Name"]):
> +            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
>                   mod["Found"] = True
> +    except:
> +        pass
>   
>       # check if we have at least one loaded module
>       if True not in [mod["Found"] for mod in mods] and b_flag is not None:

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

ping?

W dniu 16.12.2015 o 15:14, Kamil Rytarowski pisze:
> ping?
>
> W dniu 09.12.2015 o 14:19, Kamil Rytarowski pisze:
>> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
>> /proc/modules. Built-in ones aren't listed there and therefore they 
>> are not
>> being found by the script.
>>
>> Add support for checking built-in modules with parsing the sysfs files.
>>
>> This commit obsoletes the /proc/modules parsing approach.
>>
>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>> Signed-off-by: David Marchand <david.marchand@6wind.com>
>> ---
>>   tools/dpdk_nic_bind.py | 27 +++++++++++++++++----------
>>   1 file changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
>> index f02454e..e161062 100755
>> --- a/tools/dpdk_nic_bind.py
>> +++ b/tools/dpdk_nic_bind.py
>> @@ -156,22 +156,29 @@ def check_modules():
>>       '''Checks that igb_uio is loaded'''
>>       global dpdk_drivers
>>   -    fd = file("/proc/modules")
>> -    loaded_mods = fd.readlines()
>> -    fd.close()
>> -
>>       # list of supported modules
>>       mods =  [{"Name" : driver, "Found" : False} for driver in 
>> dpdk_drivers]
>>         # first check if module is loaded
>> -    for line in loaded_mods:
>> +    try:
>> +        # Get list of syfs modules, some of them might be builtin 
>> and merge with mods
>> +        sysfs_path = '/sys/module/'
>> +
>> +        # Get the list of directories in sysfs_path
>> +        sysfs_mods = [os.path.join(sysfs_path,o) for o in 
>> os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]
>> +
>> +        # Extract the last element of '/sys/module/abc' in the array
>> +        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
>> +
>> +        # special case for vfio_pci (module is named vfio-pci,
>> +        # but its .ko is named vfio_pci)
>> +        sysfs_mods = map(lambda a: a if a != 'vfio_pci' else 
>> 'vfio-pci', sysfs_mods)
>> +
>>           for mod in mods:
>> -            if line.startswith(mod["Name"]):
>> -                mod["Found"] = True
>> -            # special case for vfio_pci (module is named vfio-pci,
>> -            # but its .ko is named vfio_pci)
>> -            elif line.replace("_", "-").startswith(mod["Name"]):
>> +            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
>>                   mod["Found"] = True
>> +    except:
>> +        pass
>>         # check if we have at least one loaded module
>>       if True not in [mod["Found"] for mod in mods] and b_flag is not 
>> None:
>

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Yuanhan Liu]

Hi Kamil,

First of all, sorry for no one has reviewed your patches for over one
month! You may want to ping more often (say, per week) next time if it
still happenes :)

Another thing is that there is no maintainer for tools code.

On Wed, Dec 09, 2015 at 02:19:57PM +0100, Kamil Rytarowski wrote:
> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
> 
> Add support for checking built-in modules with parsing the sysfs files.
> 
> This commit obsoletes the /proc/modules parsing approach.
> 
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  tools/dpdk_nic_bind.py | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
> index f02454e..e161062 100755
> --- a/tools/dpdk_nic_bind.py
> +++ b/tools/dpdk_nic_bind.py
> @@ -156,22 +156,29 @@ def check_modules():
>      '''Checks that igb_uio is loaded'''
>      global dpdk_drivers
>  
> -    fd = file("/proc/modules")
> -    loaded_mods = fd.readlines()
> -    fd.close()
> -
>      # list of supported modules
>      mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
>  
>      # first check if module is loaded
> -    for line in loaded_mods:
> +    try:
> +        # Get list of syfs modules, some of them might be builtin and merge with mods
> +        sysfs_path = '/sys/module/'
> +
> +        # Get the list of directories in sysfs_path
> +        sysfs_mods = [os.path.join(sysfs_path,o) for o in os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]

Minor nit: it's quite a long line; you may need break it. And space is
needed after ','. 

Otherwise, this patch looks good to me.

	--yliu

Re: [dpdk-dev] [PATCH v5 2/2] eal/linux: Add support for handling built-in kernel modules

[Yuanhan Liu]

On Wed, Dec 09, 2015 at 02:19:58PM +0100, Kamil Rytarowski wrote:
> Currently rte_eal_check_module() detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
> 
> Add support for checking built-in modules with parsing the sysfs files
> 
> This commit obsoletes the /proc/modules parsing approach.
> 
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>

Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Thanks.

	--yliu

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Thomas Monjalon]

Hi Kamil,

2015-12-09 14:19, Kamil Rytarowski:
> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
> 
> Add support for checking built-in modules with parsing the sysfs files.
> 
> This commit obsoletes the /proc/modules parsing approach.
> 
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

I have a doubt about this tag:
> Signed-off-by: David Marchand <david.marchand@6wind.com>
What do you mean here?

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Thank you, I will submit improved version as v6.

W dniu 18.01.2016 o 15:21, Yuanhan Liu pisze:
> Hi Kamil,
>
> First of all, sorry for no one has reviewed your patches for over one
> month! You may want to ping more often (say, per week) next time if it
> still happenes :)
>
> Another thing is that there is no maintainer for tools code.
>
> On Wed, Dec 09, 2015 at 02:19:57PM +0100, Kamil Rytarowski wrote:
>> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
>> /proc/modules. Built-in ones aren't listed there and therefore they are not
>> being found by the script.
>>
>> Add support for checking built-in modules with parsing the sysfs files.
>>
>> This commit obsoletes the /proc/modules parsing approach.
>>
>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>> Signed-off-by: David Marchand <david.marchand@6wind.com>
>> ---
>>   tools/dpdk_nic_bind.py | 27 +++++++++++++++++----------
>>   1 file changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
>> index f02454e..e161062 100755
>> --- a/tools/dpdk_nic_bind.py
>> +++ b/tools/dpdk_nic_bind.py
>> @@ -156,22 +156,29 @@ def check_modules():
>>       '''Checks that igb_uio is loaded'''
>>       global dpdk_drivers
>>   
>> -    fd = file("/proc/modules")
>> -    loaded_mods = fd.readlines()
>> -    fd.close()
>> -
>>       # list of supported modules
>>       mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
>>   
>>       # first check if module is loaded
>> -    for line in loaded_mods:
>> +    try:
>> +        # Get list of syfs modules, some of them might be builtin and merge with mods
>> +        sysfs_path = '/sys/module/'
>> +
>> +        # Get the list of directories in sysfs_path
>> +        sysfs_mods = [os.path.join(sysfs_path,o) for o in os.listdir(sysfs_path) if os.path.isdir(os.path.join(sysfs_path,o))]
> Minor nit: it's quite a long line; you may need break it. And space is
> needed after ','.
>
> Otherwise, this patch looks good to me.
>
> 	--yliu

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 18.01.2016 o 15:32, Thomas Monjalon pisze:
> Hi Kamil,
>
> 2015-12-09 14:19, Kamil Rytarowski:
>> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
>> /proc/modules. Built-in ones aren't listed there and therefore they are not
>> being found by the script.
>>
>> Add support for checking built-in modules with parsing the sysfs files.
>>
>> This commit obsoletes the /proc/modules parsing approach.
>>
>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> I have a doubt about this tag:
>> Signed-off-by: David Marchand <david.marchand@6wind.com>
> What do you mean here?

Excuse me, it should be:  Acked-by: David Marchand 
<david.marchand@6wind.com>

http://dpdk.org/ml/archives/dev/2015-December/029720.html

Re: [dpdk-dev] [PATCH v5 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 18.01.2016 o 15:22, Yuanhan Liu pisze:
> On Wed, Dec 09, 2015 at 02:19:58PM +0100, Kamil Rytarowski wrote:
>> Currently rte_eal_check_module() detects Linux kernel modules via reading
>> /proc/modules. Built-in ones aren't listed there and therefore they are not
>> being found by the script.
>>
>> Add support for checking built-in modules with parsing the sysfs files
>>
>> This commit obsoletes the /proc/modules parsing approach.
>>
>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>> Signed-off-by: David Marchand <david.marchand@6wind.com>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> Thanks.
>
> 	--yliu

Thank you, I will submit v6 and note you with the the Acked-by line.

[dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[krytarowski]

From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

Currently dpdk_nic_bind.py detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files.

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
Acked-by: David Marchand <david.marchand@6wind.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 tools/dpdk_nic_bind.py | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
index f02454e..1d16d9f 100755
--- a/tools/dpdk_nic_bind.py
+++ b/tools/dpdk_nic_bind.py
@@ -156,22 +156,32 @@ def check_modules():
     '''Checks that igb_uio is loaded'''
     global dpdk_drivers
 
-    fd = file("/proc/modules")
-    loaded_mods = fd.readlines()
-    fd.close()
-
     # list of supported modules
     mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
 
     # first check if module is loaded
-    for line in loaded_mods:
+    try:
+        # Get list of syfs modules, some of them might be builtin and merge with mods
+        sysfs_path = '/sys/module/'
+
+        # Get the list of directories in sysfs_path
+        sysfs_mods = [os.path.join(sysfs_path, o) for o
+                      in os.listdir(sysfs_path)
+                      if os.path.isdir(os.path.join(sysfs_path, o))]
+
+        # Extract the last element of '/sys/module/abc' in the array
+        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
+
+        # special case for vfio_pci (module is named vfio-pci,
+        # but its .ko is named vfio_pci)
+        sysfs_mods = map(lambda a:
+                         a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
+
         for mod in mods:
-            if line.startswith(mod["Name"]):
-                mod["Found"] = True
-            # special case for vfio_pci (module is named vfio-pci,
-            # but its .ko is named vfio_pci)
-            elif line.replace("_", "-").startswith(mod["Name"]):
+            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
                 mod["Found"] = True
+    except:
+        pass
 
     # check if we have at least one loaded module
     if True not in [mod["Found"] for mod in mods] and b_flag is not None:
-- 
1.9.1

[dpdk-dev] [PATCH v6 2/2] eal/linux: Add support for handling built-in kernel modules

[krytarowski]

From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

Currently rte_eal_check_module() detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
Acked-by: David Marchand <david.marchand@6wind.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 635ec36..21a4a32 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -901,27 +901,33 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_check_module(const char *module_name)
 {
-	char mod_name[30]; /* Any module names can be longer than 30 bytes? */
-	int ret = 0;
+	char sysfs_mod_name[PATH_MAX];
+	struct stat st;
 	int n;
 
 	if (NULL == module_name)
 		return -1;
 
-	FILE *fd = fopen("/proc/modules", "r");
-	if (NULL == fd) {
-		RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
-			" error %i (%s)\n", errno, strerror(errno));
+	/* Check if there is sysfs mounted */
+	if (stat("/sys/module", &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
+			errno, strerror(errno));
 		return -1;
 	}
-	while (!feof(fd)) {
-		n = fscanf(fd, "%29s %*[^\n]", mod_name);
-		if ((n == 1) && !strcmp(mod_name, module_name)) {
-			ret = 1;
-			break;
-		}
+
+	/* A module might be built-in, therefore try sysfs */
+	n = snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
+	if (n < 0 || n > PATH_MAX) {
+		RTE_LOG(DEBUG, EAL, "Could not format module path\n");
+		return -1;
 	}
-	fclose(fd);
 
-	return ret;
+	if (stat(sysfs_mod_name, &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "Module %s not found! error %i (%s)\n",
+		        sysfs_mod_name, errno, strerror(errno));
+		return 0;
+	}
+
+	/* Module has been found */
+	return 1;
 }
-- 
1.9.1

Re: [dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

ping?

W dniu 20.01.2016 o 10:48, krytarowski@caviumnetworks.com pisze:
> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>
> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
>
> Add support for checking built-in modules with parsing the sysfs files.
>
> This commit obsoletes the /proc/modules parsing approach.
>
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> Acked-by: David Marchand <david.marchand@6wind.com>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>   tools/dpdk_nic_bind.py | 30 ++++++++++++++++++++----------
>   1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
> index f02454e..1d16d9f 100755
> --- a/tools/dpdk_nic_bind.py
> +++ b/tools/dpdk_nic_bind.py
> @@ -156,22 +156,32 @@ def check_modules():
>       '''Checks that igb_uio is loaded'''
>       global dpdk_drivers
>   
> -    fd = file("/proc/modules")
> -    loaded_mods = fd.readlines()
> -    fd.close()
> -
>       # list of supported modules
>       mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
>   
>       # first check if module is loaded
> -    for line in loaded_mods:
> +    try:
> +        # Get list of syfs modules, some of them might be builtin and merge with mods
> +        sysfs_path = '/sys/module/'
> +
> +        # Get the list of directories in sysfs_path
> +        sysfs_mods = [os.path.join(sysfs_path, o) for o
> +                      in os.listdir(sysfs_path)
> +                      if os.path.isdir(os.path.join(sysfs_path, o))]
> +
> +        # Extract the last element of '/sys/module/abc' in the array
> +        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
> +
> +        # special case for vfio_pci (module is named vfio-pci,
> +        # but its .ko is named vfio_pci)
> +        sysfs_mods = map(lambda a:
> +                         a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
> +
>           for mod in mods:
> -            if line.startswith(mod["Name"]):
> -                mod["Found"] = True
> -            # special case for vfio_pci (module is named vfio-pci,
> -            # but its .ko is named vfio_pci)
> -            elif line.replace("_", "-").startswith(mod["Name"]):
> +            if mod["Found"] == False and (mod["Name"] in sysfs_mods):
>                   mod["Found"] = True
> +    except:
> +        pass
>   
>       # check if we have at least one loaded module
>       if True not in [mod["Found"] for mod in mods] and b_flag is not None:

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Thomas Monjalon]

2016-01-19 17:35, Kamil Rytarowski:
> 
> W dniu 18.01.2016 o 15:32, Thomas Monjalon pisze:
> > Hi Kamil,
> >
> > 2015-12-09 14:19, Kamil Rytarowski:
> >> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
> >> /proc/modules. Built-in ones aren't listed there and therefore they are not
> >> being found by the script.
> >>
> >> Add support for checking built-in modules with parsing the sysfs files.
> >>
> >> This commit obsoletes the /proc/modules parsing approach.
> >>
> >> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> > I have a doubt about this tag:
> >> Signed-off-by: David Marchand <david.marchand@6wind.com>
> > What do you mean here?
> 
> Excuse me, it should be:  Acked-by: David Marchand 
> <david.marchand@6wind.com>
> 
> http://dpdk.org/ml/archives/dev/2015-December/029720.html

The ack was only for the patch 2/2

Re: [dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[Thomas Monjalon]

2016-01-20 10:48, krytarowski@caviumnetworks.com:
> --- a/tools/dpdk_nic_bind.py
> +++ b/tools/dpdk_nic_bind.py
> -    for line in loaded_mods:
> +    try:
> +        # Get list of syfs modules, some of them might be builtin and merge with mods

Please could you explain this comment?
Is it remaining from previous versions of the patch?

[...]
> +        # special case for vfio_pci (module is named vfio-pci,
> +        # but its .ko is named vfio_pci)

Isn't it common to have dash replaced by underscore for kernel modules?

Re: [dpdk-dev] [PATCH v5 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 26.01.2016 o 16:12, Thomas Monjalon pisze:
> 2016-01-19 17:35, Kamil Rytarowski:
>> W dniu 18.01.2016 o 15:32, Thomas Monjalon pisze:
>>> Hi Kamil,
>>>
>>> 2015-12-09 14:19, Kamil Rytarowski:
>>>> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
>>>> /proc/modules. Built-in ones aren't listed there and therefore they are not
>>>> being found by the script.
>>>>
>>>> Add support for checking built-in modules with parsing the sysfs files.
>>>>
>>>> This commit obsoletes the /proc/modules parsing approach.
>>>>
>>>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>>> I have a doubt about this tag:
>>>> Signed-off-by: David Marchand <david.marchand@6wind.com>
>>> What do you mean here?
>> Excuse me, it should be:  Acked-by: David Marchand
>> <david.marchand@6wind.com>
>>
>> http://dpdk.org/ml/archives/dev/2015-December/029720.html
> The ack was only for the patch 2/2


I see. I will correct it.

Re: [dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 26.01.2016 o 16:23, Thomas Monjalon pisze:
> 2016-01-20 10:48, krytarowski@caviumnetworks.com:
>> --- a/tools/dpdk_nic_bind.py
>> +++ b/tools/dpdk_nic_bind.py
>> -    for line in loaded_mods:
>> +    try:
>> +        # Get list of syfs modules, some of them might be builtin and merge with mods
> Please could you explain this comment?
> Is it remaining from previous versions of the patch?

Yes. It might be changed to:
# Get list of sysfs modules (both built-in and dynamically loaded)

> [...]
>> +        # special case for vfio_pci (module is named vfio-pci,
>> +        # but its .ko is named vfio_pci)
> Isn't it common to have dash replaced by underscore for kernel modules?
>

I retained the logic for special case of vfio-pci. At the moment 
(according to my knowledge) there are no other DPDK modules with this 
name replacement.

I checked few example Linux modules and if a module is named with dash, 
it's being replaced to underscore. The modprobe(8) tool can accept both 
names as interchangeable (with dash and underscore).

Would you like to make it a general rule and replace all dashes with 
underscores?

Thank you

Re: [dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[Panu Matilainen]

On 01/28/2016 01:17 PM, Kamil Rytarowski wrote:
>
>
> W dniu 26.01.2016 o 16:23, Thomas Monjalon pisze:
>> 2016-01-20 10:48, krytarowski@caviumnetworks.com:
>>> --- a/tools/dpdk_nic_bind.py
>>> +++ b/tools/dpdk_nic_bind.py
>>> -    for line in loaded_mods:
>>> +    try:
>>> +        # Get list of syfs modules, some of them might be builtin
>>> and merge with mods
>> Please could you explain this comment?
>> Is it remaining from previous versions of the patch?
>
> Yes. It might be changed to:
> # Get list of sysfs modules (both built-in and dynamically loaded)
>
>> [...]
>>> +        # special case for vfio_pci (module is named vfio-pci,
>>> +        # but its .ko is named vfio_pci)
>> Isn't it common to have dash replaced by underscore for kernel modules?
>>
>
> I retained the logic for special case of vfio-pci. At the moment
> (according to my knowledge) there are no other DPDK modules with this
> name replacement.
>
> I checked few example Linux modules and if a module is named with dash,
> it's being replaced to underscore. The modprobe(8) tool can accept both
> names as interchangeable (with dash and underscore).
>
> Would you like to make it a general rule and replace all dashes with
> underscores?

It would be nice to behave the same as modprobe wrt dash and underscore, 
yes.

	- Panu -

> Thank you

Re: [dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 28.01.2016 o 12:22, Panu Matilainen pisze:
> On 01/28/2016 01:17 PM, Kamil Rytarowski wrote:
>> I retained the logic for special case of vfio-pci. At the moment
>> (according to my knowledge) there are no other DPDK modules with this
>> name replacement.
>>
>> I checked few example Linux modules and if a module is named with dash,
>> it's being replaced to underscore. The modprobe(8) tool can accept both
>> names as interchangeable (with dash and underscore).
>>
>> Would you like to make it a general rule and replace all dashes with
>> underscores?
>
> It would be nice to behave the same as modprobe wrt dash and 
> underscore, yes.
>
>     - Panu -
>

My patch is intended to support built-in modules, the rest isn't that 
trivial without changing the behavior.

I prototyped it and it added extra unnecessary complexity, while we just 
want to handle vfio_pci -> vfio-pci.

I'm going to submit new version with improved comment in the code. 
Please continue possible improvements in separate threads.

[dpdk-dev] [PATCH v7 1/2] tools: Add support for handling built-in kernel modules

[krytarowski]

From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

Currently dpdk_nic_bind.py detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files.

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
---
 tools/dpdk_nic_bind.py | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/tools/dpdk_nic_bind.py b/tools/dpdk_nic_bind.py
index f02454e..85cb6f1 100755
--- a/tools/dpdk_nic_bind.py
+++ b/tools/dpdk_nic_bind.py
@@ -156,22 +156,32 @@ def check_modules():
     '''Checks that igb_uio is loaded'''
     global dpdk_drivers
 
-    fd = file("/proc/modules")
-    loaded_mods = fd.readlines()
-    fd.close()
-
     # list of supported modules
     mods =  [{"Name" : driver, "Found" : False} for driver in dpdk_drivers]
 
     # first check if module is loaded
-    for line in loaded_mods:
+    try:
+        # Get list of sysfs modules (both built-in and dynamically loaded)
+        sysfs_path = '/sys/module/'
+
+        # Get the list of directories in sysfs_path
+        sysfs_mods = [os.path.join(sysfs_path, o) for o
+                      in os.listdir(sysfs_path)
+                      if os.path.isdir(os.path.join(sysfs_path, o))]
+
+        # Extract the last element of '/sys/module/abc' in the array
+        sysfs_mods = [a.split('/')[-1] for a in sysfs_mods]
+
+        # special case for vfio_pci (module is named vfio-pci,
+        # but its .ko is named vfio_pci)
+        sysfs_mods = map(lambda a:
+                         a if a != 'vfio_pci' else 'vfio-pci', sysfs_mods)
+
         for mod in mods:
-            if line.startswith(mod["Name"]):
-                mod["Found"] = True
-            # special case for vfio_pci (module is named vfio-pci,
-            # but its .ko is named vfio_pci)
-            elif line.replace("_", "-").startswith(mod["Name"]):
+            if mod["Name"] in sysfs_mods:
                 mod["Found"] = True
+    except:
+        pass
 
     # check if we have at least one loaded module
     if True not in [mod["Found"] for mod in mods] and b_flag is not None:
-- 
1.9.1

[dpdk-dev] [PATCH v7 2/2] eal/linux: Add support for handling built-in kernel modules

[krytarowski]

From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

Currently rte_eal_check_module() detects Linux kernel modules via reading
/proc/modules. Built-in ones aren't listed there and therefore they are not
being found by the script.

Add support for checking built-in modules with parsing the sysfs files

This commit obsoletes the /proc/modules parsing approach.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
Acked-by: David Marchand <david.marchand@6wind.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 635ec36..21a4a32 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -901,27 +901,33 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_check_module(const char *module_name)
 {
-	char mod_name[30]; /* Any module names can be longer than 30 bytes? */
-	int ret = 0;
+	char sysfs_mod_name[PATH_MAX];
+	struct stat st;
 	int n;
 
 	if (NULL == module_name)
 		return -1;
 
-	FILE *fd = fopen("/proc/modules", "r");
-	if (NULL == fd) {
-		RTE_LOG(ERR, EAL, "Open /proc/modules failed!"
-			" error %i (%s)\n", errno, strerror(errno));
+	/* Check if there is sysfs mounted */
+	if (stat("/sys/module", &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "sysfs is not mounted! error %i (%s)\n",
+			errno, strerror(errno));
 		return -1;
 	}
-	while (!feof(fd)) {
-		n = fscanf(fd, "%29s %*[^\n]", mod_name);
-		if ((n == 1) && !strcmp(mod_name, module_name)) {
-			ret = 1;
-			break;
-		}
+
+	/* A module might be built-in, therefore try sysfs */
+	n = snprintf(sysfs_mod_name, PATH_MAX, "/sys/module/%s", module_name);
+	if (n < 0 || n > PATH_MAX) {
+		RTE_LOG(DEBUG, EAL, "Could not format module path\n");
+		return -1;
 	}
-	fclose(fd);
 
-	return ret;
+	if (stat(sysfs_mod_name, &st) != 0) {
+		RTE_LOG(DEBUG, EAL, "Module %s not found! error %i (%s)\n",
+		        sysfs_mod_name, errno, strerror(errno));
+		return 0;
+	}
+
+	/* Module has been found */
+	return 1;
 }
-- 
1.9.1

Re: [dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[Thomas Monjalon]

2016-01-28 12:17, Kamil Rytarowski:
> 
> W dniu 26.01.2016 o 16:23, Thomas Monjalon pisze:
> > 2016-01-20 10:48, krytarowski@caviumnetworks.com:
> >> --- a/tools/dpdk_nic_bind.py
> >> +++ b/tools/dpdk_nic_bind.py
> >> -    for line in loaded_mods:
> >> +    try:
> >> +        # Get list of syfs modules, some of them might be builtin and merge with mods
> > Please could you explain this comment?
> > Is it remaining from previous versions of the patch?
> 
> Yes. It might be changed to:
> # Get list of sysfs modules (both built-in and dynamically loaded)

OK

> > [...]
> >> +        # special case for vfio_pci (module is named vfio-pci,
> >> +        # but its .ko is named vfio_pci)
> > Isn't it common to have dash replaced by underscore for kernel modules?
> >
> 
> I retained the logic for special case of vfio-pci. At the moment 
> (according to my knowledge) there are no other DPDK modules with this 
> name replacement.
> 
> I checked few example Linux modules and if a module is named with dash, 
> it's being replaced to underscore. The modprobe(8) tool can accept both 
> names as interchangeable (with dash and underscore).
> 
> Would you like to make it a general rule and replace all dashes with 
> underscores?

I don't know. Do what you think is best.
Thanks

Re: [dpdk-dev] [PATCH v6 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

W dniu 28.01.2016 o 14:52, Thomas Monjalon pisze:
> 2016-01-28 12:17, Kamil Rytarowski:
> [...]
>>>> +        # special case for vfio_pci (module is named vfio-pci,
>>>> +        # but its .ko is named vfio_pci)
>>> Isn't it common to have dash replaced by underscore for kernel modules?
>>>
>> I retained the logic for special case of vfio-pci. At the moment
>> (according to my knowledge) there are no other DPDK modules with this
>> name replacement.
>>
>> I checked few example Linux modules and if a module is named with dash,
>> it's being replaced to underscore. The modprobe(8) tool can accept both
>> names as interchangeable (with dash and underscore).
>>
>> Would you like to make it a general rule and replace all dashes with
>> underscores?
> I don't know. Do what you think is best.
> Thanks
I suggest to leave it as it is. There is no need to add a layer of 
abstraction for a single known exception.

Also, this is out of scope of my patch.

Thank you.

Re: [dpdk-dev] [PATCH v7 1/2] tools: Add support for handling built-in kernel modules

[Yuanhan Liu]

On Thu, Jan 28, 2016 at 02:13:53PM +0100, krytarowski@caviumnetworks.com wrote:
> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> 
> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
> 
> Add support for checking built-in modules with parsing the sysfs files.
> 
> This commit obsoletes the /proc/modules parsing approach.
> 
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

	--yliu

Re: [dpdk-dev] [PATCH v7 1/2] tools: Add support for handling built-in kernel modules

[Kamil Rytarowski]

ping?

W dniu 29.01.2016 o 08:21, Yuanhan Liu pisze:
> On Thu, Jan 28, 2016 at 02:13:53PM +0100, krytarowski@caviumnetworks.com wrote:
>> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>>
>> Currently dpdk_nic_bind.py detects Linux kernel modules via reading
>> /proc/modules. Built-in ones aren't listed there and therefore they are not
>> being found by the script.
>>
>> Add support for checking built-in modules with parsing the sysfs files.
>>
>> This commit obsoletes the /proc/modules parsing approach.
>>
>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> 	--yliu

Re: [dpdk-dev] [PATCH v7 2/2] eal/linux: Add support for handling built-in kernel modules

[Thomas Monjalon]

2016-01-28 14:13, krytarowski@caviumnetworks.com:
> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> 
> Currently rte_eal_check_module() detects Linux kernel modules via reading
> /proc/modules. Built-in ones aren't listed there and therefore they are not
> being found by the script.
> 
> Add support for checking built-in modules with parsing the sysfs files
> 
> This commit obsoletes the /proc/modules parsing approach.
> 
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> Acked-by: David Marchand <david.marchand@6wind.com>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

An include is missing:
#include <sys/stat.h>
After adding this line,
Series applied, thanks

Re: [dpdk-dev] [PATCH v7 2/2] eal/linux: Add support for handling built-in kernel modules

[Kamil Rytarowski]

Thank you!

W dniu 09.02.2016 o 15:56, Thomas Monjalon pisze:
> 2016-01-28 14:13, krytarowski@caviumnetworks.com:
>> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>>
>> Currently rte_eal_check_module() detects Linux kernel modules via reading
>> /proc/modules. Built-in ones aren't listed there and therefore they are not
>> being found by the script.
>>
>> Add support for checking built-in modules with parsing the sysfs files
>>
>> This commit obsoletes the /proc/modules parsing approach.
>>
>> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>> Acked-by: David Marchand <david.marchand@6wind.com>
>> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> An include is missing:
> #include <sys/stat.h>
> After adding this line,
> Series applied, thanks
>