From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 1CCB3A0096 for ; Wed, 10 Apr 2019 18:45:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 070401B131; Wed, 10 Apr 2019 18:45:55 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id A56141B105 for ; Wed, 10 Apr 2019 18:45:53 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1D849308CF99 for ; Wed, 10 Apr 2019 16:45:53 +0000 (UTC) Received: from rh.redhat.com (ovpn-117-94.ams2.redhat.com [10.36.117.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3F28E5D967; Wed, 10 Apr 2019 16:45:52 +0000 (UTC) From: Kevin Traynor To: David Marchand Cc: dpdk stable Date: Wed, 10 Apr 2019 17:44:00 +0100 Message-Id: <20190410164411.10546-52-ktraynor@redhat.com> In-Reply-To: <20190410164411.10546-1-ktraynor@redhat.com> References: <20190410164411.10546-1-ktraynor@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.44]); Wed, 10 Apr 2019 16:45:53 +0000 (UTC) Subject: [dpdk-stable] patch 'eal: fix core list validation with disabled cores' has been queued to LTS release 18.11.2 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, FYI, your patch has been queued to LTS release 18.11.2 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 04/16/19. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Thanks. Kevin Traynor --- >From 6beb539dd41db01cdf10f61767d93eff2e16f08f Mon Sep 17 00:00:00 2001 From: David Marchand Date: Wed, 13 Feb 2019 21:06:59 +0100 Subject: [PATCH] eal: fix core list validation with disabled cores [ upstream commit 1598c729595d8d59ebb9dce190401846da16c09c ] -l and -c options are two ways to select the cores used by DPDK. Their format differs, but the checks on the selected cores are the same. Use an intermediate array to separate the specific parsing checks from the common consistency checks. The parsing functions now concentrate on validating the passed string and do nothing more. We can report all invalid core indexes rather than only the first error. In the error log message, reporting [0, cfg->lcore_count - 1] as a valid range is then wrong when the core list is not continuous. Example on my 8 cpus laptop with core 2 and 6 disabled. echo 0 > /sys/devices/system/cpu/cpu2/online echo 0 > /sys/devices/system/cpu/cpu6/online Before: ./master/app/testpmd -l 0-7 --no-huge -m 512 -- --total-num-mbufs 2048 EAL: Detected 6 lcore(s) EAL: Detected 1 NUMA nodes EAL: invalid core list, please check core numbers are in [0, 5] range ... After: ./master/app/testpmd -l 0-7 --no-huge -m 512 -- --total-num-mbufs 2048 EAL: Detected 6 lcore(s) EAL: Detected 1 NUMA nodes EAL: lcore 2 unavailable EAL: lcore 6 unavailable EAL: invalid core list, please check specified cores are part of 0-1,3-5,7 ... Fixes: d888cb8b9613 ("eal: add core list input format") Fixes: b38693b612b4 ("eal: fix core number validation") Signed-off-by: David Marchand --- lib/librte_eal/common/eal_common_options.c | 203 +++++++++++++++------ 1 file changed, 145 insertions(+), 58 deletions(-) diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c index 74ca26340..d4ab5e235 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c @@ -419,19 +419,42 @@ eal_service_cores_parsed(void) static int -eal_parse_coremask(const char *coremask) +update_lcore_config(int *cores) { struct rte_config *cfg = rte_eal_get_configuration(); - int i, j, idx = 0; + unsigned int count = 0; + unsigned int i; + int ret = 0; + + for (i = 0; i < RTE_MAX_LCORE; i++) { + if (cores[i] != -1) { + if (!lcore_config[i].detected) { + RTE_LOG(ERR, EAL, "lcore %u unavailable\n", i); + ret = -1; + continue; + } + cfg->lcore_role[i] = ROLE_RTE; + count++; + } else { + cfg->lcore_role[i] = ROLE_OFF; + } + lcore_config[i].core_index = cores[i]; + } + if (!ret) + cfg->lcore_count = count; + return ret; +} + +static int +eal_parse_coremask(const char *coremask, int *cores) +{ unsigned count = 0; - char c; + int i, j, idx; int val; + char c; - if (eal_service_cores_parsed()) - RTE_LOG(WARNING, EAL, - "Service cores parsed before dataplane cores. " - "Please ensure -c is before -s or -S\n"); + for (idx = 0; idx < RTE_MAX_LCORE; idx++) + cores[idx] = -1; + idx = 0; - if (coremask == NULL) - return -1; /* Remove all blank characters ahead and after . * Remove 0x/0X if exists. @@ -458,16 +481,6 @@ eal_parse_coremask(const char *coremask) { if ((1 << j) & val) { - if (!lcore_config[idx].detected) { - RTE_LOG(ERR, EAL, "lcore %u " - "unavailable\n", idx); - return -1; - } - - cfg->lcore_role[idx] = ROLE_RTE; - lcore_config[idx].core_index = count; + cores[idx] = count; count++; - } else { - cfg->lcore_role[idx] = ROLE_OFF; - lcore_config[idx].core_index = -1; } } @@ -476,12 +489,6 @@ eal_parse_coremask(const char *coremask) if (coremask[i] != '0') return -1; - for (; idx < RTE_MAX_LCORE; idx++) { - cfg->lcore_role[idx] = ROLE_OFF; - lcore_config[idx].core_index = -1; - } if (count == 0) return -1; - /* Update the count of enabled logical cores of the EAL configuration */ - cfg->lcore_count = count; return 0; } @@ -564,7 +571,6 @@ eal_parse_service_corelist(const char *corelist) static int -eal_parse_corelist(const char *corelist) +eal_parse_corelist(const char *corelist, int *cores) { - struct rte_config *cfg = rte_eal_get_configuration(); unsigned count = 0; char *end = NULL; @@ -572,11 +578,6 @@ eal_parse_corelist(const char *corelist) int idx; - if (eal_service_cores_parsed()) - RTE_LOG(WARNING, EAL, - "Service cores parsed before dataplane cores. " - "Please ensure -l is before -s or -S\n"); - - if (corelist == NULL) - return -1; + for (idx = 0; idx < RTE_MAX_LCORE; idx++) + cores[idx] = -1; /* Remove all blank characters ahead */ @@ -584,10 +585,4 @@ eal_parse_corelist(const char *corelist) corelist++; - /* Reset config */ - for (idx = 0; idx < RTE_MAX_LCORE; idx++) { - cfg->lcore_role[idx] = ROLE_OFF; - lcore_config[idx].core_index = -1; - } - /* Get list of cores */ min = RTE_MAX_LCORE; @@ -599,8 +594,8 @@ eal_parse_corelist(const char *corelist) errno = 0; idx = strtol(corelist, &end, 10); - if (idx < 0 || idx >= (int)cfg->lcore_count) - return -1; if (errno || end == NULL) return -1; + if (idx < 0 || idx >= RTE_MAX_LCORE) + return -1; while (isblank(*end)) end++; @@ -612,7 +607,6 @@ eal_parse_corelist(const char *corelist) min = idx; for (idx = min; idx <= max; idx++) { - if (cfg->lcore_role[idx] != ROLE_RTE) { - cfg->lcore_role[idx] = ROLE_RTE; - lcore_config[idx].core_index = count; + if (cores[idx] == -1) { + cores[idx] = count; count++; } @@ -626,8 +620,4 @@ eal_parse_corelist(const char *corelist) if (count == 0) return -1; - - /* Update the count of enabled logical cores of the EAL configuration */ - cfg->lcore_count = count; - return 0; } @@ -1105,4 +1095,73 @@ eal_parse_iova_mode(const char *name) } +/* caller is responsible for freeing the returned string */ +static char * +available_cores(void) +{ + char *str = NULL; + int previous; + int sequence; + char *tmp; + int idx; + + /* find the first available cpu */ + for (idx = 0; idx < RTE_MAX_LCORE; idx++) { + if (!lcore_config[idx].detected) + continue; + break; + } + if (idx >= RTE_MAX_LCORE) + return NULL; + + /* first sequence */ + if (asprintf(&str, "%d", idx) < 0) + return NULL; + previous = idx; + sequence = 0; + + for (idx++ ; idx < RTE_MAX_LCORE; idx++) { + if (!lcore_config[idx].detected) + continue; + + if (idx == previous + 1) { + previous = idx; + sequence = 1; + continue; + } + + /* finish current sequence */ + if (sequence) { + if (asprintf(&tmp, "%s-%d", str, previous) < 0) { + free(str); + return NULL; + } + free(str); + str = tmp; + } + + /* new sequence */ + if (asprintf(&tmp, "%s,%d", str, idx) < 0) { + free(str); + return NULL; + } + free(str); + str = tmp; + previous = idx; + sequence = 0; + } + + /* finish last sequence */ + if (sequence) { + if (asprintf(&tmp, "%s-%d", str, previous) < 0) { + free(str); + return NULL; + } + free(str); + str = tmp; + } + + return str; +} + int eal_parse_common_option(int opt, const char *optarg, @@ -1111,5 +1170,4 @@ 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) { @@ -1135,7 +1193,21 @@ eal_parse_common_option(int opt, const char *optarg, break; /* coremask */ - case 'c': - if (eal_parse_coremask(optarg) < 0) { - RTE_LOG(ERR, EAL, "invalid coremask\n"); + case 'c': { + int lcore_indexes[RTE_MAX_LCORE]; + + if (eal_service_cores_parsed()) + RTE_LOG(WARNING, EAL, + "Service cores parsed before dataplane cores. Please ensure -c is before -s or -S\n"); + if (eal_parse_coremask(optarg, lcore_indexes) < 0) { + RTE_LOG(ERR, EAL, "invalid coremask syntax\n"); + return -1; + } + if (update_lcore_config(lcore_indexes) < 0) { + char *available = available_cores(); + + RTE_LOG(ERR, EAL, + "invalid coremask, please check specified cores are part of %s\n", + available); + free(available); return -1; } @@ -1151,10 +1223,24 @@ eal_parse_common_option(int opt, const char *optarg, core_parsed = LCORE_OPT_MSK; break; + } /* corelist */ - case 'l': - if (eal_parse_corelist(optarg) < 0) { + case 'l': { + int lcore_indexes[RTE_MAX_LCORE]; + + if (eal_service_cores_parsed()) + RTE_LOG(WARNING, EAL, + "Service cores parsed before dataplane cores. Please ensure -l is before -s or -S\n"); + + if (eal_parse_corelist(optarg, lcore_indexes) < 0) { + RTE_LOG(ERR, EAL, "invalid core list syntax\n"); + return -1; + } + if (update_lcore_config(lcore_indexes) < 0) { + char *available = available_cores(); + RTE_LOG(ERR, EAL, - "invalid core list, please check core numbers are in [0, %u] range\n", - cfg->lcore_count-1); + "invalid core list, please check specified cores are part of %s\n", + available); + free(available); return -1; } @@ -1170,4 +1256,5 @@ eal_parse_common_option(int opt, const char *optarg, core_parsed = LCORE_OPT_LST; break; + } /* service coremask */ case 's': -- 2.20.1 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2019-04-10 14:06:11.883745285 +0100 +++ 0052-eal-fix-core-list-validation-with-disabled-cores.patch 2019-04-10 14:06:08.003291061 +0100 @@ -1,8 +1,10 @@ -From 1598c729595d8d59ebb9dce190401846da16c09c Mon Sep 17 00:00:00 2001 +From 6beb539dd41db01cdf10f61767d93eff2e16f08f Mon Sep 17 00:00:00 2001 From: David Marchand Date: Wed, 13 Feb 2019 21:06:59 +0100 Subject: [PATCH] eal: fix core list validation with disabled cores +[ upstream commit 1598c729595d8d59ebb9dce190401846da16c09c ] + -l and -c options are two ways to select the cores used by DPDK. Their format differs, but the checks on the selected cores are the same. Use an intermediate array to separate the specific parsing checks from @@ -36,7 +38,6 @@ Fixes: d888cb8b9613 ("eal: add core list input format") Fixes: b38693b612b4 ("eal: fix core number validation") -Cc: stable@dpdk.org Signed-off-by: David Marchand --- @@ -44,10 +45,10 @@ 1 file changed, 145 insertions(+), 58 deletions(-) diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c -index 8a431457b..8dcfeb9b2 100644 +index 74ca26340..d4ab5e235 100644 --- a/lib/librte_eal/common/eal_common_options.c +++ b/lib/librte_eal/common/eal_common_options.c -@@ -420,19 +420,42 @@ eal_service_cores_parsed(void) +@@ -419,19 +419,42 @@ eal_service_cores_parsed(void) static int -eal_parse_coremask(const char *coremask) @@ -99,7 +100,7 @@ - return -1; /* Remove all blank characters ahead and after . * Remove 0x/0X if exists. -@@ -459,16 +482,6 @@ eal_parse_coremask(const char *coremask) +@@ -458,16 +481,6 @@ eal_parse_coremask(const char *coremask) { if ((1 << j) & val) { - if (!lcore_config[idx].detected) { @@ -117,7 +118,7 @@ - lcore_config[idx].core_index = -1; } } -@@ -477,12 +490,6 @@ eal_parse_coremask(const char *coremask) +@@ -476,12 +489,6 @@ eal_parse_coremask(const char *coremask) if (coremask[i] != '0') return -1; - for (; idx < RTE_MAX_LCORE; idx++) { @@ -130,7 +131,7 @@ - cfg->lcore_count = count; return 0; } -@@ -565,7 +572,6 @@ eal_parse_service_corelist(const char *corelist) +@@ -564,7 +571,6 @@ eal_parse_service_corelist(const char *corelist) static int -eal_parse_corelist(const char *corelist) @@ -139,7 +140,7 @@ - struct rte_config *cfg = rte_eal_get_configuration(); unsigned count = 0; char *end = NULL; -@@ -573,11 +579,6 @@ eal_parse_corelist(const char *corelist) +@@ -572,11 +578,6 @@ eal_parse_corelist(const char *corelist) int idx; - if (eal_service_cores_parsed()) @@ -153,7 +154,7 @@ + cores[idx] = -1; /* Remove all blank characters ahead */ -@@ -585,10 +586,4 @@ eal_parse_corelist(const char *corelist) +@@ -584,10 +585,4 @@ eal_parse_corelist(const char *corelist) corelist++; - /* Reset config */ @@ -164,7 +165,7 @@ - /* Get list of cores */ min = RTE_MAX_LCORE; -@@ -600,8 +595,8 @@ eal_parse_corelist(const char *corelist) +@@ -599,8 +594,8 @@ eal_parse_corelist(const char *corelist) errno = 0; idx = strtol(corelist, &end, 10); - if (idx < 0 || idx >= (int)cfg->lcore_count) @@ -175,7 +176,7 @@ + return -1; while (isblank(*end)) end++; -@@ -613,7 +608,6 @@ eal_parse_corelist(const char *corelist) +@@ -612,7 +607,6 @@ eal_parse_corelist(const char *corelist) min = idx; for (idx = min; idx <= max; idx++) { - if (cfg->lcore_role[idx] != ROLE_RTE) { @@ -185,7 +186,7 @@ + cores[idx] = count; count++; } -@@ -627,8 +621,4 @@ eal_parse_corelist(const char *corelist) +@@ -626,8 +620,4 @@ eal_parse_corelist(const char *corelist) if (count == 0) return -1; - @@ -194,7 +195,7 @@ - return 0; } -@@ -1106,4 +1096,73 @@ eal_parse_iova_mode(const char *name) +@@ -1105,4 +1095,73 @@ eal_parse_iova_mode(const char *name) } +/* caller is responsible for freeing the returned string */ @@ -268,13 +269,13 @@ + int eal_parse_common_option(int opt, const char *optarg, -@@ -1112,5 +1171,4 @@ eal_parse_common_option(int opt, const char *optarg, +@@ -1111,5 +1170,4 @@ 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) { -@@ -1136,7 +1194,21 @@ eal_parse_common_option(int opt, const char *optarg, +@@ -1135,7 +1193,21 @@ eal_parse_common_option(int opt, const char *optarg, break; /* coremask */ - case 'c': @@ -299,7 +300,7 @@ + free(available); return -1; } -@@ -1152,10 +1224,24 @@ eal_parse_common_option(int opt, const char *optarg, +@@ -1151,10 +1223,24 @@ eal_parse_common_option(int opt, const char *optarg, core_parsed = LCORE_OPT_MSK; break; + } @@ -328,7 +329,7 @@ + free(available); return -1; } -@@ -1171,4 +1257,5 @@ eal_parse_common_option(int opt, const char *optarg, +@@ -1170,4 +1256,5 @@ eal_parse_common_option(int opt, const char *optarg, core_parsed = LCORE_OPT_LST; break; + }