Hello,

On Wed, 9 Sept 2026 at 11:41, Loftus, Ciara <[email protected]> wrote:
>
> > 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.

Indeed good catch, I'll fix it in next revision.


-- 
David Marchand

Reply via email to