> Subject: [PATCH v6 2/2] net/iavf: fix duplicate MAC addresses install
>
> On port restart, all MAC addresses get pushed *twice* to the hardware,
> once by the driver and once by the eth_dev_mac_restore() in ethdev.
>
> On the other hand, MAC address filters are reset in the hardware
> by the PF only when a VF reset is triggered.
>
> Strictly speaking, the mac restore on port (re)start is unneeded,
> if no VF reset happened, so we can announce to ethdev that no mac
> restoration is needed via a get_restore_flags callback.
>
> Then, move the mac restoration to the VF reset handler.
>
> Fixes: 3d42086def30 ("net/iavf: preserve MAC address with i40e PF Linux
> driver")
> Cc: [email protected]
>
> Signed-off-by: David Marchand <[email protected]>
> ---
> Changes since v4:
> - rebased on next-net-intel,
>
> Changes since v4:
> - moved mac restoration in iavf_post_reset_reconfig,
>
> ---
> drivers/net/intel/iavf/iavf_ethdev.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> b/drivers/net/intel/iavf/iavf_ethdev.c
> index bbd1f08ff0..bec7b3b6d7 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -292,11 +292,12 @@ iavf_get_restore_flags(__rte_unused struct
> rte_eth_dev *dev,
> __rte_unused enum rte_eth_dev_operation op)
> {
> /*
> - * The unicast and multicast promiscuous settings persist across a
> + * The mac addresses, unicast and multicast promiscuous settings
> persist across a
> * stop/start; they are only cleared by a VF reset, which the driver
> * restores itself. So ethdev does not need to restore them on start.
> */
> - return RTE_ETH_RESTORE_ALL & ~(RTE_ETH_RESTORE_PROMISC |
> + return RTE_ETH_RESTORE_ALL & ~(RTE_ETH_RESTORE_MAC_ADDR |
> + RTE_ETH_RESTORE_PROMISC |
> RTE_ETH_RESTORE_ALLMULTI);
> }
>
> @@ -1095,15 +1096,14 @@ iavf_dev_start(struct rte_eth_dev *dev)
> rte_intr_enable(intr_handle);
> }
>
> - /* Set all mac addrs */
> - iavf_add_del_all_mac_addr(adapter, true);
> -
> - if (!adapter->mac_primary_set)
> - adapter->mac_primary_set = true;
> -
> - /* Set all multicast addresses */
> - iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> >mc_addrs_num,
> - true);
> + if (!adapter->mac_primary_set) {
> + if (iavf_add_del_eth_addr(adapter, &dev->data-
> >mac_addrs[0], true,
> + VIRTCHNL_ETHER_ADDR_PRIMARY) != 0)
> + PMD_DRV_LOG(ERR, "failed to add primary MAC:"
> RTE_ETHER_ADDR_PRT_FMT,
> + RTE_ETHER_ADDR_BYTES(&dev->data-
> >mac_addrs[0]));
> + else
> + adapter->mac_primary_set = true;
> + }
>
> rte_spinlock_init(&vf->phc_time_aq_lock);
>
> @@ -3434,6 +3434,11 @@ iavf_post_reset_reconfig(struct rte_eth_dev
> *dev)
> int ret = 0;
> bool allmulti = false, allunicast = false;
> struct iavf_adapter *adapter = IAVF_DEV_PRIVATE_TO_ADAPTER(dev-
> >data->dev_private);
> + struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> >dev_private);
> +
> + /* After a VF reset, all MAC addresses got flushed, restore them. */
> + iavf_add_del_all_mac_addr(adapter, true);
Could this lead to a double install of the primary on the reset path? In
iavf_handle_hw_reset, dev_start can run before
iavf_post_reset_reconfig and can install the primary MAC and set
mac_primary_set. iavf_post_reset_reconfig then calls
iavf_add_del_all_mac_addr which can install the primary again as it
doesn't check mac_primary_set.
> + iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> >mc_addrs_num, true);
>
> /* Restore pre-reset unicast promiscuous and multicast promiscuous
> states */
> if (dev->data->promiscuous)
> --
> 2.54.0
