diff mbox series

ethdev: fix one MAC address occupies two index in mac addrs

Message ID 20210922033630.41130-1-humin29@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers
Series ethdev: fix one MAC address occupies two index in mac addrs |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional fail Functional Testing issues
ci/iol-mellanox-Functional fail Functional Testing issues
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed

Commit Message

humin (Q) Sept. 22, 2021, 3:36 a.m. UTC 
  From: Huisong Li <lihuisong@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: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Andrew Rybchenko Sept. 22, 2021, 6:39 a.m. UTC | #1 
On 9/22/21 6:36 AM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@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: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@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
hard to do. If the change is accepted, the behaviour must be
documented in rte_eth_dev_default_mac_addr_set() description.
lihuisong (C) Sept. 22, 2021, 7:43 a.m. UTC | #2 
在 2021/9/22 14:39, Andrew Rybchenko 写道:
> On 9/22/21 6:36 AM, Min Hu (Connor) wrote:
>> From: Huisong Li <lihuisong@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: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@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.
Andrew Rybchenko Sept. 22, 2021, 8:02 a.m. UTC | #3 
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 <lihuisong@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: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@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.
lihuisong (C) Sept. 22, 2021, 9:48 a.m. UTC | #4 
在 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 <lihuisong@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: stable@dpdk.org
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Signed-off-by: Min Hu (Connor) <humin29@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.
> .

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.
Thomas Monjalon Oct. 5, 2021, 7:21 p.m. UTC | #5 
22/09/2021 05:36, Min Hu (Connor):
> From: Huisong Li <lihuisong@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

Please describe with words.
Reading similar MAC addresses is not a fun game.

> 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 <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
humin (Q) Oct. 8, 2021, 7:02 a.m. UTC | #6 
Hi, Thomas,

在 2021/10/6 3:21, Thomas Monjalon 写道:
> 22/09/2021 05:36, Min Hu (Connor):
>> From: Huisong Li <lihuisong@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
> 
> Please describe with words.
> Reading similar MAC addresses is not a fun game.

I do not catch you, could you please be
more detailed, thanks.

> 
>> 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 <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> 
> 
> 
> .
>
Thomas Monjalon Oct. 8, 2021, 10:04 a.m. UTC | #7 
08/10/2021 09:02, Min Hu (Connor):
> Hi, Thomas,
> 
> 在 2021/10/6 3:21, Thomas Monjalon 写道:
> > 22/09/2021 05:36, Min Hu (Connor):
> >> From: Huisong Li <lihuisong@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
> > 
> > Please describe with words.
> > Reading similar MAC addresses is not a fun game.
> 
> I do not catch you, could you please be
> more detailed, thanks.

Me too, I don't catch you.
Please explain the problem in the commit log
so we can understand without the example.

> >> 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 <lihuisong@huawei.com>
> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
humin (Q) Oct. 9, 2021, 9:53 a.m. UTC | #8 
Hi, Thomas,

The dev->data->mac_addrs[0] will be changed to a new MAC address when 
applications modify
the default MAC address by rte_eth_dev_default_mac_addr_set() API. 
However, If the new default
MAC address has been added as a non-default MAC address by 
rte_eth_dev_mac_addr_add() API, the
rte_eth_dev_default_mac_addr_set() API doesn't remove it from 
dev->data->mac_addrs[].
As a result, one MAC address occupies two index capacities in 
dev->data->mac_addrs[].
This patch adds the logic of removing MAC addresses for this scenario.

Is that will be more clear? Hope for your reply

在 2021/10/8 18:04, Thomas Monjalon 写道:
> 08/10/2021 09:02, Min Hu (Connor):
>> Hi, Thomas,
>>
>> 在 2021/10/6 3:21, Thomas Monjalon 写道:
>>> 22/09/2021 05:36, Min Hu (Connor):
>>>> From: Huisong Li <lihuisong@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
>>>
>>> Please describe with words.
>>> Reading similar MAC addresses is not a fun game.
>>
>> I do not catch you, could you please be
>> more detailed, thanks.
> 
> Me too, I don't catch you.
> Please explain the problem in the commit log
> so we can understand without the example.
> 
>>>> 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 <lihuisong@huawei.com>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> 
> 
> 
> .
>
Thomas Monjalon Oct. 11, 2021, 9:02 a.m. UTC | #9 
09/10/2021 11:53, Min Hu (Connor):
> Hi, Thomas,
> 
> The dev->data->mac_addrs[0] will be changed to a new MAC address when 
> applications modify
> the default MAC address by rte_eth_dev_default_mac_addr_set() API. 
> However, If the new default
> MAC address has been added as a non-default MAC address by 
> rte_eth_dev_mac_addr_add() API, the
> rte_eth_dev_default_mac_addr_set() API doesn't remove it from 
> dev->data->mac_addrs[].
> As a result, one MAC address occupies two index capacities in 
> dev->data->mac_addrs[].
> This patch adds the logic of removing MAC addresses for this scenario.
> 
> Is that will be more clear? Hope for your reply

Yes, that's the explanation I was expecting. Thank you!


> 在 2021/10/8 18:04, Thomas Monjalon 写道:
> > 08/10/2021 09:02, Min Hu (Connor):
> >> Hi, Thomas,
> >>
> >> 在 2021/10/6 3:21, Thomas Monjalon 写道:
> >>> 22/09/2021 05:36, Min Hu (Connor):
> >>>> From: Huisong Li <lihuisong@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
> >>>
> >>> Please describe with words.
> >>> Reading similar MAC addresses is not a fun game.
> >>
> >> I do not catch you, could you please be
> >> more detailed, thanks.
> > 
> > Me too, I don't catch you.
> > Please explain the problem in the commit log
> > so we can understand without the example.
> > 
> >>>> 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 <lihuisong@huawei.com>
> >>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
diff mbox series

Patch

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]);