On 9/22/21 10:43 AM, Huisong Li wrote: > > 在 2021/9/22 14:39, Andrew Rybchenko 写道: >> On 9/22/21 6:36 AM, Min Hu (Connor) wrote: >>> From: Huisong Li <lihuis...@huawei.com> >>> >>> Use the testpmd to perform the following operations: >>> 1) mac_addr add 0 00:18:2D:00:00:90 >>> 2) mac_addr add 0 00:18:2D:00:00:91 >>> 3) mac_addr add 0 00:18:2D:00:00:92 >>> 4) mac_addr set 0 00:18:2D:00:00:91 >>> 5) show port 0 macs >>> Number of MAC address added: 4 >>> 00:18:2D:00:00:91 >>> 00:18:2D:00:00:90 >>> 00:18:2D:00:00:91 >>> 00:18:2D:00:00:92 >>> >>> This is due to the reason that if the address has been added as a >>> non-default MAC address by rte_eth_dev_mac_addr_add API, it doesn't >>> remove >>> from dev->data->mac_addrs[] when set default MAC address with the same >>> address. >>> >>> 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> >>> --- >>> lib/ethdev/rte_ethdev.c | 15 +++++++++++++++ >>> 1 file changed, 15 insertions(+) >>> >>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >>> index daf5ca9242..77657a3314 100644 >>> --- a/lib/ethdev/rte_ethdev.c >>> +++ b/lib/ethdev/rte_ethdev.c >>> @@ -4360,6 +4360,7 @@ int >>> rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct >>> rte_ether_addr *addr) >>> { >>> struct rte_eth_dev *dev; >>> + int index; >>> int ret; >>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>> @@ -4381,6 +4382,20 @@ rte_eth_dev_default_mac_addr_set(uint16_t >>> port_id, struct rte_ether_addr *addr) >>> if (ret < 0) >>> return ret; >>> + /* >>> + * 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 NIC data structure */ >>> + rte_ether_addr_copy(&null_mac_addr, >>> + &dev->data->mac_addrs[index]); >>> + /* reset pool bitmap */ >>> + dev->data->mac_pool_sel[index] = 0; >>> + } >>> + >>> /* Update default address in NIC data structure */ >>> rte_ether_addr_copy(addr, &dev->data->mac_addrs[0]); >>> >> If I change default MAC to something else later, should the old >> default MAC be returned to some specific pools? I guess it is > > Since the old default MAC address is invalid, which has been removed > from hardware. > > This MAC address does not need to be added to some specific pools, and > occupies > > one position in mac addrs. > >> hard to do. If the change is accepted, the behaviour must be >> documented in rte_eth_dev_default_mac_addr_set() description. >> . > > I think it's not necessary. Because old default MAC is no longer valid > if we modify > > default MAC with a new MAC address. This is a definition of > rte_eth_dev_default_mac_addr_set(). > > The current modification does not change the definition of the interface.
Not sure that I understand: set default MAC-A add MAC-B to pool 1 set default MAC-B set default MAC-C since I've not removed MAC-B from pool 1, I'd expect it to be there.