[dpdk-dev] [PATCH v2 2/5] distributor: new packet distributor library

Richardson, Bruce bruce.richardson at intel.com
Tue Jun 3 16:33:16 CEST 2014



> -----Original Message-----
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Tuesday, June 03, 2014 4:01 AM
> To: Richardson, Bruce
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/5] distributor: new packet distributor library
> 
> On Mon, Jun 02, 2014 at 09:40:04PM +0000, Richardson, Bruce wrote:
> >
> >
> > > -----Original Message-----
> > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > Sent: Thursday, May 29, 2014 6:48 AM
> > > To: Richardson, Bruce
> > > Cc: dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 2/5] distributor: new packet distributor
> library
> > >
> > > > +
> > > > +/* flush the distributor, so that there are no outstanding packets in flight
> or
> > > > + * queued up. */
> > > Its not clear to me that this is a distributor only function.  You modified the
> > > comments to indicate that lcores can't preform double duty as both a worker
> > > and
> > > a distributor, which is fine, but it implies that there is a clear distinction
> > > between functions that are 'worker' functions and 'distributor' functions.
> > > While its for the most part clear-ish (workers call rte_distributor_get_pkt and
> > > rte_distibutor_return_pkt, distibutors calls rte_distributor_create/process.
> > > This is in a grey area.  the analogy I'm thinking of here are kernel
> workqueues.
> > > Theres a specific workqueue thread that processes the workqueue, but any
> > > process
> > > can sync or flush the workqueue, leading me to think this process can be
> called
> > > by a worker lcore.
> >
> > I can update comments here further, but I was hoping the way things were
> right now was clear enough. In the header and C files, I have the functions
> explicitly split up into distributor and worker function sets, with a big block of
> text in the header at the start of each section explaining the threading use of the
> follow functions.
> >
> Very well, we can let use be the determinant here.  We can leave it as is, and
> if reports of lockups come in, we can change it, otherwise no harm done.
> 
Since I'm not a big fan of the "let's wait for the lock-ups" approach, I'll add in a single-line addition to each function's doxygen comment that should make its way into the official API docs. :-)


More information about the dev mailing list