From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 6DF6AAAD3 for ; Mon, 21 May 2018 11:41:44 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 May 2018 02:41:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,426,1520924400"; d="scan'208";a="225905523" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga005.jf.intel.com with ESMTP; 21 May 2018 02:41:43 -0700 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 21 May 2018 02:41:42 -0700 Received: from bgsmsx151.gar.corp.intel.com (10.224.48.42) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 21 May 2018 02:41:42 -0700 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.244]) by BGSMSX151.gar.corp.intel.com ([169.254.3.7]) with mapi id 14.03.0319.002; Mon, 21 May 2018 15:11:38 +0530 From: "Varghese, Vipin" To: "Van Haaren, Harry" , "dev@dpdk.org" CC: "thomas@monjalon.net" Thread-Topic: [PATCH v2] eal/service: improve error checking of coremasks Thread-Index: AQHT7GVLq1jGOtIzDEK6wFxr8yx4ZqQ59aqg Date: Mon, 21 May 2018 09:41:38 +0000 Message-ID: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F7A83@BGSMSX101.gar.corp.intel.com> References: <1526395965-133647-1-git-send-email-harry.van.haaren@intel.com> <1526399782-156061-1-git-send-email-harry.van.haaren@intel.com> In-Reply-To: <1526399782-156061-1-git-send-email-harry.van.haaren@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzAzZjRjOTMtYzBmMy00OGViLTkxODgtMDcyMzQ2ODI4OTRhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNG9jYW1OVzcwVDdDODY1SnV4UkNXYXNcL3phem93VkxDU3pZRnhVemV0VUF0K1Q1VFNUdUltV1E3RWZIM3A3Z3gifQ== dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [10.223.10.10] 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: Mon, 21 May 2018 09:41:44 -0000 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.net; > Varghese, Vipin > Subject: [PATCH v2] eal/service: improve error checking of coremasks >=20 > This commit improves the error checking performed on the core masks (or l= ists) > of the service cores, in particular with respect to the data-plane (RTE) = cores of > DPDK. >=20 > With this commit, invalid configurations are detected at runtime, and war= ning > messages are printed to inform the user. >=20 > For example specifying the coremask as 0xf, and the service coremask as 0= xff00 > is invalid as not all service-cores are contained within the coremask. A = warning is > now printed to inform the user. >=20 > Reported-by: Vipin Varghese > Signed-off-by: Harry van Haaren >=20 > --- >=20 > v2, thanks for review: > - Consistency in message endings - vs . (Thomas) > - Wrap lines as they're very long otherwise (Thomas) >=20 > Cc: thomas@monjalon.net > Cc: vipin.varghese@intel.com >=20 > @Thomas, please consider this patch for RC4, it adds checks and prints > warnings, better usability, no functional changes. > --- > lib/librte_eal/common/eal_common_options.c | 43 > ++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) >=20 > 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; >=20 > 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; >=20 > + 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 service c= ores were in the coremask. Please ensure -c or -l includes service cores'.= =20 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; > } >=20 > 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; >=20 > + 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; >=20 > 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; >=20 > + 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; > } >=20 > @@ -516,6 +554,11 @@ eal_parse_corelist(const char *corelist) > char *end =3D NULL; > int min, max; >=20 > + 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; >=20 > -- > 2.7.4