[dpdk-dev,2/2] service: add runtime service core check disable
Checks
Commit Message
This commit adds a new function to disable the runtime mapped
service-cores check. This allows an application to take responsibility
of running unmapped services.
This feature is useful in cases like unit tests, where the application
code (or unit test in this case) requires accurate control over when
the service function is called to ensure correct behaviour, and when
an application has an advanced use-case and wishes to manage services
manually.
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
lib/librte_eal/common/include/rte_service.h | 23 ++++++++++++++++++++++-
lib/librte_eal/common/rte_service.c | 22 ++++++++++++++++++++--
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
4 files changed, 44 insertions(+), 3 deletions(-)
Comments
On Mon, Oct 23, 2017 at 06:16:52PM +0100, Harry van Haaren wrote:
> This commit adds a new function to disable the runtime mapped
> service-cores check. This allows an application to take responsibility
> of running unmapped services.
>
> This feature is useful in cases like unit tests, where the application
> code (or unit test in this case) requires accurate control over when
> the service function is called to ensure correct behaviour, and when
> an application has an advanced use-case and wishes to manage services
> manually.
>
Hey Harry,
The patch looks good from a logical point of view, I have integrated this with
4/7 and 5/7 of the patch set [1] for removing eventdev schedule API and everything
works well.
My only concern is that the function name is a bit tricky to grasp. Unless the
application(dev) knows the internal working of rte_service_runstate_get() it
would be a bit tricky to understand the role of
rte_service_set_runstate_mapped_check().
I think we should think of a more appropriate API name for this function.
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> lib/librte_eal/common/include/rte_service.h | 23 ++++++++++++++++++++++-
> lib/librte_eal/common/rte_service.c | 22 ++++++++++++++++++++--
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> 4 files changed, 44 insertions(+), 3 deletions(-)
<snip>
> + * @param enabled When zero, the check is disabled. Non-zero enables the check.
> + *
> + * @retval 0 Success
> + * @retval -EINVAL Invalid service ID
> + */
> +int32_t rte_service_set_runstate_mapped_check(uint32_t id, int32_t enabled);
> +
s/enabled/enable
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> * This function runs a service callback from a non-service lcore context.
> * The *id* of the service to be run is passed in, and the service-callback
> * is executed on the calling lcore immediately if possible. If the service is
>
<snip>
> --
> 2.7.4
>
Regards,
Pavan
> From: Pavan Nikhilesh Bhagavatula [mailto:pbhagavatula@caviumnetworks.com]
> Sent: Tuesday, October 24, 2017 6:25 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 2/2] service: add runtime service core check disable
>
> On Mon, Oct 23, 2017 at 06:16:52PM +0100, Harry van Haaren wrote:
> > This commit adds a new function to disable the runtime mapped
> > service-cores check. This allows an application to take responsibility
> > of running unmapped services.
> >
> > This feature is useful in cases like unit tests, where the application
> > code (or unit test in this case) requires accurate control over when
> > the service function is called to ensure correct behaviour, and when
> > an application has an advanced use-case and wishes to manage services
> > manually.
> >
>
> Hey Harry,
>
> The patch looks good from a logical point of view, I have integrated this
> with
> 4/7 and 5/7 of the patch set [1] for removing eventdev schedule API and
> everything
> works well.
>
> My only concern is that the function name is a bit tricky to grasp. Unless
> the
> application(dev) knows the internal working of rte_service_runstate_get() it
> would be a bit tricky to understand the role of
> rte_service_set_runstate_mapped_check().
>
> I think we should think of a more appropriate API name for this function.
Suggestions welcome!
Otherwise I hope the documentation and unit-test usages explain the functionality ok.
Cheers, -Harry
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > ---
> > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> > lib/librte_eal/common/include/rte_service.h | 23
> ++++++++++++++++++++++-
> > lib/librte_eal/common/rte_service.c | 22
> ++++++++++++++++++++--
> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> > 4 files changed, 44 insertions(+), 3 deletions(-)
> <snip>
> > + * @param enabled When zero, the check is disabled. Non-zero enables the
> check.
> > + *
> > + * @retval 0 Success
> > + * @retval -EINVAL Invalid service ID
> > + */
> > +int32_t rte_service_set_runstate_mapped_check(uint32_t id, int32_t
> enabled);
> > +
> s/enabled/enable
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > * This function runs a service callback from a non-service lcore
> context.
> > * The *id* of the service to be run is passed in, and the service-
> callback
> > * is executed on the calling lcore immediately if possible. If the
> service is
> >
> <snip>
> > --
> > 2.7.4
> >
>
> Regards,
> Pavan
On Wed, Oct 25, 2017 at 09:09:53AM +0000, Van Haaren, Harry wrote:
> > From: Pavan Nikhilesh Bhagavatula [mailto:pbhagavatula@caviumnetworks.com]
> > Sent: Tuesday, October 24, 2017 6:25 PM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH 2/2] service: add runtime service core check disable
> >
> > On Mon, Oct 23, 2017 at 06:16:52PM +0100, Harry van Haaren wrote:
> > > This commit adds a new function to disable the runtime mapped
> > > service-cores check. This allows an application to take responsibility
> > > of running unmapped services.
> > >
> > > This feature is useful in cases like unit tests, where the application
> > > code (or unit test in this case) requires accurate control over when
> > > the service function is called to ensure correct behaviour, and when
> > > an application has an advanced use-case and wishes to manage services
> > > manually.
> > >
> >
> > Hey Harry,
> >
> > The patch looks good from a logical point of view, I have integrated this
> > with
> > 4/7 and 5/7 of the patch set [1] for removing eventdev schedule API and
> > everything
> > works well.
> >
> > My only concern is that the function name is a bit tricky to grasp. Unless
> > the
> > application(dev) knows the internal working of rte_service_runstate_get() it
> > would be a bit tricky to understand the role of
> > rte_service_set_runstate_mapped_check().
> >
> > I think we should think of a more appropriate API name for this function.
>
>
> Suggestions welcome!
>
> Otherwise I hope the documentation and unit-test usages explain the functionality ok.
>
> Cheers, -Harry
>
I think the documentation would be sufficient as I don't have a better
suggestion for the function name.
-Pavan
>
> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > > ---
> > > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> > > lib/librte_eal/common/include/rte_service.h | 23
> > ++++++++++++++++++++++-
> > > lib/librte_eal/common/rte_service.c | 22
> > ++++++++++++++++++++--
> > > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> > > 4 files changed, 44 insertions(+), 3 deletions(-)
> > <snip>
> > > + * @param enabled When zero, the check is disabled. Non-zero enables the
> > check.
> > > + *
> > > + * @retval 0 Success
> > > + * @retval -EINVAL Invalid service ID
> > > + */
> > > +int32_t rte_service_set_runstate_mapped_check(uint32_t id, int32_t
> > enabled);
> > > +
> > s/enabled/enable
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > + *
> > > * This function runs a service callback from a non-service lcore
> > context.
> > > * The *id* of the service to be run is passed in, and the service-
> > callback
> > > * is executed on the calling lcore immediately if possible. If the
> > service is
> > >
> > <snip>
> > > --
> > > 2.7.4
> > >
> >
> > Regards,
> > Pavan
Acked-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
@@ -232,6 +232,7 @@ EXPERIMENTAL {
rte_service_run_iter_on_app_lcore;
rte_service_runstate_get;
rte_service_runstate_set;
+ rte_service_set_runstate_mapped_check;
rte_service_set_stats_enable;
rte_service_start_with_defaults;
@@ -199,7 +199,12 @@ int32_t rte_service_runstate_set(uint32_t id, uint32_t runstate);
* @b EXPERIMENTAL: this API may change without prior notice
*
* Get the runstate for the service with *id*. See *rte_service_runstate_set*
- * for details of runstates.
+ * for details of runstates. A service can call this function to ensure that
+ * the application has indicated that it will receive CPU cycles. Either a
+ * service-core is mapped (default case), or the application has explicitly
+ * disabled the check that a service-cores is mapped to the service and takes
+ * responsibility to run the service manually using the available function
+ * *rte_service_run_iter_on_app_lcore* to do so.
*
* @retval 1 Service is running
* @retval 0 Service is stopped
@@ -211,6 +216,22 @@ int32_t rte_service_runstate_get(uint32_t id);
* @warning
* @b EXPERIMENTAL: this API may change without prior notice
*
+ * Enable or disable the check for a service-core being mapped to the service.
+ * An application can disable the check when takes the responsibility to run a
+ * service itself using *rte_service_run_iter_on_app_lcore*.
+ *
+ * @param id The id of the service to set the check on
+ * @param enabled When zero, the check is disabled. Non-zero enables the check.
+ *
+ * @retval 0 Success
+ * @retval -EINVAL Invalid service ID
+ */
+int32_t rte_service_set_runstate_mapped_check(uint32_t id, int32_t enabled);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
* This function runs a service callback from a non-service lcore context.
* The *id* of the service to be run is passed in, and the service-callback
* is executed on the calling lcore immediately if possible. If the service is
@@ -54,6 +54,7 @@
#define SERVICE_F_REGISTERED (1 << 0)
#define SERVICE_F_STATS_ENABLED (1 << 1)
+#define SERVICE_F_START_CHECK (1 << 2)
/* runstates for services and lcores, denoting if they are active or not */
#define RUNSTATE_STOPPED 0
@@ -180,6 +181,19 @@ int32_t rte_service_set_stats_enable(uint32_t id, int32_t enabled)
return 0;
}
+int32_t rte_service_set_runstate_mapped_check(uint32_t id, int32_t enabled)
+{
+ struct rte_service_spec_impl *s;
+ SERVICE_VALID_GET_OR_ERR_RET(id, s, 0);
+
+ if (enabled)
+ s->internal_flags |= SERVICE_F_START_CHECK;
+ else
+ s->internal_flags &= ~(SERVICE_F_START_CHECK);
+
+ return 0;
+}
+
uint32_t
rte_service_get_count(void)
{
@@ -241,7 +255,7 @@ rte_service_component_register(const struct rte_service_spec *spec,
struct rte_service_spec_impl *s = &rte_services[free_slot];
s->spec = *spec;
- s->internal_flags |= SERVICE_F_REGISTERED;
+ s->internal_flags |= SERVICE_F_REGISTERED | SERVICE_F_START_CHECK;
rte_smp_wmb();
rte_service_count++;
@@ -309,9 +323,13 @@ rte_service_runstate_get(uint32_t id)
struct rte_service_spec_impl *s;
SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
rte_smp_rmb();
+
+ int check_disabled = !(s->internal_flags & SERVICE_F_START_CHECK);
+ int lcore_mapped = (s->num_mapped_cores > 0);
+
return (s->app_runstate == RUNSTATE_RUNNING) &&
(s->comp_runstate == RUNSTATE_RUNNING) &&
- (s->num_mapped_cores > 0);
+ (check_disabled | lcore_mapped);
}
static inline void
@@ -236,6 +236,7 @@ EXPERIMENTAL {
rte_service_run_iter_on_app_lcore;
rte_service_runstate_get;
rte_service_runstate_set;
+ rte_service_set_runstate_mapped_check;
rte_service_set_stats_enable;
rte_service_start_with_defaults;