From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 757852C2F for ; Wed, 17 May 2017 15:49:18 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 May 2017 06:49:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,354,1491289200"; d="scan'208";a="262701742" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by fmsmga004.fm.intel.com with ESMTP; 17 May 2017 06:49:16 -0700 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.12]) by IRSMSX108.ger.corp.intel.com ([169.254.11.239]) with mapi id 14.03.0319.002; Wed, 17 May 2017 14:47:16 +0100 From: "Ananyev, Konstantin" To: "Richardson, Bruce" CC: "Van Haaren, Harry" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [RFC] service core concept header implementation Thread-Index: AQHSxADBGAiAhtsYeEGWzMy/UpgPLaH4e72ggAAF8oCAABxDsA== Date: Wed, 17 May 2017 13:47:15 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583FAF76D7@IRSMSX109.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> <2601191342CEEE43887BDE71AB9772583FAF750C@IRSMSX109.ger.corp.intel.com> <20170517125806.GA14804@bricha3-MOBL3.ger.corp.intel.com> In-Reply-To: <20170517125806.GA14804@bricha3-MOBL3.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.180] 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: Wed, 17 May 2017 13:49:19 -0000 Hi Bruce, > > >=20 > Hi Konstantin, >=20 > Thanks for the feedback, the specific detail of which I'll perhaps leave > for Harry to reply to or include in a later version of this patchset. > However, from a higher level, I think the big difference in what we > envisage compared to what you suggest in your comments is that these are > not services set up by the application. If the app wants to run > something it uses remote_launch as now. This is instead for other > components to request threads - or a share of a thread - for their own > use, since they cannot call remote_launch directly, as the app owns the > threads, not the individual libraries. Ok, thanks for clarification about who supposed to be the main consumer for= that. That makes sense. Though I am not sure why the API suggested wouldn't fit your purposes.=20 Though I think both PMD/core libraries and app layer might be interested i= n that: i.e. app might also have some background processing tasks, that might need to be= run on several cores (or core slices). Konstantin > See also comments made in reply to Thomas mail. >=20 > Hope this helps clarify things a little. >=20 > /Bruce >=20 > > > > > > 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. > > > > > > 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 > > > > > > > ... > > > > > + > > > +/** > > > + * Signature of callback back function to run a service. > > > + */ > > > +typedef void (*rte_eal_service_func)(void *args); > > > + > > > +struct rte_service_config { > > > + /* name of the service */ > > > + char name[RTE_SERVICE_NAMESIZE]; > > > + /* cores that run this service */ > > > + uint64_t coremask; > > > > Why uint64_t? > > Even now by default we support more than 64 cores. > > We have rte_cpuset_t for such things. > > > > Ok, the first main question: > > Is that possible to register multiple services with intersecting corema= sks or not? > > If not, then I suppose there is no practical difference from what we ha= ve right now > > with eal_remote_launch() . > > If yes, then several questions arise: > > a) How the service scheduling function will going to switch from one= service to another > > on particular core? > > As we don't have interrupt framework for that, I suppose the onl= y choice is to rely > > on service voluntary fiving up cpu. Is that so? > > b) Would it be possible to specify percentage of core cycles that serv= ice can consume? > > I suppose the answer is 'no', at least for current iteration. > > > > > + /* when set, multiple lcores can run this service simultaneously wi= thout > > > + * the need for software atomics to ensure that two cores do not > > > + * attempt to run the service at the same time. > > > + */ > > > + uint8_t multithread_capable; > > > > Ok, and what would happen if this flag is not set, and user specified m= ore > > then one cpu in the coremask? > > > > > +}; > > > + > > > +/** @internal - this will not be visible to application, defined in = a seperate > > > + * rte_eal_service_impl.h header. > > > > Why not? > > If the application registers the service, it for sure needs to know wh= at exactly that service > > would do (cb func and data). > > > > > The application does not need to be exposed > > > + * to the actual function pointers - so hide them. */ > > > +struct rte_service { > > > + /* callback to be called to run the service */ > > > + rte_eal_service_func cb; > > > + /* args to the callback function */ > > > + void *cb_args; > > > > If user plans to run that service on multiple cores, then he might > > need a separate instance of cb_args for each core. > > > > > + /* configuration of the service */ > > > + struct rte_service_config config; > > > +}; > > > + > > > +/** > > > + * @internal - Only DPDK internal components request "service" cores= . > > > + * > > > + * Registers a service in EAL. > > > + * > > > + * Registered services' configurations are exposed to an application= using > > > + * *rte_eal_service_get_all*. These services have names which can be= understood > > > + * by the application, and the application logic can decide how to a= llocate > > > + * cores to run the various services. > > > + * > > > + * This function is expected to be called by a DPDK component to ind= icate that > > > + * it require a CPU to run a specific function in order for it to pe= rform its > > > + * processing. An example of such a component is the eventdev softwa= re PMD. > > > + * > > > + * The config struct should be filled in as appropriate by the PMD. = For example > > > + * the name field should include some level of detail (e.g. "ethdev_= p1_rx_q3" > > > + * might mean RX from an ethdev from port 1, on queue 3). > > > + * > > > + * @param service > > > + * The service structure to be registered with EAL. > > > + * > > > + * @return > > > + * On success, zero > > > + * On failure, a negative error > > > + */ > > > +int rte_eal_service_register(const struct rte_service *service); > > > + > > > +/** > > > + * Get the count of services registered in the EAL. > > > + * > > > + * @return the number of services registered with EAL. > > > + */ > > > +uint32_t rte_eal_service_get_count(); > > > + > > > +/** > > > + * Writes all registered services to the application supplied array. > > > + * > > > + * This function can be used by the application to understand if and= what > > > + * services require running. Each service provides a config struct e= xposing > > > + * attributes of the service, which can be used by the application t= o decide on > > > + * its strategy of running services on cores. > > > + * > > > + * @param service_config > > > + * An array of service config structures to be filled in > > > + * > > > + * @param n > > > + * The size of the service config array > > > + * > > > + * @return 0 on successful providing all service configs > > > + * -ENOSPC if array passed in is too small > > > + */ > > > +int rte_eal_service_get_all(const struct rte_service_config *service= _config, > > > + uint32_t n); > > > > I'd suggest to follow snprintf() notation here: always return number of= all existing services. > > Then you wouldn't need rte_eal_service_get_count() at all. > > > > > + > > > +/** > > > + * Use *lcore_id* as a service core. > > > + * > > > + * EAL will internally remove *lcore_id* from the application access= ible > > > + * core list. As a result, the application will never have its code = run on > > > + * the service core, making the service cores totally transparent to= an app. > > > + * > > > + * This function should be called before *rte_eal_service_set_corema= sk* so that > > > + * when setting the core mask the value can be error checked to ensu= re that EAL > > > + * has an lcore backing the coremask specified. > > > + */ > > > +int rte_eal_service_use_lcore(uint16_t lcore_id); > > > + > > > +/** > > > + * Set a coremask for a service. > > > + * > > > + * A configuration for a service indicates which cores run the servi= ce, and > > > + * what other parameters are required to correclty handle the underl= ying device. > > > + * > > > + * Examples of advanced usage might be a HW device that supports mul= tiple lcores > > > + * transmitting packets on the same queue - the hardware provides a = mechanism > > > + * for multithreaded enqueue. In this case, the atomic_ > > > + * > > > + * @return 0 Success > > > + * -ENODEV Device with *name* does not exist > > > + * -ENOTSUP Configuration is un-supported > > > + */ > > > +int rte_eal_service_set_coremask(const char *name, uint64_t coremask= ); > > > > Not sure why do we need that function? > > We do know a service coremask after register() is executed. > > Would it be possible to add/remove cores from the service on the fly? > > I.E without stopping the actual service scheduling function first? > > > > >6. The application now calls remote_launch() as usual, and the applica= tion > > > + * can perform its processing as required without manually handli= ng the > > > + * partitioning of lcore resources for DPDK functionality. > > > > Ok, if the user has to do remote_launch() manually for each service cor= e anyway... > > Again it means user can't start/stop individual services, it has to st= op the whole service > > core first(and all services it is running) to stop a particular service= . > > Sounds not very convenient for the user from my perspective. > > Wonder can we have an API like that: > > > > /* > > * reserves all cores in coremask as service cores. > > * in fact it would mean eal_remote_launch(service_main_func) on each o= f them. > > */ > > int rte_eal_service_start(const rte_cpuset_t *coremask); > > > > /* > > * stops all services and returns service lcores back to the EAL. > > */ > > int rte_eal_service_stop((const rte_cpuset_t *coremask); > > > > struct rte_service_config {name; cb_func; flags; ...}; > > > > struct rte_service *rte_service_register(struct rte_service_config *cf= g); > > rte_service_unregister(struct rte_service *srv); > > > > /* > > * adds/deletes lcore from the list of service cpus to run this servic= e on. > > * An open question - should we allow to do add/del lcore while servic= e is running or not. > > * From user perspective, it would be much more convenient to allow. > > */ > > int rte_service_add_lcore(struct rte_service *, uint32_t lcore_id, void= *lcore_cb_arg); > > int rte_service_del_lcore(struct rte_service *, uint32_t lcore_id); > > > > /* > > * starts/stops the service. > > */ > > int rte_service_start(struct rte_service *srv, const rte_cpuset_t *co= remask); > > int rte_service_stop(struct rte_service *srv, const rte_cpuset_t *cor= emask); > > > > > > > > > > > > > > > >