[v3,2/5] net/bnxt: fix redundant MAC address check
diff mbox series

Message ID 20191104225026.97569-3-ajit.khaparde@broadcom.com
State Accepted, archived
Delegated to: Ajit Khaparde
Headers show
Series
  • bnxt patchset with bug fixes
Related show

Checks

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

Commit Message

Ajit Khaparde Nov. 4, 2019, 10:50 p.m. UTC
From: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>

filter->mac_index is used to check, if a same mac is
already programmed. Hence, filter->dflt member is not
needed which is also used for mac addr redundancy check.

This patch fixes it by moving mac_index based redundant
check from bnxt_mac_addr_add_op to bnxt_add_mac_filter

Fixes: 6118503d8071 ("net/bnxt: fix VLAN filtering")

Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 107 ++++++++++++---------------------
 drivers/net/bnxt/bnxt_filter.c |   1 +
 drivers/net/bnxt/bnxt_filter.h |   7 ++-
 drivers/net/bnxt/bnxt_hwrm.c   |   9 +--
 drivers/net/bnxt/bnxt_rxq.c    |   2 +
 5 files changed, 51 insertions(+), 75 deletions(-)

Patch
diff mbox series

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7d9459f0a..0b0ce8785 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1018,8 +1018,7 @@  static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 				bnxt_hwrm_clear_l2_filter(bp, filter);
 				filter->mac_index = INVALID_MAC_INDEX;
 				memset(&filter->l2_addr, 0, RTE_ETHER_ADDR_LEN);
-				STAILQ_INSERT_TAIL(&bp->free_filter_list,
-						   filter, next);
+				bnxt_free_filter(bp, filter);
 			}
 			filter = temp_filter;
 		}
@@ -1027,19 +1026,21 @@  static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 }
 
 static int bnxt_add_mac_filter(struct bnxt *bp, struct bnxt_vnic_info *vnic,
-			       struct rte_ether_addr *mac_addr, uint32_t index)
+			       struct rte_ether_addr *mac_addr, uint32_t index,
+			       uint32_t pool)
 {
 	struct bnxt_filter_info *filter;
 	int rc = 0;
 
-	filter = STAILQ_FIRST(&vnic->filter);
-	/* During bnxt_mac_addr_add_op, default MAC is
-	 * already programmed, so skip it. But, when
-	 * hw-vlan-filter is turned OFF from ON, default
-	 * MAC filter should be restored
-	 */
-	if (index == 0 && filter->dflt)
-		return 0;
+	/* Attach requested MAC address to the new l2_filter */
+	STAILQ_FOREACH(filter, &vnic->filter, next) {
+		if (filter->mac_index == index) {
+			PMD_DRV_LOG(ERR,
+				    "MAC addr already existed for pool %d\n",
+				    pool);
+			return 0;
+		}
+	}
 
 	filter = bnxt_alloc_filter(bp);
 	if (!filter) {
@@ -1047,7 +1048,6 @@  static int bnxt_add_mac_filter(struct bnxt *bp, struct bnxt_vnic_info *vnic,
 		return -ENODEV;
 	}
 
-	filter->mac_index = index;
 	/* bnxt_alloc_filter copies default MAC to filter->l2_addr. So,
 	 * if the MAC that's been programmed now is a different one, then,
 	 * copy that addr to filter->l2_addr
@@ -1058,14 +1058,12 @@  static int bnxt_add_mac_filter(struct bnxt *bp, struct bnxt_vnic_info *vnic,
 
 	rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
 	if (!rc) {
-		if (filter->mac_index == 0) {
-			filter->dflt = true;
+		filter->mac_index = index;
+		if (filter->mac_index == 0)
 			STAILQ_INSERT_HEAD(&vnic->filter, filter, next);
-		} else {
+		else
 			STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
-		}
 	} else {
-		filter->mac_index = INVALID_MAC_INDEX;
 		memset(&filter->l2_addr, 0, RTE_ETHER_ADDR_LEN);
 		bnxt_free_filter(bp, filter);
 	}
@@ -1079,7 +1077,6 @@  static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
 	struct bnxt_vnic_info *vnic = &bp->vnic_info[pool];
-	struct bnxt_filter_info *filter;
 	int rc = 0;
 
 	rc = is_bnxt_in_error(bp);
@@ -1095,16 +1092,8 @@  static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 		PMD_DRV_LOG(ERR, "VNIC not found for pool %d!\n", pool);
 		return -EINVAL;
 	}
-	/* Attach requested MAC address to the new l2_filter */
-	STAILQ_FOREACH(filter, &vnic->filter, next) {
-		if (filter->mac_index == index) {
-			PMD_DRV_LOG(ERR,
-				"MAC addr already existed for pool %d\n", pool);
-			return 0;
-		}
-	}
 
-	rc = bnxt_add_mac_filter(bp, vnic, mac_addr, index);
+	rc = bnxt_add_mac_filter(bp, vnic, mac_addr, index, pool);
 
 	return rc;
 }
@@ -1726,33 +1715,22 @@  static int bnxt_del_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 	int rc = 0;
 	uint32_t chk = HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_IVLAN;
 
-	/* if VLAN exists && VLAN matches vlan_id
-	 *      remove the MAC+VLAN filter
-	 *      add a new MAC only filter
-	 * else
-	 *      VLAN filter doesn't exist, just skip and continue
-	 */
 	vnic = BNXT_GET_DEFAULT_VNIC(bp);
 	filter = STAILQ_FIRST(&vnic->filter);
 	while (filter) {
 		/* Search for this matching MAC+VLAN filter */
-		if ((filter->enables & chk) &&
-		    (filter->l2_ivlan == vlan_id &&
-		     filter->l2_ivlan_mask != 0) &&
-		    !memcmp(filter->l2_addr, bp->mac_addr,
-			    RTE_ETHER_ADDR_LEN)) {
+		if (bnxt_vlan_filter_exists(bp, filter, chk, vlan_id)) {
 			/* Delete the filter */
 			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 			if (rc)
 				return rc;
 			STAILQ_REMOVE(&vnic->filter, filter,
 				      bnxt_filter_info, next);
-			STAILQ_INSERT_TAIL(&bp->free_filter_list, filter, next);
-
+			bnxt_free_filter(bp, filter);
 			PMD_DRV_LOG(INFO,
-				    "Del Vlan filter for %d\n",
+				    "Deleted vlan filter for %d\n",
 				    vlan_id);
-			return rc;
+			return 0;
 		}
 		filter = STAILQ_NEXT(filter, next);
 	}
@@ -1780,11 +1758,7 @@  static int bnxt_add_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 	filter = STAILQ_FIRST(&vnic->filter);
 	/* Check if the VLAN has already been added */
 	while (filter) {
-		if ((filter->enables & chk) &&
-		    (filter->l2_ivlan == vlan_id &&
-		     filter->l2_ivlan_mask == 0x0FFF) &&
-		     !memcmp(filter->l2_addr, bp->mac_addr,
-			     RTE_ETHER_ADDR_LEN))
+		if (bnxt_vlan_filter_exists(bp, filter, chk, vlan_id))
 			return -EEXIST;
 
 		filter = STAILQ_NEXT(filter, next);
@@ -1817,18 +1791,17 @@  static int bnxt_add_vlan_filter(struct bnxt *bp, uint16_t vlan_id)
 		 * not able to create the filter in hardware.
 		 */
 		filter->fw_l2_filter_id = UINT64_MAX;
-		STAILQ_INSERT_TAIL(&bp->free_filter_list, filter, next);
+		bnxt_free_filter(bp, filter);
 		return rc;
-	} else {
-		/* Add this new filter to the list */
-		if (vlan_id == 0) {
-			filter->dflt = true;
-			STAILQ_INSERT_HEAD(&vnic->filter, filter, next);
-		} else {
-			STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
-		}
 	}
 
+	filter->mac_index = 0;
+	/* Add this new filter to the list */
+	if (vlan_id == 0)
+		STAILQ_INSERT_HEAD(&vnic->filter, filter, next);
+	else
+		STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
+
 	PMD_DRV_LOG(INFO,
 		    "Added Vlan filter for %d\n", vlan_id);
 	return rc;
@@ -1859,19 +1832,17 @@  static int bnxt_del_dflt_mac_filter(struct bnxt *bp,
 
 	filter = STAILQ_FIRST(&vnic->filter);
 	while (filter) {
-		if (filter->dflt &&
+		if (filter->mac_index == 0 &&
 		    !memcmp(filter->l2_addr, bp->mac_addr,
 			    RTE_ETHER_ADDR_LEN)) {
 			rc = bnxt_hwrm_clear_l2_filter(bp, filter);
-			if (rc)
-				return rc;
-			filter->dflt = false;
-			STAILQ_REMOVE(&vnic->filter, filter,
-				      bnxt_filter_info, next);
-			STAILQ_INSERT_TAIL(&bp->free_filter_list,
-					   filter, next);
-			filter->fw_l2_filter_id = -1;
-			break;
+			if (!rc) {
+				STAILQ_REMOVE(&vnic->filter, filter,
+					      bnxt_filter_info, next);
+				bnxt_free_filter(bp, filter);
+				filter->fw_l2_filter_id = UINT64_MAX;
+			}
+			return rc;
 		}
 		filter = STAILQ_NEXT(filter, next);
 	}
@@ -1894,10 +1865,10 @@  bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 	vnic = BNXT_GET_DEFAULT_VNIC(bp);
 	if (!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
 		/* Remove any VLAN filters programmed */
-		for (i = 0; i < 4095; i++)
+		for (i = 0; i < RTE_ETHER_MAX_VLAN_ID; i++)
 			bnxt_del_vlan_filter(bp, i);
 
-		rc = bnxt_add_mac_filter(bp, vnic, NULL, 0);
+		rc = bnxt_add_mac_filter(bp, vnic, NULL, 0, 0);
 		if (rc)
 			return rc;
 	} else {
diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 1c8e3e329..7e29468b7 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -34,6 +34,7 @@  struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp)
 	}
 	STAILQ_REMOVE_HEAD(&bp->free_filter_list, next);
 
+	filter->mac_index = INVALID_MAC_INDEX;
 	/* Default to L2 MAC Addr filter */
 	filter->flags = HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
 	filter->enables = HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
diff --git a/drivers/net/bnxt/bnxt_filter.h b/drivers/net/bnxt/bnxt_filter.h
index e09b435db..7415b3612 100644
--- a/drivers/net/bnxt/bnxt_filter.h
+++ b/drivers/net/bnxt/bnxt_filter.h
@@ -8,6 +8,12 @@ 
 
 #include <rte_ether.h>
 
+#define bnxt_vlan_filter_exists(bp, filter, chk, vlan_id)	\
+		(((filter)->enables & (chk)) &&			\
+		 ((filter)->l2_ivlan == (vlan_id) &&		\
+		  (filter)->l2_ivlan_mask == 0x0FFF) &&		\
+		 !memcmp((filter)->l2_addr, (bp)->mac_addr,	\
+			 RTE_ETHER_ADDR_LEN))
 struct bnxt;
 
 #define BNXT_FLOW_L2_VALID_FLAG			BIT(0)
@@ -71,7 +77,6 @@  struct bnxt_filter_info {
 	uint16_t                ip_addr_type;
 	uint16_t                ethertype;
 	uint32_t		priority;
-	uint8_t			dflt;
 };
 
 struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index c777c73bd..daed4ee76 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2468,18 +2468,15 @@  int bnxt_set_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	int rc = 0;
 
 	STAILQ_FOREACH(filter, &vnic->filter, next) {
-		if (filter->filter_type == HWRM_CFA_EM_FILTER) {
+		if (filter->filter_type == HWRM_CFA_EM_FILTER)
 			rc = bnxt_hwrm_set_em_filter(bp, filter->dst_id,
 						     filter);
-		} else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER) {
+		else if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
 			rc = bnxt_hwrm_set_ntuple_filter(bp, filter->dst_id,
 							 filter);
-		} else {
+		else
 			rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id,
 						     filter);
-			if (!rc)
-				filter->dflt = 1;
-		}
 		if (rc)
 			break;
 	}
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index d55adc33a..6420281d3 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -63,6 +63,7 @@  int bnxt_mq_rx_configure(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto err_out;
 		}
+		filter->mac_index = 0;
 		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
 		STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 		goto out;
@@ -146,6 +147,7 @@  int bnxt_mq_rx_configure(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto err_out;
 		}
+		filter->mac_index = 0;
 		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
 		/*
 		 * TODO: Configure & associate CFA rule for