[RFC] Service Cores concept

[RFC] Service Cores concept

[Harry van Haaren]

This RFC introduces the concept of a service-core. A service core
invokes a function when an lcore is required to perform some work.

An example use-case is the eventdev; with the octeontx PMD, events are
scheduled in the hardware. With the software eventdev PMD an lcore is
required to perform this scheduling behaviour. Similar software schemes
can be applied for Traffic-Management/QoS for example.

The concept is to allow a software function register itself with EAL as
a "service", which requires CPU time to perform its duties. Multiple
services can be registered in an application, if more than one service
exists. The application can retrieve a list of services, and decide how
many "service cores" to use. The number of service cores is removed
from the application usage, and they are mapped to services based on
an application supplied coremask.

The application now continues as normal, without having to manually
schedule and implement arbitration of CPU time for the SW services.

Please note this is an RFC, and API updates to make it cleaner (moving
things to private .h files etc) are expected. Think about the concept
first - then look at the detail :)

This RFC addresses the discussion in this thread (split over two months),
and participants of those discussions are on --cc:
http://dpdk.org/ml/archives/dev/2017-April/064544.html
http://dpdk.org/ml/archives/dev/2017-May/065176.html

Regards, -Harry

Harry van Haaren (1):
  RFC: service core concept header implementation

 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

-- 
2.7.4

[RFC] service core concept header implementation

[Harry van Haaren]

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 <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
+ * 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.
+ *
+ * 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 {
+	/* name of the service */
+	char name[RTE_SERVICE_NAMESIZE];
+	/* cores that run this service */
+	uint64_t coremask;
+	/* 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;
+};
+
+/** @internal - this will not be visible to application, defined in a seperate
+ * rte_eal_service_impl.h header. 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;
+	/* 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 allocate
+ * cores to run the various services.
+ *
+ * This function is expected to be called by a DPDK component to indicate that
+ * it require a CPU to run a specific function in order for it to perform its
+ * processing. An example of such a component is the eventdev software 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 exposing
+ * attributes of the service, which can be used by the application to 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);
+
+/**
+ * Use *lcore_id* as a service core.
+ *
+ * EAL will internally remove *lcore_id* from the application accessible
+ * 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_coremask* so that
+ * when setting the core mask the value can be error checked to ensure 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 service, and
+ * what other parameters are required to correclty handle the underlying device.
+ *
+ * Examples of advanced usage might be a HW device that supports multiple 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);
+
+
+#ifdef __cplusplus
+}
+#endif
+
+
+#endif /* _RTE_SERVICE_H_ */
-- 
2.7.4

Re: [RFC] service core concept header implementation

[Jerin Jacob]

-----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

> +	/* 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;
> +};
> +
> +/** @internal - this will not be visible to application, defined in a seperate
> + * rte_eal_service_impl.h header. 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;
> +	/* configuration of the service */
> +	struct rte_service_config config;
> +};
> +
> +/**
> + * @internal - Only DPDK internal components request "service" cores.

remove @internal here

> + *
> + * 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 allocate
> + * cores to run the various services.
> + *
> + * This function is expected to be called by a DPDK component to indicate that
> + * it require a CPU to run a specific function in order for it to perform its
> + * processing. An example of such a component is the eventdev software 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 exposing
> + * attributes of the service, which can be used by the application to decide on
> + * its strategy of running services on cores.
> + *
> + * @param service_config

param [out] 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);
> +
> +/**
> + * Use *lcore_id* as a service core.
> + *
> + * EAL will internally remove *lcore_id* from the application accessible
> + * 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_coremask* so that
> + * when setting the core mask the value can be error checked to ensure 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 service, and
> + * what other parameters are required to correclty handle the underlying device.
> + *
> + * Examples of advanced usage might be a HW device that supports multiple lcores
> + * transmitting packets on the same queue - the hardware provides a mechanism
> + * for multithreaded enqueue. In this case, the atomic_
Missing text after atomic_.


missing @param
> + *
> + * @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);
> +
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +
> +#endif /* _RTE_SERVICE_H_ */
> -- 
> 2.7.4
> 

Re: [RFC] service core concept header implementation

[Jerin Jacob]

-----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

Re: [RFC] service core concept header implementation

[Van Haaren, Harry]

> 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 page. This RFC was pretty "fresh", mostly to check if the community agrees with the general concept of service-cores. The implementation details (although going well) will have some churn I expect :)

I'd like opinions from others in the community - I'll leave this thread "open" 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 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

Noted

> > > + * 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.


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, where the work is shared between the cores. Hence, setting coremasks and setting 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 function-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 and
> > 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 = 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 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

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,
> 
> 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 implementation for some ideas then.


> 2) What would be the tear down sequence of the service core especially when
> device stop or close happens?

Good question. Being able to "turn off" a single service while keeping other services running would be useful I think. That might require some extra tracking at the implementation layer, but perhaps something that's worth doing. Opinions and use-cases welcome here!

Re: [RFC] service core concept header implementation

[Jerin Jacob]

-----Original Message-----
> Date: Fri, 5 May 2017 15:48:02 +0000
> 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>
> Subject: RE: [RFC] service core concept header implementation
> 
> 
> > 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 page. This RFC was pretty "fresh", mostly to check if the community agrees with the general concept of service-cores. The implementation details (although going well) will have some churn I expect :)
> 
> I'd like opinions from others in the community - I'll leave this thread "open" for a while to get more feedback, and rework a v2 RFC then.
> 
> Thanks for your review - Harry
> 
> 
> > >
> > > 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, where the work is shared between the cores. Hence, setting coremasks and setting 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 function-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 and
> > > 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 );

I thought, The application may not need to set the explicit
rte_eal_service_set_lcore_type, it can be internally set by
rte_eal_service_set_coremask

i.e

for( i < services_count )
    rte_eal_service_set_coremask( app_decided_coremask_here );

int ret = rte_eal_service_apply();

rte_eal_service_set_coremask(app_decided_coremask_here)
{
	proposed_implementation;
	for_each_bit_set(app_decided_coremask_here)
		rte_eal_service_set_lcore_type(SERVICE);
}


> 
> int ret = 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 */
> 
> > 2) What would be the tear down sequence of the service core especially when
> > device stop or close happens?
> 
> Good question. Being able to "turn off" a single service while keeping other services running would be useful I think. That might require some extra tracking at the implementation layer, but perhaps something that's worth doing. Opinions and use-cases welcome here!

I think it makes sense to add "turn off" support for each service
functions as we are multiplexing the service core.


> 
> 
> 

Re: [RFC] service core concept header implementation

[Ananyev, Konstantin]

Hi Harry,
I had a look, my questions/comments below.
>From  my perspective it looks like an attempt to introduce simple scheduling library inside DPDK.
Though some things left unclear from my perspective, but probably I just misunderstood your
intentions here.
Thanks
Konstantin

> 
> 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 <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
> 

...

> +
> +/**
> + * 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 coremasks or not?
If not, then I suppose there is no practical difference from what we have 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 only 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 service can consume?
     I suppose the answer is 'no', at least for current iteration. 

> +	/* 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;

Ok, and what would happen if this flag is not set, and user specified more
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 what 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 allocate
> + * cores to run the various services.
> + *
> + * This function is expected to be called by a DPDK component to indicate that
> + * it require a CPU to run a specific function in order for it to perform its
> + * processing. An example of such a component is the eventdev software 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 exposing
> + * attributes of the service, which can be used by the application to 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 accessible
> + * 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_coremask* so that
> + * when setting the core mask the value can be error checked to ensure 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 service, and
> + * what other parameters are required to correclty handle the underlying device.
> + *
> + * Examples of advanced usage might be a HW device that supports multiple 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 application
> + *    can perform its processing as required without manually handling the
> + *    partitioning of lcore resources for DPDK functionality.

Ok, if the user has to do remote_launch() manually for each service core anyway...
Again it means user can't start/stop individual services,  it has to stop 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 of 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  *cfg);
rte_service_unregister(struct rte_service *srv);

/*
  * adds/deletes lcore from the list of service cpus to run this service on.
  * An open question - should we allow to do add/del lcore while service 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   *coremask);
int rte_service_stop(struct rte_service *srv, const rte_cpuset_t   *coremask);

Re: [RFC] service core concept header implementation

[Bruce Richardson]

On Wed, May 17, 2017 at 12:47:52PM +0000, Ananyev, Konstantin wrote:
> Hi Harry,
> I had a look, my questions/comments below.
> From  my perspective it looks like an attempt to introduce simple scheduling library inside DPDK.
> Though some things left unclear from my perspective, but probably I just misunderstood your
> intentions here.
> Thanks
> Konstantin
> 

Hi Konstantin,

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.
See also comments made in reply to Thomas mail.

Hope this helps clarify things a little.

/Bruce

> > 
> > 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 <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
> > 
> 
> ...
> 
> > +
> > +/**
> > + * 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 coremasks or not?
> If not, then I suppose there is no practical difference from what we have 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 only 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 service can consume?
>      I suppose the answer is 'no', at least for current iteration. 
> 
> > +	/* 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;
> 
> Ok, and what would happen if this flag is not set, and user specified more
> 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 what 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 allocate
> > + * cores to run the various services.
> > + *
> > + * This function is expected to be called by a DPDK component to indicate that
> > + * it require a CPU to run a specific function in order for it to perform its
> > + * processing. An example of such a component is the eventdev software 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 exposing
> > + * attributes of the service, which can be used by the application to 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 accessible
> > + * 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_coremask* so that
> > + * when setting the core mask the value can be error checked to ensure 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 service, and
> > + * what other parameters are required to correclty handle the underlying device.
> > + *
> > + * Examples of advanced usage might be a HW device that supports multiple 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 application
> > + *    can perform its processing as required without manually handling the
> > + *    partitioning of lcore resources for DPDK functionality.
> 
> Ok, if the user has to do remote_launch() manually for each service core anyway...
> Again it means user can't start/stop individual services,  it has to stop 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 of 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  *cfg);
> rte_service_unregister(struct rte_service *srv);
> 
> /*
>   * adds/deletes lcore from the list of service cpus to run this service on.
>   * An open question - should we allow to do add/del lcore while service 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   *coremask);
> int rte_service_stop(struct rte_service *srv, const rte_cpuset_t   *coremask);
> 
> 
> 
> 
> 
> 
> 
> 

Re: [RFC] service core concept header implementation

[Ananyev, Konstantin]

Hi Bruce,

> >
> 
> Hi Konstantin,
> 
> 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. 
Though I think both PMD/core libraries  and app layer might be interested in 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.
> 
> Hope this helps clarify things a little.
> 
> /Bruce
> 
> > >
> > > 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 <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
> > >
> >
> > ...
> >
> > > +
> > > +/**
> > > + * 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 coremasks or not?
> > If not, then I suppose there is no practical difference from what we have 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 only 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 service can consume?
> >      I suppose the answer is 'no', at least for current iteration.
> >
> > > +	/* 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;
> >
> > Ok, and what would happen if this flag is not set, and user specified more
> > 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 what 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 allocate
> > > + * cores to run the various services.
> > > + *
> > > + * This function is expected to be called by a DPDK component to indicate that
> > > + * it require a CPU to run a specific function in order for it to perform its
> > > + * processing. An example of such a component is the eventdev software 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 exposing
> > > + * attributes of the service, which can be used by the application to 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 accessible
> > > + * 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_coremask* so that
> > > + * when setting the core mask the value can be error checked to ensure 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 service, and
> > > + * what other parameters are required to correclty handle the underlying device.
> > > + *
> > > + * Examples of advanced usage might be a HW device that supports multiple 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 application
> > > + *    can perform its processing as required without manually handling the
> > > + *    partitioning of lcore resources for DPDK functionality.
> >
> > Ok, if the user has to do remote_launch() manually for each service core anyway...
> > Again it means user can't start/stop individual services,  it has to stop 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 of 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  *cfg);
> > rte_service_unregister(struct rte_service *srv);
> >
> > /*
> >   * adds/deletes lcore from the list of service cpus to run this service on.
> >   * An open question - should we allow to do add/del lcore while service 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   *coremask);
> > int rte_service_stop(struct rte_service *srv, const rte_cpuset_t   *coremask);
> >
> >
> >
> >
> >
> >
> >
> >

Re: [RFC] service core concept header implementation

[Van Haaren, Harry]

Hi All,


<snip> Service Cores RFC, more details and discussion </snip>


> Though I think both PMD/core libraries  and app layer might be interested in that: i.e.
> app might also have some background processing tasks, that might need to be run
> on several cores (or core slices).


Thanks everybody for the input on the RFC - appreciated! From an application point-of-view, I see merit in Konstantin's suggestions for the API, over the RFC I sent previously. I will re-work the API taking inspiration from both APIs and send an RFCv2, you'll be on CC :)


Cheers, -Harry

Re: [RFC] Service Cores concept

[Thomas Monjalon]

03/05/2017 13:29, Harry van Haaren:
> The concept is to allow a software function register itself with EAL as
> a "service", which requires CPU time to perform its duties. Multiple
> services can be registered in an application, if more than one service
> exists. The application can retrieve a list of services, and decide how
> many "service cores" to use. The number of service cores is removed
> from the application usage, and they are mapped to services based on
> an application supplied coremask.
> 
> The application now continues as normal, without having to manually
> schedule and implement arbitration of CPU time for the SW services.

I think it should not be the DPDK responsibility to schedule threads.
The mainloops and scheduling are application design choices.

If I understand well the idea of your proposal, it is a helper for
the application to configure the thread scheduling of known services.
So I think we could add interrupt processing and other thread creations
in this concept.
Could we also embed the rte_eal_mp_remote_launch() calls in this concept?

Re: [RFC] Service Cores concept

[Wiles, Keith]

> On May 16, 2017, at 3:11 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 03/05/2017 13:29, Harry van Haaren:
>> The concept is to allow a software function register itself with EAL as
>> a "service", which requires CPU time to perform its duties. Multiple
>> services can be registered in an application, if more than one service
>> exists. The application can retrieve a list of services, and decide how
>> many "service cores" to use. The number of service cores is removed
>> from the application usage, and they are mapped to services based on
>> an application supplied coremask.
>> 
>> The application now continues as normal, without having to manually
>> schedule and implement arbitration of CPU time for the SW services.
> 
> I think it should not be the DPDK responsibility to schedule threads.
> The mainloops and scheduling are application design choices.
> 
> If I understand well the idea of your proposal, it is a helper for
> the application to configure the thread scheduling of known services.
> So I think we could add interrupt processing and other thread creations
> in this concept.
> Could we also embed the rte_eal_mp_remote_launch() calls in this concept?

I did not really see the RFC as replacing the current design in DPDK for thread handling, but that maybe just me. What I saw was a design to create and destroy threads more dynamically or possible some type of thread pool design.

DPDK needs a thread handler anyway (as we have one now) and the current one seems to work, but it does not allow for someone to replace the thread handling with something different. What I would suggest is figure out how to pull DPDK current thread handling out into a plugin design, then someone can replace that plugin with his own thread handling scheme.

We need to keep DPDK stable, so adding the plugin support and pulling out the current design needs to be transparent to all current applications using the current APIs and methods.

Regards,
Keith

Re: [RFC] Service Cores concept

[Bruce Richardson]

On Wed, May 17, 2017 at 12:11:10AM +0200, Thomas Monjalon wrote:
> 03/05/2017 13:29, Harry van Haaren:
> > The concept is to allow a software function register itself with EAL as
> > a "service", which requires CPU time to perform its duties. Multiple
> > services can be registered in an application, if more than one service
> > exists. The application can retrieve a list of services, and decide how
> > many "service cores" to use. The number of service cores is removed
> > from the application usage, and they are mapped to services based on
> > an application supplied coremask.
> > 
> > The application now continues as normal, without having to manually
> > schedule and implement arbitration of CPU time for the SW services.
> 
> I think it should not be the DPDK responsibility to schedule threads.
> The mainloops and scheduling are application design choices.
> 
> If I understand well the idea of your proposal, it is a helper for
> the application to configure the thread scheduling of known services.
> So I think we could add interrupt processing and other thread creations
> in this concept.
> Could we also embed the rte_eal_mp_remote_launch() calls in this concept?


There are a couple of parts of this:
1. Allowing libraries and drivers to register the fact that they require
background processing, e.g. as a SW fallback for functionality that
would otherwise be implemented in hardware
2. Providing support for easily multi-plexing these independent
functions from different libs onto a different core, compared to the
normal operation of DPDK of firing a single run-forever function on each
core.
3. Providing support for the application to configure the running of
these background services on specific cores.
4. Once configured, hiding these services and the cores they run on from
the rest of the application, so that the rest of the app logic does not
need to change depending on whether service cores are in use or not. For
instance, removing the service cores from the core list in foreach-lcore
loops, and preventing the EAL from trying to run app functions on the
cores when the app calls mp_remote_launch.

Overall, the objective is to provide us a way to have software
equivalents of hardware functions in as transparent a manner as
possible. There is a certain amount of scheduling being done by the
DPDK, but it is still very much under the control of the app.

As for other things being able to use this concept, definite +1 for
interrupt threads and similar. I would not see mp_remote_launch as being
affected here in any significant way (except from the hiding service
cores from it, obviously)

/Bruce

Re: [RFC] Service Cores concept

[Thomas Monjalon]

17/05/2017 12:32, Bruce Richardson:
> On Wed, May 17, 2017 at 12:11:10AM +0200, Thomas Monjalon wrote:
> > 03/05/2017 13:29, Harry van Haaren:
> > > The concept is to allow a software function register itself with EAL as
> > > a "service", which requires CPU time to perform its duties. Multiple
> > > services can be registered in an application, if more than one service
> > > exists. The application can retrieve a list of services, and decide how
> > > many "service cores" to use. The number of service cores is removed
> > > from the application usage, and they are mapped to services based on
> > > an application supplied coremask.
> > > 
> > > The application now continues as normal, without having to manually
> > > schedule and implement arbitration of CPU time for the SW services.
> > 
> > I think it should not be the DPDK responsibility to schedule threads.
> > The mainloops and scheduling are application design choices.
> > 
> > If I understand well the idea of your proposal, it is a helper for
> > the application to configure the thread scheduling of known services.
> > So I think we could add interrupt processing and other thread creations
> > in this concept.
> > Could we also embed the rte_eal_mp_remote_launch() calls in this concept?
> 
> 
> There are a couple of parts of this:
> 1. Allowing libraries and drivers to register the fact that they require
> background processing, e.g. as a SW fallback for functionality that
> would otherwise be implemented in hardware
> 2. Providing support for easily multi-plexing these independent
> functions from different libs onto a different core, compared to the
> normal operation of DPDK of firing a single run-forever function on each
> core.
> 3. Providing support for the application to configure the running of
> these background services on specific cores.
> 4. Once configured, hiding these services and the cores they run on from
> the rest of the application, so that the rest of the app logic does not
> need to change depending on whether service cores are in use or not. For
> instance, removing the service cores from the core list in foreach-lcore
> loops, and preventing the EAL from trying to run app functions on the
> cores when the app calls mp_remote_launch.
> 
> Overall, the objective is to provide us a way to have software
> equivalents of hardware functions in as transparent a manner as
> possible. There is a certain amount of scheduling being done by the
> DPDK, but it is still very much under the control of the app.
> 
> As for other things being able to use this concept, definite +1 for
> interrupt threads and similar. I would not see mp_remote_launch as being
> affected here in any significant way (except from the hiding service
> cores from it, obviously)

OK to register CPU needs for services (including interrupts processing).

Then we could take this opportunity to review how threads are managed.
We will have three types of cores:
- not used
- reserved for services
- used for polling / application processing
It is fine to reserve/define CPU from DPDK point of view.

Then DPDK launch threads on cores. Maybe we should allow the application
to choose how threads are launched and managed.
Keith was talking about a plugin approach for thread management I think.

Re: [RFC] Service Cores concept

[Bruce Richardson]

On Wed, May 17, 2017 at 01:28:14PM +0200, Thomas Monjalon wrote:
> 17/05/2017 12:32, Bruce Richardson:
> > On Wed, May 17, 2017 at 12:11:10AM +0200, Thomas Monjalon wrote:
> > > 03/05/2017 13:29, Harry van Haaren:
> > > > The concept is to allow a software function register itself with EAL as
> > > > a "service", which requires CPU time to perform its duties. Multiple
> > > > services can be registered in an application, if more than one service
> > > > exists. The application can retrieve a list of services, and decide how
> > > > many "service cores" to use. The number of service cores is removed
> > > > from the application usage, and they are mapped to services based on
> > > > an application supplied coremask.
> > > > 
> > > > The application now continues as normal, without having to manually
> > > > schedule and implement arbitration of CPU time for the SW services.
> > > 
> > > I think it should not be the DPDK responsibility to schedule threads.
> > > The mainloops and scheduling are application design choices.
> > > 
> > > If I understand well the idea of your proposal, it is a helper for
> > > the application to configure the thread scheduling of known services.
> > > So I think we could add interrupt processing and other thread creations
> > > in this concept.
> > > Could we also embed the rte_eal_mp_remote_launch() calls in this concept?
> > 
> > 
> > There are a couple of parts of this:
> > 1. Allowing libraries and drivers to register the fact that they require
> > background processing, e.g. as a SW fallback for functionality that
> > would otherwise be implemented in hardware
> > 2. Providing support for easily multi-plexing these independent
> > functions from different libs onto a different core, compared to the
> > normal operation of DPDK of firing a single run-forever function on each
> > core.
> > 3. Providing support for the application to configure the running of
> > these background services on specific cores.
> > 4. Once configured, hiding these services and the cores they run on from
> > the rest of the application, so that the rest of the app logic does not
> > need to change depending on whether service cores are in use or not. For
> > instance, removing the service cores from the core list in foreach-lcore
> > loops, and preventing the EAL from trying to run app functions on the
> > cores when the app calls mp_remote_launch.
> > 
> > Overall, the objective is to provide us a way to have software
> > equivalents of hardware functions in as transparent a manner as
> > possible. There is a certain amount of scheduling being done by the
> > DPDK, but it is still very much under the control of the app.
> > 
> > As for other things being able to use this concept, definite +1 for
> > interrupt threads and similar. I would not see mp_remote_launch as being
> > affected here in any significant way (except from the hiding service
> > cores from it, obviously)
> 
> OK to register CPU needs for services (including interrupts processing).
> 
> Then we could take this opportunity to review how threads are managed.
> We will have three types of cores:
> - not used
> - reserved for services
> - used for polling / application processing
> It is fine to reserve/define CPU from DPDK point of view.
> 
> Then DPDK launch threads on cores. Maybe we should allow the application
> to choose how threads are launched and managed.
> Keith was talking about a plugin approach for thread management I think.

For thread management, I'd view us as extending what we have with some
EAL APIs rather than replacing what is there already. What I think we
could with would be APIs to:
* spawn an additional thread on a core i.e. add a bit to the coremask
* shutdown a thread on a core i.e. remove a bit from the coremask
* register an existing thread with DPDK, i.e. give it an lcore_id
  internally so that it can use DPDK data structures as a first-class
  citizen.

However, while needed, this is separate work from the service cores concept.

Regards,
/Bruce

Discuss plugin threading model for DPDK.

[Wiles, Keith]

> On May 17, 2017, at 4:28 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> OK to register CPU needs for services (including interrupts processing).
> 
> Then we could take this opportunity to review how threads are managed.
> We will have three types of cores:
> - not used
> - reserved for services
> - used for polling / application processing
> It is fine to reserve/define CPU from DPDK point of view.
> 
> Then DPDK launch threads on cores. Maybe we should allow the application
> to choose how threads are launched and managed.
> Keith was talking about a plugin approach for thread management I think.
Thomas,
So, not to hijack this thread or maybe I misunderstood your comment I changed the subject.

Maybe we can look at the plugin model for a DPDK threading model to allow someone to use their own threading solution.

Is this required or just another enhancement?

Regards,
Keith

Re: Discuss plugin threading model for DPDK.

[Thomas Monjalon]

17/05/2017 16:51, Wiles, Keith:
> 
> > On May 17, 2017, at 4:28 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 
> > OK to register CPU needs for services (including interrupts processing).
> > 
> > Then we could take this opportunity to review how threads are managed.
> > We will have three types of cores:
> > - not used
> > - reserved for services
> > - used for polling / application processing
> > It is fine to reserve/define CPU from DPDK point of view.
> > 
> > Then DPDK launch threads on cores. Maybe we should allow the application
> > to choose how threads are launched and managed.
> > Keith was talking about a plugin approach for thread management I think.
> Thomas,
> So, not to hijack this thread or maybe I misunderstood your comment I changed the subject.
> 
> Maybe we can look at the plugin model for a DPDK threading model to allow someone to use their own threading solution.
> 
> Is this required or just another enhancement?

It is another enhancement.
As the service core would be a new API, we should check that it is
compatible with a possible evolution of the underlying thread model.
And I think it can be a good opportunity to draw a complete view of
how DPDK could evolve regarding the thread model.