在 2023/2/3 5:10, Thomas Monjalon 写道:
02/02/2023 19:09, Ferruh Yigit:
On 2/2/2023 12:36 PM, Huisong Li wrote:
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.
Same comment from past seems already valid, I am not looking to the set
for a while, sorry if this is already discussed and decided,
if not, I am referring to the side effect that setting MAC addresses
cause to remove MAC addresses, think following case:
add(MAC1) -> MAC1
add(MAC2) -> MAC1, MAC2
add(MAC3) -> MAC1, MAC2, MAC3
add(MAC4) -> MAC1, MAC2, MAC3, MAC4
set(MAC3) -> MAC3, MAC2, MAC4
set(MAC4) -> MAC4, MAC2
set(MAC2) -> MAC2
I am not exactly clear what is the intention with set(),
That's the problem, nobody is clear with the current behavior.
The doc says "Set the default MAC address." and nothing else.
Indeed. But we can see the following information.
From the ethdev layer, this set() API always replaces the old default
address (index 0) without adding the old one.
From the PMD layer, set() interface of some PMDs, such as i40e, ice,
hns3 and so on (as far as I know),
also do remove the hardware entry of the old default address.
if there is
single MAC I guess intention is to replace it with new one, but if there
are multiple MACs and one of them are already in the list intention may
be just to change the default MAC.
The assumption in this patch is that "Set" means "Replace", not "Swap".
So this patch takes the approach 1/ Replace and keep Unique.
If above assumption is correct, what about following:
set(MAC) {
if only_default_mac_exist
replace_default_mac
if MAC exists in list
swap MAC and list[0]
else
replace_default_mac
}
This approach 2/ is a mix of Swap and Replace.
The old default MAC destiny depends on whether
we have added the new MAC as "secondary" before setting as new default.
This swap prevents removing MAC side affect, does it make sense?
Another approach would be 3/ to do an "Always Swap"
even if the new MAC didn't exist before,
you keep the old default MAC as a secondary MAC.
And the current approach 0/ is to Replace default MAC address
without touching the secondary addresses at all.
So we have 4 choices.
We could vote, roll a dice, or find a strong argument?
According to the implement of set() in ethdev and PMD layer, it always
use "Replace", not "Swap".
If we use "Swap" now, the behavior of this API will be changed.
I'm not sure if the application can accept this change or has other effects.
BTW, it seems that the ethernet port in kernel also replaces the old
address if we modify the one.
Use the test command: ifconfig eth0 hw ether new_mac
.
