On Tue, Jun 03, 2014 at 02:33:16PM +0000, Richardson, Bruce wrote: > > > > -----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. :-) > If you're planning on collapsing the flush routine into an iterative call to distributor_process anyway, then, sure, I'd appreciate it.
Thanks! Neil
