[PATCH v12 10/13] net/sxe: add xstats function

[Stephen Hemminger]

On Mon, 28 Jul 2025 17:05:14 +0800
liujie5 at linkdatatechnology.com wrote:

> +s32 sxe_eth_stats_get(struct rte_eth_dev *eth_dev,
> +				struct rte_eth_stats *stats)
> +{
> +	struct sxe_adapter *adapter = eth_dev->data->dev_private;
> +	struct sxe_stats_info *stats_info = &adapter->stats_info;
> +	struct sxe_hw *hw = &adapter->hw;
> +	u32 i;
> +	u64 rx_packets = 0;
> +	u64 rx_bytes = 0;
> +	s32 ret = 0;
> +
> +	sxe_hw_stats_get(hw, &stats_info->hw_stats);
> +
> +	if (stats == NULL) {
> +		ret = -EINVAL;
> +		PMD_LOG_ERR(DRV, "input param stats is null.");
> +		goto l_out;
> +	}
> +

Several things wrong with the stats function:
   - first this function and others in ops are assuming s32 is the same as int.
     it is on the platforms you build on now, but it might not always be.
     It is best practice to implement the operation with the same function signature
     as it is defined in ethdev_driver.h

   - second, the stats pointer can not be NULL when called by ethdev stats.
     So the check here is unnecessary.  It looks like the code is also calling this
     internally, with stats = NULL to update hw_stats. If so just call sxe_hw_stats_get instead.

> +	for (i = 0; i < RTE_MIN_T(SXE_QUEUE_STAT_COUNT,
> +		RTE_ETHDEV_QUEUE_STAT_CNTRS, typeof(i)); i++) {
> +		rx_packets += stats_info->hw_stats.qprc[i];
> +		rx_bytes += stats_info->hw_stats.qbrc[i];

   - this loop is wrong, if there are more queues that RTE_ETHDEV_QUEUE_STAT_CNTRS
     then you still want to count those packets in the total.

   - lastly, the recent update to ethdev queue stats changed the args to the callback.
     you need to fix this for both PF and VF. Rebase on main.