DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support
@ 2019-10-02 23:25 Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 01/20] net/bnxt: return standard error codes for HWRM command Ajit Khaparde
                   ` (21 more replies)
  0 siblings, 22 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Among other changes, this patchset adds support to:
- create filters with RSS action.
- create source MAC filters.
- use user provided priority to place rule appropriately in HW.

This patch has been rebased to dpdk-next-net commit
8587a8b9eddefa39e4ceac7e9385efcc5e73307c

Please apply.


Ajit Khaparde (13):
  net/bnxt: return standard error codes for HWRM command
  net/bnxt: refactor code to allow dynamic creation of VNIC
  net/bnxt: allow flow creation when RSS is enabled
  net/bnxt: add support to create SMAC and inner DMAC filters
  net/bnxt: add support for RSS action
  net/bnxt: parse priority attribute for flow creation
  net/bnxt: delete and flush L2 filters cleanly
  net/bnxt: cleanup vnic after flow validate
  net/bnxt: allow only unicast MAC address filter creation
  net/bnxt: check device is started before flow creation
  net/bnxt: handle cleanup if flow creation fails
  net/bnxt: drop untagged frames when specified
  net/bnxt: handle flow flush handling

Kalesh AP (2):
  net/bnxt: fix an issue in setting default MAC address
  net/bnxt: fix multicast filter programming

Rahul Gupta (1):
  net/bnxt: properly handle ring cleanup in case of error

Somnath Kotur (1):
  net/bnxt: check for invalid VNIC ID in vnic tpa cfg

Venkat Duvvuru (3):
  net/bnxt: validate RSS hash key length
  net/bnxt: synchronize between flow related functions
  net/bnxt: fix VLAN filtering code path

 drivers/net/bnxt/bnxt.h        |  12 +
 drivers/net/bnxt/bnxt_ethdev.c | 254 ++++++---
 drivers/net/bnxt/bnxt_filter.c |  18 +-
 drivers/net/bnxt/bnxt_filter.h |  18 +
 drivers/net/bnxt/bnxt_flow.c   | 904 ++++++++++++++++++++++++++++-----
 drivers/net/bnxt/bnxt_hwrm.c   | 190 +++++--
 drivers/net/bnxt/bnxt_hwrm.h   |   7 +-
 drivers/net/bnxt/bnxt_ring.c   |  37 ++
 drivers/net/bnxt/bnxt_rxq.c    |  29 +-
 drivers/net/bnxt/bnxt_vnic.c   |  38 +-
 drivers/net/bnxt/bnxt_vnic.h   |   6 +
 11 files changed, 1240 insertions(+), 273 deletions(-)

-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 01/20] net/bnxt: return standard error codes for HWRM command
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 02/20] net/bnxt: refactor code to allow dynamic creation of VNIC Ajit Khaparde
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

If the FW returns an error for an HWRM request, it does not necessarily
return standard error codes. Convert these HWRM errors to standard errno.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index f476b10e9c..187d378544 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -219,8 +219,14 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg,
 		rte_spinlock_unlock(&bp->hwrm_lock); \
 		if (rc == HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED) \
 			rc = -EACCES; \
-		else if (rc > 0) \
+		else if (rc == HWRM_ERR_CODE_RESOURCE_ALLOC_ERROR) \
+			rc = -ENOSPC; \
+		else if (rc == HWRM_ERR_CODE_INVALID_PARAMS) \
 			rc = -EINVAL; \
+		else if (rc == HWRM_ERR_CODE_CMD_NOT_SUPPORTED) \
+			rc = -ENOTSUP; \
+		else if (rc > 0) \
+			rc = -EIO; \
 		return rc; \
 	} \
 	if (resp->error_code) { \
@@ -241,8 +247,14 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg,
 		rte_spinlock_unlock(&bp->hwrm_lock); \
 		if (rc == HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED) \
 			rc = -EACCES; \
-		else if (rc > 0) \
+		else if (rc == HWRM_ERR_CODE_RESOURCE_ALLOC_ERROR) \
+			rc = -ENOSPC; \
+		else if (rc == HWRM_ERR_CODE_INVALID_PARAMS) \
 			rc = -EINVAL; \
+		else if (rc == HWRM_ERR_CODE_CMD_NOT_SUPPORTED) \
+			rc = -ENOTSUP; \
+		else if (rc > 0) \
+			rc = -EIO; \
 		return rc; \
 	} \
 } while (0)
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 02/20] net/bnxt: refactor code to allow dynamic creation of VNIC
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 01/20] net/bnxt: return standard error codes for HWRM command Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 03/20] net/bnxt: allow flow creation when RSS is enabled Ajit Khaparde
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, Kalesh AP, Santoshkumar Karanappa Rastapur,
	Rahul Gupta, Venkat Duvvuru

Refactor code to allow dynamic creation of VNIC for RSS
or Queue Action during flow create.

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |  1 +
 drivers/net/bnxt/bnxt_ethdev.c | 54 ++++++++++------------------------
 drivers/net/bnxt/bnxt_flow.c   | 12 ++++++++
 drivers/net/bnxt/bnxt_hwrm.c   |  1 +
 drivers/net/bnxt/bnxt_rxq.c    | 27 +++++++----------
 drivers/net/bnxt/bnxt_vnic.c   | 36 +++++++++++++++++++++++
 drivers/net/bnxt/bnxt_vnic.h   |  3 ++
 7 files changed, 78 insertions(+), 56 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index f8a550ba54..a383ad4af6 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -454,6 +454,7 @@ struct bnxt {
 
 	unsigned int		rx_nr_rings;
 	unsigned int		rx_cp_nr_rings;
+	unsigned int		rx_num_qs_per_vnic;
 	struct bnxt_rx_queue **rx_queues;
 	const void		*rx_mem_zone;
 	struct rx_port_stats    *hw_rx_port_stats;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index bc9a910ee3..d8ffe45f63 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -318,17 +318,10 @@ static int bnxt_init_chip(struct bnxt *bp)
 	for (i = 0; i < bp->nr_vnics; i++) {
 		struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 		struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
-		uint32_t size = sizeof(*vnic->fw_grp_ids) * bp->max_ring_grps;
 
-		vnic->fw_grp_ids = rte_zmalloc("vnic_fw_grp_ids", size, 0);
-		if (!vnic->fw_grp_ids) {
-			PMD_DRV_LOG(ERR,
-				    "Failed to alloc %d bytes for group ids\n",
-				    size);
-			rc = -ENOMEM;
+		rc = bnxt_vnic_grp_alloc(bp, vnic);
+		if (rc)
 			goto err_out;
-		}
-		memset(vnic->fw_grp_ids, -1, size);
 
 		PMD_DRV_LOG(DEBUG, "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
 			    i, vnic, vnic->fw_grp_ids);
@@ -384,7 +377,7 @@ static int bnxt_init_chip(struct bnxt *bp)
 			goto err_out;
 		}
 
-		for (j = 0; j < bp->rx_nr_rings; j++) {
+		for (j = 0; j < bp->rx_num_qs_per_vnic; j++) {
 			rxq = bp->eth_dev->data->rx_queues[j];
 
 			PMD_DRV_LOG(DEBUG,
@@ -1353,8 +1346,6 @@ static int bnxt_rss_hash_update_op(struct rte_eth_dev *eth_dev,
 	struct bnxt *bp = eth_dev->data->dev_private;
 	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	struct bnxt_vnic_info *vnic;
-	uint16_t hash_type = 0;
-	unsigned int i;
 	int rc;
 
 	rc = is_bnxt_in_error(bp);
@@ -1376,35 +1367,20 @@ static int bnxt_rss_hash_update_op(struct rte_eth_dev *eth_dev,
 	bp->flags |= BNXT_FLAG_UPDATE_HASH;
 	memcpy(&bp->rss_conf, rss_conf, sizeof(*rss_conf));
 
-	if (rss_conf->rss_hf & ETH_RSS_IPV4)
-		hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV4;
-	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
-		hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV4;
-	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
-		hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV4;
-	if (rss_conf->rss_hf & ETH_RSS_IPV6)
-		hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV6;
-	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
-		hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV6;
-	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
-		hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV6;
-
-	/* Update the RSS VNIC(s) */
-	for (i = 0; i < bp->nr_vnics; i++) {
-		vnic = &bp->vnic_info[i];
-		vnic->hash_type = hash_type;
+	/* Update the default RSS VNIC(s) */
+	vnic = &bp->vnic_info[0];
+	vnic->hash_type = bnxt_rte_to_hwrm_hash_types(rss_conf->rss_hf);
 
-		/*
-		 * Use the supplied key if the key length is
-		 * acceptable and the rss_key is not NULL
-		 */
-		if (rss_conf->rss_key &&
-		    rss_conf->rss_key_len <= HW_HASH_KEY_SIZE)
-			memcpy(vnic->rss_hash_key, rss_conf->rss_key,
-			       rss_conf->rss_key_len);
+	/*
+	 * Use the supplied key if the key length is
+	 * acceptable and the rss_key is not NULL
+	 */
+	if (rss_conf->rss_key && rss_conf->rss_key_len <= HW_HASH_KEY_SIZE)
+		memcpy(vnic->rss_hash_key,
+		       rss_conf->rss_key,
+		       rss_conf->rss_key_len);
 
-		bnxt_hwrm_vnic_rss_cfg(bp, vnic);
-	}
+	bnxt_hwrm_vnic_rss_cfg(bp, vnic);
 	return 0;
 }
 
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index be9b6fad39..13bdaf0340 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1071,6 +1071,13 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 	int ret = 0;
 	uint32_t tun_type;
 
+	if (BNXT_VF(bp) && !BNXT_VF_IS_TRUSTED(bp)) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Failed to create flow, Not a Trusted VF!");
+		return NULL;
+	}
+
 	flow = rte_zmalloc("bnxt_flow", sizeof(struct rte_flow), 0);
 	if (!flow) {
 		rte_flow_error_set(error, ENOMEM,
@@ -1249,6 +1256,11 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 	struct bnxt_vnic_info *vnic = flow->vnic;
 	int ret = 0;
 
+	if (!filter) {
+		ret = -EINVAL;
+		goto done;
+	}
+
 	if (filter->filter_type == HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
 	    filter->enables == filter->tunnel_type) {
 		ret = bnxt_handle_tunnel_redirect_destroy(bp,
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 187d378544..35fb7dca25 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2327,6 +2327,7 @@ int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 		else
 			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
+		bnxt_free_filter(bp, filter);
 		//if (rc)
 			//break;
 	}
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index dac8ccb167..667d0486e3 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -100,6 +100,7 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
 		}
 	}
 	nb_q_per_grp = bp->rx_cp_nr_rings / pools;
+	bp->rx_num_qs_per_vnic = nb_q_per_grp;
 	PMD_DRV_LOG(DEBUG, "pools = %u nb_q_per_grp = %u\n",
 		    pools, nb_q_per_grp);
 	start_grp_id = 0;
@@ -158,29 +159,16 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
 out:
 	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) {
 		struct rte_eth_rss_conf *rss = &dev_conf->rx_adv_conf.rss_conf;
-		uint16_t hash_type = 0;
 
 		if (bp->flags & BNXT_FLAG_UPDATE_HASH) {
 			rss = &bp->rss_conf;
 			bp->flags &= ~BNXT_FLAG_UPDATE_HASH;
 		}
 
-		if (rss->rss_hf & ETH_RSS_IPV4)
-			hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV4;
-		if (rss->rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
-			hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV4;
-		if (rss->rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
-			hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV4;
-		if (rss->rss_hf & ETH_RSS_IPV6)
-			hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV6;
-		if (rss->rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
-			hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV6;
-		if (rss->rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
-			hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV6;
-
 		for (i = 0; i < bp->nr_vnics; i++) {
 			vnic = &bp->vnic_info[i];
-			vnic->hash_type = hash_type;
+			vnic->hash_type =
+				bnxt_rte_to_hwrm_hash_types(rss->rss_hf);
 
 			/*
 			 * Use the supplied key if the key length is
@@ -469,7 +457,9 @@ int bnxt_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 				    vnic, bp->grp_info[rx_queue_id].fw_grp_id);
 		}
 
-		rc = bnxt_vnic_rss_configure(bp, vnic);
+		PMD_DRV_LOG(DEBUG, "Rx Queue Count %d\n", vnic->rx_queue_cnt);
+		if (vnic->rx_queue_cnt > 1)
+			rc = bnxt_vnic_rss_configure(bp, vnic);
 	}
 
 	if (rc == 0)
@@ -522,7 +512,10 @@ int bnxt_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 		vnic = rxq->vnic;
 		if (BNXT_HAS_RING_GRPS(bp))
 			vnic->fw_grp_ids[rx_queue_id] = INVALID_HW_RING_ID;
-		rc = bnxt_vnic_rss_configure(bp, vnic);
+
+		PMD_DRV_LOG(DEBUG, "Rx Queue Count %d\n", vnic->rx_queue_cnt);
+		if (vnic->rx_queue_cnt > 1)
+			rc = bnxt_vnic_rss_configure(bp, vnic);
 	}
 
 	if (rc == 0)
diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index 9ea99388b7..4f3f9b3594 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -222,3 +222,39 @@ int bnxt_alloc_vnic_mem(struct bnxt *bp)
 	bp->vnic_info = vnic_mem;
 	return 0;
 }
+
+int bnxt_vnic_grp_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic)
+{
+	uint32_t size = sizeof(*vnic->fw_grp_ids) * bp->max_ring_grps;
+
+	vnic->fw_grp_ids = rte_zmalloc("vnic_fw_grp_ids", size, 0);
+	if (!vnic->fw_grp_ids) {
+		PMD_DRV_LOG(ERR,
+			    "Failed to alloc %d bytes for group ids\n",
+			    size);
+		return -ENOMEM;
+	}
+	memset(vnic->fw_grp_ids, -1, size);
+
+	return 0;
+}
+
+uint16_t bnxt_rte_to_hwrm_hash_types(uint64_t rte_type)
+{
+	uint16_t hwrm_type = 0;
+
+	if (rte_type & ETH_RSS_IPV4)
+		hwrm_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV4;
+	if (rte_type & ETH_RSS_NONFRAG_IPV4_TCP)
+		hwrm_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV4;
+	if (rte_type & ETH_RSS_NONFRAG_IPV4_UDP)
+		hwrm_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV4;
+	if (rte_type & ETH_RSS_IPV6)
+		hwrm_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV6;
+	if (rte_type & ETH_RSS_NONFRAG_IPV6_TCP)
+		hwrm_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV6;
+	if (rte_type & ETH_RSS_NONFRAG_IPV6_UDP)
+		hwrm_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV6;
+
+	return hwrm_type;
+}
diff --git a/drivers/net/bnxt/bnxt_vnic.h b/drivers/net/bnxt/bnxt_vnic.h
index 16a0d57635..cb2707f36a 100644
--- a/drivers/net/bnxt/bnxt_vnic.h
+++ b/drivers/net/bnxt/bnxt_vnic.h
@@ -42,6 +42,7 @@ struct bnxt_vnic_info {
 
 	uint16_t	cos_rule;
 	uint16_t	lb_rule;
+	uint16_t	rx_queue_cnt;
 	bool		vlan_strip;
 	bool		func_default;
 	bool		bd_stall;
@@ -63,4 +64,6 @@ void bnxt_free_vnic_attributes(struct bnxt *bp);
 int bnxt_alloc_vnic_attributes(struct bnxt *bp);
 void bnxt_free_vnic_mem(struct bnxt *bp);
 int bnxt_alloc_vnic_mem(struct bnxt *bp);
+int bnxt_vnic_grp_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic);
+uint16_t bnxt_rte_to_hwrm_hash_types(uint64_t rte_type);
 #endif
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 03/20] net/bnxt: allow flow creation when RSS is enabled
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 01/20] net/bnxt: return standard error codes for HWRM command Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 02/20] net/bnxt: refactor code to allow dynamic creation of VNIC Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 04/20] net/bnxt: add support to create SMAC and inner DMAC filters Ajit Khaparde
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Venkat Duvvuru

Currently flow creation is allowed with queue action only
when RSS is disabled. Remove this restriction. Flows can be
created when RSS is enabled.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |   1 +
 drivers/net/bnxt/bnxt_ethdev.c |   2 +-
 drivers/net/bnxt/bnxt_flow.c   | 177 +++++++++++++++++++++++++++------
 drivers/net/bnxt/bnxt_vnic.h   |   2 +
 4 files changed, 149 insertions(+), 33 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index a383ad4af6..c5dceb7d28 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -554,6 +554,7 @@ struct bnxt {
 int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete);
 int bnxt_rcv_msg_from_vf(struct bnxt *bp, uint16_t vf_id, void *msg);
 int is_bnxt_in_error(struct bnxt *bp);
+uint16_t bnxt_rss_ctxts(const struct bnxt *bp);
 
 int bnxt_map_fw_health_status_regs(struct bnxt *bp);
 uint32_t bnxt_read_fw_status_reg(struct bnxt *bp, uint32_t index);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index d8ffe45f63..c953979cca 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -189,7 +189,7 @@ int is_bnxt_in_error(struct bnxt *bp)
  * High level utility functions
  */
 
-static uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
+uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
 {
 	if (!BNXT_CHIP_THOR(bp))
 		return 1;
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 13bdaf0340..4c45fd23bf 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -14,6 +14,8 @@
 #include "bnxt.h"
 #include "bnxt_filter.h"
 #include "bnxt_hwrm.h"
+#include "bnxt_ring.h"
+#include "bnxt_rxq.h"
 #include "bnxt_vnic.h"
 #include "bnxt_util.h"
 #include "hsi_struct_def_dpdk.h"
@@ -151,23 +153,13 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 	int use_ntuple;
 	uint32_t en = 0;
 	uint32_t en_ethertype;
-	int dflt_vnic, rc = 0;
+	int dflt_vnic;
 
 	use_ntuple = bnxt_filter_type_check(pattern, error);
 	PMD_DRV_LOG(DEBUG, "Use NTUPLE %d\n", use_ntuple);
 	if (use_ntuple < 0)
 		return use_ntuple;
 
-	if (use_ntuple && (bp->eth_dev->data->dev_conf.rxmode.mq_mode &
-	    ETH_MQ_RX_RSS)) {
-		PMD_DRV_LOG(ERR, "Cannot create ntuple flow on RSS queues\n");
-		rte_flow_error_set(error, EINVAL,
-				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
-				   "Cannot create flow on RSS queues");
-		rc = -rte_errno;
-		return rc;
-	}
-
 	filter->filter_type = use_ntuple ?
 		HWRM_CFA_NTUPLE_FILTER : HWRM_CFA_EM_FILTER;
 	en_ethertype = use_ntuple ?
@@ -715,17 +707,6 @@ bnxt_flow_parse_attr(const struct rte_flow_attr *attr,
 				   "No support for priority.");
 		return -rte_errno;
 	}
-
-	/* Not supported */
-	if (attr->group) {
-		rte_flow_error_set(error,
-				   EINVAL,
-				   RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
-				   attr,
-				   "No support for group.");
-		return -rte_errno;
-	}
-
 	return 0;
 }
 
@@ -764,6 +745,50 @@ bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 	return filter1;
 }
 
+static int bnxt_vnic_prep(struct bnxt *bp, struct bnxt_vnic_info *vnic)
+{
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+	uint64_t rx_offloads = dev_conf->rxmode.offloads;
+	int rc;
+
+	rc = bnxt_vnic_grp_alloc(bp, vnic);
+	if (rc)
+		goto ret;
+
+	rc = bnxt_hwrm_vnic_alloc(bp, vnic);
+	if (rc) {
+		PMD_DRV_LOG(ERR, "HWRM vnic alloc failure rc: %x\n", rc);
+		goto ret;
+	}
+	bp->nr_vnics++;
+
+	/* RSS context is required only when there is more than one RSS ring */
+	if (vnic->rx_queue_cnt > 1) {
+		rc = bnxt_hwrm_vnic_ctx_alloc(bp, vnic, 0 /* ctx_idx 0 */);
+		if (rc) {
+			PMD_DRV_LOG(ERR,
+				    "HWRM vnic ctx alloc failure: %x\n", rc);
+			goto ret;
+		}
+	} else {
+		PMD_DRV_LOG(DEBUG, "No RSS context required\n");
+	}
+
+	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+		vnic->vlan_strip = true;
+	else
+		vnic->vlan_strip = false;
+
+	rc = bnxt_hwrm_vnic_cfg(bp, vnic);
+	if (rc)
+		goto ret;
+
+	bnxt_hwrm_vnic_plcmode_cfg(bp, vnic);
+
+ret:
+	return rc;
+}
+
 static int
 bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			     const struct rte_flow_item pattern[],
@@ -775,12 +800,14 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 	const struct rte_flow_action *act =
 		bnxt_flow_non_void_action(actions);
 	struct bnxt *bp = dev->data->dev_private;
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	const struct rte_flow_action_queue *act_q;
 	const struct rte_flow_action_vf *act_vf;
 	struct bnxt_vnic_info *vnic, *vnic0;
 	struct bnxt_filter_info *filter1;
+	struct bnxt_rx_queue *rxq = NULL;
+	int dflt_vnic, vnic_id;
 	uint32_t vf = 0;
-	int dflt_vnic;
 	int rc;
 
 	rc =
@@ -800,7 +827,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 	case RTE_FLOW_ACTION_TYPE_QUEUE:
 		/* Allow this flow. Redirect to a VNIC. */
 		act_q = (const struct rte_flow_action_queue *)act->conf;
-		if (act_q->index >= bp->rx_nr_rings) {
+		if (!act_q->index || act_q->index >= bp->rx_nr_rings) {
 			rte_flow_error_set(error,
 					   EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ACTION,
@@ -811,18 +838,78 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		}
 		PMD_DRV_LOG(DEBUG, "Queue index %d\n", act_q->index);
 
-		vnic0 = &bp->vnic_info[0];
-		vnic =  &bp->vnic_info[act_q->index];
+		vnic_id = attr->group;
+		if (!vnic_id) {
+			PMD_DRV_LOG(DEBUG, "Group id is 0\n");
+			vnic_id = act_q->index;
+		}
+
+		vnic = &bp->vnic_info[vnic_id];
 		if (vnic == NULL) {
 			rte_flow_error_set(error,
 					   EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ACTION,
 					   act,
-					   "No matching VNIC for queue ID.");
+					   "No matching VNIC found.");
 			rc = -rte_errno;
 			goto ret;
 		}
+		if (vnic->rx_queue_cnt) {
+			if (vnic->rx_queue_cnt > 1 ||
+			    vnic->start_grp_id != act_q->index) {
+				PMD_DRV_LOG(ERR,
+					    "VNIC already in use\n");
+				rte_flow_error_set(error,
+						   EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ACTION,
+						   act,
+						   "VNIC already in use");
+				rc = -rte_errno;
+				goto ret;
+			}
+			goto use_vnic;
+		}
+
+		rxq = bp->rx_queues[act_q->index];
+
+		if (!(dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) && rxq)
+			goto use_vnic;
+
+		if (!rxq ||
+		    bp->vnic_info[0].fw_grp_ids[act_q->index] !=
+		    INVALID_HW_RING_ID ||
+		    !rxq->rx_deferred_start) {
+			PMD_DRV_LOG(ERR,
+				    "Queue invalid or used with other VNIC\n");
+			rte_flow_error_set(error,
+					   EINVAL,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "Queue invalid queue or in use");
+			rc = -rte_errno;
+			goto ret;
+		}
+
+use_vnic:
+		rxq->vnic = vnic;
+		vnic->rx_queue_cnt++;
+		vnic->start_grp_id = act_q->index;
+		vnic->end_grp_id = act_q->index;
+		vnic->func_default = 0;	//This is not a default VNIC.
+
+		PMD_DRV_LOG(DEBUG, "VNIC found\n");
+
+		rc = bnxt_vnic_prep(bp, vnic);
+		if (rc)
+			goto ret;
+
+		PMD_DRV_LOG(DEBUG,
+			    "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
+			    act_q->index, vnic, vnic->fw_grp_ids);
 
+		vnic->ff_pool_idx = vnic_id;
+		PMD_DRV_LOG(DEBUG,
+			    "Setting vnic ff_idx %d\n", vnic->ff_pool_idx);
 		filter->dst_id = vnic->fw_vnic_id;
 		filter1 = bnxt_get_l2_filter(bp, filter, vnic);
 		if (filter1 == NULL) {
@@ -989,9 +1076,12 @@ bnxt_match_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 	struct rte_flow *flow;
 	int i;
 
-	for (i = bp->nr_vnics - 1; i >= 0; i--) {
+	for (i = bp->max_vnics; i >= 0; i--) {
 		struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
 
+		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
+			continue;
+
 		STAILQ_FOREACH(flow, &vnic->flow_list, next) {
 			mf = flow->filter;
 
@@ -1063,8 +1153,8 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		 struct rte_flow_error *error)
 {
 	struct bnxt *bp = dev->data->dev_private;
-	struct bnxt_filter_info *filter;
 	struct bnxt_vnic_info *vnic = NULL;
+	struct bnxt_filter_info *filter;
 	bool update_flow = false;
 	struct rte_flow *flow;
 	unsigned int i;
@@ -1168,12 +1258,35 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		ret = bnxt_hwrm_set_ntuple_filter(bp, filter->dst_id, filter);
 	}
 
-	for (i = 0; i < bp->nr_vnics; i++) {
+	for (i = 0; i < bp->max_vnics; i++) {
 		vnic = &bp->vnic_info[i];
-		if (filter->dst_id == vnic->fw_vnic_id)
+		if (vnic->fw_vnic_id != INVALID_VNIC_ID &&
+		    filter->dst_id == vnic->fw_vnic_id) {
+			PMD_DRV_LOG(ERR, "Found matching VNIC Id %d\n",
+				    vnic->ff_pool_idx);
 			break;
+		}
 	}
 done:
+	if (!ret) {
+		flow->filter = filter;
+		flow->vnic = vnic;
+		/* VNIC is set only in case of queue or RSS action */
+		if (vnic) {
+			/*
+			 * RxQ0 is not used for flow filters.
+			 */
+
+			if (update_flow) {
+				ret = -EXDEV;
+				goto free_flow;
+			}
+			STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
+		}
+		PMD_DRV_LOG(ERR, "Successfully created flow.\n");
+		STAILQ_INSERT_TAIL(&vnic->flow_list, flow, next);
+		return flow;
+	}
 	if (!ret) {
 		flow->filter = filter;
 		flow->vnic = vnic;
@@ -1196,7 +1309,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		rte_flow_error_set(error, ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Flow with pattern exists, updating destination queue");
-	else
+	else if (!rte_errno)
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to create flow.");
diff --git a/drivers/net/bnxt/bnxt_vnic.h b/drivers/net/bnxt/bnxt_vnic.h
index cb2707f36a..ec3a3ff51f 100644
--- a/drivers/net/bnxt/bnxt_vnic.h
+++ b/drivers/net/bnxt/bnxt_vnic.h
@@ -9,6 +9,8 @@
 #include <sys/queue.h>
 #include <stdbool.h>
 
+#define INVALID_VNIC_ID		((uint16_t)-1)
+
 struct bnxt_vnic_info {
 	STAILQ_ENTRY(bnxt_vnic_info)	next;
 	uint8_t		ff_pool_idx;
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 04/20] net/bnxt: add support to create SMAC and inner DMAC filters
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (2 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 03/20] net/bnxt: allow flow creation when RSS is enabled Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 05/20] net/bnxt: add support for RSS action Ajit Khaparde
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, Rahul Gupta, Kalesh AP

We are currently creating only outer DMAC filters.
Create SMAC and inner DMAC filters using HWRM_CFA_L2_FILTER_ALLOC.
For this the HWRM_CFA_L2_FILTER_ALLOC has already been updated.

Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |   4 ++
 drivers/net/bnxt/bnxt_ethdev.c |   9 ++-
 drivers/net/bnxt/bnxt_filter.h |   8 +++
 drivers/net/bnxt/bnxt_flow.c   | 118 +++++++++++++++++++++++++--------
 drivers/net/bnxt/bnxt_hwrm.c   |  44 +++++++++++-
 drivers/net/bnxt/bnxt_hwrm.h   |   4 ++
 drivers/net/bnxt/bnxt_rxq.c    |   2 +
 7 files changed, 160 insertions(+), 29 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index c5dceb7d28..8602ab3346 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -439,6 +439,7 @@ struct bnxt {
 #define BNXT_FLAG_NEW_RM			BIT(23)
 #define BNXT_FLAG_INIT_DONE			BIT(24)
 #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TS		BIT(25)
+#define BNXT_FLAG_ADV_FLOW_MGMT			BIT(26)
 #define BNXT_PF(bp)		(!((bp)->flags & BNXT_FLAG_VF))
 #define BNXT_VF(bp)		((bp)->flags & BNXT_FLAG_VF)
 #define BNXT_NPAR(bp)		((bp)->port_partition_type)
@@ -452,6 +453,9 @@ struct bnxt {
 #define BNXT_HAS_NQ(bp)		BNXT_CHIP_THOR(bp)
 #define BNXT_HAS_RING_GRPS(bp)	(!BNXT_CHIP_THOR(bp))
 
+	uint32_t		flow_flags;
+#define BNXT_FLOW_FLAG_L2_HDR_SRC_FILTER_EN	BIT(0)
+
 	unsigned int		rx_nr_rings;
 	unsigned int		rx_cp_nr_rings;
 	unsigned int		rx_num_qs_per_vnic;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index c953979cca..7cd3213558 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1039,6 +1039,7 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 
 	filter->mac_index = index;
 	memcpy(filter->l2_addr, mac_addr, RTE_ETHER_ADDR_LEN);
+	filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
 
 	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
 	if (!rc) {
@@ -1739,6 +1740,7 @@ static int bnxt_add_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 	filter->l2_ivlan = vlan_id;
 	filter->l2_ivlan_mask = 0x0FFF;
 	filter->enables |= en;
+	filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
 	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
 	if (rc) {
 		/* Free the newly allocated filter as we were
@@ -1904,7 +1906,8 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev,
 
 		memcpy(filter->l2_addr, bp->mac_addr, RTE_ETHER_ADDR_LEN);
 		memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
-		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
+		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX |
+			HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
 		filter->enables |=
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK;
@@ -4433,6 +4436,10 @@ static int bnxt_init_fw(struct bnxt *bp)
 	if (rc)
 		return -EIO;
 
+	rc = bnxt_hwrm_cfa_adv_flow_mgmt_qcaps(bp);
+	if (rc)
+		return rc;
+
 	rc = bnxt_hwrm_queue_qportcfg(bp);
 	if (rc)
 		return rc;
diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 4fda3f03a0..52d3582ba0 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -9,6 +9,13 @@
 #include <rte_ether.h>
 
 struct bnxt;
+
+#define BNXT_FLOW_L2_VALID_FLAG			BIT(0)
+#define BNXT_FLOW_L2_SRC_VALID_FLAG		BIT(1)
+#define BNXT_FLOW_L2_INNER_SRC_VALID_FLAG	BIT(2)
+#define BNXT_FLOW_L2_DST_VALID_FLAG		BIT(3)
+#define BNXT_FLOW_L2_INNER_DST_VALID_FLAG	BIT(4)
+
 struct bnxt_filter_info {
 	STAILQ_ENTRY(bnxt_filter_info)	next;
 	uint64_t		fw_l2_filter_id;
@@ -28,6 +35,7 @@ struct bnxt_filter_info {
 	uint32_t		enables;
 	uint8_t			l2_addr[RTE_ETHER_ADDR_LEN];
 	uint8_t			l2_addr_mask[RTE_ETHER_ADDR_LEN];
+	uint32_t		valid_flags;
 	uint16_t		l2_ovlan;
 	uint16_t		l2_ovlan_mask;
 	uint16_t		l2_ivlan;
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 4c45fd23bf..d1a8c4459a 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -83,14 +83,17 @@ bnxt_filter_type_check(const struct rte_flow_item pattern[],
 	const struct rte_flow_item *item =
 		bnxt_flow_non_void_item(pattern);
 	int use_ntuple = 1;
+	bool has_vlan = 0;
 
 	while (item->type != RTE_FLOW_ITEM_TYPE_END) {
 		switch (item->type) {
+		case RTE_FLOW_ITEM_TYPE_ANY:
 		case RTE_FLOW_ITEM_TYPE_ETH:
-			use_ntuple = 1;
+			use_ntuple = 0;
 			break;
 		case RTE_FLOW_ITEM_TYPE_VLAN:
 			use_ntuple = 0;
+			has_vlan = 1;
 			break;
 		case RTE_FLOW_ITEM_TYPE_IPV4:
 		case RTE_FLOW_ITEM_TYPE_IPV6:
@@ -98,28 +101,25 @@ bnxt_filter_type_check(const struct rte_flow_item pattern[],
 		case RTE_FLOW_ITEM_TYPE_UDP:
 			/* FALLTHROUGH */
 			/* need ntuple match, reset exact match */
-			if (!use_ntuple) {
-				PMD_DRV_LOG(ERR,
-					"VLAN flow cannot use NTUPLE filter\n");
-				rte_flow_error_set
-					(error,
-					 EINVAL,
-					 RTE_FLOW_ERROR_TYPE_ITEM,
-					 item,
-					 "Cannot use VLAN with NTUPLE");
-				return -rte_errno;
-			}
 			use_ntuple |= 1;
 			break;
-		case RTE_FLOW_ITEM_TYPE_ANY:
-			use_ntuple = 0;
-			break;
 		default:
 			PMD_DRV_LOG(DEBUG, "Unknown Flow type\n");
 			use_ntuple |= 0;
 		}
 		item++;
 	}
+
+	if (has_vlan && use_ntuple) {
+		PMD_DRV_LOG(ERR,
+			    "VLAN flow cannot use NTUPLE filter\n");
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_ITEM,
+				   item,
+				   "Cannot use VLAN with NTUPLE");
+		return -rte_errno;
+	}
+
 	return use_ntuple;
 }
 
@@ -146,13 +146,14 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 	uint8_t vni_mask[] = {0xFF, 0xFF, 0xFF};
 	uint8_t tni_mask[] = {0xFF, 0xFF, 0xFF};
 	const struct rte_flow_item_vf *vf_spec;
-	uint32_t tenant_id_be = 0;
+	uint32_t tenant_id_be = 0, valid_flags = 0;
 	bool vni_masked = 0;
 	bool tni_masked = 0;
+	uint32_t en_ethertype;
+	uint8_t inner = 0;
 	uint32_t vf = 0;
-	int use_ntuple;
 	uint32_t en = 0;
-	uint32_t en_ethertype;
+	int use_ntuple;
 	int dflt_vnic;
 
 	use_ntuple = bnxt_filter_type_check(pattern, error);
@@ -177,6 +178,12 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 		}
 
 		switch (item->type) {
+		case RTE_FLOW_ITEM_TYPE_ANY:
+			inner =
+			((const struct rte_flow_item_any *)item->spec)->num > 3;
+			if (inner)
+				PMD_DRV_LOG(DEBUG, "Parse inner header\n");
+			break;
 		case RTE_FLOW_ITEM_TYPE_ETH:
 			if (!item->spec || !item->mask)
 				break;
@@ -213,18 +220,24 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 
 			if (rte_is_broadcast_ether_addr(&eth_mask->dst)) {
 				rte_memcpy(filter->dst_macaddr,
-					   &eth_spec->dst, 6);
+					   &eth_spec->dst, RTE_ETHER_ADDR_LEN);
 				en |= use_ntuple ?
 					NTUPLE_FLTR_ALLOC_INPUT_EN_DST_MACADDR :
 					EM_FLOW_ALLOC_INPUT_EN_DST_MACADDR;
+				valid_flags |= inner ?
+					BNXT_FLOW_L2_INNER_DST_VALID_FLAG :
+					BNXT_FLOW_L2_DST_VALID_FLAG;
 			}
 
 			if (rte_is_broadcast_ether_addr(&eth_mask->src)) {
 				rte_memcpy(filter->src_macaddr,
-					   &eth_spec->src, 6);
+					   &eth_spec->src, RTE_ETHER_ADDR_LEN);
 				en |= use_ntuple ?
 					NTUPLE_FLTR_ALLOC_INPUT_EN_SRC_MACADDR :
 					EM_FLOW_ALLOC_INPUT_EN_SRC_MACADDR;
+				valid_flags |= inner ?
+					BNXT_FLOW_L2_INNER_SRC_VALID_FLAG :
+					BNXT_FLOW_L2_SRC_VALID_FLAG;
 			} /*
 			   * else {
 			   *  PMD_DRV_LOG(ERR, "Handle this condition\n");
@@ -669,6 +682,7 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 		item++;
 	}
 	filter->enables = en;
+	filter->valid_flags = valid_flags;
 
 	return 0;
 }
@@ -725,16 +739,45 @@ bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 	if (memcmp(f0->l2_addr, nf->dst_macaddr, RTE_ETHER_ADDR_LEN) == 0)
 		return f0;
 
-	/* This flow needs DST MAC which is not same as port/l2 */
-	PMD_DRV_LOG(DEBUG, "Create L2 filter for DST MAC\n");
+	/* Alloc new L2 filter.
+	 * This flow needs MAC filter which does not match port/l2 MAC.
+	 */
 	filter1 = bnxt_get_unused_filter(bp);
 	if (filter1 == NULL)
 		return NULL;
 
-	filter1->flags = HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
+	filter1->flags = HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_XDP_DISABLE;
+	filter1->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
+	if (nf->valid_flags & BNXT_FLOW_L2_SRC_VALID_FLAG ||
+	    nf->valid_flags & BNXT_FLOW_L2_DST_VALID_FLAG) {
+		filter1->flags |=
+			HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
+		PMD_DRV_LOG(DEBUG, "Create Outer filter\n");
+	}
+
+	if (nf->filter_type == HWRM_CFA_L2_FILTER &&
+	    (nf->valid_flags & BNXT_FLOW_L2_SRC_VALID_FLAG ||
+	     nf->valid_flags & BNXT_FLOW_L2_INNER_SRC_VALID_FLAG)) {
+		PMD_DRV_LOG(DEBUG, "Create L2 filter for SRC MAC\n");
+		filter1->flags |=
+			HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_SOURCE_VALID;
+		memcpy(filter1->l2_addr, nf->src_macaddr, RTE_ETHER_ADDR_LEN);
+	} else {
+		PMD_DRV_LOG(DEBUG, "Create L2 filter for DST MAC\n");
+		memcpy(filter1->l2_addr, nf->dst_macaddr, RTE_ETHER_ADDR_LEN);
+	}
+
+	if (nf->valid_flags & BNXT_FLOW_L2_DST_VALID_FLAG ||
+	    nf->valid_flags & BNXT_FLOW_L2_INNER_DST_VALID_FLAG) {
+		/* Tell the FW where to place the filter in the table. */
+		filter1->pri_hint =
+			HWRM_CFA_L2_FILTER_ALLOC_INPUT_PRI_HINT_BELOW_FILTER;
+		/* This will place the filter in TCAM */
+		filter1->l2_filter_id_hint = (uint64_t)-1;
+	}
+
 	filter1->enables = HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
 			L2_FILTER_ALLOC_INPUT_EN_L2_ADDR_MASK;
-	memcpy(filter1->l2_addr, nf->dst_macaddr, RTE_ETHER_ADDR_LEN);
 	memset(filter1->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
 	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id,
 				     filter1);
@@ -843,6 +886,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			PMD_DRV_LOG(DEBUG, "Group id is 0\n");
 			vnic_id = act_q->index;
 		}
+		PMD_DRV_LOG(DEBUG, "VNIC found\n");
 
 		vnic = &bp->vnic_info[vnic_id];
 		if (vnic == NULL) {
@@ -872,7 +916,8 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 
 		rxq = bp->rx_queues[act_q->index];
 
-		if (!(dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) && rxq)
+		if (!(dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) && rxq &&
+		    vnic->fw_vnic_id != INVALID_HW_RING_ID)
 			goto use_vnic;
 
 		if (!rxq ||
@@ -917,8 +962,20 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			goto ret;
 		}
 
+		if (!(filter->valid_flags &
+		      ~(BNXT_FLOW_L2_DST_VALID_FLAG |
+			BNXT_FLOW_L2_SRC_VALID_FLAG |
+			BNXT_FLOW_L2_INNER_SRC_VALID_FLAG |
+			BNXT_FLOW_L2_INNER_DST_VALID_FLAG))) {
+			PMD_DRV_LOG(DEBUG, "L2 filter created\n");
+			filter->flags = filter1->flags;
+			filter->enables = filter1->enables;
+			filter->filter_type = HWRM_CFA_L2_FILTER;
+			memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
+			filter->pri_hint = filter1->pri_hint;
+			filter->l2_filter_id_hint = filter1->l2_filter_id_hint;
+		}
 		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
-		PMD_DRV_LOG(DEBUG, "VNIC found\n");
 		break;
 	case RTE_FLOW_ACTION_TYPE_DROP:
 		vnic0 = &bp->vnic_info[0];
@@ -1009,6 +1066,15 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 
 		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
 		break;
+	case RTE_FLOW_ACTION_TYPE_RSS:
+		rte_flow_error_set(error,
+				   ENOTSUP,
+				   RTE_FLOW_ERROR_TYPE_ACTION,
+				   act,
+				   "This action is not supported right now.");
+		rc = -rte_errno;
+		goto ret;
+		//break;
 
 	default:
 		rte_flow_error_set(error,
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 35fb7dca25..ceaeb609a4 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -425,8 +425,6 @@ int bnxt_hwrm_set_l2_filter(struct bnxt *bp,
 	HWRM_PREP(req, CFA_L2_FILTER_ALLOC, BNXT_USE_CHIMP_MB);
 
 	req.flags = rte_cpu_to_le_32(filter->flags);
-	req.flags |=
-	rte_cpu_to_le_32(HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST);
 
 	enables = filter->enables |
 	      HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_DST_ID;
@@ -456,6 +454,11 @@ int bnxt_hwrm_set_l2_filter(struct bnxt *bp,
 		req.src_id = rte_cpu_to_le_32(filter->src_id);
 	if (enables & HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_SRC_TYPE)
 		req.src_type = filter->src_type;
+	if (filter->pri_hint) {
+		req.pri_hint = filter->pri_hint;
+		req.l2_filter_id_hint =
+			rte_cpu_to_le_64(filter->l2_filter_id_hint);
+	}
 
 	req.enables = rte_cpu_to_le_32(enables);
 
@@ -1017,6 +1020,11 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
 	if (dev_caps_cfg &
 	    HWRM_VER_GET_OUTPUT_DEV_CAPS_CFG_TRUSTED_VF_SUPPORTED)
 		PMD_DRV_LOG(DEBUG, "FW supports Trusted VFs\n");
+	if (dev_caps_cfg &
+	    HWRM_VER_GET_OUTPUT_DEV_CAPS_CFG_CFA_ADV_FLOW_MGNT_SUPPORTED) {
+		bp->flags |= BNXT_FLAG_ADV_FLOW_MGMT;
+		PMD_DRV_LOG(DEBUG, "FW supports advanced flow management\n");
+	}
 
 error:
 	HWRM_UNLOCK();
@@ -4912,3 +4920,35 @@ int bnxt_hwrm_port_ts_query(struct bnxt *bp, uint8_t path, uint64_t *timestamp)
 
 	return rc;
 }
+
+int bnxt_hwrm_cfa_adv_flow_mgmt_qcaps(struct bnxt *bp)
+{
+	struct hwrm_cfa_adv_flow_mgnt_qcaps_output *resp =
+					bp->hwrm_cmd_resp_addr;
+	struct hwrm_cfa_adv_flow_mgnt_qcaps_input req = {0};
+	uint32_t flags = 0;
+	int rc = 0;
+
+	if (!(bp->flags & BNXT_FLAG_ADV_FLOW_MGMT))
+		return rc;
+
+	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp))) {
+		PMD_DRV_LOG(DEBUG,
+			    "Not a PF or trusted VF. Command not supported\n");
+		return 0;
+	}
+
+	HWRM_PREP(req, CFA_ADV_FLOW_MGNT_QCAPS, BNXT_USE_KONG(bp));
+	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_KONG(bp));
+
+	HWRM_CHECK_RESULT();
+	flags = rte_le_to_cpu_32(resp->flags);
+	HWRM_UNLOCK();
+
+	if (flags & HWRM_CFA_ADV_FLOW_MGNT_QCAPS_L2_HDR_SRC_FILTER_EN) {
+		bp->flow_flags |= BNXT_FLOW_FLAG_L2_HDR_SRC_FILTER_EN;
+		PMD_DRV_LOG(INFO, "Source L2 header filtering enabled\n");
+	}
+
+	return rc;
+}
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index 5190f43290..3a49c493db 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -38,6 +38,9 @@ struct bnxt_cp_ring_info;
 #define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MINIMAL_STATIC \
 	HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESERVATION_STRATEGY_MINIMAL_STATIC
 
+#define HWRM_CFA_ADV_FLOW_MGNT_QCAPS_L2_HDR_SRC_FILTER_EN \
+HWRM_CFA_ADV_FLOW_MGNT_QCAPS_OUTPUT_FLAGS_L2_HEADER_SOURCE_FIELDS_SUPPORTED
+
 #define HWRM_SPEC_CODE_1_8_4		0x10804
 #define HWRM_SPEC_CODE_1_9_0		0x10900
 #define HWRM_SPEC_CODE_1_9_2		0x10902
@@ -210,4 +213,5 @@ int bnxt_hwrm_error_recovery_qcfg(struct bnxt *bp);
 int bnxt_hwrm_fw_reset(struct bnxt *bp);
 int bnxt_hwrm_port_ts_query(struct bnxt *bp, uint8_t path,
 			    uint64_t *timestamp);
+int bnxt_hwrm_cfa_adv_flow_mgmt_qcaps(struct bnxt *bp);
 #endif
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index 667d0486e3..371534db6b 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -64,6 +64,7 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto err_out;
 		}
+		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
 		STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 		goto out;
 	}
@@ -145,6 +146,7 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto err_out;
 		}
+		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
 		/*
 		 * TODO: Configure & associate CFA rule for
 		 * each VNIC for each VMDq with MACVLAN, MACVLAN+TC
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 05/20] net/bnxt: add support for RSS action
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (3 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 04/20] net/bnxt: add support to create SMAC and inner DMAC filters Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 06/20] net/bnxt: parse priority attribute for flow creation Ajit Khaparde
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, Somnath Kotur, Kalesh AP,
	Santoshkumar Karanappa Rastapur, Rahul Gupta, Venkat Duvvuru

Add support for RSS action during flow creation.

group id should not be 0 when RSS action is specified. Driver will
return an error for such a flow.

If a group id is used to create a filter with “n” RSS queues, it cannot
be used to create a filter with a different number of RSS queues till
all the flows using that combination are deleted.

While creating a flow if a group id groups a certain Rx queue ids for
RSS, the same group id shall not create a flow with a different group of
Rx queue ids till all the flows belonging to the group ids are deleted.

While creating a flow if a group id groups a certain Rx queue ids for
RSS, the same queue ids shall not be used with a different group id till
all flows created with that group id are deleted.

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.h |   2 +
 drivers/net/bnxt/bnxt_flow.c   | 377 ++++++++++++++++++++++++++++-----
 drivers/net/bnxt/bnxt_hwrm.c   |  13 ++
 drivers/net/bnxt/bnxt_vnic.c   |   2 +-
 drivers/net/bnxt/bnxt_vnic.h   |   1 +
 5 files changed, 337 insertions(+), 58 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 52d3582ba0..8bd9d35329 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -19,6 +19,7 @@ struct bnxt;
 struct bnxt_filter_info {
 	STAILQ_ENTRY(bnxt_filter_info)	next;
 	uint64_t		fw_l2_filter_id;
+	struct bnxt_filter_info *matching_l2_fltr_ptr;
 	uint64_t		fw_em_filter_id;
 	uint64_t		fw_ntuple_filter_id;
 #define INVALID_MAC_INDEX	((uint16_t)-1)
@@ -33,6 +34,7 @@ struct bnxt_filter_info {
 	/* Filter Characteristics */
 	uint32_t		flags;
 	uint32_t		enables;
+	uint32_t		l2_ref_cnt;
 	uint8_t			l2_addr[RTE_ETHER_ADDR_LEN];
 	uint8_t			l2_addr_mask[RTE_ETHER_ADDR_LEN];
 	uint32_t		valid_flags;
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index d1a8c4459a..6703076921 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -157,9 +157,9 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 	int dflt_vnic;
 
 	use_ntuple = bnxt_filter_type_check(pattern, error);
-	PMD_DRV_LOG(DEBUG, "Use NTUPLE %d\n", use_ntuple);
 	if (use_ntuple < 0)
 		return use_ntuple;
+	PMD_DRV_LOG(DEBUG, "Use NTUPLE %d\n", use_ntuple);
 
 	filter->filter_type = use_ntuple ?
 		HWRM_CFA_NTUPLE_FILTER : HWRM_CFA_EM_FILTER;
@@ -724,13 +724,13 @@ bnxt_flow_parse_attr(const struct rte_flow_attr *attr,
 	return 0;
 }
 
-struct bnxt_filter_info *
-bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
-		   struct bnxt_vnic_info *vnic)
+static struct bnxt_filter_info *
+bnxt_find_matching_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 {
-	struct bnxt_filter_info *filter1, *f0;
+	struct bnxt_filter_info *mf, *f0;
 	struct bnxt_vnic_info *vnic0;
-	int rc;
+	struct rte_flow *flow;
+	int i;
 
 	vnic0 = &bp->vnic_info[0];
 	f0 = STAILQ_FIRST(&vnic0->filter);
@@ -739,8 +739,43 @@ bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 	if (memcmp(f0->l2_addr, nf->dst_macaddr, RTE_ETHER_ADDR_LEN) == 0)
 		return f0;
 
+	for (i = bp->max_vnics - 1; i >= 0; i--) {
+		struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
+
+		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
+			continue;
+
+		STAILQ_FOREACH(flow, &vnic->flow_list, next) {
+			mf = flow->filter;
+
+			if (mf->matching_l2_fltr_ptr)
+				continue;
+
+			if (mf->ethertype == nf->ethertype &&
+			    mf->l2_ovlan == nf->l2_ovlan &&
+			    mf->l2_ovlan_mask == nf->l2_ovlan_mask &&
+			    mf->l2_ivlan == nf->l2_ivlan &&
+			    mf->l2_ivlan_mask == nf->l2_ivlan_mask &&
+			    !memcmp(mf->src_macaddr, nf->src_macaddr,
+				    RTE_ETHER_ADDR_LEN) &&
+			    !memcmp(mf->dst_macaddr, nf->dst_macaddr,
+				    RTE_ETHER_ADDR_LEN))
+				return mf;
+		}
+	}
+	return NULL;
+}
+
+static struct bnxt_filter_info *
+bnxt_create_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
+		      struct bnxt_vnic_info *vnic)
+{
+	struct bnxt_filter_info *filter1;
+	int rc;
+
 	/* Alloc new L2 filter.
-	 * This flow needs MAC filter which does not match port/l2 MAC.
+	 * This flow needs MAC filter which does not match any existing
+	 * L2 filters.
 	 */
 	filter1 = bnxt_get_unused_filter(bp);
 	if (filter1 == NULL)
@@ -785,9 +820,28 @@ bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 		bnxt_free_filter(bp, filter1);
 		return NULL;
 	}
+	filter1->l2_ref_cnt++;
 	return filter1;
 }
 
+struct bnxt_filter_info *
+bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
+		   struct bnxt_vnic_info *vnic)
+{
+	struct bnxt_filter_info *l2_filter = NULL;
+
+	l2_filter = bnxt_find_matching_l2_filter(bp, nf);
+	if (l2_filter) {
+		l2_filter->l2_ref_cnt++;
+		nf->matching_l2_fltr_ptr = l2_filter;
+	} else {
+		l2_filter = bnxt_create_l2_filter(bp, nf, vnic);
+		nf->matching_l2_fltr_ptr = NULL;
+	}
+
+	return l2_filter;
+}
+
 static int bnxt_vnic_prep(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 {
 	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
@@ -832,6 +886,66 @@ static int bnxt_vnic_prep(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	return rc;
 }
 
+static int match_vnic_rss_cfg(struct bnxt *bp,
+			      struct bnxt_vnic_info *vnic,
+			      const struct rte_flow_action_rss *rss)
+{
+	unsigned int match = 0, i;
+
+	if (vnic->rx_queue_cnt != rss->queue_num)
+		return -EINVAL;
+
+	for (i = 0; i < rss->queue_num; i++) {
+		if (!bp->rx_queues[rss->queue[i]]->vnic->rx_queue_cnt &&
+		    !bp->rx_queues[rss->queue[i]]->rx_started)
+			return -EINVAL;
+	}
+
+	for (i = 0; i < vnic->rx_queue_cnt; i++) {
+		int j;
+
+		for (j = 0; j < vnic->rx_queue_cnt; j++) {
+			if (bp->grp_info[rss->queue[i]].fw_grp_id ==
+			    vnic->fw_grp_ids[j])
+				match++;
+		}
+	}
+
+	if (match != vnic->rx_queue_cnt) {
+		PMD_DRV_LOG(ERR,
+			    "VNIC queue count %d vs queues matched %d\n",
+			    match, vnic->rx_queue_cnt);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void
+bnxt_update_filter_flags_en(struct bnxt_filter_info *filter,
+			    struct bnxt_filter_info *filter1,
+			    int use_ntuple)
+{
+	if (!use_ntuple &&
+	    !(filter->valid_flags &
+	      ~(BNXT_FLOW_L2_DST_VALID_FLAG |
+		BNXT_FLOW_L2_SRC_VALID_FLAG |
+		BNXT_FLOW_L2_INNER_SRC_VALID_FLAG |
+		BNXT_FLOW_L2_INNER_DST_VALID_FLAG))) {
+		filter->flags = filter1->flags;
+		filter->enables = filter1->enables;
+		filter->filter_type = HWRM_CFA_L2_FILTER;
+		memcpy(filter->l2_addr, filter1->l2_addr, RTE_ETHER_ADDR_LEN);
+		memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
+		filter->pri_hint = filter1->pri_hint;
+		filter->l2_filter_id_hint = filter1->l2_filter_id_hint;
+	}
+	filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
+	filter->l2_ref_cnt = filter1->l2_ref_cnt;
+	PMD_DRV_LOG(DEBUG, "l2_filter: %p fw_l2_filter_id %lx l2_ref_cnt %d\n",
+		    filter1, filter->fw_l2_filter_id, filter->l2_ref_cnt);
+}
+
 static int
 bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			     const struct rte_flow_item pattern[],
@@ -846,12 +960,14 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	const struct rte_flow_action_queue *act_q;
 	const struct rte_flow_action_vf *act_vf;
+	struct bnxt_filter_info *filter1 = NULL;
+	const struct rte_flow_action_rss *rss;
 	struct bnxt_vnic_info *vnic, *vnic0;
-	struct bnxt_filter_info *filter1;
 	struct bnxt_rx_queue *rxq = NULL;
 	int dflt_vnic, vnic_id;
-	uint32_t vf = 0;
-	int rc;
+	unsigned int rss_idx;
+	uint32_t vf = 0, i;
+	int rc, use_ntuple;
 
 	rc =
 	bnxt_validate_and_parse_flow_type(bp, attr, pattern, error, filter);
@@ -866,6 +982,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 	if (filter->filter_type == HWRM_CFA_EM_FILTER)
 		filter->flags = HWRM_CFA_EM_FLOW_ALLOC_INPUT_FLAGS_PATH_RX;
 
+	use_ntuple = bnxt_filter_type_check(pattern, error);
 	switch (act->type) {
 	case RTE_FLOW_ACTION_TYPE_QUEUE:
 		/* Allow this flow. Redirect to a VNIC. */
@@ -886,7 +1003,6 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			PMD_DRV_LOG(DEBUG, "Group id is 0\n");
 			vnic_id = act_q->index;
 		}
-		PMD_DRV_LOG(DEBUG, "VNIC found\n");
 
 		vnic = &bp->vnic_info[vnic_id];
 		if (vnic == NULL) {
@@ -920,10 +1036,13 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		    vnic->fw_vnic_id != INVALID_HW_RING_ID)
 			goto use_vnic;
 
+		//if (!rxq ||
+		    //bp->vnic_info[0].fw_grp_ids[act_q->index] !=
+		    //INVALID_HW_RING_ID ||
+		    //!rxq->rx_deferred_start) {
 		if (!rxq ||
 		    bp->vnic_info[0].fw_grp_ids[act_q->index] !=
-		    INVALID_HW_RING_ID ||
-		    !rxq->rx_deferred_start) {
+		    INVALID_HW_RING_ID) {
 			PMD_DRV_LOG(ERR,
 				    "Queue invalid or used with other VNIC\n");
 			rte_flow_error_set(error,
@@ -935,8 +1054,8 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			goto ret;
 		}
 
-use_vnic:
 		rxq->vnic = vnic;
+		rxq->rx_started = 1;
 		vnic->rx_queue_cnt++;
 		vnic->start_grp_id = act_q->index;
 		vnic->end_grp_id = act_q->index;
@@ -952,6 +1071,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			    "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
 			    act_q->index, vnic, vnic->fw_grp_ids);
 
+use_vnic:
 		vnic->ff_pool_idx = vnic_id;
 		PMD_DRV_LOG(DEBUG,
 			    "Setting vnic ff_idx %d\n", vnic->ff_pool_idx);
@@ -962,20 +1082,9 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			goto ret;
 		}
 
-		if (!(filter->valid_flags &
-		      ~(BNXT_FLOW_L2_DST_VALID_FLAG |
-			BNXT_FLOW_L2_SRC_VALID_FLAG |
-			BNXT_FLOW_L2_INNER_SRC_VALID_FLAG |
-			BNXT_FLOW_L2_INNER_DST_VALID_FLAG))) {
-			PMD_DRV_LOG(DEBUG, "L2 filter created\n");
-			filter->flags = filter1->flags;
-			filter->enables = filter1->enables;
-			filter->filter_type = HWRM_CFA_L2_FILTER;
-			memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
-			filter->pri_hint = filter1->pri_hint;
-			filter->l2_filter_id_hint = filter1->l2_filter_id_hint;
-		}
-		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
+		PMD_DRV_LOG(DEBUG, "new fltr: %p l2fltr: %p l2_ref_cnt: %d\n",
+			    filter, filter1, filter1->l2_ref_cnt);
+		bnxt_update_filter_flags_en(filter, filter1, use_ntuple);
 		break;
 	case RTE_FLOW_ACTION_TYPE_DROP:
 		vnic0 = &bp->vnic_info[0];
@@ -1067,15 +1176,158 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
 		break;
 	case RTE_FLOW_ACTION_TYPE_RSS:
-		rte_flow_error_set(error,
-				   ENOTSUP,
-				   RTE_FLOW_ERROR_TYPE_ACTION,
-				   act,
-				   "This action is not supported right now.");
-		rc = -rte_errno;
-		goto ret;
-		//break;
+		rss = (const struct rte_flow_action_rss *)act->conf;
+
+		vnic_id = attr->group;
+		if (!vnic_id) {
+			PMD_DRV_LOG(ERR, "Group id cannot be 0\n");
+			rte_flow_error_set(error,
+					   EINVAL,
+					   RTE_FLOW_ERROR_TYPE_ATTR,
+					   NULL,
+					   "Group id cannot be 0");
+			rc = -rte_errno;
+			goto ret;
+		}
+
+		vnic = &bp->vnic_info[vnic_id];
+		if (vnic == NULL) {
+			rte_flow_error_set(error,
+					   EINVAL,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "No matching VNIC for RSS group.");
+			rc = -rte_errno;
+			goto ret;
+		}
+		PMD_DRV_LOG(DEBUG, "VNIC found\n");
+
+		/* Check if requested RSS config matches RSS config of VNIC
+		 * only if it is not a fresh VNIC configuration.
+		 * Otherwise the existing VNIC configuration can be used.
+		 */
+		if (vnic->rx_queue_cnt) {
+			rc = match_vnic_rss_cfg(bp, vnic, rss);
+			if (rc) {
+				PMD_DRV_LOG(ERR,
+					    "VNIC and RSS config mismatch\n");
+				rte_flow_error_set(error,
+						   EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ACTION,
+						   act,
+						   "VNIC and RSS cfg mismatch");
+				rc = -rte_errno;
+				goto ret;
+			}
+			goto vnic_found;
+		}
 
+		for (i = 0; i < rss->queue_num; i++) {
+			PMD_DRV_LOG(DEBUG, "RSS action Queue %d\n",
+				    rss->queue[i]);
+
+			if (!rss->queue[i] ||
+			    rss->queue[i] >= bp->rx_nr_rings ||
+			    !bp->rx_queues[rss->queue[i]]) {
+				rte_flow_error_set(error,
+						   EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ACTION,
+						   act,
+						   "Invalid queue ID for RSS");
+				rc = -rte_errno;
+				goto ret;
+			}
+			rxq = bp->rx_queues[rss->queue[i]];
+
+			//if (bp->vnic_info[0].fw_grp_ids[rss->queue[i]] !=
+			    //INVALID_HW_RING_ID ||
+			    //!rxq->rx_deferred_start) {
+			if (bp->vnic_info[0].fw_grp_ids[rss->queue[i]] !=
+			    INVALID_HW_RING_ID) {
+				PMD_DRV_LOG(ERR,
+					    "queue active with other VNIC\n");
+				rte_flow_error_set(error,
+						   EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ACTION,
+						   act,
+						   "Invalid queue ID for RSS");
+				rc = -rte_errno;
+				goto ret;
+			}
+
+			rxq->vnic = vnic;
+			rxq->rx_started = 1;
+			vnic->rx_queue_cnt++;
+		}
+
+		vnic->start_grp_id = rss->queue[0];
+		vnic->end_grp_id = rss->queue[rss->queue_num - 1];
+		vnic->func_default = 0;	//This is not a default VNIC.
+
+		rc = bnxt_vnic_prep(bp, vnic);
+		if (rc)
+			goto ret;
+
+		PMD_DRV_LOG(DEBUG,
+			    "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
+			    vnic_id, vnic, vnic->fw_grp_ids);
+
+		vnic->ff_pool_idx = vnic_id;
+		PMD_DRV_LOG(DEBUG,
+			    "Setting vnic ff_pool_idx %d\n", vnic->ff_pool_idx);
+
+		/* This can be done only after vnic_grp_alloc is done. */
+		for (i = 0; i < vnic->rx_queue_cnt; i++) {
+			vnic->fw_grp_ids[i] =
+				bp->grp_info[rss->queue[i]].fw_grp_id;
+			/* Make sure vnic0 does not use these rings. */
+			bp->vnic_info[0].fw_grp_ids[rss->queue[i]] =
+				INVALID_HW_RING_ID;
+		}
+
+		for (rss_idx = 0; rss_idx < HW_HASH_INDEX_SIZE; ) {
+			for (i = 0; i < vnic->rx_queue_cnt; i++)
+				vnic->rss_table[rss_idx++] =
+					vnic->fw_grp_ids[i];
+		}
+
+		/* Configure RSS only if the queue count is > 1 */
+		if (vnic->rx_queue_cnt > 1) {
+			vnic->hash_type =
+				bnxt_rte_to_hwrm_hash_types(rss->types);
+
+			if (!rss->key_len) {
+				/* If hash key has not been specified,
+				 * use random hash key.
+				 */
+				prandom_bytes(vnic->rss_hash_key,
+					      HW_HASH_KEY_SIZE);
+			} else {
+				if (rss->key_len > HW_HASH_KEY_SIZE)
+					memcpy(vnic->rss_hash_key,
+					       rss->key,
+					       HW_HASH_KEY_SIZE);
+				else
+					memcpy(vnic->rss_hash_key,
+					       rss->key,
+					       rss->key_len);
+			}
+			bnxt_hwrm_vnic_rss_cfg(bp, vnic);
+		} else {
+			PMD_DRV_LOG(DEBUG, "No RSS config required\n");
+		}
+
+vnic_found:
+		filter->dst_id = vnic->fw_vnic_id;
+		filter1 = bnxt_get_l2_filter(bp, filter, vnic);
+		if (filter1 == NULL) {
+			rc = -ENOSPC;
+			goto ret;
+		}
+
+		PMD_DRV_LOG(DEBUG, "L2 filter created\n");
+		bnxt_update_filter_flags_en(filter, filter1, use_ntuple);
+		break;
 	default:
 		rte_flow_error_set(error,
 				   EINVAL,
@@ -1086,10 +1338,11 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		goto ret;
 	}
 
-	if (filter1) {
+	if (filter1 && !filter->matching_l2_fltr_ptr) {
 		bnxt_free_filter(bp, filter1);
 		filter1->fw_l2_filter_id = -1;
 	}
+
 done:
 	act = bnxt_flow_non_void_action(++act);
 	if (act->type != RTE_FLOW_ACTION_TYPE_END) {
@@ -1135,6 +1388,28 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 	return ret;
 }
 
+static void
+bnxt_update_filter(struct bnxt *bp, struct bnxt_filter_info *old_filter,
+		   struct bnxt_filter_info *new_filter)
+{
+	/* Clear the new L2 filter that was created in the previous step in
+	 * bnxt_validate_and_parse_flow. For L2 filters, we will use the new
+	 * filter which points to the new destination queue and so we clear
+	 * the previous L2 filter. For ntuple filters, we are going to reuse
+	 * the old L2 filter and create new NTUPLE filter with this new
+	 * destination queue subsequently during bnxt_flow_create.
+	 */
+	if (new_filter->filter_type == HWRM_CFA_L2_FILTER) {
+		bnxt_hwrm_clear_l2_filter(bp, old_filter);
+		bnxt_hwrm_set_l2_filter(bp, new_filter->dst_id, new_filter);
+	} else {
+		if (new_filter->filter_type == HWRM_CFA_EM_FILTER)
+			bnxt_hwrm_clear_em_filter(bp, old_filter);
+		if (new_filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
+			bnxt_hwrm_clear_ntuple_filter(bp, old_filter);
+	}
+}
+
 static int
 bnxt_match_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 {
@@ -1184,24 +1459,13 @@ bnxt_match_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 				    sizeof(nf->dst_ipaddr_mask))) {
 				if (mf->dst_id == nf->dst_id)
 					return -EEXIST;
-				/* Clear the new L2 filter that was created
-				 * earlier in bnxt_validate_and_parse_flow.
-				 */
-				bnxt_hwrm_clear_l2_filter(bp, nf);
-				/*
-				 * Same Flow, Different queue
-				 * Clear the old ntuple filter
-				 * Reuse the matching L2 filter
-				 * ID for the new filter
-				 */
-				nf->fw_l2_filter_id = mf->fw_l2_filter_id;
-				if (nf->filter_type == HWRM_CFA_EM_FILTER)
-					bnxt_hwrm_clear_em_filter(bp, mf);
-				if (nf->filter_type == HWRM_CFA_NTUPLE_FILTER)
-					bnxt_hwrm_clear_ntuple_filter(bp, mf);
 				/* Free the old filter, update flow
 				 * with new filter
 				 */
+				bnxt_update_filter(bp, mf, nf);
+				STAILQ_REMOVE(&vnic->filter, mf,
+					      bnxt_filter_info, next);
+				STAILQ_INSERT_TAIL(&vnic->filter, nf, next);
 				bnxt_free_filter(bp, mf);
 				flow->filter = nf;
 				return -EXDEV;
@@ -1334,7 +1598,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		}
 	}
 done:
-	if (!ret) {
+	if (!ret || update_flow) {
 		flow->filter = filter;
 		flow->vnic = vnic;
 		/* VNIC is set only in case of queue or RSS action */
@@ -1372,8 +1636,8 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Matching Flow exists.");
 	else if (ret == -EXDEV)
-		rte_flow_error_set(error, ret,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+		rte_flow_error_set(error, 0,
+				   RTE_FLOW_ERROR_TYPE_NONE, NULL,
 				   "Flow with pattern exists, updating destination queue");
 	else if (!rte_errno)
 		rte_flow_error_set(error, -ret,
@@ -1458,8 +1722,7 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 		ret = bnxt_hwrm_clear_em_filter(bp, filter);
 	if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 		ret = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-	else
-		ret = bnxt_hwrm_clear_l2_filter(bp, filter);
+	ret = bnxt_hwrm_clear_l2_filter(bp, filter);
 
 done:
 	if (!ret) {
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index ceaeb609a4..7c1aaa6102 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -373,12 +373,25 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
 			   struct bnxt_filter_info *filter)
 {
 	int rc = 0;
+	struct bnxt_filter_info *l2_filter = filter;
 	struct hwrm_cfa_l2_filter_free_input req = {.req_type = 0 };
 	struct hwrm_cfa_l2_filter_free_output *resp = bp->hwrm_cmd_resp_addr;
 
 	if (filter->fw_l2_filter_id == UINT64_MAX)
 		return 0;
 
+	if (filter->matching_l2_fltr_ptr)
+		l2_filter = filter->matching_l2_fltr_ptr;
+
+	PMD_DRV_LOG(DEBUG, "filter: %p l2_filter: %p ref_cnt: %d\n",
+		    filter, l2_filter, l2_filter->l2_ref_cnt);
+
+	if (l2_filter->l2_ref_cnt > 0)
+		l2_filter->l2_ref_cnt--;
+
+	if (l2_filter->l2_ref_cnt > 0)
+		return 0;
+
 	HWRM_PREP(req, CFA_L2_FILTER_FREE, BNXT_USE_CHIMP_MB);
 
 	req.l2_filter_id = rte_cpu_to_le_64(filter->fw_l2_filter_id);
diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index 4f3f9b3594..2f0ed10265 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -16,7 +16,7 @@
  * VNIC Functions
  */
 
-static void prandom_bytes(void *dest_ptr, size_t len)
+void prandom_bytes(void *dest_ptr, size_t len)
 {
 	char *dest = (char *)dest_ptr;
 	uint64_t rb;
diff --git a/drivers/net/bnxt/bnxt_vnic.h b/drivers/net/bnxt/bnxt_vnic.h
index ec3a3ff51f..de34b21eb8 100644
--- a/drivers/net/bnxt/bnxt_vnic.h
+++ b/drivers/net/bnxt/bnxt_vnic.h
@@ -67,5 +67,6 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp);
 void bnxt_free_vnic_mem(struct bnxt *bp);
 int bnxt_alloc_vnic_mem(struct bnxt *bp);
 int bnxt_vnic_grp_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic);
+void prandom_bytes(void *dest_ptr, size_t len);
 uint16_t bnxt_rte_to_hwrm_hash_types(uint64_t rte_type);
 #endif
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 06/20] net/bnxt: parse priority attribute for flow creation
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (4 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 05/20] net/bnxt: add support for RSS action Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 07/20] net/bnxt: delete and flush L2 filters cleanly Ajit Khaparde
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Lance Richardson

Parse priority attribute during flow creation.
This information will be used to give a hint to the FW to
place the flow rule accordingly in the CFA tables.

Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.h |  1 +
 drivers/net/bnxt/bnxt_flow.c   | 25 +++++++++++--------------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 8bd9d35329..6bcdc258b7 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -68,6 +68,7 @@ struct bnxt_filter_info {
 	uint16_t                ip_protocol;
 	uint16_t                ip_addr_type;
 	uint16_t                ethertype;
+	uint32_t		priority;
 };
 
 struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 6703076921..0887bd7f2f 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -227,6 +227,9 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 				valid_flags |= inner ?
 					BNXT_FLOW_L2_INNER_DST_VALID_FLAG :
 					BNXT_FLOW_L2_DST_VALID_FLAG;
+				filter->priority = attr->priority;
+				PMD_DRV_LOG(DEBUG,
+					    "Creating a priority flow\n");
 			}
 
 			if (rte_is_broadcast_ether_addr(&eth_mask->src)) {
@@ -712,15 +715,6 @@ bnxt_flow_parse_attr(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* Not supported */
-	if (attr->priority) {
-		rte_flow_error_set(error,
-				   EINVAL,
-				   RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
-				   attr,
-				   "No support for priority.");
-		return -rte_errno;
-	}
 	return 0;
 }
 
@@ -802,13 +796,16 @@ bnxt_create_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 		memcpy(filter1->l2_addr, nf->dst_macaddr, RTE_ETHER_ADDR_LEN);
 	}
 
-	if (nf->valid_flags & BNXT_FLOW_L2_DST_VALID_FLAG ||
-	    nf->valid_flags & BNXT_FLOW_L2_INNER_DST_VALID_FLAG) {
+	if (nf->priority &&
+	    (nf->valid_flags & BNXT_FLOW_L2_DST_VALID_FLAG ||
+	     nf->valid_flags & BNXT_FLOW_L2_INNER_DST_VALID_FLAG)) {
 		/* Tell the FW where to place the filter in the table. */
-		filter1->pri_hint =
+		if (nf->priority > 65535) {
+			filter1->pri_hint =
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_PRI_HINT_BELOW_FILTER;
-		/* This will place the filter in TCAM */
-		filter1->l2_filter_id_hint = (uint64_t)-1;
+			/* This will place the filter in TCAM */
+			filter1->l2_filter_id_hint = (uint64_t)-1;
+		}
 	}
 
 	filter1->enables = HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 07/20] net/bnxt: delete and flush L2 filters cleanly
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (5 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 06/20] net/bnxt: parse priority attribute for flow creation Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 08/20] net/bnxt: cleanup vnic after flow validate Ajit Khaparde
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Venkat Duvvuru, Kalesh AP

Once the last filter associated with a VNIC is deleted when using
RSS action or the Queue action free the VNIC. Also free the RSS
context if the VNIC is using it.

Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.c | 18 +++++-------
 drivers/net/bnxt/bnxt_flow.c   | 41 ++++++++++++++++++++++++++--
 drivers/net/bnxt/bnxt_hwrm.c   | 50 ++++++++++++++++++++--------------
 3 files changed, 75 insertions(+), 34 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 34db988181..e95d47d296 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -117,6 +117,13 @@ void bnxt_free_filter_mem(struct bnxt *bp)
 	max_filters = bp->max_l2_ctx;
 	for (i = 0; i < max_filters; i++) {
 		filter = &bp->filter_info[i];
+		if (filter->fw_ntuple_filter_id != ((uint64_t)-1) &&
+		    filter->filter_type == HWRM_CFA_NTUPLE_FILTER) {
+			/* Call HWRM to try to free filter again */
+			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
+		}
+		filter->fw_ntuple_filter_id = UINT64_MAX;
+
 		if (filter->fw_l2_filter_id != ((uint64_t)-1) &&
 		    filter->filter_type == HWRM_CFA_L2_FILTER) {
 			PMD_DRV_LOG(DEBUG, "L2 filter is not free\n");
@@ -129,17 +136,6 @@ void bnxt_free_filter_mem(struct bnxt *bp)
 		}
 		filter->fw_l2_filter_id = UINT64_MAX;
 
-		if (filter->fw_ntuple_filter_id != ((uint64_t)-1) &&
-		    filter->filter_type == HWRM_CFA_NTUPLE_FILTER) {
-			PMD_DRV_LOG(ERR, "NTUPLE filter is not free\n");
-			/* Call HWRM to try to free filter again */
-			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-			if (rc)
-				PMD_DRV_LOG(ERR,
-					    "Cannot free NTUPLE filter: %d\n",
-					    rc);
-		}
-		filter->fw_ntuple_filter_id = UINT64_MAX;
 	}
 	STAILQ_INIT(&bp->free_filter_list);
 
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 0887bd7f2f..d092a76e99 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1636,7 +1636,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		rte_flow_error_set(error, 0,
 				   RTE_FLOW_ERROR_TYPE_NONE, NULL,
 				   "Flow with pattern exists, updating destination queue");
-	else if (!rte_errno)
+	else
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to create flow.");
@@ -1715,6 +1715,7 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 	ret = bnxt_match_filter(bp, filter);
 	if (ret == 0)
 		PMD_DRV_LOG(ERR, "Could not find matching flow\n");
+
 	if (filter->filter_type == HWRM_CFA_EM_FILTER)
 		ret = bnxt_hwrm_clear_em_filter(bp, filter);
 	if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
@@ -1723,9 +1724,40 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 
 done:
 	if (!ret) {
+		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
 		bnxt_free_filter(bp, filter);
 		STAILQ_REMOVE(&vnic->flow_list, flow, rte_flow, next);
 		rte_free(flow);
+
+		/* If this was the last flow associated with this vnic,
+		 * switch the queue back to RSS pool.
+		 */
+		if (vnic && STAILQ_EMPTY(&vnic->flow_list)) {
+			rte_free(vnic->fw_grp_ids);
+			if (vnic->rx_queue_cnt > 1) {
+				if (BNXT_CHIP_THOR(bp)) {
+					int j;
+
+					for (j = 0; j < vnic->num_lb_ctxts;
+					     j++) {
+						bnxt_hwrm_vnic_ctx_free(bp,
+                                                                        vnic,
+                                                                        vnic->fw_grp_ids[j]);
+						vnic->fw_grp_ids[j] =
+							INVALID_HW_RING_ID;
+					}
+					vnic->num_lb_ctxts = 0;
+				} else {
+					bnxt_hwrm_vnic_ctx_free(bp,
+								vnic,
+								vnic->rss_rule);
+					vnic->rss_rule = INVALID_HW_RING_ID;
+				}
+			}
+			bnxt_hwrm_vnic_free(bp, vnic);
+			vnic->rx_queue_cnt = 0;
+			bp->nr_vnics--;
+		}
 	} else {
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
@@ -1744,8 +1776,11 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 	unsigned int i;
 	int ret = 0;
 
-	for (i = 0; i < bp->nr_vnics; i++) {
+	for (i = 0; i < bp->max_vnics; i++) {
 		vnic = &bp->vnic_info[i];
+		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
+			continue;
+
 		STAILQ_FOREACH(flow, &vnic->flow_list, next) {
 			struct bnxt_filter_info *filter = flow->filter;
 
@@ -1766,6 +1801,8 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 				ret = bnxt_hwrm_clear_em_filter(bp, filter);
 			if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 				ret = bnxt_hwrm_clear_ntuple_filter(bp, filter);
+			else if (!i)
+				ret = bnxt_hwrm_clear_l2_filter(bp, filter);
 
 			if (ret) {
 				rte_flow_error_set
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 7c1aaa6102..5cb394994e 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2362,7 +2362,8 @@ bnxt_clear_hwrm_vnic_flows(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	struct rte_flow *flow;
 	int rc = 0;
 
-	STAILQ_FOREACH(flow, &vnic->flow_list, next) {
+	while (!STAILQ_EMPTY(&vnic->flow_list)) {
+		flow = STAILQ_FIRST(&vnic->flow_list);
 		filter = flow->filter;
 		PMD_DRV_LOG(DEBUG, "filter type %d\n", filter->filter_type);
 		if (filter->filter_type == HWRM_CFA_EM_FILTER)
@@ -2424,13 +2425,12 @@ void bnxt_free_all_hwrm_resources(struct bnxt *bp)
 	 * Cleanup VNICs in reverse order, to make sure the L2 filter
 	 * from vnic0 is last to be cleaned up.
 	 */
-	for (i = bp->nr_vnics - 1; i >= 0; i--) {
+	for (i = bp->max_vnics - 1; i >= 0; i--) {
 		struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
 
-		if (vnic->fw_vnic_id == INVALID_HW_RING_ID) {
-			PMD_DRV_LOG(DEBUG, "Invalid vNIC ID\n");
-			return;
-		}
+		// If the VNIC ID is invalid we are not currently using the VNIC
+		if (vnic->fw_vnic_id == INVALID_HW_RING_ID)
+			continue;
 
 		bnxt_clear_hwrm_vnic_flows(bp, vnic);
 
@@ -4307,23 +4307,31 @@ int bnxt_vnic_rss_configure(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	if (BNXT_CHIP_THOR(bp))
 		return bnxt_vnic_rss_configure_thor(bp, vnic);
 
-	/*
-	 * 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 (vnic->fw_vnic_id == INVALID_HW_RING_ID)
+		return 0;
+
+	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];
 		}
-		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 bnxt_hwrm_vnic_rss_cfg(bp, vnic);
+
+	return 0;
 }
 
 static void bnxt_hwrm_set_coal_params(struct bnxt_coal *hw_coal,
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 08/20] net/bnxt: cleanup vnic after flow validate
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (6 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 07/20] net/bnxt: delete and flush L2 filters cleanly Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 09/20] net/bnxt: fix an issue in setting default MAC address Ajit Khaparde
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Venkat Duvvuru

When an application issues flow validate, we free the temporary
filter that is created. But the vnic is not freed up. This can
potentially interfere with subsequent flow creation. So free the vnic.

Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 76 +++++++++++++++++++++---------------
 drivers/net/bnxt/bnxt_hwrm.c | 41 ++++++++++++-------
 drivers/net/bnxt/bnxt_hwrm.h |  3 +-
 3 files changed, 73 insertions(+), 47 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index d092a76e99..aedbd6d13b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1355,6 +1355,25 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 	return rc;
 }
 
+static
+struct bnxt_vnic_info *find_matching_vnic(struct bnxt *bp,
+					  struct bnxt_filter_info *filter)
+{
+	struct bnxt_vnic_info *vnic = NULL;
+	unsigned int i;
+
+	for (i = 0; i < bp->max_vnics; i++) {
+		vnic = &bp->vnic_info[i];
+		if (vnic->fw_vnic_id != INVALID_VNIC_ID &&
+		    filter->dst_id == vnic->fw_vnic_id) {
+			PMD_DRV_LOG(DEBUG, "Found matching VNIC Id %d\n",
+				    vnic->ff_pool_idx);
+			return vnic;
+		}
+	}
+	return NULL;
+}
+
 static int
 bnxt_flow_validate(struct rte_eth_dev *dev,
 		   const struct rte_flow_attr *attr,
@@ -1363,6 +1382,7 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 		   struct rte_flow_error *error)
 {
 	struct bnxt *bp = dev->data->dev_private;
+	struct bnxt_vnic_info *vnic = NULL;
 	struct bnxt_filter_info *filter;
 	int ret = 0;
 
@@ -1378,6 +1398,26 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 
 	ret = bnxt_validate_and_parse_flow(dev, pattern, actions, attr,
 					   error, filter);
+
+	vnic = find_matching_vnic(bp, filter);
+	if (vnic) {
+		if (STAILQ_EMPTY(&vnic->filter)) {
+			rte_free(vnic->fw_grp_ids);
+			bnxt_hwrm_vnic_ctx_free(bp, vnic);
+			bnxt_hwrm_vnic_free(bp, vnic);
+			vnic->rx_queue_cnt = 0;
+			bp->nr_vnics--;
+			PMD_DRV_LOG(DEBUG, "Free VNIC\n");
+		}
+	}
+
+	if (filter->filter_type == HWRM_CFA_EM_FILTER)
+		bnxt_hwrm_clear_em_filter(bp, filter);
+	else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
+		bnxt_hwrm_clear_ntuple_filter(bp, filter);
+	else
+		bnxt_hwrm_clear_l2_filter(bp, filter);
+
 	/* No need to hold on to this filter if we are just validating flow */
 	filter->fw_l2_filter_id = UINT64_MAX;
 	bnxt_free_filter(bp, filter);
@@ -1414,7 +1454,7 @@ bnxt_match_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 	struct rte_flow *flow;
 	int i;
 
-	for (i = bp->max_vnics; i >= 0; i--) {
+	for (i = bp->max_vnics - 1; i >= 0; i--) {
 		struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
 
 		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
@@ -1484,7 +1524,6 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 	struct bnxt_filter_info *filter;
 	bool update_flow = false;
 	struct rte_flow *flow;
-	unsigned int i;
 	int ret = 0;
 	uint32_t tun_type;
 
@@ -1585,15 +1624,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		ret = bnxt_hwrm_set_ntuple_filter(bp, filter->dst_id, filter);
 	}
 
-	for (i = 0; i < bp->max_vnics; i++) {
-		vnic = &bp->vnic_info[i];
-		if (vnic->fw_vnic_id != INVALID_VNIC_ID &&
-		    filter->dst_id == vnic->fw_vnic_id) {
-			PMD_DRV_LOG(ERR, "Found matching VNIC Id %d\n",
-				    vnic->ff_pool_idx);
-			break;
-		}
-	}
+	vnic = find_matching_vnic(bp, filter);
 done:
 	if (!ret || update_flow) {
 		flow->filter = filter;
@@ -1734,26 +1765,9 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 		 */
 		if (vnic && STAILQ_EMPTY(&vnic->flow_list)) {
 			rte_free(vnic->fw_grp_ids);
-			if (vnic->rx_queue_cnt > 1) {
-				if (BNXT_CHIP_THOR(bp)) {
-					int j;
-
-					for (j = 0; j < vnic->num_lb_ctxts;
-					     j++) {
-						bnxt_hwrm_vnic_ctx_free(bp,
-                                                                        vnic,
-                                                                        vnic->fw_grp_ids[j]);
-						vnic->fw_grp_ids[j] =
-							INVALID_HW_RING_ID;
-					}
-					vnic->num_lb_ctxts = 0;
-				} else {
-					bnxt_hwrm_vnic_ctx_free(bp,
-								vnic,
-								vnic->rss_rule);
-					vnic->rss_rule = INVALID_HW_RING_ID;
-				}
-			}
+			if (vnic->rx_queue_cnt > 1)
+				bnxt_hwrm_vnic_ctx_free(bp, vnic);
+
 			bnxt_hwrm_vnic_free(bp, vnic);
 			vnic->rx_queue_cnt = 0;
 			bp->nr_vnics--;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 5cb394994e..89697b83ac 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1776,8 +1776,9 @@ int bnxt_hwrm_vnic_ctx_alloc(struct bnxt *bp,
 	return rc;
 }
 
-int bnxt_hwrm_vnic_ctx_free(struct bnxt *bp,
-			    struct bnxt_vnic_info *vnic, uint16_t ctx_idx)
+static
+int _bnxt_hwrm_vnic_ctx_free(struct bnxt *bp,
+			     struct bnxt_vnic_info *vnic, uint16_t ctx_idx)
 {
 	int rc = 0;
 	struct hwrm_vnic_rss_cos_lb_ctx_free_input req = {.req_type = 0 };
@@ -1800,6 +1801,28 @@ int bnxt_hwrm_vnic_ctx_free(struct bnxt *bp,
 	return rc;
 }
 
+int bnxt_hwrm_vnic_ctx_free(struct bnxt *bp, struct bnxt_vnic_info *vnic)
+{
+	int rc = 0;
+
+	if (BNXT_CHIP_THOR(bp)) {
+		int j;
+
+		for (j = 0; j < vnic->num_lb_ctxts; j++) {
+			rc = _bnxt_hwrm_vnic_ctx_free(bp,
+						      vnic,
+						      vnic->fw_grp_ids[j]);
+			vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
+		}
+		vnic->num_lb_ctxts = 0;
+	} else {
+		rc = _bnxt_hwrm_vnic_ctx_free(bp, vnic, vnic->rss_rule);
+		vnic->rss_rule = INVALID_HW_RING_ID;
+	}
+
+	return rc;
+}
+
 int bnxt_hwrm_vnic_free(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 {
 	int rc = 0;
@@ -2416,7 +2439,7 @@ void bnxt_free_tunnel_ports(struct bnxt *bp)
 
 void bnxt_free_all_hwrm_resources(struct bnxt *bp)
 {
-	int i, j;
+	int i;
 
 	if (bp->vnic_info == NULL)
 		return;
@@ -2436,17 +2459,7 @@ void bnxt_free_all_hwrm_resources(struct bnxt *bp)
 
 		bnxt_clear_hwrm_vnic_filters(bp, vnic);
 
-		if (BNXT_CHIP_THOR(bp)) {
-			for (j = 0; j < vnic->num_lb_ctxts; j++) {
-				bnxt_hwrm_vnic_ctx_free(bp, vnic,
-							vnic->fw_grp_ids[j]);
-				vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
-			}
-			vnic->num_lb_ctxts = 0;
-		} else {
-			bnxt_hwrm_vnic_ctx_free(bp, vnic, vnic->rss_rule);
-			vnic->rss_rule = INVALID_HW_RING_ID;
-		}
+		bnxt_hwrm_vnic_ctx_free(bp, vnic);
 
 		bnxt_hwrm_vnic_tpa_cfg(bp, vnic, false);
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index 3a49c493db..07181d4020 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -112,8 +112,7 @@ int bnxt_hwrm_vnic_qcfg(struct bnxt *bp, struct bnxt_vnic_info *vnic,
 				int16_t fw_vf_id);
 int bnxt_hwrm_vnic_ctx_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic,
 			     uint16_t ctx_idx);
-int bnxt_hwrm_vnic_ctx_free(struct bnxt *bp, struct bnxt_vnic_info *vnic,
-			    uint16_t ctx_idx);
+int bnxt_hwrm_vnic_ctx_free(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 int bnxt_hwrm_vnic_free(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 int bnxt_hwrm_vnic_rss_cfg(struct bnxt *bp,
 			   struct bnxt_vnic_info *vnic);
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 09/20] net/bnxt: fix an issue in setting default MAC address
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (7 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 08/20] net/bnxt: cleanup vnic after flow validate Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 10/20] net/bnxt: allow only unicast MAC address filter creation Ajit Khaparde
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP, Somnath Kotur

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

Driver was incorrectly programming the MAC with the already
configured one instead of the newly requested MAC by user.

Also, fix to restore the old mac address back to the default
vnic filter if the mac update operation fails.

Fixes: 68f589f2c728e ("net/bnxt: fix setting primary MAC address")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7cd3213558..2c9eaaa2ef 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1904,7 +1904,7 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev,
 		if (filter->mac_index != 0)
 			continue;
 
-		memcpy(filter->l2_addr, bp->mac_addr, RTE_ETHER_ADDR_LEN);
+		memcpy(filter->l2_addr, addr, RTE_ETHER_ADDR_LEN);
 		memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
 		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX |
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
@@ -1913,8 +1913,11 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev,
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK;
 
 		rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
-		if (rc)
+		if (rc) {
+			memcpy(filter->l2_addr, bp->mac_addr,
+			       RTE_ETHER_ADDR_LEN);
 			return rc;
+		}
 
 		memcpy(bp->mac_addr, addr, RTE_ETHER_ADDR_LEN);
 		PMD_DRV_LOG(DEBUG, "Set MAC addr\n");
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 10/20] net/bnxt: allow only unicast MAC address filter creation
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (8 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 09/20] net/bnxt: fix an issue in setting default MAC address Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 11/20] net/bnxt: properly handle ring cleanup in case of error Ajit Khaparde
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Santoshkumar Karanappa Rastapur

Check if the application is trying to create filters using
broadcast and multicast MAC address and reject it.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index aedbd6d13b..cdfbb6c22f 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -219,6 +219,14 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 			}
 
 			if (rte_is_broadcast_ether_addr(&eth_mask->dst)) {
+				if (!rte_is_unicast_ether_addr(&eth_spec->dst)) {
+					rte_flow_error_set(error,
+							   EINVAL,
+							   RTE_FLOW_ERROR_TYPE_ITEM,
+							   item,
+							   "DMAC is invalid");
+					return -rte_errno;
+				}
 				rte_memcpy(filter->dst_macaddr,
 					   &eth_spec->dst, RTE_ETHER_ADDR_LEN);
 				en |= use_ntuple ?
@@ -233,6 +241,14 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 			}
 
 			if (rte_is_broadcast_ether_addr(&eth_mask->src)) {
+				if (!rte_is_unicast_ether_addr(&eth_spec->src)) {
+					rte_flow_error_set(error,
+							   EINVAL,
+							   RTE_FLOW_ERROR_TYPE_ITEM,
+							   item,
+							   "SMAC is invalid");
+					return -rte_errno;
+				}
 				rte_memcpy(filter->src_macaddr,
 					   &eth_spec->src, RTE_ETHER_ADDR_LEN);
 				en |= use_ntuple ?
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 11/20] net/bnxt: properly handle ring cleanup in case of error
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (9 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 10/20] net/bnxt: allow only unicast MAC address filter creation Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 12/20] net/bnxt: check device is started before flow creation Ajit Khaparde
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Venkat Duvvuru, Kalesh Anakkur Purayil

From: Rahul Gupta <rahul.gupta@broadcom.com>

Initialize all rings to INVALID_HW_RING_ID.
This can be used to determine the rings to free if allocation fails.

Signed-off-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ring.c | 37 ++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 68a690de5a..a3e5a68714 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -583,6 +583,42 @@ int bnxt_alloc_hwrm_rx_ring(struct bnxt *bp, int queue_index)
 	return rc;
 }
 
+/* Initialise all rings to -1, its used to free rings later if allocation
+ * of few rings fails.
+ */
+static void bnxt_init_all_rings(struct bnxt *bp)
+{
+	unsigned int i = 0;
+	struct bnxt_rx_queue *rxq;
+	struct bnxt_ring *cp_ring;
+	struct bnxt_ring *ring;
+	struct bnxt_rx_ring_info *rxr;
+	struct bnxt_tx_queue *txq;
+
+	for (i = 0; i < bp->rx_cp_nr_rings; i++) {
+		rxq = bp->rx_queues[i];
+		/* Rx-compl */
+		cp_ring = rxq->cp_ring->cp_ring_struct;
+		cp_ring->fw_ring_id = INVALID_HW_RING_ID;
+		/* Rx-Reg */
+		rxr = rxq->rx_ring;
+		ring = rxr->rx_ring_struct;
+		ring->fw_ring_id = INVALID_HW_RING_ID;
+		/* Rx-AGG */
+		ring = rxr->ag_ring_struct;
+		ring->fw_ring_id = INVALID_HW_RING_ID;
+	}
+	for (i = 0; i < bp->tx_cp_nr_rings; i++) {
+		txq = bp->tx_queues[i];
+		/* Tx cmpl */
+		cp_ring = txq->cp_ring->cp_ring_struct;
+		cp_ring->fw_ring_id = INVALID_HW_RING_ID;
+		/*Tx Ring */
+		ring = txq->tx_ring->tx_ring_struct;
+		ring->fw_ring_id = INVALID_HW_RING_ID;
+	}
+}
+
 /* ring_grp usage:
  * [0] = default completion ring
  * [1 -> +rx_cp_nr_rings] = rx_cp, rx rings
@@ -596,6 +632,7 @@ int bnxt_alloc_hwrm_rings(struct bnxt *bp)
 	int rc = 0;
 
 	bnxt_init_dflt_coal(&coal);
+	bnxt_init_all_rings(bp);
 
 	for (i = 0; i < bp->rx_cp_nr_rings; i++) {
 		struct bnxt_rx_queue *rxq = bp->rx_queues[i];
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 12/20] net/bnxt: check device is started before flow creation
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (10 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 11/20] net/bnxt: properly handle ring cleanup in case of error Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 13/20] net/bnxt: check for invalid VNIC ID in vnic tpa cfg Ajit Khaparde
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, Venkat Duvvuru

Check device is started before flow creation.
Since the vnic data structures aren't created until device start,
the driver dereferences NULL vnic if flow creation is attempted before
device is started,

Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index cdfbb6c22f..7fdd50d706 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1550,6 +1550,15 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		return NULL;
 	}
 
+	if (!dev->data->dev_started) {
+		rte_flow_error_set(error,
+				   EINVAL,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "Device must be started");
+		return NULL;
+	}
+
 	flow = rte_zmalloc("bnxt_flow", sizeof(struct rte_flow), 0);
 	if (!flow) {
 		rte_flow_error_set(error, ENOMEM,
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 13/20] net/bnxt: check for invalid VNIC ID in vnic tpa cfg
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (11 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 12/20] net/bnxt: check device is started before flow creation Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 14/20] net/bnxt: validate RSS hash key length Ajit Khaparde
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, Rahul Gupta, Kalesh Anakkur Purayil

From: Somnath Kotur <somnath.kotur@broadcom.com>

If driver init/probe fails as part of cleanup/rollback, we may end
up invoking this HWRM cmd even on an invalid vNIC which will unecessarily
log an error message as the cmd will fail.
Check for invalid ID before issuing the HWRM cmd

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 89697b83ac..e5f8fda9a3 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1965,6 +1965,11 @@ int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
 	if (BNXT_CHIP_THOR(bp))
 		return 0;
 
+	if (vnic->fw_vnic_id == INVALID_HW_RING_ID) {
+		PMD_DRV_LOG(DEBUG, "Invalid vNIC ID\n");
+		return 0;
+	}
+
 	HWRM_PREP(req, VNIC_TPA_CFG, BNXT_USE_CHIMP_MB);
 
 	if (enable) {
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 14/20] net/bnxt: validate RSS hash key length
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (12 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 13/20] net/bnxt: check for invalid VNIC ID in vnic tpa cfg Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 15/20] net/bnxt: handle cleanup if flow creation fails Ajit Khaparde
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, Venkat Duvvuru, Somnath Kotur, Rahul Gupta,
	Santoshkumar Karanappa Rastapur, Kalesh Anakkur Purayil

From: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>

In bnxt_rss_hash_update_op, driver is proceeding with
bnxt_hwrm_vnic_rss_cfg even though RSS hashkey length is invalid.

This patch fixes the problem by returning -EINVAL when RSS hashkey length
is invalid.

Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 2c9eaaa2ef..354fa4c630 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1373,14 +1373,20 @@ static int bnxt_rss_hash_update_op(struct rte_eth_dev *eth_dev,
 	vnic->hash_type = bnxt_rte_to_hwrm_hash_types(rss_conf->rss_hf);
 
 	/*
-	 * Use the supplied key if the key length is
-	 * acceptable and the rss_key is not NULL
+	 * If hashkey is not specified, use the previously configured
+	 * hashkey
 	 */
-	if (rss_conf->rss_key && rss_conf->rss_key_len <= HW_HASH_KEY_SIZE)
-		memcpy(vnic->rss_hash_key,
-		       rss_conf->rss_key,
-		       rss_conf->rss_key_len);
+	if (!rss_conf->rss_key)
+		goto rss_config;
 
+	if (rss_conf->rss_key_len != HW_HASH_KEY_SIZE) {
+		PMD_DRV_LOG(ERR,
+			    "Invalid hashkey length, should be 16 bytes\n");
+		return -EINVAL;
+	}
+	memcpy(vnic->rss_hash_key, rss_conf->rss_key, rss_conf->rss_key_len);
+
+rss_config:
 	bnxt_hwrm_vnic_rss_cfg(bp, vnic);
 	return 0;
 }
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 15/20] net/bnxt: handle cleanup if flow creation fails
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (13 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 14/20] net/bnxt: validate RSS hash key length Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 16/20] net/bnxt: synchronize between flow related functions Ajit Khaparde
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Venkat Duvvuru, Kalesh Anakkur Purayil

If flow creation fails because of an HWRM command failure or
or some other reason, reset the vnic and rxq info set earlier.

Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 66 +++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 7fdd50d706..938d24be61 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -177,6 +177,14 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 			return -rte_errno;
 		}
 
+		if (!item->spec || !item->mask) {
+			rte_flow_error_set(error, EINVAL,
+					   RTE_FLOW_ERROR_TYPE_ITEM,
+					   item,
+					   "spec/mask is NULL");
+			return -rte_errno;
+		}
+
 		switch (item->type) {
 		case RTE_FLOW_ITEM_TYPE_ANY:
 			inner =
@@ -971,11 +979,11 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		bnxt_flow_non_void_action(actions);
 	struct bnxt *bp = dev->data->dev_private;
 	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+	struct bnxt_vnic_info *vnic = NULL, *vnic0 = NULL;
 	const struct rte_flow_action_queue *act_q;
 	const struct rte_flow_action_vf *act_vf;
 	struct bnxt_filter_info *filter1 = NULL;
 	const struct rte_flow_action_rss *rss;
-	struct bnxt_vnic_info *vnic, *vnic0;
 	struct bnxt_rx_queue *rxq = NULL;
 	int dflt_vnic, vnic_id;
 	unsigned int rss_idx;
@@ -1077,8 +1085,15 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(DEBUG, "VNIC found\n");
 
 		rc = bnxt_vnic_prep(bp, vnic);
-		if (rc)
+		if (rc)  {
+			rte_flow_error_set(error,
+					   EINVAL,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "VNIC prep fail");
+			rc = -rte_errno;
 			goto ret;
+		}
 
 		PMD_DRV_LOG(DEBUG,
 			    "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
@@ -1091,7 +1106,12 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		filter->dst_id = vnic->fw_vnic_id;
 		filter1 = bnxt_get_l2_filter(bp, filter, vnic);
 		if (filter1 == NULL) {
-			rc = -ENOSPC;
+			rte_flow_error_set(error,
+					   ENOSPC,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "Filter not available");
+			rc = -rte_errno;
 			goto ret;
 		}
 
@@ -1119,7 +1139,12 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		vnic0 = &bp->vnic_info[0];
 		filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
 		if (filter1 == NULL) {
-			rc = -ENOSPC;
+			rte_flow_error_set(error,
+					   ENOSPC,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "New filter not available");
+			rc = -rte_errno;
 			goto ret;
 		}
 
@@ -1182,6 +1207,11 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		vnic0 = &bp->vnic_info[0];
 		filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
 		if (filter1 == NULL) {
+			rte_flow_error_set(error,
+					   ENOSPC,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "New filter not available");
 			rc = -ENOSPC;
 			goto ret;
 		}
@@ -1278,8 +1308,15 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		vnic->func_default = 0;	//This is not a default VNIC.
 
 		rc = bnxt_vnic_prep(bp, vnic);
-		if (rc)
+		if (rc) {
+			rte_flow_error_set(error,
+					   EINVAL,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "VNIC prep fail");
+			rc = -rte_errno;
 			goto ret;
+		}
 
 		PMD_DRV_LOG(DEBUG,
 			    "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
@@ -1334,6 +1371,11 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		filter->dst_id = vnic->fw_vnic_id;
 		filter1 = bnxt_get_l2_filter(bp, filter, vnic);
 		if (filter1 == NULL) {
+			rte_flow_error_set(error,
+					   ENOSPC,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "New filter not available");
 			rc = -ENOSPC;
 			goto ret;
 		}
@@ -1367,7 +1409,18 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		rc = -rte_errno;
 		goto ret;
 	}
+
+	return rc;
 ret:
+
+	//TODO: Cleanup according to ACTION TYPE.
+	if (rte_errno)  {
+		if (vnic && STAILQ_EMPTY(&vnic->filter))
+			vnic->rx_queue_cnt = 0;
+
+		if (rxq && !vnic->rx_queue_cnt)
+			rxq->vnic = &bp->vnic_info[0];
+	}
 	return rc;
 }
 
@@ -1414,6 +1467,8 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 
 	ret = bnxt_validate_and_parse_flow(dev, pattern, actions, attr,
 					   error, filter);
+	if (ret)
+		goto exit;
 
 	vnic = find_matching_vnic(bp, filter);
 	if (vnic) {
@@ -1434,6 +1489,7 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 	else
 		bnxt_hwrm_clear_l2_filter(bp, filter);
 
+exit:
 	/* No need to hold on to this filter if we are just validating flow */
 	filter->fw_l2_filter_id = UINT64_MAX;
 	bnxt_free_filter(bp, filter);
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 16/20] net/bnxt: synchronize between flow related functions
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (14 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 15/20] net/bnxt: handle cleanup if flow creation fails Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 17/20] net/bnxt: drop untagged frames when specified Ajit Khaparde
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Venkat Duvvuru, Kalesh Anakkur Purayil

From: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>

Currently, there are four flow related functions, namely
bnxt_flow_create, bnxt_flow_destroy, bnxt_flow_validate,
bnxt_flow_flush. All these functions are not multi-thread safe.

This patch fixes it by synchronizing these functions with a lock.

Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |  6 +++++
 drivers/net/bnxt/bnxt_ethdev.c | 23 ++++++++++++++++
 drivers/net/bnxt/bnxt_flow.c   | 49 +++++++++++++++++++++++++++-------
 3 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 8602ab3346..c34582c0ad 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -456,6 +456,7 @@ struct bnxt {
 	uint32_t		flow_flags;
 #define BNXT_FLOW_FLAG_L2_HDR_SRC_FILTER_EN	BIT(0)
 
+	pthread_mutex_t         flow_lock;
 	unsigned int		rx_nr_rings;
 	unsigned int		rx_cp_nr_rings;
 	unsigned int		rx_num_qs_per_vnic;
@@ -567,6 +568,11 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp);
 bool is_bnxt_supported(struct rte_eth_dev *dev);
 bool bnxt_stratus_device(struct bnxt *bp);
 extern const struct rte_flow_ops bnxt_flow_ops;
+#define bnxt_acquire_flow_lock(bp) \
+	pthread_mutex_lock(&(bp)->flow_lock)
+
+#define bnxt_release_flow_lock(bp) \
+	pthread_mutex_unlock(&(bp)->flow_lock)
 
 extern int bnxt_logtype_driver;
 #define PMD_DRV_LOG_RAW(level, fmt, args...) \
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 354fa4c630..9f8a63afab 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4476,6 +4476,17 @@ static int bnxt_init_fw(struct bnxt *bp)
 	return 0;
 }
 
+static int
+bnxt_init_locks(struct bnxt *bp)
+{
+	int err;
+
+	err = pthread_mutex_init(&bp->flow_lock, NULL);
+	if (err)
+		PMD_DRV_LOG(ERR, "Unable to initialize flow_lock\n");
+	return err;
+}
+
 static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev)
 {
 	int rc;
@@ -4533,6 +4544,10 @@ static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev)
 	if (rc)
 		return rc;
 
+	rc = bnxt_init_locks(bp);
+	if (rc)
+		return rc;
+
 	return 0;
 }
 
@@ -4613,6 +4628,12 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
 	return rc;
 }
 
+static void
+bnxt_uninit_locks(struct bnxt *bp)
+{
+	pthread_mutex_destroy(&bp->flow_lock);
+}
+
 static int
 bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
 {
@@ -4674,6 +4695,8 @@ bnxt_dev_uninit(struct rte_eth_dev *eth_dev)
 	eth_dev->rx_pkt_burst = NULL;
 	eth_dev->tx_pkt_burst = NULL;
 
+	bnxt_uninit_locks(bp);
+
 	return rc;
 }
 
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 938d24be61..58d7cc2261 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1455,13 +1455,17 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 	struct bnxt_filter_info *filter;
 	int ret = 0;
 
+	bnxt_acquire_flow_lock(bp);
 	ret = bnxt_flow_args_validate(attr, pattern, actions, error);
-	if (ret != 0)
+	if (ret != 0) {
+		bnxt_release_flow_lock(bp);
 		return ret;
+	}
 
 	filter = bnxt_get_unused_filter(bp);
 	if (filter == NULL) {
 		PMD_DRV_LOG(ERR, "Not enough resources for a new flow.\n");
+		bnxt_release_flow_lock(bp);
 		return -ENOMEM;
 	}
 
@@ -1493,6 +1497,7 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 	/* No need to hold on to this filter if we are just validating flow */
 	filter->fw_l2_filter_id = UINT64_MAX;
 	bnxt_free_filter(bp, filter);
+	bnxt_release_flow_lock(bp);
 
 	return ret;
 }
@@ -1623,6 +1628,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		return flow;
 	}
 
+	bnxt_acquire_flow_lock(bp);
 	ret = bnxt_flow_args_validate(attr, pattern, actions, error);
 	if (ret != 0) {
 		PMD_DRV_LOG(ERR, "Not a validate flow.\n");
@@ -1724,6 +1730,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		}
 		PMD_DRV_LOG(ERR, "Successfully created flow.\n");
 		STAILQ_INSERT_TAIL(&vnic->flow_list, flow, next);
+		bnxt_release_flow_lock(bp);
 		return flow;
 	}
 	if (!ret) {
@@ -1754,6 +1761,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 				   "Failed to create flow.");
 	rte_free(flow);
 	flow = NULL;
+	bnxt_release_flow_lock(bp);
 	return flow;
 }
 
@@ -1804,13 +1812,28 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 		  struct rte_flow_error *error)
 {
 	struct bnxt *bp = dev->data->dev_private;
-	struct bnxt_filter_info *filter = flow->filter;
-	struct bnxt_vnic_info *vnic = flow->vnic;
+	struct bnxt_filter_info *filter;
+	struct bnxt_vnic_info *vnic;
 	int ret = 0;
 
+	bnxt_acquire_flow_lock(bp);
+	if (!flow) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Invalid flow: failed to destroy flow.");
+		bnxt_release_flow_lock(bp);
+		return -EINVAL;
+	}
+
+	filter = flow->filter;
+	vnic = flow->vnic;
+
 	if (!filter) {
-		ret = -EINVAL;
-		goto done;
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Invalid flow: failed to destroy flow.");
+		bnxt_release_flow_lock(bp);
+		return -EINVAL;
 	}
 
 	if (filter->filter_type == HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
@@ -1818,10 +1841,12 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 		ret = bnxt_handle_tunnel_redirect_destroy(bp,
 							  filter,
 							  error);
-		if (!ret)
+		if (!ret) {
 			goto done;
-		else
+		} else {
+			bnxt_release_flow_lock(bp);
 			return ret;
+		}
 	}
 
 	ret = bnxt_match_filter(bp, filter);
@@ -1859,6 +1884,7 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 				   "Failed to destroy flow.");
 	}
 
+	bnxt_release_flow_lock(bp);
 	return ret;
 }
 
@@ -1871,6 +1897,7 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 	unsigned int i;
 	int ret = 0;
 
+	bnxt_acquire_flow_lock(bp);
 	for (i = 0; i < bp->max_vnics; i++) {
 		vnic = &bp->vnic_info[i];
 		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
@@ -1886,10 +1913,12 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 				bnxt_handle_tunnel_redirect_destroy(bp,
 								    filter,
 								    error);
-				if (!ret)
+				if (!ret) {
 					goto done;
-				else
+				} else {
+					bnxt_release_flow_lock(bp);
 					return ret;
+				}
 			}
 
 			if (filter->filter_type == HWRM_CFA_EM_FILTER)
@@ -1906,6 +1935,7 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 					 RTE_FLOW_ERROR_TYPE_HANDLE,
 					 NULL,
 					 "Failed to flush flow in HW.");
+				bnxt_release_flow_lock(bp);
 				return -rte_errno;
 			}
 done:
@@ -1916,6 +1946,7 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 		}
 	}
 
+	bnxt_release_flow_lock(bp);
 	return ret;
 }
 
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 17/20] net/bnxt: drop untagged frames when specified
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (15 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 16/20] net/bnxt: synchronize between flow related functions Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-03 13:17   ` Ferruh Yigit
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 18/20] net/bnxt: fix VLAN filtering code path Ajit Khaparde
                   ` (4 subsequent siblings)
  21 siblings, 1 reply; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Somnath Kotur, Kalesh Anakkur Purayil

When a drop action for L2 filters is specified, support it.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.h |  6 ++++
 drivers/net/bnxt/bnxt_flow.c   | 61 ++++++++++++++++++++++++++++++----
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 6bcdc258b7..6e90a98257 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -15,6 +15,8 @@ struct bnxt;
 #define BNXT_FLOW_L2_INNER_SRC_VALID_FLAG	BIT(2)
 #define BNXT_FLOW_L2_DST_VALID_FLAG		BIT(3)
 #define BNXT_FLOW_L2_INNER_DST_VALID_FLAG	BIT(4)
+#define BNXT_FLOW_L2_DROP_FLAG			BIT(5)
+#define BNXT_FLOW_PARSE_INNER_FLAG		BIT(6)
 
 struct bnxt_filter_info {
 	STAILQ_ENTRY(bnxt_filter_info)	next;
@@ -148,4 +150,8 @@ struct bnxt_filter_info *bnxt_get_l2_filter(struct bnxt *bp,
 	HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_ENABLES_MIRROR_VNIC_ID
 #define NTUPLE_FLTR_ALLOC_INPUT_EN_MIRROR_VNIC_ID	\
 	HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_ENABLES_MIRROR_VNIC_ID
+#define L2_FILTER_ALLOC_INPUT_EN_T_NUM_VLANS \
+	HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_T_NUM_VLANS
+#define L2_FILTER_ALLOC_INPUT_EN_NUM_VLANS \
+	HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_NUM_VLANS
 #endif
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 58d7cc2261..9c27f751a8 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -275,6 +275,8 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
 					rte_be_to_cpu_16(eth_spec->type);
 				en |= en_ethertype;
 			}
+			if (inner)
+				valid_flags |= BNXT_FLOW_PARSE_INNER_FLAG;
 
 			break;
 		case RTE_FLOW_ITEM_TYPE_VLAN:
@@ -832,9 +834,36 @@ bnxt_create_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 		}
 	}
 
-	filter1->enables = HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
+	if (nf->valid_flags & (BNXT_FLOW_L2_DST_VALID_FLAG |
+			       BNXT_FLOW_L2_SRC_VALID_FLAG |
+			       BNXT_FLOW_L2_INNER_SRC_VALID_FLAG |
+			       BNXT_FLOW_L2_INNER_DST_VALID_FLAG)) {
+		filter1->enables =
+			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
 			L2_FILTER_ALLOC_INPUT_EN_L2_ADDR_MASK;
-	memset(filter1->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
+		memset(filter1->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
+	}
+
+	if (nf->valid_flags & BNXT_FLOW_L2_DROP_FLAG) {
+		filter1->flags |=
+			HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_DROP;
+		if (nf->ethertype == RTE_ETHER_TYPE_IPV4) {
+			/* Num VLANs for drop filter will/should be 0.
+			 * If the req is memset to 0, then the count will
+			 * be automatically set to 0.
+			 */
+			if (nf->valid_flags & BNXT_FLOW_PARSE_INNER_FLAG) {
+				filter1->enables |=
+					L2_FILTER_ALLOC_INPUT_EN_T_NUM_VLANS;
+			} else {
+				filter1->enables |=
+					L2_FILTER_ALLOC_INPUT_EN_NUM_VLANS;
+				filter1->flags |=
+				HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
+			}
+		}
+	}
+
 	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id,
 				     filter1);
 	if (rc) {
@@ -952,7 +981,9 @@ bnxt_update_filter_flags_en(struct bnxt_filter_info *filter,
 	      ~(BNXT_FLOW_L2_DST_VALID_FLAG |
 		BNXT_FLOW_L2_SRC_VALID_FLAG |
 		BNXT_FLOW_L2_INNER_SRC_VALID_FLAG |
-		BNXT_FLOW_L2_INNER_DST_VALID_FLAG))) {
+		BNXT_FLOW_L2_INNER_DST_VALID_FLAG |
+		BNXT_FLOW_L2_DROP_FLAG |
+		BNXT_FLOW_PARSE_INNER_FLAG))) {
 		filter->flags = filter1->flags;
 		filter->enables = filter1->enables;
 		filter->filter_type = HWRM_CFA_L2_FILTER;
@@ -1121,19 +1152,27 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		break;
 	case RTE_FLOW_ACTION_TYPE_DROP:
 		vnic0 = &bp->vnic_info[0];
+		filter->dst_id = vnic0->fw_vnic_id;
+		filter->valid_flags |= BNXT_FLOW_L2_DROP_FLAG;
 		filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
 		if (filter1 == NULL) {
+			rte_flow_error_set(error,
+					   ENOSPC,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "Filter not available");
 			rc = -ENOSPC;
 			goto ret;
 		}
 
-		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
 		if (filter->filter_type == HWRM_CFA_EM_FILTER)
 			filter->flags =
 				HWRM_CFA_EM_FLOW_ALLOC_INPUT_FLAGS_DROP;
-		else
+		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			filter->flags =
 				HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_FLAGS_DROP;
+
+		bnxt_update_filter_flags_en(filter, filter1);
 		break;
 	case RTE_FLOW_ACTION_TYPE_COUNT:
 		vnic0 = &bp->vnic_info[0];
@@ -1861,6 +1900,15 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 
 done:
 	if (!ret) {
+		/* If it is a L2 drop filter, when the filter is created,
+		 * the FW updates the BC/MC records.
+		 * Once this filter is removed, issue the set_rx_mask command
+		 * to reset the BC/MC records in the HW to the settings
+		 * before the drop counter is created.
+		 */
+		if (filter->valid_flags & BNXT_FLOW_L2_DROP_FLAG)
+			bnxt_set_rx_mask_no_vlan(bp, &bp->vnic_info[0]);
+
 		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
 		bnxt_free_filter(bp, filter);
 		STAILQ_REMOVE(&vnic->flow_list, flow, rte_flow, next);
@@ -1869,7 +1917,8 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 		/* If this was the last flow associated with this vnic,
 		 * switch the queue back to RSS pool.
 		 */
-		if (vnic && STAILQ_EMPTY(&vnic->flow_list)) {
+		if (vnic && !vnic->func_default &&
+		    STAILQ_EMPTY(&vnic->flow_list)) {
 			rte_free(vnic->fw_grp_ids);
 			if (vnic->rx_queue_cnt > 1)
 				bnxt_hwrm_vnic_ctx_free(bp, vnic);
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 18/20] net/bnxt: fix VLAN filtering code path
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (16 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 17/20] net/bnxt: drop untagged frames when specified Ajit Khaparde
@ 2019-10-02 23:25 ` Ajit Khaparde
  2019-10-02 23:26 ` [dpdk-dev] [PATCH v2 19/20] net/bnxt: fix multicast filter programming Ajit Khaparde
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:25 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Venkat Duvvuru, stable, Somnath Kotur

From: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>

Currently, when hw-vlan-filter is enabled on testpmd, driver is
receiving all vlan packets. Instead, it should only receive untagged
packets and vlan packets for which the VLAN filter is programmed.
This is because, the default rule to match on MAC is not getting
deleted, when hw-vlan-filter is ON.

This patch fixes the problem, by deleting the default MAC rule and
programming a new rule to receive only untagged packets, when
hw-vlan-filter is enabled & another rule for each vlan, as and when
that vlan is configured on that port.

Fixes: 246c5cc5f05e ("net/bnxt: use correct flags during VLAN configuration")
Cc: stable@dpdk.org
Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 153 ++++++++++++++++++++++++++-------
 drivers/net/bnxt/bnxt_filter.h |   1 +
 drivers/net/bnxt/bnxt_hwrm.c   |   9 +-
 3 files changed, 128 insertions(+), 35 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 9f8a63afab..e305ad4163 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1001,6 +1001,53 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 	}
 }
 
+static int bnxt_add_mac_filter(struct bnxt *bp, struct bnxt_vnic_info *vnic,
+			       struct rte_ether_addr *mac_addr, uint32_t index)
+{
+	struct bnxt_filter_info *filter;
+	int rc = 0;
+
+	filter = STAILQ_FIRST(&vnic->filter);
+	/* During bnxt_mac_addr_add_op, default MAC is
+	 * already programmed, so skip it. But, when
+	 * hw-vlan-filter is turned OFF from ON, default
+	 * MAC filter should be restored
+	 */
+	if (filter->dflt)
+		return 0;
+
+	filter = bnxt_alloc_filter(bp);
+	if (!filter) {
+		PMD_DRV_LOG(ERR, "L2 filter alloc failed\n");
+		return -ENODEV;
+	}
+
+	filter->mac_index = index;
+	/* bnxt_alloc_filter copies default MAC to filter->l2_addr. So,
+	 * if the MAC that's been programmed now is a different one, then,
+	 * copy that addr to filter->l2_addr
+	 */
+	if (mac_addr)
+		memcpy(filter->l2_addr, mac_addr, RTE_ETHER_ADDR_LEN);
+	filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
+
+	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
+	if (!rc) {
+		if (filter->mac_index == 0) {
+			filter->dflt = true;
+			STAILQ_INSERT_HEAD(&vnic->filter, filter, next);
+		} else {
+			STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
+		}
+	} else {
+		filter->mac_index = INVALID_MAC_INDEX;
+		memset(&filter->l2_addr, 0, RTE_ETHER_ADDR_LEN);
+		bnxt_free_filter(bp, filter);
+	}
+
+	return rc;
+}
+
 static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 				struct rte_ether_addr *mac_addr,
 				uint32_t index, uint32_t pool)
@@ -1031,24 +1078,8 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 			return 0;
 		}
 	}
-	filter = bnxt_alloc_filter(bp);
-	if (!filter) {
-		PMD_DRV_LOG(ERR, "L2 filter alloc failed\n");
-		return -ENODEV;
-	}
 
-	filter->mac_index = index;
-	memcpy(filter->l2_addr, mac_addr, RTE_ETHER_ADDR_LEN);
-	filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
-
-	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
-	if (!rc) {
-		STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
-	} else {
-		filter->mac_index = INVALID_MAC_INDEX;
-		memset(&filter->l2_addr, 0, RTE_ETHER_ADDR_LEN);
-		bnxt_free_filter(bp, filter);
-	}
+	rc = bnxt_add_mac_filter(bp, vnic, mac_addr, index);
 
 	return rc;
 }
@@ -1683,9 +1714,10 @@ static int bnxt_del_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 	filter = STAILQ_FIRST(&vnic->filter);
 	while (filter) {
 		/* Search for this matching MAC+VLAN filter */
-		if (filter->enables & chk && filter->l2_ivlan == vlan_id &&
-		    !memcmp(filter->l2_addr,
-			    bp->mac_addr,
+		if ((filter->enables & chk) &&
+		    (filter->l2_ivlan == vlan_id &&
+		     filter->l2_ivlan_mask != 0) &&
+		    !memcmp(filter->l2_addr, bp->mac_addr,
 			    RTE_ETHER_ADDR_LEN)) {
 			/* Delete the filter */
 			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
@@ -1726,8 +1758,11 @@ static int bnxt_add_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 	filter = STAILQ_FIRST(&vnic->filter);
 	/* Check if the VLAN has already been added */
 	while (filter) {
-		if (filter->enables & chk && filter->l2_ivlan == vlan_id &&
-		    !memcmp(filter->l2_addr, bp->mac_addr, RTE_ETHER_ADDR_LEN))
+		if ((filter->enables & chk) &&
+		    (filter->l2_ivlan == vlan_id &&
+		     filter->l2_ivlan_mask == 0x0FFF) &&
+		     !memcmp(filter->l2_addr, bp->mac_addr,
+			     RTE_ETHER_ADDR_LEN))
 			return -EEXIST;
 
 		filter = STAILQ_NEXT(filter, next);
@@ -1743,10 +1778,17 @@ static int bnxt_add_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 		return -ENOMEM;
 	}
 	/* MAC + VLAN ID filter */
+	/* If l2_ivlan == 0 and l2_ivlan_mask != 0, only
+	 * untagged packets are received
+	 *
+	 * If l2_ivlan != 0 and l2_ivlan_mask != 0, untagged
+	 * packets and only the programmed vlan's packets are received
+	 */
 	filter->l2_ivlan = vlan_id;
 	filter->l2_ivlan_mask = 0x0FFF;
 	filter->enables |= en;
 	filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
+
 	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
 	if (rc) {
 		/* Free the newly allocated filter as we were
@@ -1755,10 +1797,16 @@ static int bnxt_add_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 		filter->fw_l2_filter_id = UINT64_MAX;
 		STAILQ_INSERT_TAIL(&bp->free_filter_list, filter, next);
 		return rc;
+	} else {
+		/* Add this new filter to the list */
+		if (vlan_id == 0) {
+			filter->dflt = true;
+			STAILQ_INSERT_HEAD(&vnic->filter, filter, next);
+		} else {
+			STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
+		}
 	}
 
-	/* Add this new filter to the list */
-	STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 	PMD_DRV_LOG(INFO,
 		    "Added Vlan filter for %d\n", vlan_id);
 	return rc;
@@ -1781,11 +1829,39 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev,
 		return bnxt_del_vlan_filter(bp, vlan_id);
 }
 
+static int bnxt_del_dflt_mac_filter(struct bnxt *bp,
+				    struct bnxt_vnic_info *vnic)
+{
+	struct bnxt_filter_info *filter;
+	int rc;
+
+	filter = STAILQ_FIRST(&vnic->filter);
+	while (filter) {
+		if (filter->dflt &&
+		    !memcmp(filter->l2_addr, bp->mac_addr,
+			    RTE_ETHER_ADDR_LEN)) {
+			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
+			if (rc)
+				return rc;
+			filter->dflt = false;
+			STAILQ_REMOVE(&vnic->filter, filter,
+				      bnxt_filter_info, next);
+			STAILQ_INSERT_TAIL(&bp->free_filter_list,
+					   filter, next);
+			filter->fw_l2_filter_id = -1;
+			break;
+		}
+		filter = STAILQ_NEXT(filter, next);
+	}
+	return 0;
+}
+
 static int
 bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 {
 	struct bnxt *bp = dev->data->dev_private;
 	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
+	struct bnxt_vnic_info *vnic;
 	unsigned int i;
 	int rc;
 
@@ -1793,15 +1869,28 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 	if (rc)
 		return rc;
 
-	if (mask & ETH_VLAN_FILTER_MASK) {
-		if (!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
-			/* Remove any VLAN filters programmed */
-			for (i = 0; i < 4095; i++)
-				bnxt_del_vlan_filter(bp, i);
-		}
-		PMD_DRV_LOG(DEBUG, "VLAN Filtering: %d\n",
-			!!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER));
+	vnic = BNXT_GET_DEFAULT_VNIC(bp);
+	if (!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
+		/* Remove any VLAN filters programmed */
+		for (i = 0; i < 4095; i++)
+			bnxt_del_vlan_filter(bp, i);
+
+		rc = bnxt_add_mac_filter(bp, vnic, NULL, 0);
+		if (rc)
+			return rc;
+	} else {
+		/* Default filter will allow packets that match the
+		 * dest mac. So, it has to be deleted, otherwise, we
+		 * will endup receiving vlan packets for which the
+		 * filter is not programmed, when hw-vlan-filter
+		 * configuration is ON
+		 */
+		bnxt_del_dflt_mac_filter(bp, vnic);
+		/* This filter will allow only untagged packets */
+		bnxt_add_vlan_filter(bp, 0);
 	}
+	PMD_DRV_LOG(DEBUG, "VLAN Filtering: %d\n",
+		    !!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER));
 
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 6e90a98257..e09b435dbf 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -71,6 +71,7 @@ struct bnxt_filter_info {
 	uint16_t                ip_addr_type;
 	uint16_t                ethertype;
 	uint32_t		priority;
+	uint8_t			dflt;
 };
 
 struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index e5f8fda9a3..2a7e0d6d15 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2415,15 +2415,18 @@ int bnxt_set_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	int rc = 0;
 
 	STAILQ_FOREACH(filter, &vnic->filter, next) {
-		if (filter->filter_type == HWRM_CFA_EM_FILTER)
+		if (filter->filter_type == HWRM_CFA_EM_FILTER) {
 			rc = bnxt_hwrm_set_em_filter(bp, filter->dst_id,
 						     filter);
-		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
+		} else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER) {
 			rc = bnxt_hwrm_set_ntuple_filter(bp, filter->dst_id,
 							 filter);
-		else
+		} else {
 			rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id,
 						     filter);
+			if (!rc)
+				filter->dflt = 1;
+		}
 		if (rc)
 			break;
 	}
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 19/20] net/bnxt: fix multicast filter programming
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (17 preceding siblings ...)
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 18/20] net/bnxt: fix VLAN filtering code path Ajit Khaparde
@ 2019-10-02 23:26 ` Ajit Khaparde
  2019-10-02 23:26 ` [dpdk-dev] [PATCH v2 20/20] net/bnxt: handle flow flush handling Ajit Khaparde
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:26 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP, stable

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

Fixed multicast filter programming and allmulti programming.
Fixed to skip programming multicast macs if the user requests
allmulti mode.

Also removed a comment in bnxt_hwrm_cfa_l2_set_rx_mask() which is
no longer valid now.

Fixes: d69851df12b2 ("net/bnxt: support multicast filter and set MAC addr")
Cc: stable@dpdk.org

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c |  4 ++++
 drivers/net/bnxt/bnxt_hwrm.c   | 11 ++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index e305ad4163..02eacf7965 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2053,6 +2053,10 @@ bnxt_dev_set_mc_addr_list_op(struct rte_eth_dev *eth_dev,
 	}
 
 	vnic->mc_addr_cnt = i;
+	if (vnic->mc_addr_cnt)
+		vnic->flags |= BNXT_VNIC_INFO_MCAST;
+	else
+		vnic->flags &= ~BNXT_VNIC_INFO_MCAST;
 
 allmulti:
 	return bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL);
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 2a7e0d6d15..011cd05ae0 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -295,20 +295,17 @@ int bnxt_hwrm_cfa_l2_set_rx_mask(struct bnxt *bp,
 	HWRM_PREP(req, CFA_L2_SET_RX_MASK, BNXT_USE_CHIMP_MB);
 	req.vnic_id = rte_cpu_to_le_16(vnic->fw_vnic_id);
 
-	/* FIXME add multicast flag, when multicast adding options is supported
-	 * by ethtool.
-	 */
 	if (vnic->flags & BNXT_VNIC_INFO_BCAST)
 		mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_BCAST;
 	if (vnic->flags & BNXT_VNIC_INFO_UNTAGGED)
 		mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_VLAN_NONVLAN;
+
 	if (vnic->flags & BNXT_VNIC_INFO_PROMISC)
 		mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_PROMISCUOUS;
-	if (vnic->flags & BNXT_VNIC_INFO_ALLMULTI)
+
+	if (vnic->flags & BNXT_VNIC_INFO_ALLMULTI) {
 		mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_ALL_MCAST;
-	if (vnic->flags & BNXT_VNIC_INFO_MCAST)
-		mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_MCAST;
-	if (vnic->mc_addr_cnt) {
+	} else if (vnic->flags & BNXT_VNIC_INFO_MCAST) {
 		mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_MCAST;
 		req.num_mc_entries = rte_cpu_to_le_32(vnic->mc_addr_cnt);
 		req.mc_tbl_addr = rte_cpu_to_le_64(vnic->mc_list_dma_addr);
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [dpdk-dev] [PATCH v2 20/20] net/bnxt: handle flow flush handling
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (18 preceding siblings ...)
  2019-10-02 23:26 ` [dpdk-dev] [PATCH v2 19/20] net/bnxt: fix multicast filter programming Ajit Khaparde
@ 2019-10-02 23:26 ` Ajit Khaparde
  2019-10-03 12:39   ` Ferruh Yigit
  2019-10-03 13:18 ` [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ferruh Yigit
  2019-10-03 14:29 ` Ferruh Yigit
  21 siblings, 1 reply; 25+ messages in thread
From: Ajit Khaparde @ 2019-10-02 23:26 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Rahul Gupta, Venkat Duvvuru, Kalesh Anakkur Purayil

We are not freeing all the flows when a flow_flush is called.
Iterate through all the flows belonging to all the VNICs in use and
free the filters.

Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 63 ++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 9c27f751a8..2653ce9761 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1161,7 +1161,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 					   RTE_FLOW_ERROR_TYPE_ACTION,
 					   act,
 					   "Filter not available");
-			rc = -ENOSPC;
+			rc = -rte_errno;
 			goto ret;
 		}
 
@@ -1172,7 +1172,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 			filter->flags =
 				HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_FLAGS_DROP;
 
-		bnxt_update_filter_flags_en(filter, filter1);
+		bnxt_update_filter_flags_en(filter, filter1, use_ntuple);
 		break;
 	case RTE_FLOW_ACTION_TYPE_COUNT:
 		vnic0 = &bp->vnic_info[0];
@@ -1251,7 +1251,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 					   RTE_FLOW_ERROR_TYPE_ACTION,
 					   act,
 					   "New filter not available");
-			rc = -ENOSPC;
+			rc = -rte_errno;
 			goto ret;
 		}
 
@@ -1415,7 +1415,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 					   RTE_FLOW_ERROR_TYPE_ACTION,
 					   act,
 					   "New filter not available");
-			rc = -ENOSPC;
+			rc = -rte_errno;
 			goto ret;
 		}
 
@@ -1520,7 +1520,6 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
 			bnxt_hwrm_vnic_ctx_free(bp, vnic);
 			bnxt_hwrm_vnic_free(bp, vnic);
 			vnic->rx_queue_cnt = 0;
-			bp->nr_vnics--;
 			PMD_DRV_LOG(DEBUG, "Free VNIC\n");
 		}
 	}
@@ -1753,36 +1752,20 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 	vnic = find_matching_vnic(bp, filter);
 done:
 	if (!ret || update_flow) {
-		flow->filter = filter;
-		flow->vnic = vnic;
-		/* VNIC is set only in case of queue or RSS action */
-		if (vnic) {
-			/*
-			 * RxQ0 is not used for flow filters.
-			 */
-
-			if (update_flow) {
-				ret = -EXDEV;
-				goto free_flow;
-			}
-			STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
-		}
-		PMD_DRV_LOG(ERR, "Successfully created flow.\n");
-		STAILQ_INSERT_TAIL(&vnic->flow_list, flow, next);
-		bnxt_release_flow_lock(bp);
-		return flow;
-	}
-	if (!ret) {
 		flow->filter = filter;
 		flow->vnic = vnic;
 		if (update_flow) {
 			ret = -EXDEV;
 			goto free_flow;
 		}
+
+		STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 		PMD_DRV_LOG(ERR, "Successfully created flow.\n");
 		STAILQ_INSERT_TAIL(&vnic->flow_list, flow, next);
+		bnxt_release_flow_lock(bp);
 		return flow;
 	}
+
 free_filter:
 	bnxt_free_filter(bp, filter);
 free_flow:
@@ -1925,7 +1908,6 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 
 			bnxt_hwrm_vnic_free(bp, vnic);
 			vnic->rx_queue_cnt = 0;
-			bp->nr_vnics--;
 		}
 	} else {
 		rte_flow_error_set(error, -ret,
@@ -1941,6 +1923,7 @@ static int
 bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 {
 	struct bnxt *bp = dev->data->dev_private;
+	struct bnxt_filter_info *filter = NULL;
 	struct bnxt_vnic_info *vnic;
 	struct rte_flow *flow;
 	unsigned int i;
@@ -1949,11 +1932,12 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 	bnxt_acquire_flow_lock(bp);
 	for (i = 0; i < bp->max_vnics; i++) {
 		vnic = &bp->vnic_info[i];
-		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
+		if (vnic && vnic->fw_vnic_id == INVALID_VNIC_ID)
 			continue;
 
-		STAILQ_FOREACH(flow, &vnic->flow_list, next) {
-			struct bnxt_filter_info *filter = flow->filter;
+		while (!STAILQ_EMPTY(&vnic->flow_list)) {
+			flow = STAILQ_FIRST(&vnic->flow_list);
+			filter = flow->filter;
 
 			if (filter->filter_type ==
 			    HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
@@ -1974,7 +1958,7 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 				ret = bnxt_hwrm_clear_em_filter(bp, filter);
 			if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 				ret = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-			else if (!i)
+			else if (i)
 				ret = bnxt_hwrm_clear_l2_filter(bp, filter);
 
 			if (ret) {
@@ -1988,10 +1972,27 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 				return -rte_errno;
 			}
 done:
-			bnxt_free_filter(bp, filter);
 			STAILQ_REMOVE(&vnic->flow_list, flow,
 				      rte_flow, next);
+
+			STAILQ_REMOVE(&vnic->filter,
+				      filter,
+				      bnxt_filter_info,
+				      next);
+			bnxt_free_filter(bp, filter);
+
 			rte_free(flow);
+
+			/* If this was the last flow associated with this vnic,
+			 * switch the queue back to RSS pool.
+			 */
+			if (STAILQ_EMPTY(&vnic->flow_list)) {
+				rte_free(vnic->fw_grp_ids);
+				if (vnic->rx_queue_cnt > 1)
+					bnxt_hwrm_vnic_ctx_free(bp, vnic);
+				bnxt_hwrm_vnic_free(bp, vnic);
+				vnic->rx_queue_cnt = 0;
+			}
 		}
 	}
 
-- 
2.20.1 (Apple Git-117)


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [dpdk-dev] [PATCH v2 20/20] net/bnxt: handle flow flush handling
  2019-10-02 23:26 ` [dpdk-dev] [PATCH v2 20/20] net/bnxt: handle flow flush handling Ajit Khaparde
@ 2019-10-03 12:39   ` Ferruh Yigit
  0 siblings, 0 replies; 25+ messages in thread
From: Ferruh Yigit @ 2019-10-03 12:39 UTC (permalink / raw)
  To: Ajit Khaparde, dev; +Cc: Rahul Gupta, Venkat Duvvuru, Kalesh Anakkur Purayil

On 10/3/2019 12:26 AM, Ajit Khaparde wrote:
> We are not freeing all the flows when a flow_flush is called.
> Iterate through all the flows belonging to all the VNICs in use and
> free the filters.

For the patch title "handle flow flush handling", I think here 'handle' means
fixing the "flow flush handling", I am updating that way. Can you please send
the fixes tag for it, I can update later in the next-net?

Same question for other patches in this patchset that start with "handle xxx",
is that handle means fix something? If so can you please send commit log updates
for them too?

Thanks.

> 
> Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
> Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
> Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [dpdk-dev] [PATCH v2 17/20] net/bnxt: drop untagged frames when specified
  2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 17/20] net/bnxt: drop untagged frames when specified Ajit Khaparde
@ 2019-10-03 13:17   ` Ferruh Yigit
  0 siblings, 0 replies; 25+ messages in thread
From: Ferruh Yigit @ 2019-10-03 13:17 UTC (permalink / raw)
  To: Ajit Khaparde, dev; +Cc: Rahul Gupta, Somnath Kotur, Kalesh Anakkur Purayil

On 10/3/2019 12:25 AM, Ajit Khaparde wrote:
> When a drop action for L2 filters is specified, support it.
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>

<...>

> @@ -1121,19 +1152,27 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_DROP:
>  		vnic0 = &bp->vnic_info[0];
> +		filter->dst_id = vnic0->fw_vnic_id;
> +		filter->valid_flags |= BNXT_FLOW_L2_DROP_FLAG;
>  		filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
>  		if (filter1 == NULL) {
> +			rte_flow_error_set(error,
> +					   ENOSPC,
> +					   RTE_FLOW_ERROR_TYPE_ACTION,
> +					   act,
> +					   "Filter not available");
>  			rc = -ENOSPC;
>  			goto ret;
>  		}
>  
> -		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
>  		if (filter->filter_type == HWRM_CFA_EM_FILTER)
>  			filter->flags =
>  				HWRM_CFA_EM_FLOW_ALLOC_INPUT_FLAGS_DROP;
> -		else
> +		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
>  			filter->flags =
>  				HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_FLAGS_DROP;
> +
> +		bnxt_update_filter_flags_en(filter, filter1);

This function gets three parameter [1], you are breaking the build here and
fixing it later in the patches, I will fix this while merging.
But please test patch by patch build next time.

[1]
bnxt_update_filter_flags_en(filter, filter1, use_ntuple);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (19 preceding siblings ...)
  2019-10-02 23:26 ` [dpdk-dev] [PATCH v2 20/20] net/bnxt: handle flow flush handling Ajit Khaparde
@ 2019-10-03 13:18 ` Ferruh Yigit
  2019-10-03 14:29 ` Ferruh Yigit
  21 siblings, 0 replies; 25+ messages in thread
From: Ferruh Yigit @ 2019-10-03 13:18 UTC (permalink / raw)
  To: Ajit Khaparde, dev

On 10/3/2019 12:25 AM, Ajit Khaparde wrote:
> Among other changes, this patchset adds support to:
> - create filters with RSS action.
> - create source MAC filters.
> - use user provided priority to place rule appropriately in HW.
> 
> This patch has been rebased to dpdk-next-net commit
> 8587a8b9eddefa39e4ceac7e9385efcc5e73307c
> 
> Please apply.
> 
> 
> Ajit Khaparde (13):
>   net/bnxt: return standard error codes for HWRM command
>   net/bnxt: refactor code to allow dynamic creation of VNIC
>   net/bnxt: allow flow creation when RSS is enabled
>   net/bnxt: add support to create SMAC and inner DMAC filters
>   net/bnxt: add support for RSS action
>   net/bnxt: parse priority attribute for flow creation
>   net/bnxt: delete and flush L2 filters cleanly
>   net/bnxt: cleanup vnic after flow validate
>   net/bnxt: allow only unicast MAC address filter creation
>   net/bnxt: check device is started before flow creation
>   net/bnxt: handle cleanup if flow creation fails
>   net/bnxt: drop untagged frames when specified
>   net/bnxt: handle flow flush handling
> 
> Kalesh AP (2):
>   net/bnxt: fix an issue in setting default MAC address
>   net/bnxt: fix multicast filter programming
> 
> Rahul Gupta (1):
>   net/bnxt: properly handle ring cleanup in case of error
> 
> Somnath Kotur (1):
>   net/bnxt: check for invalid VNIC ID in vnic tpa cfg
> 
> Venkat Duvvuru (3):
>   net/bnxt: validate RSS hash key length
>   net/bnxt: synchronize between flow related functions
>   net/bnxt: fix VLAN filtering code path

32-bit build error [1] fixed while merging, please confirm the fix in the repo.


[1]
In file included from .../drivers/net/bnxt/bnxt_flow.c:14:
.../drivers/net/bnxt/bnxt_flow.c: In function ‘bnxt_update_filter_flags_en’:
.../drivers/net/bnxt/bnxt.h:579:50: error: format ‘%lx’ expects argument of type
‘long unsigned int’, but argument 6 has type ‘uint64_t’ {aka ‘long long unsigned
int’} [-Werror=format=]
  579 |  rte_log(RTE_LOG_ ## level, bnxt_logtype_driver, "%s(): " fmt, \
      |                                                  ^~~~~~~~
.../drivers/net/bnxt/bnxt.h:583:2: note: in expansion of macro ‘PMD_DRV_LOG_RAW’
  583 |  PMD_DRV_LOG_RAW(level, fmt, ## args)
      |  ^~~~~~~~~~~~~~~
.../drivers/net/bnxt/bnxt_flow.c:997:2: note: in expansion of macro ‘PMD_DRV_LOG’
  997 |  PMD_DRV_LOG(DEBUG, "l2_filter: %p fw_l2_filter_id %lx l2_ref_cnt %d\n",
      |  ^~~~~~~~~~~
.../drivers/net/bnxt/bnxt_flow.c:997:54: note: format string is defined here
  997 |  PMD_DRV_LOG(DEBUG, "l2_filter: %p fw_l2_filter_id %lx l2_ref_cnt %d\n",
      |                                                    ~~^
      |                                                      |
      |                                                      long unsigned int
      |                                                    %llx


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support
  2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
                   ` (20 preceding siblings ...)
  2019-10-03 13:18 ` [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ferruh Yigit
@ 2019-10-03 14:29 ` Ferruh Yigit
  21 siblings, 0 replies; 25+ messages in thread
From: Ferruh Yigit @ 2019-10-03 14:29 UTC (permalink / raw)
  To: Ajit Khaparde, dev

On 10/3/2019 12:25 AM, Ajit Khaparde wrote:
> Among other changes, this patchset adds support to:
> - create filters with RSS action.
> - create source MAC filters.
> - use user provided priority to place rule appropriately in HW.
> 
> This patch has been rebased to dpdk-next-net commit
> 8587a8b9eddefa39e4ceac7e9385efcc5e73307c
> 
> Please apply.
> 
> 
> Ajit Khaparde (13):
>   net/bnxt: return standard error codes for HWRM command
>   net/bnxt: refactor code to allow dynamic creation of VNIC
>   net/bnxt: allow flow creation when RSS is enabled
>   net/bnxt: add support to create SMAC and inner DMAC filters
>   net/bnxt: add support for RSS action
>   net/bnxt: parse priority attribute for flow creation
>   net/bnxt: delete and flush L2 filters cleanly
>   net/bnxt: cleanup vnic after flow validate
>   net/bnxt: allow only unicast MAC address filter creation
>   net/bnxt: check device is started before flow creation
>   net/bnxt: handle cleanup if flow creation fails
>   net/bnxt: drop untagged frames when specified
>   net/bnxt: handle flow flush handling
> 
> Kalesh AP (2):
>   net/bnxt: fix an issue in setting default MAC address
>   net/bnxt: fix multicast filter programming
> 
> Rahul Gupta (1):
>   net/bnxt: properly handle ring cleanup in case of error
> 
> Somnath Kotur (1):
>   net/bnxt: check for invalid VNIC ID in vnic tpa cfg
> 
> Venkat Duvvuru (3):
>   net/bnxt: validate RSS hash key length
>   net/bnxt: synchronize between flow related functions
>   net/bnxt: fix VLAN filtering code path

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2019-10-03 14:29 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 23:25 [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 01/20] net/bnxt: return standard error codes for HWRM command Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 02/20] net/bnxt: refactor code to allow dynamic creation of VNIC Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 03/20] net/bnxt: allow flow creation when RSS is enabled Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 04/20] net/bnxt: add support to create SMAC and inner DMAC filters Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 05/20] net/bnxt: add support for RSS action Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 06/20] net/bnxt: parse priority attribute for flow creation Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 07/20] net/bnxt: delete and flush L2 filters cleanly Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 08/20] net/bnxt: cleanup vnic after flow validate Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 09/20] net/bnxt: fix an issue in setting default MAC address Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 10/20] net/bnxt: allow only unicast MAC address filter creation Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 11/20] net/bnxt: properly handle ring cleanup in case of error Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 12/20] net/bnxt: check device is started before flow creation Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 13/20] net/bnxt: check for invalid VNIC ID in vnic tpa cfg Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 14/20] net/bnxt: validate RSS hash key length Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 15/20] net/bnxt: handle cleanup if flow creation fails Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 16/20] net/bnxt: synchronize between flow related functions Ajit Khaparde
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 17/20] net/bnxt: drop untagged frames when specified Ajit Khaparde
2019-10-03 13:17   ` Ferruh Yigit
2019-10-02 23:25 ` [dpdk-dev] [PATCH v2 18/20] net/bnxt: fix VLAN filtering code path Ajit Khaparde
2019-10-02 23:26 ` [dpdk-dev] [PATCH v2 19/20] net/bnxt: fix multicast filter programming Ajit Khaparde
2019-10-02 23:26 ` [dpdk-dev] [PATCH v2 20/20] net/bnxt: handle flow flush handling Ajit Khaparde
2019-10-03 12:39   ` Ferruh Yigit
2019-10-03 13:18 ` [dpdk-dev] [PATCH v2 00/20] bnxt patchset to improve rte flow support Ferruh Yigit
2019-10-03 14:29 ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).