From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 6922E1B432 for ; Thu, 12 Jul 2018 09:35:23 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 42E7121B4C; Thu, 12 Jul 2018 03:35:22 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 12 Jul 2018 03:35:22 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=vpUIzikCv3YwRKw3QS1od9QfLG u67d8SL88AIuIPMps=; b=AHGrFxDnVV0NqZj6iZfo/R7IWVeLAzOb7tDtODyjpf fsW6PBEWYO4sAhXFkKnA3KKQg4UjHrgvUveTb3tFTcbajHE+ZV36OJ1BzRaYV/QT 5KkmqV4fdwmYp2o/RPvy23lv9fXrJpIN6o6Ieh9r6LSCTl9jgdvYn8ccfsRStYzJ s= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=vpUIzi kCv3YwRKw3QS1od9QfLGu67d8SL88AIuIPMps=; b=Ze6YM+ZIF8ACLuYu0o3/kX xa2yjpwP5pAIyXby5cqx+H49avw+Dq+vHQW/Tb4DACgPSmzE+ZcRVBwfGd82jqYG t9zLLaQgYrHv2VHNrCO6q0pPGxTwvYZ/CXvwxTAat56x1DamBtAFoLFs4i7vz5QO /DT8TCbhLGlrBMKsL5TSgdJ+NiPjfeYbaxCKq2vI5axoqoV14r3IWQaj7xMd3FU4 10a1w61Ap7IKMgWwReGpFHhuKwDwYsHEBygV5AyHDHclcGJWvbE50QplSNmQ6zZn YdTBYCkjUdJbqPlor3yOR9m2mj8lg2I9h0+m2QeRPn1yVOXViQxcjOYmZ6zmBS6w == X-ME-Proxy: X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 77DB41027D; Thu, 12 Jul 2018 03:35:21 -0400 (EDT) From: Thomas Monjalon To: "Van Haaren, Harry" Cc: dev@dpdk.org, "Varghese, Vipin" Date: Thu, 12 Jul 2018 09:35:20 +0200 Message-ID: <30628394.ucWzCd8NzF@xps> In-Reply-To: <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> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F7A83@BGSMSX101.gar.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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: Thu, 12 Jul 2018 07:35:23 -0000 Hi Harry, What is the status of this patch? 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.net; > > 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 (RTE) 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 prints > > 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 = 0; > > char c; > > int val; > > + uint32_t taken_lcore_count = 0; > > > > if (coremask == 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] == ROLE_RTE) > > + taken_lcore_count++; > > + > > lcore_config[idx].core_role = ROLE_SERVICE; > > count++; > > } > > @@ -374,11 +379,28 @@ eal_parse_service_coremask(const char *coremask) > > if (count == 0) > > return -1; > > > > + if (core_parsed && taken_lcore_count != 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 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 = count; > > return 0; > > } > > > > static int > > +eal_service_cores_parsed(void) > > +{ > > + int idx; > > + for (idx = 0; idx < RTE_MAX_LCORE; idx++) { > > + if (lcore_config[idx].core_role == ROLE_SERVICE) > > + return 1; > > + } > > + return 0; > > +} > > + > > +static int > > eal_parse_coremask(const char *coremask) { > > struct rte_config *cfg = 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 == 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] = ROLE_RTE; > > lcore_config[idx].core_index = count; > > count++; > > @@ -449,6 +477,7 @@ eal_parse_service_corelist(const char *corelist) > > unsigned count = 0; > > char *end = NULL; > > int min, max; > > + uint32_t taken_lcore_count = 0; > > > > if (corelist == NULL) > > return -1; > > @@ -490,6 +519,9 @@ eal_parse_service_corelist(const char *corelist) > > idx); > > return -1; > > } > > + if (cfg->lcore_role[idx] == ROLE_RTE) > > + taken_lcore_count++; > > + > > lcore_config[idx].core_role = > > ROLE_SERVICE; > > count++; > > @@ -504,6 +536,12 @@ eal_parse_service_corelist(const char *corelist) > > if (count == 0) > > return -1; > > > > + if (core_parsed && taken_lcore_count != 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 = 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 == NULL) > > return -1; > > > > -- > > 2.7.4 >