2016-08-24 13:46, Yuanhan Liu: > 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.
+1 It may have sense to introduce a generic run-time option for stats.
