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 > > > > > 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. > > >
