From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 3B0711C01 for ; Fri, 13 Jul 2018 19:27:11 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Jul 2018 10:27:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,348,1526367600"; d="scan'208";a="64501991" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by FMSMGA003.fm.intel.com with ESMTP; 13 Jul 2018 10:27:04 -0700 Received: from irsmsx156.ger.corp.intel.com (10.108.20.68) by IRSMSX153.ger.corp.intel.com (163.33.192.75) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 13 Jul 2018 18:27:04 +0100 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.110]) by IRSMSX156.ger.corp.intel.com ([169.254.3.205]) with mapi id 14.03.0319.002; Fri, 13 Jul 2018 18:27:04 +0100 From: "Van Haaren, Harry" To: Thomas Monjalon CC: "dev@dpdk.org" , "Varghese, Vipin" Thread-Topic: [dpdk-dev] [PATCH v2] eal/service: improve error checking of coremasks Thread-Index: AQHT7GVLmt36K+Cmn0iTRnIU1vM+DqQ55kYAgFGV8QCAAj4XAA== Date: Fri, 13 Jul 2018 17:27:03 +0000 Message-ID: References: <1526395965-133647-1-git-send-email-harry.van.haaren@intel.com> <1526399782-156061-1-git-send-email-harry.van.haaren@intel.com> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F7A83@BGSMSX101.gar.corp.intel.com> <30628394.ucWzCd8NzF@xps> In-Reply-To: <30628394.ucWzCd8NzF@xps> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODk1MjdiMDEtN2E3YS00MDRkLTkyYWEtYjI3Y2M2ZGQyZjIxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNExSU3FSTTMwd3RtQmd2SlpzaFNUNmp0TkhVeXhDZERuamJBRE91XC9IalJMajN6dXFZS0hYV1JzYStNSmpUYVUifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 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 v2] eal/service: improve error checking of coremasks 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, 13 Jul 2018 17:27:13 -0000 Hi Thomas, I've sent a v3 for this patch. Twice actually, including dev@dpdk.org the 2= nd time :) Thanks for the ping, -Harry > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Thursday, July 12, 2018 8:35 AM > To: Van Haaren, Harry > Cc: dev@dpdk.org; Varghese, Vipin > Subject: Re: [dpdk-dev] [PATCH v2] eal/service: improve error checking of > coremasks >=20 > Hi Harry, >=20 > What is the status of this patch? >=20 >=20 > 21/05/2018 11:41, Varghese, Vipin: > > Hi Harry, > > > > This look ok to me, except for one warning rewrite else its ACK from my > end. > > > > > -----Original Message----- > > > From: Van Haaren, Harry > > > Sent: Tuesday, May 15, 2018 9:26 PM > > > To: dev@dpdk.org > > > Cc: Van Haaren, Harry ; thomas@monjalon.n= et; > > > Varghese, Vipin > > > Subject: [PATCH v2] eal/service: improve error checking of coremasks > > > > > > This commit improves the error checking performed on the core masks (= or > lists) > > > of the service cores, in particular with respect to the data-plane (R= TE) > cores of > > > DPDK. > > > > > > With this commit, invalid configurations are detected at runtime, and > warning > > > messages are printed to inform the user. > > > > > > For example specifying the coremask as 0xf, and the service coremask = as > 0xff00 > > > is invalid as not all service-cores are contained within the coremask= . A > warning is > > > now printed to inform the user. > > > > > > Reported-by: Vipin Varghese > > > Signed-off-by: Harry van Haaren > > > > > > --- > > > > > > v2, thanks for review: > > > - Consistency in message endings - vs . (Thomas) > > > - Wrap lines as they're very long otherwise (Thomas) > > > > > > Cc: thomas@monjalon.net > > > Cc: vipin.varghese@intel.com > > > > > > @Thomas, please consider this patch for RC4, it adds checks and print= s > > > warnings, better usability, no functional changes. > > > --- > > > lib/librte_eal/common/eal_common_options.c | 43 > > > ++++++++++++++++++++++++++++++ > > > 1 file changed, 43 insertions(+) > > > > > > diff --git a/lib/librte_eal/common/eal_common_options.c > > > b/lib/librte_eal/common/eal_common_options.c > > > index ecebb29..9f3a484 100644 > > > --- a/lib/librte_eal/common/eal_common_options.c > > > +++ b/lib/librte_eal/common/eal_common_options.c > > > @@ -315,6 +315,7 @@ eal_parse_service_coremask(const char *coremask) > > > unsigned int count =3D 0; > > > char c; > > > int val; > > > + uint32_t taken_lcore_count =3D 0; > > > > > > if (coremask =3D=3D NULL) > > > return -1; > > > @@ -358,6 +359,10 @@ eal_parse_service_coremask(const char *coremask) > > > "lcore %u unavailable\n", idx); > > > return -1; > > > } > > > + > > > + if (cfg->lcore_role[idx] =3D=3D ROLE_RTE) > > > + taken_lcore_count++; > > > + > > > lcore_config[idx].core_role =3D ROLE_SERVICE; > > > count++; > > > } > > > @@ -374,11 +379,28 @@ eal_parse_service_coremask(const char *coremask= ) > > > if (count =3D=3D 0) > > > return -1; > > > > > > + if (core_parsed && taken_lcore_count !=3D count) { > > > + RTE_LOG(ERR, EAL, > > > + "Warning: not all service cores were in the coremask. " > > > + "Please ensure -c or -l includes service cores\n"); > > > > Current execution will throw warning message as 'Warning: not all servi= ce > cores were in the coremask. Please ensure -c or -l includes service cores= '. > > > > 1) Should we re-write this with ' RTE_LOG(WARN, EAL,' and removing > 'Warning: ' > > 2) Warning message as "service cores not in data plane core mask ". > > 3) If we share information "Please ensure -c or -l includes service > cores\n" is not it expected to rte_panic? So should we remove this line? > > > > > + } > > > + > > > cfg->service_lcore_count =3D count; > > > return 0; > > > } > > > > > > static int > > > +eal_service_cores_parsed(void) > > > +{ > > > + int idx; > > > + for (idx =3D 0; idx < RTE_MAX_LCORE; idx++) { > > > + if (lcore_config[idx].core_role =3D=3D ROLE_SERVICE) > > > + return 1; > > > + } > > > + return 0; > > > +} > > > + > > > +static int > > > eal_parse_coremask(const char *coremask) { > > > struct rte_config *cfg =3D rte_eal_get_configuration(); @@ -387,6 > > > +409,11 @@ eal_parse_coremask(const char *coremask) > > > char c; > > > int val; > > > > > > + if (eal_service_cores_parsed()) > > > + RTE_LOG(ERR, EAL, > > > + "Warning: Service cores parsed before dataplane cores. > > > " > > > + "Please ensure -c is before -s or -S.\n"); > > > + > > > if (coremask =3D=3D NULL) > > > return -1; > > > /* Remove all blank characters ahead and after . > > > @@ -418,6 +445,7 @@ eal_parse_coremask(const char *coremask) > > > "unavailable\n", idx); > > > return -1; > > > } > > > + > > > cfg->lcore_role[idx] =3D ROLE_RTE; > > > lcore_config[idx].core_index =3D count; > > > count++; > > > @@ -449,6 +477,7 @@ eal_parse_service_corelist(const char *corelist) > > > unsigned count =3D 0; > > > char *end =3D NULL; > > > int min, max; > > > + uint32_t taken_lcore_count =3D 0; > > > > > > if (corelist =3D=3D NULL) > > > return -1; > > > @@ -490,6 +519,9 @@ eal_parse_service_corelist(const char *corelist) > > > idx); > > > return -1; > > > } > > > + if (cfg->lcore_role[idx] =3D=3D ROLE_RTE) > > > + taken_lcore_count++; > > > + > > > lcore_config[idx].core_role =3D > > > ROLE_SERVICE; > > > count++; > > > @@ -504,6 +536,12 @@ eal_parse_service_corelist(const char *corelist) > > > if (count =3D=3D 0) > > > return -1; > > > > > > + if (core_parsed && taken_lcore_count !=3D count) { > > > + RTE_LOG(ERR, EAL, > > > + "Warning: not all service cores were in the coremask. " > > > + "Please ensure -c or -l includes service cores\n"); > > > + } > > > + > > > return 0; > > > } > > > > > > @@ -516,6 +554,11 @@ eal_parse_corelist(const char *corelist) > > > char *end =3D NULL; > > > int min, max; > > > > > > + if (eal_service_cores_parsed()) > > > + RTE_LOG(ERR, EAL, > > > + "Warning: Service cores parsed before dataplane cores. > > > " > > > + "Please ensure -l is before -s or -S.\n"); > > > + > > > if (corelist =3D=3D NULL) > > > return -1; > > > > > > -- > > > 2.7.4 > > >=20 >=20 >=20 >=20