From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 12BF22FDD for ; Thu, 6 Jul 2017 16:47:23 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Jul 2017 07:47:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,317,1496127600"; d="scan'208";a="282970606" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga004.fm.intel.com with ESMTP; 06 Jul 2017 07:47:21 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.211]) by IRSMSX101.ger.corp.intel.com ([169.254.1.242]) with mapi id 14.03.0319.002; Thu, 6 Jul 2017 15:47:21 +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+VBa3oRWh06Lyx2A3RPi4KJDjzOAgANUrBA= Date: Thu, 6 Jul 2017 14:47:20 +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> In-Reply-To: <20170704124548.GB14921@jerin> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTY3N2JmNGQtM2NkYS00ZTFjLWFiMzUtNmJiN2FkZjM1ZjYwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkJHV2laRDY1UjdGbjRGVGZtVmNrYk1nckpZYUNlNGxoWlJWK3pZT0crb0U9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] 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: Thu, 06 Jul 2017 14:47:24 -0000 > 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 >=20 > -----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 Haaren > > > > 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) > > } >=20 > Missing the --help update for service coremask details. >=20 > I think, EAL arguments are documented in another area of doc directory > as well. Update the documents. Will double check / fix this. Replying here now to advance discussion below= ;=20 > > 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; >=20 > Why not to use rte_service_lcore_add(idx) here. So that in future some > 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 a= lmost anything 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 -> val= idate-> config -> run" workflow. 3) Allocate temp memory to store the service-core indexes, and later free t= hat back (feels hacky to me?) Current scheme of (1) makes the most sense to me. > 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 be > stolen by service core. But it looks like RTE_LCORE_FOREACH not honoring > previous rte_service_lcore_add() functions. >=20 > 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_config[= ] method above, then I can prioritize this and try get a patchset up ASAP.