* [dpdk-dev] [PATCH] eventdev: add producer enqueue hint @ 2017-06-12 11:46 Jerin Jacob 2017-06-26 15:44 ` Eads, Gage 0 siblings, 1 reply; 5+ messages in thread From: Jerin Jacob @ 2017-06-12 11:46 UTC (permalink / raw) To: dev Cc: bruce.richardson, harry.van.haaren, hemant.agrawal, gage.eads, nipun.gupta, narender.vangati, nikhil.rao, Jerin Jacob Some PMD like OCTEONTX can have optimized handling of events if the PMD knows it is a producer pattern in advance. For instance, OCTEONTX PMD can support burst mode if op == RTE_EVENT_OP_NEW. Since the event producer initialize(set all_op_new == 1) the event object before the main producer loop, This scheme does not call for any performance regression on other PMDs. Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com> --- Another option is to add a flag in enqueue API or have parallel enqueue API. --- drivers/event/octeontx/ssovf_worker.c | 12 ++++++++++-- lib/librte_eventdev/rte_eventdev.h | 10 +++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/event/octeontx/ssovf_worker.c b/drivers/event/octeontx/ssovf_worker.c index ad3fe684d..209c595cf 100644 --- a/drivers/event/octeontx/ssovf_worker.c +++ b/drivers/event/octeontx/ssovf_worker.c @@ -196,8 +196,16 @@ ssows_enq(void *port, const struct rte_event *ev) uint16_t __hot ssows_enq_burst(void *port, const struct rte_event ev[], uint16_t nb_events) { - RTE_SET_USED(nb_events); - return ssows_enq(port, ev); + uint16_t i; + struct ssows *ws = port; + + if (ev[0].all_op_new) { + rte_smp_wmb(); + for (i = 0; i < nb_events; i++) + ssows_new_event(ws, &ev[i]); + return i; + } else + return ssows_enq(port, ev); } void diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h index a248fe90e..1c1a46593 100644 --- a/lib/librte_eventdev/rte_eventdev.h +++ b/lib/librte_eventdev/rte_eventdev.h @@ -933,7 +933,15 @@ struct rte_event { * and is undefined on dequeue. * @see RTE_EVENT_OP_NEW, (RTE_EVENT_OP_*) */ - uint8_t rsvd:4; + uint8_t all_op_new:1; + /**< Valid only with event enqueue operation - This hint + * indicates that the enqueue request has only the + * events with op == RTE_EVENT_OP_NEW. + * The event producer, typically use this pattern to + * inject the events to eventdev. + * @see RTE_EVENT_OP_NEW rte_event_enqueue_burst() + */ + uint8_t rsvd:3; /**< Reserved for future use */ uint8_t sched_type:2; /**< Scheduler synchronization type (RTE_SCHED_TYPE_*) -- 2.13.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint 2017-06-12 11:46 [dpdk-dev] [PATCH] eventdev: add producer enqueue hint Jerin Jacob @ 2017-06-26 15:44 ` Eads, Gage 2017-06-27 8:08 ` Jerin Jacob 0 siblings, 1 reply; 5+ messages in thread From: Eads, Gage @ 2017-06-26 15:44 UTC (permalink / raw) To: Jerin Jacob, dev Cc: Richardson, Bruce, Van Haaren, Harry, hemant.agrawal, nipun.gupta, Vangati, Narender, Rao, Nikhil > -----Original Message----- > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Monday, June 12, 2017 6:46 AM > To: dev@dpdk.org > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Van Haaren, Harry > <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; Eads, Gage > <gage.eads@intel.com>; nipun.gupta@nxp.com; Vangati, Narender > <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>; Jerin Jacob > <jerin.jacob@caviumnetworks.com> > Subject: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint > > Some PMD like OCTEONTX can have optimized handling of events if the PMD > knows it is a producer pattern in advance. > For instance, OCTEONTX PMD can support burst mode if op == > RTE_EVENT_OP_NEW. > > Since the event producer initialize(set all_op_new == 1) the event object before > the main producer loop, This scheme does not call for any performance > regression on other PMDs. > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com> > --- > Another option is to add a flag in enqueue API or have parallel enqueue API. > --- > drivers/event/octeontx/ssovf_worker.c | 12 ++++++++++-- > lib/librte_eventdev/rte_eventdev.h | 10 +++++++++- > 2 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/drivers/event/octeontx/ssovf_worker.c > b/drivers/event/octeontx/ssovf_worker.c > index ad3fe684d..209c595cf 100644 > --- a/drivers/event/octeontx/ssovf_worker.c > +++ b/drivers/event/octeontx/ssovf_worker.c > @@ -196,8 +196,16 @@ ssows_enq(void *port, const struct rte_event *ev) > uint16_t __hot ssows_enq_burst(void *port, const struct rte_event ev[], > uint16_t nb_events) { > - RTE_SET_USED(nb_events); > - return ssows_enq(port, ev); > + uint16_t i; > + struct ssows *ws = port; > + > + if (ev[0].all_op_new) { > + rte_smp_wmb(); > + for (i = 0; i < nb_events; i++) > + ssows_new_event(ws, &ev[i]); > + return i; > + } else > + return ssows_enq(port, ev); > } > > void > diff --git a/lib/librte_eventdev/rte_eventdev.h > b/lib/librte_eventdev/rte_eventdev.h > index a248fe90e..1c1a46593 100644 > --- a/lib/librte_eventdev/rte_eventdev.h > +++ b/lib/librte_eventdev/rte_eventdev.h > @@ -933,7 +933,15 @@ struct rte_event { > * and is undefined on dequeue. > * @see RTE_EVENT_OP_NEW, (RTE_EVENT_OP_*) > */ > - uint8_t rsvd:4; > + uint8_t all_op_new:1; > + /**< Valid only with event enqueue operation - This hint > + * indicates that the enqueue request has only the > + * events with op == RTE_EVENT_OP_NEW. > + * The event producer, typically use this pattern to > + * inject the events to eventdev. > + * @see RTE_EVENT_OP_NEW > rte_event_enqueue_burst() > + */ > + uint8_t rsvd:3; > /**< Reserved for future use */ > uint8_t sched_type:2; > /**< Scheduler synchronization type > (RTE_SCHED_TYPE_*) > -- > 2.13.1 I slightly prefer the parallel enqueue API -- I can see folks making the mistake of setting all_op_new without setting the op to RTE_EVENT_OP_NEW, and later adding a "forward-only" enqueue API could be interesting for the sw PMD -- but this looks fine to me. Curious if others have any thoughts. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint 2017-06-26 15:44 ` Eads, Gage @ 2017-06-27 8:08 ` Jerin Jacob 2017-06-27 8:44 ` Van Haaren, Harry 0 siblings, 1 reply; 5+ messages in thread From: Jerin Jacob @ 2017-06-27 8:08 UTC (permalink / raw) To: Eads, Gage Cc: dev, Richardson, Bruce, Van Haaren, Harry, hemant.agrawal, nipun.gupta, Vangati, Narender, Rao, Nikhil -----Original Message----- > Date: Mon, 26 Jun 2017 15:44:10 +0000 > From: "Eads, Gage" <gage.eads@intel.com> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org" > <dev@dpdk.org> > CC: "Richardson, Bruce" <bruce.richardson@intel.com>, "Van Haaren, Harry" > <harry.van.haaren@intel.com>, "hemant.agrawal@nxp.com" > <hemant.agrawal@nxp.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>, > "Vangati, Narender" <narender.vangati@intel.com>, "Rao, Nikhil" > <nikhil.rao@intel.com> > Subject: RE: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint > > > > > -----Original Message----- > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > Sent: Monday, June 12, 2017 6:46 AM > > To: dev@dpdk.org > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Van Haaren, Harry > > <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; Eads, Gage > > <gage.eads@intel.com>; nipun.gupta@nxp.com; Vangati, Narender > > <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>; Jerin Jacob > > <jerin.jacob@caviumnetworks.com> > > Subject: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint > > > > Some PMD like OCTEONTX can have optimized handling of events if the PMD > > knows it is a producer pattern in advance. > > For instance, OCTEONTX PMD can support burst mode if op == > > RTE_EVENT_OP_NEW. > > > > Since the event producer initialize(set all_op_new == 1) the event object before > > the main producer loop, This scheme does not call for any performance > > regression on other PMDs. > > > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com> > > --- > > Another option is to add a flag in enqueue API or have parallel enqueue API. > > --- > > drivers/event/octeontx/ssovf_worker.c | 12 ++++++++++-- > > lib/librte_eventdev/rte_eventdev.h | 10 +++++++++- > > 2 files changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/event/octeontx/ssovf_worker.c > > b/drivers/event/octeontx/ssovf_worker.c > > index ad3fe684d..209c595cf 100644 > > --- a/drivers/event/octeontx/ssovf_worker.c > > +++ b/drivers/event/octeontx/ssovf_worker.c > > @@ -196,8 +196,16 @@ ssows_enq(void *port, const struct rte_event *ev) > > uint16_t __hot ssows_enq_burst(void *port, const struct rte_event ev[], > > uint16_t nb_events) { > > - RTE_SET_USED(nb_events); > > - return ssows_enq(port, ev); > > + uint16_t i; > > + struct ssows *ws = port; > > + > > + if (ev[0].all_op_new) { > > + rte_smp_wmb(); > > + for (i = 0; i < nb_events; i++) > > + ssows_new_event(ws, &ev[i]); > > + return i; > > + } else > > + return ssows_enq(port, ev); > > } > > > > void > > diff --git a/lib/librte_eventdev/rte_eventdev.h > > b/lib/librte_eventdev/rte_eventdev.h > > index a248fe90e..1c1a46593 100644 > > --- a/lib/librte_eventdev/rte_eventdev.h > > +++ b/lib/librte_eventdev/rte_eventdev.h > > @@ -933,7 +933,15 @@ struct rte_event { > > * and is undefined on dequeue. > > * @see RTE_EVENT_OP_NEW, (RTE_EVENT_OP_*) > > */ > > - uint8_t rsvd:4; > > + uint8_t all_op_new:1; > > + /**< Valid only with event enqueue operation - This hint > > + * indicates that the enqueue request has only the > > + * events with op == RTE_EVENT_OP_NEW. > > + * The event producer, typically use this pattern to > > + * inject the events to eventdev. > > + * @see RTE_EVENT_OP_NEW > > rte_event_enqueue_burst() > > + */ > > + uint8_t rsvd:3; > > /**< Reserved for future use */ > > uint8_t sched_type:2; > > /**< Scheduler synchronization type > > (RTE_SCHED_TYPE_*) > > -- > > 2.13.1 > > I slightly prefer the parallel enqueue API -- I can see folks making the mistake of setting all_op_new without setting the op to RTE_EVENT_OP_NEW, and later adding a "forward-only" enqueue API could be interesting for the sw PMD -- but this looks fine to me. Curious if others have any thoughts. If forward-only parallel enqueue API interesting for the SW PMD then I can drop this one and introduce forward-only API. Let me know if others have any thoughts? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint 2017-06-27 8:08 ` Jerin Jacob @ 2017-06-27 8:44 ` Van Haaren, Harry 2017-06-27 10:17 ` Jerin Jacob 0 siblings, 1 reply; 5+ messages in thread From: Van Haaren, Harry @ 2017-06-27 8:44 UTC (permalink / raw) To: Jerin Jacob, Eads, Gage Cc: dev, Richardson, Bruce, hemant.agrawal, nipun.gupta, Vangati, Narender, Rao, Nikhil > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Tuesday, June 27, 2017 9:08 AM > To: Eads, Gage <gage.eads@intel.com> > Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Van Haaren, Harry > <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Vangati, > Narender <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com> > Subject: Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint <snip some patch code> > > > void > > > diff --git a/lib/librte_eventdev/rte_eventdev.h > > > b/lib/librte_eventdev/rte_eventdev.h > > > index a248fe90e..1c1a46593 100644 > > > --- a/lib/librte_eventdev/rte_eventdev.h > > > +++ b/lib/librte_eventdev/rte_eventdev.h > > > @@ -933,7 +933,15 @@ struct rte_event { > > > * and is undefined on dequeue. > > > * @see RTE_EVENT_OP_NEW, (RTE_EVENT_OP_*) > > > */ > > > - uint8_t rsvd:4; > > > + uint8_t all_op_new:1; > > > + /**< Valid only with event enqueue operation - This hint > > > + * indicates that the enqueue request has only the > > > + * events with op == RTE_EVENT_OP_NEW. > > > + * The event producer, typically use this pattern to > > > + * inject the events to eventdev. > > > + * @see RTE_EVENT_OP_NEW > > > rte_event_enqueue_burst() > > > + */ > > > + uint8_t rsvd:3; > > > /**< Reserved for future use */ > > > uint8_t sched_type:2; > > > /**< Scheduler synchronization type > > > (RTE_SCHED_TYPE_*) > > > -- > > > 2.13.1 > > > > I slightly prefer the parallel enqueue API -- I can see folks making the mistake of > setting all_op_new without setting the op to RTE_EVENT_OP_NEW, and later adding a > "forward-only" enqueue API could be interesting for the sw PMD -- but this looks fine to > me. Curious if others have any thoughts. > > If forward-only parallel enqueue API interesting for the SW PMD then I > can drop this one and introduce forward-only API. Let me know if others > have any thoughts? To make sure I understand correctly, the "parallel API" idea is to add a new function pointer per-PMD, and dedicate it to enqueueing a burst of packets with the same OP? So the end result would be function(s) in the public API like this: rte_event_enqueue_burst_new(port, new_events, n_events); rte_event_enqueue_burst_forward(port, new_events, n_events); Given these are a "specialization" of the generic enqueue_burst() function, the PMD is not obliged to implement them. If they are NULL, the eventdev.c infrastructure can just point the burst_new() and burst_forward() to the generic enqueue without any performance delta? The cost is some added code in the public header and infrastructure. The gain is that we don't overload the current API with new behavior. Assuming my description of the parallel proposal above is correct, +1 for the parallel function approach. I like APIs that "do what they say on the tin" :) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint 2017-06-27 8:44 ` Van Haaren, Harry @ 2017-06-27 10:17 ` Jerin Jacob 0 siblings, 0 replies; 5+ messages in thread From: Jerin Jacob @ 2017-06-27 10:17 UTC (permalink / raw) To: Van Haaren, Harry Cc: Eads, Gage, dev, Richardson, Bruce, hemant.agrawal, nipun.gupta, Vangati, Narender, Rao, Nikhil -----Original Message----- > Date: Tue, 27 Jun 2017 08:44:34 +0000 > From: "Van Haaren, Harry" <harry.van.haaren@intel.com> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Eads, Gage" > <gage.eads@intel.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>, "Rao, Nikhil" > <nikhil.rao@intel.com> > Subject: RE: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint > > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > Sent: Tuesday, June 27, 2017 9:08 AM > > To: Eads, Gage <gage.eads@intel.com> > > Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Van Haaren, Harry > > <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Vangati, > > Narender <narender.vangati@intel.com>; Rao, Nikhil <nikhil.rao@intel.com> > > Subject: Re: [dpdk-dev] [PATCH] eventdev: add producer enqueue hint > > <snip some patch code> > > > > > void > > > > diff --git a/lib/librte_eventdev/rte_eventdev.h > > > > b/lib/librte_eventdev/rte_eventdev.h > > > > index a248fe90e..1c1a46593 100644 > > > > --- a/lib/librte_eventdev/rte_eventdev.h > > > > +++ b/lib/librte_eventdev/rte_eventdev.h > > > > @@ -933,7 +933,15 @@ struct rte_event { > > > > * and is undefined on dequeue. > > > > * @see RTE_EVENT_OP_NEW, (RTE_EVENT_OP_*) > > > > */ > > > > - uint8_t rsvd:4; > > > > + uint8_t all_op_new:1; > > > > + /**< Valid only with event enqueue operation - This hint > > > > + * indicates that the enqueue request has only the > > > > + * events with op == RTE_EVENT_OP_NEW. > > > > + * The event producer, typically use this pattern to > > > > + * inject the events to eventdev. > > > > + * @see RTE_EVENT_OP_NEW > > > > rte_event_enqueue_burst() > > > > + */ > > > > + uint8_t rsvd:3; > > > > /**< Reserved for future use */ > > > > uint8_t sched_type:2; > > > > /**< Scheduler synchronization type > > > > (RTE_SCHED_TYPE_*) > > > > -- > > > > 2.13.1 > > > > > > I slightly prefer the parallel enqueue API -- I can see folks making the mistake of > > setting all_op_new without setting the op to RTE_EVENT_OP_NEW, and later adding a > > "forward-only" enqueue API could be interesting for the sw PMD -- but this looks fine to > > me. Curious if others have any thoughts. > > > > If forward-only parallel enqueue API interesting for the SW PMD then I > > can drop this one and introduce forward-only API. Let me know if others > > have any thoughts? > > > To make sure I understand correctly, the "parallel API" idea is to add a new function pointer per-PMD, and dedicate it to enqueueing a burst of packets with the same OP? So the end result would be function(s) in the public API like this: > > rte_event_enqueue_burst_new(port, new_events, n_events); > rte_event_enqueue_burst_forward(port, new_events, n_events); > > Given these are a "specialization" of the generic enqueue_burst() function, the PMD is not obliged to implement them. If they are NULL, the eventdev.c infrastructure can just point the burst_new() and burst_forward() to the generic enqueue without any performance delta? > > The cost is some added code in the public header and infrastructure. > The gain is that we don't overload the current API with new behavior. > > > Assuming my description of the parallel proposal above is correct, +1 for the parallel function approach. I like APIs that "do what they say on the tin" :) Yes. We are on the same page. I will send the v2. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-27 10:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-12 11:46 [dpdk-dev] [PATCH] eventdev: add producer enqueue hint Jerin Jacob 2017-06-26 15:44 ` Eads, Gage 2017-06-27 8:08 ` Jerin Jacob 2017-06-27 8:44 ` Van Haaren, Harry 2017-06-27 10:17 ` Jerin Jacob
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).