Re: [dpdk-dev] [PATCH 2/2] service: add runtime service core check disable

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> 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