Hi Pawel, I don't see much different there. If replacing '@' to '.'; '()' to '[]'; and ',' to '/'; they're almost the same. Without having rx/tx case, so ':' is useless in our case. Considering the semantic, '@'(at) is more readable than '.' for core assignment.
-Liang Cunming > -----Original Message----- > From: Wodkowski, PawelX > Sent: Thursday, January 22, 2015 11:17 PM > To: Ananyev, Konstantin; Richardson, Bruce; Liang, Cunming > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for > cpu > assignment > > Hi, > I want to mention that similar but for me much more readable syntax have > Pktgen-DPDK for defining core - port mapping. Maybe we can adopt this syntax > for new '--lcores' parameter. > > See '-m' parameter syntax on Pktgen readme. > https://github.com/pktgen/Pktgen-DPDK/blob/master/dpdk/examples/pktgen/R > EADME.md > > > -----Original Message----- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ananyev, Konstantin > > Sent: Thursday, January 22, 2015 3:34 PM > > To: Richardson, Bruce; Liang, Cunming > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for > > cpu > > assignment > > > > Hi Bruce, > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson > > > Sent: Thursday, January 22, 2015 12:19 PM > > > To: Liang, Cunming > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' > > > for cpu > > assignment > > > > > > On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote: > > > > It supports one new eal long option '--lcores' for EAL thread cpuset > > assignment. > > > > > > > > The format pattern: > > > > --lcores='lcores[@cpus]<,lcores[@cpus]>' > > > > lcores, cpus could be a single digit or a group. > > > > '(' and ')' are necessary if it's a group. > > > > If not supply '@cpus', the value of cpus uses the same as lcores. > > > > > > > > e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below > > > > lcore 0 runs on cpuset 0x41 (cpu 0,6) > > > > lcore 1 runs on cpuset 0x2 (cpu 1) > > > > lcore 2 runs on cpuset 0xe0 (cpu 5,6,7) > > > > lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2) > > > > lcore 6 runs on cpuset 0x41 (cpu 0,6) > > > > > > > > > > This strikes me as very confusing, though a couple of tweaks might help > > > with > > > readability. The lcore 0 at the end is especially confusing. > > > > Didn't get you here: do you find (0,6) confusing, right? > > Because braces implicitly specifies affinity for group of en-braced lcores? > > > > > Perhaps we can > > > limit the allowed formats here, > > > * require the lcore_id to be specified - the lack of an lcore id for the > > > last part > > > makes having it as lcore 0 surprising. > > > > Again, not sure I understand you properly: lcore_id(s) are always specified > > explicitly. > > Physical cpus part might be omitted. > > > > > * only allow one lcore id to be given for each set of cores. > > > > So you mean for '(3-5)@(0,2)' user would have to: '3@(0,2),4@(0,2),5@(0,2)'? > > I don't see big difference here, but imagine you'd like to create a pool of > > 32 EAL- > > threads running on same cpu set. > > With current syntax it is just something like: '(32-63)@(0-7)'. > > With what you proposing it will be a very long list. > > > > > > > > I think it may still be readable if we allow the core set to be omitted > > > if its > > > to be the same as the lcore_id. > > > > I think that is supported. > > See lcore_id=1 in Steve's example above. > > As I understand: --lcores='0,2,3-5' is equal to '-l 0,2,3-5' and to '-c > > 0x3d'. > > > > Konstantin > > > > > > > > It's probably still not going to be very tidy, but I think we can improve > > > things. > > > > > > /Bruce > > > > > > > Signed-off-by: Cunming Liang <cunming.liang at intel.com> > > > > --- > > > > lib/librte_eal/common/eal_common_launch.c | 1 - > > > > lib/librte_eal/common/eal_common_options.c | 262 > > ++++++++++++++++++++++++++++- > > > > lib/librte_eal/common/eal_options.h | 2 + > > > > lib/librte_eal/linuxapp/eal/Makefile | 1 + > > > > 4 files changed, 261 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/lib/librte_eal/common/eal_common_launch.c > > b/lib/librte_eal/common/eal_common_launch.c > > > > index 599f83b..2d732b1 100644 > > > > --- a/lib/librte_eal/common/eal_common_launch.c > > > > +++ b/lib/librte_eal/common/eal_common_launch.c > > > > @@ -117,4 +117,3 @@ rte_eal_mp_wait_lcore(void) > > > > rte_eal_wait_lcore(lcore_id); > > > > } > > > > } > > > > - > > > > diff --git a/lib/librte_eal/common/eal_common_options.c > > b/lib/librte_eal/common/eal_common_options.c > > > > index e2810ab..fc47588 100644 > > > > --- a/lib/librte_eal/common/eal_common_options.c > > > > +++ b/lib/librte_eal/common/eal_common_options.c > > > > @@ -45,6 +45,7 @@ > > > > #include <rte_lcore.h> > > > > #include <rte_version.h> > > > > #include <rte_devargs.h> > > > > +#include <rte_memcpy.h> > > > > > > > > #include "eal_internal_cfg.h" > > > > #include "eal_options.h" > > > > @@ -85,6 +86,7 @@ eal_long_options[] = { > > > > {OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM}, > > > > {OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM}, > > > > {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM}, > > > > + {OPT_LCORES, 1, 0, OPT_LCORES_NUM}, > > > > {0, 0, 0, 0} > > > > }; > > > > > > > > @@ -255,9 +257,11 @@ eal_parse_corelist(const char *corelist) > > > > if (min == RTE_MAX_LCORE) > > > > min = idx; > > > > for (idx = min; idx <= max; idx++) { > > > > - cfg->lcore_role[idx] = ROLE_RTE; > > > > - lcore_config[idx].core_index = count; > > > > - count++; > > > > + if (cfg->lcore_role[idx] != ROLE_RTE) { > > > > + cfg->lcore_role[idx] = ROLE_RTE; > > > > + lcore_config[idx].core_index = > > > > count; > > > > + count++; > > > > + } > > > > } > > > > min = RTE_MAX_LCORE; > > > > } else > > > > @@ -289,6 +293,241 @@ eal_parse_master_lcore(const char *arg) > > > > return 0; > > > > } > > > > > > > > +/* > > > > + * Parse elem, the elem could be single number or '(' ')' group > > > > + * Within group elem, '-' used for a range seperator; > > > > + * ',' used for a single number. > > > > + */ > > > > +static int > > > > +eal_parse_set(const char *input, uint16_t set[], unsigned num) > > > > +{ > > > > + unsigned idx; > > > > + const char *str = input; > > > > + char *end = NULL; > > > > + unsigned min, max; > > > > + > > > > + memset(set, 0, num * sizeof(uint16_t)); > > > > + > > > > + while (isblank(*str)) > > > > + str++; > > > > + > > > > + /* only digit or left bracket is qulify for start point */ > > > > + if ((!isdigit(*str) && *str != '(') || *str == '\0') > > > > + return -1; > > > > + > > > > + /* process single number */ > > > > + if (*str != '(') { > > > > + errno = 0; > > > > + idx = strtoul(str, &end, 10); > > > > + if (errno || end == NULL || idx >= num) > > > > + return -1; > > > > + else { > > > > + while (isblank(*end)) > > > > + end++; > > > > + > > > > + if (*end != ',' && *end != '\0' && > > > > + *end != '@') > > > > + return -1; > > > > + > > > > + set[idx] = 1; > > > > + return end - input; > > > > + } > > > > + } > > > > + > > > > + /* process set within bracket */ > > > > + str++; > > > > + while (isblank(*str)) > > > > + str++; > > > > + if (*str == '\0') > > > > + return -1; > > > > + > > > > + min = RTE_MAX_LCORE; > > > > + do { > > > > + > > > > + /* go ahead to the first digit */ > > > > + while (isblank(*str)) > > > > + str++; > > > > + if (!isdigit(*str)) > > > > + return -1; > > > > + > > > > + /* get the digit value */ > > > > + errno = 0; > > > > + idx = strtoul(str, &end, 10); > > > > + if (errno || end == NULL || idx >= num) > > > > + return -1; > > > > + > > > > + /* go ahead to separator '-',',' and ')' */ > > > > + while (isblank(*end)) > > > > + end++; > > > > + if (*end == '-') { > > > > + if (min == RTE_MAX_LCORE) > > > > + min = idx; > > > > + else /* avoid continuous '-' */ > > > > + return -1; > > > > + } else if ((*end == ',') || (*end == ')')) { > > > > + max = idx; > > > > + if (min == RTE_MAX_LCORE) > > > > + min = idx; > > > > + for (idx = RTE_MIN(min, max); > > > > + idx <= RTE_MAX(min, max); idx++) > > > > + set[idx] = 1; > > > > + > > > > + min = RTE_MAX_LCORE; > > > > + } else > > > > + return -1; > > > > + > > > > + str = end + 1; > > > > + } while (*end != '\0' && *end != ')'); > > > > + > > > > + return str - input; > > > > +} > > > > + > > > > +/* convert from set array to cpuset bitmap */ > > > > +static inline int > > > > +convert_to_cpuset(rte_cpuset_t *cpusetp, > > > > + uint16_t *set, unsigned num) > > > > +{ > > > > + unsigned idx; > > > > + > > > > + CPU_ZERO(cpusetp); > > > > + > > > > + for (idx = 0; idx < num; idx++) { > > > > + if (!set[idx]) > > > > + continue; > > > > + > > > > + if (!lcore_config[idx].detected) { > > > > + RTE_LOG(ERR, EAL, "core %u " > > > > + "unavailable\n", idx); > > > > + return -1; > > > > + } > > > > + > > > > + CPU_SET(idx, cpusetp); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +/* > > > > + * The format pattern: --lcores='lcores[@cpus]<,lcores[@cpus]>' > > > > + * lcores, cpus could be a single digit or a group. > > > > + * '(' and ')' are necessary if it's a group. > > > > + * If not supply '@cpus', the value of cpus uses the same as lcores. > > > > + * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means start 7 EAL thread as below > > > > + * lcore 0 runs on cpuset 0x41 (cpu 0,6) > > > > + * lcore 1 runs on cpuset 0x2 (cpu 1) > > > > + * lcore 2 runs on cpuset 0xe0 (cpu 5,6,7) > > > > + * lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2) > > > > + * lcore 6 runs on cpuset 0x41 (cpu 0,6) > > > > + */ > > > > +static int > > > > +eal_parse_lcores(const char *lcores) > > > > +{ > > > > + struct rte_config *cfg = rte_eal_get_configuration(); > > > > + static uint16_t set[RTE_MAX_LCORE]; > > > > + unsigned idx = 0; > > > > + int i; > > > > + unsigned count = 0; > > > > + const char *lcore_start = NULL; > > > > + const char *end = NULL; > > > > + int offset; > > > > + rte_cpuset_t cpuset; > > > > + int ret = -1; > > > > + > > > > + if (lcores == NULL) > > > > + return -1; > > > > + > > > > + /* Remove all blank characters ahead and after */ > > > > + while (isblank(*lcores)) > > > > + lcores++; > > > > + i = strnlen(lcores, sysconf(_SC_ARG_MAX)); > > > > + while ((i > 0) && isblank(lcores[i - 1])) > > > > + i--; > > > > + > > > > + CPU_ZERO(&cpuset); > > > > + > > > > + /* Reset lcore config */ > > > > + for (idx = 0; idx < RTE_MAX_LCORE; idx++) { > > > > + cfg->lcore_role[idx] = ROLE_OFF; > > > > + lcore_config[idx].core_index = -1; > > > > + CPU_ZERO(&lcore_config[idx].cpuset); > > > > + } > > > > + > > > > + /* Get list of cores */ > > > > + do { > > > > + while (isblank(*lcores)) > > > > + lcores++; > > > > + if (*lcores == '\0') > > > > + goto err; > > > > + > > > > + /* record lcore_set start point */ > > > > + lcore_start = lcores; > > > > + > > > > + /* go across a complete bracket */ > > > > + if (*lcore_start == '(') { > > > > + lcores += strcspn(lcores, ")"); > > > > + if (*lcores++ == '\0') > > > > + goto err; > > > > + } > > > > + > > > > + /* scan the separator '@', ','(next) or '\0'(finish) */ > > > > + lcores += strcspn(lcores, "@,"); > > > > + > > > > + if (*lcores == '@') { > > > > + /* explict assign cpu_set */ > > > > + offset = eal_parse_set(lcores + 1, set, > > > > RTE_DIM(set)); > > > > + if (offset < 0) > > > > + goto err; > > > > + > > > > + /* prepare cpu_set and update the end cursor */ > > > > + if (0 > convert_to_cpuset(&cpuset, > > > > + set, RTE_DIM(set))) > > > > + goto err; > > > > + end = lcores + 1 + offset; > > > > + } else /* ',' or '\0' */ > > > > + /* haven't given cpu_set, current loop done */ > > > > + end = lcores; > > > > + > > > > + if (*end != ',' && *end != '\0') > > > > + goto err; > > > > + > > > > + /* parse lcore_set from start point */ > > > > + if (0 > eal_parse_set(lcore_start, set, RTE_DIM(set))) > > > > + goto err; > > > > + > > > > + /* without '@', by default using lcore_set as cpu_set */ > > > > + if (*lcores != '@' && > > > > + 0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set))) > > > > + goto err; > > > > + > > > > + /* start to update lcore_set */ > > > > + for (idx = 0; idx < RTE_MAX_LCORE; idx++) { > > > > + if (!set[idx]) > > > > + continue; > > > > + > > > > + if (cfg->lcore_role[idx] != ROLE_RTE) { > > > > + lcore_config[idx].core_index = count; > > > > + cfg->lcore_role[idx] = ROLE_RTE; > > > > + count++; > > > > + } > > > > + rte_memcpy(&lcore_config[idx].cpuset, &cpuset, > > > > + sizeof(rte_cpuset_t)); > > > > + } > > > > + > > > > + lcores = end + 1; > > > > + } while (*end != '\0'); > > > > + > > > > + if (count == 0) > > > > + goto err; > > > > + > > > > + cfg->lcore_count = count; > > > > + lcores_parsed = 1; > > > > + ret = 0; > > > > + > > > > +err: > > > > + > > > > + return ret; > > > > +} > > > > + > > > > static int > > > > eal_parse_syslog(const char *facility, struct internal_config *conf) > > > > { > > > > @@ -489,6 +728,13 @@ eal_parse_common_option(int opt, const char > > *optarg, > > > > conf->log_level = log; > > > > break; > > > > } > > > > + case OPT_LCORES_NUM: > > > > + if (eal_parse_lcores(optarg) < 0) { > > > > + RTE_LOG(ERR, EAL, "invalid parameter for --" > > > > + OPT_LCORES "\n"); > > > > + return -1; > > > > + } > > > > + break; > > > > > > > > /* don't know what to do, leave this to caller */ > > > > default: > > > > @@ -527,7 +773,7 @@ eal_check_common_options(struct internal_config > > *internal_cfg) > > > > > > > > if (!lcores_parsed) { > > > > RTE_LOG(ERR, EAL, "CPU cores must be enabled with > > > > options " > > > > - "-c or -l\n"); > > > > + "-c, -l or --lcores\n"); > > > > return -1; > > > > } > > > > if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) { > > > > @@ -583,6 +829,14 @@ eal_common_usage(void) > > > > " The argument format is > <c1>[-c2][,c3[-c4],...]\n" > > > > " where c1, c2, etc are core indexes > > > > between > 0 and %d\n" > > > > " --"OPT_MASTER_LCORE" ID: Core ID that is used as > master\n" > > > > + " --"OPT_LCORES" MAP: maps between lcore_set to > > phys_cpu_set\n" > > > > + " The argument format is\n" > > > > + " > 'lcores[@cpus]<,lcores[@cpus],...>'\n" > > > > + " lcores and cpus list are grouped by > > > > '(' and > ')'\n" > > > > + " Within the group, '-' is used for range > separator,\n" > > > > + " ',' is used for single number > > > > separator.\n" > > > > + " '( )' can be omitted for single element > group, '@' \n" > > > > + " can be omitted if cpus and lcores has > > > > the > same value\n" > > > > " -n NUM : Number of memory channels\n" > > > > " -v : Display version information on > > > > startup\n" > > > > " -m MB : memory to allocate (see also -- > > "OPT_SOCKET_MEM")\n" > > > > diff --git a/lib/librte_eal/common/eal_options.h > > b/lib/librte_eal/common/eal_options.h > > > > index e476f8d..a1cc59f 100644 > > > > --- a/lib/librte_eal/common/eal_options.h > > > > +++ b/lib/librte_eal/common/eal_options.h > > > > @@ -77,6 +77,8 @@ enum { > > > > OPT_CREATE_UIO_DEV_NUM, > > > > #define OPT_VFIO_INTR "vfio-intr" > > > > OPT_VFIO_INTR_NUM, > > > > +#define OPT_LCORES "lcores" > > > > + OPT_LCORES_NUM, > > > > OPT_LONG_MAX_NUM > > > > }; > > > > > > > > diff --git a/lib/librte_eal/linuxapp/eal/Makefile > > b/lib/librte_eal/linuxapp/eal/Makefile > > > > index 0e9c447..025d836 100644 > > > > --- a/lib/librte_eal/linuxapp/eal/Makefile > > > > +++ b/lib/librte_eal/linuxapp/eal/Makefile > > > > @@ -95,6 +95,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE > > > > CFLAGS_eal_pci.o := -D_GNU_SOURCE > > > > CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE > > > > CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE > > > > +CFLAGS_eal_common_options.o := -D_GNU_SOURCE > > > > > > > > # workaround for a gcc bug with noreturn attribute > > > > # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 > > > > -- > > > > 1.8.1.4 > > > >
