From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 6A3B43B5 for ; Mon, 6 Oct 2014 19:27:31 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 06 Oct 2014 10:25:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,664,1406617200"; d="scan'208";a="601336571" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga001.fm.intel.com with ESMTP; 06 Oct 2014 10:34:37 -0700 Received: from irsmsx154.ger.corp.intel.com (163.33.192.96) by IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 6 Oct 2014 18:34:14 +0100 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.253]) by IRSMSX154.ger.corp.intel.com ([169.254.12.124]) with mapi id 14.03.0195.001; Mon, 6 Oct 2014 18:34:13 +0100 From: "Pattan, Reshma" To: Neil Horman Thread-Topic: [dpdk-dev] [PATCH v3] distributor_app: new sample app Thread-Index: AQHP3Jrg0aT67DPn+0CmG3jigM5MIJwZexeAgAAMNwCAABbFAIABs1bw///0VQCAAAuBAIAACHUAgAfH1lD///ylgIAAQA5g Date: Mon, 6 Oct 2014 17:34:13 +0000 Message-ID: <3AEA2BF9852C6F48A459DA490692831FE21865@IRSMSX109.ger.corp.intel.com> References: <1411568210-2555-1-git-send-email-reshma.pattan@intel.com> <1412073577-12248-1-git-send-email-reshma.pattan@intel.com> <20140930113445.GB2193@hmsreliant.think-freely.org> <20140930121828.GA9312@BRICHA3-MOBL> <20140930133958.GG2193@hmsreliant.think-freely.org> <3AEA2BF9852C6F48A459DA490692831FE20FB6@IRSMSX109.ger.corp.intel.com> <20141001145620.GB24028@localhost.localdomain> <20141001153730.GA9292@BRICHA3-MOBL> <20141001160746.GF24028@localhost.localdomain> <3AEA2BF9852C6F48A459DA490692831FE2177E@IRSMSX109.ger.corp.intel.com> <20141006144448.GB22304@hmsreliant.think-freely.org> In-Reply-To: <20141006144448.GB22304@hmsreliant.think-freely.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v3] distributor_app: new sample app X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Oct 2014 17:27:32 -0000 > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Monday, October 6, 2014 3:45 PM > To: Pattan, Reshma > Cc: dev@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@tuxdriver.com] > > > Sent: Wednesday, October 1, 2014 5:08 PM > > > To: Richardson, Bruce > > > Cc: Pattan, Reshma; dev@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 in= coming > 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 wh= ile loop > which is conditionally based on packet indexing runs in a continuous loop > without breaking and rx thread will not proceed with next statement execu= tion > 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. N= ot 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 yo= u 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 ex= it. 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 i= t. > 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_distributor= e_process to get to completion? = 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 doi= ng 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 t= he 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 sen= der 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 s= ole use of the intended recipient(s). Any review or distribution by others = is strictly prohibited. If you are not the intended recipient, please conta= ct the sender and delete all copies.