[PATCH 0/2] configure RSS and handle metadata correctly

[PATCH 0/2] configure RSS and handle metadata correctly

[Chaoyong He]

Select the appropriate RSS configuration in the initial process logic
and modify the logic of processing metadata based on RSS configuration
in the RX function.

Long Wu (2):
  net/nfp: standardize the use of RSS-related codes
  net/nfp: modify RSS's processing logic

 drivers/net/nfp/nfp_common.c    |  25 +++++++-
 drivers/net/nfp/nfp_common.h    |   7 +++
 drivers/net/nfp/nfp_ctrl.h      |  18 +++++-
 drivers/net/nfp/nfp_ethdev.c    |   7 +--
 drivers/net/nfp/nfp_ethdev_vf.c |   7 +--
 drivers/net/nfp/nfp_rxtx.c      | 108 ++++++++++++++++++++------------
 6 files changed, 122 insertions(+), 50 deletions(-)

-- 
2.29.3

[PATCH 1/2] net/nfp: standardize the use of RSS-related codes

[Chaoyong He]

From: Long Wu <long.wu@corigine.com>

The usage of RTE_ETH_MQ_RX_RSS and RTE_ETH_MQ_RX_RSS_FLAG are mixed in
nfp_net_configure(), use RTE_ETH_MQ_RX_RSS_FLAG uniformly.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/nfp_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index 907777a9e4..a545a10013 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -171,7 +171,7 @@ nfp_net_configure(struct rte_eth_dev *dev)
 	}
 
 	/* Checking RX mode */
-	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS &&
+	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG &&
 	    !(hw->cap & NFP_NET_CFG_CTRL_RSS_ANY)) {
 		PMD_INIT_LOG(INFO, "RSS not supported");
 		return -EINVAL;
-- 
2.29.3

[PATCH 2/2] net/nfp: modify RSS's processing logic

[Chaoyong He]

From: Long Wu <long.wu@corigine.com>

The initial logic only support the single type metadata and this
commit add the support of chained type metadata. This commit also
make the relation between the RSS capability (v1/v2) and these
two types of metadata more clear.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/nfp_common.c    |  23 +++++++
 drivers/net/nfp/nfp_common.h    |   7 +++
 drivers/net/nfp/nfp_ctrl.h      |  18 +++++-
 drivers/net/nfp/nfp_ethdev.c    |   7 +--
 drivers/net/nfp/nfp_ethdev_vf.c |   7 +--
 drivers/net/nfp/nfp_rxtx.c      | 108 ++++++++++++++++++++------------
 6 files changed, 121 insertions(+), 49 deletions(-)

diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index a545a10013..a1e37ada11 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -1584,6 +1584,29 @@ nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name)
 	return 0;
 }
 
+void
+nfp_net_init_metadata_format(struct nfp_net_hw *hw)
+{
+	/*
+	 * ABI 4.x and ctrl vNIC always use chained metadata, in other cases we allow use of
+	 * single metadata if only RSS(v1) is supported by hw capability, and RSS(v2)
+	 * also indicate that we are using chained metadata.
+	 */
+	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) == 4) {
+		hw->meta_format = NFP_NET_METAFORMAT_CHANINED;
+	} else if ((hw->cap & NFP_NET_CFG_CTRL_CHAIN_META) != 0) {
+		hw->meta_format = NFP_NET_METAFORMAT_CHANINED;
+		/*
+		 * RSS is incompatible with chained metadata. hw->cap just represents
+		 * firmware's ability rather than the firmware's configuration. We decide
+		 * to reduce the confusion to allow us can use hw->cap to identify RSS later.
+		 */
+		hw->cap &= ~NFP_NET_CFG_CTRL_RSS;
+	} else {
+		hw->meta_format = NFP_NET_METAFORMAT_SINGLE;
+	}
+}
+
 /*
  * Local variables:
  * c-file-style: "Linux"
diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h
index 980f3cad89..d33675eb99 100644
--- a/drivers/net/nfp/nfp_common.h
+++ b/drivers/net/nfp/nfp_common.h
@@ -127,6 +127,11 @@ enum nfp_qcp_ptr {
 	NFP_QCP_WRITE_PTR
 };
 
+enum nfp_net_meta_format {
+	NFP_NET_METAFORMAT_SINGLE,
+	NFP_NET_METAFORMAT_CHANINED,
+};
+
 struct nfp_pf_dev {
 	/* Backpointer to associated pci device */
 	struct rte_pci_device *pci_dev;
@@ -203,6 +208,7 @@ struct nfp_net_hw {
 	uint32_t max_mtu;
 	uint32_t mtu;
 	uint32_t rx_offset;
+	enum nfp_net_meta_format meta_format;
 
 	/* Current values for control */
 	uint32_t ctrl;
@@ -455,6 +461,7 @@ int nfp_net_tx_desc_limits(struct nfp_net_hw *hw,
 		uint16_t *min_tx_desc,
 		uint16_t *max_tx_desc);
 int nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name);
+void nfp_net_init_metadata_format(struct nfp_net_hw *hw);
 
 #define NFP_NET_DEV_PRIVATE_TO_HW(adapter)\
 	(&((struct nfp_net_adapter *)adapter)->hw)
diff --git a/drivers/net/nfp/nfp_ctrl.h b/drivers/net/nfp/nfp_ctrl.h
index 1069ff9485..bdc39f8974 100644
--- a/drivers/net/nfp/nfp_ctrl.h
+++ b/drivers/net/nfp/nfp_ctrl.h
@@ -110,6 +110,7 @@
 #define   NFP_NET_CFG_CTRL_MSIX_TX_OFF    (0x1 << 26) /* Disable MSIX for TX */
 #define   NFP_NET_CFG_CTRL_LSO2           (0x1 << 28) /* LSO/TSO (version 2) */
 #define   NFP_NET_CFG_CTRL_RSS2           (0x1 << 29) /* RSS (version 2) */
+#define   NFP_NET_CFG_CTRL_CSUM_COMPLETE  (0x1 << 30) /* Checksum complete */
 #define   NFP_NET_CFG_CTRL_LIVE_ADDR      (0x1U << 31)/* live MAC addr change */
 #define NFP_NET_CFG_UPDATE              0x0004
 #define   NFP_NET_CFG_UPDATE_GEN          (0x1 <<  0) /* General update */
@@ -135,6 +136,8 @@
 #define NFP_NET_CFG_CTRL_LSO_ANY (NFP_NET_CFG_CTRL_LSO | NFP_NET_CFG_CTRL_LSO2)
 #define NFP_NET_CFG_CTRL_RSS_ANY (NFP_NET_CFG_CTRL_RSS | NFP_NET_CFG_CTRL_RSS2)
 
+#define NFP_NET_CFG_CTRL_CHAIN_META (NFP_NET_CFG_CTRL_RSS2 | \
+					NFP_NET_CFG_CTRL_CSUM_COMPLETE)
 /*
  * Read-only words (0x0030 - 0x0050):
  * @NFP_NET_CFG_VERSION:     Firmware version number
@@ -218,7 +221,7 @@
 
 /*
  * RSS configuration (0x0100 - 0x01ac):
- * Used only when NFP_NET_CFG_CTRL_RSS is enabled
+ * Used only when NFP_NET_CFG_CTRL_RSS_ANY is enabled
  * @NFP_NET_CFG_RSS_CFG:     RSS configuration word
  * @NFP_NET_CFG_RSS_KEY:     RSS "secret" key
  * @NFP_NET_CFG_RSS_ITBL:    RSS indirection table
@@ -334,6 +337,19 @@
 /* PF multiport offset */
 #define NFP_PF_CSR_SLICE_SIZE	(32 * 1024)
 
+/*
+ * nfp_net_cfg_ctrl_rss() - Get RSS flag based on firmware's capability
+ * @hw_cap: The firmware's capabilities
+ */
+static inline uint32_t
+nfp_net_cfg_ctrl_rss(uint32_t hw_cap)
+{
+	if ((hw_cap & NFP_NET_CFG_CTRL_RSS2) != 0)
+		return NFP_NET_CFG_CTRL_RSS2;
+
+	return NFP_NET_CFG_CTRL_RSS;
+}
+
 #endif /* _NFP_CTRL_H_ */
 /*
  * Local variables:
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index fed7b1ab13..47d5dff16c 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -134,10 +134,7 @@ nfp_net_start(struct rte_eth_dev *dev)
 	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
 		nfp_net_rss_config_default(dev);
 		update |= NFP_NET_CFG_UPDATE_RSS;
-		if (hw->cap & NFP_NET_CFG_CTRL_RSS2)
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
-		else
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
+		new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
 	}
 
 	/* Enable device */
@@ -611,6 +608,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
 		hw->cap &= ~NFP_NET_CFG_CTRL_TXVLAN;
 
+	nfp_net_init_metadata_format(hw);
+
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
 	else
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index c1f8a0fa0f..7834b2ee0c 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -95,10 +95,7 @@ nfp_netvf_start(struct rte_eth_dev *dev)
 	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
 		nfp_net_rss_config_default(dev);
 		update |= NFP_NET_CFG_UPDATE_RSS;
-		if (hw->cap & NFP_NET_CFG_CTRL_RSS2)
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
-		else
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
+		new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
 	}
 
 	/* Enable device */
@@ -373,6 +370,8 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
 		hw->cap &= ~NFP_NET_CFG_CTRL_TXVLAN;
 
+	nfp_net_init_metadata_format(hw);
+
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
 	else
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index 17a04cec5e..1c5a230145 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -116,26 +116,18 @@ nfp_net_rx_queue_count(void *rx_queue)
 	return count;
 }
 
-/* nfp_net_parse_meta() - Parse the metadata from packet */
-static void
-nfp_net_parse_meta(struct nfp_meta_parsed *meta,
-		struct nfp_net_rx_desc *rxd,
-		struct nfp_net_rxq *rxq,
-		struct rte_mbuf *mbuf)
+/* nfp_net_parse_chained_meta() - Parse the chained metadata from packet */
+static bool
+nfp_net_parse_chained_meta(uint8_t *meta_base,
+		rte_be32_t meta_header,
+		struct nfp_meta_parsed *meta)
 {
+	uint8_t *meta_offset;
 	uint32_t meta_info;
 	uint32_t vlan_info;
-	uint8_t *meta_offset;
-	struct nfp_net_hw *hw = rxq->hw;
 
-	if (unlikely((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2) ||
-			NFP_DESC_META_LEN(rxd) == 0))
-		return;
-
-	meta_offset = rte_pktmbuf_mtod(mbuf, uint8_t *);
-	meta_offset -= NFP_DESC_META_LEN(rxd);
-	meta_info = rte_be_to_cpu_32(*(rte_be32_t *)meta_offset);
-	meta_offset += 4;
+	meta_info = rte_be_to_cpu_32(meta_header);
+	meta_offset = meta_base + 4;
 
 	for (; meta_info != 0; meta_info >>= NFP_NET_META_FIELD_SIZE, meta_offset += 4) {
 		switch (meta_info & NFP_NET_META_FIELD_MASK) {
@@ -157,9 +149,11 @@ nfp_net_parse_meta(struct nfp_meta_parsed *meta,
 			break;
 		default:
 			/* Unsupported metadata can be a performance issue */
-			return;
+			return false;
 		}
 	}
+
+	return true;
 }
 
 /*
@@ -170,33 +164,18 @@ nfp_net_parse_meta(struct nfp_meta_parsed *meta,
  */
 static void
 nfp_net_parse_meta_hash(const struct nfp_meta_parsed *meta,
-		struct nfp_net_rx_desc *rxd,
 		struct nfp_net_rxq *rxq,
 		struct rte_mbuf *mbuf)
 {
-	uint32_t hash;
-	uint32_t hash_type;
 	struct nfp_net_hw *hw = rxq->hw;
 
 	if ((hw->ctrl & NFP_NET_CFG_CTRL_RSS_ANY) == 0)
 		return;
 
-	if (likely((hw->cap & NFP_NET_CFG_CTRL_RSS_ANY) != 0 &&
-			NFP_DESC_META_LEN(rxd) != 0)) {
-		hash = meta->hash;
-		hash_type = meta->hash_type;
-	} else {
-		if ((rxd->rxd.flags & PCIE_DESC_RX_RSS) == 0)
-			return;
-
-		hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
-		hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
-	}
-
-	mbuf->hash.rss = hash;
+	mbuf->hash.rss = meta->hash;
 	mbuf->ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
 
-	switch (hash_type) {
+	switch (meta->hash_type) {
 	case NFP_NET_RSS_IPV4:
 		mbuf->packet_type |= RTE_PTYPE_INNER_L3_IPV4;
 		break;
@@ -223,6 +202,21 @@ nfp_net_parse_meta_hash(const struct nfp_meta_parsed *meta,
 	}
 }
 
+/*
+ * nfp_net_parse_single_meta() - Parse the single metadata
+ *
+ * The RSS hash and hash-type are prepended to the packet data.
+ * Get it from metadata area.
+ */
+static inline void
+nfp_net_parse_single_meta(uint8_t *meta_base,
+		rte_be32_t meta_header,
+		struct nfp_meta_parsed *meta)
+{
+	meta->hash_type = rte_be_to_cpu_32(meta_header);
+	meta->hash = rte_be_to_cpu_32(*(rte_be32_t *)(meta_base + 4));
+}
+
 /*
  * nfp_net_parse_meta_vlan() - Set mbuf vlan_strip data based on metadata info
  *
@@ -304,6 +298,45 @@ nfp_net_parse_meta_qinq(const struct nfp_meta_parsed *meta,
 	mb->ol_flags |= RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_RX_QINQ_STRIPPED;
 }
 
+/* nfp_net_parse_meta() - Parse the metadata from packet */
+static void
+nfp_net_parse_meta(struct nfp_net_rx_desc *rxds,
+		struct nfp_net_rxq *rxq,
+		struct nfp_net_hw *hw,
+		struct rte_mbuf *mb)
+{
+	uint8_t *meta_base;
+	rte_be32_t meta_header;
+	struct nfp_meta_parsed meta = {};
+
+	if (unlikely(NFP_DESC_META_LEN(rxds) == 0))
+		return;
+
+	meta_base = rte_pktmbuf_mtod(mb, uint8_t *);
+	meta_base -= NFP_DESC_META_LEN(rxds);
+	meta_header = *(rte_be32_t *)meta_base;
+
+	switch (hw->meta_format) {
+	case NFP_NET_METAFORMAT_CHANINED:
+		if (nfp_net_parse_chained_meta(meta_base, meta_header, &meta)) {
+			nfp_net_parse_meta_hash(&meta, rxq, mb);
+			nfp_net_parse_meta_vlan(&meta, rxds, rxq, mb);
+			nfp_net_parse_meta_qinq(&meta, rxq, mb);
+		} else {
+			PMD_RX_LOG(DEBUG, "RX chained metadata format is wrong!");
+		}
+		break;
+	case NFP_NET_METAFORMAT_SINGLE:
+		if ((rxds->rxd.flags & PCIE_DESC_RX_RSS) != 0) {
+			nfp_net_parse_single_meta(meta_base, meta_header, &meta);
+			nfp_net_parse_meta_hash(&meta, rxq, mb);
+		}
+		break;
+	default:
+		PMD_RX_LOG(DEBUG, "RX metadata do not exist.");
+	}
+}
+
 /*
  * RX path design:
  *
@@ -341,7 +374,6 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	struct nfp_net_hw *hw;
 	struct rte_mbuf *mb;
 	struct rte_mbuf *new_mb;
-	struct nfp_meta_parsed meta;
 	uint16_t nb_hold;
 	uint64_t dma_addr;
 	uint16_t avail;
@@ -437,11 +469,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		mb->next = NULL;
 		mb->port = rxq->port_id;
 
-		memset(&meta, 0, sizeof(meta));
-		nfp_net_parse_meta(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_hash(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_vlan(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_qinq(&meta, rxq, mb);
+		nfp_net_parse_meta(rxds, rxq, hw, mb);
 
 		/* Checking the checksum flag */
 		nfp_net_rx_cksum(rxq, rxds, mb);
-- 
2.29.3

[PATCH v2 0/2] configure RSS and handle metadata correctly

[Chaoyong He]

Select the appropriate RSS configuration in the initial process logic
and modify the logic of processing metadata based on RSS configuration
in the RX function.

---
v2:
* Correct spelling error
---
Long Wu (2):
  net/nfp: standardize the use of RSS-related codes
  net/nfp: modify RSS's processing logic

 drivers/net/nfp/nfp_common.c    |  25 +++++++-
 drivers/net/nfp/nfp_common.h    |   7 +++
 drivers/net/nfp/nfp_ctrl.h      |  18 +++++-
 drivers/net/nfp/nfp_ethdev.c    |   7 +--
 drivers/net/nfp/nfp_ethdev_vf.c |   7 +--
 drivers/net/nfp/nfp_rxtx.c      | 108 ++++++++++++++++++++------------
 6 files changed, 122 insertions(+), 50 deletions(-)

-- 
2.29.3

[PATCH v2 1/2] net/nfp: standardize the use of RSS-related codes

[Chaoyong He]

From: Long Wu <long.wu@corigine.com>

The usage of RTE_ETH_MQ_RX_RSS and RTE_ETH_MQ_RX_RSS_FLAG are mixed in
nfp_net_configure(), use RTE_ETH_MQ_RX_RSS_FLAG uniformly.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/nfp_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index 907777a9e4..a545a10013 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -171,7 +171,7 @@ nfp_net_configure(struct rte_eth_dev *dev)
 	}
 
 	/* Checking RX mode */
-	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS &&
+	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG &&
 	    !(hw->cap & NFP_NET_CFG_CTRL_RSS_ANY)) {
 		PMD_INIT_LOG(INFO, "RSS not supported");
 		return -EINVAL;
-- 
2.29.3

[PATCH v2 2/2] net/nfp: modify RSS's processing logic

[Chaoyong He]

From: Long Wu <long.wu@corigine.com>

The initial logic only support the single type metadata and this
commit add the support of chained type metadata. This commit also
make the relation between the RSS capability (v1/v2) and these
two types of metadata more clear.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/nfp_common.c    |  23 +++++++
 drivers/net/nfp/nfp_common.h    |   7 +++
 drivers/net/nfp/nfp_ctrl.h      |  18 +++++-
 drivers/net/nfp/nfp_ethdev.c    |   7 +--
 drivers/net/nfp/nfp_ethdev_vf.c |   7 +--
 drivers/net/nfp/nfp_rxtx.c      | 108 ++++++++++++++++++++------------
 6 files changed, 121 insertions(+), 49 deletions(-)

diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index a545a10013..a1e37ada11 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -1584,6 +1584,29 @@ nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name)
 	return 0;
 }
 
+void
+nfp_net_init_metadata_format(struct nfp_net_hw *hw)
+{
+	/*
+	 * ABI 4.x and ctrl vNIC always use chained metadata, in other cases we allow use of
+	 * single metadata if only RSS(v1) is supported by hw capability, and RSS(v2)
+	 * also indicate that we are using chained metadata.
+	 */
+	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) == 4) {
+		hw->meta_format = NFP_NET_METAFORMAT_CHANINED;
+	} else if ((hw->cap & NFP_NET_CFG_CTRL_CHAIN_META) != 0) {
+		hw->meta_format = NFP_NET_METAFORMAT_CHANINED;
+		/*
+		 * RSS is incompatible with chained metadata. hw->cap just represents
+		 * firmware's ability rather than the firmware's configuration. We decide
+		 * to reduce the confusion to allow us can use hw->cap to identify RSS later.
+		 */
+		hw->cap &= ~NFP_NET_CFG_CTRL_RSS;
+	} else {
+		hw->meta_format = NFP_NET_METAFORMAT_SINGLE;
+	}
+}
+
 /*
  * Local variables:
  * c-file-style: "Linux"
diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h
index 980f3cad89..d33675eb99 100644
--- a/drivers/net/nfp/nfp_common.h
+++ b/drivers/net/nfp/nfp_common.h
@@ -127,6 +127,11 @@ enum nfp_qcp_ptr {
 	NFP_QCP_WRITE_PTR
 };
 
+enum nfp_net_meta_format {
+	NFP_NET_METAFORMAT_SINGLE,
+	NFP_NET_METAFORMAT_CHANINED,
+};
+
 struct nfp_pf_dev {
 	/* Backpointer to associated pci device */
 	struct rte_pci_device *pci_dev;
@@ -203,6 +208,7 @@ struct nfp_net_hw {
 	uint32_t max_mtu;
 	uint32_t mtu;
 	uint32_t rx_offset;
+	enum nfp_net_meta_format meta_format;
 
 	/* Current values for control */
 	uint32_t ctrl;
@@ -455,6 +461,7 @@ int nfp_net_tx_desc_limits(struct nfp_net_hw *hw,
 		uint16_t *min_tx_desc,
 		uint16_t *max_tx_desc);
 int nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name);
+void nfp_net_init_metadata_format(struct nfp_net_hw *hw);
 
 #define NFP_NET_DEV_PRIVATE_TO_HW(adapter)\
 	(&((struct nfp_net_adapter *)adapter)->hw)
diff --git a/drivers/net/nfp/nfp_ctrl.h b/drivers/net/nfp/nfp_ctrl.h
index 1069ff9485..bdc39f8974 100644
--- a/drivers/net/nfp/nfp_ctrl.h
+++ b/drivers/net/nfp/nfp_ctrl.h
@@ -110,6 +110,7 @@
 #define   NFP_NET_CFG_CTRL_MSIX_TX_OFF    (0x1 << 26) /* Disable MSIX for TX */
 #define   NFP_NET_CFG_CTRL_LSO2           (0x1 << 28) /* LSO/TSO (version 2) */
 #define   NFP_NET_CFG_CTRL_RSS2           (0x1 << 29) /* RSS (version 2) */
+#define   NFP_NET_CFG_CTRL_CSUM_COMPLETE  (0x1 << 30) /* Checksum complete */
 #define   NFP_NET_CFG_CTRL_LIVE_ADDR      (0x1U << 31)/* live MAC addr change */
 #define NFP_NET_CFG_UPDATE              0x0004
 #define   NFP_NET_CFG_UPDATE_GEN          (0x1 <<  0) /* General update */
@@ -135,6 +136,8 @@
 #define NFP_NET_CFG_CTRL_LSO_ANY (NFP_NET_CFG_CTRL_LSO | NFP_NET_CFG_CTRL_LSO2)
 #define NFP_NET_CFG_CTRL_RSS_ANY (NFP_NET_CFG_CTRL_RSS | NFP_NET_CFG_CTRL_RSS2)
 
+#define NFP_NET_CFG_CTRL_CHAIN_META (NFP_NET_CFG_CTRL_RSS2 | \
+					NFP_NET_CFG_CTRL_CSUM_COMPLETE)
 /*
  * Read-only words (0x0030 - 0x0050):
  * @NFP_NET_CFG_VERSION:     Firmware version number
@@ -218,7 +221,7 @@
 
 /*
  * RSS configuration (0x0100 - 0x01ac):
- * Used only when NFP_NET_CFG_CTRL_RSS is enabled
+ * Used only when NFP_NET_CFG_CTRL_RSS_ANY is enabled
  * @NFP_NET_CFG_RSS_CFG:     RSS configuration word
  * @NFP_NET_CFG_RSS_KEY:     RSS "secret" key
  * @NFP_NET_CFG_RSS_ITBL:    RSS indirection table
@@ -334,6 +337,19 @@
 /* PF multiport offset */
 #define NFP_PF_CSR_SLICE_SIZE	(32 * 1024)
 
+/*
+ * nfp_net_cfg_ctrl_rss() - Get RSS flag based on firmware's capability
+ * @hw_cap: The firmware's capabilities
+ */
+static inline uint32_t
+nfp_net_cfg_ctrl_rss(uint32_t hw_cap)
+{
+	if ((hw_cap & NFP_NET_CFG_CTRL_RSS2) != 0)
+		return NFP_NET_CFG_CTRL_RSS2;
+
+	return NFP_NET_CFG_CTRL_RSS;
+}
+
 #endif /* _NFP_CTRL_H_ */
 /*
  * Local variables:
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index fed7b1ab13..47d5dff16c 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -134,10 +134,7 @@ nfp_net_start(struct rte_eth_dev *dev)
 	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
 		nfp_net_rss_config_default(dev);
 		update |= NFP_NET_CFG_UPDATE_RSS;
-		if (hw->cap & NFP_NET_CFG_CTRL_RSS2)
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
-		else
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
+		new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
 	}
 
 	/* Enable device */
@@ -611,6 +608,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
 		hw->cap &= ~NFP_NET_CFG_CTRL_TXVLAN;
 
+	nfp_net_init_metadata_format(hw);
+
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
 	else
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index c1f8a0fa0f..7834b2ee0c 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -95,10 +95,7 @@ nfp_netvf_start(struct rte_eth_dev *dev)
 	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
 		nfp_net_rss_config_default(dev);
 		update |= NFP_NET_CFG_UPDATE_RSS;
-		if (hw->cap & NFP_NET_CFG_CTRL_RSS2)
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
-		else
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
+		new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
 	}
 
 	/* Enable device */
@@ -373,6 +370,8 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
 		hw->cap &= ~NFP_NET_CFG_CTRL_TXVLAN;
 
+	nfp_net_init_metadata_format(hw);
+
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
 	else
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index 17a04cec5e..1c5a230145 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -116,26 +116,18 @@ nfp_net_rx_queue_count(void *rx_queue)
 	return count;
 }
 
-/* nfp_net_parse_meta() - Parse the metadata from packet */
-static void
-nfp_net_parse_meta(struct nfp_meta_parsed *meta,
-		struct nfp_net_rx_desc *rxd,
-		struct nfp_net_rxq *rxq,
-		struct rte_mbuf *mbuf)
+/* nfp_net_parse_chained_meta() - Parse the chained metadata from packet */
+static bool
+nfp_net_parse_chained_meta(uint8_t *meta_base,
+		rte_be32_t meta_header,
+		struct nfp_meta_parsed *meta)
 {
+	uint8_t *meta_offset;
 	uint32_t meta_info;
 	uint32_t vlan_info;
-	uint8_t *meta_offset;
-	struct nfp_net_hw *hw = rxq->hw;
 
-	if (unlikely((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2) ||
-			NFP_DESC_META_LEN(rxd) == 0))
-		return;
-
-	meta_offset = rte_pktmbuf_mtod(mbuf, uint8_t *);
-	meta_offset -= NFP_DESC_META_LEN(rxd);
-	meta_info = rte_be_to_cpu_32(*(rte_be32_t *)meta_offset);
-	meta_offset += 4;
+	meta_info = rte_be_to_cpu_32(meta_header);
+	meta_offset = meta_base + 4;
 
 	for (; meta_info != 0; meta_info >>= NFP_NET_META_FIELD_SIZE, meta_offset += 4) {
 		switch (meta_info & NFP_NET_META_FIELD_MASK) {
@@ -157,9 +149,11 @@ nfp_net_parse_meta(struct nfp_meta_parsed *meta,
 			break;
 		default:
 			/* Unsupported metadata can be a performance issue */
-			return;
+			return false;
 		}
 	}
+
+	return true;
 }
 
 /*
@@ -170,33 +164,18 @@ nfp_net_parse_meta(struct nfp_meta_parsed *meta,
  */
 static void
 nfp_net_parse_meta_hash(const struct nfp_meta_parsed *meta,
-		struct nfp_net_rx_desc *rxd,
 		struct nfp_net_rxq *rxq,
 		struct rte_mbuf *mbuf)
 {
-	uint32_t hash;
-	uint32_t hash_type;
 	struct nfp_net_hw *hw = rxq->hw;
 
 	if ((hw->ctrl & NFP_NET_CFG_CTRL_RSS_ANY) == 0)
 		return;
 
-	if (likely((hw->cap & NFP_NET_CFG_CTRL_RSS_ANY) != 0 &&
-			NFP_DESC_META_LEN(rxd) != 0)) {
-		hash = meta->hash;
-		hash_type = meta->hash_type;
-	} else {
-		if ((rxd->rxd.flags & PCIE_DESC_RX_RSS) == 0)
-			return;
-
-		hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
-		hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
-	}
-
-	mbuf->hash.rss = hash;
+	mbuf->hash.rss = meta->hash;
 	mbuf->ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
 
-	switch (hash_type) {
+	switch (meta->hash_type) {
 	case NFP_NET_RSS_IPV4:
 		mbuf->packet_type |= RTE_PTYPE_INNER_L3_IPV4;
 		break;
@@ -223,6 +202,21 @@ nfp_net_parse_meta_hash(const struct nfp_meta_parsed *meta,
 	}
 }
 
+/*
+ * nfp_net_parse_single_meta() - Parse the single metadata
+ *
+ * The RSS hash and hash-type are prepended to the packet data.
+ * Get it from metadata area.
+ */
+static inline void
+nfp_net_parse_single_meta(uint8_t *meta_base,
+		rte_be32_t meta_header,
+		struct nfp_meta_parsed *meta)
+{
+	meta->hash_type = rte_be_to_cpu_32(meta_header);
+	meta->hash = rte_be_to_cpu_32(*(rte_be32_t *)(meta_base + 4));
+}
+
 /*
  * nfp_net_parse_meta_vlan() - Set mbuf vlan_strip data based on metadata info
  *
@@ -304,6 +298,45 @@ nfp_net_parse_meta_qinq(const struct nfp_meta_parsed *meta,
 	mb->ol_flags |= RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_RX_QINQ_STRIPPED;
 }
 
+/* nfp_net_parse_meta() - Parse the metadata from packet */
+static void
+nfp_net_parse_meta(struct nfp_net_rx_desc *rxds,
+		struct nfp_net_rxq *rxq,
+		struct nfp_net_hw *hw,
+		struct rte_mbuf *mb)
+{
+	uint8_t *meta_base;
+	rte_be32_t meta_header;
+	struct nfp_meta_parsed meta = {};
+
+	if (unlikely(NFP_DESC_META_LEN(rxds) == 0))
+		return;
+
+	meta_base = rte_pktmbuf_mtod(mb, uint8_t *);
+	meta_base -= NFP_DESC_META_LEN(rxds);
+	meta_header = *(rte_be32_t *)meta_base;
+
+	switch (hw->meta_format) {
+	case NFP_NET_METAFORMAT_CHANINED:
+		if (nfp_net_parse_chained_meta(meta_base, meta_header, &meta)) {
+			nfp_net_parse_meta_hash(&meta, rxq, mb);
+			nfp_net_parse_meta_vlan(&meta, rxds, rxq, mb);
+			nfp_net_parse_meta_qinq(&meta, rxq, mb);
+		} else {
+			PMD_RX_LOG(DEBUG, "RX chained metadata format is wrong!");
+		}
+		break;
+	case NFP_NET_METAFORMAT_SINGLE:
+		if ((rxds->rxd.flags & PCIE_DESC_RX_RSS) != 0) {
+			nfp_net_parse_single_meta(meta_base, meta_header, &meta);
+			nfp_net_parse_meta_hash(&meta, rxq, mb);
+		}
+		break;
+	default:
+		PMD_RX_LOG(DEBUG, "RX metadata do not exist.");
+	}
+}
+
 /*
  * RX path design:
  *
@@ -341,7 +374,6 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	struct nfp_net_hw *hw;
 	struct rte_mbuf *mb;
 	struct rte_mbuf *new_mb;
-	struct nfp_meta_parsed meta;
 	uint16_t nb_hold;
 	uint64_t dma_addr;
 	uint16_t avail;
@@ -437,11 +469,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		mb->next = NULL;
 		mb->port = rxq->port_id;
 
-		memset(&meta, 0, sizeof(meta));
-		nfp_net_parse_meta(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_hash(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_vlan(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_qinq(&meta, rxq, mb);
+		nfp_net_parse_meta(rxds, rxq, hw, mb);
 
 		/* Checking the checksum flag */
 		nfp_net_rx_cksum(rxq, rxds, mb);
-- 
2.29.3

[PATCH v2 0/2] configure RSS and handle metadata correctly

[Chaoyong He]

Select the appropriate RSS configuration in the initial process logic
and modify the logic of processing metadata based on RSS configuration
in the RX function.

---
v2:
* Correct spelling error
v3:
* Correct spelling error
---
Long Wu (2):
  net/nfp: standardize the use of RSS-related codes
  net/nfp: modify RSS's processing logic

 drivers/net/nfp/nfp_common.c    |  25 +++++++-
 drivers/net/nfp/nfp_common.h    |   7 +++
 drivers/net/nfp/nfp_ctrl.h      |  18 +++++-
 drivers/net/nfp/nfp_ethdev.c    |   7 +--
 drivers/net/nfp/nfp_ethdev_vf.c |   7 +--
 drivers/net/nfp/nfp_rxtx.c      | 108 ++++++++++++++++++++------------
 6 files changed, 122 insertions(+), 50 deletions(-)

-- 
2.29.3

[PATCH v2 1/2] net/nfp: standardize the use of RSS-related codes

[Chaoyong He]

From: Long Wu <long.wu@corigine.com>

The usage of RTE_ETH_MQ_RX_RSS and RTE_ETH_MQ_RX_RSS_FLAG are mixed in
nfp_net_configure(), use RTE_ETH_MQ_RX_RSS_FLAG uniformly.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/nfp_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index 907777a9e4..a545a10013 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -171,7 +171,7 @@ nfp_net_configure(struct rte_eth_dev *dev)
 	}
 
 	/* Checking RX mode */
-	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS &&
+	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG &&
 	    !(hw->cap & NFP_NET_CFG_CTRL_RSS_ANY)) {
 		PMD_INIT_LOG(INFO, "RSS not supported");
 		return -EINVAL;
-- 
2.29.3

[PATCH v2 2/2] net/nfp: modify RSS's processing logic

[Chaoyong He]

From: Long Wu <long.wu@corigine.com>

The initial logic only support the single type metadata and this
commit add the support of chained type metadata. This commit also
make the relation between the RSS capability (v1/v2) and these
two types of metadata more clear.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/nfp_common.c    |  23 +++++++
 drivers/net/nfp/nfp_common.h    |   7 +++
 drivers/net/nfp/nfp_ctrl.h      |  18 +++++-
 drivers/net/nfp/nfp_ethdev.c    |   7 +--
 drivers/net/nfp/nfp_ethdev_vf.c |   7 +--
 drivers/net/nfp/nfp_rxtx.c      | 108 ++++++++++++++++++++------------
 6 files changed, 121 insertions(+), 49 deletions(-)

diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index a545a10013..a1e37ada11 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -1584,6 +1584,29 @@ nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name)
 	return 0;
 }
 
+void
+nfp_net_init_metadata_format(struct nfp_net_hw *hw)
+{
+	/*
+	 * ABI 4.x and ctrl vNIC always use chained metadata, in other cases we allow use of
+	 * single metadata if only RSS(v1) is supported by hw capability, and RSS(v2)
+	 * also indicate that we are using chained metadata.
+	 */
+	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) == 4) {
+		hw->meta_format = NFP_NET_METAFORMAT_CHAINED;
+	} else if ((hw->cap & NFP_NET_CFG_CTRL_CHAIN_META) != 0) {
+		hw->meta_format = NFP_NET_METAFORMAT_CHAINED;
+		/*
+		 * RSS is incompatible with chained metadata. hw->cap just represents
+		 * firmware's ability rather than the firmware's configuration. We decide
+		 * to reduce the confusion to allow us can use hw->cap to identify RSS later.
+		 */
+		hw->cap &= ~NFP_NET_CFG_CTRL_RSS;
+	} else {
+		hw->meta_format = NFP_NET_METAFORMAT_SINGLE;
+	}
+}
+
 /*
  * Local variables:
  * c-file-style: "Linux"
diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h
index 980f3cad89..d33675eb99 100644
--- a/drivers/net/nfp/nfp_common.h
+++ b/drivers/net/nfp/nfp_common.h
@@ -127,6 +127,11 @@ enum nfp_qcp_ptr {
 	NFP_QCP_WRITE_PTR
 };
 
+enum nfp_net_meta_format {
+	NFP_NET_METAFORMAT_SINGLE,
+	NFP_NET_METAFORMAT_CHAINED,
+};
+
 struct nfp_pf_dev {
 	/* Backpointer to associated pci device */
 	struct rte_pci_device *pci_dev;
@@ -203,6 +208,7 @@ struct nfp_net_hw {
 	uint32_t max_mtu;
 	uint32_t mtu;
 	uint32_t rx_offset;
+	enum nfp_net_meta_format meta_format;
 
 	/* Current values for control */
 	uint32_t ctrl;
@@ -455,6 +461,7 @@ int nfp_net_tx_desc_limits(struct nfp_net_hw *hw,
 		uint16_t *min_tx_desc,
 		uint16_t *max_tx_desc);
 int nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name);
+void nfp_net_init_metadata_format(struct nfp_net_hw *hw);
 
 #define NFP_NET_DEV_PRIVATE_TO_HW(adapter)\
 	(&((struct nfp_net_adapter *)adapter)->hw)
diff --git a/drivers/net/nfp/nfp_ctrl.h b/drivers/net/nfp/nfp_ctrl.h
index 1069ff9485..bdc39f8974 100644
--- a/drivers/net/nfp/nfp_ctrl.h
+++ b/drivers/net/nfp/nfp_ctrl.h
@@ -110,6 +110,7 @@
 #define   NFP_NET_CFG_CTRL_MSIX_TX_OFF    (0x1 << 26) /* Disable MSIX for TX */
 #define   NFP_NET_CFG_CTRL_LSO2           (0x1 << 28) /* LSO/TSO (version 2) */
 #define   NFP_NET_CFG_CTRL_RSS2           (0x1 << 29) /* RSS (version 2) */
+#define   NFP_NET_CFG_CTRL_CSUM_COMPLETE  (0x1 << 30) /* Checksum complete */
 #define   NFP_NET_CFG_CTRL_LIVE_ADDR      (0x1U << 31)/* live MAC addr change */
 #define NFP_NET_CFG_UPDATE              0x0004
 #define   NFP_NET_CFG_UPDATE_GEN          (0x1 <<  0) /* General update */
@@ -135,6 +136,8 @@
 #define NFP_NET_CFG_CTRL_LSO_ANY (NFP_NET_CFG_CTRL_LSO | NFP_NET_CFG_CTRL_LSO2)
 #define NFP_NET_CFG_CTRL_RSS_ANY (NFP_NET_CFG_CTRL_RSS | NFP_NET_CFG_CTRL_RSS2)
 
+#define NFP_NET_CFG_CTRL_CHAIN_META (NFP_NET_CFG_CTRL_RSS2 | \
+					NFP_NET_CFG_CTRL_CSUM_COMPLETE)
 /*
  * Read-only words (0x0030 - 0x0050):
  * @NFP_NET_CFG_VERSION:     Firmware version number
@@ -218,7 +221,7 @@
 
 /*
  * RSS configuration (0x0100 - 0x01ac):
- * Used only when NFP_NET_CFG_CTRL_RSS is enabled
+ * Used only when NFP_NET_CFG_CTRL_RSS_ANY is enabled
  * @NFP_NET_CFG_RSS_CFG:     RSS configuration word
  * @NFP_NET_CFG_RSS_KEY:     RSS "secret" key
  * @NFP_NET_CFG_RSS_ITBL:    RSS indirection table
@@ -334,6 +337,19 @@
 /* PF multiport offset */
 #define NFP_PF_CSR_SLICE_SIZE	(32 * 1024)
 
+/*
+ * nfp_net_cfg_ctrl_rss() - Get RSS flag based on firmware's capability
+ * @hw_cap: The firmware's capabilities
+ */
+static inline uint32_t
+nfp_net_cfg_ctrl_rss(uint32_t hw_cap)
+{
+	if ((hw_cap & NFP_NET_CFG_CTRL_RSS2) != 0)
+		return NFP_NET_CFG_CTRL_RSS2;
+
+	return NFP_NET_CFG_CTRL_RSS;
+}
+
 #endif /* _NFP_CTRL_H_ */
 /*
  * Local variables:
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index fed7b1ab13..47d5dff16c 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -134,10 +134,7 @@ nfp_net_start(struct rte_eth_dev *dev)
 	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
 		nfp_net_rss_config_default(dev);
 		update |= NFP_NET_CFG_UPDATE_RSS;
-		if (hw->cap & NFP_NET_CFG_CTRL_RSS2)
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
-		else
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
+		new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
 	}
 
 	/* Enable device */
@@ -611,6 +608,8 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
 		hw->cap &= ~NFP_NET_CFG_CTRL_TXVLAN;
 
+	nfp_net_init_metadata_format(hw);
+
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
 	else
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index c1f8a0fa0f..7834b2ee0c 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -95,10 +95,7 @@ nfp_netvf_start(struct rte_eth_dev *dev)
 	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS) {
 		nfp_net_rss_config_default(dev);
 		update |= NFP_NET_CFG_UPDATE_RSS;
-		if (hw->cap & NFP_NET_CFG_CTRL_RSS2)
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS2;
-		else
-			new_ctrl |= NFP_NET_CFG_CTRL_RSS;
+		new_ctrl |= nfp_net_cfg_ctrl_rss(hw->cap);
 	}
 
 	/* Enable device */
@@ -373,6 +370,8 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
 		hw->cap &= ~NFP_NET_CFG_CTRL_TXVLAN;
 
+	nfp_net_init_metadata_format(hw);
+
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
 	else
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index 17a04cec5e..1c5a230145 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -116,26 +116,18 @@ nfp_net_rx_queue_count(void *rx_queue)
 	return count;
 }
 
-/* nfp_net_parse_meta() - Parse the metadata from packet */
-static void
-nfp_net_parse_meta(struct nfp_meta_parsed *meta,
-		struct nfp_net_rx_desc *rxd,
-		struct nfp_net_rxq *rxq,
-		struct rte_mbuf *mbuf)
+/* nfp_net_parse_chained_meta() - Parse the chained metadata from packet */
+static bool
+nfp_net_parse_chained_meta(uint8_t *meta_base,
+		rte_be32_t meta_header,
+		struct nfp_meta_parsed *meta)
 {
+	uint8_t *meta_offset;
 	uint32_t meta_info;
 	uint32_t vlan_info;
-	uint8_t *meta_offset;
-	struct nfp_net_hw *hw = rxq->hw;
 
-	if (unlikely((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2) ||
-			NFP_DESC_META_LEN(rxd) == 0))
-		return;
-
-	meta_offset = rte_pktmbuf_mtod(mbuf, uint8_t *);
-	meta_offset -= NFP_DESC_META_LEN(rxd);
-	meta_info = rte_be_to_cpu_32(*(rte_be32_t *)meta_offset);
-	meta_offset += 4;
+	meta_info = rte_be_to_cpu_32(meta_header);
+	meta_offset = meta_base + 4;
 
 	for (; meta_info != 0; meta_info >>= NFP_NET_META_FIELD_SIZE, meta_offset += 4) {
 		switch (meta_info & NFP_NET_META_FIELD_MASK) {
@@ -157,9 +149,11 @@ nfp_net_parse_meta(struct nfp_meta_parsed *meta,
 			break;
 		default:
 			/* Unsupported metadata can be a performance issue */
-			return;
+			return false;
 		}
 	}
+
+	return true;
 }
 
 /*
@@ -170,33 +164,18 @@ nfp_net_parse_meta(struct nfp_meta_parsed *meta,
  */
 static void
 nfp_net_parse_meta_hash(const struct nfp_meta_parsed *meta,
-		struct nfp_net_rx_desc *rxd,
 		struct nfp_net_rxq *rxq,
 		struct rte_mbuf *mbuf)
 {
-	uint32_t hash;
-	uint32_t hash_type;
 	struct nfp_net_hw *hw = rxq->hw;
 
 	if ((hw->ctrl & NFP_NET_CFG_CTRL_RSS_ANY) == 0)
 		return;
 
-	if (likely((hw->cap & NFP_NET_CFG_CTRL_RSS_ANY) != 0 &&
-			NFP_DESC_META_LEN(rxd) != 0)) {
-		hash = meta->hash;
-		hash_type = meta->hash_type;
-	} else {
-		if ((rxd->rxd.flags & PCIE_DESC_RX_RSS) == 0)
-			return;
-
-		hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
-		hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
-	}
-
-	mbuf->hash.rss = hash;
+	mbuf->hash.rss = meta->hash;
 	mbuf->ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
 
-	switch (hash_type) {
+	switch (meta->hash_type) {
 	case NFP_NET_RSS_IPV4:
 		mbuf->packet_type |= RTE_PTYPE_INNER_L3_IPV4;
 		break;
@@ -223,6 +202,21 @@ nfp_net_parse_meta_hash(const struct nfp_meta_parsed *meta,
 	}
 }
 
+/*
+ * nfp_net_parse_single_meta() - Parse the single metadata
+ *
+ * The RSS hash and hash-type are prepended to the packet data.
+ * Get it from metadata area.
+ */
+static inline void
+nfp_net_parse_single_meta(uint8_t *meta_base,
+		rte_be32_t meta_header,
+		struct nfp_meta_parsed *meta)
+{
+	meta->hash_type = rte_be_to_cpu_32(meta_header);
+	meta->hash = rte_be_to_cpu_32(*(rte_be32_t *)(meta_base + 4));
+}
+
 /*
  * nfp_net_parse_meta_vlan() - Set mbuf vlan_strip data based on metadata info
  *
@@ -304,6 +298,45 @@ nfp_net_parse_meta_qinq(const struct nfp_meta_parsed *meta,
 	mb->ol_flags |= RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_RX_QINQ_STRIPPED;
 }
 
+/* nfp_net_parse_meta() - Parse the metadata from packet */
+static void
+nfp_net_parse_meta(struct nfp_net_rx_desc *rxds,
+		struct nfp_net_rxq *rxq,
+		struct nfp_net_hw *hw,
+		struct rte_mbuf *mb)
+{
+	uint8_t *meta_base;
+	rte_be32_t meta_header;
+	struct nfp_meta_parsed meta = {};
+
+	if (unlikely(NFP_DESC_META_LEN(rxds) == 0))
+		return;
+
+	meta_base = rte_pktmbuf_mtod(mb, uint8_t *);
+	meta_base -= NFP_DESC_META_LEN(rxds);
+	meta_header = *(rte_be32_t *)meta_base;
+
+	switch (hw->meta_format) {
+	case NFP_NET_METAFORMAT_CHAINED:
+		if (nfp_net_parse_chained_meta(meta_base, meta_header, &meta)) {
+			nfp_net_parse_meta_hash(&meta, rxq, mb);
+			nfp_net_parse_meta_vlan(&meta, rxds, rxq, mb);
+			nfp_net_parse_meta_qinq(&meta, rxq, mb);
+		} else {
+			PMD_RX_LOG(DEBUG, "RX chained metadata format is wrong!");
+		}
+		break;
+	case NFP_NET_METAFORMAT_SINGLE:
+		if ((rxds->rxd.flags & PCIE_DESC_RX_RSS) != 0) {
+			nfp_net_parse_single_meta(meta_base, meta_header, &meta);
+			nfp_net_parse_meta_hash(&meta, rxq, mb);
+		}
+		break;
+	default:
+		PMD_RX_LOG(DEBUG, "RX metadata do not exist.");
+	}
+}
+
 /*
  * RX path design:
  *
@@ -341,7 +374,6 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	struct nfp_net_hw *hw;
 	struct rte_mbuf *mb;
 	struct rte_mbuf *new_mb;
-	struct nfp_meta_parsed meta;
 	uint16_t nb_hold;
 	uint64_t dma_addr;
 	uint16_t avail;
@@ -437,11 +469,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		mb->next = NULL;
 		mb->port = rxq->port_id;
 
-		memset(&meta, 0, sizeof(meta));
-		nfp_net_parse_meta(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_hash(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_vlan(&meta, rxds, rxq, mb);
-		nfp_net_parse_meta_qinq(&meta, rxq, mb);
+		nfp_net_parse_meta(rxds, rxq, hw, mb);
 
 		/* Checking the checksum flag */
 		nfp_net_rx_cksum(rxq, rxds, mb);
-- 
2.29.3

Re: [PATCH v2 1/2] net/nfp: standardize the use of RSS-related codes

[Ferruh Yigit]

On 2/21/2023 3:55 AM, Chaoyong He wrote:
> From: Long Wu <long.wu@corigine.com>
> 
> The usage of RTE_ETH_MQ_RX_RSS and RTE_ETH_MQ_RX_RSS_FLAG are mixed in
> nfp_net_configure(), use RTE_ETH_MQ_RX_RSS_FLAG uniformly.
> 
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> ---
>  drivers/net/nfp/nfp_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> index 907777a9e4..a545a10013 100644
> --- a/drivers/net/nfp/nfp_common.c
> +++ b/drivers/net/nfp/nfp_common.c
> @@ -171,7 +171,7 @@ nfp_net_configure(struct rte_eth_dev *dev)
>  	}
>  
>  	/* Checking RX mode */
> -	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS &&
> +	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG &&
>  	    !(hw->cap & NFP_NET_CFG_CTRL_RSS_ANY)) {
>  		PMD_INIT_LOG(INFO, "RSS not supported");
>  		return -EINVAL;

They are same values, but when used as a mask, flag one can be more proper.

BUT,
Not sure how correct to '&' an enum element, enums supposed to be
abstraction on underneath values, right?
For this case user should know/care enum values which is wrong, perhaps
to OR flags to make enum elements was a mistake on our end.

Anyway, with above usage all enum elements that has RSS_FLAG is taken
into account, like 'RTE_ETH_MQ_RX_VMDQ_RSS' (RSS mode with VMDq), are
you sure that is the intention, is there a chance you meant:

 if (rxmode->mq_mode == RTE_ETH_MQ_RX_RSS && ...)


btw, what if mq_mode is 'RTE_ETH_MQ_RX_VMDQ_DCB', is it supported?
	


I just recognized there are various similar usage in drivers, and most
of them comes from same commit [1], we can discuss and fix that separately.

[1]
73fb89dd6a00 ("drivers/net: fix RSS hash offload flag if no RSS")

RE: [PATCH v2 1/2] net/nfp: standardize the use of RSS-related codes

[Chaoyong He]

> On 2/21/2023 3:55 AM, Chaoyong He wrote:
> > From: Long Wu <long.wu@corigine.com>
> >
> > The usage of RTE_ETH_MQ_RX_RSS and RTE_ETH_MQ_RX_RSS_FLAG are
> mixed in
> > nfp_net_configure(), use RTE_ETH_MQ_RX_RSS_FLAG uniformly.
> >
> > Signed-off-by: Long Wu <long.wu@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> > ---
> >  drivers/net/nfp/nfp_common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/nfp/nfp_common.c
> > b/drivers/net/nfp/nfp_common.c index 907777a9e4..a545a10013 100644
> > --- a/drivers/net/nfp/nfp_common.c
> > +++ b/drivers/net/nfp/nfp_common.c
> > @@ -171,7 +171,7 @@ nfp_net_configure(struct rte_eth_dev *dev)
> >  	}
> >
> >  	/* Checking RX mode */
> > -	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS &&
> > +	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG &&
> >  	    !(hw->cap & NFP_NET_CFG_CTRL_RSS_ANY)) {
> >  		PMD_INIT_LOG(INFO, "RSS not supported");
> >  		return -EINVAL;
> 
> They are same values, but when used as a mask, flag one can be more
> proper.
> 
> BUT,
> Not sure how correct to '&' an enum element, enums supposed to be
> abstraction on underneath values, right?
> For this case user should know/care enum values which is wrong, perhaps to
> OR flags to make enum elements was a mistake on our end.
> 
> Anyway, with above usage all enum elements that has RSS_FLAG is taken
> into account, like 'RTE_ETH_MQ_RX_VMDQ_RSS' (RSS mode with VMDq),
> are you sure that is the intention, is there a chance you meant:
> 
>  if (rxmode->mq_mode == RTE_ETH_MQ_RX_RSS && ...)
> 
> 
> btw, what if mq_mode is 'RTE_ETH_MQ_RX_VMDQ_DCB', is it supported?
> 
For now, we don't support 'RTE_ETH_MQ_RX_VMDQ_DCB'.

> 
> I just recognized there are various similar usage in drivers, and most of them
> comes from same commit [1], we can discuss and fix that separately.
> 
I saw you plan drop the RFC commit [1], so what we should do about this patch?
1. Keep our modify.
2. Follow your modify.
3. We just drop this commit. (your patch series will correct it anyway).
Please give me an advice to guide my next move, thanks.

> [1]
> 73fb89dd6a00 ("drivers/net: fix RSS hash offload flag if no RSS")

Re: [PATCH v2 1/2] net/nfp: standardize the use of RSS-related codes

[Ferruh Yigit]

On 2/28/2023 8:46 AM, Chaoyong He wrote:
>> On 2/21/2023 3:55 AM, Chaoyong He wrote:
>>> From: Long Wu <long.wu@corigine.com>
>>>
>>> The usage of RTE_ETH_MQ_RX_RSS and RTE_ETH_MQ_RX_RSS_FLAG are
>> mixed in
>>> nfp_net_configure(), use RTE_ETH_MQ_RX_RSS_FLAG uniformly.
>>>
>>> Signed-off-by: Long Wu <long.wu@corigine.com>
>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>> ---
>>>  drivers/net/nfp/nfp_common.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/nfp/nfp_common.c
>>> b/drivers/net/nfp/nfp_common.c index 907777a9e4..a545a10013 100644
>>> --- a/drivers/net/nfp/nfp_common.c
>>> +++ b/drivers/net/nfp/nfp_common.c
>>> @@ -171,7 +171,7 @@ nfp_net_configure(struct rte_eth_dev *dev)
>>>  	}
>>>
>>>  	/* Checking RX mode */
>>> -	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS &&
>>> +	if (rxmode->mq_mode & RTE_ETH_MQ_RX_RSS_FLAG &&
>>>  	    !(hw->cap & NFP_NET_CFG_CTRL_RSS_ANY)) {
>>>  		PMD_INIT_LOG(INFO, "RSS not supported");
>>>  		return -EINVAL;
>>
>> They are same values, but when used as a mask, flag one can be more
>> proper.
>>
>> BUT,
>> Not sure how correct to '&' an enum element, enums supposed to be
>> abstraction on underneath values, right?
>> For this case user should know/care enum values which is wrong, perhaps to
>> OR flags to make enum elements was a mistake on our end.
>>
>> Anyway, with above usage all enum elements that has RSS_FLAG is taken
>> into account, like 'RTE_ETH_MQ_RX_VMDQ_RSS' (RSS mode with VMDq),
>> are you sure that is the intention, is there a chance you meant:
>>
>>  if (rxmode->mq_mode == RTE_ETH_MQ_RX_RSS && ...)
>>
>>
>> btw, what if mq_mode is 'RTE_ETH_MQ_RX_VMDQ_DCB', is it supported?
>>
> For now, we don't support 'RTE_ETH_MQ_RX_VMDQ_DCB'.
> 

My point was it is not checked in the driver, and that is the case for
most of the drivers, which means if user requests it, it will be
silently ignored, and if application depends on this feature this will
break the app.

So 'mq_mode' config option is not honored properly, for majority of the
cases all user/driver interested in is RSS, only a few drivers that
support other multi queue modes handles other cases.

We may improve this bit better in the ethdev layer, like providing a
mq_mode capability, etc...

>>
>> I just recognized there are various similar usage in drivers, and most of them
>> comes from same commit [1], we can discuss and fix that separately.
>>
> I saw you plan drop the RFC commit [1], so what we should do about this patch?
> 1. Keep our modify.
> 2. Follow your modify.
> 3. We just drop this commit. (your patch series will correct it anyway).
> Please give me an advice to guide my next move, thanks.
> 

Keep your modify,
when we continue to '&' 'rxmode->mq_mode' value, should use _FLAG as you
are updating in your patch.

>> [1]
>> 73fb89dd6a00 ("drivers/net: fix RSS hash offload flag if no RSS")

Re: [PATCH v2 0/2] configure RSS and handle metadata correctly

[Ferruh Yigit]

On 2/21/2023 3:55 AM, Chaoyong He wrote:
> Select the appropriate RSS configuration in the initial process logic
> and modify the logic of processing metadata based on RSS configuration
> in the RX function.
> 
> ---
> v2:
> * Correct spelling error
> v3:
> * Correct spelling error
> ---
> Long Wu (2):
>   net/nfp: standardize the use of RSS-related codes
>   net/nfp: modify RSS's processing logic
> 

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