[dpdk-dev] [PATCH v2 0/6] bonding: locks

[Iremonger, Bernard]

Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, June 10, 2016 3:46 PM
> To: Iremonger, Bernard <bernard.iremonger at intel.com>
> Cc: dev at dpdk.org; Doherty, Declan <declan.doherty at intel.com>; Ananyev,
> Konstantin <konstantin.ananyev at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 0/6] bonding: locks
> 
> On Thu, May 26, 2016 at 05:38:41PM +0100, Bernard Iremonger wrote:
> > Add spinlock to bonding rx and tx queues.
> > Take spinlock in rx and tx burst functions.
> > Take all spinlocks in slave add and remove functions.
> > With spinlocks in place remove memcpy of slaves.
> >
> > Changes in v2:
> > Replace patch 1.
> > Add patch 2 and reorder patches.
> > Add spinlock to bonding rx and tx queues.
> > Take all spinlocks in slave add and remove functions.
> > Replace readlocks with spinlocks.
> >
> > Bernard Iremonger (6):
> >   bonding: add spinlock to rx and tx queues
> >   bonding: grab queue spinlocks in slave add and remove
> >   bonding: take queue spinlock in rx/tx burst functions
> >   bonding: add spinlock to stop function
> >   bonding: add spinlock to link update function
> >   bonding: remove memcpy from burst functions
> >
> >  drivers/net/bonding/rte_eth_bond_api.c     |  52 +++++++-
> >  drivers/net/bonding/rte_eth_bond_pmd.c     | 196
> ++++++++++++++++++-----------
> >  drivers/net/bonding/rte_eth_bond_private.h |   4 +-
> >  3 files changed, 173 insertions(+), 79 deletions(-)
> >
> > --
> 
> The patches in this set are missing any explanation for the reasons behind
> each patch. The commit messages are empty for every patch.
> 
> I'm also concerned at the fact that this patchset is adding lock operations all
> over the bonding PMD. While other PMDs need synchronization between
> control plane and data plane threads so that e.g. you don't do IO on a
> stopped port, they don't use locks so as to get max performance. Nowhere
> in the patchset is there an explanation as to why bonding is so different that
> it needs locks where other PMDs can do without them. This should also be
> explained in each individual patch as to why the area covered by the patch
> needs locks in this PMD (again, compared to other PMDs)
> 
> Thanks,
> /Bruce

I will be sending a v3 for this patchset.
The empty commit messages were an oversight on my part, this will be corrected in the v3.
I will also try to explain why the locks are needed.

Regards,

Bernard.