From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <harry.van.haaren@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
by dpdk.org (Postfix) with ESMTP id 2E9E62986
for <dev@dpdk.org>; Fri, 5 May 2017 17:48:05 +0200 (CEST)
Received: from fmsmga004.fm.intel.com ([10.253.24.48])
by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
05 May 2017 08:48:05 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.38,293,1491289200"; d="scan'208";a="257394273"
Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75])
by fmsmga004.fm.intel.com with ESMTP; 05 May 2017 08:48:03 -0700
Received: from irsmsx102.ger.corp.intel.com ([169.254.2.153]) by
IRSMSX153.ger.corp.intel.com ([163.33.192.75]) with mapi id 14.03.0319.002;
Fri, 5 May 2017 16:48:02 +0100
From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "Richardson, Bruce"
<bruce.richardson@intel.com>, "hemant.agrawal@nxp.com"
<hemant.agrawal@nxp.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
"Vangati, Narender" <narender.vangati@intel.com>, "Eads, Gage"
<gage.eads@intel.com>
Thread-Topic: [RFC] service core concept header implementation
Thread-Index: AQHSxACR6paJvJqAzkOCT2rlM3T1nqHjqJkAgABa7wCAAd15MA==
Date: Fri, 5 May 2017 15:48:02 +0000
Message-ID: <E923DB57A917B54B9182A2E928D00FA612A3586E@IRSMSX102.ger.corp.intel.com>
References: <1493810961-139469-1-git-send-email-harry.van.haaren@intel.com>
<1493810961-139469-2-git-send-email-harry.van.haaren@intel.com>
<20170504063543.GA2794@jerin> <20170504120121.GA29312@jerin>
In-Reply-To: <20170504120121.GA29312@jerin>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMmM1MmE4MTQtMDZjZi00NTgzLTgzZDEtMWU4MmQ2OTY3MWFjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6InZSMTJwYm15UlBsYmFUUXVwMVB3WEx3Z1IxSnJQMitGY2ROYktYeU1lREU9In0=
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 10.0.102.7
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] [RFC] service core concept header implementation
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: Fri, 05 May 2017 15:48:06 -0000
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
<snip email headers>
> > Hi Harry,
> >
> > Overall it looks good. Some initial comments
Off to a good start! Replies inline, in general I think we're on the same p=
age. This RFC was pretty "fresh", mostly to check if the community agrees w=
ith the general concept of service-cores. The implementation details (altho=
ugh going well) will have some churn I expect :)
I'd like opinions from others in the community - I'll leave this thread "op=
en" for a while to get more feedback, and rework a v2 RFC then.
Thanks for your review - Harry
> > > This patch adds a service header to DPDK EAL. This header is
> > > an RFC for a mechanism to allow DPDK components request a
> > > callback function to be invoked.
> > >
> > > The application can set the number of service cores, and
> > > a coremask for each particular services. The implementation
> > > of this functionality in rte_services.c (not completed) would
> > > use atomics to ensure that a service can only be polled by a
> > > single lcore at a time.
> >
> > single lcore at a time "if multipthread_capable flag is not set". Right=
?
> >
> > >
> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > > ---
> > > lib/librte_eal/common/include/rte_service.h | 211 ++++++++++++++++++=
++++++++++
> > > 1 file changed, 211 insertions(+)
> > > create mode 100644 lib/librte_eal/common/include/rte_service.h
> > >
> > > diff --git a/lib/librte_eal/common/include/rte_service.h
> b/lib/librte_eal/common/include/rte_service.h
> > > new file mode 100644
> > > index 0000000..cfa26f3
> > > --- /dev/null
> > > +++ b/lib/librte_eal/common/include/rte_service.h
> > > @@ -0,0 +1,211 @@
> > > +/*
> > > + * BSD LICENSE
> > > + *
> > > + * Copyright(c) 2017 Intel Corporation. All rights reserved.
> > > + *
> > > + * Redistribution and use in source and binary forms, with or with=
out
> > > + * modification, are permitted provided that the following conditi=
ons
> > > + * are met:
> > > + *
> > > + * * Redistributions of source code must retain the above copyri=
ght
> > > + * notice, this list of conditions and the following disclaime=
r.
> > > + * * Redistributions in binary form must reproduce the above cop=
yright
> > > + * notice, this list of conditions and the following disclaime=
r in
> > > + * the documentation and/or other materials provided with the
> > > + * distribution.
> > > + * * Neither the name of Intel Corporation nor the names of its
> > > + * contributors may be used to endorse or promote products der=
ived
> > > + * from this software without specific prior written permissio=
n.
> > > + *
> > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBU=
TORS
> > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT N=
OT
> > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNE=
SS FOR
> > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPY=
RIGHT
> > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCID=
ENTAL,
> > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NO=
T
> > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS O=
F USE,
> > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND =
ON ANY
> > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR =
TORT
> > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF T=
HE USE
> > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DA=
MAGE.
> > > + */
> > > +
> > > +#ifndef _RTE_SERVICE_H_
> > > +#define _RTE_SERVICE_H_
> > > +
> > > +/**
> > > + * @file
> > > + *
> > > + * Service functions
> > > + *
> > > + * The service functionality provided by this header allows a DPDK c=
omponent
> > > + * to indicate that it requires a function call in order for it to p=
erform
> > > + * its processing.
> > > + *
> > > + * An example usage of this functionality would be a component that =
registers
> > > + * a service to perform a particular packet processing duty: for exa=
mple the
> > > + * eventdev software PMD. At startup the application requests all se=
rvices
> > > + * that have been registered, and the app decides how many cores wil=
l run the
> >
> > s/app/application
Noted
> > > + * required services. The EAL removes these number of cores from the=
available
> > > + * runtime cores, and dedicates them to performing service-core work=
loads. The
> > > + * application has access to the remaining lcores as normal.
> > > + *
> > > + * An example of the service core infrastructure with an application
> > > + * configuring a single service (the eventdev sw PMD), and dedicatin=
g one core
> > > + * exclusively to run the service would interact with the API as fol=
lows:
> > > + *
> > > + * 1. Eventdev SW PMD calls *rte_eal_service_register*, indicating t=
hat it
> > > + * requires an lcore to call a function in order to operate. EAL =
registers
> > > + * this service, but performs no other actions yet.
> > > + *
> > > + * 2. Application calls *rte_eal_service_get*, allowing EAL to provi=
de it
> > > + * with an array of *rte_service_config* structures. These struct=
ures
> > > + * provide the application with the name of the service, along wi=
th
> > > + * metadata provided by the service when it was registered.
> > > + *
> > > + * 3. The application logic iterates over the services that require =
running,
> > > + * and decides to run the eventdev sw PMD service using one lcore=
.
> > > + *
> > > + * 4. The application calls *rte_eal_service_use_lcore* multiple tim=
es, once
> > > + * for each lcore that should be used as a service core. These co=
res are
> > > + * removed from the application usage, and EAL will refuse to lau=
nch
> > > + * user-specified functions on these cores.
> > > + *
> > > + * 5. The application calls *rte_eal_service_set_coremask* to set th=
e coremask
> > > + * for the service. Note that EAL is already aware of ALL lcores =
that will
> > > + * be used for service-core purposes (see step 4 above) which all=
ows EAL to
> > > + * error-check the coremask here, and ensure at least one core is=
going to
> > > + * be running the service.
> >
> > Regarding step 4 and 5, It looks like,
> > a) The lcore_id information is duplicate in step 4 and 5.
Not really duplicated - keep in mind we do not want to have a 1:1 mapping, =
the goal of this is to allow e.g. 3 cores -> 5 services type mappings, wher=
e the work is shared between the cores. Hence, setting coremasks and settin=
g lcores to use are very related, but not quite the same.
> > b) On rte_eal_service_set_coremask() failure, you may the need
> > inverse of rte_eal_service_use_lcore()(setting back to worker/normal
> > lcore)
We can pass the "type" of lcore as an argument to a function:
#define RTE_EAL_SERVICE_TYPE_APPLICATION 0
#define RTE_EAL_SERVICE_TYPE_SERVICE 1
rte_eal_service_set_lcore_type(uint32_t type);
Allows adding more core types (if anybody can think of any) and avoids func=
tion-count bloat :)
> > But I understand the need for step 5.
> >
> > How about changing the (4) and (5) as
> >
> > step 4) rte_eal_service_set_coremask
> > step 5) rte_eal_service_apply(void)(or similar name) for error check an=
d
> > ensure at least on core is going to be running the service.
So the sequence would be:
for( i < app_n_service_cores )
rte_eal_service_set_lcore_type( SERVICE );
for( i < services_count )
rte_eal_serivce_set_coremask( app_decided_coremask_here );
int ret =3D rte_eal_service_apply();
if(ret) {
/* application can rte_panic(), or reset CPU cores to default APP state =
and continue */
}
/* application profits from service-cores feature in EAL */
> >
> > > + *
> > > + * 6. The application now calls remote_launch() as usual, and the ap=
plication
> > > + * can perform its processing as required without manually handli=
ng the
> > > + * partitioning of lcore resources for DPDK functionality.
> > > + */
> > > +
> > > +#ifdef __cplusplus
> > > +extern "C" {
> > > +#endif
> > > +
> > > +#include <stdint.h>
> > > +
> > > +#define RTE_SERVICE_NAMESIZE 32
> > > +
> > > +/**
> > > + * Signature of callback back function to run a service.
> > > + */
> > > +typedef void (*rte_eal_service_func)(void *args);
> > > +
> > > +struct rte_service_config {
> >
> > I think, better name is rte_service_spec or something similar
Sure, seems more representative.
> > > + /* name of the service */
> > > + char name[RTE_SERVICE_NAMESIZE];
> > > + /* cores that run this service */
> >
> > If I understand it correctly,
> > a) Its for NUMA
> > b) Basically advertising the core(s) which capable or preferred to run =
the
> > service. Not the number of cores "required" to run the service.
> > if so, update the comments
Yes NUMA needs to be taken into account.
> > > + uint64_t coremask;
> >
> > 64bits are not enough to represent the coremask. May be array of
> > uint64_t or array of int can be used. I think, latter is more inline
> > with exiting eal scheme
Yes aware of this when posting - but given its an RFC didn't address. Good =
point though :)
> Two more questions,
>=20
> 1) If its for only for NUMA. Is it socket_id mask enough ?
I'll work on a solution for v2 RFC, and investigate the EAL coremask implem=
entation for some ideas then.
> 2) What would be the tear down sequence of the service core especially wh=
en
> device stop or close happens?
Good question. Being able to "turn off" a single service while keeping othe=
r services running would be useful I think. That might require some extra t=
racking at the implementation layer, but perhaps something that's worth doi=
ng. Opinions and use-cases welcome here!