* [dpdk-dev] [PATCH 1/2] net/i40e: fix lack of MAC type when set MAC address
2021-03-19 9:20 [dpdk-dev] [PATCH 0/2] fix lack of MAC type when set MAC address Robin Zhang
@ 2021-03-19 9:20 ` Robin Zhang
2021-03-19 9:20 ` [dpdk-dev] [PATCH 2/2] net/iavf: " Robin Zhang
2021-04-16 2:03 ` [dpdk-dev] [PATCH 0/2] " Chen, BoX C
2 siblings, 0 replies; 5+ messages in thread
From: Robin Zhang @ 2021-03-19 9:20 UTC (permalink / raw)
To: beilei.xing, jia.guo, jingjing.wu; +Cc: dev, Robin Zhang
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>
---
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.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [dpdk-dev] [PATCH 2/2] net/iavf: fix lack of MAC type when set MAC address
2021-03-19 9:20 [dpdk-dev] [PATCH 0/2] fix lack of MAC type when set MAC address Robin Zhang
2021-03-19 9:20 ` [dpdk-dev] [PATCH 1/2] net/i40e: " Robin Zhang
@ 2021-03-19 9:20 ` Robin Zhang
2021-04-16 2:03 ` [dpdk-dev] [PATCH 0/2] " Chen, BoX C
2 siblings, 0 replies; 5+ messages in thread
From: Robin Zhang @ 2021-03-19 9:20 UTC (permalink / raw)
To: beilei.xing, jia.guo, jingjing.wu; +Cc: dev, Robin Zhang
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: cb25d4323fbf ("net/avf: enable MAC VLAN and promisc ops")
Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
---
drivers/net/iavf/iavf.h | 2 +-
drivers/net/iavf/iavf_ethdev.c | 16 +++++++---------
drivers/net/iavf/iavf_vchnl.c | 5 ++++-
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index c934d2e614..d1ae5a3f0b 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -328,7 +328,7 @@ int iavf_query_stats(struct iavf_adapter *adapter,
int iavf_config_promisc(struct iavf_adapter *adapter, bool enable_unicast,
bool enable_multicast);
int iavf_add_del_eth_addr(struct iavf_adapter *adapter,
- struct rte_ether_addr *addr, bool add);
+ struct rte_ether_addr *addr, bool add, uint8_t type);
int iavf_add_del_vlan(struct iavf_adapter *adapter, uint16_t vlanid, bool add);
int iavf_fdir_add(struct iavf_adapter *adapter, struct iavf_fdir_conf *filter);
int iavf_fdir_del(struct iavf_adapter *adapter, struct iavf_fdir_conf *filter);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 4d37722022..ca8b2013ea 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -998,7 +998,7 @@ iavf_dev_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *addr,
return -EINVAL;
}
- err = iavf_add_del_eth_addr(adapter, addr, true);
+ err = iavf_add_del_eth_addr(adapter, addr, true, VIRTCHNL_ETHER_ADDR_EXTRA);
if (err) {
PMD_DRV_LOG(ERR, "fail to add MAC address");
return -EIO;
@@ -1020,7 +1020,7 @@ iavf_dev_del_mac_addr(struct rte_eth_dev *dev, uint32_t index)
addr = &dev->data->mac_addrs[index];
- err = iavf_add_del_eth_addr(adapter, addr, false);
+ err = iavf_add_del_eth_addr(adapter, addr, false, VIRTCHNL_ETHER_ADDR_EXTRA);
if (err)
PMD_DRV_LOG(ERR, "fail to delete MAC address");
@@ -1331,17 +1331,15 @@ iavf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
struct iavf_adapter *adapter =
IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
- struct rte_ether_addr *perm_addr, *old_addr;
+ struct rte_ether_addr *old_addr;
int ret;
old_addr = (struct rte_ether_addr *)hw->mac.addr;
- perm_addr = (struct rte_ether_addr *)hw->mac.perm_addr;
- /* If the MAC address is configured by host, skip the setting */
- if (rte_is_valid_assigned_ether_addr(perm_addr))
- return -EPERM;
+ if (rte_is_same_ether_addr(old_addr, mac_addr))
+ return 0;
- ret = iavf_add_del_eth_addr(adapter, old_addr, false);
+ ret = iavf_add_del_eth_addr(adapter, old_addr, false, VIRTCHNL_ETHER_ADDR_PRIMARY);
if (ret)
PMD_DRV_LOG(ERR, "Fail to delete old MAC:"
" %02X:%02X:%02X:%02X:%02X:%02X",
@@ -1352,7 +1350,7 @@ iavf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
old_addr->addr_bytes[4],
old_addr->addr_bytes[5]);
- ret = iavf_add_del_eth_addr(adapter, mac_addr, true);
+ ret = iavf_add_del_eth_addr(adapter, mac_addr, true, VIRTCHNL_ETHER_ADDR_PRIMARY);
if (ret)
PMD_DRV_LOG(ERR, "Fail to add new MAC:"
" %02X:%02X:%02X:%02X:%02X:%02X",
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index aae5b900bb..45096cbfae 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1166,6 +1166,7 @@ iavf_add_del_all_mac_addr(struct iavf_adapter *adapter, 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],
@@ -1261,7 +1262,7 @@ iavf_config_promisc(struct iavf_adapter *adapter,
int
iavf_add_del_eth_addr(struct iavf_adapter *adapter, struct rte_ether_addr *addr,
- bool add)
+ bool add, uint8_t type)
{
struct virtchnl_ether_addr_list *list;
struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
@@ -1273,6 +1274,7 @@ iavf_add_del_eth_addr(struct iavf_adapter *adapter, struct rte_ether_addr *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));
@@ -1550,6 +1552,7 @@ iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
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.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread