Hi Andrew and PMD maintainers,

Can you have a look?

在 2022/10/20 17:31, Huisong Li 写道:
The dev->data->mac_addrs[0] will be changed to a new MAC address when
applications modify the default MAC address by .mac_addr_set(). However,
if the new default one has been added as a non-default MAC address by
.mac_addr_add(), the .mac_addr_set() doesn't remove it from the mac_addrs
list. As a result, one MAC address occupies two indexes in the list. Like:
add(MAC1)
add(MAC2)
add(MAC3)
add(MAC4)
set_default(MAC3)
default=MAC3, filters=MAC1, MAC2, MAC3, MAC4

In addition, some PMDs, such as i40e, ice, hns3 and so on, do remove the
old default MAC when set default MAC. If user continues to do
set_default(MAC5), and the mac_addrs list is default=MAC5, filters=(MAC1,
MAC2, MAC3, MAC4). At this moment, user can still see MAC3 from the list,
but packets with MAC3 aren't actually received by the PMD.

So this patch adds a remove operation in set default MAC API documents
this behavior.

Fixes: 854d8ad4ef68 ("ethdev: add default mac address modifier")
Cc: sta...@dpdk.org

Signed-off-by: Huisong Li <lihuis...@huawei.com>
Signed-off-by: Min Hu (Connor) <humi...@huawei.com>
---
v5:
  - merge the second patch into the first patch.
  - add error log when rollback failed.

v4:
   - fix broken in the patchwork

v3:
   - first explicitly remove the non-default MAC, then set default one.
   - document default and non-default MAC address

v2:
   - fixed commit log.

---
  lib/ethdev/ethdev_driver.h |  7 ++++++-
  lib/ethdev/rte_ethdev.c    | 41 ++++++++++++++++++++++++++++++++++++--
  2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 1300acc95d..6291be783c 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -116,7 +116,12 @@ struct rte_eth_dev_data {
uint64_t rx_mbuf_alloc_failed; /**< Rx ring mbuf allocation failures */ - /** Device Ethernet link address. @see rte_eth_dev_release_port() */
+       /**
+        * Device Ethernet link address. The index zero of the array is as the
+        * index of the default address, and other indexes can't be the same
+        * as the address corresponding to index 0.
+        * @see rte_eth_dev_release_port()
+        */
        struct rte_ether_addr *mac_addrs;
        /** Bitmap associating MAC addresses to pools */
        uint64_t mac_pool_sel[RTE_ETH_NUM_RECEIVE_MAC_ADDR];
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5d5e18db1e..be4b37c9c1 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4498,7 +4498,10 @@ rte_eth_dev_mac_addr_remove(uint16_t port_id, struct 
rte_ether_addr *addr)
  int
  rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct rte_ether_addr 
*addr)
  {
+       uint64_t mac_pool_sel_bk = 0;
        struct rte_eth_dev *dev;
+       uint32_t pool;
+       int index;
        int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
@@ -4517,16 +4520,50 @@ rte_eth_dev_default_mac_addr_set(uint16_t port_id, 
struct rte_ether_addr *addr)
        if (*dev->dev_ops->mac_addr_set == NULL)
                return -ENOTSUP;
+ /*
+        * If the address has been added as a non-default MAC address by
+        * rte_eth_dev_mac_addr_add API, it should be removed from
+        * dev->data->mac_addrs[].
+        */
+       index = eth_dev_get_mac_addr_index(port_id, addr);
+       if (index > 0) {
+               /* Remove address in dev data structure */
+               mac_pool_sel_bk = dev->data->mac_pool_sel[index];
+               ret = rte_eth_dev_mac_addr_remove(port_id, addr);
+               if (ret < 0) {
+                       RTE_ETHDEV_LOG(ERR, "Delete MAC address from the MAC list of 
ethdev port %u.\n",
+                                      port_id);
+                       return ret;
+               }
+               /* Reset pool bitmap */
+               dev->data->mac_pool_sel[index] = 0;
+       }
        ret = (*dev->dev_ops->mac_addr_set)(dev, addr);
        if (ret < 0)
-               return ret;
+               goto out;
/* Update default address in NIC data structure */
        rte_ether_addr_copy(addr, &dev->data->mac_addrs[0]);
return 0;
-}
+out:
+       if (index > 0) {
+               pool = 0;
+               do {
+                       if (mac_pool_sel_bk & UINT64_C(1)) {
+                               if (rte_eth_dev_mac_addr_add(port_id, addr,
+                                                            pool) != 0)
+                                       RTE_ETHDEV_LOG(ERR, "failed to restore MAC 
pool id(%u) in port %u.\n",
+                                                      pool, port_id);
+                       }
+                       mac_pool_sel_bk >>= 1;
+                       pool++;
+               } while (mac_pool_sel_bk != 0);
+       }
+
+       return ret;
+}
/*
   * Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to find

Reply via email to