From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 1BE9E271 for ; Fri, 7 Jul 2017 12:57:36 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Jul 2017 03:57:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,322,1496127600"; d="scan'208";a="876375783" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by FMSMGA003.fm.intel.com with ESMTP; 07 Jul 2017 03:57:35 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.211]) by IRSMSX106.ger.corp.intel.com ([169.254.8.236]) with mapi id 14.03.0319.002; Fri, 7 Jul 2017 11:57:34 +0100 From: "Van Haaren, Harry" To: Jerin Jacob CC: "dev@dpdk.org" , "thomas@monjalon.net" , "Wiles, Keith" , "Richardson, Bruce" Thread-Topic: [PATCH v3 3/7] service cores: coremask parsing Thread-Index: AQHS83sg+VBa3oRWh06Lyx2A3RPi4KJDjzOAgANUrBCAAUC0AIAAE1zw Date: Fri, 7 Jul 2017 10:57:34 +0000 Message-ID: References: <1498735421-100164-1-git-send-email-harry.van.haaren@intel.com> <1499031314-7172-1-git-send-email-harry.van.haaren@intel.com> <1499031314-7172-4-git-send-email-harry.van.haaren@intel.com> <20170704124548.GB14921@jerin> <20170707104529.GA24514@jerin> In-Reply-To: <20170707104529.GA24514@jerin> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTQ0N2RhZTUtNDI4Yy00ZjQ1LTlhZDgtMzkzOWUzNTI4YWY2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IklLNWc5OU5pRGx5T0JTWDg2Q0pXUVJEdzFaQWh0SitieXBoYUZQNWpESWM9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 3/7] service cores: coremask parsing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Jul 2017 10:57:37 -0000 > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Friday, July 7, 2017 11:46 AM > To: Van Haaren, Harry > Cc: dev@dpdk.org; thomas@monjalon.net; Wiles, Keith ; Richardson, > Bruce > Subject: Re: [PATCH v3 3/7] service cores: coremask parsing >=20 > -----Original Message----- > > Date: Thu, 6 Jul 2017 14:47:20 +0000 > > From: "Van Haaren, Harry" > > To: Jerin Jacob > > CC: "dev@dpdk.org" , "thomas@monjalon.net" > > , "Wiles, Keith" , > > "Richardson, Bruce" > > Subject: RE: [PATCH v3 3/7] service cores: coremask parsing > > > > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > > Sent: Tuesday, July 4, 2017 1:46 PM > > > To: Van Haaren, Harry > > > Cc: dev@dpdk.org; thomas@monjalon.net; Wiles, Keith ; > Richardson, > > > Bruce > > > Subject: Re: [PATCH v3 3/7] service cores: coremask parsing > > > > > > -----Original Message----- > > > > Date: Sun, 2 Jul 2017 22:35:10 +0100 > > > > From: Harry van Haaren > > > > To: dev@dpdk.org > > > > CC: jerin.jacob@caviumnetworks.com, thomas@monjalon.net, > > > > keith.wiles@intel.com, bruce.richardson@intel.com, Harry van Haare= n > > > > > > > > Subject: [PATCH v3 3/7] service cores: coremask parsing > > > > X-Mailer: git-send-email 2.7.4 > > > > > > > > Add logic for parsing a coremask from EAL, which allows > > > > the application to be unaware of the cores being taken from > > > > its coremask. > > > > > > > > Signed-off-by: Harry van Haaren > > > > > > > > diff --git a/lib/librte_eal/common/eal_common_options.c > > > b/lib/librte_eal/common/eal_common_options.c > > > > index f470195..cee200c 100644 > > > > --- a/lib/librte_eal/common/eal_common_options.c > > > > +++ b/lib/librte_eal/common/eal_common_options.c > > > > @@ -61,6 +61,7 @@ const char > > > > eal_short_options[] =3D > > > > "b:" /* pci-blacklist */ > > > > "c:" /* coremask */ > > > > + "s:" /* service coremask */ > > > > "d:" /* driver */ > > > > "h" /* help */ > > > > "l:" /* corelist */ > > > > @@ -267,6 +268,73 @@ static int xdigit2val(unsigned char c) > > > > } > > > > > > Missing the --help update for service coremask details. > > > > > > I think, EAL arguments are documented in another area of doc director= y > > > as well. Update the documents. > > > > Will double check / fix this. Replying here now to advance discussion b= elow; > > > > > > static int > > > > +eal_parse_service_coremask(const char *coremask) > > > > +{ > > > > + struct rte_config *cfg =3D rte_eal_get_configuration(); > > > > + int i, j, idx =3D 0; > > > > + unsigned int count =3D 0; > > > > + char c; > > > > + int val; > > > > + > > > > + if (coremask =3D=3D NULL) > > > > + return -1; > > > > + /* Remove all blank characters ahead and after . > > > > + * Remove 0x/0X if exists. > > > > + */ > > > > + while (isblank(*coremask)) > > > > + coremask++; > > > > + if (coremask[0] =3D=3D '0' && ((coremask[1] =3D=3D 'x') > > > > + || (coremask[1] =3D=3D 'X'))) > > > > + coremask +=3D 2; > > > > + i =3D strlen(coremask); > > > > + while ((i > 0) && isblank(coremask[i - 1])) > > > > + i--; > > > > + > > > > + if (i =3D=3D 0) > > > > + return -1; > > > > + > > > > + for (i =3D i - 1; i >=3D 0 && idx < RTE_MAX_LCORE; i--) { > > > > + c =3D coremask[i]; > > > > + if (isxdigit(c) =3D=3D 0) { > > > > + /* invalid characters */ > > > > + return -1; > > > > + } > > > > + val =3D xdigit2val(c); > > > > + for (j =3D 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE; > > > > + j++, idx++) { > > > > + if ((1 << j) & val) { > > > > + /* handle master lcore already parsed */ > > > > + uint32_t lcore =3D idx; > > > > + if (master_lcore_parsed && > > > > + cfg->master_lcore =3D=3D lcore) > > > > + continue; > > > > + > > > > + if (!lcore_config[idx].detected) { > > > > + RTE_LOG(ERR, EAL, > > > > + "lcore %u unavailable\n", idx); > > > > + return -1; > > > > + } > > > > + lcore_config[idx].core_role =3D ROLE_SERVICE; > > > > > > Why not to use rte_service_lcore_add(idx) here. So that in future som= e > > > changes we don't need to touch this file. > > > > The issue here is that the hugepages memory that service-cores requires= is not available > at this point. Keep in mind, the EAL parse-opts runs before almost anythi= ng else (makes > sense, given we can specify e.g. --no-huge). > > > > Given that there is not rte_malloc() available at this point, we have a= few options: > > 1) Use existing allocated mem, e.g. the lcore_config[] array as above. > > 2) Delay the parsing of service-core mask until later. Breaks "parse ->= validate-> > config -> run" workflow. > > 3) Allocate temp memory to store the service-core indexes, and later fr= ee that back > (feels hacky to me?) > > > > Current scheme of (1) makes the most sense to me. >=20 > Yes. Make sense to keep option 1. Great, > One suggestion: > There is a lot duplicate code between new eal_parse_service_coremask() an= d > eal_parse_coremask() on the same file. I think, we can add a common parsi= ng logic > and on match, the actions can be invoked through function pointer which > is passed in the parsing function. Agreed - lets leave this cleanup of EAL to another patchset. That patchset = can consolidate the various coremask functions in EAL and other parts of DP= DK (I recall a similar comment for eventdev_pipeline sample-app). > > > I added following code in unit testcase and I have 8 cores system. So= I > > > was expecting cores prints from "0 3 4 5 6 7" as lcore 1 and 2 will b= e > > > stolen by service core. But it looks like RTE_LCORE_FOREACH not honor= ing > > > previous rte_service_lcore_add() functions. > > > > > > testsuite_setup(void) > > > { > > > + int i; > > > + rte_service_lcore_add(1); > > > + rte_service_lcore_add(2); > > > + > > > + RTE_LCORE_FOREACH(i) > > > + printf("cores %d\n", i); > > > > > > Root cause found - and fixed. If you don't strongly object to lcore_con= fig[] method > above, then I can prioritize this and try get a patchset up ASAP. >=20 > Great!! I'm working on improving one of the unit tests to launch app functions to v= erify ROLE_RTE, and will post then. Thanks for the input, -Harry