On Mon, 28 Jul 2025 17:05:14 +0800
[email protected] 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.
