[dpdk-dev,6/6] service cores: enable event/sw with service

Message ID 1498208779-166205-6-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Changes Requested, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Van Haaren, Harry June 23, 2017, 9:06 a.m. UTC
  This commit shows how easy it is to enable a specific
DPDK component with a service callback, in order to get
CPU cycles for it.

The beauty of this method is that the service is unaware
of how much CPU time it is getting - the application can
decide how to split and slice cores and map them to the
registered services.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 drivers/event/sw/sw_evdev.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
  

Comments

Jerin Jacob June 26, 2017, 1:46 p.m. UTC | #1
-----Original Message-----
> Date: Fri, 23 Jun 2017 10:06:19 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: thomas@monjalon.net, jerin.jacob@caviumnetworks.com,
>  keith.wiles@intel.com, bruce.richardson@intel.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH 6/6] service cores: enable event/sw with service
> X-Mailer: git-send-email 2.7.4
> 
> This commit shows how easy it is to enable a specific
> DPDK component with a service callback, in order to get
> CPU cycles for it.
> 
> The beauty of this method is that the service is unaware
> of how much CPU time it is getting - the application can
> decide how to split and slice cores and map them to the
> registered services.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
>  drivers/event/sw/sw_evdev.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> index a31aaa6..f55cad9 100644
> --- a/drivers/event/sw/sw_evdev.c
> +++ b/drivers/event/sw/sw_evdev.c
> @@ -38,6 +38,9 @@
>  #include <rte_ring.h>
>  #include <rte_errno.h>
>  
> +#include <rte_service_private.h>
> +#include <rte_cycles.h>

If rte_service_private.h needs the rte_cycles.h then I think it can be
included in rte_service_private.h.

> +
>  #include "sw_evdev.h"
>  #include "iq_ring.h"
>  #include "event_ring.h"
> @@ -695,6 +698,14 @@ set_credit_quanta(const char *key __rte_unused, const char *value, void *opaque)
>  	return 0;
>  }
>  
> +
> +static int32_t sw_sched_service_func(void *args)
> +{
> +	struct rte_eventdev *dev = args;
> +	sw_event_schedule(dev);
> +	return 0;
> +}
> +
>  static int
>  sw_probe(struct rte_vdev_device *vdev)
>  {
> @@ -806,6 +817,17 @@ sw_probe(struct rte_vdev_device *vdev)
>  	sw->credit_update_quanta = credit_quanta;
>  	sw->sched_quanta = sched_quanta;
>  
> +	/* register service with EAL */
> +	struct rte_service_spec service;
> +	memset(&service, 0, sizeof(struct rte_service_spec));
> +	snprintf(service.name, sizeof(service.name), "%s_service", name);

Should we add socket_id as well in the service name?


> +	service.socket_id = socket_id;
> +	service.callback = sw_sched_service_func;
> +	service.callback_userdata = (void *)dev;
> +
> +	int32_t ret = rte_service_register(&service);
> +	printf("serivce register = %d, cb ud %p\n", ret, dev);

sw_pmd_log?

> +
>  	return 0;

Should we also check rte_service_is_running() in sw pmd start function
to verify application did everything to setup the service function on
service lcore? or means for feedback?


>  }
>  
> -- 
> 2.7.4
>
  
Van Haaren, Harry June 29, 2017, 11:15 a.m. UTC | #2
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, June 26, 2017 2:47 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; thomas@monjalon.net; Wiles, Keith <keith.wiles@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>
> Subject: Re: [PATCH 6/6] service cores: enable event/sw with service
> 
> -----Original Message-----
> > Date: Fri, 23 Jun 2017 10:06:19 +0100
> > From: Harry van Haaren <harry.van.haaren@intel.com>
<snip>
> > +#include <rte_service_private.h>
> > +#include <rte_cycles.h>
> 
> If rte_service_private.h needs the rte_cycles.h then I think it can be
> included in rte_service_private.h.

Actually it was stale, a leftover from during development - removed.


> > +	/* register service with EAL */
> > +	struct rte_service_spec service;
> > +	memset(&service, 0, sizeof(struct rte_service_spec));
> > +	snprintf(service.name, sizeof(service.name), "%s_service", name);
> 
> Should we add socket_id as well in the service name?

The user can name the PMD instance with socket ID if they so wish. The name of the PMD instance must already be unique - so we won't gain anything by adding socket ID there either... I'm not seeing value?

The socket ID is available to the application by reading   struct service_spec.


> > +	service.socket_id = socket_id;
> > +	service.callback = sw_sched_service_func;
> > +	service.callback_userdata = (void *)dev;
> > +
> > +	int32_t ret = rte_service_register(&service);
> > +	printf("serivce register = %d, cb ud %p\n", ret, dev);
> 
> sw_pmd_log?

Removed - little value in seeing pointer values bar during development :)


> > +
> >  	return 0;
> 
> Should we also check rte_service_is_running() in sw pmd start function
> to verify application did everything to setup the service function on
> service lcore? or means for feedback?

Yes good idea - rte_service_is_running will return true if the service callback will be called by at least one service lcore - exactly what we need to check.

This check is implemented in v2, on the way soon. As noted in the first email, I'll kick off a mail about discussing how to best use service-cores, and how to enable applications benefit most from service cores (including not using them if they don't want to :)
  

Patch

diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index a31aaa6..f55cad9 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -38,6 +38,9 @@ 
 #include <rte_ring.h>
 #include <rte_errno.h>
 
+#include <rte_service_private.h>
+#include <rte_cycles.h>
+
 #include "sw_evdev.h"
 #include "iq_ring.h"
 #include "event_ring.h"
@@ -695,6 +698,14 @@  set_credit_quanta(const char *key __rte_unused, const char *value, void *opaque)
 	return 0;
 }
 
+
+static int32_t sw_sched_service_func(void *args)
+{
+	struct rte_eventdev *dev = args;
+	sw_event_schedule(dev);
+	return 0;
+}
+
 static int
 sw_probe(struct rte_vdev_device *vdev)
 {
@@ -806,6 +817,17 @@  sw_probe(struct rte_vdev_device *vdev)
 	sw->credit_update_quanta = credit_quanta;
 	sw->sched_quanta = sched_quanta;
 
+	/* register service with EAL */
+	struct rte_service_spec service;
+	memset(&service, 0, sizeof(struct rte_service_spec));
+	snprintf(service.name, sizeof(service.name), "%s_service", name);
+	service.socket_id = socket_id;
+	service.callback = sw_sched_service_func;
+	service.callback_userdata = (void *)dev;
+
+	int32_t ret = rte_service_register(&service);
+	printf("serivce register = %d, cb ud %p\n", ret, dev);
+
 	return 0;
 }