From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com
[66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 6922E1B432
for <dev@dpdk.org>; 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: <xmx:ugRHWy1omHlOnUXI53hYYsOSJcF3_aBIoEYLCqPpkE74ydxNVjiDmA>
<xmx:ugRHWxDc__Pc1CLdWiwFjLyZP5bfh0xwOzkI770phdM8BR8m8HLAjQ>
<xmx:ugRHW2uCI7n1QSorf4lnqmJBkuH3Ci6TXSt6n9gX7nMPUoCUWg9c2w>
<xmx:ugRHW7wJO6IXHTU0A2Cp1_49hFVZBcCzKCc7MGXjSCzgXixqE0psWQ>
<xmx:ugRHW9Mt_gg4qamwPqkSubLS0B4HWI6GqngMJox17qdeX22J-erUnA>
<xmx:ugRHW5w5w5k8GKddseke7GTnFpxF3OChHmZEfWCEjzfs3tjN-FyO3w>
X-ME-Sender: <xms:uQRHWyZmLrX9vRG60kdGUGvPDAEGpcjvKdtSqEA-vnPWvYk3lqIzDw>
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 <thomas@monjalon.net>
To: "Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: dev@dpdk.org, "Varghese, Vipin" <vipin.varghese@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <harry.van.haaren@intel.com>; thomas@monjalon.net;
> > Varghese, Vipin <vipin.varghese@intel.com>
> > 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 <vipin.varghese@intel.com>
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > ---
> >
> > 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
>