From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 368111518 for ; Wed, 24 Aug 2016 07:36:12 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 23 Aug 2016 22:36:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,569,1464678000"; d="scan'208";a="1030396765" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga001.fm.intel.com with ESMTP; 23 Aug 2016 22:36:10 -0700 Date: Wed, 24 Aug 2016 13:46:02 +0800 From: Yuanhan Liu To: Panu Matilainen Cc: "Yang, Zhiyong" , "dev@dpdk.org" Message-ID: <20160824054602.GS30752@yliu-dev.sh.intel.com> References: <1471608966-39077-1-git-send-email-zhiyong.yang@intel.com> <08c5733b-6758-2d98-df4a-a2261ecf43e4@redhat.com> <45bf611c-fbd5-d485-566a-5752cd58ed7c@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <45bf611c-fbd5-d485-566a-5752cd58ed7c@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] vhost: add pmd xstats X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Aug 2016 05:36:12 -0000 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