net: hns3: update the device mac address asynchronously

driver inclusion
category: bugfix
bugzilla: NA
CVE: NA

--------------------------------------------

Currently, we change the device address directly in the path
f hns3_set_mac_address(). There is a race with calling
hclge_sync_vport_mac_table(), both them can add/delete mac
address without vport->mac_list_lock protection. This patch
fixes it by only updating mac list, then adding/deleting the
mac address by hclge_sync_vport_mac_table().
Signed-off-by: NJian Shen <shenjian15@huawei.com>
Signed-off-by: Nshenhao <shenhao21@huawei.com>
Reviewed-by: NZhong Zhaohui <zhongzhaohui@huawei.com>
Signed-off-by: NYang Yingliang <yangyingliang@huawei.com>

drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c

@@ -7469,8 +7469,6 @@ int hclge_update_mac_list(struct hclge_vport *vport,
 	list = (mac_type == HCLGE_MAC_ADDR_UC) ?
 		&vport->uc_mac_list : &vport->mc_mac_list;
 
-	set_bit(HCLGE_VPORT_STATE_MAC_TBL_CHANGE, &vport->state);
-
 	spin_lock_bh(&vport->mac_list_lock);
 
 	/* if the mac addr is already in the mac list, no need to add a new
@@ -7481,6 +7479,7 @@ int hclge_update_mac_list(struct hclge_vport *vport,
 	if (mac_node) {
 		hclge_update_mac_node(mac_node, state);
 		spin_unlock_bh(&vport->mac_list_lock);
+		set_bit(HCLGE_VPORT_STATE_MAC_TBL_CHANGE, &vport->state);
 		return 0;
 	}
 
@@ -7496,6 +7495,8 @@ int hclge_update_mac_list(struct hclge_vport *vport,
 		return -ENOMEM;
 	}
 
+	set_bit(HCLGE_VPORT_STATE_MAC_TBL_CHANGE, &vport->state);
+
 	mac_node->state = state;
 	ether_addr_copy(mac_node->mac_addr, addr);
 	list_add_tail(&mac_node->node, list);
@@ -8193,45 +8194,47 @@ static void hclge_get_mac_addr(struct hnae3_handle *handle, u8 *p)
 	ether_addr_copy(p, hdev->hw.mac.mac_addr);
 }
 
-/* use new node to replace old node in the list, if old address is still in
- * mac table, add it to list tail, and try to remove in service task
- */
-void hclge_replace_mac_node(struct list_head *list, const u8 *old_addr,
-			    const u8 *new_addr, bool keep_old)
+int hclge_update_mac_node_for_dev_addr(struct hclge_vport *vport,
+				       const u8 *old_addr, const u8 *new_addr)
 {
-	struct hclge_vport_mac_addr_cfg *mac_node, *new_node;
+	struct hclge_vport_mac_addr_cfg *old_node, *new_node;
+	struct list_head *list = &vport->uc_mac_list;
 
-	mac_node = hclge_find_mac_node(list, old_addr);
-	if (!mac_node) {
+	new_node = hclge_find_mac_node(list, new_addr);
+	if (!new_node) {
 		new_node = kzalloc(sizeof(*new_node), GFP_ATOMIC);
 		if (!new_node)
-			return;
-		new_node->state = HCLGE_MAC_ACTIVE;
+			return -ENOMEM;
+		new_node->state = HCLGE_MAC_TO_ADD;
 		ether_addr_copy(new_node->mac_addr, new_addr);
-		list_add_tail(&new_node->node, list);
-		return;
+		list_add(&new_node->node, list);
+	} else {
+		if (new_node->state == HCLGE_MAC_TO_DEL)
+			new_node->state = HCLGE_MAC_ACTIVE;
+
+		/* make sure the new addr is in the list head, avoid dev
+		 * addr may be not re-added into mac table for the umv space
+		 * limitation after global/imp reset which will clear mac
+		 * table by hardware.
+		 */
+		list_move(&new_node->node, list);
 	}
 
-	mac_node->state = HCLGE_MAC_ACTIVE;
-	ether_addr_copy(mac_node->mac_addr, new_addr);
-	if (keep_old) {
-		new_node = kzalloc(sizeof(*new_node), GFP_ATOMIC);
-		if (!new_node)
-			return;
-		new_node->state = HCLGE_MAC_TO_DEL;
-		ether_addr_copy(new_node->mac_addr, old_addr);
-		list_add_tail(&new_node->node, list);
+	if (old_addr && !ether_addr_equal(old_addr, new_addr)) {
+		old_node = hclge_find_mac_node(list, old_addr);
+		if (old_node) {
+			if (old_node->state == HCLGE_MAC_TO_ADD) {
+				list_del(&old_node->node);
+				kfree(old_node);
+			} else {
+				old_node->state = HCLGE_MAC_TO_DEL;
+			}
+		}
 	}
-}
 
-void hclge_modify_mac_node_state(struct list_head *list, const u8 *addr,
-				 enum HCLGE_MAC_NODE_STATE state)
-{
-	struct hclge_vport_mac_addr_cfg *mac_node;
+	set_bit(HCLGE_VPORT_STATE_MAC_TBL_CHANGE, &vport->state);
 
-	mac_node = hclge_find_mac_node(list, addr);
-	if (mac_node)
-		mac_node->state = state;
+	return 0;
 }
 
 static int hclge_set_mac_addr(struct hnae3_handle *handle, void *p,
@@ -8240,7 +8243,7 @@ static int hclge_set_mac_addr(struct hnae3_handle *handle, void *p,
 	const unsigned char *new_addr = (const unsigned char *)p;
 	struct hclge_vport *vport = hclge_get_vport(handle);
 	struct hclge_dev *hdev = vport->back;
-	bool old_mac_removed = true;
+	unsigned char *old_addr = NULL;
 	int ret;
 
 	/* mac addr check */
@@ -8253,63 +8256,33 @@ static int hclge_set_mac_addr(struct hnae3_handle *handle, void *p,
 		return -EINVAL;
 	}
 
-	if ((!is_first || is_kdump_kernel()) &&
-	    hclge_rm_uc_addr_common(vport, hdev->hw.mac.mac_addr)) {
-		dev_warn(&hdev->pdev->dev,
-			 "remove old uc mac address fail.\n");
-		old_mac_removed = false;
-	}
-
-	ret = hclge_add_uc_addr_common(vport, new_addr);
+	ret = hclge_pause_addr_cfg(hdev, new_addr);
 	if (ret) {
 		dev_err(&hdev->pdev->dev,
-			"add uc mac address fail, ret =%d.\n",
+			"Failed to configure mac pause address, ret=%d\n",
 			ret);
-
-		if (!is_first) {
-			ret = hclge_add_uc_addr_common(vport,
-						       hdev->hw.mac.mac_addr);
-			if (ret) {
-				old_mac_removed = true;
-				dev_err(&hdev->pdev->dev,
-					"restore uc mac address fail.\n");
-			} else {
-				old_mac_removed = false;
-			}
-		}
-
-		/* if old dev addr restore failed, add to the uc mac list,
-		 * and try to restore it in service task
-		 */
-		if (old_mac_removed) {
-			spin_lock_bh(&vport->mac_list_lock);
-			hclge_modify_mac_node_state(&vport->uc_mac_list,
-						    hdev->hw.mac.mac_addr,
-						    HCLGE_MAC_TO_ADD);
-			spin_unlock_bh(&vport->mac_list_lock);
-		}
-
-		return -EIO;
+		return ret;
 	}
 
-	/* new dev addr add success, replace the old dev addr in uc mac list.
-	 * if old dev addr delete fail, keep it in the mac list, and try to del
-	 * it in service task.
-	 */
-	spin_lock_bh(&vport->mac_list_lock);
-	hclge_replace_mac_node(&vport->uc_mac_list, hdev->hw.mac.mac_addr,
-			       new_addr, !old_mac_removed);
-	spin_unlock_bh(&vport->mac_list_lock);
+	if (!is_first)
+		old_addr = hdev->hw.mac.mac_addr;
 
-	ret = hclge_pause_addr_cfg(hdev, new_addr);
+	spin_lock_bh(&vport->mac_list_lock);
+	ret = hclge_update_mac_node_for_dev_addr(vport, old_addr, new_addr);
 	if (ret) {
 		dev_err(&hdev->pdev->dev,
-			"configure mac pause address fail, ret =%d.\n",
-			ret);
-		return -EIO;
+			"Failed to change the mac addr:%pM, ret =%d\n",
+			new_addr, ret);
+		spin_unlock_bh(&vport->mac_list_lock);
+		return ret;
 	}
-
+	/* we must update dev addr with spin lock protect, preventing dev addr
+	 * being removed by set_rx_mode path.
+	 */
 	ether_addr_copy(hdev->hw.mac.mac_addr, new_addr);
+	spin_unlock_bh(&vport->mac_list_lock);
+
+	hclge_task_schedule(hdev, 0);
 
 	return 0;
 }

drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h

@@ -1018,10 +1018,8 @@ int hclge_update_mac_list(struct hclge_vport *vport,
 			  enum HCLGE_MAC_NODE_STATE state,
 			  enum HCLGE_MAC_ADDR_TYPE mac_type,
 			  const unsigned char *addr);
-void hclge_replace_mac_node(struct list_head *list, const u8 *old_addr,
-			    const u8 *new_addr, bool keep_old);
-void hclge_modify_mac_node_state(struct list_head *list, const u8 *addr,
-				 enum HCLGE_MAC_NODE_STATE state);
+int hclge_update_mac_node_for_dev_addr(struct hclge_vport *vport,
+				       const u8 *old_addr, const u8 *new_addr);
 void hclge_rm_vport_all_mac_table(struct hclge_vport *vport, bool is_del_list,
 				  enum HCLGE_MAC_ADDR_TYPE mac_type);
 void hclge_rm_vport_all_vlan_table(struct hclge_vport *vport, bool is_del_list);

drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c

@@ -344,36 +344,6 @@ void hclge_inform_vf_promisc_info(struct hclge_vport *vport)
 			   HCLGE_MBX_PUSH_PROMISC_INFO, dest_vfid);
 }
 
-static int hclge_modify_vf_mac_addr(struct hclge_vport *vport,
-				    const u8 *old_addr, const u8 *new_addr)
-{
-	bool old_mac_removed = true;
-	int ret;
-
-	ret = hclge_rm_uc_addr_common(vport, old_addr);
-	if (ret)
-		old_mac_removed = false;
-
-	ret = hclge_add_uc_addr_common(vport, new_addr);
-	if (ret) {
-		ret = hclge_add_uc_addr_common(vport, old_addr);
-		if (ret) {
-			spin_lock_bh(&vport->mac_list_lock);
-			hclge_modify_mac_node_state(&vport->uc_mac_list,
-						    old_addr, HCLGE_MAC_TO_ADD);
-			spin_unlock_bh(&vport->mac_list_lock);
-		}
-		return -EIO;
-
-	} else {
-		spin_lock_bh(&vport->mac_list_lock);
-		hclge_replace_mac_node(&vport->uc_mac_list, old_addr, new_addr,
-				       !old_mac_removed);
-		spin_unlock_bh(&vport->mac_list_lock);
-	}
-	return ret;
-}
-
 static int hclge_set_vf_uc_mac_addr(struct hclge_vport *vport,
 				    struct hclge_mbx_vf_to_pf_cmd *mbx_req)
 {
@@ -381,7 +351,7 @@ static int hclge_set_vf_uc_mac_addr(struct hclge_vport *vport,
 
 	const u8 *mac_addr = (const u8 *)(mbx_req->msg.data);
 	struct hclge_dev *hdev = vport->back;
-	int status = 0;
+	int status;
 
 	if (mbx_req->msg.subcode == HCLGE_MBX_MAC_VLAN_UC_MODIFY) {
 		const u8 *old_addr = (const u8 *)
@@ -397,25 +367,17 @@ static int hclge_set_vf_uc_mac_addr(struct hclge_vport *vport,
 		if (!is_valid_ether_addr(mac_addr))
 			return -EINVAL;
 
-		if (is_zero_ether_addr(old_addr)) {
-			status = hclge_add_uc_addr_common(vport, mac_addr);
-			if (!status)
-				hclge_update_mac_list(vport, HCLGE_MAC_ACTIVE,
-						      HCLGE_MAC_ADDR_UC,
-						      mac_addr);
-			else
-				hclge_update_mac_list(vport, HCLGE_MAC_TO_ADD,
-						      HCLGE_MAC_ADDR_UC,
-						      mac_addr);
-		} else {
-			hclge_modify_vf_mac_addr(vport, old_addr, mac_addr);
-		}
+		spin_lock_bh(&vport->mac_list_lock);
+		status = hclge_update_mac_node_for_dev_addr(vport, old_addr,
+							    mac_addr);
+		spin_unlock_bh(&vport->mac_list_lock);
+		hclge_task_schedule(hdev, 0);
 	} else if (mbx_req->msg.subcode == HCLGE_MBX_MAC_VLAN_UC_ADD) {
-		hclge_update_mac_list(vport, HCLGE_MAC_TO_ADD,
-				      HCLGE_MAC_ADDR_UC, mac_addr);
+		status = hclge_update_mac_list(vport, HCLGE_MAC_TO_ADD,
+					       HCLGE_MAC_ADDR_UC, mac_addr);
 	} else if (mbx_req->msg.subcode == HCLGE_MBX_MAC_VLAN_UC_REMOVE) {
-		hclge_update_mac_list(vport, HCLGE_MAC_TO_DEL,
-				      HCLGE_MAC_ADDR_UC, mac_addr);
+		status = hclge_update_mac_list(vport, HCLGE_MAC_TO_DEL,
+					       HCLGE_MAC_ADDR_UC, mac_addr);
 	} else {
 		dev_err(&hdev->pdev->dev,
 			"failed to set unicast mac addr, unknown subcode %u\n",