Re: [PATCH] doc: update TAP device features

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

On 10/9/2024 5:53 PM, Stephen Hemminger wrote:
> On Sun, 6 Oct 2024 21:36:31 +0100
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> 
>> On 10/4/2024 9:48 PM, Stephen Hemminger wrote:
>>> On Fri, 4 Oct 2024 02:48:21 +0100
>>> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>>   
>>>>> +Stats per queue      = Y
>>>>>    
>>>>
>>>> This feature name is misleading,
>>>> it is for 'rte_eth_dev_set_[rt]x_queue_stats_mapping()' API, which is
>>>> indeed for covering limitation for some drivers.
>>>> Tap does support getting stats per queue, but doesn't support above
>>>> documented feature.  
>>>
>>> Just looked and the current state of the feature matrix is not correct
>>> for this part of the table. Although the feature intro says it is
>>> stats per queue mapping, that is not what other drivers are doing.
>>>
>>> Only these drivers have queue_stats_mapping_set:
>>>   cnxk, enic, igc, ixgbe, txgbe, virtio
>>>
>>> These drivers claim to have 'Stats per queue' but do not have stats mapping:
>>>   af_xdp, ark, atlantic, avp, bnxt, cxgbe, failsafe, fm10, hinic, hns3,
>>>   ionic, mlx4, mlx5, netvsc, nfb, nfp, ngbe, qede, thunderx, vmxnet3
>>>
>>> So the overwhelming practice is that the features.rst description is wrong!
>>>  
>>
>> You are right.
>>
>> What about update the 'features.rst', and extend the definition of
>> 'Stats per queue' mapping + queue stats in basic stats or extended stats.
>> This will be make name of the feature more intuitive.
> 
> Like this?
> 
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index cd0115ffb3..a351d7b3b5 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -727,8 +727,6 @@ Basic stats
>  Support basic statistics such as: ipackets, opackets, ibytes, obytes,
>  imissed, ierrors, oerrors, rx_nombuf.
>  
> -And per queue stats: q_ipackets, q_opackets, q_ibytes, q_obytes, q_errors.
> -
>  These apply to all drivers.
>  
>  * **[implements] eth_dev_ops**: ``stats_get``, ``stats_reset``.
> @@ -754,7 +752,8 @@ Supports Extended Statistics, changes from driver to driver.
>  Stats per queue
>  ---------------
>  
> -Supports configuring per-queue stat counter mapping.
> +Supports per queue stats: q_ipackets, q_opackets, q_ibytes, q_obytes, q_errors.
> +May also support configuring per-queue stat counter mapping.
>  
>  * **[implements] eth_dev_ops**: ``queue_stats_mapping_set``.
>  * **[related]    API**: ``rte_eth_dev_set_rx_queue_stats_mapping()``,
>

Yes, plus 'implements' & 'related' fields needs to be extended.