On Mon, Sep 07, 2026 at 03:21:27PM +0200, David Marchand wrote: > On Mon, 7 Sept 2026 at 14:57, Bruce Richardson > <[email protected]> wrote: > > > > On Mon, Sep 07, 2026 at 01:17:23PM +0200, David Marchand wrote: > > > MAC addresses removal was tied with VMDq pools even when not used. > > > So if VMDq is not enabled, no address would be ever removed. > > > > > > Fixes: 9de506a6c781 ("ethdev: skip VMDq pools unless configured") > > > > > > Signed-off-by: David Marchand <[email protected]> > > > --- > > > I could not test the change as my setups are KO atm. > > > > > > Probably worth squashing in 9de506a6c781 before pulling to main > > > > > > Changes since v1: > > > - fixed compilation, > > > > > > --- > > > drivers/net/intel/i40e/i40e_ethdev.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/intel/i40e/i40e_ethdev.c > > > b/drivers/net/intel/i40e/i40e_ethdev.c > > > index b6b2d291ee..3c01354c39 100644 > > > --- a/drivers/net/intel/i40e/i40e_ethdev.c > > > +++ b/drivers/net/intel/i40e/i40e_ethdev.c > > > @@ -4509,11 +4509,16 @@ i40e_macaddr_remove(struct rte_eth_dev *dev, > > > uint32_t index) > > > struct rte_ether_addr *macaddr; > > > int ret; > > > uint32_t i; > > > + bool vmdq; > > > uint64_t pool_sel; > > > > > > macaddr = &(data->mac_addrs[index]); > > > > > > - pool_sel = dev->data->mac_pool_sel[index]; > > > + vmdq = (dev->data->dev_conf.rxmode.mq_mode & > > > RTE_ETH_MQ_RX_VMDQ_FLAG) != 0; > > > + if (!vmdq) > > > + pool_sel = 1; > > > + else > > > + pool_sel = dev->data->mac_pool_sel[index]; > > > > > The explanation makes sense, but do we really need the new temporary > > variable, rather than just checking the flag directly in the "if"? > > > > Also, the rest of the code in this function, and the add function about it, > > uses "pf->flags & I40E_FLAG_VMDQ" as a check for vmdq support. Is there a > > reason we can't use that flag also here, rather than checking the rx > > mq_mode flags? > > No, I just did not notice this internal flag and I simply used the > ethdev level config. > > What do you think of: > > $ git diff next-net/for-main -- drivers/net/intel/i40e > diff --git a/drivers/net/intel/i40e/i40e_ethdev.c > b/drivers/net/intel/i40e/i40e_ethdev.c > index b6b2d291ee..0d914d86ea 100644 > --- a/drivers/net/intel/i40e/i40e_ethdev.c > +++ b/drivers/net/intel/i40e/i40e_ethdev.c > @@ -4513,6 +4513,12 @@ i40e_macaddr_remove(struct rte_eth_dev *dev, > uint32_t index) > > macaddr = &(data->mac_addrs[index]); > > + if (!(pf->flags & I40E_FLAG_VMDQ)) { > + if (i40e_vsi_delete_mac(pf->main_vsi, macaddr) != 0) > + PMD_DRV_LOG(ERR, "Failed to remove MACVLAN filter"); > + return; > + } > + > pool_sel = dev->data->mac_pool_sel[index]; > > for (i = 0; i < sizeof(pool_sel) * CHAR_BIT; i++) { > @@ -4521,8 +4527,7 @@ i40e_macaddr_remove(struct rte_eth_dev *dev, > uint32_t index) > vsi = pf->main_vsi; > else { > /* No VMDQ pool enabled or configured */ > - if (!(pf->flags & I40E_FLAG_VMDQ) || > - (i > pf->nb_cfg_vmdq_vsi)) { > + if (i > pf->nb_cfg_vmdq_vsi) { > PMD_DRV_LOG(ERR, > "No VMDQ pool > enabled/configured"); > return; > > That looks simpler. Thanks.
/Bruce
