DPDK patches and discussions
 help / color / mirror / Atom feed
From: Suanming.Mou <mousuanming@huawei.com>
To: "Varghese, Vipin" <vipin.varghese@intel.com>,
	"Pattan, Reshma" <reshma.pattan@intel.com>
Cc: "'dev@dpdk.org'" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] app/pdump: exits once primary app exited
Date: Fri, 26 Apr 2019 20:08:25 +0800	[thread overview]
Message-ID: <ee413010-8b73-c1e0-90b7-98586af2f94b@huawei.com> (raw)
Message-ID: <20190426120825.KQJpA-WPdacUWgr7MXT3NZ5gnAWJHR88iQ9kGBXQ-sk@z> (raw)
In-Reply-To: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D33E2E3@BGSMSX101.gar.corp.intel.com>


On 2019/4/26 18:56, Varghese, Vipin wrote:
>
> I will leave this suggestion open for comments from the maintainer.
>
Hi,

Thanks for your suggestion. I have also tried to add an slave core to 
monitor the primary status this afternoon.  It works.

I doubt if it can be add an new option as you suggested, but which will 
also require people who complain the exiting to add an extra slave core 
for that.

Please waiting for the new patch in one or two days.

> snipped
>
> Hi,
>
> Looks like something in email format setting is affecting the style. 
> Please find my replies below
>
> snipped
>
>     As per the current suggested code flow check is added to while loop in function `dump_packets'.
>
> Thanks for the reply. Since want to make it clean, the code was here.
> However, it seems need to take care of the performance impact first.
> Response> thanks for acknowledging the same.
>
>     Questions:
>
>     1. What is impact in performance with and without patch?
>
> A1. Do a little trick as the patch below to tested the impact in the single core mode on Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz with no pkts.
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index 3d208548fa13..804011b187c4 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -141,7 +141,7 @@ struct parse_val {
>   
>   static int num_tuples;
>   static struct rte_eth_conf port_conf_default;
> -static volatile uint8_t quit_signal;
> +static volatile uint32_t quit_signal;
>   static uint8_t multiple_core_capture;
>   
>   /**< display usage */
> @@ -868,6 +868,7 @@ struct parse_val {
>   dump_packets(void)
>   {
>         int i;
> +      uint64_t start, end;
>         uint32_t lcore_id = 0;
>   
>         if (!multiple_core_capture) {
> @@ -880,10 +881,20 @@ struct parse_val {
>                                pdump_t[i].device_id,
>                                pdump_t[i].queue);
>   
> -              while (!quit_signal) {
> +              /* make it hot */
> +              rte_eal_primary_proc_alive(NULL);
> +              rte_eal_primary_proc_alive(NULL)
> +
> +              start = rte_rdtsc();
> +              while (quit_signal < 50000) {
> +                     /* Just testing with and w/o the 'if' line below */
> +                     if (rte_eal_primary_proc_alive(NULL))
> +                             quit_signal++;
>                        for (i = 0; i < num_tuples; i++)
>                                pdump_packets(&pdump_t[i]);
>                 }
> +              end = rte_rdtsc();
> +              printf("Totally count:%u, cost tsc:%lu\n", quit_signal, end - start);
>   
>                 return;
>         }
> The total tsc cost is about 338809671 with rte_eal_primary_proc_alive().
> And the tsc cost is just about 513573 without rte_eal_primary_proc_alive().
> The dpdk-pdump had also used taskset to bind to specify isolate core.
> So it seems the patch do a great performance impact.
> Response> thanks for confirming the suspicion.
> Maybe another async method should be introduced to monitor the primary status.
> Response> yes, without affecting the capture thread.
>
>     2. For various packet sizes and port speed what are delta in drops for packet capture?
>
> A2. Refer to A1, it's not needed anymore.
> Response> A1 there is performance impact.
> Note: If pdump application is still alive when primary is not running, primary cannot be started. Is this a cue that pdump is still alive and has to be terminated?
> Yes, some guys complained that the residual dpdk-pdump impact the restart of the primary app and refuse to add other mechanisms e.g. to kill the dpdk-pdump in the app to avoid that case.
> So the patch was created.
> Is there any other ways to avoid that.
> Response> in my humble opinion, best way around is add user option 
> like ‘--exit’; which then will add periodic rte_timer for user desired 
> seconds ‘0.1, 0.5, 1.0, 5.0’. The timer callback can run on master 
> core which sets ‘quit_signal’ once primary is no longer alive. In case 
> of ‘multi thread’ capture master thread is not involved in 
> dump_packets thus avoiding any packet drops or performance issue..

  parent reply	other threads:[~2019-04-26 12:09 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-25 16:35 Suanming.Mou
2019-04-25 15:51 ` Varghese, Vipin
2019-04-25 15:51   ` Varghese, Vipin
2019-04-26  7:11   ` Suanming.Mou
2019-04-26  7:11     ` Suanming.Mou
2019-04-26 10:54     ` Varghese, Vipin
2019-04-26 10:54       ` Varghese, Vipin
2019-04-26 10:56       ` Varghese, Vipin
2019-04-26 10:56         ` Varghese, Vipin
2019-04-26 12:08         ` Suanming.Mou [this message]
2019-04-26 12:08           ` Suanming.Mou
2019-04-26 13:46           ` Burakov, Anatoly
2019-04-26 13:46             ` Burakov, Anatoly
2019-04-26 14:32             ` Suanming.Mou
2019-04-26 14:32               ` Suanming.Mou
2019-04-26 14:39               ` Burakov, Anatoly
2019-04-26 14:39                 ` Burakov, Anatoly
2019-04-26 14:49                 ` Suanming.Mou
2019-04-26 14:49                   ` Suanming.Mou
2019-04-26 14:50                   ` Burakov, Anatoly
2019-04-26 14:50                     ` Burakov, Anatoly
2019-04-25 16:35 ` Suanming.Mou
2019-04-28  4:58 ` [dpdk-dev] [PATCH v2] app/pdump: add exit_with_primary option support Suanming.Mou
2019-04-28  4:58   ` Suanming.Mou
2019-04-28  5:07   ` [dpdk-dev] [PATCH v3] " Suanming.Mou
2019-04-28  5:07     ` Suanming.Mou
2019-04-30  3:39     ` [dpdk-dev] [PATCH v4] app/pdump: add pudmp exits with primary support Suanming.Mou
2019-04-30  2:33       ` Varghese, Vipin
2019-04-30  2:33         ` Varghese, Vipin
2019-04-30  3:43         ` Suanming.Mou
2019-04-30  3:43           ` Suanming.Mou
2019-04-30  5:03           ` Varghese, Vipin
2019-04-30  5:03             ` Varghese, Vipin
2019-04-30  9:34         ` Burakov, Anatoly
2019-04-30  9:34           ` Burakov, Anatoly
2019-04-30 10:37           ` Varghese, Vipin
2019-04-30 10:37             ` Varghese, Vipin
2019-04-30 16:38             ` Burakov, Anatoly
2019-04-30 16:38               ` Burakov, Anatoly
2019-04-30  3:39       ` Suanming.Mou
2019-04-30 11:35       ` [dpdk-dev] [PATCH v5] Make pdump exits with primary Suanming.Mou
2019-04-30 11:35         ` Suanming.Mou
2019-04-30 11:35         ` [dpdk-dev] [PATCH v5] app/pdump: add pudmp exits with primary support Suanming.Mou
2019-04-30  9:42           ` Burakov, Anatoly
2019-04-30  9:42             ` Burakov, Anatoly
2019-04-30 11:25             ` Suanming.Mou
2019-04-30 11:25               ` Suanming.Mou
2019-04-30 16:39               ` Burakov, Anatoly
2019-04-30 16:39                 ` Burakov, Anatoly
2019-05-02  3:07                 ` Suanming.Mou
2019-05-02  3:07                   ` Suanming.Mou
2019-04-30 11:35           ` Suanming.Mou
2019-04-30 12:44           ` Pattan, Reshma
2019-04-30 12:44             ` Pattan, Reshma
2019-05-02  5:20           ` [dpdk-dev] [PATCH v6] " Suanming.Mou
2019-05-02  5:20             ` Suanming.Mou
2019-05-02  8:04             ` Varghese, Vipin
2019-05-02  8:04               ` Varghese, Vipin
2019-05-02  8:32               ` Suanming.Mou
2019-05-02  8:32                 ` Suanming.Mou
2019-05-02  9:12                 ` Burakov, Anatoly
2019-05-02  9:12                   ` Burakov, Anatoly
2019-05-02  9:22                 ` Varghese, Vipin
2019-05-02  9:22                   ` Varghese, Vipin
2019-05-02  9:54             ` Pattan, Reshma
2019-05-02  9:54               ` Pattan, Reshma
2019-05-02 10:40               ` Suanming.Mou
2019-05-02 10:40                 ` Suanming.Mou
2019-05-02 12:35             ` [dpdk-dev] [PATCH v7] " Suanming.Mou
2019-05-02 11:03               ` Pattan, Reshma
2019-05-02 11:03                 ` Pattan, Reshma
2019-05-02 11:31               ` Burakov, Anatoly
2019-05-02 11:31                 ` Burakov, Anatoly
2019-05-02 12:35                 ` mousuanming
2019-05-02 12:35                   ` mousuanming
2019-05-02 12:35               ` Suanming.Mou
2019-05-03  5:48               ` [dpdk-dev] [PATCH v8] " Suanming.Mou
2019-05-03  5:48                 ` Suanming.Mou
2019-05-04 21:17                 ` Thomas Monjalon
2019-05-04 21:17                   ` Thomas Monjalon
2019-05-05  1:20                   ` Suanming.Mou
2019-05-05  1:20                     ` Suanming.Mou
2019-05-05  9:42                     ` Thomas Monjalon
2019-05-05  9:42                       ` Thomas Monjalon
2019-05-05 11:13                       ` Suanming.Mou
2019-05-05 11:13                         ` Suanming.Mou
2019-05-08  8:04                 ` Thomas Monjalon
2019-05-08  8:04                   ` Thomas Monjalon
2019-05-08  9:37                   ` Suanming.Mou
2019-05-08  9:37                     ` Suanming.Mou
2019-05-08 10:22                     ` Thomas Monjalon
2019-05-08 10:22                       ` Thomas Monjalon
2019-05-08 13:14                       ` Suanming.Mou
2019-05-08 13:14                         ` Suanming.Mou
2019-05-15  5:10                 ` [dpdk-dev] [PATCH v9] app/pdump: exit with primary process Suanming.Mou
2019-05-15  5:10                   ` Suanming.Mou
2019-06-20 12:32                   ` Pattan, Reshma
2019-06-23 22:30                     ` Thomas Monjalon
2019-07-10 14:04                       ` Suanming Mou
2019-07-10 22:28                         ` Thomas Monjalon
2019-04-29  9:14   ` [dpdk-dev] [PATCH v2] app/pdump: add exit_with_primary option support Burakov, Anatoly
2019-04-29  9:14     ` Burakov, Anatoly
2019-04-29  9:43     ` Suanming.Mou
2019-04-29  9:43       ` Suanming.Mou
2019-04-29 10:42       ` Burakov, Anatoly
2019-04-29 10:42         ` Burakov, Anatoly

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=ee413010-8b73-c1e0-90b7-98586af2f94b@huawei.com \
    --to=mousuanming@huawei.com \
    --cc=dev@dpdk.org \
    --cc=reshma.pattan@intel.com \
    --cc=vipin.varghese@intel.com \
    /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).