DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/6] bnxt patchset
@ 2019-12-17  4:17 Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 1/6] net/bnxt: fix link failure during port toggle by increasing link wait time Somnath Kotur
                   ` (11 more replies)
  0 siblings, 12 replies; 25+ messages in thread
From: Somnath Kotur @ 2019-12-17  4:17 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Please apply.

Santoshkumar Karanappa Rastapur (2):
  net/bnxt: fix link failure during port toggle by increasing link wait
    time
  net/bnxt: fix non matching flow hitting filter rule

Somnath Kotur (4):
  net/bnxt: fix to use first valid profile if lossy profile not found
  net/bnxt: fix flow flush to sync with flow destroy routine
  net/bnxt: fix to keep the L2 filter intact so it can be reused
  net/bnxt: fix to free l2 filters while clearing vnic flows/filters

 drivers/net/bnxt/bnxt.h        |  10 ++-
 drivers/net/bnxt/bnxt_cpr.c    |   2 +-
 drivers/net/bnxt/bnxt_ethdev.c |  18 +++--
 drivers/net/bnxt/bnxt_filter.h |   4 +
 drivers/net/bnxt/bnxt_flow.c   | 163 ++++++++++++++++-------------------------
 drivers/net/bnxt/bnxt_hwrm.c   |  64 ++++++++++++----
 drivers/net/bnxt/bnxt_hwrm.h   |   3 +
 7 files changed, 143 insertions(+), 121 deletions(-)

-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 1/6] net/bnxt: fix link failure during port toggle by increasing link wait time
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
@ 2019-12-17  4:17 ` Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 2/6] net/bnxt: fix to use first valid profile if lossy profile not found Somnath Kotur
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Somnath Kotur @ 2019-12-17  4:17 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

From: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>

We need to wait for up to 500ms to receive async event notification after
forcing link down. Similarly we need to wait for up to 10s for link to
come up after configuring the hardware for link up.

Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        | 10 ++++++----
 drivers/net/bnxt/bnxt_cpr.c    |  2 +-
 drivers/net/bnxt/bnxt_ethdev.c | 18 +++++++++++++-----
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index bd8f660..257994d 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -231,9 +231,10 @@ struct bnxt_pf_info {
 	uint8_t			evb_mode;
 };
 
-/* Max wait time is 10 * 100ms = 1s */
-#define BNXT_LINK_WAIT_CNT	10
-#define BNXT_LINK_WAIT_INTERVAL	100
+/* Max wait time for link up is 10s and link down is 500ms */
+#define BNXT_LINK_UP_WAIT_CNT	200
+#define BNXT_LINK_DOWN_WAIT_CNT	10
+#define BNXT_LINK_WAIT_INTERVAL	50
 struct bnxt_link_info {
 	uint32_t		phy_flags;
 	uint8_t			mac_type;
@@ -657,7 +658,8 @@ struct bnxt {
 };
 
 int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete);
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+		     bool exp_link_status);
 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);
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 2c3129f..bb316b9 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -63,7 +63,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CHANGE:
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE:
 		/* FALLTHROUGH */
-		bnxt_link_update_op(bp->eth_dev, 0);
+		bnxt_link_update(bp->eth_dev, 0, ETH_LINK_UP);
 		break;
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD:
 		PMD_DRV_LOG(INFO, "Async event: PF driver unloaded\n");
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 64f88e9..36905c2 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -859,7 +859,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
 
-	bnxt_link_update_op(eth_dev, 1);
+	bnxt_link_update(eth_dev, 1, ETH_LINK_UP);
 
 	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
 		vlan_mask |= ETH_VLAN_FILTER_MASK;
@@ -943,7 +943,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
 	 * During reset recovery, there is no need to wait
 	 */
 	if (!is_bnxt_in_error(bp))
-		rte_delay_ms(BNXT_LINK_WAIT_INTERVAL * 2);
+		bnxt_link_update(eth_dev, 1, ETH_LINK_DOWN);
 
 	/* Clean queue intr-vector mapping */
 	rte_intr_efd_disable(intr_handle);
@@ -1094,12 +1094,14 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 	return rc;
 }
 
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+		     bool exp_link_status)
 {
 	int rc = 0;
 	struct bnxt *bp = eth_dev->data->dev_private;
 	struct rte_eth_link new;
-	unsigned int cnt = BNXT_LINK_WAIT_CNT;
+	int cnt = exp_link_status ? BNXT_LINK_UP_WAIT_CNT :
+		  BNXT_LINK_DOWN_WAIT_CNT;
 
 	rc = is_bnxt_in_error(bp);
 	if (rc)
@@ -1117,7 +1119,7 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
 			goto out;
 		}
 
-		if (!wait_to_complete || new.link_status)
+		if (!wait_to_complete || new.link_status == exp_link_status)
 			break;
 
 		rte_delay_ms(BNXT_LINK_WAIT_INTERVAL);
@@ -1139,6 +1141,12 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
 	return rc;
 }
 
+static int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
+			       int wait_to_complete)
+{
+	return bnxt_link_update(eth_dev, wait_to_complete, ETH_LINK_UP);
+}
+
 static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 2/6] net/bnxt: fix to use first valid profile if lossy profile not found
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 1/6] net/bnxt: fix link failure during port toggle by increasing link wait time Somnath Kotur
@ 2019-12-17  4:17 ` Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 3/6] net/bnxt: fix flow flush to sync with flow destroy routine Somnath Kotur
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Somnath Kotur @ 2019-12-17  4:17 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

In the case when CoS classification is disabled, driver was iterating
looking for only lossy profiles as that is what is expected to be used
for regular NIC operations. But in certain custom profiles, there were
no lossy profiles configured, only lossless profiles instead.
To handle such cases, it is better to fallback to using the first valid
profile.

Fixes: 698aa7e95 ("net/bnxt: add code to determine the Tx COS queue")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 44 ++++++++++++++++++++++++++++++++++++--------
 drivers/net/bnxt/bnxt_hwrm.h |  3 +++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 64dc78a..694d2d0 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1211,6 +1211,35 @@ static int bnxt_hwrm_port_phy_qcfg(struct bnxt *bp,
 	return rc;
 }
 
+static bool bnxt_find_lossy_profile(struct bnxt *bp)
+{
+	int i = 0;
+
+	for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+		if (bp->tx_cos_queue[i].profile ==
+		    HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
+			bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+			return true;
+		}
+	}
+	return false;
+}
+
+static void bnxt_find_first_valid_profile(struct bnxt *bp)
+{
+	int i = 0;
+
+	for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+		if (bp->tx_cos_queue[i].profile !=
+		    HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN &&
+		    bp->tx_cos_queue[i].id !=
+		    HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN) {
+			bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+			break;
+		}
+	}
+}
+
 int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
 {
 	int rc = 0;
@@ -1270,14 +1299,13 @@ int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
 						bp->tx_cos_queue[i].id;
 			}
 		} else {
-			for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
-				if (bp->tx_cos_queue[i].profile ==
-					HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
-					bp->tx_cosq_id[0] =
-						bp->tx_cos_queue[i].id;
-					break;
-				}
-			}
+			/* When CoS classification is disabled, for normal NIC
+			 * operations, ideally we should look to use LOSSY.
+			 * If not found, fallback to the first valid profile
+			 */
+			if (!bnxt_find_lossy_profile(bp))
+				bnxt_find_first_valid_profile(bp);
+
 		}
 	}
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index abe5de9..d8d1360 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -35,6 +35,9 @@
 #define HWRM_QUEUE_SERVICE_PROFILE_LOSSY \
 	HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_LOSSY
 
+#define HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN \
+	HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_UNKNOWN
+
 #define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MINIMAL_STATIC \
 	HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESERVATION_STRATEGY_MINIMAL_STATIC
 #define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MAXIMAL \
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 3/6] net/bnxt: fix flow flush to sync with flow destroy routine
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 1/6] net/bnxt: fix link failure during port toggle by increasing link wait time Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 2/6] net/bnxt: fix to use first valid profile if lossy profile not found Somnath Kotur
@ 2019-12-17  4:17 ` Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 4/6] net/bnxt: fix non matching flow hitting filter rule Somnath Kotur
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Somnath Kotur @ 2019-12-17  4:17 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Sync flow flush routine with flow destroy so that the operations
performed per flow during a flush are the same as that are done for an
individual flow destroy by having a common function to call for both.
One of the things that was missed in the flow flush routine was the
deletion of the L2 filter that would have been created as part of an
n-tuple filter.
Also, decrement the l2_ref_cnt for a filter in the case of a filter
update as it would've bumped up previously in validate_and_parse_flow()

Fixes: a0800839 ("net/bnxt: handle flow flush handling")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 132 +++++++++++++++----------------------------
 1 file changed, 46 insertions(+), 86 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 4381cd7..59e55c3 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1537,10 +1537,13 @@ struct bnxt_vnic_info *find_matching_vnic(struct bnxt *bp,
 	 * 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.
+	 * destination queue subsequently during bnxt_flow_create. So we
+	 * decrement the ref cnt of the L2 filter that would've been bumped
+	 * up previously in bnxt_validate_and_parse_flow as the old n-tuple
+	 * filter that was referencing it will be deleted now.
 	 */
+	bnxt_hwrm_clear_l2_filter(bp, old_filter);
 	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)
@@ -1828,46 +1831,24 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp,
 }
 
 static int
-bnxt_flow_destroy(struct rte_eth_dev *dev,
-		  struct rte_flow *flow,
-		  struct rte_flow_error *error)
+_bnxt_flow_destroy(struct bnxt *bp,
+		   struct rte_flow *flow,
+		    struct rte_flow_error *error)
 {
-	struct bnxt *bp = dev->data->dev_private;
 	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) {
-		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 &&
 	    filter->enables == filter->tunnel_type) {
-		ret = bnxt_handle_tunnel_redirect_destroy(bp,
-							  filter,
-							  error);
-		if (!ret) {
+		ret = bnxt_handle_tunnel_redirect_destroy(bp, filter, error);
+		if (!ret)
 			goto done;
-		} else {
-			bnxt_release_flow_lock(bp);
+		else
 			return ret;
-		}
 	}
 
 	ret = bnxt_match_filter(bp, filter);
@@ -1919,7 +1900,36 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp,
 				   "Failed to destroy flow.");
 	}
 
+	return ret;
+}
+
+static int
+bnxt_flow_destroy(struct rte_eth_dev *dev,
+		  struct rte_flow *flow,
+		  struct rte_flow_error *error)
+{
+	struct bnxt *bp = dev->data->dev_private;
+	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;
+	}
+
+	if (!flow->filter) {
+		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;
+	}
+	ret = _bnxt_flow_destroy(bp, flow, error);
 	bnxt_release_flow_lock(bp);
+
 	return ret;
 }
 
@@ -1927,7 +1937,6 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp,
 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;
@@ -1941,66 +1950,17 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp,
 
 		while (!STAILQ_EMPTY(&vnic->flow_list)) {
 			flow = STAILQ_FIRST(&vnic->flow_list);
-			filter = flow->filter;
-
-			if (filter->filter_type ==
-			    HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
-			    filter->enables == filter->tunnel_type) {
-				ret =
-				bnxt_handle_tunnel_redirect_destroy(bp,
-								    filter,
-								    error);
-				if (!ret) {
-					goto done;
-				} else {
-					bnxt_release_flow_lock(bp);
-					return ret;
-				}
-			}
-
-			if (filter->filter_type == HWRM_CFA_EM_FILTER)
-				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
-					(error,
-					 -ret,
-					 RTE_FLOW_ERROR_TYPE_HANDLE,
-					 NULL,
-					 "Failed to flush flow in HW.");
-				bnxt_release_flow_lock(bp);
-				return -rte_errno;
-			}
-done:
-			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 (!flow->filter)
+				continue;
 
-			/* 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;
-			}
+			ret = _bnxt_flow_destroy(bp, flow, error);
+			if (ret)
+				break;
 		}
 	}
-
 	bnxt_release_flow_lock(bp);
+
 	return ret;
 }
 
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 4/6] net/bnxt: fix non matching flow hitting filter rule
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (2 preceding siblings ...)
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 3/6] net/bnxt: fix flow flush to sync with flow destroy routine Somnath Kotur
@ 2019-12-17  4:17 ` Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 5/6] net/bnxt: fix to keep the L2 filter intact so it can be reused Somnath Kotur
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Somnath Kotur @ 2019-12-17  4:17 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

From: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>

As part of ntuple filter, we were creating L2 filter with the ntuple
redirect queue resulting in any L2 matching flow getting steered to
this queue. For ntuple filters, we need to create the L2 filter with
default queue. The user specified redirect queue will be set while
creating the ntuple filter in hardware.

Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 59e55c3..f926f9b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1128,7 +1128,16 @@ static int match_vnic_rss_cfg(struct bnxt *bp,
 		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);
+
+		/* For ntuple filter, create the L2 filter with default VNIC.
+		 * The user specified redirect queue will be set while creating
+		 * the ntuple filter in hardware.
+		 */
+		vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
+		if (use_ntuple)
+			filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
+		else
+			filter1 = bnxt_get_l2_filter(bp, filter, vnic);
 		if (filter1 == NULL) {
 			rte_flow_error_set(error,
 					   ENOSPC,
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 5/6] net/bnxt: fix to keep the L2 filter intact so it can be reused
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (3 preceding siblings ...)
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 4/6] net/bnxt: fix non matching flow hitting filter rule Somnath Kotur
@ 2019-12-17  4:17 ` Somnath Kotur
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 6/6] net/bnxt: fix to free l2 filters while clearing vnic flows/filters Somnath Kotur
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Somnath Kotur @ 2019-12-17  4:17 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

The software L2 filter was being released back to the free pool,
after it was created in HW and the filter corresponding to an actual
'flow' would have reference to the HW L2 filter.
But if this 'flow were to be deleted, then this HW L2 filter also
would be gone.
Fix this by storing the L2 filter created originally either for an
n-tuple flow or otherwise as part of the vnic's filter list.
This would require the filter_info struct to have a backptr to the
vnic which it came from.

Fixes: 0ba82dee ("net/bnxt: fix to check for L2 filters that doesn't have any references")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.h |  4 ++++
 drivers/net/bnxt/bnxt_flow.c   | 20 +++++++++-----------
 drivers/net/bnxt/bnxt_hwrm.c   | 14 +++++++++++++-
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 8cafa8f..5c96b34 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -80,6 +80,10 @@ struct bnxt_filter_info {
 	uint16_t                ethertype;
 	uint32_t		priority;
 	uint32_t		mark;
+	/* Backptr to vnic. As of now, used only by an L2 filter
+	 * to remember which vnic it was created on
+	 */
+	struct			bnxt_vnic_info *vnic;
 };
 
 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 f926f9b..660760c 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -746,10 +746,9 @@
 {
 	struct bnxt_filter_info *mf, *f0;
 	struct bnxt_vnic_info *vnic0;
-	struct rte_flow *flow;
 	int i;
 
-	vnic0 = &bp->vnic_info[0];
+	vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
 	f0 = STAILQ_FIRST(&vnic0->filter);
 
 	/* This flow has same DST MAC as the port/l2 filter. */
@@ -762,8 +761,7 @@
 		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
 			continue;
 
-		STAILQ_FOREACH(flow, &vnic->flow_list, next) {
-			mf = flow->filter;
+		STAILQ_FOREACH(mf, &vnic->filter, next) {
 
 			if (mf->matching_l2_fltr_ptr)
 				continue;
@@ -798,6 +796,8 @@
 	if (filter1 == NULL)
 		return NULL;
 
+	memcpy(filter1, nf, sizeof(*filter1));
+
 	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 ||
@@ -880,11 +880,14 @@ struct bnxt_filter_info *
 	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;
+		if (l2_filter) {
+			STAILQ_INSERT_TAIL(&vnic->filter, l2_filter, next);
+			l2_filter->vnic = vnic;
+		}
 	}
+	nf->matching_l2_fltr_ptr = l2_filter;
 
 	return l2_filter;
 }
@@ -1437,11 +1440,6 @@ static int match_vnic_rss_cfg(struct bnxt *bp,
 		goto ret;
 	}
 
-	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);
 	while (act->type != RTE_FLOW_ACTION_TYPE_END)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 694d2d0..1b19fba 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -363,10 +363,11 @@ int bnxt_hwrm_cfa_vlan_antispoof_cfg(struct bnxt *bp, uint16_t fid,
 }
 
 int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
-			   struct bnxt_filter_info *filter)
+			     struct bnxt_filter_info *filter)
 {
 	int rc = 0;
 	struct bnxt_filter_info *l2_filter = filter;
+	struct bnxt_vnic_info *vnic = NULL;
 	struct hwrm_cfa_l2_filter_free_input req = {.req_type = 0 };
 	struct hwrm_cfa_l2_filter_free_output *resp = bp->hwrm_cmd_resp_addr;
 
@@ -379,6 +380,9 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
 	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)
+		return 0;
+
 	if (l2_filter->l2_ref_cnt > 0)
 		l2_filter->l2_ref_cnt--;
 
@@ -395,6 +399,14 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
 	HWRM_UNLOCK();
 
 	filter->fw_l2_filter_id = UINT64_MAX;
+	if (l2_filter->l2_ref_cnt == 0) {
+		vnic = l2_filter->vnic;
+		if (vnic) {
+			STAILQ_REMOVE(&vnic->filter, l2_filter,
+				      bnxt_filter_info, next);
+			bnxt_free_filter(bp, l2_filter);
+		}
+	}
 
 	return 0;
 }
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 6/6] net/bnxt: fix to free l2 filters while clearing vnic flows/filters
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (4 preceding siblings ...)
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 5/6] net/bnxt: fix to keep the L2 filter intact so it can be reused Somnath Kotur
@ 2019-12-17  4:17 ` Somnath Kotur
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 0/5] bnxt patchset Ajit Khaparde
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Somnath Kotur @ 2019-12-17  4:17 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Now that L2 filters can be re-used for an n-tuple filter(s), delete
L2 filter as well so the reference count of an L2 filter (if reused)
can be decremented appropriately.

Fixes: 0ba82dee ("net/bnxt: fix to check for L2 filters that doesn't have any references")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 1b19fba..50272dc 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2496,8 +2496,7 @@ int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 			rc = bnxt_hwrm_clear_em_filter(bp, filter);
 		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-		else
-			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
+		rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
 		bnxt_free_filter(bp, filter);
 	}
@@ -2519,8 +2518,7 @@ int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 			rc = bnxt_hwrm_clear_em_filter(bp, filter);
 		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-		else
-			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
+		rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 
 		STAILQ_REMOVE(&vnic->flow_list, flow, rte_flow, next);
 		rte_free(flow);
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v2 0/5] bnxt patchset
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (5 preceding siblings ...)
  2019-12-17  4:17 ` [dpdk-dev] [PATCH 6/6] net/bnxt: fix to free l2 filters while clearing vnic flows/filters Somnath Kotur
@ 2019-12-21  2:29 ` Ajit Khaparde
  2019-12-21  2:50   ` Ajit Khaparde
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 1/5] net/bnxt: fix link failure during port toggle Ajit Khaparde
                   ` (4 subsequent siblings)
  11 siblings, 2 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-12-21  2:29 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Fixes for bnxt PMD.

v1->v2:
 - Updated Fixes tag and commit logs
 - Rebased patch 4,5
 - Squashed patch 5 and 6

Santoshkumar Karanappa Rastapur (2):
  net/bnxt: fix link failure during port toggle
  net/bnxt: fix non matching flow hitting filter rule

Somnath Kotur (3):
  net/bnxt: fix to use first valid profile
  net/bnxt: fix flow flush to sync with flow destroy
  net/bnxt: fix to reuse an L2 filter

 drivers/net/bnxt/bnxt.h        |  10 +-
 drivers/net/bnxt/bnxt_cpr.c    |   2 +-
 drivers/net/bnxt/bnxt_ethdev.c |  18 +++-
 drivers/net/bnxt/bnxt_filter.h |   4 +
 drivers/net/bnxt/bnxt_flow.c   | 163 +++++++++++++--------------------
 drivers/net/bnxt/bnxt_hwrm.c   |  64 ++++++++++---
 drivers/net/bnxt/bnxt_hwrm.h   |   3 +
 7 files changed, 143 insertions(+), 121 deletions(-)

-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v2 1/5] net/bnxt: fix link failure during port toggle
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (6 preceding siblings ...)
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 0/5] bnxt patchset Ajit Khaparde
@ 2019-12-21  2:29 ` Ajit Khaparde
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 2/5] net/bnxt: fix to use first valid profile Ajit Khaparde
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-12-21  2:29 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Santoshkumar Karanappa Rastapur, stable, Somnath Kotur

From: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>

We need to wait for up to 500ms to receive async event notification after
forcing link down. Similarly we need to wait for up to 10s for link to
come up after configuring the hardware for link up.

Fixes: c09f57b49c13 ("net/bnxt: add start/stop/link update operations")
Cc: stable@dpdk.org

Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        | 10 ++++++----
 drivers/net/bnxt/bnxt_cpr.c    |  2 +-
 drivers/net/bnxt/bnxt_ethdev.c | 18 +++++++++++++-----
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index ab18e8acd..ab0b8dde1 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -231,9 +231,10 @@ struct bnxt_pf_info {
 	uint8_t			evb_mode;
 };
 
-/* Max wait time is 10 * 100ms = 1s */
-#define BNXT_LINK_WAIT_CNT	10
-#define BNXT_LINK_WAIT_INTERVAL	100
+/* Max wait time for link up is 10s and link down is 500ms */
+#define BNXT_LINK_UP_WAIT_CNT	200
+#define BNXT_LINK_DOWN_WAIT_CNT	10
+#define BNXT_LINK_WAIT_INTERVAL	50
 struct bnxt_link_info {
 	uint32_t		phy_flags;
 	uint8_t			mac_type;
@@ -656,7 +657,8 @@ struct bnxt {
 };
 
 int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete);
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+		     bool exp_link_status);
 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);
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 2c3129fe2..bb316b9e0 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -63,7 +63,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CHANGE:
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE:
 		/* FALLTHROUGH */
-		bnxt_link_update_op(bp->eth_dev, 0);
+		bnxt_link_update(bp->eth_dev, 0, ETH_LINK_UP);
 		break;
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD:
 		PMD_DRV_LOG(INFO, "Async event: PF driver unloaded\n");
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 1b4ed293d..88df82b86 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -856,7 +856,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
 
-	bnxt_link_update_op(eth_dev, 1);
+	bnxt_link_update(eth_dev, 1, ETH_LINK_UP);
 
 	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
 		vlan_mask |= ETH_VLAN_FILTER_MASK;
@@ -940,7 +940,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
 	 * During reset recovery, there is no need to wait
 	 */
 	if (!is_bnxt_in_error(bp))
-		rte_delay_ms(BNXT_LINK_WAIT_INTERVAL * 2);
+		bnxt_link_update(eth_dev, 1, ETH_LINK_DOWN);
 
 	/* Clean queue intr-vector mapping */
 	rte_intr_efd_disable(intr_handle);
@@ -1086,12 +1086,14 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 	return rc;
 }
 
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+		     bool exp_link_status)
 {
 	int rc = 0;
 	struct bnxt *bp = eth_dev->data->dev_private;
 	struct rte_eth_link new;
-	unsigned int cnt = BNXT_LINK_WAIT_CNT;
+	int cnt = exp_link_status ? BNXT_LINK_UP_WAIT_CNT :
+		  BNXT_LINK_DOWN_WAIT_CNT;
 
 	rc = is_bnxt_in_error(bp);
 	if (rc)
@@ -1109,7 +1111,7 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
 			goto out;
 		}
 
-		if (!wait_to_complete || new.link_status)
+		if (!wait_to_complete || new.link_status == exp_link_status)
 			break;
 
 		rte_delay_ms(BNXT_LINK_WAIT_INTERVAL);
@@ -1131,6 +1133,12 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
 	return rc;
 }
 
+static int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
+			       int wait_to_complete)
+{
+	return bnxt_link_update(eth_dev, wait_to_complete, ETH_LINK_UP);
+}
+
 static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v2 2/5] net/bnxt: fix to use first valid profile
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (7 preceding siblings ...)
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 1/5] net/bnxt: fix link failure during port toggle Ajit Khaparde
@ 2019-12-21  2:29 ` Ajit Khaparde
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 3/5] net/bnxt: fix flow flush to sync with flow destroy Ajit Khaparde
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-12-21  2:29 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable

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

In the case when CoS classification is disabled, driver was iterating
looking for only lossy profiles as that is what is expected to be used
for regular NIC operations. But in certain custom profiles, there were
no lossy profiles configured, only lossless profiles instead.
To handle such cases, it is better to fallback to using the first valid
profile.

Fixes: 698aa7e95325 ("net/bnxt: add code to determine the Tx COS queue")
Cc: stable@dpdk.org

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 44 +++++++++++++++++++++++++++++-------
 drivers/net/bnxt/bnxt_hwrm.h |  3 +++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index d0dcd561c..bee4c154f 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1210,6 +1210,35 @@ static int bnxt_hwrm_port_phy_qcfg(struct bnxt *bp,
 	return rc;
 }
 
+static bool bnxt_find_lossy_profile(struct bnxt *bp)
+{
+	int i = 0;
+
+	for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+		if (bp->tx_cos_queue[i].profile ==
+		    HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
+			bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+			return true;
+		}
+	}
+	return false;
+}
+
+static void bnxt_find_first_valid_profile(struct bnxt *bp)
+{
+	int i = 0;
+
+	for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+		if (bp->tx_cos_queue[i].profile !=
+		    HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN &&
+		    bp->tx_cos_queue[i].id !=
+		    HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN) {
+			bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+			break;
+		}
+	}
+}
+
 int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
 {
 	int rc = 0;
@@ -1269,14 +1298,13 @@ int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
 						bp->tx_cos_queue[i].id;
 			}
 		} else {
-			for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
-				if (bp->tx_cos_queue[i].profile ==
-					HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
-					bp->tx_cosq_id[0] =
-						bp->tx_cos_queue[i].id;
-					break;
-				}
-			}
+			/* When CoS classification is disabled, for normal NIC
+			 * operations, ideally we should look to use LOSSY.
+			 * If not found, fallback to the first valid profile
+			 */
+			if (!bnxt_find_lossy_profile(bp))
+				bnxt_find_first_valid_profile(bp);
+
 		}
 	}
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index abe5de9db..d8d1360f9 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -35,6 +35,9 @@ struct bnxt_cp_ring_info;
 #define HWRM_QUEUE_SERVICE_PROFILE_LOSSY \
 	HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_LOSSY
 
+#define HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN \
+	HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_UNKNOWN
+
 #define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MINIMAL_STATIC \
 	HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESERVATION_STRATEGY_MINIMAL_STATIC
 #define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MAXIMAL \
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v2 3/5] net/bnxt: fix flow flush to sync with flow destroy
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (8 preceding siblings ...)
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 2/5] net/bnxt: fix to use first valid profile Ajit Khaparde
@ 2019-12-21  2:29 ` Ajit Khaparde
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 4/5] net/bnxt: fix non matching flow hitting filter rule Ajit Khaparde
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 5/5] net/bnxt: fix to reuse an L2 filter Ajit Khaparde
  11 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-12-21  2:29 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable, Santoshkumar Karanappa Rastapur

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

Sync flow flush routine with flow destroy so that the operations
performed per flow during a flush are the same as that are done for an
individual flow destroy by having a common function to call for both.
One of the things that was missed in the flow flush routine was the
deletion of the L2 filter that would have been created as part of an
n-tuple filter.
Also, decrement the l2_ref_cnt for a filter in the case of a filter
update as it would've bumped up previously in validate_and_parse_flow()

Fixes: 89278c59d97c58 ("net/bnxt: fix flow flush handling")
Cc: stable@dpdk.org

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 132 ++++++++++++-----------------------
 1 file changed, 46 insertions(+), 86 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 7bd6811f1..7343b7e7b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1536,10 +1536,13 @@ bnxt_update_filter(struct bnxt *bp, struct bnxt_filter_info *old_filter,
 	 * 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.
+	 * destination queue subsequently during bnxt_flow_create. So we
+	 * decrement the ref cnt of the L2 filter that would've been bumped
+	 * up previously in bnxt_validate_and_parse_flow as the old n-tuple
+	 * filter that was referencing it will be deleted now.
 	 */
+	bnxt_hwrm_clear_l2_filter(bp, old_filter);
 	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)
@@ -1816,46 +1819,24 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp,
 }
 
 static int
-bnxt_flow_destroy(struct rte_eth_dev *dev,
-		  struct rte_flow *flow,
-		  struct rte_flow_error *error)
+_bnxt_flow_destroy(struct bnxt *bp,
+		   struct rte_flow *flow,
+		    struct rte_flow_error *error)
 {
-	struct bnxt *bp = dev->data->dev_private;
 	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) {
-		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 &&
 	    filter->enables == filter->tunnel_type) {
-		ret = bnxt_handle_tunnel_redirect_destroy(bp,
-							  filter,
-							  error);
-		if (!ret) {
+		ret = bnxt_handle_tunnel_redirect_destroy(bp, filter, error);
+		if (!ret)
 			goto done;
-		} else {
-			bnxt_release_flow_lock(bp);
+		else
 			return ret;
-		}
 	}
 
 	ret = bnxt_match_filter(bp, filter);
@@ -1902,7 +1883,36 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 				   "Failed to destroy flow.");
 	}
 
+	return ret;
+}
+
+static int
+bnxt_flow_destroy(struct rte_eth_dev *dev,
+		  struct rte_flow *flow,
+		  struct rte_flow_error *error)
+{
+	struct bnxt *bp = dev->data->dev_private;
+	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;
+	}
+
+	if (!flow->filter) {
+		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;
+	}
+	ret = _bnxt_flow_destroy(bp, flow, error);
 	bnxt_release_flow_lock(bp);
+
 	return ret;
 }
 
@@ -1910,7 +1920,6 @@ 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;
@@ -1924,66 +1933,17 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 
 		while (!STAILQ_EMPTY(&vnic->flow_list)) {
 			flow = STAILQ_FIRST(&vnic->flow_list);
-			filter = flow->filter;
-
-			if (filter->filter_type ==
-			    HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
-			    filter->enables == filter->tunnel_type) {
-				ret =
-				bnxt_handle_tunnel_redirect_destroy(bp,
-								    filter,
-								    error);
-				if (!ret) {
-					goto done;
-				} else {
-					bnxt_release_flow_lock(bp);
-					return ret;
-				}
-			}
-
-			if (filter->filter_type == HWRM_CFA_EM_FILTER)
-				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
-					(error,
-					 -ret,
-					 RTE_FLOW_ERROR_TYPE_HANDLE,
-					 NULL,
-					 "Failed to flush flow in HW.");
-				bnxt_release_flow_lock(bp);
-				return -rte_errno;
-			}
-done:
-			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 (!flow->filter)
+				continue;
 
-			/* 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;
-			}
+			ret = _bnxt_flow_destroy(bp, flow, error);
+			if (ret)
+				break;
 		}
 	}
-
 	bnxt_release_flow_lock(bp);
+
 	return ret;
 }
 
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v2 4/5] net/bnxt: fix non matching flow hitting filter rule
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (9 preceding siblings ...)
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 3/5] net/bnxt: fix flow flush to sync with flow destroy Ajit Khaparde
@ 2019-12-21  2:29 ` Ajit Khaparde
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 5/5] net/bnxt: fix to reuse an L2 filter Ajit Khaparde
  11 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-12-21  2:29 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Santoshkumar Karanappa Rastapur, stable, Somnath Kotur

From: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>

As part of ntuple filter, we were creating L2 filter with the ntuple
redirect queue resulting in any L2 matching flow getting steered to
this queue. For ntuple filters, we need to create the L2 filter with
default queue. The user specified redirect queue will be set while
creating the ntuple filter in hardware.

Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Cc: stable@dpdk.org

Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 7343b7e7b..855994a6b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1126,7 +1126,16 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		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);
+
+		/* For ntuple filter, create the L2 filter with default VNIC.
+		 * The user specified redirect queue will be set while creating
+		 * the ntuple filter in hardware.
+		 */
+		vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
+		if (use_ntuple)
+			filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
+		else
+			filter1 = bnxt_get_l2_filter(bp, filter, vnic);
 		if (filter1 == NULL) {
 			rte_flow_error_set(error,
 					   ENOSPC,
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v2 5/5] net/bnxt: fix to reuse an L2 filter
  2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
                   ` (10 preceding siblings ...)
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 4/5] net/bnxt: fix non matching flow hitting filter rule Ajit Khaparde
@ 2019-12-21  2:29 ` Ajit Khaparde
  11 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-12-21  2:29 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable

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

The software L2 filter was being released back to the free pool,
though it was created in HW and the filter corresponding to an actual
'flow' would have reference to the HW L2 filter.
But if this 'flow were to be deleted, then this HW L2 filter also
would be gone.
Fix this by storing the L2 filter created originally either for an
n-tuple flow or otherwise as part of the vnic's filter list.
This would require the filter_info struct to have a backptr to the
vnic which it came from.
Now that L2 filters can be re-used for an n-tuple filter(s), delete
L2 filter as well so the reference count of an L2 filter (if reused)
can be decremented appropriately.

Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Cc: stable@dpdk.org

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.h |  4 ++++
 drivers/net/bnxt/bnxt_flow.c   | 20 +++++++++-----------
 drivers/net/bnxt/bnxt_hwrm.c   | 20 +++++++++++++++-----
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 9db3e7487..fc40f112b 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -77,6 +77,10 @@ struct bnxt_filter_info {
 	uint16_t                ip_addr_type;
 	uint16_t                ethertype;
 	uint32_t		priority;
+	/* Backptr to vnic. As of now, used only by an L2 filter
+	 * to remember which vnic it was created on
+	 */
+	struct			bnxt_vnic_info *vnic;
 };
 
 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 855994a6b..2bd492ea3 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -746,10 +746,9 @@ bnxt_find_matching_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 {
 	struct bnxt_filter_info *mf, *f0;
 	struct bnxt_vnic_info *vnic0;
-	struct rte_flow *flow;
 	int i;
 
-	vnic0 = &bp->vnic_info[0];
+	vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
 	f0 = STAILQ_FIRST(&vnic0->filter);
 
 	/* This flow has same DST MAC as the port/l2 filter. */
@@ -762,8 +761,7 @@ bnxt_find_matching_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
 			continue;
 
-		STAILQ_FOREACH(flow, &vnic->flow_list, next) {
-			mf = flow->filter;
+		STAILQ_FOREACH(mf, &vnic->filter, next) {
 
 			if (mf->matching_l2_fltr_ptr)
 				continue;
@@ -798,6 +796,8 @@ bnxt_create_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 	if (filter1 == NULL)
 		return NULL;
 
+	memcpy(filter1, nf, sizeof(*filter1));
+
 	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 ||
@@ -880,11 +880,14 @@ bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 	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;
+		if (l2_filter) {
+			STAILQ_INSERT_TAIL(&vnic->filter, l2_filter, next);
+			l2_filter->vnic = vnic;
+		}
 	}
+	nf->matching_l2_fltr_ptr = l2_filter;
 
 	return l2_filter;
 }
@@ -1429,11 +1432,6 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		goto ret;
 	}
 
-	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) {
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index bee4c154f..8178213e5 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -363,10 +363,11 @@ int bnxt_hwrm_cfa_vlan_antispoof_cfg(struct bnxt *bp, uint16_t fid,
 }
 
 int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
-			   struct bnxt_filter_info *filter)
+			     struct bnxt_filter_info *filter)
 {
 	int rc = 0;
 	struct bnxt_filter_info *l2_filter = filter;
+	struct bnxt_vnic_info *vnic = NULL;
 	struct hwrm_cfa_l2_filter_free_input req = {.req_type = 0 };
 	struct hwrm_cfa_l2_filter_free_output *resp = bp->hwrm_cmd_resp_addr;
 
@@ -379,6 +380,9 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
 	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)
+		return 0;
+
 	if (l2_filter->l2_ref_cnt > 0)
 		l2_filter->l2_ref_cnt--;
 
@@ -395,6 +399,14 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
 	HWRM_UNLOCK();
 
 	filter->fw_l2_filter_id = UINT64_MAX;
+	if (l2_filter->l2_ref_cnt == 0) {
+		vnic = l2_filter->vnic;
+		if (vnic) {
+			STAILQ_REMOVE(&vnic->filter, l2_filter,
+				      bnxt_filter_info, next);
+			bnxt_free_filter(bp, l2_filter);
+		}
+	}
 
 	return 0;
 }
@@ -2483,8 +2495,7 @@ int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 			rc = bnxt_hwrm_clear_em_filter(bp, filter);
 		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-		else
-			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
+		rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
 		bnxt_free_filter(bp, filter);
 	}
@@ -2506,8 +2517,7 @@ bnxt_clear_hwrm_vnic_flows(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 			rc = bnxt_hwrm_clear_em_filter(bp, filter);
 		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-		else
-			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
+		rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 
 		STAILQ_REMOVE(&vnic->flow_list, flow, rte_flow, next);
 		rte_free(flow);
-- 
2.21.0 (Apple Git-122.2)


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

* Re: [dpdk-dev] [PATCH v2 0/5] bnxt patchset
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 0/5] bnxt patchset Ajit Khaparde
@ 2019-12-21  2:50   ` Ajit Khaparde
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
  1 sibling, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2019-12-21  2:50 UTC (permalink / raw)
  To: dpdk-dev; +Cc: Ferruh Yigit

On Fri, Dec 20, 2019 at 6:31 PM Ajit Khaparde <ajit.khaparde@broadcom.com>
wrote:

> Fixes for bnxt PMD.
>
> v1->v2:
>  - Updated Fixes tag and commit logs
>  - Rebased patch 4,5
>  - Squashed patch 5 and 6
>
> Santoshkumar Karanappa Rastapur (2):
>   net/bnxt: fix link failure during port toggle
>   net/bnxt: fix non matching flow hitting filter rule
>
> Somnath Kotur (3):
>   net/bnxt: fix to use first valid profile
>   net/bnxt: fix flow flush to sync with flow destroy
>   net/bnxt: fix to reuse an L2 filter
>
>  drivers/net/bnxt/bnxt.h        |  10 +-
>  drivers/net/bnxt/bnxt_cpr.c    |   2 +-
>  drivers/net/bnxt/bnxt_ethdev.c |  18 +++-
>  drivers/net/bnxt/bnxt_filter.h |   4 +
>  drivers/net/bnxt/bnxt_flow.c   | 163 +++++++++++++--------------------
>  drivers/net/bnxt/bnxt_hwrm.c   |  64 ++++++++++---
>  drivers/net/bnxt/bnxt_hwrm.h   |   3 +
>  7 files changed, 143 insertions(+), 121 deletions(-)
>
Patchset applied to dpdk-next-net-brcm

Thanks

>
> --
> 2.21.0 (Apple Git-122.2)
>
>

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

* [dpdk-dev] [PATCH v3 0/7] bnxt patchset
  2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 0/5] bnxt patchset Ajit Khaparde
  2019-12-21  2:50   ` Ajit Khaparde
@ 2020-01-07  0:37   ` Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 1/7] net/bnxt: fix link failure during port toggle Ajit Khaparde
                       ` (7 more replies)
  1 sibling, 8 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

v2->v3:
 - Rebased patches against latest dpdk-next-net
 - sync commit logs

Ajit Khaparde (2):
  net/bnxt: add support for flow mark action
  net/bnxt: fix to not overwrite error message

Santoshkumar Karanappa Rastapur (2):
  net/bnxt: fix link failure during port toggle
  net/bnxt: fix non matching flow hitting filter rule

Somnath Kotur (3):
  net/bnxt: fix to use first valid profile
  net/bnxt: fix flow flush to sync with flow destroy
  net/bnxt: fix to reuse an L2 filter

 drivers/net/bnxt/bnxt.h        |  15 ++-
 drivers/net/bnxt/bnxt_cpr.c    |   2 +-
 drivers/net/bnxt/bnxt_ethdev.c |  34 ++++-
 drivers/net/bnxt/bnxt_filter.h |   7 +
 drivers/net/bnxt/bnxt_flow.c   | 233 +++++++++++++++++----------------
 drivers/net/bnxt/bnxt_hwrm.c   |  66 ++++++++--
 drivers/net/bnxt/bnxt_hwrm.h   |   3 +
 drivers/net/bnxt/bnxt_rxr.c    |  41 +++++-
 drivers/net/bnxt/bnxt_rxr.h    |  11 ++
 9 files changed, 277 insertions(+), 135 deletions(-)

-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 1/7] net/bnxt: fix link failure during port toggle
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
@ 2020-01-07  0:37     ` Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 2/7] net/bnxt: fix to use first valid profile Ajit Khaparde
                       ` (6 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Santoshkumar Karanappa Rastapur, stable, Somnath Kotur

From: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>

We need to wait for up to 500ms to receive async event notification after
forcing link down. Similarly we need to wait for up to 10s for link to
come up after configuring the hardware for link up.

Fixes: c09f57b49c13 ("net/bnxt: add start/stop/link update operations")
Cc: stable@dpdk.org

Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        | 10 ++++++----
 drivers/net/bnxt/bnxt_cpr.c    |  2 +-
 drivers/net/bnxt/bnxt_ethdev.c | 18 +++++++++++++-----
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index ab18e8acd..ab0b8dde1 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -231,9 +231,10 @@ struct bnxt_pf_info {
 	uint8_t			evb_mode;
 };
 
-/* Max wait time is 10 * 100ms = 1s */
-#define BNXT_LINK_WAIT_CNT	10
-#define BNXT_LINK_WAIT_INTERVAL	100
+/* Max wait time for link up is 10s and link down is 500ms */
+#define BNXT_LINK_UP_WAIT_CNT	200
+#define BNXT_LINK_DOWN_WAIT_CNT	10
+#define BNXT_LINK_WAIT_INTERVAL	50
 struct bnxt_link_info {
 	uint32_t		phy_flags;
 	uint8_t			mac_type;
@@ -656,7 +657,8 @@ struct bnxt {
 };
 
 int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete);
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+		     bool exp_link_status);
 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);
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 2c3129fe2..bb316b9e0 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -63,7 +63,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CHANGE:
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE:
 		/* FALLTHROUGH */
-		bnxt_link_update_op(bp->eth_dev, 0);
+		bnxt_link_update(bp->eth_dev, 0, ETH_LINK_UP);
 		break;
 	case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD:
 		PMD_DRV_LOG(INFO, "Async event: PF driver unloaded\n");
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 1b4ed293d..88df82b86 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -856,7 +856,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
 
-	bnxt_link_update_op(eth_dev, 1);
+	bnxt_link_update(eth_dev, 1, ETH_LINK_UP);
 
 	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
 		vlan_mask |= ETH_VLAN_FILTER_MASK;
@@ -940,7 +940,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
 	 * During reset recovery, there is no need to wait
 	 */
 	if (!is_bnxt_in_error(bp))
-		rte_delay_ms(BNXT_LINK_WAIT_INTERVAL * 2);
+		bnxt_link_update(eth_dev, 1, ETH_LINK_DOWN);
 
 	/* Clean queue intr-vector mapping */
 	rte_intr_efd_disable(intr_handle);
@@ -1086,12 +1086,14 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 	return rc;
 }
 
-int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
+int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
+		     bool exp_link_status)
 {
 	int rc = 0;
 	struct bnxt *bp = eth_dev->data->dev_private;
 	struct rte_eth_link new;
-	unsigned int cnt = BNXT_LINK_WAIT_CNT;
+	int cnt = exp_link_status ? BNXT_LINK_UP_WAIT_CNT :
+		  BNXT_LINK_DOWN_WAIT_CNT;
 
 	rc = is_bnxt_in_error(bp);
 	if (rc)
@@ -1109,7 +1111,7 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
 			goto out;
 		}
 
-		if (!wait_to_complete || new.link_status)
+		if (!wait_to_complete || new.link_status == exp_link_status)
 			break;
 
 		rte_delay_ms(BNXT_LINK_WAIT_INTERVAL);
@@ -1131,6 +1133,12 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
 	return rc;
 }
 
+static int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
+			       int wait_to_complete)
+{
+	return bnxt_link_update(eth_dev, wait_to_complete, ETH_LINK_UP);
+}
+
 static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 2/7] net/bnxt: fix to use first valid profile
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 1/7] net/bnxt: fix link failure during port toggle Ajit Khaparde
@ 2020-01-07  0:37     ` Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 3/7] net/bnxt: fix flow flush to sync with flow destroy Ajit Khaparde
                       ` (5 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable

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

In the case when CoS classification is disabled, driver was iterating
looking for only lossy profiles as that is what is expected to be used
for regular NIC operations. But in certain custom profiles, there were
no lossy profiles configured, only lossless profiles instead.
To handle such cases, it is better to fallback to using the first valid
profile.

Fixes: 698aa7e95325 ("net/bnxt: add code to determine the Tx COS queue")
Cc: stable@dpdk.org

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 44 +++++++++++++++++++++++++++++-------
 drivers/net/bnxt/bnxt_hwrm.h |  3 +++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index d0dcd561c..bee4c154f 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1210,6 +1210,35 @@ static int bnxt_hwrm_port_phy_qcfg(struct bnxt *bp,
 	return rc;
 }
 
+static bool bnxt_find_lossy_profile(struct bnxt *bp)
+{
+	int i = 0;
+
+	for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+		if (bp->tx_cos_queue[i].profile ==
+		    HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
+			bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+			return true;
+		}
+	}
+	return false;
+}
+
+static void bnxt_find_first_valid_profile(struct bnxt *bp)
+{
+	int i = 0;
+
+	for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
+		if (bp->tx_cos_queue[i].profile !=
+		    HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN &&
+		    bp->tx_cos_queue[i].id !=
+		    HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN) {
+			bp->tx_cosq_id[0] = bp->tx_cos_queue[i].id;
+			break;
+		}
+	}
+}
+
 int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
 {
 	int rc = 0;
@@ -1269,14 +1298,13 @@ int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
 						bp->tx_cos_queue[i].id;
 			}
 		} else {
-			for (i = BNXT_COS_QUEUE_COUNT - 1; i >= 0; i--) {
-				if (bp->tx_cos_queue[i].profile ==
-					HWRM_QUEUE_SERVICE_PROFILE_LOSSY) {
-					bp->tx_cosq_id[0] =
-						bp->tx_cos_queue[i].id;
-					break;
-				}
-			}
+			/* When CoS classification is disabled, for normal NIC
+			 * operations, ideally we should look to use LOSSY.
+			 * If not found, fallback to the first valid profile
+			 */
+			if (!bnxt_find_lossy_profile(bp))
+				bnxt_find_first_valid_profile(bp);
+
 		}
 	}
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index abe5de9db..d8d1360f9 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -35,6 +35,9 @@ struct bnxt_cp_ring_info;
 #define HWRM_QUEUE_SERVICE_PROFILE_LOSSY \
 	HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_LOSSY
 
+#define HWRM_QUEUE_SERVICE_PROFILE_UNKNOWN \
+	HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_UNKNOWN
+
 #define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MINIMAL_STATIC \
 	HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESERVATION_STRATEGY_MINIMAL_STATIC
 #define HWRM_FUNC_RESOURCE_QCAPS_OUTPUT_VF_RESV_STRATEGY_MAXIMAL \
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 3/7] net/bnxt: fix flow flush to sync with flow destroy
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 1/7] net/bnxt: fix link failure during port toggle Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 2/7] net/bnxt: fix to use first valid profile Ajit Khaparde
@ 2020-01-07  0:37     ` Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 4/7] net/bnxt: fix non matching flow hitting filter rule Ajit Khaparde
                       ` (4 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable, Santoshkumar Karanappa Rastapur

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

Sync flow flush routine with flow destroy so that the operations
performed per flow during a flush are the same as that are done for an
individual flow destroy by having a common function to call for both.
One of the things that was missed in the flow flush routine was the
deletion of the L2 filter that would have been created as part of an
n-tuple filter.
Also, decrement the l2_ref_cnt for a filter in the case of a filter
update as it would've bumped up previously in validate_and_parse_flow()

Fixes: 89278c59d97c58 ("net/bnxt: fix flow flush handling")
Cc: stable@dpdk.org

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 132 ++++++++++++-----------------------
 1 file changed, 46 insertions(+), 86 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 7bd6811f1..7343b7e7b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1536,10 +1536,13 @@ bnxt_update_filter(struct bnxt *bp, struct bnxt_filter_info *old_filter,
 	 * 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.
+	 * destination queue subsequently during bnxt_flow_create. So we
+	 * decrement the ref cnt of the L2 filter that would've been bumped
+	 * up previously in bnxt_validate_and_parse_flow as the old n-tuple
+	 * filter that was referencing it will be deleted now.
 	 */
+	bnxt_hwrm_clear_l2_filter(bp, old_filter);
 	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)
@@ -1816,46 +1819,24 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp,
 }
 
 static int
-bnxt_flow_destroy(struct rte_eth_dev *dev,
-		  struct rte_flow *flow,
-		  struct rte_flow_error *error)
+_bnxt_flow_destroy(struct bnxt *bp,
+		   struct rte_flow *flow,
+		    struct rte_flow_error *error)
 {
-	struct bnxt *bp = dev->data->dev_private;
 	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) {
-		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 &&
 	    filter->enables == filter->tunnel_type) {
-		ret = bnxt_handle_tunnel_redirect_destroy(bp,
-							  filter,
-							  error);
-		if (!ret) {
+		ret = bnxt_handle_tunnel_redirect_destroy(bp, filter, error);
+		if (!ret)
 			goto done;
-		} else {
-			bnxt_release_flow_lock(bp);
+		else
 			return ret;
-		}
 	}
 
 	ret = bnxt_match_filter(bp, filter);
@@ -1902,7 +1883,36 @@ bnxt_flow_destroy(struct rte_eth_dev *dev,
 				   "Failed to destroy flow.");
 	}
 
+	return ret;
+}
+
+static int
+bnxt_flow_destroy(struct rte_eth_dev *dev,
+		  struct rte_flow *flow,
+		  struct rte_flow_error *error)
+{
+	struct bnxt *bp = dev->data->dev_private;
+	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;
+	}
+
+	if (!flow->filter) {
+		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;
+	}
+	ret = _bnxt_flow_destroy(bp, flow, error);
 	bnxt_release_flow_lock(bp);
+
 	return ret;
 }
 
@@ -1910,7 +1920,6 @@ 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;
@@ -1924,66 +1933,17 @@ bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 
 		while (!STAILQ_EMPTY(&vnic->flow_list)) {
 			flow = STAILQ_FIRST(&vnic->flow_list);
-			filter = flow->filter;
-
-			if (filter->filter_type ==
-			    HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
-			    filter->enables == filter->tunnel_type) {
-				ret =
-				bnxt_handle_tunnel_redirect_destroy(bp,
-								    filter,
-								    error);
-				if (!ret) {
-					goto done;
-				} else {
-					bnxt_release_flow_lock(bp);
-					return ret;
-				}
-			}
-
-			if (filter->filter_type == HWRM_CFA_EM_FILTER)
-				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
-					(error,
-					 -ret,
-					 RTE_FLOW_ERROR_TYPE_HANDLE,
-					 NULL,
-					 "Failed to flush flow in HW.");
-				bnxt_release_flow_lock(bp);
-				return -rte_errno;
-			}
-done:
-			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 (!flow->filter)
+				continue;
 
-			/* 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;
-			}
+			ret = _bnxt_flow_destroy(bp, flow, error);
+			if (ret)
+				break;
 		}
 	}
-
 	bnxt_release_flow_lock(bp);
+
 	return ret;
 }
 
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 4/7] net/bnxt: fix non matching flow hitting filter rule
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
                       ` (2 preceding siblings ...)
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 3/7] net/bnxt: fix flow flush to sync with flow destroy Ajit Khaparde
@ 2020-01-07  0:37     ` Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 5/7] net/bnxt: fix to reuse an L2 filter Ajit Khaparde
                       ` (3 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Santoshkumar Karanappa Rastapur, stable, Somnath Kotur

From: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>

As part of ntuple filter, we were creating L2 filter with the ntuple
redirect queue resulting in any L2 matching flow getting steered to
this queue. For ntuple filters, we need to create the L2 filter with
default queue. The user specified redirect queue will be set while
creating the ntuple filter in hardware.

Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Cc: stable@dpdk.org

Signed-off-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 7343b7e7b..855994a6b 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1126,7 +1126,16 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		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);
+
+		/* For ntuple filter, create the L2 filter with default VNIC.
+		 * The user specified redirect queue will be set while creating
+		 * the ntuple filter in hardware.
+		 */
+		vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
+		if (use_ntuple)
+			filter1 = bnxt_get_l2_filter(bp, filter, vnic0);
+		else
+			filter1 = bnxt_get_l2_filter(bp, filter, vnic);
 		if (filter1 == NULL) {
 			rte_flow_error_set(error,
 					   ENOSPC,
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 5/7] net/bnxt: fix to reuse an L2 filter
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
                       ` (3 preceding siblings ...)
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 4/7] net/bnxt: fix non matching flow hitting filter rule Ajit Khaparde
@ 2020-01-07  0:37     ` Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 6/7] net/bnxt: add support for flow mark action Ajit Khaparde
                       ` (2 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable

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

The software L2 filter was being released back to the free pool,
though it was created in HW and the filter corresponding to an actual
'flow' would have reference to the HW L2 filter.
But if this 'flow were to be deleted, then this HW L2 filter also
would be gone.
Fix this by storing the L2 filter created originally either for an
n-tuple flow or otherwise as part of the vnic's filter list.
This would require the filter_info struct to have a backptr to the
vnic which it came from.
Now that L2 filters can be re-used for an n-tuple filter(s), delete
L2 filter as well so the reference count of an L2 filter (if reused)
can be decremented appropriately.

Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Cc: stable@dpdk.org

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.h |  4 ++++
 drivers/net/bnxt/bnxt_flow.c   | 20 +++++++++-----------
 drivers/net/bnxt/bnxt_hwrm.c   | 20 +++++++++++++++-----
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index 9db3e7487..fc40f112b 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -77,6 +77,10 @@ struct bnxt_filter_info {
 	uint16_t                ip_addr_type;
 	uint16_t                ethertype;
 	uint32_t		priority;
+	/* Backptr to vnic. As of now, used only by an L2 filter
+	 * to remember which vnic it was created on
+	 */
+	struct			bnxt_vnic_info *vnic;
 };
 
 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 855994a6b..2bd492ea3 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -746,10 +746,9 @@ bnxt_find_matching_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 {
 	struct bnxt_filter_info *mf, *f0;
 	struct bnxt_vnic_info *vnic0;
-	struct rte_flow *flow;
 	int i;
 
-	vnic0 = &bp->vnic_info[0];
+	vnic0 = BNXT_GET_DEFAULT_VNIC(bp);
 	f0 = STAILQ_FIRST(&vnic0->filter);
 
 	/* This flow has same DST MAC as the port/l2 filter. */
@@ -762,8 +761,7 @@ bnxt_find_matching_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
 		if (vnic->fw_vnic_id == INVALID_VNIC_ID)
 			continue;
 
-		STAILQ_FOREACH(flow, &vnic->flow_list, next) {
-			mf = flow->filter;
+		STAILQ_FOREACH(mf, &vnic->filter, next) {
 
 			if (mf->matching_l2_fltr_ptr)
 				continue;
@@ -798,6 +796,8 @@ bnxt_create_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 	if (filter1 == NULL)
 		return NULL;
 
+	memcpy(filter1, nf, sizeof(*filter1));
+
 	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 ||
@@ -880,11 +880,14 @@ bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
 	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;
+		if (l2_filter) {
+			STAILQ_INSERT_TAIL(&vnic->filter, l2_filter, next);
+			l2_filter->vnic = vnic;
+		}
 	}
+	nf->matching_l2_fltr_ptr = l2_filter;
 
 	return l2_filter;
 }
@@ -1429,11 +1432,6 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		goto ret;
 	}
 
-	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) {
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index bee4c154f..8178213e5 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -363,10 +363,11 @@ int bnxt_hwrm_cfa_vlan_antispoof_cfg(struct bnxt *bp, uint16_t fid,
 }
 
 int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
-			   struct bnxt_filter_info *filter)
+			     struct bnxt_filter_info *filter)
 {
 	int rc = 0;
 	struct bnxt_filter_info *l2_filter = filter;
+	struct bnxt_vnic_info *vnic = NULL;
 	struct hwrm_cfa_l2_filter_free_input req = {.req_type = 0 };
 	struct hwrm_cfa_l2_filter_free_output *resp = bp->hwrm_cmd_resp_addr;
 
@@ -379,6 +380,9 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
 	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)
+		return 0;
+
 	if (l2_filter->l2_ref_cnt > 0)
 		l2_filter->l2_ref_cnt--;
 
@@ -395,6 +399,14 @@ int bnxt_hwrm_clear_l2_filter(struct bnxt *bp,
 	HWRM_UNLOCK();
 
 	filter->fw_l2_filter_id = UINT64_MAX;
+	if (l2_filter->l2_ref_cnt == 0) {
+		vnic = l2_filter->vnic;
+		if (vnic) {
+			STAILQ_REMOVE(&vnic->filter, l2_filter,
+				      bnxt_filter_info, next);
+			bnxt_free_filter(bp, l2_filter);
+		}
+	}
 
 	return 0;
 }
@@ -2483,8 +2495,7 @@ int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 			rc = bnxt_hwrm_clear_em_filter(bp, filter);
 		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-		else
-			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
+		rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
 		bnxt_free_filter(bp, filter);
 	}
@@ -2506,8 +2517,7 @@ bnxt_clear_hwrm_vnic_flows(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 			rc = bnxt_hwrm_clear_em_filter(bp, filter);
 		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			rc = bnxt_hwrm_clear_ntuple_filter(bp, filter);
-		else
-			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
+		rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 
 		STAILQ_REMOVE(&vnic->flow_list, flow, rte_flow, next);
 		rte_free(flow);
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 6/7] net/bnxt: add support for flow mark action
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
                       ` (4 preceding siblings ...)
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 5/7] net/bnxt: fix to reuse an L2 filter Ajit Khaparde
@ 2020-01-07  0:37     ` Ajit Khaparde
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 7/7] net/bnxt: fix to not overwrite error message Ajit Khaparde
  2020-01-07 23:04     ` [dpdk-dev] [PATCH v3 0/7] bnxt patchset Ajit Khaparde
  7 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Lance Richardson

Add support for RTE_FLOW_ACTION_TYPE_MARK.
Use the flow_id provided by FW during flow creation to lookup the
mark id provided by the application.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |  5 +++
 drivers/net/bnxt/bnxt_ethdev.c | 16 ++++++++-
 drivers/net/bnxt/bnxt_filter.h |  3 ++
 drivers/net/bnxt/bnxt_flow.c   | 66 ++++++++++++++++++++++++++++------
 drivers/net/bnxt/bnxt_hwrm.c   |  2 ++
 drivers/net/bnxt/bnxt_rxr.c    | 41 ++++++++++++++++++++-
 drivers/net/bnxt/bnxt_rxr.h    | 11 ++++++
 7 files changed, 132 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index ab0b8dde1..21ca059b8 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -515,6 +515,7 @@ struct bnxt {
 #define BNXT_FLAG_INIT_DONE			BIT(21)
 #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TS		BIT(22)
 #define BNXT_FLAG_ADV_FLOW_MGMT			BIT(23)
+#define BNXT_FLAG_RX_VECTOR_PKT_MODE		BIT(24)
 #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)
@@ -654,6 +655,10 @@ struct bnxt {
 
 	/* Struct to hold adapter error recovery related info */
 	struct bnxt_error_recovery_info *recovery_info;
+#define BNXT_MARK_TABLE_SZ	(sizeof(uint32_t)  * 64 * 1024)
+/* TCAM and EM should be 16-bit only. Other modes not supported. */
+#define BNXT_FLOW_ID_MASK	0x0000ffff
+	uint32_t		*mark_table;
 };
 
 int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 88df82b86..7b5df9ac1 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -458,6 +458,10 @@ static int bnxt_init_chip(struct bnxt *bp)
 	}
 	bnxt_print_link_info(bp->eth_dev);
 
+	bp->mark_table = rte_zmalloc("bnxt_mark_table", BNXT_MARK_TABLE_SZ, 0);
+	if (!bp->mark_table)
+		PMD_DRV_LOG(ERR, "Allocation of mark table failed\n");
+
 	return 0;
 
 err_free:
@@ -740,8 +744,10 @@ static int bnxt_scattered_rx(struct rte_eth_dev *eth_dev)
 }
 
 static eth_rx_burst_t
-bnxt_receive_function(__rte_unused struct rte_eth_dev *eth_dev)
+bnxt_receive_function(struct rte_eth_dev *eth_dev)
 {
+	struct bnxt *bp = eth_dev->data->dev_private;
+
 #ifdef RTE_ARCH_X86
 #ifndef RTE_LIBRTE_IEEE1588
 	/*
@@ -762,6 +768,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev *eth_dev)
 		DEV_RX_OFFLOAD_VLAN_FILTER))) {
 		PMD_DRV_LOG(INFO, "Using vector mode receive for port %d\n",
 			    eth_dev->data->port_id);
+		bp->flags |= BNXT_FLAG_RX_VECTOR_PKT_MODE;
 		return bnxt_recv_pkts_vec;
 	}
 	PMD_DRV_LOG(INFO, "Vector mode receive disabled for port %d\n",
@@ -773,6 +780,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev *eth_dev)
 		    eth_dev->data->dev_conf.rxmode.offloads);
 #endif
 #endif
+	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 	return bnxt_recv_pkts;
 }
 
@@ -956,6 +964,8 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
 	bnxt_int_handler(eth_dev);
 	bnxt_shutdown_nic(bp);
 	bnxt_hwrm_if_change(bp, 0);
+	memset(bp->mark_table, 0, BNXT_MARK_TABLE_SZ);
+	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 	bp->dev_stopped = 1;
 	bp->rx_cosq_cnt = 0;
 }
@@ -976,6 +986,9 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 		bp->grp_info = NULL;
 	}
 
+	rte_free(bp->mark_table);
+	bp->mark_table = NULL;
+
 	bnxt_dev_uninit(eth_dev);
 }
 
@@ -4775,6 +4788,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
 	bp = eth_dev->data->dev_private;
 
 	bp->dev_stopped = 1;
+	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 
 	if (bnxt_vf_pciid(pci_dev->id.device_id))
 		bp->flags |= BNXT_FLAG_VF;
diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index fc40f112b..8f8a4c13b 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -23,9 +23,11 @@ struct bnxt;
 #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)
+#define BNXT_FLOW_MARK_FLAG			BIT(7)
 
 struct bnxt_filter_info {
 	STAILQ_ENTRY(bnxt_filter_info)	next;
+	uint32_t		flow_id;
 	uint64_t		fw_l2_filter_id;
 	struct bnxt_filter_info *matching_l2_fltr_ptr;
 	uint64_t		fw_em_filter_id;
@@ -81,6 +83,7 @@ struct bnxt_filter_info {
 	 * to remember which vnic it was created on
 	 */
 	struct			bnxt_vnic_info *vnic;
+	uint32_t		mark;
 };
 
 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 2bd492ea3..707aedcec 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -994,6 +994,7 @@ bnxt_update_filter_flags_en(struct bnxt_filter_info *filter,
 	}
 	filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
 	filter->l2_ref_cnt = filter1->l2_ref_cnt;
+	filter->flow_id = filter1->flow_id;
 	PMD_DRV_LOG(DEBUG,
 		"l2_filter: %p fw_l2_filter_id %" PRIx64 " l2_ref_cnt %u\n",
 		filter1, filter->fw_l2_filter_id, filter->l2_ref_cnt);
@@ -1036,6 +1037,8 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		filter->flags = HWRM_CFA_EM_FLOW_ALLOC_INPUT_FLAGS_PATH_RX;
 
 	use_ntuple = bnxt_filter_type_check(pattern, error);
+
+start:
 	switch (act->type) {
 	case RTE_FLOW_ACTION_TYPE_QUEUE:
 		/* Allow this flow. Redirect to a VNIC. */
@@ -1191,6 +1194,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		}
 
 		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
+		filter->flow_id = filter1->flow_id;
 		filter->flags = HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_FLAGS_METER;
 		break;
 	case RTE_FLOW_ACTION_TYPE_VF:
@@ -1259,6 +1263,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		}
 
 		filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
+		filter->flow_id = filter1->flow_id;
 		break;
 	case RTE_FLOW_ACTION_TYPE_RSS:
 		rss = (const struct rte_flow_action_rss *)act->conf;
@@ -1422,6 +1427,34 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(DEBUG, "L2 filter created\n");
 		bnxt_update_filter_flags_en(filter, filter1, use_ntuple);
 		break;
+	case RTE_FLOW_ACTION_TYPE_MARK:
+		if (bp->flags & BNXT_FLAG_RX_VECTOR_PKT_MODE) {
+			PMD_DRV_LOG(DEBUG,
+				    "Disable vector processing for mark\n");
+			rte_flow_error_set(error,
+					   ENOTSUP,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "Disable vector processing for mark");
+			rc = -rte_errno;
+			goto ret;
+		}
+
+		if (bp->mark_table == NULL) {
+			rte_flow_error_set(error,
+					   ENOMEM,
+					   RTE_FLOW_ERROR_TYPE_ACTION,
+					   act,
+					   "Mark table not allocated.");
+			rc = -rte_errno;
+			goto ret;
+		}
+
+		filter->valid_flags |= BNXT_FLOW_MARK_FLAG;
+		filter->mark = ((const struct rte_flow_action_mark *)
+				act->conf)->id;
+		PMD_DRV_LOG(DEBUG, "Mark the flow %d\n", filter->mark);
+		break;
 	default:
 		rte_flow_error_set(error,
 				   EINVAL,
@@ -1434,20 +1467,17 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 
 done:
 	act = bnxt_flow_non_void_action(++act);
-	if (act->type != RTE_FLOW_ACTION_TYPE_END) {
-		rte_flow_error_set(error,
-				   EINVAL,
-				   RTE_FLOW_ERROR_TYPE_ACTION,
-				   act,
-				   "Invalid action.");
-		rc = -rte_errno;
-		goto ret;
-	}
+	while (act->type != RTE_FLOW_ACTION_TYPE_END)
+		goto start;
 
 	return rc;
 ret:
 
-	//TODO: Cleanup according to ACTION TYPE.
+	if (filter1) {
+		bnxt_hwrm_clear_l2_filter(bp, filter1);
+		bnxt_free_filter(bp, filter1);
+	}
+
 	if (rte_errno)  {
 		if (vnic && STAILQ_EMPTY(&vnic->filter))
 			vnic->rx_queue_cnt = 0;
@@ -1759,6 +1789,17 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 		STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 		PMD_DRV_LOG(DEBUG, "Successfully created flow.\n");
 		STAILQ_INSERT_TAIL(&vnic->flow_list, flow, next);
+		if (filter->valid_flags & BNXT_FLOW_MARK_FLAG) {
+			PMD_DRV_LOG(DEBUG,
+				    "Mark action: mark id 0x%x, flow id 0x%x\n",
+				    filter->mark, filter->flow_id);
+
+			/* TCAM and EM should be 16-bit only.
+			 * Other modes not supported.
+			 */
+			bp->mark_table[filter->flow_id & BNXT_FLOW_ID_MASK] =
+				filter->mark;
+		}
 		bnxt_release_flow_lock(bp);
 		return flow;
 	}
@@ -1850,6 +1891,11 @@ _bnxt_flow_destroy(struct bnxt *bp,
 	if (ret == 0)
 		PMD_DRV_LOG(ERR, "Could not find matching flow\n");
 
+	if (filter->valid_flags & BNXT_FLOW_MARK_FLAG) {
+		bp->mark_table[filter->flow_id & BNXT_FLOW_ID_MASK] = 0;
+		filter->flow_id = 0;
+	}
+
 	if (filter->filter_type == HWRM_CFA_EM_FILTER)
 		ret = bnxt_hwrm_clear_em_filter(bp, filter);
 	if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 8178213e5..50272dcf7 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -485,6 +485,7 @@ int bnxt_hwrm_set_l2_filter(struct bnxt *bp,
 	HWRM_CHECK_RESULT();
 
 	filter->fw_l2_filter_id = rte_le_to_cpu_64(resp->l2_filter_id);
+	filter->flow_id = rte_le_to_cpu_32(resp->flow_id);
 	HWRM_UNLOCK();
 
 	return rc;
@@ -4321,6 +4322,7 @@ int bnxt_hwrm_set_ntuple_filter(struct bnxt *bp,
 	HWRM_CHECK_RESULT();
 
 	filter->fw_ntuple_filter_id = rte_le_to_cpu_64(resp->ntuple_filter_id);
+	filter->flow_id = rte_le_to_cpu_32(resp->flow_id);
 	HWRM_UNLOCK();
 
 	return rc;
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 3b713c242..1ae0c3c38 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -489,7 +489,8 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 		mbuf->hash.rss = rxcmp->rss_hash;
 		mbuf->ol_flags |= PKT_RX_RSS_HASH;
 	} else {
-		mbuf->hash.fdir.id = rxcmp1->cfa_code;
+		mbuf->hash.fdir.id = bnxt_get_cfa_code_or_mark_id(rxq->bp,
+								  rxcmp1);
 		mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
 	}
 #ifdef RTE_LIBRTE_IEEE1588
@@ -895,3 +896,41 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
 
 	return 0;
 }
+
+uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp,
+				      struct rx_pkt_cmpl_hi *rxcmp1)
+{
+	uint32_t cfa_code = 0;
+	uint8_t meta_fmt =  0;
+	uint16_t flags2 = 0;
+	uint32_t meta =  0;
+
+	cfa_code = rte_le_to_cpu_16(rxcmp1->cfa_code);
+	if (!cfa_code)
+		return 0;
+
+	if (cfa_code && !bp->mark_table[cfa_code])
+		return cfa_code;
+
+	flags2 = rte_le_to_cpu_16(rxcmp1->flags2);
+	meta = rte_le_to_cpu_32(rxcmp1->metadata);
+	if (meta) {
+		meta >>= BNXT_RX_META_CFA_CODE_SHIFT;
+
+		/*
+		 * The flags field holds extra bits of info from [6:4]
+		 * which indicate if the flow is in TCAM or EM or EEM
+		 */
+		meta_fmt = (flags2 & BNXT_CFA_META_FMT_MASK) >>
+			   BNXT_CFA_META_FMT_SHFT;
+
+		/*
+		 * meta_fmt == 4 => 'b100 => 'b10x => EM.
+		 * meta_fmt == 5 => 'b101 => 'b10x => EM + VLAN
+		 * meta_fmt == 6 => 'b110 => 'b11x => EEM
+		 * meta_fmt == 7 => 'b111 => 'b11x => EEM + VLAN.
+		 */
+		meta_fmt >>= BNXT_CFA_META_FMT_EM_EEM_SHFT;
+	}
+	return bp->mark_table[cfa_code];
+}
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index 76bf88d70..bf860020c 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -226,4 +226,15 @@ uint16_t bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 int bnxt_rxq_vec_setup(struct bnxt_rx_queue *rxq);
 #endif
 
+uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp,
+				      struct rx_pkt_cmpl_hi *rxcmp1);
+#define BNXT_RX_META_CFA_CODE_SHIFT		19
+#define BNXT_CFA_CODE_META_SHIFT		16
+#define BNXT_RX_META_CFA_CODE_INT_ACT_REC_BIT	0x8000000
+#define BNXT_RX_META_CFA_CODE_EEM_BIT		0x4000000
+#define BNXT_CFA_META_FMT_MASK			0x70
+#define BNXT_CFA_META_FMT_SHFT			4
+#define BNXT_CFA_META_FMT_EM_EEM_SHFT		1
+#define BNXT_CFA_META_FMT_EEM			3
+
 #endif
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 7/7] net/bnxt: fix to not overwrite error message
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
                       ` (5 preceding siblings ...)
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 6/7] net/bnxt: add support for flow mark action Ajit Khaparde
@ 2020-01-07  0:37     ` Ajit Khaparde
  2020-01-07  9:18       ` Ferruh Yigit
  2020-01-07 23:04     ` [dpdk-dev] [PATCH v3 0/7] bnxt patchset Ajit Khaparde
  7 siblings, 1 reply; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07  0:37 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable, Lance Richardson

In some cases when flow creation fails, we  overwrite the specific
error message with a generic error message. This patch fixes it.

Fixes: d24610f7bfda ("net/bnxt: allow flow creation when RSS is enabled")
Cc: stable@dpdk.org

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 707aedcec..cde1fa41c 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1485,7 +1485,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 		if (rxq && !vnic->rx_queue_cnt)
 			rxq->vnic = &bp->vnic_info[0];
 	}
-	return rc;
+	return -rte_errno;
 }
 
 static
@@ -1815,7 +1815,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
+	else if (!rte_errno)
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to create flow.");
-- 
2.21.0 (Apple Git-122.2)


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

* Re: [dpdk-dev] [PATCH v3 7/7] net/bnxt: fix to not overwrite error message
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 7/7] net/bnxt: fix to not overwrite error message Ajit Khaparde
@ 2020-01-07  9:18       ` Ferruh Yigit
  2020-01-07 21:50         ` Ajit Khaparde
  0 siblings, 1 reply; 25+ messages in thread
From: Ferruh Yigit @ 2020-01-07  9:18 UTC (permalink / raw)
  To: Ajit Khaparde, dev; +Cc: stable, Lance Richardson

On 1/7/2020 12:37 AM, Ajit Khaparde wrote:
> In some cases when flow creation fails, we  overwrite the specific
> error message with a generic error message. This patch fixes it.
> 
> Fixes: d24610f7bfda ("net/bnxt: allow flow creation when RSS is enabled")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_flow.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
> index 707aedcec..cde1fa41c 100644
> --- a/drivers/net/bnxt/bnxt_flow.c
> +++ b/drivers/net/bnxt/bnxt_flow.c
> @@ -1485,7 +1485,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
>  		if (rxq && !vnic->rx_queue_cnt)
>  			rxq->vnic = &bp->vnic_info[0];
>  	}
> -	return rc;
> +	return -rte_errno;

The result will be same as far as I can see, you may get rid of the "rc =
-rte_errno;" assignments before "goto ret;" with this change if you want.

Also commit log seems describing the below change not this one ...

>  }
>  
>  static
> @@ -1815,7 +1815,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
> +	else if (!rte_errno)
>  		rte_flow_error_set(error, -ret,
>  				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>  				   "Failed to create flow.");
> 


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

* Re: [dpdk-dev] [PATCH v3 7/7] net/bnxt: fix to not overwrite error message
  2020-01-07  9:18       ` Ferruh Yigit
@ 2020-01-07 21:50         ` Ajit Khaparde
  0 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07 21:50 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dpdk-dev, dpdk stable, Lance Richardson

On Tue, Jan 7, 2020 at 1:18 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 1/7/2020 12:37 AM, Ajit Khaparde wrote:
> > In some cases when flow creation fails, we  overwrite the specific
> > error message with a generic error message. This patch fixes it.
> >
> > Fixes: d24610f7bfda ("net/bnxt: allow flow creation when RSS is enabled")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
> > ---
> >  drivers/net/bnxt/bnxt_flow.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
> > index 707aedcec..cde1fa41c 100644
> > --- a/drivers/net/bnxt/bnxt_flow.c
> > +++ b/drivers/net/bnxt/bnxt_flow.c
> > @@ -1485,7 +1485,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev
> *dev,
> >               if (rxq && !vnic->rx_queue_cnt)
> >                       rxq->vnic = &bp->vnic_info[0];
> >       }
> > -     return rc;
> > +     return -rte_errno;
>
> The result will be same as far as I can see, you may get rid of the "rc =
> -rte_errno;" assignments before "goto ret;" with this change if you want.
>
I had that change lined up for my next set. But I can send it early.


>
> Also commit log seems describing the below change not this one ...
>
> >  }
> >
> >  static
> > @@ -1815,7 +1815,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
> > +     else if (!rte_errno)
> >               rte_flow_error_set(error, -ret,
> >                                  RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> >                                  "Failed to create flow.");
> >
>
>

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

* Re: [dpdk-dev] [PATCH v3 0/7] bnxt patchset
  2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
                       ` (6 preceding siblings ...)
  2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 7/7] net/bnxt: fix to not overwrite error message Ajit Khaparde
@ 2020-01-07 23:04     ` Ajit Khaparde
  7 siblings, 0 replies; 25+ messages in thread
From: Ajit Khaparde @ 2020-01-07 23:04 UTC (permalink / raw)
  To: dpdk-dev; +Cc: Ferruh Yigit

On Mon, Jan 6, 2020 at 4:37 PM Ajit Khaparde <ajit.khaparde@broadcom.com>
wrote:

> v2->v3:
>  - Rebased patches against latest dpdk-next-net
>  - sync commit logs
>

Patchset applied to dpdk-next-net-brcm. Thanks

>
> Ajit Khaparde (2):
>   net/bnxt: add support for flow mark action
>   net/bnxt: fix to not overwrite error message
>
> Santoshkumar Karanappa Rastapur (2):
>   net/bnxt: fix link failure during port toggle
>   net/bnxt: fix non matching flow hitting filter rule
>
> Somnath Kotur (3):
>   net/bnxt: fix to use first valid profile
>   net/bnxt: fix flow flush to sync with flow destroy
>   net/bnxt: fix to reuse an L2 filter
>
>  drivers/net/bnxt/bnxt.h        |  15 ++-
>  drivers/net/bnxt/bnxt_cpr.c    |   2 +-
>  drivers/net/bnxt/bnxt_ethdev.c |  34 ++++-
>  drivers/net/bnxt/bnxt_filter.h |   7 +
>  drivers/net/bnxt/bnxt_flow.c   | 233 +++++++++++++++++----------------
>  drivers/net/bnxt/bnxt_hwrm.c   |  66 ++++++++--
>  drivers/net/bnxt/bnxt_hwrm.h   |   3 +
>  drivers/net/bnxt/bnxt_rxr.c    |  41 +++++-
>  drivers/net/bnxt/bnxt_rxr.h    |  11 ++
>  9 files changed, 277 insertions(+), 135 deletions(-)
>
> --
> 2.21.0 (Apple Git-122.2)
>
>

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

end of thread, other threads:[~2020-01-07 23:05 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17  4:17 [dpdk-dev] [PATCH 0/6] bnxt patchset Somnath Kotur
2019-12-17  4:17 ` [dpdk-dev] [PATCH 1/6] net/bnxt: fix link failure during port toggle by increasing link wait time Somnath Kotur
2019-12-17  4:17 ` [dpdk-dev] [PATCH 2/6] net/bnxt: fix to use first valid profile if lossy profile not found Somnath Kotur
2019-12-17  4:17 ` [dpdk-dev] [PATCH 3/6] net/bnxt: fix flow flush to sync with flow destroy routine Somnath Kotur
2019-12-17  4:17 ` [dpdk-dev] [PATCH 4/6] net/bnxt: fix non matching flow hitting filter rule Somnath Kotur
2019-12-17  4:17 ` [dpdk-dev] [PATCH 5/6] net/bnxt: fix to keep the L2 filter intact so it can be reused Somnath Kotur
2019-12-17  4:17 ` [dpdk-dev] [PATCH 6/6] net/bnxt: fix to free l2 filters while clearing vnic flows/filters Somnath Kotur
2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 0/5] bnxt patchset Ajit Khaparde
2019-12-21  2:50   ` Ajit Khaparde
2020-01-07  0:37   ` [dpdk-dev] [PATCH v3 0/7] " Ajit Khaparde
2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 1/7] net/bnxt: fix link failure during port toggle Ajit Khaparde
2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 2/7] net/bnxt: fix to use first valid profile Ajit Khaparde
2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 3/7] net/bnxt: fix flow flush to sync with flow destroy Ajit Khaparde
2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 4/7] net/bnxt: fix non matching flow hitting filter rule Ajit Khaparde
2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 5/7] net/bnxt: fix to reuse an L2 filter Ajit Khaparde
2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 6/7] net/bnxt: add support for flow mark action Ajit Khaparde
2020-01-07  0:37     ` [dpdk-dev] [PATCH v3 7/7] net/bnxt: fix to not overwrite error message Ajit Khaparde
2020-01-07  9:18       ` Ferruh Yigit
2020-01-07 21:50         ` Ajit Khaparde
2020-01-07 23:04     ` [dpdk-dev] [PATCH v3 0/7] bnxt patchset Ajit Khaparde
2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 1/5] net/bnxt: fix link failure during port toggle Ajit Khaparde
2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 2/5] net/bnxt: fix to use first valid profile Ajit Khaparde
2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 3/5] net/bnxt: fix flow flush to sync with flow destroy Ajit Khaparde
2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 4/5] net/bnxt: fix non matching flow hitting filter rule Ajit Khaparde
2019-12-21  2:29 ` [dpdk-dev] [PATCH v2 5/5] net/bnxt: fix to reuse an L2 filter Ajit Khaparde

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).