[dpdk-dev,v2] examples/exception_path: fix shift operation in lcore setup
Commit Message
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(-)
Comments
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
@@ -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");
}