[dpdk-dev,v4,2/2] event/sw: support device stop flush callback
Checks
Commit Message
This commit also adds a flush callback test to the sw eventdev's selftest
suite.
Signed-off-by: Gage Eads <gage.eads@intel.com>
---
drivers/event/sw/sw_evdev.c | 25 ++++++++++-
drivers/event/sw/sw_evdev_selftest.c | 80 +++++++++++++++++++++++++++++++++++-
2 files changed, 102 insertions(+), 3 deletions(-)
Comments
> From: Eads, Gage
> Sent: Tuesday, March 20, 2018 2:13 PM
> To: dev@dpdk.org
> Cc: jerin.jacob@caviumnetworks.com; Van Haaren, Harry
> <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; Richardson, Bruce
> <bruce.richardson@intel.com>; santosh.shukla@caviumnetworks.com;
> nipun.gupta@nxp.com
> Subject: [PATCH v4 2/2] event/sw: support device stop flush callback
>
> This commit also adds a flush callback test to the sw eventdev's selftest
> suite.
>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> ---
> drivers/event/sw/sw_evdev.c | 25 ++++++++++-
> drivers/event/sw/sw_evdev_selftest.c | 80
> +++++++++++++++++++++++++++++++++++-
> 2 files changed, 102 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> index 0e89f11..11f394f 100644
> --- a/drivers/event/sw/sw_evdev.c
> +++ b/drivers/event/sw/sw_evdev.c
> @@ -362,8 +362,25 @@ sw_init_qid_iqs(struct sw_evdev *sw)
> }
>
> static void
> -sw_clean_qid_iqs(struct sw_evdev *sw)
> +sw_flush_iq(struct rte_eventdev *dev, struct sw_iq *iq)
> {
> + struct sw_evdev *sw = sw_pmd_priv(dev);
> +
> + while (iq_count(iq) > 0) {
> + struct rte_event event;
> +
> + iq_dequeue_burst(sw, iq, &event, 1);
> +
> + dev->dev_ops->dev_stop_flush(dev->data->dev_id,
> + event,
> + dev->data->dev_stop_flush_arg);
Adding check that dev_stop_flush() is non-NULL?
[Update: Ah I see you do this below already. Still, better check twice
I think, the data path isn't running here anyway in case future me
decides to call sw_flush_iq() without performing the check]
if(dev->dev_ops->dev_stop_flush)
dev->dev_ops->dev_stop_flush(...);
> @@ -702,7 +723,7 @@ static void
> sw_stop(struct rte_eventdev *dev)
> {
> struct sw_evdev *sw = sw_pmd_priv(dev);
> - sw_clean_qid_iqs(sw);
> + sw_clean_qid_iqs(dev);
Based on the port buffers comment on 1/2, we probably need a
sw_clean_port_buffers(sw); here to return any events in the port
owned SW buffers?
Rest looks good to me!
> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Friday, March 23, 2018 12:06 PM
> To: Eads, Gage <gage.eads@intel.com>; dev@dpdk.org
> Cc: jerin.jacob@caviumnetworks.com; hemant.agrawal@nxp.com; Richardson,
> Bruce <bruce.richardson@intel.com>; santosh.shukla@caviumnetworks.com;
> nipun.gupta@nxp.com
> Subject: RE: [PATCH v4 2/2] event/sw: support device stop flush callback
>
> > From: Eads, Gage
> > Sent: Tuesday, March 20, 2018 2:13 PM
> > To: dev@dpdk.org
> > Cc: jerin.jacob@caviumnetworks.com; Van Haaren, Harry
> > <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; Richardson,
> > Bruce <bruce.richardson@intel.com>; santosh.shukla@caviumnetworks.com;
> > nipun.gupta@nxp.com
> > Subject: [PATCH v4 2/2] event/sw: support device stop flush callback
> >
> > This commit also adds a flush callback test to the sw eventdev's
> > selftest suite.
> >
> > Signed-off-by: Gage Eads <gage.eads@intel.com>
> > ---
> > drivers/event/sw/sw_evdev.c | 25 ++++++++++-
> > drivers/event/sw/sw_evdev_selftest.c | 80
> > +++++++++++++++++++++++++++++++++++-
> > 2 files changed, 102 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> > index 0e89f11..11f394f 100644
> > --- a/drivers/event/sw/sw_evdev.c
> > +++ b/drivers/event/sw/sw_evdev.c
> > @@ -362,8 +362,25 @@ sw_init_qid_iqs(struct sw_evdev *sw) }
> >
> > static void
> > -sw_clean_qid_iqs(struct sw_evdev *sw)
> > +sw_flush_iq(struct rte_eventdev *dev, struct sw_iq *iq)
> > {
> > + struct sw_evdev *sw = sw_pmd_priv(dev);
> > +
> > + while (iq_count(iq) > 0) {
> > + struct rte_event event;
> > +
> > + iq_dequeue_burst(sw, iq, &event, 1);
> > +
> > + dev->dev_ops->dev_stop_flush(dev->data->dev_id,
> > + event,
> > + dev->data->dev_stop_flush_arg);
>
>
> Adding check that dev_stop_flush() is non-NULL?
>
> [Update: Ah I see you do this below already. Still, better check twice I think, the
> data path isn't running here anyway in case future me decides to call
> sw_flush_iq() without performing the check]
Agreed, will fix in v5.
>
> if(dev->dev_ops->dev_stop_flush)
> dev->dev_ops->dev_stop_flush(...);
>
>
>
>
> > @@ -702,7 +723,7 @@ static void
> > sw_stop(struct rte_eventdev *dev)
> > {
> > struct sw_evdev *sw = sw_pmd_priv(dev);
> > - sw_clean_qid_iqs(sw);
> > + sw_clean_qid_iqs(dev);
>
> Based on the port buffers comment on 1/2, we probably need a
> sw_clean_port_buffers(sw); here to return any events in the port owned SW
> buffers?
>
Agreed, if there is buy-in on the approach discussed in patch #1.
> > [Update: Ah I see you do this below already. Still, better check twice I think, the
> > data path isn't running here anyway in case future me decides to call
> > sw_flush_iq() without performing the check]
>
> Agreed, will fix in v5.
Gage,
Could please send the v5 so that I can include it in RC1 pull request.
If it's ok with you, I'll resubmit these two patches separately. The sw implementation has some unresolved race conditions with the scheduler service that may take some time to fix, but this shouldn't block the first patch. That patch needs a fix too (v4 includes a now-incorrect comment about stop not draining events from ports), so I'll fix and resubmit that.
Thanks,
Gage
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, April 2, 2018 3:03 AM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org;
> hemant.agrawal@nxp.com; Richardson, Bruce <bruce.richardson@intel.com>;
> santosh.shukla@caviumnetworks.com; nipun.gupta@nxp.com
> Subject: Re: [PATCH v4 2/2] event/sw: support device stop flush callback
>
> > > [Update: Ah I see you do this below already. Still, better check
> > > twice I think, the data path isn't running here anyway in case
> > > future me decides to call
> > > sw_flush_iq() without performing the check]
> >
> > Agreed, will fix in v5.
>
> Gage,
>
> Could please send the v5 so that I can include it in RC1 pull request.
>
-----Original Message-----
> Date: Mon, 2 Apr 2018 15:50:42 +0000
> From: "Eads, Gage" <gage.eads@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "Van Haaren, Harry" <harry.van.haaren@intel.com>, "dev@dpdk.org"
> <dev@dpdk.org>, "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
> "Richardson, Bruce" <bruce.richardson@intel.com>,
> "santosh.shukla@caviumnetworks.com" <santosh.shukla@caviumnetworks.com>,
> "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>
> Subject: RE: [PATCH v4 2/2] event/sw: support device stop flush callback
>
> If it's ok with you, I'll resubmit these two patches separately. The sw implementation has some unresolved race conditions with the scheduler service that may take some time to fix, but this shouldn't block the first patch. That patch needs a fix too (v4 includes a now-incorrect comment about stop not draining events from ports), so I'll fix and resubmit that.
OK. So that we can merge this common code and octeontx changes.
How about changing the following comment rte_event_dev_stop()
exiting:
+ * This function does not drain events from event ports; the
application is responsible for flushing events from all ports before stopping the
device.
proposed:(Just to share the view)
# You can add the fact that, all events across the ports will show up in
callback as we concluded.
#Application may invoke stop operation on the event adapter to
avoid event being generated.(or something similar) i.e adapter needs to
stopped before stopping the eventdev to avoid continuous looping on
getting the events(over rte_event_dev_register_callback()) from event source(ethdev Rx adapter).
>
> Thanks,
> Gage
>
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, April 2, 2018 3:03 AM
> > To: Eads, Gage <gage.eads@intel.com>
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org;
> > hemant.agrawal@nxp.com; Richardson, Bruce <bruce.richardson@intel.com>;
> > santosh.shukla@caviumnetworks.com; nipun.gupta@nxp.com
> > Subject: Re: [PATCH v4 2/2] event/sw: support device stop flush callback
> >
> > > > [Update: Ah I see you do this below already. Still, better check
> > > > twice I think, the data path isn't running here anyway in case
> > > > future me decides to call
> > > > sw_flush_iq() without performing the check]
> > >
> > > Agreed, will fix in v5.
> >
> > Gage,
> >
> > Could please send the v5 so that I can include it in RC1 pull request.
> >
>
@@ -362,8 +362,25 @@ sw_init_qid_iqs(struct sw_evdev *sw)
}
static void
-sw_clean_qid_iqs(struct sw_evdev *sw)
+sw_flush_iq(struct rte_eventdev *dev, struct sw_iq *iq)
{
+ struct sw_evdev *sw = sw_pmd_priv(dev);
+
+ while (iq_count(iq) > 0) {
+ struct rte_event event;
+
+ iq_dequeue_burst(sw, iq, &event, 1);
+
+ dev->dev_ops->dev_stop_flush(dev->data->dev_id,
+ event,
+ dev->data->dev_stop_flush_arg);
+ }
+}
+
+static void
+sw_clean_qid_iqs(struct rte_eventdev *dev)
+{
+ struct sw_evdev *sw = sw_pmd_priv(dev);
int i, j;
/* Release the IQ memory of all configured qids */
@@ -373,7 +390,11 @@ sw_clean_qid_iqs(struct sw_evdev *sw)
for (j = 0; j < SW_IQS_MAX; j++) {
if (!qid->iq[j].head)
continue;
+
+ if (dev->dev_ops->dev_stop_flush)
+ sw_flush_iq(dev, &qid->iq[j]);
iq_free_chunk_list(sw, qid->iq[j].head);
+
qid->iq[j].head = NULL;
}
}
@@ -702,7 +723,7 @@ static void
sw_stop(struct rte_eventdev *dev)
{
struct sw_evdev *sw = sw_pmd_priv(dev);
- sw_clean_qid_iqs(sw);
+ sw_clean_qid_iqs(dev);
sw_xstats_uninit(sw);
sw->started = 0;
rte_smp_wmb();
@@ -28,6 +28,7 @@
#define MAX_PORTS 16
#define MAX_QIDS 16
#define NUM_PACKETS (1<<18)
+#define DEQUEUE_DEPTH 128
static int evdev;
@@ -147,7 +148,7 @@ init(struct test *t, int nb_queues, int nb_ports)
.nb_event_ports = nb_ports,
.nb_event_queue_flows = 1024,
.nb_events_limit = 4096,
- .nb_event_port_dequeue_depth = 128,
+ .nb_event_port_dequeue_depth = DEQUEUE_DEPTH,
.nb_event_port_enqueue_depth = 128,
};
int ret;
@@ -2807,6 +2808,77 @@ holb(struct test *t) /* test to check we avoid basic head-of-line blocking */
return -1;
}
+static void
+flush(uint8_t dev_id __rte_unused, struct rte_event event, void *arg)
+{
+ *((uint8_t *) arg) += (event.u64 == 0xCA11BACC) ? 1 : 0;
+}
+
+static int
+dev_stop_flush(struct test *t) /* test to check we can properly flush events */
+{
+ const struct rte_event new_ev = {
+ .op = RTE_EVENT_OP_NEW,
+ .u64 = 0xCA11BACC
+ /* all other fields zero */
+ };
+ struct rte_event ev = new_ev;
+ uint8_t count = 0;
+ int i;
+
+ if (init(t, 1, 1) < 0 ||
+ create_ports(t, 1) < 0 ||
+ create_atomic_qids(t, 1) < 0) {
+ printf("%d: Error initializing device\n", __LINE__);
+ return -1;
+ }
+
+ /* Link the queue so *_start() doesn't error out */
+ if (rte_event_port_link(evdev, t->port[0], NULL, NULL, 0) != 1) {
+ printf("%d: Error linking queue to port\n", __LINE__);
+ goto err;
+ }
+
+ if (rte_event_dev_start(evdev) < 0) {
+ printf("%d: Error with start call\n", __LINE__);
+ goto err;
+ }
+
+ for (i = 0; i < DEQUEUE_DEPTH + 1; i++) {
+ if (rte_event_enqueue_burst(evdev, t->port[0], &ev, 1) != 1) {
+ printf("%d: Error enqueuing events\n", __LINE__);
+ goto err;
+ }
+ }
+
+ /* Schedule the events from the port to the IQ. At least one event
+ * should be remaining in the queue.
+ */
+ rte_service_run_iter_on_app_lcore(t->service_id, 1);
+
+ if (rte_event_dev_stop_flush_callback_register(evdev, flush, &count)) {
+ printf("%d: Error installing the flush callback\n", __LINE__);
+ goto err;
+ }
+
+ cleanup(t);
+
+ if (count == 0) {
+ printf("%d: Error executing the flush callback\n", __LINE__);
+ goto err;
+ }
+
+ if (rte_event_dev_stop_flush_callback_register(evdev, NULL, NULL)) {
+ printf("%d: Error uninstalling the flush callback\n", __LINE__);
+ goto err;
+ }
+
+ return 0;
+err:
+ rte_event_dev_dump(evdev, stdout);
+ cleanup(t);
+ return -1;
+}
static int
worker_loopback_worker_fn(void *arg)
{
@@ -3211,6 +3283,12 @@ test_sw_eventdev(void)
printf("ERROR - Head-of-line-blocking test FAILED.\n");
goto test_fail;
}
+ printf("*** Running Stop Flush test...\n");
+ ret = dev_stop_flush(t);
+ if (ret != 0) {
+ printf("ERROR - Stop Flush test FAILED.\n");
+ goto test_fail;
+ }
if (rte_lcore_count() >= 3) {
printf("*** Running Worker loopback test...\n");
ret = worker_loopback(t, 0);