[dpdk-dev,1/4] net/qede: fix multicast filtering

Message ID 1526775346-10643-1-git-send-email-rasesh.mody@cavium.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Mody, Rasesh May 20, 2018, 12:15 a.m. UTC
  From: Harish Patil <harish.patil@cavium.com>

This patch is to fix multicast filtering using set_mc_addr_list().

Fixes: 77fac1b54fc9 ("net/qede: fix filtering code")
Cc: stable@dpdk.org

Signed-off-by: Harish Patil <harish.patil@cavium.com>
Signed-off-by: Shahed Shaikh <shahed.shaikh@cavium.com>
Signed-off-by: Rasesh Mody <rasesh.mody@cavium.com>
---
 drivers/net/qede/base/ecore_l2.c      |   12 +--
 drivers/net/qede/base/ecore_l2_api.h  |    2 +-
 drivers/net/qede/base/ecore_sriov.c   |    3 +-
 drivers/net/qede/base/ecore_vf.c      |    5 +-
 drivers/net/qede/base/ecore_vfpf_if.h |    8 +-
 drivers/net/qede/qede_ethdev.c        |  179 ++++++++++++++++++---------------
 6 files changed, 114 insertions(+), 95 deletions(-)
  

Comments

Ferruh Yigit May 20, 2018, 10:02 p.m. UTC | #1
On 5/20/2018 1:15 AM, Rasesh Mody wrote:
> From: Harish Patil <harish.patil@cavium.com>
> 
> This patch is to fix multicast filtering using set_mc_addr_list().
> 
> Fixes: 77fac1b54fc9 ("net/qede: fix filtering code")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Harish Patil <harish.patil@cavium.com>
> Signed-off-by: Shahed Shaikh <shahed.shaikh@cavium.com>
> Signed-off-by: Rasesh Mody <rasesh.mody@cavium.com>

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

Patch

diff --git a/drivers/net/qede/base/ecore_l2.c b/drivers/net/qede/base/ecore_l2.c
index c897fa5..fdb928a 100644
--- a/drivers/net/qede/base/ecore_l2.c
+++ b/drivers/net/qede/base/ecore_l2.c
@@ -690,7 +690,7 @@  enum _ecore_status_t
 
 	p_ramrod->common.update_approx_mcast_flg = 1;
 	for (i = 0; i < ETH_MULTICAST_MAC_BINS_IN_REGS; i++) {
-		u32 *p_bins = (u32 *)p_params->bins;
+		u32 *p_bins = p_params->bins;
 
 		p_ramrod->approx_mcast.bins[i] = OSAL_CPU_TO_LE32(p_bins[i]);
 	}
@@ -1568,8 +1568,8 @@  u8 ecore_mcast_bin_from_mac(u8 *mac)
 			  enum spq_mode comp_mode,
 			  struct ecore_spq_comp_cb *p_comp_data)
 {
-	unsigned long bins[ETH_MULTICAST_MAC_BINS_IN_REGS];
 	struct vport_update_ramrod_data *p_ramrod = OSAL_NULL;
+	u32 bins[ETH_MULTICAST_MAC_BINS_IN_REGS];
 	struct ecore_spq_entry *p_ent = OSAL_NULL;
 	struct ecore_sp_init_data init_data;
 	u8 abs_vport_id = 0;
@@ -1608,8 +1608,7 @@  u8 ecore_mcast_bin_from_mac(u8 *mac)
 	/* explicitly clear out the entire vector */
 	OSAL_MEMSET(&p_ramrod->approx_mcast.bins,
 		    0, sizeof(p_ramrod->approx_mcast.bins));
-	OSAL_MEMSET(bins, 0, sizeof(unsigned long) *
-		    ETH_MULTICAST_MAC_BINS_IN_REGS);
+	OSAL_MEMSET(bins, 0, sizeof(u32) * ETH_MULTICAST_MAC_BINS_IN_REGS);
 	/* filter ADD op is explicit set op and it removes
 	*  any existing filters for the vport.
 	*/
@@ -1618,16 +1617,15 @@  u8 ecore_mcast_bin_from_mac(u8 *mac)
 			u32 bit;
 
 			bit = ecore_mcast_bin_from_mac(p_filter_cmd->mac[i]);
-			OSAL_SET_BIT(bit, bins);
+			bins[bit / 32] |= 1 << (bit % 32);
 		}
 
 		/* Convert to correct endianity */
 		for (i = 0; i < ETH_MULTICAST_MAC_BINS_IN_REGS; i++) {
 			struct vport_update_ramrod_mcast *p_ramrod_bins;
-			u32 *p_bins = (u32 *)bins;
 
 			p_ramrod_bins = &p_ramrod->approx_mcast;
-			p_ramrod_bins->bins[i] = OSAL_CPU_TO_LE32(p_bins[i]);
+			p_ramrod_bins->bins[i] = OSAL_CPU_TO_LE32(bins[i]);
 		}
 	}
 
diff --git a/drivers/net/qede/base/ecore_l2_api.h b/drivers/net/qede/base/ecore_l2_api.h
index ed9837b..8cbe8dd 100644
--- a/drivers/net/qede/base/ecore_l2_api.h
+++ b/drivers/net/qede/base/ecore_l2_api.h
@@ -332,7 +332,7 @@  struct ecore_sp_vport_update_params {
 	u8			anti_spoofing_en;
 	u8			update_accept_any_vlan_flg;
 	u8			accept_any_vlan;
-	unsigned long		bins[8];
+	u32			bins[8];
 	struct ecore_rss_params	*rss_params;
 	struct ecore_filter_accept_flags accept_flags;
 	struct ecore_sge_tpa_params *sge_tpa_params;
diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index 0279709..93674a2 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -2979,8 +2979,7 @@  void *ecore_iov_search_list_tlvs(struct ecore_hwfn *p_hwfn,
 
 	p_data->update_approx_mcast_flg = 1;
 	OSAL_MEMCPY(p_data->bins, p_mcast_tlv->bins,
-		    sizeof(unsigned long) *
-		    ETH_MULTICAST_MAC_BINS_IN_REGS);
+		    sizeof(u32) * ETH_MULTICAST_MAC_BINS_IN_REGS);
 	*tlvs_mask |= 1 << ECORE_IOV_VP_UPDATE_MCAST;
 }
 
diff --git a/drivers/net/qede/base/ecore_vf.c b/drivers/net/qede/base/ecore_vf.c
index e0f2dd5..8a08911 100644
--- a/drivers/net/qede/base/ecore_vf.c
+++ b/drivers/net/qede/base/ecore_vf.c
@@ -1275,8 +1275,7 @@  enum _ecore_status_t
 		resp_size += sizeof(struct pfvf_def_resp_tlv);
 
 		OSAL_MEMCPY(p_mcast_tlv->bins, p_params->bins,
-			    sizeof(unsigned long) *
-			    ETH_MULTICAST_MAC_BINS_IN_REGS);
+			    sizeof(u32) * ETH_MULTICAST_MAC_BINS_IN_REGS);
 	}
 
 	update_rx = p_params->accept_flags.update_rx_mode_config;
@@ -1473,7 +1472,7 @@  void ecore_vf_pf_filter_mcast(struct ecore_hwfn *p_hwfn,
 			u32 bit;
 
 			bit = ecore_mcast_bin_from_mac(p_filter_cmd->mac[i]);
-			OSAL_SET_BIT(bit, sp_params.bins);
+			sp_params.bins[bit / 32] |= 1 << (bit % 32);
 		}
 	}
 
diff --git a/drivers/net/qede/base/ecore_vfpf_if.h b/drivers/net/qede/base/ecore_vfpf_if.h
index ecb0064..c6af9ca 100644
--- a/drivers/net/qede/base/ecore_vfpf_if.h
+++ b/drivers/net/qede/base/ecore_vfpf_if.h
@@ -396,7 +396,13 @@  struct vfpf_vport_update_mcast_bin_tlv {
 	struct channel_tlv	tl;
 	u8			padding[4];
 
-	u64		bins[8];
+	/* This was a mistake; There are only 256 approx bins,
+	 * and in HSI they're divided into 32-bit values.
+	 * As old VFs used to set-bit to the values on its side,
+	 * the upper half of the array is never expected to contain any data.
+	 */
+	u64		bins[4];
+	u64		obsolete_bins[4];
 };
 
 struct vfpf_vport_update_accept_param_tlv {
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 50a63be..3e1a62c 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -894,47 +894,69 @@  static void qede_set_ucast_cmn_params(struct ecore_filter_ucast *ucast)
 }
 
 static int
-qede_mcast_filter(struct rte_eth_dev *eth_dev, struct ecore_filter_ucast *mcast,
-		  bool add)
+qede_add_mcast_filters(struct rte_eth_dev *eth_dev, struct ether_addr *mc_addrs,
+		       uint32_t mc_addrs_num)
 {
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
 	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
-	struct ether_addr *mac_addr;
-	struct qede_mcast_entry *tmp = NULL;
-	struct qede_mcast_entry *m;
+	struct ecore_filter_mcast mcast;
+	struct qede_mcast_entry *m = NULL;
+	uint8_t i;
+	int rc;
 
-	mac_addr  = (struct ether_addr *)mcast->mac;
-	if (add) {
-		SLIST_FOREACH(tmp, &qdev->mc_list_head, list) {
-			if (memcmp(mac_addr, &tmp->mac, ETHER_ADDR_LEN) == 0) {
-				DP_ERR(edev,
-					"Multicast MAC is already added\n");
-				return -EEXIST;
-			}
-		}
+	for (i = 0; i < mc_addrs_num; i++) {
 		m = rte_malloc(NULL, sizeof(struct qede_mcast_entry),
-			RTE_CACHE_LINE_SIZE);
+			       RTE_CACHE_LINE_SIZE);
 		if (!m) {
-			DP_ERR(edev,
-				"Did not allocate memory for mcast\n");
+			DP_ERR(edev, "Did not allocate memory for mcast\n");
 			return -ENOMEM;
 		}
-		ether_addr_copy(mac_addr, &m->mac);
+		ether_addr_copy(&mc_addrs[i], &m->mac);
 		SLIST_INSERT_HEAD(&qdev->mc_list_head, m, list);
-		qdev->num_mc_addr++;
-	} else {
-		SLIST_FOREACH(tmp, &qdev->mc_list_head, list) {
-			if (memcmp(mac_addr, &tmp->mac, ETHER_ADDR_LEN) == 0)
-				break;
-		}
-		if (tmp == NULL) {
-			DP_INFO(edev, "Multicast mac is not found\n");
-			return -EINVAL;
-		}
-		SLIST_REMOVE(&qdev->mc_list_head, tmp,
-			     qede_mcast_entry, list);
-		qdev->num_mc_addr--;
 	}
+	memset(&mcast, 0, sizeof(mcast));
+	mcast.num_mc_addrs = mc_addrs_num;
+	mcast.opcode = ECORE_FILTER_ADD;
+	for (i = 0; i < mc_addrs_num; i++)
+		ether_addr_copy(&mc_addrs[i], (struct ether_addr *)
+							&mcast.mac[i]);
+	rc = ecore_filter_mcast_cmd(edev, &mcast, ECORE_SPQ_MODE_CB, NULL);
+	if (rc != ECORE_SUCCESS) {
+		DP_ERR(edev, "Failed to add multicast filter (rc = %d\n)", rc);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int qede_del_mcast_filters(struct rte_eth_dev *eth_dev)
+{
+	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
+	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
+	struct qede_mcast_entry *tmp = NULL;
+	struct ecore_filter_mcast mcast;
+	int j;
+	int rc;
+
+	memset(&mcast, 0, sizeof(mcast));
+	mcast.num_mc_addrs = qdev->num_mc_addr;
+	mcast.opcode = ECORE_FILTER_REMOVE;
+	j = 0;
+	SLIST_FOREACH(tmp, &qdev->mc_list_head, list) {
+		ether_addr_copy(&tmp->mac, (struct ether_addr *)&mcast.mac[j]);
+		j++;
+	}
+	rc = ecore_filter_mcast_cmd(edev, &mcast, ECORE_SPQ_MODE_CB, NULL);
+	if (rc != ECORE_SUCCESS) {
+		DP_ERR(edev, "Failed to delete multicast filter\n");
+		return -1;
+	}
+	/* Init the list */
+	while (!SLIST_EMPTY(&qdev->mc_list_head)) {
+		tmp = SLIST_FIRST(&qdev->mc_list_head);
+		SLIST_REMOVE_HEAD(&qdev->mc_list_head, list);
+	}
+	SLIST_INIT(&qdev->mc_list_head);
 
 	return 0;
 }
@@ -945,59 +967,22 @@  static void qede_set_ucast_cmn_params(struct ecore_filter_ucast *ucast)
 {
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
 	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
-	enum _ecore_status_t rc;
-	struct ecore_filter_mcast mcast;
-	struct qede_mcast_entry *tmp;
-	uint16_t j = 0;
+	enum _ecore_status_t rc = ECORE_INVAL;
 
-	/* Multicast */
-	if (is_multicast_ether_addr((struct ether_addr *)ucast->mac)) {
-		if (add) {
-			if (qdev->num_mc_addr >= ECORE_MAX_MC_ADDRS) {
-				DP_ERR(edev,
-				       "Mcast filter table limit exceeded, "
-				       "Please enable mcast promisc mode\n");
-				return -ECORE_INVAL;
-			}
-		}
-		rc = qede_mcast_filter(eth_dev, ucast, add);
-		if (rc == 0) {
-			DP_INFO(edev, "num_mc_addrs = %u\n", qdev->num_mc_addr);
-			memset(&mcast, 0, sizeof(mcast));
-			mcast.num_mc_addrs = qdev->num_mc_addr;
-			mcast.opcode = ECORE_FILTER_ADD;
-			SLIST_FOREACH(tmp, &qdev->mc_list_head, list) {
-				ether_addr_copy(&tmp->mac,
-					(struct ether_addr *)&mcast.mac[j]);
-				j++;
-			}
-			rc = ecore_filter_mcast_cmd(edev, &mcast,
-						    ECORE_SPQ_MODE_CB, NULL);
-		}
-		if (rc != ECORE_SUCCESS) {
-			DP_ERR(edev, "Failed to add multicast filter"
-			       " rc = %d, op = %d\n", rc, add);
-		}
-	} else { /* Unicast */
-		if (add) {
-			if (qdev->num_uc_addr >=
-			    qdev->dev_info.num_mac_filters) {
-				DP_ERR(edev,
-				       "Ucast filter table limit exceeded,"
-				       " Please enable promisc mode\n");
-				return -ECORE_INVAL;
-			}
-		}
-		rc = qede_ucast_filter(eth_dev, ucast, add);
-		if (rc == 0)
-			rc = ecore_filter_ucast_cmd(edev, ucast,
-						    ECORE_SPQ_MODE_CB, NULL);
-		if (rc != ECORE_SUCCESS) {
-			DP_ERR(edev, "MAC filter failed, rc = %d, op = %d\n",
-			       rc, add);
-		}
+	if (add && (qdev->num_uc_addr >= qdev->dev_info.num_mac_filters)) {
+		DP_ERR(edev, "Ucast filter table limit exceeded,"
+			      " Please enable promisc mode\n");
+			return ECORE_INVAL;
 	}
 
+	rc = qede_ucast_filter(eth_dev, ucast, add);
+	if (rc == 0)
+		rc = ecore_filter_ucast_cmd(edev, ucast,
+					    ECORE_SPQ_MODE_CB, NULL);
+	if (rc != ECORE_SUCCESS)
+		DP_ERR(edev, "MAC filter failed, rc = %d, op = %d\n",
+		       rc, add);
+
 	return rc;
 }
 
@@ -2042,6 +2027,35 @@  static void qede_allmulticast_disable(struct rte_eth_dev *eth_dev)
 				QED_FILTER_RX_MODE_TYPE_REGULAR);
 }
 
+static int
+qede_set_mc_addr_list(struct rte_eth_dev *eth_dev, struct ether_addr *mc_addrs,
+		      uint32_t mc_addrs_num)
+{
+	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
+	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
+	uint8_t i;
+
+	if (mc_addrs_num > ECORE_MAX_MC_ADDRS) {
+		DP_ERR(edev, "Reached max multicast filters limit,"
+			     "Please enable multicast promisc mode\n");
+		return -ENOSPC;
+	}
+
+	for (i = 0; i < mc_addrs_num; i++) {
+		if (!is_multicast_ether_addr(&mc_addrs[i])) {
+			DP_ERR(edev, "Not a valid multicast MAC\n");
+			return -EINVAL;
+		}
+	}
+
+	/* Flush all existing entries */
+	if (qede_del_mcast_filters(eth_dev))
+		return -1;
+
+	/* Set new mcast list */
+	return qede_add_mcast_filters(eth_dev, mc_addrs, mc_addrs_num);
+}
+
 static int qede_flow_ctrl_set(struct rte_eth_dev *eth_dev,
 			      struct rte_eth_fc_conf *fc_conf)
 {
@@ -2926,6 +2940,7 @@  int qede_dev_filter_ctrl(struct rte_eth_dev *eth_dev,
 	.promiscuous_disable = qede_promiscuous_disable,
 	.allmulticast_enable = qede_allmulticast_enable,
 	.allmulticast_disable = qede_allmulticast_disable,
+	.set_mc_addr_list = qede_set_mc_addr_list,
 	.dev_stop = qede_dev_stop,
 	.dev_close = qede_dev_close,
 	.stats_get = qede_get_stats,
@@ -2966,6 +2981,7 @@  int qede_dev_filter_ctrl(struct rte_eth_dev *eth_dev,
 	.promiscuous_disable = qede_promiscuous_disable,
 	.allmulticast_enable = qede_allmulticast_enable,
 	.allmulticast_disable = qede_allmulticast_disable,
+	.set_mc_addr_list = qede_set_mc_addr_list,
 	.dev_stop = qede_dev_stop,
 	.dev_close = qede_dev_close,
 	.stats_get = qede_get_stats,
@@ -3174,6 +3190,7 @@  static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
 	SLIST_INIT(&adapter->fdir_info.fdir_list_head);
 	SLIST_INIT(&adapter->vlan_list_head);
 	SLIST_INIT(&adapter->uc_list_head);
+	SLIST_INIT(&adapter->mc_list_head);
 	adapter->mtu = ETHER_MTU;
 	adapter->vport_started = false;