From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>,
"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>,
"Van Haaren, Harry" <harry.van.haaren@intel.com>
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
Date: Wed, 25 Jul 2018 13:34:40 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB977258DF51D668@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891268E779810@IRSMSX107.ger.corp.intel.com>
> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Wednesday, July 25, 2018 1:41 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay Haimovsky <motih@mellanox.com>; 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; Van Haaren, Harry <harry.van.haaren@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
>
>
>
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Wednesday, July 25, 2018 1:18 PM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; 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; Van
> > Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> >
> >
> >
> > > -----Original Message-----
> > > From: Dumitrescu, Cristian
> > > Sent: Wednesday, July 25, 2018 12:57 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mordechay
> > Haimovsky <motih@mellanox.com>; 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; Van Haaren, Harry
> > <harry.van.haaren@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using ctrl+d
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Wednesday, July 25, 2018 12:39 PM
> > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mordechay
> > > > Haimovsky <motih@mellanox.com>; 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;
> > Van
> > > > Haaren, Harry <harry.van.haaren@intel.com>
> > > > 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 <konstantin.ananyev@intel.com>; Mordechay
> > > > Haimovsky <motih@mellanox.com>; 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
> > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > ctrl+d
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ananyev, Konstantin
> > > > > > Sent: Wednesday, July 25, 2018 10:04 AM
> > > > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> > Mordechay
> > > > > > Haimovsky <motih@mellanox.com>; 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
> > > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd exit using
> > > > 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 <motih@mellanox.com>; 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
> > > > > > > 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 <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.
> > > > > >
> > > > > > 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) the
> > cores he
> > > > owns :)
> > > > What I am saying: there is a special thing (developed by Harry) service
> > 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.
>
> 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 time
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 be changed.
Though yes there are drawbacks: softnic would rely on presence of service core,
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 service).
Another thought - could it be registered as alarm handler, so it could be called from
DPDK interrupt thread?
Konstantin
>
> > I'd better live without softnic support in testpmd :)
>
> Thanks a lot for your help ;(
>
> > Though I still don't understand why do you feel that service cores are not
> > good enough for you?
>
> You don't seem to realize there is an issue that we need to root cause and hopefully fix rather than hiding it. This has nothing to do
> with service core.
>
> > 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.
>
> Service core could be used here, as long as a new core is not wasted. Running this on the same core as CLI is also a valid solution.
>
> > Konstantin
> >
> > >
> > >
> > > > >
> > > > > We did use the non-blocking cmdline_poll() function instead of the
> > > > blocking cmdline_interact() in the past without any issues. The
> > > > > issues reported by Moti come as a surprise. It is probably good to 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_poll()
> > 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 library. It
> > > > would
> > > > > > also prevent us from testing some Soft NIC functionality.
next prev parent reply other threads:[~2018-07-25 13:35 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
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 [this message]
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=2601191342CEEE43887BDE71AB977258DF51D668@irsmsx105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=bernard.iremonger@intel.com \
--cc=cristian.dumitrescu@intel.com \
--cc=dev@dpdk.org \
--cc=harry.van.haaren@intel.com \
--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).