From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id CF08A1C01 for ; Wed, 25 Jul 2018 15:35:02 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Jul 2018 06:35:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,401,1526367600"; d="scan'208";a="70209330" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by fmsmga002.fm.intel.com with ESMTP; 25 Jul 2018 06:34:43 -0700 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by IRSMSX103.ger.corp.intel.com (163.33.3.157) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 25 Jul 2018 14:34:41 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.195]) by irsmsx155.ger.corp.intel.com ([169.254.14.181]) with mapi id 14.03.0319.002; Wed, 25 Jul 2018 14:34:40 +0100 From: "Ananyev, Konstantin" To: "Dumitrescu, Cristian" , "Mordechay Haimovsky" , Thomas Monjalon , "Singh, Jasvinder" CC: "dev@dpdk.org" , "Iremonger, Bernard" , "Pattan, Reshma" , "olivier.matz@6wind.com" , "Van Haaren, Harry" Thread-Topic: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d Thread-Index: AQHUInJOpj9peW6IqE+E2Z0dzzEFa6SeLF2AgAATHYCAACKJAIAAJ7IAgAEd51D///iJAIAAMZYQ///1yACAABRUsP//+AmAAAN317A= Date: Wed, 25 Jul 2018 13:34:40 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258DF51D668@irsmsx105.ger.corp.intel.com> References: <20180723104425.10090-1-jasvinder.singh@intel.com> <1876510.4y0gDTZx5Q@xps> <1925999.Zqez9Xlb98@xps> <3EB4FA525960D640B5BDFFD6A3D891268E7792AF@IRSMSX107.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258DF51D3C1@irsmsx105.ger.corp.intel.com> <3EB4FA525960D640B5BDFFD6A3D891268E779722@IRSMSX107.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258DF51D5A2@irsmsx105.ger.corp.intel.com> <3EB4FA525960D640B5BDFFD6A3D891268E7797DD@IRSMSX107.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258DF51D5D3@irsmsx105.ger.corp.intel.com> <3EB4FA525960D640B5BDFFD6A3D891268E779810@IRSMSX107.ger.corp.intel.com> In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891268E779810@IRSMSX107.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTI0ZGQyMmMtMjE2OS00NjRjLTllNDEtYTI3Yzg0NzRlMjJkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicHRYRGRmeExBK2FadEV1Rm1tUmZcL1liRkF5ck44SURoUmFsSnMrck53cFdkN0tuWURcLzVyY1hrOHNlNDJkRWcxIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jul 2018 13:35:03 -0000 > -----Original Message----- > From: Dumitrescu, Cristian > Sent: Wednesday, July 25, 2018 1:41 PM > To: Ananyev, Konstantin ; Mordechay Haimovs= ky ; Thomas Monjalon > ; Singh, Jasvinder > Cc: dev@dpdk.org; Iremonger, Bernard ; Patta= n, Reshma ; > olivier.matz@6wind.com; Van Haaren, Harry > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+= d >=20 >=20 >=20 > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Wednesday, July 25, 2018 1:18 PM > > To: Dumitrescu, Cristian ; Mordechay > > Haimovsky ; Thomas Monjalon > > ; Singh, Jasvinder > > Cc: dev@dpdk.org; Iremonger, Bernard ; > > Pattan, Reshma ; olivier.matz@6wind.com; Van > > Haaren, Harry > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctr= l+d > > > > > > > > > -----Original Message----- > > > From: Dumitrescu, Cristian > > > Sent: Wednesday, July 25, 2018 12:57 PM > > > To: Ananyev, Konstantin ; Mordechay > > Haimovsky ; Thomas Monjalon > > > ; Singh, Jasvinder > > > Cc: dev@dpdk.org; Iremonger, Bernard ; > > Pattan, Reshma ; > > > olivier.matz@6wind.com; Van Haaren, Harry > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using c= trl+d > > > > > > > > > > > > > -----Original Message----- > > > > From: Ananyev, Konstantin > > > > Sent: Wednesday, July 25, 2018 12:39 PM > > > > To: Dumitrescu, Cristian ; Mordechay > > > > Haimovsky ; Thomas Monjalon > > > > ; Singh, Jasvinder > > > > Cc: dev@dpdk.org; Iremonger, Bernard ; > > > > Pattan, Reshma ; olivier.matz@6wind.com; > > Van > > > > Haaren, Harry > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using > > ctrl+d > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Dumitrescu, Cristian > > > > > Sent: Wednesday, July 25, 2018 10:36 AM > > > > > To: Ananyev, Konstantin ; Mordechay > > > > Haimovsky ; Thomas Monjalon > > > > > ; Singh, Jasvinder > > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard > > ; > > > > Pattan, Reshma ; > > > > > olivier.matz@6wind.com > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit usi= ng > > ctrl+d > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Ananyev, Konstantin > > > > > > Sent: Wednesday, July 25, 2018 10:04 AM > > > > > > To: Dumitrescu, Cristian ; > > Mordechay > > > > > > Haimovsky ; Thomas Monjalon > > > > > > ; Singh, Jasvinder > > > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard > > ; > > > > > > Pattan, Reshma ; > > olivier.matz@6wind.com > > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit u= sing > > > > ctrl+d > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of > > Dumitrescu, > > > > > > Cristian > > > > > > > Sent: Tuesday, July 24, 2018 5:59 PM > > > > > > > To: Mordechay Haimovsky ; Thomas > > Monjalon > > > > > > ; Singh, Jasvinder > > > > > > > > > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard > > > > ; > > > > > > Pattan, Reshma ; > > > > > > > olivier.matz@6wind.com > > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit > > using > > > > ctrl+d > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Mordechay Haimovsky [mailto:motih@mellanox.com] > > > > > > > > Sent: Tuesday, July 24, 2018 3:37 PM > > > > > > > > To: Thomas Monjalon ; Singh, Jasvinder > > > > > > > > > > > > > > > > Cc: dev@dpdk.org; Iremonger, Bernard > > > > ; > > > > > > > > Pattan, Reshma ; > > > > olivier.matz@6wind.com; > > > > > > > > Dumitrescu, Cristian > > > > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd ex= it > > using > > > > > > ctrl+d > > > > > > > > > > > > > > > > Even after this fix we still have setups that use netvsc f= or > > example, > > > > on > > > > > > > > which testpmd exits with rte_panic right after loading it e= ven > > > > without > > > > > > > > touching the KBD. > > > > > > > > > > > > > > > > I recommend returning the previous prompt routine in test- > > > > > > pmd/cmdline.c > > > > > > > > and rework the SOFTNIC section there, preferably moving its= poll > > > > section > > > > > > to > > > > > > > > use rte_service in a separate file cleaning the CLI files f= rom PMD- > > > > specific > > > > > > > > implementation. > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > > > > > > Sent: Tuesday, July 24, 2018 3:34 PM > > > > > > > > > To: Jasvinder Singh > > > > > > > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com; > > > > > > > > > reshma.pattan@intel.com; Mordechay Haimovsky > > > > > > > > ; > > > > > > > > > olivier.matz@6wind.com; cristian.dumitrescu@intel.com > > > > > > > > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd = exit > > > > using > > > > > > ctrl+d > > > > > > > > > > > > > > > > > > Important note: > > > > > > > > > testpmd is currently really broken. > > > > > > > > > We cannot have a RC2 until it is fixed. > > > > > > > > > > > > > > > > > > > > > > > > > > > 24/07/2018 13:25, Thomas Monjalon: > > > > > > > > > > 23/07/2018 12:44, Jasvinder Singh: > > > > > > > > > > > Fix testpmd app exit by pressing ctrl+d, End-of-Trans= mission > > > > > > > > > > > character (EOT) on the empty command line. > > > > > > > > > > > > > > > > > > > > Please describe what is the issue and what is the cause= . > > > > > > > > > > > > > > > > > > > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic for= ward > > > > > > mode") > > > > > > > > > > > > > > > > > > > > > > Reported-by: Mordechay Haimovsky > > > > > > > > > > > > > Signed-off-by: Jasvinder Singh > > > > > > > > > > > --- > > > > > > > > > > > app/test-pmd/cmdline.c | 10 ++++++---- > > > > > > > > > > > lib/librte_cmdline/cmdline.c | 2 +- > > > > > > > > > > > > > > > > > > > > It is very suspicious to change the cmdline library. > > > > > > > > > > If there is a bug in this library, it deserves a separa= te fix. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > First, testpmd is not really broken, as only thing that chang= ed is the > > Ctrl > > > > + > > > > > > D behavior. I agree this is an issue that we need to fix, as > > > > > > > it looks that it is breaking some automation scripts for some > > people. > > > > > > > > > > > > > > The change in behavior for Ctrl + D exit is caused by replaci= ng the > > call > > > > to > > > > > > cmdline_interact() with calling cmdline_poll() in a loop. > > > > > > > These two approaches should be identical in behavior, but it = looks > > like > > > > they > > > > > > are not due to some issue in the cmdline library. > > > > > > > Jasvinder proposed a quick patch, but it looks like something= else > > needs > > > > to > > > > > > be fixed in cmdline library in order to bring > > > > > > > cmdline_poll() on parity with cmdline_interact(). Any advice = from > > > > Olivier > > > > > > would be very much appreciated! > > > > > > > > > > > > > > It is really a bad practice to use cmdline_interact() in test= pmd, as it > > is a > > > > > > blocking call that prohibits doing other things on the same > > > > > > > thread that runs the CLI. Sometimes we need to run other thin= gs on > > the > > > > > > same core, e.g. the slow softnic_manage() function. > > > > > > > > > > > > Curious why not use a service core for softnic background stuff= , and > > leave > > > > CLI > > > > > > one for CLI? > > > > > > Konstantin > > > > > > > > > > I guess for a test application you can do anything you want, but = in real > > life > > > > CPU cores are really expensive and dedicating a CPU core > > > > > just for CLI is a colossal waste. > > > > > > > > Ok, but let's application developer to decide how to use (waste) th= e > > cores he > > > > owns :) > > > > What I am saying: there is a special thing (developed by Harry) ser= vice > > cores. > > > > As I understand why of it's the purpose - allow PMD(s) to allocate = CPU > > > > resources for > > > > there background tasks in a unified and transparent way. > > > > From the description above - that seems to fit your needs (softnic > > > > background processing), no? > > > > Konstantin > > > > > > > > > > Then why not put the testpmd CLI itself on a service core? Are you > > volunteering for a patch on this? :) > > > > It was not me who broke testpmd at first place. >=20 > I don't think you understand the issue. The issue is related to using the= cmdline library in non-blocking mode, it has nothing to do > with Soft NIC. Yep, I understand that. What I am trying to say: you introduce rte_pmd_softnic_manage(), and, as I understand, it is a user responsibility to call it from time to t= ime to keep softnic working correctly. That's ok, but it means that now any app that wants to use softnic has to be modified, correct? While if you put that into service core - only startup EAL cmdline has to b= e changed.=20 Though yes there are drawbacks: softnic would rely on presence of service c= ore, and if there is no other services registered - it would probably be a waste= of cycles. But it probably could be made configurable (does user want to call softnic_manage(), or is it ok to run it as a servic= e).=20 Another thought - could it be registered as alarm handler, so it could be c= alled from DPDK interrupt thread? Konstantin >=20 > > I'd better live without softnic support in testpmd :) >=20 > Thanks a lot for your help ;( >=20 > > Though I still don't understand why do you feel that service cores are = not > > good enough for you? >=20 > You don't seem to realize there is an issue that we need to root cause an= d hopefully fix rather than hiding it. This has nothing to do > with service core. >=20 > > From my understanding they were introduced for similar purposes > > (Harry please feel free to correct me here). > > So if you think they not fit for your case - > > at least would be good to understand why. >=20 > Service core could be used here, as long as a new core is not wasted. Run= ning this on the same core as CLI is also a valid solution. >=20 > > Konstantin > > > > > > > > > > > > > > > > > > We did use the non-blocking cmdline_poll() function instead of th= e > > > > blocking cmdline_interact() in the past without any issues. The > > > > > issues reported by Moti come as a surprise. It is probably good t= o see > > what > > > > this is about and see if we can quickly fix the issue in > > > > > cmdline library. Otherwise, we can revert the usage of cmdline_po= ll() > > with > > > > cmdline_interact(). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Worst case scenario: We can revert the cmdline_poll() back to > > > > > > cmdline_interact(), this is a small change, but not the proper = way of > > > > > > > doing things, as this is simply hiding the issue in cmdline l= ibrary. It > > > > would > > > > > > also prevent us from testing some Soft NIC functionality.