Re: [RFC] service core concept header implementation

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Thu, 4 May 2017 12:05:54 +0530
> From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> To: Harry van Haaren <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org, bruce.richardson@intel.com, hemant.agrawal@nxp.com,
>  nipun.gupta@nxp.com, narender.vangati@intel.com, gage.eads@intel.com
> Subject: Re: [RFC] service core concept header implementation
> User-Agent: Mutt/1.8.2 (2017-04-18)
> 
> -----Original Message-----
> > Date: Wed, 3 May 2017 12:29:21 +0100
> > From: Harry van Haaren <harry.van.haaren@intel.com>
> > To: dev@dpdk.org
> > CC: Harry van Haaren <harry.van.haaren@intel.com>,
> >  bruce.richardson@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
> >  narender.vangati@intel.com, jerin.jacob@caviumnetworks.com,
> >  gage.eads@intel.com
> > Subject: [RFC] service core concept header implementation
> > X-Mailer: git-send-email 2.7.4
> 
> Hi Harry,
> 
> Overall it looks good. Some initial comments
> 
> > 
> > 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 without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copyright
> > + *       notice, this list of conditions and the following disclaimer 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 derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF 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 THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + */
> > +
> > +#ifndef _RTE_SERVICE_H_
> > +#define _RTE_SERVICE_H_
> > +
> > +/**
> > + * @file
> > + *
> > + * Service functions
> > + *
> > + * The service functionality provided by this header allows a DPDK component
> > + * to indicate that it requires a function call in order for it to perform
> > + * its processing.
> > + *
> > + * An example usage of this functionality would be a component that registers
> > + * a service to perform a particular packet processing duty: for example the
> > + * eventdev software PMD. At startup the application requests all services
> > + * that have been registered, and the app decides how many cores will run the
> 
> s/app/application
> 
> > + * required services. The EAL removes these number of cores from the available
> > + * runtime cores, and dedicates them to performing service-core workloads. 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 dedicating one core
> > + * exclusively to run the service would interact with the API as follows:
> > + *
> > + * 1. Eventdev SW PMD calls *rte_eal_service_register*, indicating that 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 provide it
> > + *    with an array of *rte_service_config* structures. These structures
> > + *    provide the application with the name of the service, along with
> > + *    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 times, once
> > + *    for each lcore that should be used as a service core. These cores are
> > + *    removed from the application usage, and EAL will refuse to launch
> > + *    user-specified functions on these cores.
> > + *
> > + * 5. The application calls *rte_eal_service_set_coremask* to set the 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 allows 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.
> 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)
> 
> 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 and
> ensure at least on core is going to be running the service.
> 
> > + *
> > + * 6. The application now calls remote_launch() as usual, and the application
> > + *    can perform its processing as required without manually handling 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
> 
> > +	/* 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
> 
> > +	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

Two more questions,

1) If its for only for NUMA. Is it socket_id mask enough ?
2) What would be the tear down sequence of the service core especially when
device stop or close happens?

> 
> > +	/* when set, multiple lcores can run this service simultaneously without
> > +	 * 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;
> > +};
> > +

























































































.

s