On Wed, 6 May 2026 14:35:48 +0200
David Marchand <[email protected]> wrote:
> Since the commit 88ac4396ad29 ("ethdev: add VMDq support"),
> VMDq has been imposing a maximum number of mac addresses in the
> mac_addr_add/del API.
>
> Nowadays, new Intel drivers do not support the feature and few other
> drivers implement this feature.
>
> This series proposes to flag drivers that support the feature, and
> remove the limit of number of mac addresses for others.
>
> Next step could be to remove the VMDq pool notion from the generic API.
> However I have some concern about this, as changing the quite stable
> mac_addr_add/del API now seems a lot of noise for not much benefit.
>
AI review (manual not automated) saw these:
On Wed, 6 May 2026 14:35:49 +0200
David Marchand <[email protected]> wrote:
[PATCH v2 1/5] ethdev: skip VMDq pools unless configured
--------------------------------------------------------
Warning: duplicate-add no longer returns 0 in non-VMDq mode
} else if (vmdq) {
pool_mask = dev->data->mac_pool_sel[index];
/* Check if both MAC address and pool is already there, and do
nothing */
if (pool_mask & RTE_BIT64(pool))
return 0;
}
When index >= 0 (MAC already present) and VMDq is not configured,
the early "already there" return that existed before is now skipped
entirely. The driver's mac_addr_add op gets re-invoked and
mac_pool_sel is no longer updated for !vmdq, so subsequent calls
keep re-invoking it. Pre-patch behaviour was idempotent (return 0).
Suggested fix:
} else if (vmdq) {
pool_mask = dev->data->mac_pool_sel[index];
if (pool_mask & RTE_BIT64(pool))
return 0;
} else {
return 0; /* MAC already installed, only one pool */
}
[PATCH v2 2/5] ethdev: announce VMDq capability
-----------------------------------------------
Warning: capability advertised even when max_vmdq_pools may be 0
The capability bit is set unconditionally in several drivers where
VMDq availability is conditional on hardware variant or runtime
configuration:
- net/intel/e1000/igb_ethdev.c: max_vmdq_pools = 0 for e1000_82575,
e1000_i354 (no setting), e1000_i210, e1000_i211.
- net/intel/i40e/i40e_ethdev.c: max_vmdq_pools is gated on
(pf->flags & I40E_FLAG_VMDQ).
- net/bnxt/bnxt_ethdev.c: max_vmdq_pools is set to 0 when there
are not enough resources (see "Not enough resources to support
VMDq" path).
With this patch a user can pass mq_mode |= RTE_ETH_MQ_RX_VMDQ_FLAG
through rte_eth_dev_configure() because the new capability check
passes, but the driver has no pools to honour the request.
Suggested fix: gate the capa bit on max_vmdq_pools > 0, or set it
per-MAC-type / per-flag in each driver.
Warning: VMDq capability advertised on VFs that do not configure VMDq
ixgbevf_dev_info_get() and txgbevf_dev_info_get() now advertise
RTE_ETH_DEV_CAPA_VMDQ, but ixgbevf_dev_configure() and
txgbevf_dev_configure() only handle RTE_ETH_MQ_RX_RSS_FLAG. The
feature matrices doc/guides/nics/features/ixgbe_vf.ini and
txgbe_vf.ini do not list "VMDq = Y". A VF is itself a member of a
PF VMDq pool; advertising the capa from the VF is misleading.
Warning: ipn3ke advertises VMDq=Y in its features file but is not
updated by the patch
doc/guides/nics/features/ipn3ke.ini has "VMDq = Y" but the patch
does not add RTE_ETH_DEV_CAPA_VMDQ to ipn3ke. Either update the
driver or correct the feature matrix.
Warning: missing release note
The new RTE_ETH_DEV_CAPA_VMDQ public bit and the new rejection in
rte_eth_dev_configure() (returning -EINVAL when an application sets
a VMDq mq_mode on a non-VMDq device) are user-visible API and
behavioural changes. release_26_07.rst should mention them under
"API Changes".
[PATCH v2 3/5] ethdev: hide VMDq internal sizes
-----------------------------------------------
Warning: public defines removed without deprecation cycle
RTE_ETH_NUM_RECEIVE_MAC_ADDR and RTE_ETH_VMDQ_NUM_UC_HASH_ARRAY
were declared in the application-facing rte_ethdev.h. Moving them
to ethdev_driver.h removes them from the public API. Per the DPDK
API/ABI policy this normally needs a prior deprecation notice; at
minimum it warrants a release_26_07.rst "API Changes" entry.
[PATCH v2 4/5] net/iavf: accept up to 32k unicast MAC addresses
---------------------------------------------------------------
Warning: dead store in iavf_add_del_uc_addr_bulk()
for (uint32_t i = 0; i < nb_addrs; i++) {
size_t buf_len = sizeof(struct virtchnl_ether_addr_list) +
sizeof(struct virtchnl_ether_addr) * list->num_elements;
...
buf_len = sizeof(struct virtchnl_ether_addr_list) +
sizeof(struct virtchnl_ether_addr) * list->num_elements;
The first initialiser is unconditionally overwritten by the second
assignment before any read. Drop the initialiser (or move the
declaration to the use site).
Warning: missing release note for max_mac_addrs jump
max_mac_addrs goes from 64 to 32768 and dev_data->mac_addrs is now
allocated at RTE_ETHER_ADDR_LEN * 32768 = 192 KiB per VF port. This
is a notable behaviour change for iavf users and worth a
release_26_07.rst entry.
