Re: [PATCH v20 17/25] net/pcap: avoid use of volatile

Bruce Richardson
2026-03-16T14:31:55Z

On Tue, Mar 10, 2026 at 09:09:55AM -0700, Stephen Hemminger wrote:
> Using volatile for statistics is not necessary since only one
> thread is allowed to operate on a queue at a time.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Well only one queue is allowed to change them, but other threads could read
them, so using volatile is correct in that it tells the compiler not to
cache the values in registers or on the stack. Technically the compiler
could have the stats stored locally somewhere between updates, but in
practice I don't think that situation could ever arise for stats. Therefore
I'm ok with stats being either volatile or not, I doubt the resulting code
will be any different.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> drivers/net/pcap/pcap_ethdev.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
> index 8be3a59690..20e4b8e6aa 100644
> --- a/drivers/net/pcap/pcap_ethdev.c
> +++ b/drivers/net/pcap/pcap_ethdev.c
> @@ -54,10 +54,10 @@ static uint64_t timestamp_rx_dynflag;
> static int timestamp_dynfield_offset = -1;
>
> struct queue_stat {
> - volatile unsigned long pkts;
> - volatile unsigned long bytes;
> - volatile unsigned long err_pkts;
> - volatile unsigned long rx_nombuf;
> + uint64_t pkts;
> + uint64_t bytes;
> + uint64_t err_pkts;
> + uint64_t rx_nombuf;
> };
>
> struct queue_missed_stat {
> @@ -901,11 +901,11 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
> struct eth_queue_stats *qstats)
> {
> unsigned int i;
> - unsigned long rx_packets_total = 0, rx_bytes_total = 0;
> - unsigned long rx_missed_total = 0;
> - unsigned long rx_nombuf_total = 0, rx_err_total = 0;
> - unsigned long tx_packets_total = 0, tx_bytes_total = 0;
> - unsigned long tx_packets_err_total = 0;
> + uint64_t rx_packets_total = 0, rx_bytes_total = 0;
> + uint64_t rx_missed_total = 0;
> + uint64_t rx_nombuf_total = 0, rx_err_total = 0;
> + uint64_t tx_packets_total = 0, tx_bytes_total = 0;
> + uint64_t tx_packets_err_total = 0;
> const struct pmd_internals *internal = dev->data->dev_private;
>
> for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
> --
> 2.51.0
>