01/02/2023 14:15, 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 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: sta...@dpdk.org
>
> Signed-off-by: Huisong Li <lihuis...@huawei.com>
> Acked-by: Chengwen Feng <fengcheng...@huawei.com>
[...]
> --- a/doc/guides/rel_notes/release_23_03.rst
> +++ b/doc/guides/rel_notes/release_23_03.rst
> @@ -101,10 +101,16 @@ API Changes
> Use fixed width quotes for ``function_names`` or ``struct_names``.
> Use the past tense.
>
> +
useless line?
> This section is a comment. Do not overwrite or remove it.
> Also, make sure to start the actual text at the margin.
> =======================================================
Please check the comment: "make sure to start the actual text at the margin."
> + * ethdev: ensured all entries in MAC address list is unique.
is -> are uniques
> + The function ``rte_eth_dev_default_mac_addr_set`` replaces the address
> + at index 0 of the address list. If the address was already in the
> + address list, it is removed from the rest of the list.
You need to highlight what changed:
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.
[...]
> --- 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.
> + */
You remember I asked to split lines after the dot?
> /**
> * 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.
Here you can split after the comma.
