[PATCH v4 05/10] net/iavf: fix duplicate MAC addresses install

[David Marchand]

On Mon, 13 Jul 2026 at 15:14, Loftus, Ciara <ciara.loftus at intel.com> wrote:
>
> > Subject: [PATCH v4 05/10] 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>
> > ---
> >  drivers/net/intel/iavf/iavf_ethdev.c | 31 ++++++++++++++++++++--------
> >  1 file changed, 22 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/intel/iavf/iavf_ethdev.c
> > b/drivers/net/intel/iavf/iavf_ethdev.c
> > index 179d90ec55..fe54df4b9f 100644
> > --- a/drivers/net/intel/iavf/iavf_ethdev.c
> > +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> > @@ -134,6 +134,8 @@ static int iavf_dev_vlan_filter_set(struct rte_eth_dev
> > *dev,
> >  static int iavf_vlan_tpid_set(struct rte_eth_dev *dev,
> >                            enum rte_vlan_type vlan_type, uint16_t tpid);
> >  static int iavf_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> > +static uint64_t iavf_get_restore_flags(struct rte_eth_dev *dev,
> > +                                    enum rte_eth_dev_operation op);
> >  static int iavf_dev_rss_reta_update(struct rte_eth_dev *dev,
> >                                  struct rte_eth_rss_reta_entry64 *reta_conf,
> >                                  uint16_t reta_size);
> > @@ -264,6 +266,7 @@ static const struct eth_dev_ops iavf_eth_dev_ops = {
> >       .tx_done_cleanup            = iavf_dev_tx_done_cleanup,
> >       .get_monitor_addr           = iavf_get_monitor_addr,
> >       .tm_ops_get                 = iavf_tm_ops_get,
> > +     .get_restore_flags          = iavf_get_restore_flags,
> >  };
> >
> >  static int
> > @@ -284,6 +287,13 @@ iavf_tm_ops_get(struct rte_eth_dev *dev,
> >       return 0;
> >  }
> >
> > +static uint64_t
> > +iavf_get_restore_flags(__rte_unused struct rte_eth_dev *dev,
> > +                    __rte_unused enum rte_eth_dev_operation op)
> > +{
> > +     return RTE_ETH_RESTORE_ALL & ~RTE_ETH_RESTORE_MAC_ADDR;
> > +}
> > +
> >  __rte_unused
> >  static int
> >  iavf_vfr_inprogress(struct iavf_hw *hw)
> > @@ -1074,15 +1084,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);
> >
> > @@ -3424,6 +3433,10 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev,
> > bool vf_initiated_reset)
> >               if (ret)
> >                       goto error;
> >
> > +             /* after a VF reset, all mac addresses got flushed, restore them
> > */
> > +             iavf_add_del_all_mac_addr(adapter, true);
> > +             iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> > >mc_addrs_num, true);
> > +
>
> This restore block only runs if the device was started when the handler was
> entered. It might make more sense to move it into iavf_post_reset_reconfig
> which will run for VF-initiated resets regardless of started state, and for
> PF-initiated will run if the device was started on entry.
> For PF-initiated if the device was not started we return from
> iavf_handle_hw_reset() immediately, so there's no opportunity to restore
> the MAC addresses at all. It needs to be considered how to restore the macs
> in that case.
> Need to consider as well how the restore_flags and auto_reconfig features
> work with one another. If the auto_reconfig devarg is set to zero it means
> the user doesn't want any restoration of settings upon reset.

I was not aware of this knob.
It seems crazy to me to have such specific flag to disable something
that I take foregranted during the life of a DPDK port.
I really hope no application relies on the mac addresses list being
flushed during a VF reset...

While moving the code to iavf_post_reset_reconfig kind of makes sense
to me, I honesly don't know what to do wrt restore_flags.
It seems orthogonal to me.
If ethdev decides to restore the mac addresses, I don't see why some
devargs would have something to say about it.


-- 
David Marchand