Re: [dpdk-dev] Documenting eventdev reconfiguration behavior

["Eads, Gage" <gage.eads@intel.com>]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Thursday, January 11, 2018 1:30 AM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: dev@dpdk.org; santosh.shukla@caviumnetworks.com; Hemant Agrawal
> <hemant.agrawal@nxp.com>; nipun.gupta@nxp.com; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: Documenting eventdev reconfiguration behavior
> 
> -----Original Message-----
> > Date: Tue, 9 Jan 2018 17:00:16 +0000
> > From: "Eads, Gage" <gage.eads@intel.com>
> > To: "dev@dpdk.org" <dev@dpdk.org>
> > CC: "jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>,
> > "santosh.shukla@caviumnetworks.com"
> > <santosh.shukla@caviumnetworks.com>,
> >  Hemant Agrawal <hemant.agrawal@nxp.com>, "nipun.gupta@nxp.com"
> >  <nipun.gupta@nxp.com>, "Van Haaren, Harry"
> > <harry.van.haaren@intel.com>,  "Richardson, Bruce"
> > <bruce.richardson@intel.com>
> > Subject: Documenting eventdev reconfiguration behavior
> >
> > Hi folks,
> >
> > As mentioned in a previous thread*, the eventdev documentation is unclear on
> what the user can expect when a device is stopped, possibly re-configured, and
> restarted. The documentation states that:
> >
> > * If the application wants to change the configuration (i.e. call
> > * rte_event_dev_configure(), rte_event_queue_setup(), or
> > * rte_event_port_setup()), it must call rte_event_dev_stop() first to
> > stop the
> > * device and then do the reconfiguration before calling
> > rte_event_dev_start()
> > * again. The schedule, enqueue and dequeue functions should not be
> > invoked
> > * when the device is stopped.
> >
> > And:
> >
> > /**
> > * Stop an event device. The device can be restarted with a call to
> > * rte_event_dev_start()
> > *
> > * @param dev_id
> > *   Event device identifier.
> > */
> > void
> > rte_event_dev_stop(uint8_t dev_id);
> >
> > Specifically, the documentation is unclear on whether events that are
> > queued in the device when rte_event_dev_stop() is called will remain
> > there after rte_event_dev_start()? And does this depend on whether
> > rte_event_dev_configure() or rte_event_queue_setup() are called?
> > (There are probably other aspects that need clarification, but this is
> > a starting point.)
> >
> > Hopefully we can agree on a common behavior, or else perhaps capabilities
> flags are in order. I propose we take the (in my view) simplest approach, that
> eventdevs will not preserve events when a device is stopped and restarted. (Thus
> to avoid memory leaks, the application is responsible for flushing events out of
> the device before stopping it.) This is simple to support in the sw PMD, and I
> would expect this to be simpler for hardware devices as well.
> >
> > Thoughts?
> 
> Currently in octeontx driver, we are flushing the events on stop().
> I agree the application should be involved in flushing the events as the events
> are opaque.
> On the same time, if we application needs to flush the events, it will translate to
> 1) Some sort of sync up between fastpath and control path as stop() typically will
> be called on control path
> 2) Some HW may have separate operation for flush in addition to just call the
> dequeue() in loop
> 
> Considering the above points, How about, # Registering a callback to eventdev,
> on stop(), driver will flush the events one by one, by invoking the application
> specific callback routine _and then_ call driver specific flush.
> 
> Thoughts?
> 

To reiterate, when rte_event_dev_stop() is called the eventdev will drop any inflight events. But before doing so it will execute a user callback fn, which is a mechanism intended for the user to properly dispose of the inflight events. That seems reasonable.

Since (presumably) the enqueue and dequeue functions won't be modified to return an error if called when the device is stopped (stopping an eventdev should be the uncommon case, and speed is the higher priority design point than usability/debuggability), we should at least emphasize that any workers that continue to enqueue/dequeue once the device is stopped will result in undefined behavior (I'm thinking in the rte_event_dev_stop() documentation).

If there are no counter-proposals, I'll submit a patch for this in the coming weeks.

Thanks!
Gage