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 74959A0C45; Wed, 22 Sep 2021 11:48:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F259841198; Wed, 22 Sep 2021 11:48:32 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 98DF641196 for ; Wed, 22 Sep 2021 11:48:31 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4HDtkm6HDNz8tLM for ; Wed, 22 Sep 2021 17:47:44 +0800 (CST) Received: from dggema767-chm.china.huawei.com (10.1.198.209) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2308.8; Wed, 22 Sep 2021 17:48:22 +0800 Received: from [10.66.74.184] (10.66.74.184) by dggema767-chm.china.huawei.com (10.1.198.209) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2308.8; Wed, 22 Sep 2021 17:48:22 +0800 To: Andrew Rybchenko , References: <20210922033630.41130-1-humin29@huawei.com> From: Huisong Li Message-ID: <42a49a10-9c24-88bf-0f4a-136bc1e3dd8c@huawei.com> Date: Wed, 22 Sep 2021 17:48:22 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.66.74.184] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggema767-chm.china.huawei.com (10.1.198.209) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH] ethdev: fix one MAC address occupies two index in mac addrs 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 Sender: "dev" 在 2021/9/22 16:02, Andrew Rybchenko 写道: > 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 >>>> >>>> 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: stable@dpdk.org >>>> >>>> Signed-off-by: Huisong Li >>>> Signed-off-by: Min Hu (Connor) >>>> --- >>>>   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. > . Yes. You are right. But MAC-B is removed from hardware after executing "set default MAC-C" cmd, and new default MAC-C takes effect. View from interface rte_eth_dev_mac_addr_remove(), if one MAC address is removed, the API removes the MAC from dev->data->mac_addrs[] and reset "mac_pool_sel"(that is, all pools do not have the MAC address.). This also means that above MAC-B should not exist in any of the pools and in dev->data->mac_addrs[] after executing "set default MAC-C" cmd.