From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8B600A0A02 for ; Mon, 17 May 2021 18:15:15 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 83641410EA; Mon, 17 May 2021 18:15:15 +0200 (CEST) Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by mails.dpdk.org (Postfix) with ESMTP id 98DB640041 for ; Mon, 17 May 2021 18:15:13 +0200 (CEST) Received: from 2.general.paelzer.uk.vpn ([10.172.196.173] helo=Keschdeichel.fritz.box) by youngberry.canonical.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.93) (envelope-from ) id 1lifth-0008BS-D9; Mon, 17 May 2021 16:15:13 +0000 From: Christian Ehrhardt To: Robin Zhang Cc: Yan Xia , dpdk stable Date: Mon, 17 May 2021 18:09:06 +0200 Message-Id: <20210517161039.3132619-117-christian.ehrhardt@canonical.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210517161039.3132619-1-christian.ehrhardt@canonical.com> References: <20210517161039.3132619-1-christian.ehrhardt@canonical.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'net/i40e: fix lack of MAC type when set MAC address' has been queued to stable release 19.11.9 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" 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 --- >From 332bb84d3ed4bf0609f10a69185f1d1c9bd85234 Mon Sep 17 00:00:00 2001 From: Robin Zhang 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 Tested-by: Yan Xia --- 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,