Re: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Dec 23, 2014 at 08:09:37AM -0500, Neil Horman wrote:
> On Tue, Dec 23, 2014 at 09:28:08AM +0000, Bruce Richardson wrote:
> > On Mon, Dec 22, 2014 at 12:47:09PM -0500, Neil Horman wrote:
> > > On Mon, Dec 22, 2014 at 05:33:07PM +0000, Bruce Richardson wrote:
> > > > On Mon, Dec 22, 2014 at 06:02:53PM +0100, Thomas Monjalon wrote:
> > > > > Hi Bruce,
> > > > > 
> > > > > Callbacks, as hooks for applications, give more flexibility and are
> > > > > generally a good idea.
> > > > > In DPDK the main issue will be to avoid performance degradation.
> > > > > I see you use "unlikely" for callback branching.
> > > > > Could we reduce more the impact of this test by removing the queue array,
> > > > > i.e. having port-wide callbacks instead of per-queue callbacks?
> > > > 
> > > > I can give that a try, but I don't see it making much difference if any. The
> > > > main thing to avoid with branching is branch mis-prediction, which should not
> > > > be a problem here, as the user is not going to be adding or removing callbacks
> > > > between each RX and TX call, making the branches highly predictable - i.e. always
> > > > go the same way.
> > > I was going to ask about exactly that.  You say no one will be adding/removing
> > > callbacks between RX/TX calls, but you don't know that, people will try to do so
> > > at some point.  You should add a check so that callbacks can only be
> > > registered/unregistered on stopped queues, otherwise this is extreemely racy.
> > > it won't impact performance to do so, and will save a good deal of debugging
> > > down the road at some point.
> > > 
> > > Neil
> > > 
> > Actually, I think it's worthwhile being able to do exactly that - add/remove
> > callbacks on the fly, if possible. Doing the add in a race-free manner is probably
> > easy enough, but doing the delete may well be more tricky. For now, though,
> > it might indeed be as well to limit it to stopped queues. 
> > If we can do the dynamic add/remove of callbacks, then that can lead to all
> > sorts of interesting runtime instrumentation possibilities using multiprocess
> > support.
> > 
> > /Bruce
> > 
> 
> Well, ok, but if you want to be able to do dynamic addition/removal of
> callbacks, then you either need to stop the queue, or provide locking around the
> list traversal and modification points.  All you have right now is multiple
> contexts hitting the list at the same time, and thats going to break.
> 
> Neil
> 
Yep, well aware of that. It's still a work in progress, hence the RFC part. :-)
I'll finish off the basics first, and then see what can be done in terms of
either thread safety or just restricting usage to when queues are stopped.
More polished patches hopefully to follow in the new year.

/Bruce