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 C7F8C42B25; Wed, 17 May 2023 09:45:33 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B578C4114A; Wed, 17 May 2023 09:45:33 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 537C14067B; Wed, 17 May 2023 09:45:31 +0200 (CEST) Received: from kwepemm600004.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4QLlTY2kPvzsRqr; Wed, 17 May 2023 15:43:29 +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; Wed, 17 May 2023 15:45:28 +0800 Message-ID: <6d809c54-ce59-2564-3cfb-7ffe681c8a7f@huawei.com> Date: Wed, 17 May 2023 15:45:28 +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: Ferruh Yigit CC: , , , , , , "techboard@dpdk.org" References: <20221020093102.20679-1-lihuisong@huawei.com> <20230202123625.14975-1-lihuisong@huawei.com> <9fe8b055-2530-0cc3-c5fd-6672b2f569c0@huawei.com> From: "lihuisong (C)" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) 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 在 2023/5/16 22:13, Ferruh Yigit 写道: > On 5/16/2023 12:47 PM, lihuisong (C) wrote: >> Hi Ferruh, >> >> There is no result on techboard. >> How to deal with this problem next? > +techboard for comment. > > > Btw, what was your positioning to Bruce's suggestion, > when a MAC address is in the list, fail to set it as default and enforce > user do the corrective action (delete MAC explicitly etc...). If a MAC address is in the list, rte_eth_dev_default_mac_addr_set returns failure? > If you are OK with it, that is good for me too, unless techboard objects > we can proceed with that one. > > >> /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. > .