From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
To: Mordechay Haimovsky <motih@mellanox.com>,
Thomas Monjalon <thomas@monjalon.net>,
"Singh, Jasvinder" <jasvinder.singh@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Iremonger, Bernard" <bernard.iremonger@intel.com>,
"Pattan, Reshma" <reshma.pattan@intel.com>,
"olivier.matz@6wind.com" <olivier.matz@6wind.com>
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
Date: Tue, 24 Jul 2018 16:59:28 +0000 [thread overview]
Message-ID: <3EB4FA525960D640B5BDFFD6A3D891268E7792AF@IRSMSX107.ger.corp.intel.com> (raw)
In-Reply-To: <AM0PR05MB4435909C7E4E524D3A92FA02D2550@AM0PR05MB4435.eurprd05.prod.outlook.com>
> -----Original Message-----
> From: Mordechay Haimovsky [mailto:motih@mellanox.com]
> Sent: Tuesday, July 24, 2018 3:37 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; olivier.matz@6wind.com;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
>
> Even after this fix we still have setups that use netvsc for example, on
> which testpmd exits with rte_panic right after loading it even 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 from PMD-specific
> implementation.
>
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Tuesday, July 24, 2018 3:34 PM
> > To: Jasvinder Singh <jasvinder.singh@intel.com>
> > Cc: dev@dpdk.org; bernard.iremonger@intel.com;
> > reshma.pattan@intel.com; Mordechay Haimovsky
> <motih@mellanox.com>;
> > 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-Transmission
> > > > character (EOT) on the empty command line.
> > >
> > > Please describe what is the issue and what is the cause.
> > >
> > > > Fixes: 0ad778b398c6 ("app/testpmd: rework softnic forward mode")
> > > >
> > > > Reported-by: Mordechay Haimovsky <motih@mellanox.com>
> > > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > > ---
> > > > 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 separate fix.
> >
> >
First, testpmd is not really broken, as only thing that changed 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 replacing 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 testpmd, 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 things on the same core, e.g. the slow softnic_manage() function.
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 library. It would also prevent us from testing some Soft NIC functionality.
next prev parent reply other threads:[~2018-07-24 16:59 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-23 10:44 Jasvinder Singh
2018-07-24 9:25 ` Iremonger, Bernard
2018-07-24 11:25 ` Thomas Monjalon
2018-07-24 12:33 ` Thomas Monjalon
2018-07-24 12:39 ` Pattan, Reshma
2018-07-24 12:41 ` Iremonger, Bernard
2018-07-24 13:39 ` Thomas Monjalon
2018-07-24 14:37 ` Mordechay Haimovsky
2018-07-24 16:59 ` Dumitrescu, Cristian [this message]
2018-07-25 8:18 ` Thomas Monjalon
2018-07-25 8:30 ` Singh, Jasvinder
2018-07-25 8:49 ` Dumitrescu, Cristian
2018-07-25 8:53 ` Mordechay Haimovsky
2018-07-25 9:04 ` Ananyev, Konstantin
2018-07-25 9:36 ` Dumitrescu, Cristian
2018-07-25 11:39 ` Ananyev, Konstantin
2018-07-25 11:55 ` Mordechay Haimovsky
2018-07-25 11:56 ` Dumitrescu, Cristian
2018-07-25 12:18 ` Ananyev, Konstantin
2018-07-25 12:27 ` Van Haaren, Harry
2018-07-25 13:21 ` Dumitrescu, Cristian
2018-07-25 12:41 ` Dumitrescu, Cristian
2018-07-25 13:34 ` Ananyev, Konstantin
2018-07-24 14:17 ` [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd exit for ctrl+d Reshma Pattan
2018-07-24 14:46 ` Pattan, Reshma
2018-07-24 15:31 ` Pattan, Reshma
2018-07-24 15:31 ` Iremonger, Bernard
2018-07-26 14:14 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3EB4FA525960D640B5BDFFD6A3D891268E7792AF@IRSMSX107.ger.corp.intel.com \
--to=cristian.dumitrescu@intel.com \
--cc=bernard.iremonger@intel.com \
--cc=dev@dpdk.org \
--cc=jasvinder.singh@intel.com \
--cc=motih@mellanox.com \
--cc=olivier.matz@6wind.com \
--cc=reshma.pattan@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).