-----Original Message-----
> Date: Thu, 4 May 2017 12:05:54 +0530
> From: Jerin Jacob <jerin.ja...@caviumnetworks.com>
> To: Harry van Haaren <harry.van.haa...@intel.com>
> Cc: dev@dpdk.org, bruce.richard...@intel.com, hemant.agra...@nxp.com,
> nipun.gu...@nxp.com, narender.vang...@intel.com, gage.e...@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.haa...@intel.com>
> > To: dev@dpdk.org
> > CC: Harry van Haaren <harry.van.haa...@intel.com>,
> > bruce.richard...@intel.com, hemant.agra...@nxp.com, nipun.gu...@nxp.com,
> > narender.vang...@intel.com, jerin.ja...@caviumnetworks.com,
> > gage.e...@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.haa...@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
