From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <harry.van.haaren@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id 8E4DE1B53
 for <dev@dpdk.org>; Mon, 28 Aug 2017 15:49:42 +0200 (CEST)
Received: from orsmga005.jf.intel.com ([10.7.209.41])
 by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 28 Aug 2017 06:49:40 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.41,441,1498546800"; d="scan'208";a="142753302"
Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157])
 by orsmga005.jf.intel.com with ESMTP; 28 Aug 2017 06:49:39 -0700
Received: from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by
 IRSMSX103.ger.corp.intel.com ([169.254.3.49]) with mapi id 14.03.0319.002;
 Mon, 28 Aug 2017 14:49:38 +0100
From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@caviumnetworks.com>
CC: "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH] eal: added new `rte_lcore_is_service_lcore`
 API.
Thread-Index: AQHTHCIRejLk4K1hBEGkGqXEqpO/LKKZoOZA///444CAADSUsA==
Date: Mon, 28 Aug 2017 13:49:37 +0000
Message-ID: <E923DB57A917B54B9182A2E928D00FA640C6D354@IRSMSX102.ger.corp.intel.com>
References: <1503501027-11046-1-git-send-email-pbhagavatula@caviumnetworks.com>
 <E923DB57A917B54B9182A2E928D00FA640C6C15D@IRSMSX102.ger.corp.intel.com>
 <20170828113302.GA22141@PBHAGAVATULA-LT>
In-Reply-To: <20170828113302.GA22141@PBHAGAVATULA-LT>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiY2YyOGExYzktZTNlMy00YTY3LTkzNmEtZDY5MzU5ZTIxZGJiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IlNGRW1YaVp5MHEzd01ZU2JPb2FSUDRHWGxmRDlXdjRBMGZiQ0ZROHFZUzg9In0=
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
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] eal: added new `rte_lcore_is_service_lcore`
 API.
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 28 Aug 2017 13:49:44 -0000

> From: Pavan Nikhilesh Bhagavatula [mailto:pbhagavatula@caviumnetworks.com=
]
> Sent: Monday, August 28, 2017 12:33 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] eal: added new `rte_lcore_is_service_lcor=
e` API.
>=20
> On Mon, Aug 28, 2017 at 10:59:51AM +0000, Van Haaren, Harry wrote:
> > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > > Sent: Wednesday, August 23, 2017 4:10 PM
> > > To: dev@dpdk.org
> > > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Pavan Nikhilesh
> > > <pbhagavatula@caviumnetworks.com>
> > > Subject: [dpdk-dev] [PATCH] eal: added new `rte_lcore_is_service_lcor=
e` API.
> > >
> > > This API can be used to test if an lcore(EAL thread) is a service lco=
re.
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > ---
> > >  lib/librte_eal/common/include/rte_lcore.h | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > >
> > > diff --git a/lib/librte_eal/common/include/rte_lcore.h
> > > b/lib/librte_eal/common/include/rte_lcore.h
> > > index 50e0d0f..7854ea1 100644
> > > --- a/lib/librte_eal/common/include/rte_lcore.h
> > > +++ b/lib/librte_eal/common/include/rte_lcore.h
> > > @@ -180,6 +180,24 @@ rte_lcore_is_enabled(unsigned lcore_id)
> > >  }
> > >
> > >  /**
> > > + * Test if an lcore is service lcore.
> > > + *
> > > + * @param lcore_id
> > > + *   The identifier of the lcore, which MUST be between 0 and
> > > + *   RTE_MAX_LCORE-1.
> > > + * @return
> > > + *   True if the given lcore is service; false otherwise.
> > > + */
> > > +static inline int
> > > +rte_lcore_is_service_lcore(unsigned lcore_id)
> > > +{
> > > +	struct rte_config *cfg =3D rte_eal_get_configuration();
> > > +	if (lcore_id >=3D RTE_MAX_LCORE)
> > > +		return 0;
> > > +	return cfg->lcore_role[lcore_id] =3D=3D ROLE_SERVICE;
> > > +}
> >
> > No header file and Static inline - so this is only to be used internall=
y in the service
> cores library?
> > If so, the function should actually be used, instead of only added but =
not used in the
> library itself.
> >
>=20
> The enum rte_lcore_role_t has ROLE_SERVICE which tells that a particular =
lcore is
> a service lcore as well as an EAL thread some libraries such as rte_timer=
 allow
> specific operations only over EAL threads.

Understood that role of cores is important, and that rte_timer might requir=
e this information.


> The rte_timer lib uses the rte_is_lcore_enabled() call to check if a lcor=
e is an
> EAL thread, Which checks if the lcore role is  ROLE_RTE. But it should al=
so
> allow timers to be registered on a service core as processing those timer=
s can
> be done on them.

No problem from me here either - although it's the Timers library maintaine=
r that should check this.


> This new function allows such libraries to check if the role is
> ROLE_SERVICE and allow those operations.

If the timers library requires information about service-cores, it should u=
se a public API to retrieve that information. Having "internal" functions b=
etween libraries is not nice.

I think a better design would be to add this function as a public function,=
 (add it to the .map files etc) and then call the public function from the =
timers library.

Does that sound like a good solution? -Harry


> Currently, the only rte_timer library has this specific role check. The
> following patch shows the usage in rte_timer library.
>=20
> http://dpdk.org/dev/patchwork/patch/27819/
>=20
> > Or am I mis-understanding the intent?
> >
> > -Harry
>=20
> Thanks,
> Pavan.