* [dpdk-dev] [PATCH] examples/exception_path: fix shift operation in lcore setup
@ 2016-08-03 11:44 Daniel Mrzyglod
2016-08-04 9:02 ` Ferruh Yigit
2016-08-09 12:37 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod
0 siblings, 2 replies; 4+ messages in thread
From: Daniel Mrzyglod @ 2016-08-03 11:44 UTC (permalink / raw)
To: dev; +Cc: Daniel Mrzyglod
The operaton may have an undefined behavior or yield to an unexpected result.
A bit shift operation has a shift amount which is too large or has a negative value.
Coverity issue: 30688
Fixes: ea977ff1cb0b ("examples/exception_path: fix shift operation in lcore setup")
The previous patch forget to fix values also for input_cores_mask
Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
examples/exception_path/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/exception_path/main.c b/examples/exception_path/main.c
index e5eedcc..88e7708 100644
--- a/examples/exception_path/main.c
+++ b/examples/exception_path/main.c
@@ -341,7 +341,7 @@ setup_port_lcore_affinities(void)
/* Setup port_ids[] array, and check masks were ok */
RTE_LCORE_FOREACH(i) {
- if (input_cores_mask & (1ULL << i)) {
+ if (input_cores_mask & (1ULL << (i & 0x3f))) {
/* Skip ports that are not enabled */
while ((ports_mask & (1 << rx_port)) == 0) {
rx_port++;
--
2.7.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/exception_path: fix shift operation in lcore setup
2016-08-03 11:44 [dpdk-dev] [PATCH] examples/exception_path: fix shift operation in lcore setup Daniel Mrzyglod
@ 2016-08-04 9:02 ` Ferruh Yigit
2016-08-09 12:37 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod
1 sibling, 0 replies; 4+ messages in thread
From: Ferruh Yigit @ 2016-08-04 9:02 UTC (permalink / raw)
To: Daniel Mrzyglod, dev
On 8/3/2016 12:44 PM, Daniel Mrzyglod wrote:
> The operaton may have an undefined behavior or yield to an unexpected result.
> A bit shift operation has a shift amount which is too large or has a negative value.
>
> Coverity issue: 30688
> Fixes: ea977ff1cb0b ("examples/exception_path: fix shift operation in lcore setup")
> The previous patch forget to fix values also for input_cores_mask
>
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> ---
> examples/exception_path/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/exception_path/main.c b/examples/exception_path/main.c
> index e5eedcc..88e7708 100644
> --- a/examples/exception_path/main.c
> +++ b/examples/exception_path/main.c
> @@ -341,7 +341,7 @@ setup_port_lcore_affinities(void)
>
> /* Setup port_ids[] array, and check masks were ok */
> RTE_LCORE_FOREACH(i) {
> - if (input_cores_mask & (1ULL << i)) {
> + if (input_cores_mask & (1ULL << (i & 0x3f))) {
I guess 0x3f is because "unsigned long long" is 64bits long, not sure if
we should hardcode this assumption. ULL can be >= 64bits.
RTE_LCORE_FOREACH(i) already makes sure "i" < RTE_MAX_CORE, and
RTE_MAX_CORE is 128 with current default config. So if user provides a
core value > 64, it is valid but will be ignored because of this check.
Another thing is "input_cores_mask" is also 64bits long, so even this
fixed application will not able to use this setting correctly.
I think it is good to
a) add flexible variable size set_bit/clear_bit/test_bit functions, like
Linux ones
b) make "input_cores_mask" an array that is large enough to keep
RTE_MAX_CORE
Although not sure if that is too much effort for this fix.
> /* Skip ports that are not enabled */
> while ((ports_mask & (1 << rx_port)) == 0) {
> rx_port++;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [dpdk-dev] [PATCH v2] examples/exception_path: fix shift operation in lcore setup
2016-08-03 11:44 [dpdk-dev] [PATCH] examples/exception_path: fix shift operation in lcore setup Daniel Mrzyglod
2016-08-04 9:02 ` Ferruh Yigit
@ 2016-08-09 12:37 ` Daniel Mrzyglod
2016-09-11 12:38 ` Yuanhan Liu
1 sibling, 1 reply; 4+ messages in thread
From: Daniel Mrzyglod @ 2016-08-09 12:37 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, Daniel Mrzyglod
The operaton may have an undefined behavior or yield to an unexpected result.
A bit shift operation has a shift amount which is too large or has a negative value.
As was mentioned in mailing list core list was limited to 64 so i changed bitmask
to core array
Coverity issue: 30688
Fixes: ea977ff1cb0b ("examples/exception_path: fix shift operation in lcore setup")
Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
examples/exception_path/main.c | 74 ++++++++++++++++++++++++++++++++++--------
1 file changed, 61 insertions(+), 13 deletions(-)
diff --git a/examples/exception_path/main.c b/examples/exception_path/main.c
index e5eedcc..1493338 100644
--- a/examples/exception_path/main.c
+++ b/examples/exception_path/main.c
@@ -128,11 +128,11 @@ static struct rte_mempool * pktmbuf_pool = NULL;
/* Mask of enabled ports */
static uint32_t ports_mask = 0;
-/* Mask of cores that read from NIC and write to tap */
-static uint64_t input_cores_mask = 0;
+/* Table of cores that read from NIC and write to tap */
+static uint8_t input_cores_table[RTE_MAX_LCORE];
-/* Mask of cores that read from tap and write to NIC */
-static uint64_t output_cores_mask = 0;
+/* Table of cores that read from tap and write to NIC */
+static uint8_t output_cores_table[RTE_MAX_LCORE];
/* Array storing port_id that is associated with each lcore */
static uint8_t port_ids[RTE_MAX_LCORE];
@@ -224,7 +224,7 @@ main_loop(__attribute__((unused)) void *arg)
char tap_name[IFNAMSIZ];
int tap_fd;
- if ((1ULL << lcore_id) & input_cores_mask) {
+ if (input_cores_table[lcore_id]) {
/* Create new tap interface */
snprintf(tap_name, IFNAMSIZ, "tap_dpdk_%.2u", lcore_id);
tap_fd = tap_create(tap_name);
@@ -257,7 +257,7 @@ main_loop(__attribute__((unused)) void *arg)
}
}
}
- else if ((1ULL << lcore_id) & output_cores_mask) {
+ else if (output_cores_table[lcore_id]) {
/* Create new tap interface */
snprintf(tap_name, IFNAMSIZ, "tap_dpdk_%.2u", lcore_id);
tap_fd = tap_create(tap_name);
@@ -341,7 +341,7 @@ setup_port_lcore_affinities(void)
/* Setup port_ids[] array, and check masks were ok */
RTE_LCORE_FOREACH(i) {
- if (input_cores_mask & (1ULL << i)) {
+ if (input_cores_table[i]) {
/* Skip ports that are not enabled */
while ((ports_mask & (1 << rx_port)) == 0) {
rx_port++;
@@ -350,7 +350,7 @@ setup_port_lcore_affinities(void)
}
port_ids[i] = rx_port++;
- } else if (output_cores_mask & (1ULL << (i & 0x3f))) {
+ } else if (output_cores_table[i]) {
/* Skip ports that are not enabled */
while ((ports_mask & (1 << tx_port)) == 0) {
tx_port++;
@@ -373,6 +373,54 @@ fail:
FATAL_ERROR("Invalid core/port masks specified on command line");
}
+static int parse_hex2coretable(const char *coremask_string, uint8_t *core_table,
+ int core_table_size)
+{
+
+ char portmask[RTE_MAX_LCORE];
+
+ int coremask_string_len;
+ int i = 0, j = 0;
+ uint64_t num;
+ char tmp_char;
+ char *end = NULL;
+
+ coremask_string_len = strlen(coremask_string);
+ if ((coremask_string_len > (RTE_MAX_LCORE / 4) + 2)
+ || (core_table_size > RTE_MAX_LCORE) || (coremask_string == NULL)
+ || (core_table == NULL))
+ return -1;
+
+ memset(portmask, '\0', sizeof(portmask));
+ memset(core_table, '\0', sizeof(uint8_t) * core_table_size);
+ strncpy(portmask, coremask_string, coremask_string_len);
+
+ if (coremask_string[i] == '0') {
+ ++i;
+ if (coremask_string[i] == 'X' || coremask_string[i] == 'x')
+ ++i;
+ }
+
+ j = 0;
+ while (coremask_string_len - i > 0) {
+
+ tmp_char = portmask[coremask_string_len - 1];
+ end = NULL;
+ num = strtoull(&tmp_char, &end, 16);
+ if ((end == &tmp_char))
+ return -1;
+
+ for (int z = 0; z < 4; z++)
+ if (((num) & (1 << (z))) != 0)
+ core_table[z + j * 4] = 1;
+
+ coremask_string_len--;
+ j++;
+ }
+
+ return 0;
+}
+
/* Parse the arguments given in the command line of the application */
static void
parse_args(int argc, char **argv)
@@ -382,15 +430,15 @@ parse_args(int argc, char **argv)
/* Disable printing messages within getopt() */
opterr = 0;
-
+ int reti = 0, reto = 0;
/* Parse command line */
while ((opt = getopt(argc, argv, "i:o:p:")) != EOF) {
switch (opt) {
case 'i':
- input_cores_mask = parse_unsigned(optarg);
+ reti = parse_hex2coretable(optarg, input_cores_table, RTE_MAX_LCORE);
break;
case 'o':
- output_cores_mask = parse_unsigned(optarg);
+ reto = parse_hex2coretable(optarg, output_cores_table, RTE_MAX_LCORE);
break;
case 'p':
ports_mask = parse_unsigned(optarg);
@@ -402,11 +450,11 @@ parse_args(int argc, char **argv)
}
/* Check that options were parsed ok */
- if (input_cores_mask == 0) {
+ if (reti != 0) {
print_usage(prgname);
FATAL_ERROR("IN_CORES not specified correctly");
}
- if (output_cores_mask == 0) {
+ if (reto != 0) {
print_usage(prgname);
FATAL_ERROR("OUT_CORES not specified correctly");
}
--
2.7.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH v2] examples/exception_path: fix shift operation in lcore setup
2016-08-09 12:37 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod
@ 2016-09-11 12:38 ` Yuanhan Liu
0 siblings, 0 replies; 4+ messages in thread
From: Yuanhan Liu @ 2016-09-11 12:38 UTC (permalink / raw)
To: Daniel Mrzyglod; +Cc: ferruh.yigit, dev, Thomas Monjalon
On Tue, Aug 09, 2016 at 02:37:15PM +0200, Daniel Mrzyglod wrote:
> The operaton may have an undefined behavior or yield to an unexpected result.
> A bit shift operation has a shift amount which is too large or has a negative value.
>
> As was mentioned in mailing list core list was limited to 64 so i changed bitmask
> to core array
>
> Coverity issue: 30688
> Fixes: ea977ff1cb0b ("examples/exception_path: fix shift operation in lcore setup")
>
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> ---
...
> + if ((end == &tmp_char))
> + return -1;
Hi,
FYI, my testrobot caught some errors when this patch is applied.
(BTW, gcc builds fine)
--yliu
---
x86_64-native-linuxapp-clang
============================
/root/dpdk/examples/exception_path/main.c:410:12: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
if ((end == &tmp_char))
~~~~^~~~~~~~~~~~
/root/dpdk/examples/exception_path/main.c:410:12: note: remove extraneous parentheses around the comparison to silence this warning
if ((end == &tmp_char))
~ ^ ~
/root/dpdk/examples/exception_path/main.c:410:12: note: use '=' to turn this equality comparison into an assignment
if ((end == &tmp_char))
^~
=
1 error generated.
make[1]: *** [main.o] Error 1
make: *** [all] Error 2
error: build examples/exception_path failed
error: build failed
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-11 12:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-03 11:44 [dpdk-dev] [PATCH] examples/exception_path: fix shift operation in lcore setup Daniel Mrzyglod
2016-08-04 9:02 ` Ferruh Yigit
2016-08-09 12:37 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod
2016-09-11 12:38 ` Yuanhan Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).