Re: [dpdk-dev] [PATCH] vhost: add pmd xstats

[Panu Matilainen <pmatilai@redhat.com>]

On 08/23/2016 11:04 AM, Yang, Zhiyong wrote:
> Hi,  Panu:
>
>> -----Original Message-----
>> From: Panu Matilainen [mailto:pmatilai@redhat.com]
>> Sent: Monday, August 22, 2016 3:53 PM
>> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@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-
>