On Mon, Oct 06, 2014 at 05:34:13PM +0000, Pattan, Reshma wrote: > > > > -----Original Message----- > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > Sent: Monday, October 6, 2014 3:45 PM > > To: Pattan, Reshma > > Cc: dev at dpdk.org; Richardson, Bruce > > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > > > On Mon, Oct 06, 2014 at 02:16:22PM +0000, Pattan, Reshma wrote: > > > > > > > > > > -----Original Message----- > > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > > Sent: Wednesday, October 1, 2014 5:08 PM > > > > To: Richardson, Bruce > > > > Cc: Pattan, Reshma; dev at dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app > > > > > > > > > > > > > > > > > > 1)I had sent v5 patch which handles graceful shutdown of rx > > > > > > > and tx threads upon SIGINT > > > > > > I see it and will take a look shortly, thanks. > > > > > > > > > > > > > 2)Worker thread graceful shutdown was not handled as of now as > > > > > > > it needs > > > > some change in lcore_worker logic , which will be done in future > > enhancements. > > > > > > Not sure I understand what you mean here. Can you elaborate? > > > > > > > > > rte_distributor_process which runs as part of rx thread will process > > > incoming > > packets and checks for any requests for the packets from worker threads . > > > If request is seen, it adds the packet/work to particular workers back > > > log and > > proceed with processing of next packet. > > > If no request seen the packet index will not be incremented and the > > > while loop > > which is conditionally based on packet indexing runs in a continuous loop > > without breaking and rx thread will not proceed with next statement > > execution > > until unless rte_distributor_process comes out of while loop. > > > This issue happens only when we enable graceful shutdown logic for both > > rx/worker threads, as workers threads gets killed and no request seen by rx > > thread and it stucks. > > > Hence as of now graceful shutdown logic is provided only for rx thread. > > > For > > worker threads will check what can be done in next enhancements. > > > > > > Thanks, > > > Reshma > > > > > > > I see what you're saying, Once you make a call to rte_distributor_get_pkt, > > you > > have no way to gracefully shut down use of the rte_distributor_library. Not > > just > > this application, but any application. Thats just not sane, and suggests > > that we > > integrated the rte_distributor library too soon. I would suggest that you > > prefix > > this patch with an update to the rte distributor library to allow > > rte_distributor_get_pkt and friends to return NULL if the queue is emtpy. > > Applications should be checking the return value for NULL anyway, and can > > preform the rte_pause operation. Then update this patch to do a clean > > exit. To > > say that "we will check what can be done in the next enhancements" is to say > > that this won't be addressed again until a paying custmoer gripes about it. > > Neil > > > > Hi Neil, > > We have rte_distributor_request_pkt and rte_distributor_poll_pkt() in > dpdk.org, which can be used together(in place of rte_distributor_get_pkt) > to check empty queue condition I believe. So no separate changes needed. > Though we replace to rte_distributor_get_pkt with above two, the issue will > not be solved as the thread that gets blocked is rx thread but not worker > thread. > Rx thread gets blocked in rte_distributore_process due to non-availability of > requests from worker. > How about sending dummy request_pkts upon SIGINT and allow > rte_distributore_process to get to completion? > I suppose creating a flagged dummy packet to ensure that the worker threads can exit their spin loops works fine, yes.
Neil > Thanks, > Reshma > > > > > > > > 3)Freeing of mempool is also not handled , as the framework > > > > > > > support is not > > > > available. > > > > > > Ew, I hadn't noticed that, freeing of mempools seems like > > > > > > something we should implement. > > > > > > > > > > > > > 4)Cleaning of rx/tx queues not done, as it needs some > > > > > > > extensive logic > > > > which we haven't planned as of now. Will check the possibility of doing > > > > it in > > > > future enhancements i.e in next version of sample application. > > > > > > We can't just flush the queues after we shutdown the workers? I > > > > > > presume a queue flush operation exists, yes? > > > > > > Neil > > > > > > > > > > Other than code hygiene, which does have some value in itself, I > > > > > can't really see what the practical point of such cleanup would be. > > > > > > > > > This is really the only assertion I'm trying to make. I understand > > > > this application won't suffer from exiting uncleanly, and that makes > > > > the need for preforming cleanup little more than overhead. > > > > > > > > But that said, hygine is exactly the point I'm driving at here. > > > > These are example applications, that presumably people look at when > > > > writing their own apps. If you don't do things properly, people > > > > looking at your code are less likely to do them as well. Even if it > > > > doesn't hurt for you to exit uncleanly, it will hurt someone, and if > > > > they look to these examples as a source of best practices, it seems > > > > to me that it would be in everyones interest, if best practices were > > demonstrated. > > > > > > > > Neil > > > > > > -------------------------------------------------------------- > > > Intel Shannon Limited > > > Registered in Ireland > > > Registered Office: Collinstown Industrial Park, Leixlip, County > > > Kildare Registered Number: 308263 Business address: Dromore House, > > > East Park, Shannon, Co. Clare > > > > > > This e-mail and any attachments may contain confidential material for the > > > sole > > use of the intended recipient(s). Any review or distribution by others is > > strictly > > prohibited. If you are not the intended recipient, please contact the > > sender and > > delete all copies. > > > > > > > > > > -------------------------------------------------------------- > Intel Shannon Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare > Registered Number: 308263 > Business address: Dromore House, East Park, Shannon, Co. Clare > > This e-mail and any attachments may contain confidential material for the > sole use of the intended recipient(s). Any review or distribution by others > is strictly prohibited. If you are not the intended recipient, please contact > the sender and delete all copies. > > >
