[dpdk-stable] patch 'net/i40e: fix lack of MAC type when set MAC address' has been queued to stable release 19.11.9

[Christian Ehrhardt <christian.ehrhardt@canonical.com>]

Hi,

FYI, your patch has been queued to stable release 19.11.9

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 05/19/21. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/cpaelzer/dpdk-stable-queue

This queued commit can be viewed at:
https://github.com/cpaelzer/dpdk-stable-queue/commit/332bb84d3ed4bf0609f10a69185f1d1c9bd85234

Thanks.

Christian Ehrhardt <christian.ehrhardt@canonical.com>

---
From 332bb84d3ed4bf0609f10a69185f1d1c9bd85234 Mon Sep 17 00:00:00 2001
From: Robin Zhang <robinx.zhang@intel.com>
Date: Fri, 19 Mar 2021 09:20:18 +0000
Subject: [PATCH] net/i40e: fix lack of MAC type when set MAC address

[ upstream commit 3f604ddf33cf69d9a9a7645e67a7e0b915e76b6a ]

Currently, there is no way for a VF driver to specify that it wants to
change its device/primary unicast MAC address. This makes it
difficult/impossible for the PF driver to track the VF's device/primary
unicast MAC address, which is used for VM/VF reboot and displaying on
the host. Fix this by using 2 bits of a pad byte in the
virtchnl_ether_addr structure so the VF can specify what type of MAC
it's adding/deleting.

Below are the values that should be used by all VF drivers going
forward.

VIRTCHNL_ETHER_ADDR_LEGACY(0):
- The type should only ever be 0 for legacy AVF drivers (i.e.
  drivers that don't support the new type bits). The PF drivers
  will track VF's device/primary unicast MAC using with best
  effort.

VIRTCHNL_ETHER_ADDR_PRIMARY(1):
- This type should only be used when the VF is changing their
  device/primary unicast MAC. It should be used for both delete
  and add cases related to the device/primary unicast MAC.

VIRTCHNL_ETHER_ADDR_EXTRA(2):
- This type should be used when the VF is adding and/or deleting
  MAC addresses that are not the device/primary unicast MAC. For
  example, extra unicast addresses and multicast addresses
  assuming the PF supports "extra" addresses at all.

If a PF is parsing the type field of the virtchnl_ether_addr, then it
should use the VIRTCHNL_ETHER_ADDR_TYPE_MASK to mask the first two bits
of the type field since 0, 1, and 2 are the only valid values.

For i40evf PMD, when set default MAC address, use type
VIRTCHNL_ETHER_ADDR_PRIMARY as this case is changing device/primary
unicast MAC. For other cases, such as adding or deleting extra unicast
addresses and multicast addresses, use type VIRTCHNL_ETHER_ADDR_EXTRA.

Fixes: 6d13ea8e8e49 ("net: add rte prefix to ether structures")
Fixes: caccf8b318ca ("ethdev: return diagnostic when setting MAC address")

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
Tested-by: Yan Xia <yanx.xia@intel.com>
---
 drivers/net/i40e/base/virtchnl.h  | 29 +++++++++-
 drivers/net/i40e/i40e_ethdev_vf.c | 91 +++++++++++++++----------------
 2 files changed, 73 insertions(+), 47 deletions(-)

diff --git a/drivers/net/i40e/base/virtchnl.h b/drivers/net/i40e/base/virtchnl.h
index acd7b7eb88..1b4e76f60b 100644
--- a/drivers/net/i40e/base/virtchnl.h
+++ b/drivers/net/i40e/base/virtchnl.h
@@ -388,9 +388,36 @@ VIRTCHNL_CHECK_STRUCT_LEN(12, virtchnl_queue_select);
  * PF removes the filters and returns status.
  */
 
+/* VIRTCHNL_ETHER_ADDR_LEGACY
+ * Prior to adding the @type member to virtchnl_ether_addr, there were 2 pad
+ * bytes. Moving forward all VF drivers should not set type to
+ * VIRTCHNL_ETHER_ADDR_LEGACY. This is only here to not break previous/legacy
+ * behavior. The control plane function (i.e. PF) can use a best effort method
+ * of tracking the primary/device unicast in this case, but there is no
+ * guarantee and functionality depends on the implementation of the PF.
+ */
+
+/* VIRTCHNL_ETHER_ADDR_PRIMARY
+ * All VF drivers should set @type to VIRTCHNL_ETHER_ADDR_PRIMARY for the
+ * primary/device unicast MAC address filter for VIRTCHNL_OP_ADD_ETH_ADDR and
+ * VIRTCHNL_OP_DEL_ETH_ADDR. This allows for the underlying control plane
+ * function (i.e. PF) to accurately track and use this MAC address for
+ * displaying on the host and for VM/function reset.
+ */
+
+/* VIRTCHNL_ETHER_ADDR_EXTRA
+ * All VF drivers should set @type to VIRTCHNL_ETHER_ADDR_EXTRA for any extra
+ * unicast and/or multicast filters that are being added/deleted via
+ * VIRTCHNL_OP_DEL_ETH_ADDR/VIRTCHNL_OP_ADD_ETH_ADDR respectively.
+ */
 struct virtchnl_ether_addr {
 	u8 addr[VIRTCHNL_ETH_LENGTH_OF_ADDRESS];
-	u8 pad[2];
+	u8 type;
+#define VIRTCHNL_ETHER_ADDR_LEGACY	0
+#define VIRTCHNL_ETHER_ADDR_PRIMARY	1
+#define VIRTCHNL_ETHER_ADDR_EXTRA	2
+#define VIRTCHNL_ETHER_ADDR_TYPE_MASK	3 /* first two bits of type are valid */
+	u8 pad;
 };
 
 VIRTCHNL_CHECK_STRUCT_LEN(8, virtchnl_ether_addr);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 56986991cd..49b5ead859 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -106,6 +106,9 @@ static int i40evf_dev_tx_queue_start(struct rte_eth_dev *dev,
 				     uint16_t tx_queue_id);
 static int i40evf_dev_tx_queue_stop(struct rte_eth_dev *dev,
 				    uint16_t tx_queue_id);
+static int i40evf_add_del_eth_addr(struct rte_eth_dev *dev,
+				   struct rte_ether_addr *addr,
+				   bool add, uint8_t type);
 static int i40evf_add_mac_addr(struct rte_eth_dev *dev,
 			       struct rte_ether_addr *addr,
 			       uint32_t index,
@@ -801,10 +804,9 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
 }
 
 static int
-i40evf_add_mac_addr(struct rte_eth_dev *dev,
-		    struct rte_ether_addr *addr,
-		    __rte_unused uint32_t index,
-		    __rte_unused uint32_t pool)
+i40evf_add_del_eth_addr(struct rte_eth_dev *dev,
+			struct rte_ether_addr *addr,
+			bool add, uint8_t type)
 {
 	struct virtchnl_ether_addr_list *list;
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
@@ -813,83 +815,70 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 	int err;
 	struct vf_cmd_info args;
 
-	if (rte_is_zero_ether_addr(addr)) {
-		PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
-			    addr->addr_bytes[0], addr->addr_bytes[1],
-			    addr->addr_bytes[2], addr->addr_bytes[3],
-			    addr->addr_bytes[4], addr->addr_bytes[5]);
-		return I40E_ERR_INVALID_MAC_ADDR;
-	}
-
 	list = (struct virtchnl_ether_addr_list *)cmd_buffer;
 	list->vsi_id = vf->vsi_res->vsi_id;
 	list->num_elements = 1;
+	list->list[0].type = type;
 	rte_memcpy(list->list[0].addr, addr->addr_bytes,
 					sizeof(addr->addr_bytes));
 
-	args.ops = VIRTCHNL_OP_ADD_ETH_ADDR;
+	args.ops = add ? VIRTCHNL_OP_ADD_ETH_ADDR : VIRTCHNL_OP_DEL_ETH_ADDR;
 	args.in_args = cmd_buffer;
 	args.in_args_size = sizeof(cmd_buffer);
 	args.out_buffer = vf->aq_resp;
 	args.out_size = I40E_AQ_BUF_SZ;
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
-		PMD_DRV_LOG(ERR, "fail to execute command "
-			    "OP_ADD_ETHER_ADDRESS");
-	else
-		vf->vsi.mac_num++;
-
+		PMD_DRV_LOG(ERR, "fail to execute command %s",
+			    add ? "OP_ADD_ETH_ADDR" :  "OP_DEL_ETH_ADDR");
 	return err;
 }
 
-static void
-i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
-			    struct rte_ether_addr *addr)
+static int
+i40evf_add_mac_addr(struct rte_eth_dev *dev,
+		    struct rte_ether_addr *addr,
+		    __rte_unused uint32_t index,
+		    __rte_unused uint32_t pool)
 {
-	struct virtchnl_ether_addr_list *list;
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
-	uint8_t cmd_buffer[sizeof(struct virtchnl_ether_addr_list) + \
-			sizeof(struct virtchnl_ether_addr)];
 	int err;
-	struct vf_cmd_info args;
 
-	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
-		PMD_DRV_LOG(ERR, "Invalid mac:%x-%x-%x-%x-%x-%x",
+	if (rte_is_zero_ether_addr(addr)) {
+		PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
 			    addr->addr_bytes[0], addr->addr_bytes[1],
 			    addr->addr_bytes[2], addr->addr_bytes[3],
 			    addr->addr_bytes[4], addr->addr_bytes[5]);
-		return;
+		return I40E_ERR_INVALID_MAC_ADDR;
 	}
 
-	list = (struct virtchnl_ether_addr_list *)cmd_buffer;
-	list->vsi_id = vf->vsi_res->vsi_id;
-	list->num_elements = 1;
-	rte_memcpy(list->list[0].addr, addr->addr_bytes,
-			sizeof(addr->addr_bytes));
+	err = i40evf_add_del_eth_addr(dev, addr, TRUE, VIRTCHNL_ETHER_ADDR_EXTRA);
 
-	args.ops = VIRTCHNL_OP_DEL_ETH_ADDR;
-	args.in_args = cmd_buffer;
-	args.in_args_size = sizeof(cmd_buffer);
-	args.out_buffer = vf->aq_resp;
-	args.out_size = I40E_AQ_BUF_SZ;
-	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
-		PMD_DRV_LOG(ERR, "fail to execute command "
-			    "OP_DEL_ETHER_ADDRESS");
+		PMD_DRV_LOG(ERR, "fail to add MAC address");
 	else
-		vf->vsi.mac_num--;
-	return;
+		vf->vsi.mac_num++;
+
+	return err;
 }
 
 static void
 i40evf_del_mac_addr(struct rte_eth_dev *dev, uint32_t index)
 {
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct rte_eth_dev_data *data = dev->data;
 	struct rte_ether_addr *addr;
+	int err;
 
 	addr = &data->mac_addrs[index];
 
-	i40evf_del_mac_addr_by_addr(dev, addr);
+	err = i40evf_add_del_eth_addr(dev, addr, FALSE, VIRTCHNL_ETHER_ADDR_EXTRA);
+
+	if (err)
+		PMD_DRV_LOG(ERR, "fail to delete MAC address");
+	else
+		vf->vsi.mac_num--;
+
+	return;
 }
 
 static int
@@ -2066,6 +2055,7 @@ i40evf_add_del_all_mac_addr(struct rte_eth_dev *dev, bool add)
 				continue;
 			rte_memcpy(list->list[j].addr, addr->addr_bytes,
 					 sizeof(addr->addr_bytes));
+			list->list[j].type = VIRTCHNL_ETHER_ADDR_EXTRA;
 			PMD_DRV_LOG(DEBUG, "add/rm mac:%x:%x:%x:%x:%x:%x",
 				    addr->addr_bytes[0], addr->addr_bytes[1],
 				    addr->addr_bytes[2], addr->addr_bytes[3],
@@ -2823,6 +2813,10 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_ether_addr *old_addr;
+	int ret;
+
+	old_addr = (struct rte_ether_addr *)hw->mac.addr;
 
 	if (!rte_is_valid_assigned_ether_addr(mac_addr)) {
 		PMD_DRV_LOG(ERR, "Tried to set invalid MAC address.");
@@ -2832,9 +2826,13 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 	if (vf->flags & I40E_FLAG_VF_MAC_BY_PF)
 		return -EPERM;
 
-	i40evf_del_mac_addr_by_addr(dev, (struct rte_ether_addr *)hw->mac.addr);
+	if (rte_is_same_ether_addr(old_addr, mac_addr))
+		return 0;
+
+	i40evf_add_del_eth_addr(dev, old_addr, FALSE, VIRTCHNL_ETHER_ADDR_PRIMARY);
 
-	if (i40evf_add_mac_addr(dev, mac_addr, 0, 0) != 0)
+	ret = i40evf_add_del_eth_addr(dev, mac_addr, TRUE, VIRTCHNL_ETHER_ADDR_PRIMARY);
+	if (ret)
 		return -EIO;
 
 	rte_ether_addr_copy(mac_addr, (struct rte_ether_addr *)hw->mac.addr);
@@ -2878,6 +2876,7 @@ i40evf_add_del_mc_addr_list(struct rte_eth_dev *dev,
 
 		memcpy(list->list[i].addr, mc_addrs[i].addr_bytes,
 			sizeof(list->list[i].addr));
+		list->list[i].type = VIRTCHNL_ETHER_ADDR_EXTRA;
 	}
 
 	args.ops = add ? VIRTCHNL_OP_ADD_ETH_ADDR : VIRTCHNL_OP_DEL_ETH_ADDR;
-- 
2.31.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2021-05-17 17:40:34.028447685 +0200
+++ 0117-net-i40e-fix-lack-of-MAC-type-when-set-MAC-address.patch	2021-05-17 17:40:29.343810830 +0200
@@ -1 +1 @@
-From 3f604ddf33cf69d9a9a7645e67a7e0b915e76b6a Mon Sep 17 00:00:00 2001
+From 332bb84d3ed4bf0609f10a69185f1d1c9bd85234 Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 3f604ddf33cf69d9a9a7645e67a7e0b915e76b6a ]
+
@@ -45 +46,0 @@
-Cc: stable@dpdk.org
@@ -55 +56 @@
-index 9c64fd4690..648072f5bb 100644
+index acd7b7eb88..1b4e76f60b 100644
@@ -58 +59 @@
-@@ -402,9 +402,36 @@ VIRTCHNL_CHECK_STRUCT_LEN(12, virtchnl_queue_select);
+@@ -388,9 +388,36 @@ VIRTCHNL_CHECK_STRUCT_LEN(12, virtchnl_queue_select);
@@ -97 +98 @@
-index 0c9bd8d2c6..7548062934 100644
+index 56986991cd..49b5ead859 100644
@@ -110 +111 @@
-@@ -823,10 +826,9 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
+@@ -801,10 +804,9 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
@@ -124 +125 @@
-@@ -835,83 +837,70 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
+@@ -813,83 +815,70 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
@@ -235 +236 @@
-@@ -2093,6 +2082,7 @@ i40evf_add_del_all_mac_addr(struct rte_eth_dev *dev, bool add)
+@@ -2066,6 +2055,7 @@ i40evf_add_del_all_mac_addr(struct rte_eth_dev *dev, bool add)
@@ -243,2 +244 @@
-@@ -2853,15 +2843,23 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
- 			    struct rte_ether_addr *mac_addr)
+@@ -2823,6 +2813,10 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
@@ -245,0 +246 @@
+ 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
@@ -254,2 +255,3 @@
- 		return -EINVAL;
- 	}
+@@ -2832,9 +2826,13 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
+ 	if (vf->flags & I40E_FLAG_VF_MAC_BY_PF)
+ 		return -EPERM;
@@ -269 +271 @@
-@@ -2905,6 +2903,7 @@ i40evf_add_del_mc_addr_list(struct rte_eth_dev *dev,
+@@ -2878,6 +2876,7 @@ i40evf_add_del_mc_addr_list(struct rte_eth_dev *dev,