Re: [dpdk-dev] [PATCH v2] net/pcap: imissed stats support

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

On 2/1/2021 8:30 AM, Ido Goshen wrote:
> Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> ---
> v2:
> * sum all queues (rx_missed_total += fix)
> * null pcap protection
> * inter stop/start persistancy (counter won't reset on stop)
> 
>   drivers/net/pcap/rte_eth_pcap.c | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index a32b1f3f3..18c59d61c 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -58,6 +58,8 @@ struct queue_stat {
>   	volatile unsigned long pkts;
>   	volatile unsigned long bytes;
>   	volatile unsigned long err_pkts;
> +	volatile unsigned long missed_reset;
> +	volatile unsigned long missed_mnemonic;

Can you please put some comments why 'missed_mnemonic' is required?

<...>

> @@ -695,6 +715,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>   		stats->q_ibytes[i] = internal->rx_queue[i].rx_stat.bytes;
>   		rx_packets_total += stats->q_ipackets[i];
>   		rx_bytes_total += stats->q_ibytes[i];
> +		unsigned long rx_missed = eth_pcap_stats_missed_get(dev, i) +
> +				internal->rx_queue[i].rx_stat.missed_mnemonic -
> +				internal->rx_queue[i].rx_stat.missed_reset;


Instead of including the 'missed_mnemonic' to the regular calculation, what do 
you think to save the 'imissed' value to 'missed_mnemonic' in 'port_stop' and 
load it back in the 'eth_dev_start'?
This balanced usage can simplify the code I think.