[dpdk-dev] [Linuxarm] Re: [PATCH V2 04/14] net/hns3: add Rx and Tx bytes stats

[oulijun]

在 2021/3/3 22:24, Ferruh Yigit 写道:
> On 3/3/2021 2:08 PM, oulijun wrote:
>>
>>
>> 在 2021/3/3 21:28, Ferruh Yigit 写道:
>>> On 3/2/2021 1:58 PM, Lijun Ou wrote:
>>>> From: "Min Hu (Connor)" <humin29 at huawei.com>
>>>>
>>>> In current HNS3 PMD, Rx/Tx bytes from packet stats are not
>>>> implemented.
>>>>
>>>> This patch implemented Rx/Tx bytes using soft counters.
>>>> Rx/Tx bytes stats will be enabled if the macro
>>>> RTE_LIBRTE_HNS3_PMD_SOFT_COUNTERS is defined.
>>>>
>>>> Signed-off-by: Min Hu (Connor) <humin29 at huawei.com>
>>>> Signed-off-by: Lijun Ou <oulijun at huawei.com>
>>>> ---
>>>>   drivers/net/hns3/hns3_rxtx.c          | 24 ++++++++++++++++++++++++
>>>>   drivers/net/hns3/hns3_rxtx_vec_neon.h | 15 +++++++++++++++
>>>>   drivers/net/hns3/hns3_rxtx_vec_sve.c  | 11 +++++++++++
>>>>   drivers/net/hns3/hns3_stats.c         | 22 ++++++++++++++++++----
>>>>   4 files changed, 68 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/hns3/hns3_rxtx.c 
>>>> b/drivers/net/hns3/hns3_rxtx.c
>>>> index 5e79177..a8bd2cc 100644
>>>> --- a/drivers/net/hns3/hns3_rxtx.c
>>>> +++ b/drivers/net/hns3/hns3_rxtx.c
>>>> @@ -2181,6 +2181,10 @@ hns3_recv_pkts(void *rx_queue, struct 
>>>> rte_mbuf **rx_pkts, uint16_t nb_pkts)
>>>>                              cksum_err);
>>>>           hns3_rxd_to_vlan_tci(rxq, rxm, l234_info, &rxd);
>>>> +#ifdef RTE_LIBRTE_HNS3_PMD_SOFT_COUNTERS
>>>> +        /* Increment bytes counter  */
>>>> +        rxq->basic_stats.bytes += rxm->pkt_len;
>>>> +#endif
>>>
>>> copy/paste from previous version:
>>>
>>> Why statistics enabled only with macro?
>>> It is not common to use macro to enable the stats, what do you think 
>>> to remove it, to be consistent with rest of the PMDs?
>> I'm sorry. I thought it was a success.
>>   Firstly, the macro is used to control the statistics to ensure 
>> performance and facilitate flexible usage. For example, the macro 
>> needs to be disabled when high performance is required.
>> secondly the byte statistics of other vendors are implemented by 
>> reading and writing registers. Therefore, macros are not used.By the 
>> way, the MLX driver has a precedent (code snippets can be intercepted 
>> here).
> 
> It is not convenient for a user re-compile the DPDK to be able to get 
> the ethdev byte statistics, stats are not developer/debug information, 
> end user may need them. And this recompilation may not be an option for 
> the distributed software.
> 
> How much performance affect it has if you enable it always?
> 
We theoretically analyzed that being on a critical path might have a 
performance impact. Maybe the actual mountain is negligible.
> And just to double check, isn't there a way to get this information from 
> HW without calculating it in the driver?
> 
OK
> The compile time flags, specially after meson switch, hard to enable and 
> a little hidden, it is very easy to have broken code there, that is why 
> it better to prevent compile time flags as much as possible.
> .
After internal analysis, you can accept your comments and remove macros.
I will fix it.
>