On 08/23/2016 11:04 AM, Yang, Zhiyong wrote: > Hi, Panu: > >> -----Original Message----- >> From: Panu Matilainen [mailto:pmatilai at redhat.com] >> Sent: Monday, August 22, 2016 3:53 PM >> To: Yang, Zhiyong <zhiyong.yang at intel.com>; dev at dpdk.org >> Subject: Re: [dpdk-dev] [PATCH] vhost: add pmd xstats >> >> On 08/19/2016 03:16 PM, Zhiyong Yang wrote: >>> This feature adds vhost pmd extended statistics from per queue >>> perspective for the application such as OVS etc. >>> >>> The statistics counters are based on RFC 2819 and 2863 as follows: >>> >>> rx/tx_good_packets >>> rx/tx_total_bytes >>> rx/tx_dropped_pkts >>> rx/tx_broadcast_packets >>> rx/tx_multicast_packets >>> rx/tx_ucast_packets >>> rx/tx_undersize_errors >>> rx/tx_size_64_packets >>> rx/tx_size_65_to_127_packets; >>> rx/tx_size_128_to_255_packets; >>> rx/tx_size_256_to_511_packets; >>> rx/tx_size_512_to_1023_packets; >>> rx/tx_size_1024_to_1522_packets; >>> rx/tx_1523_to_max_packets; >>> rx/tx_errors >>> rx_fragmented_errors >>> rx_jabber_errors >>> rx_unknown_protos_packets; >>> >>> No API is changed or added. >>> rte_eth_xstats_get_names() to retrieve what kinds of vhost xstats are >>> supported, >>> rte_eth_xstats_get() to retrieve vhost extended statistics, >>> rte_eth_xstats_reset() to reset vhost extended statistics. >>> >>> 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. - Panu - > > -zhiyong- >