On Fri, 11 Sept 2026 at 16:14, David Marchand <[email protected]> wrote: > > 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.
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
