[dpdk-stable] [PATCH v2] eal: fix core number validation

[dpdk-stable] [PATCH v2] eal: fix core number validation

[Hari kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

--
v2: Replace strtoul with strtol
    Modified log message
--

Signed-off-by: Hari kumar Vemula <hari.kumarx.vemula@intel.com>
---
 lib/librte_eal/common/eal_common_options.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..b24668c23 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,7 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -603,6 +603,11 @@ eal_parse_corelist(const char *corelist)
 			max = idx;
 			if (min == RTE_MAX_LCORE)
 				min = idx;
+			if ((unsigned int)idx >= cfg->lcore_count ||
+					(unsigned int)min >= cfg->lcore_count) {
+				return -1;
+			}
+
 			for (idx = min; idx <= max; idx++) {
 				if (cfg->lcore_role[idx] != ROLE_RTE) {
 					cfg->lcore_role[idx] = ROLE_RTE;
@@ -1103,6 +1108,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1151,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"Invalid core number given core range should be(0, %u)\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
-- 
2.17.2

Re: [dpdk-stable] [PATCH v2] eal: fix core number validation

[David Marchand]

On Thu, Jan 3, 2019 at 1:28 PM Hari kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
>
> Added valid range checks to fix the crash.
>
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
>
> --
> v2: Replace strtoul with strtol
>     Modified log message
>

You did not take my other comments into account and the ut updates are
missing.


-- 
David Marchand

[dpdk-stable] [PATCH v2] eal: fix core number validation

[Hari Kumar Vemula]

From: Hari kumar Vemula <hari.kumarx.vemula@intel.com>

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari kumar Vemula <hari.kumarx.vemula@intel.com>
---
 lib/librte_eal/common/eal_common_options.c | 10 ++++++++--
 test/test/test_eal_flags.c                 | 14 +++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..4a900c548 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+			if (idx < 0 || idx >= (int)cfg->lcore_count)
+				return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -603,6 +605,7 @@ eal_parse_corelist(const char *corelist)
 			max = idx;
 			if (min == RTE_MAX_LCORE)
 				min = idx;
 			for (idx = min; idx <= max; idx++) {
 				if (cfg->lcore_role[idx] != ROLE_RTE) {
 					cfg->lcore_role[idx] = ROLE_RTE;
@@ -1103,6 +1106,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1149,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"Invalid core number, core range should be (0, %u)\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..ec79b2242 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -513,6 +513,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* when core number is negative value*/
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5-7" };
+	/* when core number is maximum value*/
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "999999999" };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "1-9999999" };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +566,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

[dpdk-stable] [PATCH v3] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

--
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
--

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
 lib/librte_eal/common/eal_common_options.c |  9 +++++++--
 test/test/test_eal_flags.c                 | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..9431c024e 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+			if (idx < 0 || idx >= (int)cfg->lcore_count)
+				return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"Invalid core number, core range should be (0, %u)\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..28e68a6c9 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -513,6 +513,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* core number is negative value */
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "999999999" };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "1-9999999" };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +566,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-stable] [PATCH v3] eal: fix core number validation

[David Marchand]

Hello Hari,

On Mon, Jan 7, 2019 at 11:26 AM Hari Kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
>
> Added valid range checks to fix the crash.
>
> Added ut check for negative core values.
> Added unit test case for invalid core number range.
>
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
>
> --
> v3: Added unit test cases for invalid core number range
> v2: Replace strtoul with strtol
>     Modified log message
> --
>
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
>  lib/librte_eal/common/eal_common_options.c |  9 +++++++--
>  test/test/test_eal_flags.c                 | 14 +++++++++++++-
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index 6e3a83b98..9431c024e 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
>                 if (*corelist == '\0')
>                         return -1;
>                 errno = 0;
> -               idx = strtoul(corelist, &end, 10);
> +               idx = strtol(corelist, &end, 10);
> +                       if (idx < 0 || idx >= (int)cfg->lcore_count)
> +                               return -1;
>                 if (errno || end == NULL)
>                         return -1;
>                 while (isblank(*end))
>

Please fix indentation.


> @@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
>  {
>         static int b_used;
>         static int w_used;
> +       struct rte_config *cfg = rte_eal_get_configuration();
>
>         switch (opt) {
>         /* blacklist */
> @@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
>         /* corelist */
>         case 'l':
>                 if (eal_parse_corelist(optarg) < 0) {
> -                       RTE_LOG(ERR, EAL, "invalid core list\n");
> +                       RTE_LOG(ERR, EAL,
> +                               "Invalid core number, core range should be
> (0, %u)\n",
> +                                       cfg->lcore_count-1);
>                         return -1;
>                 }
>
>
eal_parse_corelist can error for both invalid core number but also for
incorrectly formatted input.
How about "invalid core list, please check core numbers are in [0, %u]
range" ?

diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index 2acab9d69..28e68a6c9 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -513,6 +513,16 @@ test_missing_c_flag(void)
>         const char *argv25[] = { prgname, prefix, mp_flag,
>                                  "-n", "3", "--lcores",
>                                  "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
> +       /* core number is negative value */
> +       const char * const argv26[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5" };
> +       const char * const argv27[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5-7" };
>
+       /* core number is maximum value */
> +       const char * const argv28[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "999999999" };
> +       const char * const argv29[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "1-9999999" };
>

Well, the maximum value is not really "999999999".
Please check against RTE_MAX_LCORE.


-- 
David Marchand

[dpdk-stable] [PATCH v4] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
--
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++++++--
 test/test/test_eal_flags.c                 | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..fc45bf953 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -513,6 +514,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* core number is negative value */
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "RTE_MAX_LCORE+1" };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "1-(RTE_MAX_LCORE+1)" };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +567,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-stable] [PATCH v4] eal: fix core number validation

[David Marchand]

On Fri, Jan 11, 2019 at 3:15 PM Hari Kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

>
> diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index 2acab9d69..fc45bf953 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -18,6 +18,7 @@
>  #include <sys/file.h>
>  #include <limits.h>
>
> +#include <rte_per_lcore.h>
>  #include <rte_debug.h>
>  #include <rte_string_fns.h>
>
> @@ -513,6 +514,16 @@ test_missing_c_flag(void)
>         const char *argv25[] = { prgname, prefix, mp_flag,
>                                  "-n", "3", "--lcores",
>                                  "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
>
+       /* core number is negative value */
> +       const char * const argv26[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5" };
> +       const char * const argv27[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5-7" };
>

I did not see this before, but you fixed the "-l" eal option, not
"--lcores" option.
So those unit tests are wrong.



> +       /* core number is maximum value */
> +       const char * const argv28[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "RTE_MAX_LCORE+1" };
> +       const char * const argv29[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores",
> "1-(RTE_MAX_LCORE+1)" };
>
>         if (launch_proc(argv2) != 0) {
>                 printf("Error - "
>

Passing "RTE_MAX_LCORE+1" is indeed wrong (be it with "-l" or "--lcores"
options), but I would still prefer to check the formatted value of
RTE_MAX_LCORE (no need for that +1, btw).
So please, in next version, test against "-l", RTE_STR(RTE_MAX_LCORE) and
"-l", "1-" RTE_STR(RTE_MAX_LCORE).


Thanks.

-- 
David Marchand

[dpdk-stable] [PATCH v5] eal: fix core number validation

[Hari Kumar Vemula]

From: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
--
v5: corrected unit test case for -l option
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++++++--
 test/test/test_eal_flags.c                 | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..4dc22ec36 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -513,6 +514,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* core number is negative value */
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +567,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

[dpdk-stable] [PATCH v5] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
--
v5: corrected unit test case for -l option
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++++++--
 test/test/test_eal_flags.c                 | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..4dc22ec36 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -513,6 +514,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* core number is negative value */
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +567,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-stable] [PATCH v5] eal: fix core number validation

[David Marchand]

On Mon, Jan 14, 2019 at 11:31 AM Hari Kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

> diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index 2acab9d69..4dc22ec36 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -18,6 +18,7 @@
>  #include <sys/file.h>
>  #include <limits.h>
>
> +#include <rte_per_lcore.h>
>  #include <rte_debug.h>
>  #include <rte_string_fns.h>
>
> @@ -513,6 +514,16 @@ test_missing_c_flag(void)
>         const char *argv25[] = { prgname, prefix, mp_flag,
>                                  "-n", "3", "--lcores",
>                                  "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
> +       /* core number is negative value */
> +       const char * const argv26[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l", "-5" };
> +       const char * const argv27[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l", "-5-7" };
> +       /* core number is maximum value */
> +       const char * const argv28[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
> +       const char * const argv29[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l",
> "1-"RTE_STR(RTE_MAX_LCORE) };
>

Please move this block with the other "-l" options.
You can add my Reviewed-by tag with this.


Thanks.

-- 
David Marchand

[dpdk-stable] [PATCH v6] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
v6: Changed testcase order and added reviewed-by tag
v5: Corrected unit test case for -l option
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++-
 test/test/test_eal_flags.c                 | 57 ++++++++++++++--------
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..24ca5b6a0 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -480,37 +481,47 @@ test_missing_c_flag(void)
 	/* sanity check test - valid corelist value */
 	const char *argv11[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "-l", "1-2,3" };
+	/* core number is negative value */
+	const char * const argv12[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5" };
+	const char * const argv13[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv14[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+	const char * const argv15[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 
 	/* --lcores flag but no lcores value */
-	const char *argv12[] = { prgname, prefix, mp_flag,
+	const char * const argv16[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores" };
-	const char *argv13[] = { prgname, prefix, mp_flag,
+	const char * const argv17[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", " " };
 	/* bad lcores value */
-	const char *argv14[] = { prgname, prefix, mp_flag,
+	const char * const argv18[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "1-3-5" };
-	const char *argv15[] = { prgname, prefix, mp_flag,
+	const char * const argv19[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "0-1,,2" };
-	const char *argv16[] = { prgname, prefix, mp_flag,
+	const char * const argv20[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "0-,1" };
-	const char *argv17[] = { prgname, prefix, mp_flag,
+	const char * const argv21[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(0-,2-4)" };
-	const char *argv18[] = { prgname, prefix, mp_flag,
+	const char * const argv22[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(-1,2)" };
-	const char *argv19[] = { prgname, prefix, mp_flag,
+	const char * const argv23[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(2-4)@(2-4-6)" };
-	const char *argv20[] = { prgname, prefix, mp_flag,
+	const char * const argv24[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(a,2)" };
-	const char *argv21[] = { prgname, prefix, mp_flag,
+	const char * const argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "1-3@(1,3)" };
-	const char *argv22[] = { prgname, prefix, mp_flag,
+	const char * const argv26[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "3@((1,3)" };
-	const char *argv23[] = { prgname, prefix, mp_flag,
+	const char * const argv27[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(4-7)=(1,3)" };
-	const char *argv24[] = { prgname, prefix, mp_flag,
+	const char * const argv28[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "[4-7]@(1,3)" };
 	/* sanity check of tests - valid lcores value */
-	const char *argv25[] = { prgname, prefix, mp_flag,
+	const char * const argv29[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
 
@@ -538,7 +549,11 @@ test_missing_c_flag(void)
 			|| launch_proc(argv7) == 0
 			|| launch_proc(argv8) == 0
 			|| launch_proc(argv9) == 0
-			|| launch_proc(argv10) == 0) {
+			|| launch_proc(argv10) == 0
+			|| launch_proc(argv12) == 0
+			|| launch_proc(argv13) == 0
+			|| launch_proc(argv14) == 0
+			|| launch_proc(argv15) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid -l flag\n");
 		return -1;
@@ -550,19 +565,19 @@ test_missing_c_flag(void)
 	}
 
 	/* start --lcores tests */
-	if (launch_proc(argv12) == 0 || launch_proc(argv13) == 0 ||
-	    launch_proc(argv14) == 0 || launch_proc(argv15) == 0 ||
-	    launch_proc(argv16) == 0 || launch_proc(argv17) == 0 ||
+	if (launch_proc(argv16) == 0 || launch_proc(argv17) == 0 ||
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
-	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv22) == 0 || launch_proc(argv23) == 0 ||
+	    launch_proc(argv24) == 0 || launch_proc(argv25) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
 	}
 
-	if (launch_proc(argv25) != 0) {
+	if (launch_proc(argv29) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
-- 
2.17.2

[dpdk-stable] [PATCH v6] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
v6: Changed testcase order
v5: Corrected unit test case for -l option
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++-
 test/test/test_eal_flags.c                 | 61 ++++++++++++++--------
 2 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..e3a60c7ae 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -477,40 +478,50 @@ test_missing_c_flag(void)
 				"-n", "3", "-l", "1," };
 	const char *argv10[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "-l", "1#2" };
+	/* core number is negative value */
+	const char * const argv11[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5" };
+	const char * const argv12[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv13[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+	const char * const argv14[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 	/* sanity check test - valid corelist value */
-	const char *argv11[] = { prgname, prefix, mp_flag,
+	const char * const argv15[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "-l", "1-2,3" };
 
 	/* --lcores flag but no lcores value */
-	const char *argv12[] = { prgname, prefix, mp_flag,
+	const char * const argv16[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores" };
-	const char *argv13[] = { prgname, prefix, mp_flag,
+	const char * const argv17[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", " " };
 	/* bad lcores value */
-	const char *argv14[] = { prgname, prefix, mp_flag,
+	const char * const argv18[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "1-3-5" };
-	const char *argv15[] = { prgname, prefix, mp_flag,
+	const char * const argv19[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "0-1,,2" };
-	const char *argv16[] = { prgname, prefix, mp_flag,
+	const char * const argv20[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "0-,1" };
-	const char *argv17[] = { prgname, prefix, mp_flag,
+	const char * const argv21[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(0-,2-4)" };
-	const char *argv18[] = { prgname, prefix, mp_flag,
+	const char * const argv22[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(-1,2)" };
-	const char *argv19[] = { prgname, prefix, mp_flag,
+	const char * const argv23[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(2-4)@(2-4-6)" };
-	const char *argv20[] = { prgname, prefix, mp_flag,
+	const char * const argv24[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(a,2)" };
-	const char *argv21[] = { prgname, prefix, mp_flag,
+	const char * const argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "1-3@(1,3)" };
-	const char *argv22[] = { prgname, prefix, mp_flag,
+	const char * const argv26[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "3@((1,3)" };
-	const char *argv23[] = { prgname, prefix, mp_flag,
+	const char * const argv27[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(4-7)=(1,3)" };
-	const char *argv24[] = { prgname, prefix, mp_flag,
+	const char * const argv28[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "[4-7]@(1,3)" };
 	/* sanity check of tests - valid lcores value */
-	const char *argv25[] = { prgname, prefix, mp_flag,
+	const char * const argv29[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
 
@@ -538,31 +549,35 @@ test_missing_c_flag(void)
 			|| launch_proc(argv7) == 0
 			|| launch_proc(argv8) == 0
 			|| launch_proc(argv9) == 0
-			|| launch_proc(argv10) == 0) {
+			|| launch_proc(argv10) == 0
+			|| launch_proc(argv11) == 0
+			|| launch_proc(argv12) == 0
+			|| launch_proc(argv13) == 0
+			|| launch_proc(argv14) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid -l flag\n");
 		return -1;
 	}
-	if (launch_proc(argv11) != 0) {
+	if (launch_proc(argv15) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
 	}
 
 	/* start --lcores tests */
-	if (launch_proc(argv12) == 0 || launch_proc(argv13) == 0 ||
-	    launch_proc(argv14) == 0 || launch_proc(argv15) == 0 ||
-	    launch_proc(argv16) == 0 || launch_proc(argv17) == 0 ||
+	if (launch_proc(argv16) == 0 || launch_proc(argv17) == 0 ||
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
-	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv22) == 0 || launch_proc(argv23) == 0 ||
+	    launch_proc(argv24) == 0 || launch_proc(argv25) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
 	}
 
-	if (launch_proc(argv25) != 0) {
+	if (launch_proc(argv29) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-stable] [dpdk-dev] [PATCH v6] eal: fix core number validation

[Bruce Richardson]

On Thu, Jan 17, 2019 at 12:13:12PM +0000, Hari Kumar Vemula wrote:
> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
> 
> Added valid range checks to fix the crash.
> 
> Added ut check for negative core values.
> Added unit test case for invalid core number range.
> 
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> --
> v6: Changed testcase order
> v5: Corrected unit test case for -l option
> v4: Used RTE_MAX_LCORE for max core check
> v3: Added unit test cases for invalid core number range
> v2: Replace strtoul with strtol
>     Modified log message
> ---
>  lib/librte_eal/common/eal_common_options.c |  9 +++-
>  test/test/test_eal_flags.c                 | 61 ++++++++++++++--------
>  2 files changed, 45 insertions(+), 25 deletions(-)
> 
Is this patch related to, or does it fix Bugzilla #19?

https://bugs.dpdk.org/show_bug.cgi?id=19

/Bruce

Re: [dpdk-stable] [dpdk-dev] [PATCH v6] eal: fix core number validation

[David Marchand]

On Thu, Jan 17, 2019 at 1:19 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Thu, Jan 17, 2019 at 12:13:12PM +0000, Hari Kumar Vemula wrote:
> > When incorrect core value or range provided,
> > as part of -l command line option, a crash occurs.
> >
> > Added valid range checks to fix the crash.
> >
> > Added ut check for negative core values.
> > Added unit test case for invalid core number range.
> >
> > Fixes: d888cb8b9613 ("eal: add core list input format")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> > Reviewed-by: David Marchand <david.marchand@redhat.com>
> > --
> > v6: Changed testcase order
> > v5: Corrected unit test case for -l option
> > v4: Used RTE_MAX_LCORE for max core check
> > v3: Added unit test cases for invalid core number range
> > v2: Replace strtoul with strtol
> >     Modified log message
> > ---
> >  lib/librte_eal/common/eal_common_options.c |  9 +++-
> >  test/test/test_eal_flags.c                 | 61 ++++++++++++++--------
> >  2 files changed, 45 insertions(+), 25 deletions(-)
> >
> Is this patch related to, or does it fix Bugzilla #19?
>
> https://bugs.dpdk.org/show_bug.cgi?id=19


Separate issue, from my pov.
I would say the issue happens with a dpdk process that has no cpu available
wrt CONFIG_RTE_MAX_CORES.
eal_auto_detect_cores() then removes all cores from cfg->lcore_role[], then
eal_adjust_config() an incorrect master lcore is chosen at
cfg->master_lcore = rte_get_next_lcore(-1, 0, 0); ?


-- 
David Marchand

Re: [dpdk-stable] [PATCH v6] eal: fix core number validation

[Thomas Monjalon]

17/01/2019 13:13, Hari Kumar Vemula:
> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
> 
> Added valid range checks to fix the crash.
> 
> Added ut check for negative core values.
> Added unit test case for invalid core number range.
> 
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>

Applied, thanks

Re: [dpdk-stable] [PATCH v6] eal: fix core number validation

[Thomas Monjalon]

17/01/2019 13:13, Hari Kumar Vemula:
> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
> 
> Added valid range checks to fix the crash.
> 
> Added ut check for negative core values.
> Added unit test case for invalid core number range.
> 
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>

Applied, thanks

[dpdk-stable] [PATCH v2] eal: fix core number validation

[Hari kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari kumar Vemula <hari.kumarx.vemula@intel.com>
---
 lib/librte_eal/common/eal_common_options.c | 10 ++++++++--
 test/test/test_eal_flags.c                 | 14 +++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..4a900c548 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+			if (idx < 0 || idx >= (int)cfg->lcore_count)
+				return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -603,6 +605,7 @@ eal_parse_corelist(const char *corelist)
 			max = idx;
 			if (min == RTE_MAX_LCORE)
 				min = idx;
+
 			for (idx = min; idx <= max; idx++) {
 				if (cfg->lcore_role[idx] != ROLE_RTE) {
 					cfg->lcore_role[idx] = ROLE_RTE;
@@ -1103,6 +1106,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1149,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"Invalid core number given core range should be(0, %u)\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..ec79b2242 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -513,6 +513,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* when core number is negative value*/
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5-7" };
+	/* when core number is maximum value*/
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "999999999" };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "1-9999999" };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +566,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

[dpdk-stable] [PATCH] net/bonding: fix create bonded device test failure

[Hari Kumar Vemula]

Create bonded device test is failing due to improper initialisation in
bonded device configuration. which leads to crash while setting up queues.

The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
bonded device which fails.
This is due to "rx_desc_lim" is set to 0 as default value of bonded device
during bond_alloc().
Hence nb_rx_desc (1024) is > 0 and test fails.

Fix is to set the default values of rx_desc_lim of bonded device to
appropriate value.

Fixes: 2efb58cbab ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 44deaf119..e0888e960 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2225,6 +2225,11 @@ static void
 bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct bond_dev_private *internals = dev->data->dev_private;
+	struct rte_eth_desc_lim bond_lim = {
+		.nb_max = UINT16_MAX,
+		.nb_min = 0,
+		.nb_align = 1,
+	};
 
 	uint16_t max_nb_rx_queues = UINT16_MAX;
 	uint16_t max_nb_tx_queues = UINT16_MAX;
@@ -2263,10 +2268,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	memcpy(&dev_info->default_txconf, &internals->default_txconf,
 	       sizeof(dev_info->default_txconf));
 
-	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
-	       sizeof(dev_info->rx_desc_lim));
-	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
-	       sizeof(dev_info->tx_desc_lim));
+	dev_info->rx_desc_lim = bond_lim;
+	dev_info->tx_desc_lim = bond_lim;
 
 	/**
 	 * If dedicated hw queues enabled for link bonding device in LACP mode
-- 
2.17.2

Re: [dpdk-stable] [dpdk-dev] [PATCH] net/bonding: fix create bonded device test failure

[Chas Williams]

On 1/7/19 8:01 AM, Hari Kumar Vemula wrote:
> Create bonded device test is failing due to improper initialisation in
> bonded device configuration. which leads to crash while setting up queues.
> 
> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
> bonded device which fails.
> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
> during bond_alloc().
> Hence nb_rx_desc (1024) is > 0 and test fails.
> 
> Fix is to set the default values of rx_desc_lim of bonded device to
> appropriate value.

The default values for the bond device aren't known until the first
slave is added.  Can you explain your setup?  What PMD are you
using for testing?

> 
> Fixes: 2efb58cbab ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 44deaf119..e0888e960 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -2225,6 +2225,11 @@ static void
>   bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   {
>   	struct bond_dev_private *internals = dev->data->dev_private;
> +	struct rte_eth_desc_lim bond_lim = {
> +		.nb_max = UINT16_MAX,
> +		.nb_min = 0,
> +		.nb_align = 1,
> +	};
>   
>   	uint16_t max_nb_rx_queues = UINT16_MAX;
>   	uint16_t max_nb_tx_queues = UINT16_MAX;
> @@ -2263,10 +2268,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
>   	       sizeof(dev_info->default_txconf));
>   
> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> -	       sizeof(dev_info->rx_desc_lim));
> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> -	       sizeof(dev_info->tx_desc_lim));
> +	dev_info->rx_desc_lim = bond_lim;
> +	dev_info->tx_desc_lim = bond_lim;
>   
>   	/**
>   	 * If dedicated hw queues enabled for link bonding device in LACP mode
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH] net/bonding: fix create bonded device test failure

[Ferruh Yigit]

On 1/7/2019 6:44 PM, Chas Williams wrote:
> 
> 
> On 1/7/19 8:01 AM, Hari Kumar Vemula wrote:
>> Create bonded device test is failing due to improper initialisation in
>> bonded device configuration. which leads to crash while setting up queues.
>>
>> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
>> bonded device which fails.
>> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
>> during bond_alloc().
>> Hence nb_rx_desc (1024) is > 0 and test fails.
>>
>> Fix is to set the default values of rx_desc_lim of bonded device to
>> appropriate value.
> 
> The default values for the bond device aren't known until the first
> slave is added.  Can you explain your setup?  What PMD are you
> using for testing?

As far as I understand, 'rte_eth_rx_queue_setup()' is failing with bond eth
device since 'nb_rx_desc' should be less than 'dev_info.rx_desc_lim.nb_max' but
bonding sets 0 to 'nb_max'.

But not sure how to handle this from bonding point of view, this patch gives
most permissive values, but should bonding get these values from its slaves?

> 
>>
>> Fixes: 2efb58cbab ("bond: new link bonding library")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
>> ---
>>   drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
>> index 44deaf119..e0888e960 100644
>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>> @@ -2225,6 +2225,11 @@ static void
>>   bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>>   {
>>   	struct bond_dev_private *internals = dev->data->dev_private;
>> +	struct rte_eth_desc_lim bond_lim = {
>> +		.nb_max = UINT16_MAX,
>> +		.nb_min = 0,
>> +		.nb_align = 1,
>> +	};
>>   
>>   	uint16_t max_nb_rx_queues = UINT16_MAX;
>>   	uint16_t max_nb_tx_queues = UINT16_MAX;
>> @@ -2263,10 +2268,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>>   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
>>   	       sizeof(dev_info->default_txconf));
>>   
>> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
>> -	       sizeof(dev_info->rx_desc_lim));
>> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
>> -	       sizeof(dev_info->tx_desc_lim));
>> +	dev_info->rx_desc_lim = bond_lim;
>> +	dev_info->tx_desc_lim = bond_lim;
>>   
>>   	/**
>>   	 * If dedicated hw queues enabled for link bonding device in LACP mode
>>

Re: [dpdk-stable] [PATCH] net/bonding: fix create bonded device test failure

[Pattan, Reshma]

> -----Original Message-----
> From: Vemula, Hari KumarX
> Sent: Monday, January 7, 2019 1:01 PM
> 
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> -	       sizeof(dev_info->rx_desc_lim));
> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> -	       sizeof(dev_info->tx_desc_lim));
> +	dev_info->rx_desc_lim = bond_lim;
> +	dev_info->tx_desc_lim = bond_lim;


I relooked into this, these values should be filled from salve data like the way done for max_nb_rx_queues and max_nb_tx_queues
Existing code snippet:
if (slave_info.max_rx_queues < max_nb_rx_queues)
                          max_nb_rx_queues = slave_info.max_rx_queues;

 if (slave_info.max_tx_queues < max_nb_tx_queues)
                        max_nb_tx_queues = slave_info.max_tx_queues;

So, something like below you should add for rx/tx desc limit I guess.

if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
                                max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;

if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
                                max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;

dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
        dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;

@Williams/Declan: Does this make sense?

Thanks,
Reshma

[dpdk-stable] [PATCH v2] net/bonding: fix create bonded device test failure

[Hari Kumar Vemula]

Create bonded device test is failing due to improper initialisation in
bonded device configuration. which leads to crash while setting up queues.

The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
bonded device which fails.
This is due to "rx_desc_lim" is set to 0 as default value of bonded device
during bond_alloc().
Hence nb_rx_desc (1024) is > 0 and test fails.

Fix is to set the default values of rx_desc_lim of bonded device to
appropriate value.

Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
v2: bonded device desc_lim values are received from slave configuration
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 44deaf119..23cec2549 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2228,6 +2228,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 	uint16_t max_nb_rx_queues = UINT16_MAX;
 	uint16_t max_nb_tx_queues = UINT16_MAX;
+	uint16_t max_rx_desc_lim = UINT16_MAX;
+	uint16_t max_tx_desc_lim = UINT16_MAX;
 
 	dev_info->max_mac_addrs = BOND_MAX_MAC_ADDRS;
 
@@ -2252,6 +2254,12 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 			if (slave_info.max_tx_queues < max_nb_tx_queues)
 				max_nb_tx_queues = slave_info.max_tx_queues;
+
+			if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
+				max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;
+
+			if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
+				max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;
 		}
 	}
 
@@ -2263,10 +2271,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	memcpy(&dev_info->default_txconf, &internals->default_txconf,
 	       sizeof(dev_info->default_txconf));
 
-	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
-	       sizeof(dev_info->rx_desc_lim));
-	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
-	       sizeof(dev_info->tx_desc_lim));
+	dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
+	dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;
 
 	/**
 	 * If dedicated hw queues enabled for link bonding device in LACP mode
-- 
2.17.2

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/bonding: fix create bonded device test failure

[Chas Williams]

On 1/28/19 2:28 AM, Hari Kumar Vemula wrote:
> Create bonded device test is failing due to improper initialisation in
> bonded device configuration. which leads to crash while setting up queues.
> 
> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
> bonded device which fails.
> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
> during bond_alloc().
> Hence nb_rx_desc (1024) is > 0 and test fails.
> 
> Fix is to set the default values of rx_desc_lim of bonded device to
> appropriate value.
> 
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>

Acked-by: Chas Williams <chas3@att.com>

> ---
> v2: bonded device desc_lim values are received from slave configuration
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 44deaf119..23cec2549 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -2228,6 +2228,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   
>   	uint16_t max_nb_rx_queues = UINT16_MAX;
>   	uint16_t max_nb_tx_queues = UINT16_MAX;
> +	uint16_t max_rx_desc_lim = UINT16_MAX;
> +	uint16_t max_tx_desc_lim = UINT16_MAX;
>   
>   	dev_info->max_mac_addrs = BOND_MAX_MAC_ADDRS;
>   
> @@ -2252,6 +2254,12 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   
>   			if (slave_info.max_tx_queues < max_nb_tx_queues)
>   				max_nb_tx_queues = slave_info.max_tx_queues;
> +
> +			if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
> +				max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;
> +
> +			if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
> +				max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;
>   		}
>   	}
>   
> @@ -2263,10 +2271,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
>   	       sizeof(dev_info->default_txconf));
>   
> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> -	       sizeof(dev_info->rx_desc_lim));
> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> -	       sizeof(dev_info->tx_desc_lim));
> +	dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
> +	dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;
>   
>   	/**
>   	 * If dedicated hw queues enabled for link bonding device in LACP mode
> 

[dpdk-stable] [PATCH v3] net/bonding: fix create bonded device test failure

[Hari Kumar Vemula]

test_create_bonded_device is failing due to improper initialisation in
bonded device configuration. Which leads to crash while setting up queues.

The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
bonded device which fails.
This is due to "rx_desc_lim" is set to 0 as default value of bonded device
during bond_alloc().
Hence nb_rx_desc (1024) is > 0 and test fails.

Fix is to set the default values of rx_desc_lim of bonded device to
appropriate value.
Receive the values from slaves configuration like done for other existing
slave configuration

Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Acked-by: Chas Williams <chas3@att.com>
---
v3: Modified commit message
v2: Bonded device desc_lim values are received from slave configuration
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 44deaf119..23cec2549 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2228,6 +2228,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 	uint16_t max_nb_rx_queues = UINT16_MAX;
 	uint16_t max_nb_tx_queues = UINT16_MAX;
+	uint16_t max_rx_desc_lim = UINT16_MAX;
+	uint16_t max_tx_desc_lim = UINT16_MAX;
 
 	dev_info->max_mac_addrs = BOND_MAX_MAC_ADDRS;
 
@@ -2252,6 +2254,12 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 			if (slave_info.max_tx_queues < max_nb_tx_queues)
 				max_nb_tx_queues = slave_info.max_tx_queues;
+
+			if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
+				max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;
+
+			if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
+				max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;
 		}
 	}
 
@@ -2263,10 +2271,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	memcpy(&dev_info->default_txconf, &internals->default_txconf,
 	       sizeof(dev_info->default_txconf));
 
-	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
-	       sizeof(dev_info->rx_desc_lim));
-	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
-	       sizeof(dev_info->tx_desc_lim));
+	dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
+	dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;
 
 	/**
 	 * If dedicated hw queues enabled for link bonding device in LACP mode
-- 
2.17.2

Re: [dpdk-stable] [PATCH v3] net/bonding: fix create bonded device test failure

[Ferruh Yigit]

On 2/5/2019 1:39 PM, Hari Kumar Vemula wrote:
> test_create_bonded_device is failing due to improper initialisation in
> bonded device configuration. Which leads to crash while setting up queues.
> 
> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
> bonded device which fails.
> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
> during bond_alloc().
> Hence nb_rx_desc (1024) is > 0 and test fails.
> 
> Fix is to set the default values of rx_desc_lim of bonded device to
> appropriate value.
> Receive the values from slaves configuration like done for other existing
> slave configuration
> 
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Acked-by: Chas Williams <chas3@att.com>

Applied to dpdk-next-net/master, thanks.

[dpdk-stable] [PATCH] net/bonding: fix create bonded device test failure

[Hari Kumar Vemula]

Create bonded device test is failing due to improper initialisation in
bonded device configuration. which leads to crash while setting up queues.

The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
bonded device which fails.
This is due to "rx_desc_lim" is set to 0 as default value of bonded device
during bond_alloc().
Hence nb_rx_desc (1024) is > 0 and test fails.

Fix is to set the default values of rx_desc_lim of bonded device to
appropriate value.

Fixes: 2efb58cbab ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 44deaf119..e0888e960 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2225,6 +2225,11 @@ static void
 bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct bond_dev_private *internals = dev->data->dev_private;
+	struct rte_eth_desc_lim bond_lim = {
+		.nb_max = UINT16_MAX,
+		.nb_min = 0,
+		.nb_align = 1,
+	};
 
 	uint16_t max_nb_rx_queues = UINT16_MAX;
 	uint16_t max_nb_tx_queues = UINT16_MAX;
@@ -2263,10 +2268,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	memcpy(&dev_info->default_txconf, &internals->default_txconf,
 	       sizeof(dev_info->default_txconf));
 
-	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
-	       sizeof(dev_info->rx_desc_lim));
-	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
-	       sizeof(dev_info->tx_desc_lim));
+	dev_info->rx_desc_lim = bond_lim;
+	dev_info->tx_desc_lim = bond_lim;
 
 	/**
 	 * If dedicated hw queues enabled for link bonding device in LACP mode
-- 
2.17.2