[dpdk-dev] [PATCH 08/32] net/ngbe: support basic statistics

[Jiawen Wu]

On September 16, 2021 12:51 AM, Ferruh Yigit wrote:
> On 9/8/2021 9:37 AM, Jiawen Wu wrote:
> > Support to read and clear basic statistics, and configure per-queue
> > stats counter mapping.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu at trustnetic.com>
> > ---
> >  doc/guides/nics/features/ngbe.ini  |   2 +
> >  doc/guides/nics/ngbe.rst           |   1 +
> >  drivers/net/ngbe/base/ngbe_dummy.h |   5 +
> >  drivers/net/ngbe/base/ngbe_hw.c    | 101 ++++++++++
> >  drivers/net/ngbe/base/ngbe_hw.h    |   1 +
> >  drivers/net/ngbe/base/ngbe_type.h  | 134 +++++++++++++
> >  drivers/net/ngbe/ngbe_ethdev.c     | 300
> +++++++++++++++++++++++++++++
> >  drivers/net/ngbe/ngbe_ethdev.h     |  19 ++
> >  8 files changed, 563 insertions(+)
> >
> 
> <...>
> 
> > +static int
> > +ngbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats
> > +*stats) {
> > +	struct ngbe_hw *hw = ngbe_dev_hw(dev);
> > +	struct ngbe_hw_stats *hw_stats = NGBE_DEV_STATS(dev);
> > +	struct ngbe_stat_mappings *stat_mappings =
> > +			NGBE_DEV_STAT_MAPPINGS(dev);
> > +	uint32_t i, j;
> > +
> > +	ngbe_read_stats_registers(hw, hw_stats);
> > +
> > +	if (stats == NULL)
> > +		return -EINVAL;
> > +
> > +	/* Fill out the rte_eth_stats statistics structure */
> > +	stats->ipackets = hw_stats->rx_packets;
> > +	stats->ibytes = hw_stats->rx_bytes;
> > +	stats->opackets = hw_stats->tx_packets;
> > +	stats->obytes = hw_stats->tx_bytes;
> > +
> > +	memset(&stats->q_ipackets, 0, sizeof(stats->q_ipackets));
> > +	memset(&stats->q_opackets, 0, sizeof(stats->q_opackets));
> > +	memset(&stats->q_ibytes, 0, sizeof(stats->q_ibytes));
> > +	memset(&stats->q_obytes, 0, sizeof(stats->q_obytes));
> > +	memset(&stats->q_errors, 0, sizeof(stats->q_errors));
> > +	for (i = 0; i < NGBE_MAX_QP; i++) {
> > +		uint32_t n = i / NB_QMAP_FIELDS_PER_QSM_REG;
> > +		uint32_t offset = (i % NB_QMAP_FIELDS_PER_QSM_REG) * 8;
> > +		uint32_t q_map;
> > +
> > +		q_map = (stat_mappings->rqsm[n] >> offset)
> > +				& QMAP_FIELD_RESERVED_BITS_MASK;
> > +		j = (q_map < RTE_ETHDEV_QUEUE_STAT_CNTRS
> > +		     ? q_map : q_map % RTE_ETHDEV_QUEUE_STAT_CNTRS);
> > +		stats->q_ipackets[j] += hw_stats->qp[i].rx_qp_packets;
> > +		stats->q_ibytes[j] += hw_stats->qp[i].rx_qp_bytes;
> > +
> > +		q_map = (stat_mappings->tqsm[n] >> offset)
> > +				& QMAP_FIELD_RESERVED_BITS_MASK;
> > +		j = (q_map < RTE_ETHDEV_QUEUE_STAT_CNTRS
> > +		     ? q_map : q_map % RTE_ETHDEV_QUEUE_STAT_CNTRS);
> > +		stats->q_opackets[j] += hw_stats->qp[i].tx_qp_packets;
> > +		stats->q_obytes[j] += hw_stats->qp[i].tx_qp_bytes;
> > +	}
> > +
> > +	/* Rx Errors */
> > +	stats->imissed  = hw_stats->rx_total_missed_packets +
> > +			  hw_stats->rx_dma_drop;
> > +	stats->ierrors  = hw_stats->rx_crc_errors +
> > +			  hw_stats->rx_mac_short_packet_dropped +
> > +			  hw_stats->rx_length_errors +
> > +			  hw_stats->rx_undersize_errors +
> > +			  hw_stats->rx_oversize_errors +
> > +			  hw_stats->rx_illegal_byte_errors +
> > +			  hw_stats->rx_error_bytes +
> > +			  hw_stats->rx_fragment_errors;
> > +
> > +	/* Tx Errors */
> > +	stats->oerrors  = 0;
> > +	return 0;
> 
> You can consider keeping 'stats->rx_nombuf' stats too, this needs to be
> calculated by driver.
> 

I see ' stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed' in the function rte_eth_stats_get, before calling stats_get ops.
Should I write it again here?

> <...>
> 
> > +
> >  static int
> >  ngbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info
> > *dev_info)  { @@ -1462,6 +1759,9 @@ static const struct eth_dev_ops
> > ngbe_eth_dev_ops = {
> >  	.dev_close                  = ngbe_dev_close,
> >  	.dev_reset                  = ngbe_dev_reset,
> >  	.link_update                = ngbe_dev_link_update,
> > +	.stats_get                  = ngbe_dev_stats_get,
> > +	.stats_reset                = ngbe_dev_stats_reset,
> > +	.queue_stats_mapping_set    = ngbe_dev_queue_stats_mapping_set,
> 
> 'queue_stats_mapping_set' is only needed when number of stats registers are
> less than number of queues. If this is not the case for you please drop this
> support.
> 
> And we are switching to qstats on extending stat, please see
> 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS'. This is mainly done to remove the
> compile time 'RTE_ETHDEV_QUEUE_STAT_CNTRS' limitation.
> 
> Btw, 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS' seems missing, you should
> have it in the driver.
>