Re: [PATCH v2 07/13] net/txgbe: add Tx descriptor error statistics

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

On 10/28/2024 2:31 AM, Jiawen Wu wrote:
> Count the number of packets not sent due to Tx descriptor error.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>  drivers/net/txgbe/txgbe_ethdev.c | 6 ++++++
>  drivers/net/txgbe/txgbe_rxtx.c   | 3 +++
>  drivers/net/txgbe/txgbe_rxtx.h   | 1 +
>  3 files changed, 10 insertions(+)
> 
> diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
> index bafa9cf829..0c76e986f4 100644
> --- a/drivers/net/txgbe/txgbe_ethdev.c
> +++ b/drivers/net/txgbe/txgbe_ethdev.c
> @@ -2344,6 +2344,7 @@ txgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  	struct txgbe_hw_stats *hw_stats = TXGBE_DEV_STATS(dev);
>  	struct txgbe_stat_mappings *stat_mappings =
>  			TXGBE_DEV_STAT_MAPPINGS(dev);
> +	struct txgbe_tx_queue *txq;
>  	uint32_t i, j;
>  
>  	txgbe_read_stats_registers(hw, hw_stats);
> @@ -2398,6 +2399,11 @@ txgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  
>  	/* Tx Errors */
>  	stats->oerrors  = 0;
> +	for (i = 0; i < dev->data->nb_tx_queues; i++) {
> +		txq = dev->data->tx_queues[i];
> +		stats->oerrors += txq->desc_error;
> +	}
> +

stats_get is implemented, but stats_reset also needs to be implemented
for this stat.

>  	return 0;
>  }
>  
> diff --git a/drivers/net/txgbe/txgbe_rxtx.c b/drivers/net/txgbe/txgbe_rxtx.c
> index 06acbd0881..fc9e7b14f5 100644
> --- a/drivers/net/txgbe/txgbe_rxtx.c
> +++ b/drivers/net/txgbe/txgbe_rxtx.c
> @@ -894,6 +894,7 @@ txgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  		tx_pkt = *tx_pkts++;
>  		if (txgbe_check_pkt_err(tx_pkt)) {
>  			rte_pktmbuf_free(tx_pkt);
> +			txq->desc_error++;
>  			continue;
>  		}
>  
> @@ -2523,6 +2524,7 @@ txgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
>  	txgbe_set_tx_function(dev, txq);
>  
>  	txq->ops->reset(txq);
> +	txq->desc_error = 0;
>  
>  	dev->data->tx_queues[queue_idx] = txq;
>  
> @@ -4980,6 +4982,7 @@ txgbe_tx_queue_clear_error(void *param)
>  		if (!txq->resetting)
>  			continue;
>  
> +		txq->desc_error++;
>

Why error value is increased in this function, which resets the Tx queue?
Is the intention to reset the error value here?

>  		txgbe_dev_save_tx_queue(hw, i);
>  
>  		/* tx ring reset */
> diff --git a/drivers/net/txgbe/txgbe_rxtx.h b/drivers/net/txgbe/txgbe_rxtx.h
> index e668b60b1e..622a0d3981 100644
> --- a/drivers/net/txgbe/txgbe_rxtx.h
> +++ b/drivers/net/txgbe/txgbe_rxtx.h
> @@ -412,6 +412,7 @@ struct txgbe_tx_queue {
>  	/**< indicates that IPsec TX feature is in use */
>  #endif
>  	const struct rte_memzone *mz;
> +	uint64_t	    desc_error;
>  	bool		    resetting;
>  };
>