[PATCH v6 2/2] net/iavf: fix duplicate MAC addresses install

[David Marchand]

On Fri, 11 Sept 2026 at 16:14, David Marchand <david.marchand at redhat.com> wrote:
>
> Hello,
>
> On Wed, 9 Sept 2026 at 11:41, Loftus, Ciara <ciara.loftus at intel.com> 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: stable at dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand at redhat.com>
> > > ---
> > > 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.

Interesting..

It's been so long I started touching this code, I am not sure at what
I tested now...

It seems I found a leak of the mac address array on VF reset.
iavf_dev_uninit + iavf_dev_init results in overwriting
dev->data->mac_addrs right?

This should break the mac restoration during a VF reset, regardless of
who does it, the driver or ethdev.

The multicast addresses look unaffected, as those are stored in the
iavf_info struct.

I could copy the mac address and reinsert the mac addresses during
iavf_post_reset_reconfig...
But I think it is simpler to just check eth->data->mac_addrs state in
iavf_dev_init.

Opinions?


-- 
David Marchand