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 F31F542B22; Tue, 16 May 2023 13:48:03 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79E7A410EE; Tue, 16 May 2023 13:48:03 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id C733840A8B for ; Tue, 16 May 2023 13:48:01 +0200 (CEST) Received: from kwepemm600004.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4QLDrZ4bxfzTkS8; Tue, 16 May 2023 19:43:10 +0800 (CST) Received: from [10.67.103.231] (10.67.103.231) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 16 May 2023 19:47:57 +0800 Message-ID: <9fe8b055-2530-0cc3-c5fd-6672b2f569c0@huawei.com> Date: Tue, 16 May 2023 19:47:56 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH V8] ethdev: fix one address occupies two entries in MAC addrs To: CC: , , , , , References: <20221020093102.20679-1-lihuisong@huawei.com> <20230202123625.14975-1-lihuisong@huawei.com> From: "lihuisong (C)" In-Reply-To: <20230202123625.14975-1-lihuisong@huawei.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To kwepemm600004.china.huawei.com (7.193.23.242) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Ferruh, There is no result on techboard. How to deal with this problem next? /Huisong 在 2023/2/2 20:36, 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 entries in the list. Like: > add(MAC1) > add(MAC2) > add(MAC3) > add(MAC4) > set_default(MAC3) > default=MAC3, the rest of the list=MAC1, MAC2, MAC3, MAC4 > Note: MAC3 occupies two entries. > > 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 need to ensure that the new default address is removed from the rest of > the list if the address was already in the list. > > Fixes: 854d8ad4ef68 ("ethdev: add default mac address modifier") > Cc: stable@dpdk.org > > Signed-off-by: Huisong Li > Acked-by: Chengwen Feng > --- > v8: fix some comments. > v7: add announcement in the release notes and document this behavior. > v6: fix commit log and some code comments. > 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. > --- > doc/guides/rel_notes/release_23_03.rst | 6 +++++ > lib/ethdev/ethdev_driver.h | 6 ++++- > lib/ethdev/rte_ethdev.c | 35 ++++++++++++++++++++++++-- > lib/ethdev/rte_ethdev.h | 3 +++ > 4 files changed, 47 insertions(+), 3 deletions(-) > > diff --git a/doc/guides/rel_notes/release_23_03.rst b/doc/guides/rel_notes/release_23_03.rst > index 84b112a8b1..1c9b9912c2 100644 > --- a/doc/guides/rel_notes/release_23_03.rst > +++ b/doc/guides/rel_notes/release_23_03.rst > @@ -105,6 +105,12 @@ API Changes > Also, make sure to start the actual text at the margin. > ======================================================= > > +* ethdev: ensured all entries in MAC address list are uniques. > + When setting a default MAC address with the function > + ``rte_eth_dev_default_mac_addr_set``, > + the address is now removed from the rest of the address list > + in order to ensure it is only at index 0 of the list. > + > > ABI Changes > ----------- > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > index dde3ec84ef..3994c61b86 100644 > --- a/lib/ethdev/ethdev_driver.h > +++ b/lib/ethdev/ethdev_driver.h > @@ -117,7 +117,11 @@ 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 addresses. > + * All entries are unique. > + * The first entry (index zero) is the default address. > + */ > 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 86ca303ab5..de25183619 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,44 @@ 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; > > + /* Keep address unique in 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, "Cannot remove the port %u address from the rest of list.\n", > + port_id); > + return ret; > + } > + } > 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 > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index d22de196db..2456153457 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -4356,6 +4356,9 @@ int rte_eth_dev_mac_addr_remove(uint16_t port_id, > > /** > * Set the default MAC address. > + * It replaces the address at index 0 of the MAC address list. > + * If the address was already in the MAC address list, > + * it is removed from the rest of the list. > * > * @param port_id > * The port identifier of the Ethernet device.