From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 2E9E62986 for ; 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" To: Jerin Jacob CC: "dev@dpdk.org" , "Richardson, Bruce" , "hemant.agrawal@nxp.com" , "nipun.gupta@nxp.com" , "Vangati, Narender" , "Eads, Gage" Thread-Topic: [RFC] service core concept header implementation Thread-Index: AQHSxACR6paJvJqAzkOCT2rlM3T1nqHjqJkAgABa7wCAAd15MA== Date: Fri, 5 May 2017 15:48:02 +0000 Message-ID: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 May 2017 15:48:06 -0000 > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > 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 > > > --- > > > 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 > > > + > > > +#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!