[dpdk-dev] [PATCH v2 13/22] qede: fix RSS related issues

Rasesh Mody rasesh.mody at qlogic.com
Fri Sep 30 09:06:00 CEST 2016


From: Harish Patil <harish.patil at qlogic.com>

This patch contains few RSS related changes as follows:

o Fix inadvarent initializing of rss_params outside of the
  if block in qed_update_vport() which could cause FW exception.

o Fix disabling of RSS when hash function is 0.

o Rename qede_config_rss() to qede_check_vport_rss_enable()
  for better clarity.

o Avoid code duplication using a helper function
  qede_init_rss_caps().

Fixes: 4c98f27 ("qede: support RSS hash configuration")
Fixes: 2ea6f76 ("qede: add core driver")

Signed-off-by: Harish Patil <harish.patil at qlogic.com>
---
 doc/guides/nics/qede.rst       |  2 +-
 drivers/net/qede/qede_eth_if.c |  2 +-
 drivers/net/qede/qede_ethdev.c | 77 +++++++++++++++++++++---------------
 drivers/net/qede/qede_ethdev.h | 23 ++++++-----
 drivers/net/qede/qede_rxtx.c   | 88 +++++++++++++++++-------------------------
 5 files changed, 97 insertions(+), 95 deletions(-)

diff --git a/doc/guides/nics/qede.rst b/doc/guides/nics/qede.rst
index 5b921cc..5e31c11 100644
--- a/doc/guides/nics/qede.rst
+++ b/doc/guides/nics/qede.rst
@@ -51,7 +51,7 @@ Supported Features
 - VLAN offload - Filtering and stripping
 - Stateless checksum offloads (IPv4/TCP/UDP)
 - Multiple Rx/Tx queues
-- RSS (with user configurable table/key)
+- RSS (with RETA/hash table/key)
 - TSS
 - Multiple MAC address
 - Default pause flow control
diff --git a/drivers/net/qede/qede_eth_if.c b/drivers/net/qede/qede_eth_if.c
index bf41390..a19b22e 100644
--- a/drivers/net/qede/qede_eth_if.c
+++ b/drivers/net/qede/qede_eth_if.c
@@ -143,8 +143,8 @@ qed_update_vport(struct ecore_dev *edev, struct qed_update_vport_params *params)
 		       ECORE_RSS_IND_TABLE_SIZE * sizeof(uint16_t));
 		rte_memcpy(sp_rss_params.rss_key, params->rss_params.rss_key,
 		       ECORE_RSS_KEY_SIZE * sizeof(uint32_t));
+		sp_params.rss_params = &sp_rss_params;
 	}
-	sp_params.rss_params = &sp_rss_params;
 
 	for_each_hwfn(edev, i) {
 		struct ecore_hwfn *p_hwfn = &edev->hwfns[i];
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index a376f88..c4e82d0 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -537,7 +537,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 	struct qede_dev *qdev = eth_dev->data->dev_private;
 	struct ecore_dev *edev = &qdev->edev;
 	struct rte_eth_rxmode *rxmode = &eth_dev->data->dev_conf.rxmode;
-	int rc;
+	int rc, i, j;
 
 	PMD_INIT_FUNC_TRACE(edev);
 
@@ -558,10 +558,6 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 		}
 	}
 
-	qdev->fp_num_tx = eth_dev->data->nb_tx_queues;
-	qdev->fp_num_rx = eth_dev->data->nb_rx_queues;
-	qdev->num_queues = qdev->fp_num_tx + qdev->fp_num_rx;
-
 	/* Sanity checks and throw warnings */
 	if (rxmode->enable_scatter == 1) {
 		DP_ERR(edev, "RX scatter packets is not supported\n");
@@ -580,8 +576,6 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 		DP_INFO(edev, "IP/UDP/TCP checksum offload is always enabled "
 			      "in hw\n");
 
-	SLIST_INIT(&qdev->vlan_list_head);
-
 	/* Check for the port restart case */
 	if (qdev->state != QEDE_DEV_INIT) {
 		rc = qdev->ops->vport_stop(edev, 0);
@@ -590,6 +584,10 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 		qede_dealloc_fp_resc(eth_dev);
 	}
 
+	qdev->fp_num_tx = eth_dev->data->nb_tx_queues;
+	qdev->fp_num_rx = eth_dev->data->nb_rx_queues;
+	qdev->num_queues = qdev->fp_num_tx + qdev->fp_num_rx;
+
 	/* Fastpath status block should be initialized before sending
 	 * VPORT-START in the case of VF. Anyway, do it for both VF/PF.
 	 */
@@ -604,6 +602,8 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 	if (rc != 0)
 		return rc;
 
+	SLIST_INIT(&qdev->vlan_list_head);
+
 	/* Add primary mac for PF */
 	if (IS_PF(edev))
 		qede_mac_addr_set(eth_dev, &qdev->primary_mac);
@@ -615,6 +615,10 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 
 	qdev->state = QEDE_DEV_CONFIG;
 
+	DP_INFO(edev, "Allocated RSS=%d TSS=%d (with CoS=%d)\n",
+		(int)QEDE_RSS_COUNT(qdev), (int)QEDE_TSS_COUNT(qdev),
+		qdev->num_tc);
+
 	return 0;
 }
 
@@ -1037,42 +1041,51 @@ qede_dev_supported_ptypes_get(struct rte_eth_dev *eth_dev)
 	return NULL;
 }
 
-int qede_rss_hash_update(struct rte_eth_dev *eth_dev,
-			 struct rte_eth_rss_conf *rss_conf)
+void qede_init_rss_caps(uint8_t *rss_caps, uint64_t hf)
+{
+	*rss_caps = 0;
+	*rss_caps |= (hf & ETH_RSS_IPV4)              ? ECORE_RSS_IPV4 : 0;
+	*rss_caps |= (hf & ETH_RSS_IPV6)              ? ECORE_RSS_IPV6 : 0;
+	*rss_caps |= (hf & ETH_RSS_IPV6_EX)           ? ECORE_RSS_IPV6 : 0;
+	*rss_caps |= (hf & ETH_RSS_NONFRAG_IPV4_TCP)  ? ECORE_RSS_IPV4_TCP : 0;
+	*rss_caps |= (hf & ETH_RSS_NONFRAG_IPV6_TCP)  ? ECORE_RSS_IPV6_TCP : 0;
+	*rss_caps |= (hf & ETH_RSS_IPV6_TCP_EX)       ? ECORE_RSS_IPV6_TCP : 0;
+}
+
+static int qede_rss_hash_update(struct rte_eth_dev *eth_dev,
+				struct rte_eth_rss_conf *rss_conf)
 {
 	struct qed_update_vport_params vport_update_params;
 	struct qede_dev *qdev = eth_dev->data->dev_private;
 	struct ecore_dev *edev = &qdev->edev;
-	uint8_t rss_caps;
 	uint32_t *key = (uint32_t *)rss_conf->rss_key;
 	uint64_t hf = rss_conf->rss_hf;
 	int i;
 
-	if (hf == 0)
-		DP_ERR(edev, "hash function 0 will disable RSS\n");
+	memset(&vport_update_params, 0, sizeof(vport_update_params));
 
-	rss_caps = 0;
-	rss_caps |= (hf & ETH_RSS_IPV4)              ? ECORE_RSS_IPV4 : 0;
-	rss_caps |= (hf & ETH_RSS_IPV6)              ? ECORE_RSS_IPV6 : 0;
-	rss_caps |= (hf & ETH_RSS_IPV6_EX)           ? ECORE_RSS_IPV6 : 0;
-	rss_caps |= (hf & ETH_RSS_NONFRAG_IPV4_TCP)  ? ECORE_RSS_IPV4_TCP : 0;
-	rss_caps |= (hf & ETH_RSS_NONFRAG_IPV6_TCP)  ? ECORE_RSS_IPV6_TCP : 0;
-	rss_caps |= (hf & ETH_RSS_IPV6_TCP_EX)       ? ECORE_RSS_IPV6_TCP : 0;
+	if (hf != 0) {
+		/* Enable RSS */
+		qede_init_rss_caps(&qdev->rss_params.rss_caps, hf);
+		memcpy(&vport_update_params.rss_params, &qdev->rss_params,
+		       sizeof(vport_update_params.rss_params));
+		if (key)
+			memcpy(qdev->rss_params.rss_key, rss_conf->rss_key,
+			       rss_conf->rss_key_len);
+		vport_update_params.update_rss_flg = 1;
+		qdev->rss_enabled = 1;
+	} else {
+		/* Disable RSS */
+		qdev->rss_enabled = 0;
+	}
 
 	/* If the mapping doesn't fit any supported, return */
-	if (rss_caps == 0 && hf != 0)
+	if (qdev->rss_params.rss_caps == 0 && hf != 0)
 		return -EINVAL;
 
-	memset(&vport_update_params, 0, sizeof(vport_update_params));
-
-	if (key != NULL)
-		memcpy(qdev->rss_params.rss_key, rss_conf->rss_key,
-		       rss_conf->rss_key_len);
+	DP_INFO(edev, "%s\n", (vport_update_params.update_rss_flg) ?
+				"Enabling RSS" : "Disabling RSS");
 
-	qdev->rss_params.rss_caps = rss_caps;
-	memcpy(&vport_update_params.rss_params, &qdev->rss_params,
-	       sizeof(vport_update_params.rss_params));
-	vport_update_params.update_rss_flg = 1;
 	vport_update_params.vport_id = 0;
 
 	return qdev->ops->vport_update(edev, &vport_update_params);
@@ -1110,9 +1123,9 @@ int qede_rss_hash_conf_get(struct rte_eth_dev *eth_dev,
 	return 0;
 }
 
-int qede_rss_reta_update(struct rte_eth_dev *eth_dev,
-			 struct rte_eth_rss_reta_entry64 *reta_conf,
-			 uint16_t reta_size)
+static int qede_rss_reta_update(struct rte_eth_dev *eth_dev,
+				struct rte_eth_rss_reta_entry64 *reta_conf,
+				uint16_t reta_size)
 {
 	struct qed_update_vport_params vport_update_params;
 	struct qede_dev *qdev = eth_dev->data->dev_private;
diff --git a/drivers/net/qede/qede_ethdev.h b/drivers/net/qede/qede_ethdev.h
index 526d3be..5838f33 100644
--- a/drivers/net/qede/qede_ethdev.h
+++ b/drivers/net/qede/qede_ethdev.h
@@ -64,8 +64,10 @@
 #define QEDE_MAX_TSS_CNT(edev)  ((edev)->dev_info.num_queues * \
 					(edev)->dev_info.num_tc)
 
-#define QEDE_RSS_CNT(edev)	((edev)->fp_num_rx)
-#define QEDE_TSS_CNT(edev)	((edev)->fp_num_rx * (edev)->num_tc)
+#define QEDE_QUEUE_CNT(qdev) ((qdev)->num_queues)
+#define QEDE_RSS_COUNT(qdev) ((qdev)->num_queues - (qdev)->fp_num_tx)
+#define QEDE_TSS_COUNT(qdev) (((qdev)->num_queues - (qdev)->fp_num_rx) * \
+					(qdev)->num_tc)
 
 #define QEDE_DUPLEX_FULL	1
 #define QEDE_DUPLEX_HALF	2
@@ -78,12 +80,6 @@
 
 #define QEDE_INIT_EDEV(adapter) (&((struct qede_dev *)adapter)->edev)
 
-#define QEDE_QUEUE_CNT(qdev) ((qdev)->num_queues)
-#define QEDE_RSS_COUNT(qdev) ((qdev)->num_queues - (qdev)->fp_num_tx)
-#define QEDE_TSS_COUNT(qdev) (((qdev)->num_queues - (qdev)->fp_num_rx) * \
-		(qdev)->num_tc)
-#define QEDE_TC_IDX(qdev, txqidx) ((txqidx) / QEDE_TSS_COUNT(qdev))
-
 #define QEDE_INIT(eth_dev) {					\
 	struct qede_dev *qdev = eth_dev->data->dev_private;	\
 	struct ecore_dev *edev = &qdev->edev;			\
@@ -133,7 +129,6 @@ struct qede_dev {
 	struct qed_dev_eth_info dev_info;
 	struct ecore_sb_info *sb_array;
 	struct qede_fastpath *fp_array;
-	uint16_t num_rss;
 	uint8_t num_tc;
 	uint16_t mtu;
 	bool rss_enabled;
@@ -156,6 +151,16 @@ struct qede_dev {
 static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev,
 				uint16_t vlan_id, int on);
 
+static int qede_rss_hash_update(struct rte_eth_dev *eth_dev,
+				struct rte_eth_rss_conf *rss_conf);
+
+static int qede_rss_reta_update(struct rte_eth_dev *eth_dev,
+				struct rte_eth_rss_reta_entry64 *reta_conf,
+				uint16_t reta_size);
+
+/* Non-static functions */
+void qede_init_rss_caps(uint8_t *rss_caps, uint64_t hf);
+
 int qed_fill_eth_dev_info(struct ecore_dev *edev,
 				 struct qed_dev_eth_info *info);
 int qede_dev_set_link_state(struct rte_eth_dev *eth_dev, bool link_up);
diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
index 9df0d133..ab16c04 100644
--- a/drivers/net/qede/qede_rxtx.c
+++ b/drivers/net/qede/qede_rxtx.c
@@ -64,7 +64,7 @@ void qede_rx_queue_release(void *rx_queue)
 		rte_free(rxq->sw_rx_ring);
 		rxq->sw_rx_ring = NULL;
 		rte_free(rxq);
-		rx_queue = NULL;
+		rxq = NULL;
 	}
 }
 
@@ -234,7 +234,7 @@ void qede_tx_queue_release(void *tx_queue)
 		}
 		rte_free(txq);
 	}
-	tx_queue = NULL;
+	txq = NULL;
 }
 
 int
@@ -502,9 +502,9 @@ static void qede_prandom_bytes(uint32_t *buff, size_t bytes)
 		buff[i] = rand();
 }
 
-static int
-qede_config_rss(struct rte_eth_dev *eth_dev,
-		struct qed_update_vport_rss_params *rss_params)
+static bool
+qede_check_vport_rss_enable(struct rte_eth_dev *eth_dev,
+			    struct qed_update_vport_rss_params *rss_params)
 {
 	struct rte_eth_rss_conf rss_conf;
 	enum rte_eth_rx_mq_mode mode = eth_dev->data->dev_conf.rxmode.mq_mode;
@@ -515,57 +515,46 @@ qede_config_rss(struct rte_eth_dev *eth_dev,
 	uint64_t hf;
 	uint32_t *key;
 
+	PMD_INIT_FUNC_TRACE(edev);
+
 	rss_conf = eth_dev->data->dev_conf.rx_adv_conf.rss_conf;
 	key = (uint32_t *)rss_conf.rss_key;
 	hf = rss_conf.rss_hf;
-	PMD_INIT_FUNC_TRACE(edev);
 
 	/* Check if RSS conditions are met.
 	 * Note: Even though its meaningless to enable RSS with one queue, it
 	 * could be used to produce RSS Hash, so skipping that check.
 	 */
-
 	if (!(mode & ETH_MQ_RX_RSS)) {
 		DP_INFO(edev, "RSS flag is not set\n");
-		return -EINVAL;
+		return false;
 	}
 
-	DP_INFO(edev, "RSS flag is set\n");
-
-	if (rss_conf.rss_hf == 0)
-		DP_NOTICE(edev, false, "RSS hash function = 0, disables RSS\n");
-
-	if (rss_conf.rss_key != NULL)
-		memcpy(qdev->rss_params.rss_key, rss_conf.rss_key,
-		       rss_conf.rss_key_len);
+	if (hf == 0) {
+		DP_INFO(edev, "Request to disable RSS\n");
+		return false;
+	}
 
 	memset(rss_params, 0, sizeof(*rss_params));
 
 	for (i = 0; i < ECORE_RSS_IND_TABLE_SIZE; i++)
 		rss_params->rss_ind_table[i] = qede_rxfh_indir_default(i,
-							QEDE_RSS_CNT(qdev));
+							QEDE_RSS_COUNT(qdev));
 
-	/* key and protocols */
-	if (rss_conf.rss_key == NULL)
+	if (!key)
 		qede_prandom_bytes(rss_params->rss_key,
 				   sizeof(rss_params->rss_key));
 	else
 		memcpy(rss_params->rss_key, rss_conf.rss_key,
 		       rss_conf.rss_key_len);
 
-	rss_caps = 0;
-	rss_caps |= (hf & ETH_RSS_IPV4)              ? ECORE_RSS_IPV4 : 0;
-	rss_caps |= (hf & ETH_RSS_IPV6)              ? ECORE_RSS_IPV6 : 0;
-	rss_caps |= (hf & ETH_RSS_IPV6_EX)           ? ECORE_RSS_IPV6 : 0;
-	rss_caps |= (hf & ETH_RSS_NONFRAG_IPV4_TCP)  ? ECORE_RSS_IPV4_TCP : 0;
-	rss_caps |= (hf & ETH_RSS_NONFRAG_IPV6_TCP)  ? ECORE_RSS_IPV6_TCP : 0;
-	rss_caps |= (hf & ETH_RSS_IPV6_TCP_EX)       ? ECORE_RSS_IPV6_TCP : 0;
+	qede_init_rss_caps(&rss_caps, hf);
 
 	rss_params->rss_caps = rss_caps;
 
-	DP_INFO(edev, "RSS check passes\n");
+	DP_INFO(edev, "RSS conditions are met\n");
 
-	return 0;
+	return true;
 }
 
 static int qede_start_queues(struct rte_eth_dev *eth_dev, bool clear_stats)
@@ -618,7 +607,7 @@ static int qede_start_queues(struct rte_eth_dev *eth_dev, bool clear_stats)
 			continue;
 		for (tc = 0; tc < qdev->num_tc; tc++) {
 			txq = fp->txqs[tc];
-			txq_index = tc * QEDE_RSS_CNT(qdev) + i;
+			txq_index = tc * QEDE_RSS_COUNT(qdev) + i;
 
 			p_phys_table = ecore_chain_get_pbl_phys(&txq->tx_pbl);
 			page_cnt = ecore_chain_get_page_cnt(&txq->tx_pbl);
@@ -663,14 +652,11 @@ static int qede_start_queues(struct rte_eth_dev *eth_dev, bool clear_stats)
 		vport_update_params.tx_switching_flg = 1;
 	}
 
-	if (!qede_config_rss(eth_dev, rss_params)) {
+	if (qede_check_vport_rss_enable(eth_dev, rss_params)) {
 		vport_update_params.update_rss_flg = 1;
-
 		qdev->rss_enabled = 1;
-		DP_INFO(edev, "Updating RSS flag\n");
 	} else {
 		qdev->rss_enabled = 0;
-		DP_INFO(edev, "Not Updating RSS flag\n");
 	}
 
 	rte_memcpy(&vport_update_params.rss_params, rss_params,
@@ -1124,7 +1110,7 @@ static void qede_init_fp_queue(struct rte_eth_dev *eth_dev)
 
 		if (fp->type & QEDE_FASTPATH_TX) {
 			for (tc = 0; tc < qdev->num_tc; tc++) {
-				txq_index = tc * QEDE_TSS_CNT(qdev) + txq;
+				txq_index = tc * QEDE_TSS_COUNT(qdev) + txq;
 				fp->txqs[tc] =
 					eth_dev->data->tx_queues[txq_index];
 				fp->txqs[tc]->queue_id = txq_index;
@@ -1326,6 +1312,7 @@ int qede_reset_fp_rings(struct qede_dev *qdev)
 		if (fp->type & QEDE_FASTPATH_TX) {
 			for (tc = 0; tc < qdev->num_tc; tc++) {
 				txq = fp->txqs[tc];
+				qede_tx_queue_release_mbufs(txq);
 				ecore_chain_reset(&txq->tx_pbl);
 				txq->sw_tx_cons = 0;
 				txq->sw_tx_prod = 0;
@@ -1338,29 +1325,26 @@ int qede_reset_fp_rings(struct qede_dev *qdev)
 }
 
 /* This function frees all memory of a single fp */
-static void qede_free_mem_fp(struct rte_eth_dev *eth_dev,
-			     struct qede_fastpath *fp)
-{
-	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
-	uint8_t tc;
-
-	qede_rx_queue_release(fp->rxq);
-	for (tc = 0; tc < qdev->num_tc; tc++) {
-		qede_tx_queue_release(fp->txqs[tc]);
-		eth_dev->data->tx_queues[tc] = NULL;
-	}
-}
-
 void qede_free_mem_load(struct rte_eth_dev *eth_dev)
 {
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
 	struct qede_fastpath *fp;
-	uint8_t rss_id;
+	uint16_t txq_idx;
+	uint8_t id;
+	uint8_t tc;
 
-	for_each_queue(rss_id) {
-		fp = &qdev->fp_array[rss_id];
-		qede_free_mem_fp(eth_dev, fp);
-		eth_dev->data->rx_queues[rss_id] = NULL;
+	for_each_queue(id) {
+		fp = &qdev->fp_array[id];
+		if (fp->type & QEDE_FASTPATH_RX) {
+			qede_rx_queue_release(fp->rxq);
+			eth_dev->data->rx_queues[id] = NULL;
+		} else {
+			for (tc = 0; tc < qdev->num_tc; tc++) {
+				txq_idx = fp->txqs[tc]->queue_id;
+				qede_tx_queue_release(fp->txqs[tc]);
+				eth_dev->data->tx_queues[txq_idx] = NULL;
+			}
+		}
 	}
 }
 
-- 
1.8.3.1



More information about the dev mailing list