From 3f604ddf33cf69d9a9a7645e67a7e0b915e76b6a 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 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") Cc: stable@dpdk.org 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 9c64fd4690..648072f5bb 100644 --- a/drivers/net/i40e/base/virtchnl.h +++ b/drivers/net/i40e/base/virtchnl.h @@ -402,9 +402,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 0c9bd8d2c6..7548062934 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, @@ -823,10 +826,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); @@ -835,83 +837,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 @@ -2093,6 +2082,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], @@ -2853,15 +2843,23 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr) { 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."); return -EINVAL; } - 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); @@ -2905,6 +2903,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.20.1