Skip to contentSkip to conversation

2nd parameter of driver init function can be NULL using latest code

PATCH Applied Tetsuya Mukawa 3 participants 4 messages Feb 19, 2015 – Feb 24, 2015 3 messages not in the archive

lore
Share & tools
lore
#0 lore raw

Hi,

It seems after applying below patch, 2nd parameter of PMD initialization code can be NULL when vdev option is like below.

----------------
commit c07691ae10894bb6bf284fed75829b95844eacdb

devargs: remove limit on parameters length
----------------

Here is example vdev option

--vdev 'eth_name0'
(No option after driver name case)

It seems some PMDs assumes 2nd parameter will be always not NULL, even if there is no option after driver name.

For example, before applying the patch, here is a log of bond PMD.

$ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 --vdev
'eth_bond0' -- -i
EAL: Detected lcore 0 as core 0 on socket 0
EAL: Detected lcore 1 as core 1 on socket 0
EAL: Detected lcore 2 as core 2 on socket 0
EAL: Detected lcore 3 as core 3 on socket 0
EAL: Detected lcore 4 as core 4 on socket 0
EAL: Detected lcore 5 as core 5 on socket 0
EAL: Detected lcore 6 as core 6 on socket 0
EAL: Detected lcore 7 as core 7 on socket 0
EAL: Support maximum 128 logical core(s) by configuration.
EAL: Detected 8 lcore(s)
EAL: VFIO modules not all loaded, skip VFIO support...
EAL: Setting up memory...
EAL: Ask a virtual area of 0x280000000 bytes
EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
EAL: Requesting 10 pages of size 1024MB from socket 0
EAL: TSC frequency is ~3991438 KHz
EAL: Master core 0 is ready (tid=f7fd6840)
EAL: Initializing pmd_bond for eth_bond0
EAL: Mode must be specified only once for bonded device eth_bond0
PMD: ENICPMD trace: rte_enic_pmd_init
EAL: Core 3 is ready (tid=f58e0700)
EAL: Core 2 is ready (tid=f60e1700)
EAL: Core 1 is ready (tid=f68e2700)
EAL: PCI device 0000:02:00.0 on NUMA socket -1
EAL:   probe driver: 8086:10b9 rte_em_pmd
EAL:   0000:02:00.0 not managed by UIO driver, skipping
EAL: Error - exiting with code: 1
  Cause: No probed ethernet device

After applying.

$ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 --vdev
'eth_bond0' -- -i
EAL: Detected lcore 0 as core 0 on socket 0
EAL: Detected lcore 1 as core 1 on socket 0
EAL: Detected lcore 2 as core 2 on socket 0
EAL: Detected lcore 3 as core 3 on socket 0
EAL: Detected lcore 4 as core 4 on socket 0
EAL: Detected lcore 5 as core 5 on socket 0
EAL: Detected lcore 6 as core 6 on socket 0
EAL: Detected lcore 7 as core 7 on socket 0
EAL: Support maximum 128 logical core(s) by configuration.
EAL: Detected 8 lcore(s)
EAL: VFIO modules not all loaded, skip VFIO support...
EAL: Setting up memory...
EAL: Ask a virtual area of 0x280000000 bytes
EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
EAL: Requesting 10 pages of size 1024MB from socket 0
EAL: TSC frequency is ~3991439 KHz
EAL: Master core 0 is ready (tid=f7fd6840)
EAL: Initializing pmd_bond for eth_bond0
$

It seems error is returned in PMD code.
I am not sure this is an issue. But just in case I report it, because behavior is changed.

Thanks,
Tetsuya

A message that is not in the archive (sent off-list or not yet received): 54E5ED96.9040402-AlSX/UN32fvPDbFq/[email protected]

#2 lore raw

PATCH 1/1 devargs: restore empty devargs as ""

Following commit c07691ae1089, an implicit change has been done in the devargs api.
This triggers problem in virtual pmds that did not check for parameters validity as it was implicitely valid.

Fix this by restoring the empty argument as "" and add a note in the api. Restore associated tests.

Fixes c07691ae1089 ("devargs: remove limit on parameters length")
---
 app/test/test_devargs.c                     |    2 +-
 lib/librte_eal/common/eal_common_devargs.c  |   11 +++++++----
 lib/librte_eal/common/include/rte_devargs.h |    2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)
app/test/test_devargs.c +1 -1
index 08fb781..f7fc59c 100644
@@ -107,7 +107,7 @@ test_devargs(void)
 		devargs->pci.addr.devid != 0 ||
 		devargs->pci.addr.function != 1)
 		goto fail;
-	if (devargs->args)
+	if (!devargs->args || strcmp(devargs->args, "") != 0)
 		goto fail;
 	free_devargs_list();
 
lib/librte_eal/common/eal_common_devargs.c +7 -4
index 3aace08..eadd719 100644
@@ -73,10 +73,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 	if (sep != NULL) {
 		sep[0] = '\0';
 		devargs->args = strdup(sep + 1);
-		if (devargs->args == NULL) {
-			RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
-			goto fail;
-		}
+	} else {
+		devargs->args = strdup("");
+	}
+
+	if (devargs->args == NULL) {
+		RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
+		goto fail;
 	}
 
 	switch (devargs->type) {
lib/librte_eal/common/include/rte_devargs.h +1 -1
index 996e180..6834333 100644
@@ -88,7 +88,7 @@ struct rte_devargs {
 			char drv_name[32];
 		} virtual;
 	};
-	/** Arguments string as given by user. */
+	/** Arguments string as given by user or "" for no argument. */
 	char *args;
 };
 
Signature · 1 line
1.7.10.4

A message that is not in the archive (sent off-list or not yet received): 1424770891-19243-1-git-send-email-david.marchand-pdR9zngts4EAvxtiuMwx3w@public.gmane.org

#4 lore raw

Re: [PATCH] devargs: restore empty devargs as ""

Quoted 57 lines · David Marchand

Following commit c07691ae1089, an implicit change has been done in the devargs api.
This triggers problem in virtual pmds that did not check for parameters validity as it was implicitely valid.

Fix this by restoring the empty argument as "" and add a note in the api. Restore associated tests.

Fixes c07691ae1089 ("devargs: remove limit on parameters length")
---
 app/test/test_devargs.c                     |    2 +-
 lib/librte_eal/common/eal_common_devargs.c  |   11 +++++++----
 lib/librte_eal/common/include/rte_devargs.h |    2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)
app/test/test_devargs.c +1 -1
index 08fb781..f7fc59c 100644
@@ -107,7 +107,7 @@ test_devargs(void)
 		devargs->pci.addr.devid != 0 ||
 		devargs->pci.addr.function != 1)
 		goto fail;
-	if (devargs->args)
+	if (!devargs->args || strcmp(devargs->args, "") != 0)
 		goto fail;
 	free_devargs_list();
 
lib/librte_eal/common/eal_common_devargs.c +7 -4
index 3aace08..eadd719 100644
@@ -73,10 +73,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 	if (sep != NULL) {
 		sep[0] = '\0';
 		devargs->args = strdup(sep + 1);
-		if (devargs->args == NULL) {
-			RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
-			goto fail;
-		}
+	} else {
+		devargs->args = strdup("");
+	}
+
+	if (devargs->args == NULL) {
+		RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
+		goto fail;
 	}
 
 	switch (devargs->type) {
lib/librte_eal/common/include/rte_devargs.h +0 -0
index 996e180..6834333 100644

lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 94

Quoted 8 lines
lib/librte_eal/common/include/rte_devargs.h +1 -1
@@ -88,7 +88,7 @@ struct rte_devargs {
 			char drv_name[32];
 		} virtual;
 	};
-	/** Arguments string as given by user. */
+	/** Arguments string as given by user or "" for no argument. */
 	char *args;
 };
 

Hi David,

I've confirmed this patch fixes the issue descried in above comment.

Thanks,
Tetsuya

A message that is not in the archive (sent off-list or not yet received): 54EC4F7B.6080907-AlSX/UN32fvPDbFq/[email protected]

#6 lore raw

Re: [PATCH] devargs: restore empty devargs as ""

2015-02-24 19:16, Tetsuya Mukawa:

Quoted 9 lines · David Marchand
Quoted 8 lines

Following commit c07691ae1089, an implicit change has been done in the devargs api.
This triggers problem in virtual pmds that did not check for parameters validity as it was implicitely valid.

Fix this by restoring the empty argument as "" and add a note in the api. Restore associated tests.

Quoted 7 lines
Quoted 3 lines
Fixes c07691ae1089 ("devargs: remove limit on parameters length")

Hi David,

I've confirmed this patch fixes the issue descried in above comment.

Applied, thanks