diff mbox

[dpdk-dev,5/5] net/bnxt: Support for rx/tx_queue_start/stop ops

Message ID 20180122062046.81908-6-ajit.khaparde@broadcom.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Ajit Khaparde Jan. 22, 2018, 6:20 a.m. UTC 
  Currently this is implemented entirely in the PMD as there is no explicit
support in the HW. Re-program the RSS Table without this queue on stop
and add it back to the table on start.

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 125 ++++++++++++++++++++++++++++++-----------
 drivers/net/bnxt/bnxt_rxq.h    |   2 +-
 drivers/net/bnxt/bnxt_rxr.c    |   4 ++
 drivers/net/bnxt/bnxt_rxr.h    |   3 +-
 drivers/net/bnxt/bnxt_txq.h    |   1 -
 drivers/net/bnxt/bnxt_txr.c    |  32 +++++++++++
 drivers/net/bnxt/bnxt_txr.h    |   2 +
 7 files changed, 133 insertions(+), 36 deletions(-)

Comments

Ferruh Yigit Jan. 22, 2018, 12:25 p.m. UTC | #1 
On 1/22/2018 6:20 AM, Ajit Khaparde wrote:
> Currently this is implemented entirely in the PMD as there is no explicit
> support in the HW. Re-program the RSS Table without this queue on stop
> and add it back to the table on start.
> 
> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

<...>

> @@ -1,4 +1,4 @@
> -/*-
> +/*
>   *   BSD LICENSE
>   *
>   *   Copyright(c) Broadcom Limited.

Unrelated but since I saw this, do you plan switching to the SPDX tags?

<...>

> @@ -321,8 +334,7 @@ static int bnxt_init_chip(struct bnxt *bp)
>  	     !RTE_ETH_DEV_SRIOV(bp->eth_dev).active) &&
>  	    bp->eth_dev->data->dev_conf.intr_conf.rxq != 0) {
>  		intr_vector = bp->eth_dev->data->nb_rx_queues;
> -		PMD_DRV_LOG(INFO, "%s(): intr_vector = %d\n", __func__,
> -			intr_vector);
> +		PMD_DRV_LOG(INFO, "intr_vector = %d\n", intr_vector);

With new logging macro "__func__" is duplicate, why not fix them all in the
patch 2/5 that introduces new macro?

<...>

> @@ -2969,6 +2981,49 @@ bnxt_set_eeprom_op(struct rte_eth_dev *dev,
>  	return 0;
>  }
>  
> +int bnxt_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
> +{
> +	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> +	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
> +	struct bnxt_rx_queue *rxq = bp->rx_queues[rx_queue_id];
> +	struct bnxt_vnic_info *vnic = NULL;
> +
> +	dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
> +	rxq->rx_deferred_start = false;
> +	PMD_DRV_LOG(INFO, "Rx queue started %d\n", rx_queue_id);
> +	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) {
> +		vnic = rxq->vnic;
> +		if (vnic->fw_grp_ids[rx_queue_id] != INVALID_HW_RING_ID)
> +			return 0;
> +		PMD_DRV_LOG(DEBUG, "vnic = %p fw_grp_id = %d\n",
> +			vnic, bp->grp_info[rx_queue_id + 1].fw_grp_id);
> +		vnic->fw_grp_ids[rx_queue_id] =
> +					bp->grp_info[rx_queue_id + 1].fw_grp_id;
> +		return bnxt_vnic_rss_configure(bp, vnic);
> +	}
> +
> +	return 0;
> +}
> +
> +int bnxt_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id)
> +{
> +	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> +	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
> +	struct bnxt_rx_queue *rxq = bp->rx_queues[rx_queue_id];
> +	struct bnxt_vnic_info *vnic = NULL;
> +
> +	dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
> +	rxq->rx_deferred_start = true;
> +	PMD_DRV_LOG(DEBUG, "Rx queue stopped\n");
> +
> +	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) {
> +		vnic = rxq->vnic;
> +		vnic->fw_grp_ids[rx_queue_id] = INVALID_HW_RING_ID;
> +		return bnxt_vnic_rss_configure(bp, vnic);
> +	}
> +	return 0;
> +}

There is already a source file "bnxt_rxq.c", which seems for Rxq related
functions, why not add new functions there?

<...>

> @@ -364,3 +369,30 @@ uint16_t bnxt_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  
>  	return nb_tx_pkts;
>  }
> +
> +int bnxt_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
> +{
> +	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> +	struct bnxt_tx_queue *txq = bp->tx_queues[tx_queue_id];
> +
> +	dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
> +	txq->tx_deferred_start = false;
> +	PMD_DRV_LOG(DEBUG, "Tx queue started\n");
> +
> +	return 0;
> +}
> +
> +int bnxt_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
> +{
> +	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> +	struct bnxt_tx_queue *txq = bp->tx_queues[tx_queue_id];
> +
> +	/* Handle TX completions */
> +	bnxt_handle_tx_cp(txq);
> +
> +	dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
> +	txq->tx_deferred_start = true;
> +	PMD_DRV_LOG(DEBUG, "Tx queue stopped\n");
> +
> +	return 0;
> +}

Similar question for these functions, they seem implemented in txr (Tx Ring ?)
source file, bnxt_txq.c seems better fit, what do you think?

<...>
diff mbox

Patch

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ebc2dfab2..82d2416ba 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1,4 +1,4 @@ 
-/*-
+/*
  *   BSD LICENSE
  *
  *   Copyright(c) Broadcom Limited.
@@ -200,9 +200,37 @@  static int bnxt_alloc_mem(struct bnxt *bp)
 	return rc;
 }
 
+static int bnxt_vnic_rss_configure(struct bnxt *bp, struct bnxt_vnic_info *vnic)
+{
+	unsigned int rss_idx, fw_idx, i;
+
+	if (vnic->rss_table && vnic->hash_type) {
+		/*
+		 * Fill the RSS hash & redirection table with
+		 * ring group ids for all VNICs
+		 */
+		for (rss_idx = 0, fw_idx = 0; rss_idx < HW_HASH_INDEX_SIZE;
+			rss_idx++, fw_idx++) {
+			for (i = 0; i < bp->rx_cp_nr_rings; i++) {
+				fw_idx %= bp->rx_cp_nr_rings;
+				if (vnic->fw_grp_ids[fw_idx] !=
+				    INVALID_HW_RING_ID)
+					break;
+				fw_idx++;
+			}
+			if (i == bp->rx_cp_nr_rings)
+				return 0;
+			vnic->rss_table[rss_idx] =
+				vnic->fw_grp_ids[fw_idx];
+		}
+		return bnxt_hwrm_vnic_rss_cfg(bp, vnic);
+	}
+	return 0;
+}
+
 static int bnxt_init_chip(struct bnxt *bp)
 {
-	unsigned int i, rss_idx, fw_idx;
+	unsigned int i;
 	struct rte_eth_link new;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
@@ -279,27 +307,12 @@  static int bnxt_init_chip(struct bnxt *bp)
 				i, rc);
 			goto err_out;
 		}
-		if (vnic->rss_table && vnic->hash_type) {
-			/*
-			 * Fill the RSS hash & redirection table with
-			 * ring group ids for all VNICs
-			 */
-			for (rss_idx = 0, fw_idx = 0;
-			     rss_idx < HW_HASH_INDEX_SIZE;
-			     rss_idx++, fw_idx++) {
-				if (vnic->fw_grp_ids[fw_idx] ==
-				    INVALID_HW_RING_ID)
-					fw_idx = 0;
-				vnic->rss_table[rss_idx] =
-						vnic->fw_grp_ids[fw_idx];
-			}
-			rc = bnxt_hwrm_vnic_rss_cfg(bp, vnic);
-			if (rc) {
-				PMD_DRV_LOG(ERR,
-					"HWRM vnic %d set RSS failure rc: %x\n",
-					i, rc);
-				goto err_out;
-			}
+
+		rc = bnxt_vnic_rss_configure(bp, vnic);
+		if (rc) {
+			PMD_DRV_LOG(ERR,
+				    "HWRM vnic set RSS failure rc: %x\n", rc);
+			goto err_out;
 		}
 
 		bnxt_hwrm_vnic_plcmode_cfg(bp, vnic);
@@ -321,8 +334,7 @@  static int bnxt_init_chip(struct bnxt *bp)
 	     !RTE_ETH_DEV_SRIOV(bp->eth_dev).active) &&
 	    bp->eth_dev->data->dev_conf.intr_conf.rxq != 0) {
 		intr_vector = bp->eth_dev->data->nb_rx_queues;
-		PMD_DRV_LOG(INFO, "%s(): intr_vector = %d\n", __func__,
-			intr_vector);
+		PMD_DRV_LOG(INFO, "intr_vector = %d\n", intr_vector);
 		if (intr_vector > bp->rx_cp_nr_rings) {
 			PMD_DRV_LOG(ERR, "At most %d intr queues supported",
 					bp->rx_cp_nr_rings);
@@ -342,9 +354,9 @@  static int bnxt_init_chip(struct bnxt *bp)
 				" intr_vec", bp->eth_dev->data->nb_rx_queues);
 			return -ENOMEM;
 		}
-		PMD_DRV_LOG(DEBUG, "%s(): intr_handle->intr_vec = %p "
+		PMD_DRV_LOG(DEBUG, "intr_handle->intr_vec = %p "
 			"intr_handle->nb_efd = %d intr_handle->max_intr = %d\n",
-			 __func__, intr_handle->intr_vec, intr_handle->nb_efd,
+			 intr_handle->intr_vec, intr_handle->nb_efd,
 			intr_handle->max_intr);
 	}
 
@@ -2842,8 +2854,8 @@  bnxt_get_eeprom_length_op(struct rte_eth_dev *dev)
 	uint32_t dir_entries;
 	uint32_t entry_length;
 
-	PMD_DRV_LOG(INFO, "%s(): %04x:%02x:%02x:%02x\n",
-		__func__, bp->pdev->addr.domain, bp->pdev->addr.bus,
+	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x\n",
+		bp->pdev->addr.domain, bp->pdev->addr.bus,
 		bp->pdev->addr.devid, bp->pdev->addr.function);
 
 	rc = bnxt_hwrm_nvm_get_dir_info(bp, &dir_entries, &entry_length);
@@ -2861,8 +2873,8 @@  bnxt_get_eeprom_op(struct rte_eth_dev *dev,
 	uint32_t index;
 	uint32_t offset;
 
-	PMD_DRV_LOG(INFO, "%s(): %04x:%02x:%02x:%02x in_eeprom->offset = %d "
-		"len = %d\n", __func__, bp->pdev->addr.domain,
+	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x in_eeprom->offset = %d "
+		"len = %d\n", bp->pdev->addr.domain,
 		bp->pdev->addr.bus, bp->pdev->addr.devid,
 		bp->pdev->addr.function, in_eeprom->offset, in_eeprom->length);
 
@@ -2930,8 +2942,8 @@  bnxt_set_eeprom_op(struct rte_eth_dev *dev,
 	uint8_t index, dir_op;
 	uint16_t type, ext, ordinal, attr;
 
-	PMD_DRV_LOG(INFO, "%s(): %04x:%02x:%02x:%02x in_eeprom->offset = %d "
-		"len = %d\n", __func__, bp->pdev->addr.domain,
+	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x in_eeprom->offset = %d "
+		"len = %d\n", bp->pdev->addr.domain,
 		bp->pdev->addr.bus, bp->pdev->addr.devid,
 		bp->pdev->addr.function, in_eeprom->offset, in_eeprom->length);
 
@@ -2969,6 +2981,49 @@  bnxt_set_eeprom_op(struct rte_eth_dev *dev,
 	return 0;
 }
 
+int bnxt_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+	struct bnxt_rx_queue *rxq = bp->rx_queues[rx_queue_id];
+	struct bnxt_vnic_info *vnic = NULL;
+
+	dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
+	rxq->rx_deferred_start = false;
+	PMD_DRV_LOG(INFO, "Rx queue started %d\n", rx_queue_id);
+	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) {
+		vnic = rxq->vnic;
+		if (vnic->fw_grp_ids[rx_queue_id] != INVALID_HW_RING_ID)
+			return 0;
+		PMD_DRV_LOG(DEBUG, "vnic = %p fw_grp_id = %d\n",
+			vnic, bp->grp_info[rx_queue_id + 1].fw_grp_id);
+		vnic->fw_grp_ids[rx_queue_id] =
+					bp->grp_info[rx_queue_id + 1].fw_grp_id;
+		return bnxt_vnic_rss_configure(bp, vnic);
+	}
+
+	return 0;
+}
+
+int bnxt_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+	struct bnxt_rx_queue *rxq = bp->rx_queues[rx_queue_id];
+	struct bnxt_vnic_info *vnic = NULL;
+
+	dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
+	rxq->rx_deferred_start = true;
+	PMD_DRV_LOG(DEBUG, "Rx queue stopped\n");
+
+	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) {
+		vnic = rxq->vnic;
+		vnic->fw_grp_ids[rx_queue_id] = INVALID_HW_RING_ID;
+		return bnxt_vnic_rss_configure(bp, vnic);
+	}
+	return 0;
+}
+
 /*
  * Initialization
  */
@@ -3023,6 +3078,10 @@  static const struct eth_dev_ops bnxt_dev_ops = {
 	.rx_queue_count = bnxt_rx_queue_count_op,
 	.rx_descriptor_status = bnxt_rx_descriptor_status_op,
 	.tx_descriptor_status = bnxt_tx_descriptor_status_op,
+	.rx_queue_start = bnxt_rx_queue_start,
+	.rx_queue_stop = bnxt_rx_queue_stop,
+	.tx_queue_start = bnxt_tx_queue_start,
+	.tx_queue_stop = bnxt_tx_queue_stop,
 	.filter_ctrl = bnxt_filter_ctrl_op,
 	.dev_supported_ptypes_get = bnxt_dev_supported_ptypes_get_op,
 	.get_eeprom_length    = bnxt_get_eeprom_length_op,
diff --git a/drivers/net/bnxt/bnxt_rxq.h b/drivers/net/bnxt/bnxt_rxq.h
index be190195a..6bceee087 100644
--- a/drivers/net/bnxt/bnxt_rxq.h
+++ b/drivers/net/bnxt/bnxt_rxq.h
@@ -50,6 +50,7 @@  struct bnxt_rx_queue {
 	uint16_t		reg_idx; /* RX queue register index */
 	uint16_t		port_id; /* Device port identifier */
 	uint8_t			crc_len; /* 0 if CRC stripped, 4 otherwise */
+	uint8_t			rx_deferred_start; /* not in global dev start */
 
 	struct bnxt		*bp;
 	int			index;
@@ -75,5 +76,4 @@  int bnxt_rx_queue_intr_enable_op(struct rte_eth_dev *eth_dev,
 				 uint16_t queue_id);
 int bnxt_rx_queue_intr_disable_op(struct rte_eth_dev *eth_dev,
 				  uint16_t queue_id);
-
 #endif
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 3f07c11b5..9e70c8604 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -545,6 +545,10 @@  uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	uint16_t ag_prod = rxr->ag_prod;
 	int rc = 0;
 
+	/* If Rx Q was stopped return */
+	if (rxq->rx_deferred_start)
+		return 0;
+
 	/* Handle RX burst request */
 	while (1) {
 		cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index a94373d19..f3ed49bd6 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -120,5 +120,6 @@  uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 void bnxt_free_rx_rings(struct bnxt *bp);
 int bnxt_init_rx_ring_struct(struct bnxt_rx_queue *rxq, unsigned int socket_id);
 int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq);
-
+int bnxt_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id);
+int bnxt_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 #endif
diff --git a/drivers/net/bnxt/bnxt_txq.h b/drivers/net/bnxt/bnxt_txq.h
index f753c10f2..e27c34fa9 100644
--- a/drivers/net/bnxt/bnxt_txq.h
+++ b/drivers/net/bnxt/bnxt_txq.h
@@ -71,5 +71,4 @@  int bnxt_tx_queue_setup_op(struct rte_eth_dev *eth_dev,
 			       uint16_t nb_desc,
 			       unsigned int socket_id,
 			       const struct rte_eth_txconf *tx_conf);
-
 #endif
diff --git a/drivers/net/bnxt/bnxt_txr.c b/drivers/net/bnxt/bnxt_txr.c
index 2f2c87119..2c81a37c2 100644
--- a/drivers/net/bnxt/bnxt_txr.c
+++ b/drivers/net/bnxt/bnxt_txr.c
@@ -349,6 +349,11 @@  uint16_t bnxt_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	/* Handle TX completions */
 	bnxt_handle_tx_cp(txq);
 
+	/* Tx queue was stopped; wait for it to be restarted */
+	if (txq->tx_deferred_start) {
+		PMD_DRV_LOG(DEBUG, "Tx q stopped;return\n");
+		return 0;
+	}
 	/* Handle TX burst request */
 	for (nb_tx_pkts = 0; nb_tx_pkts < nb_pkts; nb_tx_pkts++) {
 		if (bnxt_start_xmit(tx_pkts[nb_tx_pkts], txq)) {
@@ -364,3 +369,30 @@  uint16_t bnxt_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 	return nb_tx_pkts;
 }
+
+int bnxt_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
+	struct bnxt_tx_queue *txq = bp->tx_queues[tx_queue_id];
+
+	dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
+	txq->tx_deferred_start = false;
+	PMD_DRV_LOG(DEBUG, "Tx queue started\n");
+
+	return 0;
+}
+
+int bnxt_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
+	struct bnxt_tx_queue *txq = bp->tx_queues[tx_queue_id];
+
+	/* Handle TX completions */
+	bnxt_handle_tx_cp(txq);
+
+	dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
+	txq->tx_deferred_start = true;
+	PMD_DRV_LOG(DEBUG, "Tx queue stopped\n");
+
+	return 0;
+}
diff --git a/drivers/net/bnxt/bnxt_txr.h b/drivers/net/bnxt/bnxt_txr.h
index 2feac51db..d88b15ab8 100644
--- a/drivers/net/bnxt/bnxt_txr.h
+++ b/drivers/net/bnxt/bnxt_txr.h
@@ -68,6 +68,8 @@  int bnxt_init_one_tx_ring(struct bnxt_tx_queue *txq);
 int bnxt_init_tx_ring_struct(struct bnxt_tx_queue *txq, unsigned int socket_id);
 uint16_t bnxt_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			       uint16_t nb_pkts);
+int bnxt_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);
+int bnxt_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id);
 
 #define PKT_TX_OIP_IIP_TCP_UDP_CKSUM	(PKT_TX_TCP_CKSUM | PKT_TX_UDP_CKSUM | \
 					PKT_TX_IP_CKSUM | PKT_TX_OUTER_IP_CKSUM)