[dpdk-dev] [PATCH 0/2] net/cxgbe: add support for xstats API

[dpdk-dev] [PATCH 0/2] net/cxgbe: add support for xstats API

[Rahul Lakkireddy]

This series of patches add support to fetch port and queue stats via
xstats API.

Patch 1 adds support to fetch port and queue stats via xstats API.

Patch 2 removes queue stats from basic stats since they're available
via xstats API. Also removes RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.

Rahul Lakkireddy (2):
  net/cxgbe: add support for xstats API
  net/cxgbe: remove queue stats from basic stats

 drivers/net/cxgbe/cxgbe_ethdev.c | 320 ++++++++++++++++++++++++++++---
 1 file changed, 296 insertions(+), 24 deletions(-)

-- 
2.27.0

[dpdk-dev] [PATCH 1/2] net/cxgbe: add support for xstats API

[Rahul Lakkireddy]

Add support to fetch port and queue stats via xstats API.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_ethdev.c | 302 ++++++++++++++++++++++++++++++-
 1 file changed, 293 insertions(+), 9 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 550843b4d7..8dee3fc3b6 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -778,24 +778,303 @@ static int cxgbe_dev_stats_reset(struct rte_eth_dev *eth_dev)
 
 	cxgbe_stats_reset(pi);
 	for (i = 0; i < pi->n_rx_qsets; i++) {
-		struct sge_eth_rxq *rxq =
-			&s->ethrxq[pi->first_rxqset + i];
+		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + i];
 
-		rxq->stats.pkts = 0;
-		rxq->stats.rx_bytes = 0;
+		memset(&rxq->stats, 0, sizeof(rxq->stats));
 	}
 	for (i = 0; i < pi->n_tx_qsets; i++) {
-		struct sge_eth_txq *txq =
-			&s->ethtxq[pi->first_txqset + i];
+		struct sge_eth_txq *txq = &s->ethtxq[pi->first_txqset + i];
 
-		txq->stats.pkts = 0;
-		txq->stats.tx_bytes = 0;
-		txq->stats.mapping_err = 0;
+		memset(&txq->stats, 0, sizeof(txq->stats));
 	}
 
 	return 0;
 }
 
+/* Store extended statistics names and its offset in stats structure  */
+struct cxgbe_dev_xstats_name_off {
+	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	unsigned int offset;
+};
+
+static const struct cxgbe_dev_xstats_name_off cxgbe_dev_rxq_stats_strings[] = {
+	{"packets", offsetof(struct sge_eth_rx_stats, pkts)},
+	{"bytes", offsetof(struct sge_eth_rx_stats, rx_bytes)},
+	{"checksum_offloads", offsetof(struct sge_eth_rx_stats, rx_cso)},
+	{"vlan_extractions", offsetof(struct sge_eth_rx_stats, vlan_ex)},
+	{"dropped_packets", offsetof(struct sge_eth_rx_stats, rx_drops)},
+};
+
+static const struct cxgbe_dev_xstats_name_off cxgbe_dev_txq_stats_strings[] = {
+	{"packets", offsetof(struct sge_eth_tx_stats, pkts)},
+	{"bytes", offsetof(struct sge_eth_tx_stats, tx_bytes)},
+	{"tso_requests", offsetof(struct sge_eth_tx_stats, tso)},
+	{"checksum_offloads", offsetof(struct sge_eth_tx_stats, tx_cso)},
+	{"vlan_insertions", offsetof(struct sge_eth_tx_stats, vlan_ins)},
+	{"packet_mapping_errors",
+	 offsetof(struct sge_eth_tx_stats, mapping_err)},
+	{"coalesced_wrs", offsetof(struct sge_eth_tx_stats, coal_wr)},
+	{"coalesced_packets", offsetof(struct sge_eth_tx_stats, coal_pkts)},
+};
+
+static const struct cxgbe_dev_xstats_name_off cxgbe_dev_port_stats_strings[] = {
+	{"tx_bytes", offsetof(struct port_stats, tx_octets)},
+	{"tx_packets", offsetof(struct port_stats, tx_frames)},
+	{"tx_broadcast_packets", offsetof(struct port_stats, tx_bcast_frames)},
+	{"tx_multicast_packets", offsetof(struct port_stats, tx_mcast_frames)},
+	{"tx_unicast_packets", offsetof(struct port_stats, tx_ucast_frames)},
+	{"tx_error_packets", offsetof(struct port_stats, tx_error_frames)},
+	{"tx_size_64_packets", offsetof(struct port_stats, tx_frames_64)},
+	{"tx_size_65_to_127_packets",
+	 offsetof(struct port_stats, tx_frames_65_127)},
+	{"tx_size_128_to_255_packets",
+	 offsetof(struct port_stats, tx_frames_128_255)},
+	{"tx_size_256_to_511_packets",
+	 offsetof(struct port_stats, tx_frames_256_511)},
+	{"tx_size_512_to_1023_packets",
+	 offsetof(struct port_stats, tx_frames_512_1023)},
+	{"tx_size_1024_to_1518_packets",
+	 offsetof(struct port_stats, tx_frames_1024_1518)},
+	{"tx_size_1519_to_max_packets",
+	 offsetof(struct port_stats, tx_frames_1519_max)},
+	{"tx_drop_packets", offsetof(struct port_stats, tx_drop)},
+	{"tx_pause_frames", offsetof(struct port_stats, tx_pause)},
+	{"tx_ppp_pri0_packets", offsetof(struct port_stats, tx_ppp0)},
+	{"tx_ppp_pri1_packets", offsetof(struct port_stats, tx_ppp1)},
+	{"tx_ppp_pri2_packets", offsetof(struct port_stats, tx_ppp2)},
+	{"tx_ppp_pri3_packets", offsetof(struct port_stats, tx_ppp3)},
+	{"tx_ppp_pri4_packets", offsetof(struct port_stats, tx_ppp4)},
+	{"tx_ppp_pri5_packets", offsetof(struct port_stats, tx_ppp5)},
+	{"tx_ppp_pri6_packets", offsetof(struct port_stats, tx_ppp6)},
+	{"tx_ppp_pri7_packets", offsetof(struct port_stats, tx_ppp7)},
+	{"rx_bytes", offsetof(struct port_stats, rx_octets)},
+	{"rx_packets", offsetof(struct port_stats, rx_frames)},
+	{"rx_broadcast_packets", offsetof(struct port_stats, rx_bcast_frames)},
+	{"rx_multicast_packets", offsetof(struct port_stats, rx_mcast_frames)},
+	{"rx_unicast_packets", offsetof(struct port_stats, rx_ucast_frames)},
+	{"rx_too_long_packets", offsetof(struct port_stats, rx_too_long)},
+	{"rx_jabber_packets", offsetof(struct port_stats, rx_jabber)},
+	{"rx_fcs_error_packets", offsetof(struct port_stats, rx_fcs_err)},
+	{"rx_length_error_packets", offsetof(struct port_stats, rx_len_err)},
+	{"rx_symbol_error_packets",
+	 offsetof(struct port_stats, rx_symbol_err)},
+	{"rx_short_packets", offsetof(struct port_stats, rx_runt)},
+	{"rx_size_64_packets", offsetof(struct port_stats, rx_frames_64)},
+	{"rx_size_65_to_127_packets",
+	 offsetof(struct port_stats, rx_frames_65_127)},
+	{"rx_size_128_to_255_packets",
+	 offsetof(struct port_stats, rx_frames_128_255)},
+	{"rx_size_256_to_511_packets",
+	 offsetof(struct port_stats, rx_frames_256_511)},
+	{"rx_size_512_to_1023_packets",
+	 offsetof(struct port_stats, rx_frames_512_1023)},
+	{"rx_size_1024_to_1518_packets",
+	 offsetof(struct port_stats, rx_frames_1024_1518)},
+	{"rx_size_1519_to_max_packets",
+	 offsetof(struct port_stats, rx_frames_1519_max)},
+	{"rx_pause_packets", offsetof(struct port_stats, rx_pause)},
+	{"rx_ppp_pri0_packets", offsetof(struct port_stats, rx_ppp0)},
+	{"rx_ppp_pri1_packets", offsetof(struct port_stats, rx_ppp1)},
+	{"rx_ppp_pri2_packets", offsetof(struct port_stats, rx_ppp2)},
+	{"rx_ppp_pri3_packets", offsetof(struct port_stats, rx_ppp3)},
+	{"rx_ppp_pri4_packets", offsetof(struct port_stats, rx_ppp4)},
+	{"rx_ppp_pri5_packets", offsetof(struct port_stats, rx_ppp5)},
+	{"rx_ppp_pri6_packets", offsetof(struct port_stats, rx_ppp6)},
+	{"rx_ppp_pri7_packets", offsetof(struct port_stats, rx_ppp7)},
+	{"rx_bg0_dropped_packets", offsetof(struct port_stats, rx_ovflow0)},
+	{"rx_bg1_dropped_packets", offsetof(struct port_stats, rx_ovflow1)},
+	{"rx_bg2_dropped_packets", offsetof(struct port_stats, rx_ovflow2)},
+	{"rx_bg3_dropped_packets", offsetof(struct port_stats, rx_ovflow3)},
+	{"rx_bg0_truncated_packets", offsetof(struct port_stats, rx_trunc0)},
+	{"rx_bg1_truncated_packets", offsetof(struct port_stats, rx_trunc1)},
+	{"rx_bg2_truncated_packets", offsetof(struct port_stats, rx_trunc2)},
+	{"rx_bg3_truncated_packets", offsetof(struct port_stats, rx_trunc3)},
+};
+
+#define CXGBE_NB_RXQ_STATS RTE_DIM(cxgbe_dev_rxq_stats_strings)
+#define CXGBE_NB_TXQ_STATS RTE_DIM(cxgbe_dev_txq_stats_strings)
+#define CXGBE_NB_PORT_STATS RTE_DIM(cxgbe_dev_port_stats_strings)
+
+static u16 cxgbe_dev_xstats_count(struct port_info *pi)
+{
+	return CXGBE_NB_PORT_STATS +
+	       (pi->n_tx_qsets * CXGBE_NB_TXQ_STATS) +
+	       (pi->n_rx_qsets * CXGBE_NB_RXQ_STATS);
+}
+
+static int cxgbe_dev_xstats(struct rte_eth_dev *dev,
+			    struct rte_eth_xstat_name *xstats_names,
+			    struct rte_eth_xstat *xstats, unsigned int size)
+{
+	const struct cxgbe_dev_xstats_name_off *xstats_str;
+	struct port_info *pi = dev->data->dev_private;
+	struct adapter *adap = pi->adapter;
+	struct sge *s = &adap->sge;
+	struct port_stats ps;
+	u16 count, i, qid;
+	u64 *stats_ptr;
+
+	count = cxgbe_dev_xstats_count(pi);
+	if (size < count)
+		return count;
+
+	/* port stats */
+	cxgbe_stats_get(pi, &ps);
+
+	count = 0;
+	xstats_str = cxgbe_dev_port_stats_strings;
+	for (i = 0; i < CXGBE_NB_PORT_STATS; i++, count++) {
+		if (xstats_names)
+			snprintf(xstats_names[count].name,
+				 sizeof(xstats_names[count].name),
+				 "%s", xstats_str[i].name);
+		if (xstats) {
+			stats_ptr = RTE_PTR_ADD(&ps,
+						xstats_str[i].offset);
+			xstats[count].value = *stats_ptr;
+			xstats[count].id = count;
+		}
+	}
+
+	/* per-txq stats */
+	xstats_str = cxgbe_dev_txq_stats_strings;
+	for (qid = 0; qid < pi->n_tx_qsets; qid++) {
+		struct sge_eth_txq *txq = &s->ethtxq[pi->first_txqset + qid];
+
+		for (i = 0; i < CXGBE_NB_TXQ_STATS; i++, count++) {
+			if (xstats_names)
+				snprintf(xstats_names[count].name,
+					 sizeof(xstats_names[count].name),
+					 "tx_q%u_%s",
+					 qid, xstats_str[i].name);
+			if (xstats) {
+				stats_ptr = RTE_PTR_ADD(&txq->stats,
+							xstats_str[i].offset);
+				xstats[count].value = *stats_ptr;
+				xstats[count].id = count;
+			}
+		}
+	}
+
+	/* per-rxq stats */
+	xstats_str = cxgbe_dev_rxq_stats_strings;
+	for (qid = 0; qid < pi->n_rx_qsets; qid++) {
+		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + qid];
+
+		for (i = 0; i < CXGBE_NB_RXQ_STATS; i++, count++) {
+			if (xstats_names)
+				snprintf(xstats_names[count].name,
+					 sizeof(xstats_names[count].name),
+					 "rx_q%u_%s",
+					 qid, xstats_str[i].name);
+			if (xstats) {
+				stats_ptr = RTE_PTR_ADD(&rxq->stats,
+							xstats_str[i].offset);
+				xstats[count].value = *stats_ptr;
+				xstats[count].id = count;
+			}
+		}
+	}
+
+	return count;
+}
+
+/* Get port extended statistics by id.
+ */
+static int cxgbe_dev_xstats_get_by_id(struct rte_eth_dev *dev,
+				      const uint64_t *ids, uint64_t *values,
+				      unsigned int n)
+{
+	struct port_info *pi = dev->data->dev_private;
+	struct rte_eth_xstat *xstats_copy;
+	u16 count, i;
+	int ret = 0;
+
+	count = cxgbe_dev_xstats_count(pi);
+	if (!ids || !values)
+		return count;
+
+	xstats_copy = rte_calloc(NULL, count, sizeof(*xstats_copy), 0);
+	if (!xstats_copy)
+		return -ENOMEM;
+
+	cxgbe_dev_xstats(dev, NULL, xstats_copy, count);
+
+	for (i = 0; i < n; i++) {
+		if (ids[i] >= count) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+		values[i] = xstats_copy[ids[i]].value;
+	}
+
+	ret = n;
+
+out_err:
+	rte_free(xstats_copy);
+	return ret;
+}
+
+/* Get names of port extended statistics by id.
+ */
+static int cxgbe_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
+					    struct rte_eth_xstat_name *xnames,
+					    const uint64_t *ids, unsigned int n)
+{
+	struct port_info *pi = dev->data->dev_private;
+	struct rte_eth_xstat_name *xnames_copy;
+	u16 count, i;
+	int ret = 0;
+
+	count = cxgbe_dev_xstats_count(pi);
+	if (!ids || !xnames)
+		return count;
+
+	xnames_copy = rte_calloc(NULL, count, sizeof(*xnames_copy), 0);
+	if (!xnames_copy)
+		return -ENOMEM;
+
+	cxgbe_dev_xstats(dev, xnames_copy, NULL, count);
+
+	for (i = 0; i < n; i++) {
+		if (ids[i] >= count) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+		strcpy(xnames[i].name, xnames_copy[ids[i]].name);
+	}
+
+	ret = n;
+
+out_err:
+	rte_free(xnames_copy);
+	return ret;
+}
+
+/* Get port extended statistics.
+ */
+static int cxgbe_dev_xstats_get(struct rte_eth_dev *dev,
+				struct rte_eth_xstat *xstats, unsigned int n)
+{
+	return cxgbe_dev_xstats(dev, NULL, xstats, n);
+}
+
+/* Get names of port extended statistics.
+ */
+static int cxgbe_dev_xstats_get_names(struct rte_eth_dev *dev,
+				      struct rte_eth_xstat_name *xstats_names,
+				      unsigned int n)
+{
+	return cxgbe_dev_xstats(dev, xstats_names, NULL, n);
+}
+
+/* Reset port extended statistics.
+ */
+static int cxgbe_dev_xstats_reset(struct rte_eth_dev *dev)
+{
+	return cxgbe_dev_stats_reset(dev);
+}
+
 static int cxgbe_flow_ctrl_get(struct rte_eth_dev *eth_dev,
 			       struct rte_eth_fc_conf *fc_conf)
 {
@@ -1351,6 +1630,11 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = {
 	.flow_ops_get           = cxgbe_dev_flow_ops_get,
 	.stats_get		= cxgbe_dev_stats_get,
 	.stats_reset		= cxgbe_dev_stats_reset,
+	.xstats_get             = cxgbe_dev_xstats_get,
+	.xstats_get_by_id       = cxgbe_dev_xstats_get_by_id,
+	.xstats_get_names       = cxgbe_dev_xstats_get_names,
+	.xstats_get_names_by_id = cxgbe_dev_xstats_get_names_by_id,
+	.xstats_reset           = cxgbe_dev_xstats_reset,
 	.flow_ctrl_get		= cxgbe_flow_ctrl_get,
 	.flow_ctrl_set		= cxgbe_flow_ctrl_set,
 	.get_eeprom_length	= cxgbe_get_eeprom_length,
-- 
2.27.0

[dpdk-dev] [PATCH 2/2] net/cxgbe: remove queue stats from basic stats

[Rahul Lakkireddy]

Remove queue stats from basic stats because they're now available
via xstats API. Also remove RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_ethdev.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 8dee3fc3b6..141a2de808 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -747,22 +747,12 @@ static int cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
 	eth_stats->oerrors  = ps.tx_error_frames;
 
 	for (i = 0; i < pi->n_rx_qsets; i++) {
-		struct sge_eth_rxq *rxq =
-			&s->ethrxq[pi->first_rxqset + i];
+		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + i];
 
-		eth_stats->q_ipackets[i] = rxq->stats.pkts;
-		eth_stats->q_ibytes[i] = rxq->stats.rx_bytes;
-		eth_stats->ipackets += eth_stats->q_ipackets[i];
-		eth_stats->ibytes += eth_stats->q_ibytes[i];
+		eth_stats->ipackets += rxq->stats.pkts;
+		eth_stats->ibytes += rxq->stats.rx_bytes;
 	}
 
-	for (i = 0; i < pi->n_tx_qsets; i++) {
-		struct sge_eth_txq *txq =
-			&s->ethtxq[pi->first_txqset + i];
-
-		eth_stats->q_opackets[i] = txq->stats.pkts;
-		eth_stats->q_obytes[i] = txq->stats.tx_bytes;
-	}
 	return 0;
 }
 
@@ -1697,8 +1687,6 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
-	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
-
 	snprintf(name, sizeof(name), "cxgbeadapter%d", eth_dev->data->port_id);
 	adapter = rte_zmalloc(name, sizeof(*adapter), 0);
 	if (!adapter)
-- 
2.27.0

Re: [dpdk-dev] [PATCH 1/2] net/cxgbe: add support for xstats API

[Andrew Rybchenko]

On 6/3/21 6:30 PM, Rahul Lakkireddy wrote:
> Add support to fetch port and queue stats via xstats API.
> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>

[snip]

> +	count = 0;
> +	xstats_str = cxgbe_dev_port_stats_strings;
> +	for (i = 0; i < CXGBE_NB_PORT_STATS; i++, count++) {
> +		if (xstats_names)

!= NULL

DPDK coding style requires explicit comparison vs null [1]

[1] https://doc.dpdk.org/guides/contributing/coding_style.html#null-pointers

> +			snprintf(xstats_names[count].name,
> +				 sizeof(xstats_names[count].name),
> +				 "%s", xstats_str[i].name);
> +		if (xstats) {

!= NULL

> +			stats_ptr = RTE_PTR_ADD(&ps,
> +						xstats_str[i].offset);
> +			xstats[count].value = *stats_ptr;
> +			xstats[count].id = count;
> +		}
> +	}
> +
> +	/* per-txq stats */
> +	xstats_str = cxgbe_dev_txq_stats_strings;
> +	for (qid = 0; qid < pi->n_tx_qsets; qid++) {
> +		struct sge_eth_txq *txq = &s->ethtxq[pi->first_txqset + qid];
> +
> +		for (i = 0; i < CXGBE_NB_TXQ_STATS; i++, count++) {
> +			if (xstats_names)

!= NULL

> +				snprintf(xstats_names[count].name,
> +					 sizeof(xstats_names[count].name),
> +					 "tx_q%u_%s",
> +					 qid, xstats_str[i].name);
> +			if (xstats) {

!= NULL

> +				stats_ptr = RTE_PTR_ADD(&txq->stats,
> +							xstats_str[i].offset);
> +				xstats[count].value = *stats_ptr;
> +				xstats[count].id = count;
> +			}
> +		}
> +	}
> +
> +	/* per-rxq stats */
> +	xstats_str = cxgbe_dev_rxq_stats_strings;
> +	for (qid = 0; qid < pi->n_rx_qsets; qid++) {
> +		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + qid];
> +
> +		for (i = 0; i < CXGBE_NB_RXQ_STATS; i++, count++) {
> +			if (xstats_names)

!= NULL

> +				snprintf(xstats_names[count].name,
> +					 sizeof(xstats_names[count].name),
> +					 "rx_q%u_%s",
> +					 qid, xstats_str[i].name);
> +			if (xstats) {

!= NULL

> +				stats_ptr = RTE_PTR_ADD(&rxq->stats,
> +							xstats_str[i].offset);
> +				xstats[count].value = *stats_ptr;
> +				xstats[count].id = count;
> +			}
> +		}
> +	}
> +
> +	return count;
> +}
> +
> +/* Get port extended statistics by id.
> + */
> +static int cxgbe_dev_xstats_get_by_id(struct rte_eth_dev *dev,
> +				      const uint64_t *ids, uint64_t *values,
> +				      unsigned int n)
> +{
> +	struct port_info *pi = dev->data->dev_private;
> +	struct rte_eth_xstat *xstats_copy;
> +	u16 count, i;
> +	int ret = 0;
> +
> +	count = cxgbe_dev_xstats_count(pi);
> +	if (!ids || !values)

== NULL twice

> +		return count;
> +
> +	xstats_copy = rte_calloc(NULL, count, sizeof(*xstats_copy), 0);
> +	if (!xstats_copy)

== NULL

> +		return -ENOMEM;
> +
> +	cxgbe_dev_xstats(dev, NULL, xstats_copy, count);
> +
> +	for (i = 0; i < n; i++) {
> +		if (ids[i] >= count) {
> +			ret = -EINVAL;
> +			goto out_err;
> +		}
> +		values[i] = xstats_copy[ids[i]].value;
> +	}
> +
> +	ret = n;
> +
> +out_err:
> +	rte_free(xstats_copy);
> +	return ret;
> +}
> +
> +/* Get names of port extended statistics by id.
> + */
> +static int cxgbe_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
> +					    struct rte_eth_xstat_name *xnames,
> +					    const uint64_t *ids, unsigned int n)
> +{
> +	struct port_info *pi = dev->data->dev_private;
> +	struct rte_eth_xstat_name *xnames_copy;
> +	u16 count, i;
> +	int ret = 0;
> +
> +	count = cxgbe_dev_xstats_count(pi);
> +	if (!ids || !xnames)

== NULL twice

> +		return count;
> +
> +	xnames_copy = rte_calloc(NULL, count, sizeof(*xnames_copy), 0);
> +	if (!xnames_copy)

== NULL

> +		return -ENOMEM;
> +
> +	cxgbe_dev_xstats(dev, xnames_copy, NULL, count);
> +
> +	for (i = 0; i < n; i++) {
> +		if (ids[i] >= count) {
> +			ret = -EINVAL;
> +			goto out_err;
> +		}
> +		strcpy(xnames[i].name, xnames_copy[ids[i]].name);

rte_strlcpy, please

> +	}
> +
> +	ret = n;
> +
> +out_err:
> +	rte_free(xnames_copy);
> +	return ret;
> +}
> +
> +/* Get port extended statistics.
> + */
> +static int cxgbe_dev_xstats_get(struct rte_eth_dev *dev,
> +				struct rte_eth_xstat *xstats, unsigned int n)
> +{
> +	return cxgbe_dev_xstats(dev, NULL, xstats, n);
> +}
> +
> +/* Get names of port extended statistics.
> + */
> +static int cxgbe_dev_xstats_get_names(struct rte_eth_dev *dev,
> +				      struct rte_eth_xstat_name *xstats_names,
> +				      unsigned int n)
> +{
> +	return cxgbe_dev_xstats(dev, xstats_names, NULL, n);
> +}
> +
> +/* Reset port extended statistics.
> + */
> +static int cxgbe_dev_xstats_reset(struct rte_eth_dev *dev)
> +{
> +	return cxgbe_dev_stats_reset(dev);
> +}
> +
>  static int cxgbe_flow_ctrl_get(struct rte_eth_dev *eth_dev,
>  			       struct rte_eth_fc_conf *fc_conf)
>  {
> @@ -1351,6 +1630,11 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = {
>  	.flow_ops_get           = cxgbe_dev_flow_ops_get,
>  	.stats_get		= cxgbe_dev_stats_get,
>  	.stats_reset		= cxgbe_dev_stats_reset,
> +	.xstats_get             = cxgbe_dev_xstats_get,
> +	.xstats_get_by_id       = cxgbe_dev_xstats_get_by_id,
> +	.xstats_get_names       = cxgbe_dev_xstats_get_names,
> +	.xstats_get_names_by_id = cxgbe_dev_xstats_get_names_by_id,
> +	.xstats_reset           = cxgbe_dev_xstats_reset,
>  	.flow_ctrl_get		= cxgbe_flow_ctrl_get,
>  	.flow_ctrl_set		= cxgbe_flow_ctrl_set,
>  	.get_eeprom_length	= cxgbe_get_eeprom_length,
> 

Re: [dpdk-dev] [PATCH 2/2] net/cxgbe: remove queue stats from basic stats

[Andrew Rybchenko]

On 6/3/21 6:30 PM, Rahul Lakkireddy wrote:
> Remove queue stats from basic stats because they're now available
> via xstats API. Also remove RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.
> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> ---
>  drivers/net/cxgbe/cxgbe_ethdev.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
> index 8dee3fc3b6..141a2de808 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -747,22 +747,12 @@ static int cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
>  	eth_stats->oerrors  = ps.tx_error_frames;
>  
>  	for (i = 0; i < pi->n_rx_qsets; i++) {
> -		struct sge_eth_rxq *rxq =
> -			&s->ethrxq[pi->first_rxqset + i];
> +		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + i];
>  
> -		eth_stats->q_ipackets[i] = rxq->stats.pkts;
> -		eth_stats->q_ibytes[i] = rxq->stats.rx_bytes;
> -		eth_stats->ipackets += eth_stats->q_ipackets[i];
> -		eth_stats->ibytes += eth_stats->q_ibytes[i];
> +		eth_stats->ipackets += rxq->stats.pkts;
> +		eth_stats->ibytes += rxq->stats.rx_bytes;
>  	}
>  
> -	for (i = 0; i < pi->n_tx_qsets; i++) {
> -		struct sge_eth_txq *txq =
> -			&s->ethtxq[pi->first_txqset + i];
> -
> -		eth_stats->q_opackets[i] = txq->stats.pkts;
> -		eth_stats->q_obytes[i] = txq->stats.tx_bytes;
> -	}
>  	return 0;
>  }
>  
> @@ -1697,8 +1687,6 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
>  		return 0;
>  	}
>  
> -	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> -
>  	snprintf(name, sizeof(name), "cxgbeadapter%d", eth_dev->data->port_id);
>  	adapter = rte_zmalloc(name, sizeof(*adapter), 0);
>  	if (!adapter)
> 

As I understand it is correct to remove
RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS to avoid duplication of per-queue
stats in xstats. But I don't understand why per-queue
stats are removed from basic stats. It still makes sense and
useful. Am I missing something?

Re: [dpdk-dev] [PATCH 2/2] net/cxgbe: remove queue stats from basic stats

[Rahul Lakkireddy]

Hi Andrew,

On Thursday, July 07/01/21, 2021 at 17:50:19 +0300, Andrew Rybchenko wrote:
> On 6/3/21 6:30 PM, Rahul Lakkireddy wrote:
> > Remove queue stats from basic stats because they're now available
> > via xstats API. Also remove RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.
> > 
> > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> > ---
> >  drivers/net/cxgbe/cxgbe_ethdev.c | 18 +++---------------
> >  1 file changed, 3 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
> > index 8dee3fc3b6..141a2de808 100644
> > --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> > +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> > @@ -747,22 +747,12 @@ static int cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
> >  	eth_stats->oerrors  = ps.tx_error_frames;
> >  
> >  	for (i = 0; i < pi->n_rx_qsets; i++) {
> > -		struct sge_eth_rxq *rxq =
> > -			&s->ethrxq[pi->first_rxqset + i];
> > +		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + i];
> >  
> > -		eth_stats->q_ipackets[i] = rxq->stats.pkts;
> > -		eth_stats->q_ibytes[i] = rxq->stats.rx_bytes;
> > -		eth_stats->ipackets += eth_stats->q_ipackets[i];
> > -		eth_stats->ibytes += eth_stats->q_ibytes[i];
> > +		eth_stats->ipackets += rxq->stats.pkts;
> > +		eth_stats->ibytes += rxq->stats.rx_bytes;
> >  	}
> >  
> > -	for (i = 0; i < pi->n_tx_qsets; i++) {
> > -		struct sge_eth_txq *txq =
> > -			&s->ethtxq[pi->first_txqset + i];
> > -
> > -		eth_stats->q_opackets[i] = txq->stats.pkts;
> > -		eth_stats->q_obytes[i] = txq->stats.tx_bytes;
> > -	}
> >  	return 0;
> >  }
> >  
> > @@ -1697,8 +1687,6 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
> >  		return 0;
> >  	}
> >  
> > -	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> > -
> >  	snprintf(name, sizeof(name), "cxgbeadapter%d", eth_dev->data->port_id);
> >  	adapter = rte_zmalloc(name, sizeof(*adapter), 0);
> >  	if (!adapter)
> > 
> 
> As I understand it is correct to remove
> RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS to avoid duplication of per-queue
> stats in xstats. But I don't understand why per-queue
> stats are removed from basic stats. It still makes sense and
> useful. Am I missing something?

The per-queue stats are being removed from basic stats here because
these will be removed from rte_eth_stats, as per the below deprecation
notice.

* ethdev: Queue specific stats fields will be removed from ``struct rte_eth_stats``.
  Mentioned fields are: ``q_ipackets``, ``q_opackets``, ``q_ibytes``, ``q_obytes``,
  ``q_errors``.
  Instead queue stats will be received via xstats API. Current method support
  will be limited to maximum 256 queues.
  Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.

Thanks,
Rahul

Re: [dpdk-dev] [PATCH 2/2] net/cxgbe: remove queue stats from basic stats

[Andrew Rybchenko]

Hi Rahul,

On 7/1/21 6:26 PM, Rahul Lakkireddy wrote:
> Hi Andrew,
> 
> On Thursday, July 07/01/21, 2021 at 17:50:19 +0300, Andrew Rybchenko wrote:
>> On 6/3/21 6:30 PM, Rahul Lakkireddy wrote:
>>> Remove queue stats from basic stats because they're now available
>>> via xstats API. Also remove RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.
>>>
>>> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
>>> ---
>>>  drivers/net/cxgbe/cxgbe_ethdev.c | 18 +++---------------
>>>  1 file changed, 3 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
>>> index 8dee3fc3b6..141a2de808 100644
>>> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
>>> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
>>> @@ -747,22 +747,12 @@ static int cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
>>>  	eth_stats->oerrors  = ps.tx_error_frames;
>>>  
>>>  	for (i = 0; i < pi->n_rx_qsets; i++) {
>>> -		struct sge_eth_rxq *rxq =
>>> -			&s->ethrxq[pi->first_rxqset + i];
>>> +		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + i];
>>>  
>>> -		eth_stats->q_ipackets[i] = rxq->stats.pkts;
>>> -		eth_stats->q_ibytes[i] = rxq->stats.rx_bytes;
>>> -		eth_stats->ipackets += eth_stats->q_ipackets[i];
>>> -		eth_stats->ibytes += eth_stats->q_ibytes[i];
>>> +		eth_stats->ipackets += rxq->stats.pkts;
>>> +		eth_stats->ibytes += rxq->stats.rx_bytes;
>>>  	}
>>>  
>>> -	for (i = 0; i < pi->n_tx_qsets; i++) {
>>> -		struct sge_eth_txq *txq =
>>> -			&s->ethtxq[pi->first_txqset + i];
>>> -
>>> -		eth_stats->q_opackets[i] = txq->stats.pkts;
>>> -		eth_stats->q_obytes[i] = txq->stats.tx_bytes;
>>> -	}
>>>  	return 0;
>>>  }
>>>  
>>> @@ -1697,8 +1687,6 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
>>>  		return 0;
>>>  	}
>>>  
>>> -	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
>>> -
>>>  	snprintf(name, sizeof(name), "cxgbeadapter%d", eth_dev->data->port_id);
>>>  	adapter = rte_zmalloc(name, sizeof(*adapter), 0);
>>>  	if (!adapter)
>>>
>>
>> As I understand it is correct to remove
>> RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS to avoid duplication of per-queue
>> stats in xstats. But I don't understand why per-queue
>> stats are removed from basic stats. It still makes sense and
>> useful. Am I missing something?
> 
> The per-queue stats are being removed from basic stats here because
> these will be removed from rte_eth_stats, as per the below deprecation
> notice.
> 
> * ethdev: Queue specific stats fields will be removed from ``struct rte_eth_stats``.
>   Mentioned fields are: ``q_ipackets``, ``q_opackets``, ``q_ibytes``, ``q_obytes``,
>   ``q_errors``.
>   Instead queue stats will be received via xstats API. Current method support
>   will be limited to maximum 256 queues.
>   Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.

Thanks a lot. It is bad that I've lost it from my view.
Clear now.

Andrew.

[dpdk-dev] [PATCH v2 0/2] net/cxgbe: add support for xstats API

[Rahul Lakkireddy]

This series of patches add support to fetch port and queue stats via
xstats API.

Patch 1 adds support to fetch port and queue stats via xstats API.

Patch 2 removes queue stats from basic stats since they're available
via xstats API. Also removes RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.

---
v2:
- Perform explicit checks for NULL in conditions.
- Use rte_strlcpy() instead of strcpy().

Rahul Lakkireddy (2):
  net/cxgbe: add support for xstats API
  net/cxgbe: remove queue stats from basic stats

 drivers/net/cxgbe/cxgbe_ethdev.c | 321 ++++++++++++++++++++++++++++---
 1 file changed, 297 insertions(+), 24 deletions(-)

-- 
2.27.0

[dpdk-dev] [PATCH v2 1/2] net/cxgbe: add support for xstats API

[Rahul Lakkireddy]

Add support to fetch port and queue stats via xstats API.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
v2:
- Perform explicit checks for NULL in conditions.
- Use rte_strlcpy() instead of strcpy().

 drivers/net/cxgbe/cxgbe_ethdev.c | 303 ++++++++++++++++++++++++++++++-
 1 file changed, 294 insertions(+), 9 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 550843b4d7..5bbf7f28af 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -778,24 +778,304 @@ static int cxgbe_dev_stats_reset(struct rte_eth_dev *eth_dev)
 
 	cxgbe_stats_reset(pi);
 	for (i = 0; i < pi->n_rx_qsets; i++) {
-		struct sge_eth_rxq *rxq =
-			&s->ethrxq[pi->first_rxqset + i];
+		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + i];
 
-		rxq->stats.pkts = 0;
-		rxq->stats.rx_bytes = 0;
+		memset(&rxq->stats, 0, sizeof(rxq->stats));
 	}
 	for (i = 0; i < pi->n_tx_qsets; i++) {
-		struct sge_eth_txq *txq =
-			&s->ethtxq[pi->first_txqset + i];
+		struct sge_eth_txq *txq = &s->ethtxq[pi->first_txqset + i];
 
-		txq->stats.pkts = 0;
-		txq->stats.tx_bytes = 0;
-		txq->stats.mapping_err = 0;
+		memset(&txq->stats, 0, sizeof(txq->stats));
 	}
 
 	return 0;
 }
 
+/* Store extended statistics names and its offset in stats structure  */
+struct cxgbe_dev_xstats_name_off {
+	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	unsigned int offset;
+};
+
+static const struct cxgbe_dev_xstats_name_off cxgbe_dev_rxq_stats_strings[] = {
+	{"packets", offsetof(struct sge_eth_rx_stats, pkts)},
+	{"bytes", offsetof(struct sge_eth_rx_stats, rx_bytes)},
+	{"checksum_offloads", offsetof(struct sge_eth_rx_stats, rx_cso)},
+	{"vlan_extractions", offsetof(struct sge_eth_rx_stats, vlan_ex)},
+	{"dropped_packets", offsetof(struct sge_eth_rx_stats, rx_drops)},
+};
+
+static const struct cxgbe_dev_xstats_name_off cxgbe_dev_txq_stats_strings[] = {
+	{"packets", offsetof(struct sge_eth_tx_stats, pkts)},
+	{"bytes", offsetof(struct sge_eth_tx_stats, tx_bytes)},
+	{"tso_requests", offsetof(struct sge_eth_tx_stats, tso)},
+	{"checksum_offloads", offsetof(struct sge_eth_tx_stats, tx_cso)},
+	{"vlan_insertions", offsetof(struct sge_eth_tx_stats, vlan_ins)},
+	{"packet_mapping_errors",
+	 offsetof(struct sge_eth_tx_stats, mapping_err)},
+	{"coalesced_wrs", offsetof(struct sge_eth_tx_stats, coal_wr)},
+	{"coalesced_packets", offsetof(struct sge_eth_tx_stats, coal_pkts)},
+};
+
+static const struct cxgbe_dev_xstats_name_off cxgbe_dev_port_stats_strings[] = {
+	{"tx_bytes", offsetof(struct port_stats, tx_octets)},
+	{"tx_packets", offsetof(struct port_stats, tx_frames)},
+	{"tx_broadcast_packets", offsetof(struct port_stats, tx_bcast_frames)},
+	{"tx_multicast_packets", offsetof(struct port_stats, tx_mcast_frames)},
+	{"tx_unicast_packets", offsetof(struct port_stats, tx_ucast_frames)},
+	{"tx_error_packets", offsetof(struct port_stats, tx_error_frames)},
+	{"tx_size_64_packets", offsetof(struct port_stats, tx_frames_64)},
+	{"tx_size_65_to_127_packets",
+	 offsetof(struct port_stats, tx_frames_65_127)},
+	{"tx_size_128_to_255_packets",
+	 offsetof(struct port_stats, tx_frames_128_255)},
+	{"tx_size_256_to_511_packets",
+	 offsetof(struct port_stats, tx_frames_256_511)},
+	{"tx_size_512_to_1023_packets",
+	 offsetof(struct port_stats, tx_frames_512_1023)},
+	{"tx_size_1024_to_1518_packets",
+	 offsetof(struct port_stats, tx_frames_1024_1518)},
+	{"tx_size_1519_to_max_packets",
+	 offsetof(struct port_stats, tx_frames_1519_max)},
+	{"tx_drop_packets", offsetof(struct port_stats, tx_drop)},
+	{"tx_pause_frames", offsetof(struct port_stats, tx_pause)},
+	{"tx_ppp_pri0_packets", offsetof(struct port_stats, tx_ppp0)},
+	{"tx_ppp_pri1_packets", offsetof(struct port_stats, tx_ppp1)},
+	{"tx_ppp_pri2_packets", offsetof(struct port_stats, tx_ppp2)},
+	{"tx_ppp_pri3_packets", offsetof(struct port_stats, tx_ppp3)},
+	{"tx_ppp_pri4_packets", offsetof(struct port_stats, tx_ppp4)},
+	{"tx_ppp_pri5_packets", offsetof(struct port_stats, tx_ppp5)},
+	{"tx_ppp_pri6_packets", offsetof(struct port_stats, tx_ppp6)},
+	{"tx_ppp_pri7_packets", offsetof(struct port_stats, tx_ppp7)},
+	{"rx_bytes", offsetof(struct port_stats, rx_octets)},
+	{"rx_packets", offsetof(struct port_stats, rx_frames)},
+	{"rx_broadcast_packets", offsetof(struct port_stats, rx_bcast_frames)},
+	{"rx_multicast_packets", offsetof(struct port_stats, rx_mcast_frames)},
+	{"rx_unicast_packets", offsetof(struct port_stats, rx_ucast_frames)},
+	{"rx_too_long_packets", offsetof(struct port_stats, rx_too_long)},
+	{"rx_jabber_packets", offsetof(struct port_stats, rx_jabber)},
+	{"rx_fcs_error_packets", offsetof(struct port_stats, rx_fcs_err)},
+	{"rx_length_error_packets", offsetof(struct port_stats, rx_len_err)},
+	{"rx_symbol_error_packets",
+	 offsetof(struct port_stats, rx_symbol_err)},
+	{"rx_short_packets", offsetof(struct port_stats, rx_runt)},
+	{"rx_size_64_packets", offsetof(struct port_stats, rx_frames_64)},
+	{"rx_size_65_to_127_packets",
+	 offsetof(struct port_stats, rx_frames_65_127)},
+	{"rx_size_128_to_255_packets",
+	 offsetof(struct port_stats, rx_frames_128_255)},
+	{"rx_size_256_to_511_packets",
+	 offsetof(struct port_stats, rx_frames_256_511)},
+	{"rx_size_512_to_1023_packets",
+	 offsetof(struct port_stats, rx_frames_512_1023)},
+	{"rx_size_1024_to_1518_packets",
+	 offsetof(struct port_stats, rx_frames_1024_1518)},
+	{"rx_size_1519_to_max_packets",
+	 offsetof(struct port_stats, rx_frames_1519_max)},
+	{"rx_pause_packets", offsetof(struct port_stats, rx_pause)},
+	{"rx_ppp_pri0_packets", offsetof(struct port_stats, rx_ppp0)},
+	{"rx_ppp_pri1_packets", offsetof(struct port_stats, rx_ppp1)},
+	{"rx_ppp_pri2_packets", offsetof(struct port_stats, rx_ppp2)},
+	{"rx_ppp_pri3_packets", offsetof(struct port_stats, rx_ppp3)},
+	{"rx_ppp_pri4_packets", offsetof(struct port_stats, rx_ppp4)},
+	{"rx_ppp_pri5_packets", offsetof(struct port_stats, rx_ppp5)},
+	{"rx_ppp_pri6_packets", offsetof(struct port_stats, rx_ppp6)},
+	{"rx_ppp_pri7_packets", offsetof(struct port_stats, rx_ppp7)},
+	{"rx_bg0_dropped_packets", offsetof(struct port_stats, rx_ovflow0)},
+	{"rx_bg1_dropped_packets", offsetof(struct port_stats, rx_ovflow1)},
+	{"rx_bg2_dropped_packets", offsetof(struct port_stats, rx_ovflow2)},
+	{"rx_bg3_dropped_packets", offsetof(struct port_stats, rx_ovflow3)},
+	{"rx_bg0_truncated_packets", offsetof(struct port_stats, rx_trunc0)},
+	{"rx_bg1_truncated_packets", offsetof(struct port_stats, rx_trunc1)},
+	{"rx_bg2_truncated_packets", offsetof(struct port_stats, rx_trunc2)},
+	{"rx_bg3_truncated_packets", offsetof(struct port_stats, rx_trunc3)},
+};
+
+#define CXGBE_NB_RXQ_STATS RTE_DIM(cxgbe_dev_rxq_stats_strings)
+#define CXGBE_NB_TXQ_STATS RTE_DIM(cxgbe_dev_txq_stats_strings)
+#define CXGBE_NB_PORT_STATS RTE_DIM(cxgbe_dev_port_stats_strings)
+
+static u16 cxgbe_dev_xstats_count(struct port_info *pi)
+{
+	return CXGBE_NB_PORT_STATS +
+	       (pi->n_tx_qsets * CXGBE_NB_TXQ_STATS) +
+	       (pi->n_rx_qsets * CXGBE_NB_RXQ_STATS);
+}
+
+static int cxgbe_dev_xstats(struct rte_eth_dev *dev,
+			    struct rte_eth_xstat_name *xstats_names,
+			    struct rte_eth_xstat *xstats, unsigned int size)
+{
+	const struct cxgbe_dev_xstats_name_off *xstats_str;
+	struct port_info *pi = dev->data->dev_private;
+	struct adapter *adap = pi->adapter;
+	struct sge *s = &adap->sge;
+	struct port_stats ps;
+	u16 count, i, qid;
+	u64 *stats_ptr;
+
+	count = cxgbe_dev_xstats_count(pi);
+	if (size < count)
+		return count;
+
+	/* port stats */
+	cxgbe_stats_get(pi, &ps);
+
+	count = 0;
+	xstats_str = cxgbe_dev_port_stats_strings;
+	for (i = 0; i < CXGBE_NB_PORT_STATS; i++, count++) {
+		if (xstats_names != NULL)
+			snprintf(xstats_names[count].name,
+				 sizeof(xstats_names[count].name),
+				 "%s", xstats_str[i].name);
+		if (xstats != NULL) {
+			stats_ptr = RTE_PTR_ADD(&ps,
+						xstats_str[i].offset);
+			xstats[count].value = *stats_ptr;
+			xstats[count].id = count;
+		}
+	}
+
+	/* per-txq stats */
+	xstats_str = cxgbe_dev_txq_stats_strings;
+	for (qid = 0; qid < pi->n_tx_qsets; qid++) {
+		struct sge_eth_txq *txq = &s->ethtxq[pi->first_txqset + qid];
+
+		for (i = 0; i < CXGBE_NB_TXQ_STATS; i++, count++) {
+			if (xstats_names != NULL)
+				snprintf(xstats_names[count].name,
+					 sizeof(xstats_names[count].name),
+					 "tx_q%u_%s",
+					 qid, xstats_str[i].name);
+			if (xstats != NULL) {
+				stats_ptr = RTE_PTR_ADD(&txq->stats,
+							xstats_str[i].offset);
+				xstats[count].value = *stats_ptr;
+				xstats[count].id = count;
+			}
+		}
+	}
+
+	/* per-rxq stats */
+	xstats_str = cxgbe_dev_rxq_stats_strings;
+	for (qid = 0; qid < pi->n_rx_qsets; qid++) {
+		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + qid];
+
+		for (i = 0; i < CXGBE_NB_RXQ_STATS; i++, count++) {
+			if (xstats_names != NULL)
+				snprintf(xstats_names[count].name,
+					 sizeof(xstats_names[count].name),
+					 "rx_q%u_%s",
+					 qid, xstats_str[i].name);
+			if (xstats != NULL) {
+				stats_ptr = RTE_PTR_ADD(&rxq->stats,
+							xstats_str[i].offset);
+				xstats[count].value = *stats_ptr;
+				xstats[count].id = count;
+			}
+		}
+	}
+
+	return count;
+}
+
+/* Get port extended statistics by id.
+ */
+static int cxgbe_dev_xstats_get_by_id(struct rte_eth_dev *dev,
+				      const uint64_t *ids, uint64_t *values,
+				      unsigned int n)
+{
+	struct port_info *pi = dev->data->dev_private;
+	struct rte_eth_xstat *xstats_copy;
+	u16 count, i;
+	int ret = 0;
+
+	count = cxgbe_dev_xstats_count(pi);
+	if (ids == NULL || values == NULL)
+		return count;
+
+	xstats_copy = rte_calloc(NULL, count, sizeof(*xstats_copy), 0);
+	if (xstats_copy == NULL)
+		return -ENOMEM;
+
+	cxgbe_dev_xstats(dev, NULL, xstats_copy, count);
+
+	for (i = 0; i < n; i++) {
+		if (ids[i] >= count) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+		values[i] = xstats_copy[ids[i]].value;
+	}
+
+	ret = n;
+
+out_err:
+	rte_free(xstats_copy);
+	return ret;
+}
+
+/* Get names of port extended statistics by id.
+ */
+static int cxgbe_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
+					    struct rte_eth_xstat_name *xnames,
+					    const uint64_t *ids, unsigned int n)
+{
+	struct port_info *pi = dev->data->dev_private;
+	struct rte_eth_xstat_name *xnames_copy;
+	u16 count, i;
+	int ret = 0;
+
+	count = cxgbe_dev_xstats_count(pi);
+	if (ids == NULL || xnames == NULL)
+		return count;
+
+	xnames_copy = rte_calloc(NULL, count, sizeof(*xnames_copy), 0);
+	if (xnames_copy == NULL)
+		return -ENOMEM;
+
+	cxgbe_dev_xstats(dev, xnames_copy, NULL, count);
+
+	for (i = 0; i < n; i++) {
+		if (ids[i] >= count) {
+			ret = -EINVAL;
+			goto out_err;
+		}
+		rte_strlcpy(xnames[i].name, xnames_copy[ids[i]].name,
+			    sizeof(xnames[i].name));
+	}
+
+	ret = n;
+
+out_err:
+	rte_free(xnames_copy);
+	return ret;
+}
+
+/* Get port extended statistics.
+ */
+static int cxgbe_dev_xstats_get(struct rte_eth_dev *dev,
+				struct rte_eth_xstat *xstats, unsigned int n)
+{
+	return cxgbe_dev_xstats(dev, NULL, xstats, n);
+}
+
+/* Get names of port extended statistics.
+ */
+static int cxgbe_dev_xstats_get_names(struct rte_eth_dev *dev,
+				      struct rte_eth_xstat_name *xstats_names,
+				      unsigned int n)
+{
+	return cxgbe_dev_xstats(dev, xstats_names, NULL, n);
+}
+
+/* Reset port extended statistics.
+ */
+static int cxgbe_dev_xstats_reset(struct rte_eth_dev *dev)
+{
+	return cxgbe_dev_stats_reset(dev);
+}
+
 static int cxgbe_flow_ctrl_get(struct rte_eth_dev *eth_dev,
 			       struct rte_eth_fc_conf *fc_conf)
 {
@@ -1351,6 +1631,11 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = {
 	.flow_ops_get           = cxgbe_dev_flow_ops_get,
 	.stats_get		= cxgbe_dev_stats_get,
 	.stats_reset		= cxgbe_dev_stats_reset,
+	.xstats_get             = cxgbe_dev_xstats_get,
+	.xstats_get_by_id       = cxgbe_dev_xstats_get_by_id,
+	.xstats_get_names       = cxgbe_dev_xstats_get_names,
+	.xstats_get_names_by_id = cxgbe_dev_xstats_get_names_by_id,
+	.xstats_reset           = cxgbe_dev_xstats_reset,
 	.flow_ctrl_get		= cxgbe_flow_ctrl_get,
 	.flow_ctrl_set		= cxgbe_flow_ctrl_set,
 	.get_eeprom_length	= cxgbe_get_eeprom_length,
-- 
2.27.0

[dpdk-dev] [PATCH v2 2/2] net/cxgbe: remove queue stats from basic stats

[Rahul Lakkireddy]

Remove queue stats from basic stats because they're now available
via xstats API. Also remove RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
v2:
- No change.

 drivers/net/cxgbe/cxgbe_ethdev.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 5bbf7f28af..57888fdc12 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -747,22 +747,12 @@ static int cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
 	eth_stats->oerrors  = ps.tx_error_frames;
 
 	for (i = 0; i < pi->n_rx_qsets; i++) {
-		struct sge_eth_rxq *rxq =
-			&s->ethrxq[pi->first_rxqset + i];
+		struct sge_eth_rxq *rxq = &s->ethrxq[pi->first_rxqset + i];
 
-		eth_stats->q_ipackets[i] = rxq->stats.pkts;
-		eth_stats->q_ibytes[i] = rxq->stats.rx_bytes;
-		eth_stats->ipackets += eth_stats->q_ipackets[i];
-		eth_stats->ibytes += eth_stats->q_ibytes[i];
+		eth_stats->ipackets += rxq->stats.pkts;
+		eth_stats->ibytes += rxq->stats.rx_bytes;
 	}
 
-	for (i = 0; i < pi->n_tx_qsets; i++) {
-		struct sge_eth_txq *txq =
-			&s->ethtxq[pi->first_txqset + i];
-
-		eth_stats->q_opackets[i] = txq->stats.pkts;
-		eth_stats->q_obytes[i] = txq->stats.tx_bytes;
-	}
 	return 0;
 }
 
@@ -1698,8 +1688,6 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
-	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
-
 	snprintf(name, sizeof(name), "cxgbeadapter%d", eth_dev->data->port_id);
 	adapter = rte_zmalloc(name, sizeof(*adapter), 0);
 	if (!adapter)
-- 
2.27.0

Re: [dpdk-dev] [PATCH v2 0/2] net/cxgbe: add support for xstats API

[Andrew Rybchenko]

On 7/1/21 7:56 PM, Rahul Lakkireddy wrote:
> This series of patches add support to fetch port and queue stats via
> xstats API.
> 
> Patch 1 adds support to fetch port and queue stats via xstats API.

Fixed mulit-line comments to following DPDK coding style.
Just made it single-line.

> Patch 2 removes queue stats from basic stats since they're available
> via xstats API. Also removes RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag.
> 
> ---
> v2:
> - Perform explicit checks for NULL in conditions.
> - Use rte_strlcpy() instead of strcpy().
> 
> Rahul Lakkireddy (2):
>   net/cxgbe: add support for xstats API
>   net/cxgbe: remove queue stats from basic stats
> 
>  drivers/net/cxgbe/cxgbe_ethdev.c | 321 ++++++++++++++++++++++++++++---
>  1 file changed, 297 insertions(+), 24 deletions(-)
> 

Applied, thanks.