All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: Panu Matilainen <pmatilai@redhat.com>
Cc: "Yang, Zhiyong" <zhiyong.yang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH] vhost: add pmd xstats
Date: Wed, 24 Aug 2016 13:46:02 +0800	[thread overview]
Message-ID: <20160824054602.GS30752@yliu-dev.sh.intel.com> (raw)
In-Reply-To: <45bf611c-fbd5-d485-566a-5752cd58ed7c@redhat.com>

On Tue, Aug 23, 2016 at 12:45:54PM +0300, Panu Matilainen wrote:
> >>>Since collecting data of vhost_update_packet_xstats will have some
> >>>effect on RX/TX performance, so, Setting compiling switch
> >>>CONFIG_RTE_LIBRTE_PMD_VHOST_UPDATE_XSTATS=n by default in the
> >>file
> >>>config/common_base, if needing xstats data, you can enable it(y).
> >>
> >>NAK, such things need to be switchable at run-time.
> >>
> >>	- Panu -
> >
> >Considering the following reasons using the compiler switch, not
> >command-line at run-time.
> >
> >1.Similar xstats update functions are always collecting stats data in the
> >background when rx/tx are running, such as the physical NIC or virtio,
> >which have no switch. Compiler switch for vhost pmd xstats is added
> >as a option when performance is viewed as critical factor.
> >
> >2. No data structure and API in any layer support the xstats update switch
> >at run-time. Common data structure (struct rte_eth_dev_data) has no
> >device-specific data member, if implementing enable/disable of vhost_update
> >_packet_xstats at run-time, must define a flag(device-specific) in it,
> >because the definition of struct vhost_queue in the driver code
> >(eth_vhost_rx/eth_vhost_tx processing)is not visible from device perspective.
> >
> >3. I tested RX/TX with v1 patch (y) as reference based on Intel(R)
> >Xeon(R) CPU E5-2699 v3 @ 2.30GHz, for 64byts packets in burst mode, 32 packets
> >in one RX/TX processing. Overhead of vhost_update_packet_xstats is less than
> >3% for the rx/tx processing. It looks that vhost_update_packet_xstats has a
> >limited effect on performance drop.
> 
> Well, either the performance overhead is acceptable and it should always be
> on (like with physical NICs I think). Or it is not. In which case it needs
> to be turnable on and off, at run-time. Rebuilding is not an option in the
> world of distros.

I think the less than 3% overhead is acceptable here, that I agree with
Panu we should always keep it on. If someone compains it later that even
3% is too big for them, let's consider to make it be switchable at
run-time. Either we could introduce a generic eth API for that, Or
just introduce a vhost one if that doesn't make too much sense to other
eth drivers.

	--yliu

  reply	other threads:[~2016-08-24  5:36 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-19 12:16 [PATCH] vhost: add pmd xstats Zhiyong Yang
2016-08-22  7:52 ` Panu Matilainen
2016-08-23  8:04   ` Yang, Zhiyong
2016-08-23  9:45     ` Panu Matilainen
2016-08-24  5:46       ` Yuanhan Liu [this message]
2016-08-24  8:44         ` Thomas Monjalon
2016-08-24 12:37           ` Panu Matilainen
2016-08-25  9:21             ` Yang, Zhiyong
2016-08-30  2:45               ` Yao, Lei A
2016-08-30  3:03                 ` Xu, Qian Q
2016-08-30  3:21                   ` Yao, Lei A
2016-08-31  7:18                     ` Yang, Zhiyong
2016-09-01  6:37                       ` Yang, Zhiyong
2016-09-09  8:15 ` [PATCH v2] net/vhost: " Zhiyong Yang
2016-09-09  8:40   ` Van Haaren, Harry
2016-09-09  8:54     ` Yang, Zhiyong
2016-09-14  6:20   ` Yuanhan Liu
2016-09-14  7:43     ` Yang, Zhiyong
2016-09-18 13:16       ` Yuanhan Liu
2016-09-19  2:48         ` Yang, Zhiyong
2016-09-14  8:30     ` Yang, Zhiyong
2016-09-20  9:36   ` [PATCH v3 0/2] net/vhost: add pmd xstats support Zhiyong Yang
2016-09-20  9:36     ` [PATCH v3 1/2] net/vhost: add a new stats struct Zhiyong Yang
2016-09-20 10:44       ` Yuanhan Liu
2016-09-21  5:12         ` Yang, Zhiyong
2016-09-21 10:05       ` [PATCH v4 0/2] net/vhost: add pmd xstats support Zhiyong Yang
2016-09-21 10:05         ` Zhiyong Yang
2016-09-21 10:05         ` [PATCH v4 2/2] net/vhost: add pmd xstats Zhiyong Yang
2016-09-21 10:57           ` Yuanhan Liu
2016-09-22  1:42             ` Yang, Zhiyong
2016-09-22  2:09               ` Yuanhan Liu
2016-09-21 10:13       ` [PATCH v4 1/2] net/vhost: add a new defined stats struct Zhiyong Yang
2016-09-22  8:19         ` [PATCH v5 0/2] net/vhost: add pmd xstats support Zhiyong Yang
2016-09-22  8:19           ` [PATCH v5 1/2] net/vhost: add a new defined stats struct Zhiyong Yang
2016-09-28  8:33             ` [PATCH v6 0/2] net/vhost: add pmd xstats support Zhiyong Yang
2016-09-28  8:33               ` [PATCH v6 1/2] net/vhost: add a new defined stats struct Zhiyong Yang
2016-09-28 13:26                 ` [PATCH v7 0/2] net/vhost: add pmd xstats support Zhiyong Yang
2016-09-28 13:26                   ` [PATCH v7 1/2] net/vhost: add a new defined stats struct Zhiyong Yang
2016-09-29  1:55                     ` Yuanhan Liu
2016-09-29 12:35                     ` [PATCH v8 0/2] net/vhost: add pmd xstats support Zhiyong Yang
2016-09-29 12:35                       ` [PATCH v8 1/2] net/vhost: add a new defined stats struct Zhiyong Yang
2016-09-29 12:35                       ` [PATCH v8 2/2] net/vhost: add pmd xstats Zhiyong Yang
2016-09-29 13:04                         ` Yuanhan Liu
2016-09-29 13:50                           ` Yang, Zhiyong
2016-09-28 13:26                   ` [PATCH v7 " Zhiyong Yang
2016-09-29  8:48                     ` Loftus, Ciara
2016-09-29 12:02                       ` Yuanhan Liu
2016-09-29 12:22                         ` Yang, Zhiyong
2016-09-28  8:33               ` [PATCH v6 " Zhiyong Yang
2016-09-22  8:19           ` [PATCH v5 " Zhiyong Yang
2016-09-23  3:56           ` [PATCH v5 0/2] net/vhost: add pmd xstats support Yuanhan Liu
2016-09-28  2:35             ` Yuanhan Liu
2016-09-20  9:36     ` [PATCH v3 2/2] net/vhost: add pmd xstats Zhiyong Yang
2016-09-20 10:56       ` Yuanhan Liu
2016-09-21  7:22         ` Yang, Zhiyong
2016-09-20 11:50       ` Yuanhan Liu
2016-09-21  6:15         ` Yang, Zhiyong

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=20160824054602.GS30752@yliu-dev.sh.intel.com \
    --to=yuanhan.liu@linux.intel.com \
    --cc=dev@dpdk.org \
    --cc=pmatilai@redhat.com \
    --cc=zhiyong.yang@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.