[PATCH 0/2] NFP extend stats

[PATCH 0/2] NFP extend stats

[Chaoyong He]

This patch series related with NFP xstats logic:
- Fix one bug about xstats when using coreNIC firmware with multi-PF.
- Add the support of xstats for flower firmware.

Chaoyong He (2):
  net/nfp: fix xstats problem for multi PF firmware
  net/nfp: add support of xstats for flower firmware

 .../net/nfp/flower/nfp_flower_representor.c   | 47 ++++++++++++++-
 .../net/nfp/flower/nfp_flower_representor.h   |  4 ++
 drivers/net/nfp/nfp_ethdev.c                  | 50 +++++++---------
 drivers/net/nfp/nfp_net_common.c              | 58 +++++++++++++++----
 drivers/net/nfp/nfp_net_common.h              |  5 +-
 5 files changed, 121 insertions(+), 43 deletions(-)

-- 
2.39.1

[PATCH 1/2] net/nfp: fix xstats problem for multi PF firmware

[Chaoyong He]

When using multi PF firmware, the other ports always get the
xstats of the first port.

Fix it by adding the offset for other ports.

Fixes: 8ad2cc8fec37 ("net/nfp: add flag for multiple PFs support")
Cc: peng.zhang@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c     | 50 ++++++++++++++------------------
 drivers/net/nfp/nfp_net_common.h |  5 ++--
 2 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index cdc946faff..f98af2044b 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -585,8 +585,6 @@ nfp_net_uninit(struct rte_eth_dev *eth_dev)
 	if ((net_hw->super.cap & NFP_NET_CFG_CTRL_TXRWB) != 0)
 		nfp_net_txrwb_free(eth_dev);
 	nfp_ipsec_uninit(eth_dev);
-	if (net_hw->mac_stats_area != NULL)
-		nfp_cpp_area_release_free(net_hw->mac_stats_area);
 
 	return 0;
 }
@@ -617,6 +615,7 @@ nfp_pf_uninit(struct nfp_net_hw_priv *hw_priv)
 {
 	struct nfp_pf_dev *pf_dev = hw_priv->pf_dev;
 
+	nfp_cpp_area_release_free(pf_dev->mac_stats_area);
 	nfp_cpp_area_release_free(pf_dev->qc_area);
 	free(pf_dev->sym_tbl);
 	if (pf_dev->multi_pf.enabled) {
@@ -933,43 +932,31 @@ nfp_net_init(struct rte_eth_dev *eth_dev,
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
-	if (port == 0 || pf_dev->multi_pf.enabled) {
-		uint32_t min_size;
-
+	if (pf_dev->multi_pf.enabled)
 		hw->ctrl_bar = pf_dev->ctrl_bar;
-		min_size = NFP_MAC_STATS_SIZE * pf_dev->nfp_eth_table->max_index;
-		net_hw->mac_stats_bar = nfp_rtsym_map(pf_dev->sym_tbl, "_mac_stats",
-				min_size, &net_hw->mac_stats_area);
-		if (net_hw->mac_stats_bar == NULL) {
-			PMD_INIT_LOG(ERR, "nfp_rtsym_map fails for _mac_stats_bar");
-			return -EIO;
-		}
-
-		net_hw->mac_stats = net_hw->mac_stats_bar;
-	} else {
-		/* Use port offset in pf ctrl_bar for this ports control bar */
+	else
 		hw->ctrl_bar = pf_dev->ctrl_bar + (port * NFP_NET_CFG_BAR_SZ);
-		net_hw->mac_stats = app_fw_nic->ports[0]->mac_stats_bar +
+
+	net_hw->mac_stats = pf_dev->mac_stats_bar +
 				(net_hw->nfp_idx * NFP_MAC_STATS_SIZE);
-	}
 
 	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
 	PMD_INIT_LOG(DEBUG, "MAC stats: %p", net_hw->mac_stats);
 
 	err = nfp_net_common_init(pci_dev, net_hw);
 	if (err != 0)
-		goto free_area;
+		return err;
 
 	err = nfp_net_tlv_caps_parse(eth_dev);
 	if (err != 0) {
 		PMD_INIT_LOG(ERR, "Failed to parser TLV caps");
-		goto free_area;
+		return err;
 	}
 
 	err = nfp_ipsec_init(eth_dev);
 	if (err != 0) {
 		PMD_INIT_LOG(ERR, "Failed to init IPsec module");
-		goto free_area;
+		return err;
 	}
 
 	nfp_net_ethdev_ops_mount(net_hw, eth_dev);
@@ -1070,9 +1057,6 @@ nfp_net_init(struct rte_eth_dev *eth_dev,
 	rte_free(net_hw->eth_xstats_base);
 ipsec_exit:
 	nfp_ipsec_uninit(eth_dev);
-free_area:
-	if (net_hw->mac_stats_area != NULL)
-		nfp_cpp_area_release_free(net_hw->mac_stats_area);
 
 	return err;
 }
@@ -1931,6 +1915,14 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
 
 	PMD_INIT_LOG(DEBUG, "qc_bar address: %p", pf_dev->qc_bar);
 
+	pf_dev->mac_stats_bar = nfp_rtsym_map(sym_tbl, "_mac_stats",
+			NFP_MAC_STATS_SIZE * nfp_eth_table->max_index,
+			&pf_dev->mac_stats_area);
+	if (pf_dev->mac_stats_bar == NULL) {
+		PMD_INIT_LOG(ERR, "nfp_rtsym_map fails for _mac_stats");
+		goto hwqueues_cleanup;
+	}
+
 	hw_priv->pf_dev = pf_dev;
 	hw_priv->dev_info = dev_info;
 
@@ -1943,14 +1935,14 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
 		if (pf_dev->multi_pf.enabled) {
 			ret = nfp_enable_multi_pf(pf_dev);
 			if (ret != 0)
-				goto hwqueues_cleanup;
+				goto mac_stats_cleanup;
 		}
 
 		PMD_INIT_LOG(INFO, "Initializing coreNIC");
 		ret = nfp_init_app_fw_nic(hw_priv);
 		if (ret != 0) {
 			PMD_INIT_LOG(ERR, "Could not initialize coreNIC!");
-			goto hwqueues_cleanup;
+			goto mac_stats_cleanup;
 		}
 		break;
 	case NFP_APP_FW_FLOWER_NIC:
@@ -1958,13 +1950,13 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
 		ret = nfp_init_app_fw_flower(hw_priv);
 		if (ret != 0) {
 			PMD_INIT_LOG(ERR, "Could not initialize Flower!");
-			goto hwqueues_cleanup;
+			goto mac_stats_cleanup;
 		}
 		break;
 	default:
 		PMD_INIT_LOG(ERR, "Unsupported Firmware loaded");
 		ret = -EINVAL;
-		goto hwqueues_cleanup;
+		goto mac_stats_cleanup;
 	}
 
 	/* Register the CPP bridge service here for primary use */
@@ -1974,6 +1966,8 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
 
 	return 0;
 
+mac_stats_cleanup:
+	nfp_cpp_area_release_free(pf_dev->mac_stats_area);
 hwqueues_cleanup:
 	nfp_cpp_area_release_free(pf_dev->qc_area);
 sym_tbl_cleanup:
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index a4eaed6433..8d9851f969 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -116,6 +116,9 @@ struct nfp_pf_dev {
 
 	uint8_t *qc_bar;
 
+	struct nfp_cpp_area *mac_stats_area;
+	uint8_t *mac_stats_bar;
+
 	struct nfp_hwinfo *hwinfo;
 	struct nfp_rtsym_table *sym_tbl;
 
@@ -199,8 +202,6 @@ struct nfp_net_hw {
 	struct rte_eth_xstat *eth_xstats_base;
 
 	struct nfp_cpp_area *ctrl_area;
-	struct nfp_cpp_area *mac_stats_area;
-	uint8_t *mac_stats_bar;
 	uint8_t *mac_stats;
 
 	/** Sequential physical port number, only valid for CoreNIC firmware */
-- 
2.39.1

[PATCH 2/2] net/nfp: add support of xstats for flower firmware

[Chaoyong He]

Add support the extend stats for flower firmware, include the stats for
each queue.

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 .../net/nfp/flower/nfp_flower_representor.c   | 47 ++++++++++++++-
 .../net/nfp/flower/nfp_flower_representor.h   |  4 ++
 drivers/net/nfp/nfp_net_common.c              | 58 +++++++++++++++----
 3 files changed, 96 insertions(+), 13 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c b/drivers/net/nfp/flower/nfp_flower_representor.c
index f9001ab1f0..f1451fcca2 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.c
+++ b/drivers/net/nfp/flower/nfp_flower_representor.c
@@ -199,7 +199,7 @@ nfp_flower_repr_stats_get(struct rte_eth_dev *ethdev,
 	return 0;
 }
 
-static int
+int
 nfp_flower_repr_stats_reset(struct rte_eth_dev *ethdev)
 {
 	struct nfp_flower_representor *repr;
@@ -228,6 +228,8 @@ nfp_flower_repr_rx_burst(void *rx_queue,
 		struct rte_mbuf **rx_pkts,
 		uint16_t nb_pkts)
 {
+	uint32_t i;
+	uint32_t data_len;
 	unsigned int available = 0;
 	unsigned int total_dequeue;
 	struct nfp_net_rxq *rxq;
@@ -255,7 +257,13 @@ nfp_flower_repr_rx_burst(void *rx_queue,
 				"received: %u, available: %u", repr->name,
 				repr->port_id, total_dequeue, available);
 
+		data_len = 0;
+		for (i = 0; i < total_dequeue; i++)
+			data_len += rx_pkts[i]->data_len;
+
 		repr->repr_stats.ipackets += total_dequeue;
+		repr->repr_stats.q_ipackets[rxq->qidx] += total_dequeue;
+		repr->repr_stats.q_ibytes[rxq->qidx] += data_len;
 	}
 
 	return total_dequeue;
@@ -268,6 +276,7 @@ nfp_flower_repr_tx_burst(void *tx_queue,
 {
 	uint16_t i;
 	uint16_t sent;
+	uint32_t data_len;
 	void *pf_tx_queue;
 	struct nfp_net_txq *txq;
 	struct rte_eth_dev *dev;
@@ -297,7 +306,14 @@ nfp_flower_repr_tx_burst(void *tx_queue,
 	if (sent != 0) {
 		PMD_TX_LOG(DEBUG, "Representor Tx burst for %s, port_id: %#x transmitted: %hu",
 				repr->name, repr->port_id, sent);
+
+		data_len = 0;
+		for (i = 0; i < sent; i++)
+			data_len += tx_pkts[i]->data_len;
+
 		repr->repr_stats.opackets += sent;
+		repr->repr_stats.q_opackets[txq->qidx] += sent;
+		repr->repr_stats.q_obytes[txq->qidx] += data_len;
 	}
 
 	return sent;
@@ -356,6 +372,7 @@ nfp_flower_repr_uninit(struct rte_eth_dev *eth_dev)
 	struct nfp_flower_representor *repr;
 
 	repr = eth_dev->data->dev_private;
+	rte_free(repr->repr_xstats_base);
 	rte_ring_free(repr->ring);
 
 	if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
@@ -497,6 +514,12 @@ static const struct eth_dev_ops nfp_flower_repr_dev_ops = {
 
 	.flow_ops_get         = nfp_flow_ops_get,
 	.mtr_ops_get          = nfp_net_mtr_ops_get,
+
+	.xstats_get             = nfp_net_xstats_get,
+	.xstats_reset           = nfp_net_xstats_reset,
+	.xstats_get_names       = nfp_net_xstats_get_names,
+	.xstats_get_by_id       = nfp_net_xstats_get_by_id,
+	.xstats_get_names_by_id = nfp_net_xstats_get_names_by_id,
 };
 
 static uint32_t
@@ -548,7 +571,8 @@ nfp_flower_pf_repr_init(struct rte_eth_dev *eth_dev,
 	eth_dev->dev_ops = &nfp_flower_pf_repr_dev_ops;
 	eth_dev->rx_pkt_burst = nfp_net_recv_pkts;
 	eth_dev->tx_pkt_burst = nfp_flower_pf_xmit_pkts;
-	eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
+			RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
 	eth_dev->data->representor_id = 0;
 
@@ -582,6 +606,7 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 	int ret;
 	uint16_t index;
 	unsigned int numa_node;
+	struct nfp_net_hw_priv *hw_priv;
 	char ring_name[RTE_ETH_NAME_MAX_LEN];
 	struct nfp_app_fw_flower *app_fw_flower;
 	struct nfp_flower_representor *repr;
@@ -593,6 +618,7 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 
 	/* Memory has been allocated in the eth_dev_create() function */
 	repr = eth_dev->data->dev_private;
+	hw_priv = eth_dev->process_private;
 
 	/*
 	 * We need multiproduce rings as we can have multiple PF ports.
@@ -620,7 +646,8 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 	eth_dev->dev_ops = &nfp_flower_repr_dev_ops;
 	eth_dev->rx_pkt_burst = nfp_flower_repr_rx_burst;
 	eth_dev->tx_pkt_burst = nfp_flower_repr_tx_burst;
-	eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
+			RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
 	if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT)
 		eth_dev->data->representor_id = repr->vf_id;
@@ -662,6 +689,20 @@ nfp_flower_repr_init(struct rte_eth_dev *eth_dev,
 		app_fw_flower->vf_reprs[index] = repr;
 	}
 
+	if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
+		repr->mac_stats = hw_priv->pf_dev->mac_stats_bar +
+				(repr->nfp_idx * NFP_MAC_STATS_SIZE);
+	}
+
+	/* Allocate memory for extended statistics counters */
+	repr->repr_xstats_base = rte_zmalloc("rte_eth_xstat",
+			sizeof(struct rte_eth_xstat) * nfp_net_xstats_size(eth_dev), 0);
+	if (repr->repr_xstats_base == NULL) {
+		PMD_INIT_LOG(ERR, "No memory for xstats base on device %s!", repr->name);
+		ret = -ENOMEM;
+		goto mac_cleanup;
+	}
+
 	return 0;
 
 mac_cleanup:
diff --git a/drivers/net/nfp/flower/nfp_flower_representor.h b/drivers/net/nfp/flower/nfp_flower_representor.h
index d539e53b23..c068c4462c 100644
--- a/drivers/net/nfp/flower/nfp_flower_representor.h
+++ b/drivers/net/nfp/flower/nfp_flower_representor.h
@@ -20,10 +20,14 @@ struct nfp_flower_representor {
 	struct rte_ring *ring;
 	struct rte_eth_link link;
 	struct rte_eth_stats repr_stats;
+
+	struct rte_eth_xstat *repr_xstats_base;
+	uint8_t *mac_stats;
 };
 
 int nfp_flower_repr_create(struct nfp_app_fw_flower *app_fw_flower,
 		struct nfp_net_hw_priv *hw_priv);
 bool nfp_flower_repr_is_vf(struct nfp_flower_representor *repr);
+int nfp_flower_repr_stats_reset(struct rte_eth_dev *ethdev);
 
 #endif /* __NFP_FLOWER_REPRESENTOR_H__ */
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index 7541afa235..260920ecff 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -954,12 +954,23 @@ uint32_t
 nfp_net_xstats_size(const struct rte_eth_dev *dev)
 {
 	uint32_t count;
+	bool vf_flag = false;
 	struct nfp_net_hw *hw;
+	struct nfp_flower_representor *repr;
 	const uint32_t size = RTE_DIM(nfp_net_xstats);
 
-	/* If the device is a VF, then there will be no MAC stats */
-	hw = nfp_net_get_hw(dev);
-	if (hw->mac_stats == NULL) {
+	if (rte_eth_dev_is_repr(dev)) {
+		repr = dev->data->dev_private;
+		if (repr->mac_stats == NULL)
+			vf_flag = true;
+	} else {
+		hw = dev->data->dev_private;
+		if (hw->mac_stats == NULL)
+			vf_flag = true;
+	}
+
+	/* If the device is a VF or VF-repr, then there will be no MAC stats */
+	if (vf_flag) {
 		for (count = 0; count < size; count++) {
 			if (nfp_net_xstats[count].group == NFP_XSTAT_GROUP_MAC)
 				break;
@@ -989,14 +1000,29 @@ nfp_net_xstats_value(const struct rte_eth_dev *dev,
 		bool raw)
 {
 	uint64_t value;
+	uint8_t *mac_stats;
 	struct nfp_net_hw *hw;
 	struct nfp_xstat xstat;
+	struct rte_eth_xstat *xstats_base;
+	struct nfp_flower_representor *repr;
+
+	if (rte_eth_dev_is_repr(dev)) {
+		repr = dev->data->dev_private;
+		hw = repr->app_fw_flower->pf_hw;
+
+		mac_stats = repr->mac_stats;
+		xstats_base = repr->repr_xstats_base;
+	} else {
+		hw = dev->data->dev_private;
+
+		mac_stats = hw->mac_stats;
+		xstats_base = hw->eth_xstats_base;
+	}
 
-	hw = nfp_net_get_hw(dev);
 	xstat = nfp_net_xstats[index];
 
 	if (xstat.group == NFP_XSTAT_GROUP_MAC)
-		value = nn_readq(hw->mac_stats + xstat.offset);
+		value = nn_readq(mac_stats + xstat.offset);
 	else
 		value = nn_cfg_readq(&hw->super, xstat.offset);
 
@@ -1009,7 +1035,7 @@ nfp_net_xstats_value(const struct rte_eth_dev *dev,
 	 * baseline value. The result is the count of this statistic since the last time
 	 * it was "reset".
 	 */
-	return value - hw->eth_xstats_base[index].value;
+	return value - xstats_base[index].value;
 }
 
 /* NOTE: All callers ensure dev is always set. */
@@ -1130,17 +1156,29 @@ nfp_net_xstats_reset(struct rte_eth_dev *dev)
 	uint32_t id;
 	uint32_t read_size;
 	struct nfp_net_hw *hw;
+	struct rte_eth_xstat *xstats_base;
+	struct nfp_flower_representor *repr;
 
-	hw = nfp_net_get_hw(dev);
 	read_size = nfp_net_xstats_size(dev);
 
+	if (rte_eth_dev_is_repr(dev)) {
+		repr = dev->data->dev_private;
+		xstats_base = repr->repr_xstats_base;
+	} else {
+		hw = dev->data->dev_private;
+		xstats_base = hw->eth_xstats_base;
+	}
+
 	for (id = 0; id < read_size; id++) {
-		hw->eth_xstats_base[id].id = id;
-		hw->eth_xstats_base[id].value = nfp_net_xstats_value(dev, id, true);
+		xstats_base[id].id = id;
+		xstats_base[id].value = nfp_net_xstats_value(dev, id, true);
 	}
 
 	/* Successfully reset xstats, now call function to reset basic stats. */
-	return nfp_net_stats_reset(dev);
+	if (rte_eth_dev_is_repr(dev))
+		return nfp_flower_repr_stats_reset(dev);
+	else
+		return nfp_net_stats_reset(dev);
 }
 
 void
-- 
2.39.1

Re: [PATCH 0/2] NFP extend stats

[Ferruh Yigit]

On 5/23/2024 3:49 AM, Chaoyong He wrote:
> This patch series related with NFP xstats logic:
> - Fix one bug about xstats when using coreNIC firmware with multi-PF.
> - Add the support of xstats for flower firmware.
> 
> Chaoyong He (2):
>   net/nfp: fix xstats problem for multi PF firmware
>   net/nfp: add support of xstats for flower firmware
>

Series applied to dpdk-next-net/main, thanks.