Re: net/virtio: duplicated xstats

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 11/27/2023 10:19 AM, Ferruh Yigit wrote:
> On 11/24/2023 9:18 AM, Edwin Brossette wrote:
>> Hello,
>>
>> I noticed a small inconsistency in the virtio pmd's xstats.
>> The stat "rx_q0_errors" appears twice.
>> I also think the stats "rx_q0_packets", "rx_q0_bytes", "tx_q0_packets"
>> and "tx_q0_bytes" are duplicates of "rx_q0_good_packets",
>> "rx_q0_good_bytes", "tx_q0_good_packets" and "tx_q0_good_bytes"
>>
> 
> Ack, I didn't reproduce but from code I can see this inconsistency can
> occur.
> 
> 
>> I believe this issue probably appeared after this commit:
>>
>> f30e69b41f94: ethdev: add device flag to bypass auto-filled queue xstats
>> http://scm.6wind.com/vendor/dpdk.org/dpdk/commit/?id=f30e69b41f949cd4a9afb6ff39de196e661708e2 <http://scm.6wind.com/vendor/dpdk.org/dpdk/commit/?id=f30e69b41f949cd4a9afb6ff39de196e661708e2>
>>
>> From what I understand, the rxq0_error stat was originally reported by
>> the librte. However, changes were made so it is reported by the pmd instead.
>> The flag RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS was temporarily set to keep
>> the old behaviour so that every pmd could have time to adapt the change.
>> But it seems the flag was forgotten in the virtio pmd and as a result,
>> some stats are fetched at two different times when displaying xstats.
>>
> 
> Motivation is move queue stats from "basic stats" to "xstats".
> 
> 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS' is a temporary flag, as you said,
> and when driver sets it, ethdev layer copies queue stats from basic
> stats to xstats.
> In long term expectation is driver itself implement queue stats in
> xstats and unset the flag, when all drivers implement this, flag will be
> removed.
> 
> 
> The issue with virtio is it already has queue stats in the xstats, with
> the flag some queue stats duplicated.
> It seems it is not forgotten to be removed, but it was mistake to add
> flag at first place. So OK to remove the flag from virtio PMD.
> 

Fixed Maxime's email.