Re: [PATCH v3 4/5] net/iavf: accept up to 32k unicast MAC addresses

[Stephen Hemminger <stephen@networkplumber.org>]

On Sun, 10 May 2026 19:03:04 +0200
David Marchand <david.marchand@redhat.com> wrote:

> E810 hardware provides 32k switch lookups.
> Thanks to this, it is possible to allow a lot more secondary mac
> addresses than what is possible today.
> 
> In practice, the maximum number of macs available per port may be lower
> and depends on usage by other (trusted?) VFs on the same PF.
> There is no way to figure out this limit but to try adding a mac address
> and get an error from the PF driver.
> 
> Mailbox exchanges are limited to IAVF_AQ_BUF_SZ, segment messages
> accordingly.
> 
> Signed-off

AI review feedback; nothing major but worth a look

Series: ethdev: VMDq cleanup and iavf: 32k MACs / duplicate install fix
Tree: dpdk main (DPDK 26.07)

Patch 2 - ethdev: skip VMDq pools unless configured
  Info
    Doxygen of rte_eth_dev_mac_addr_add() is not updated to describe the
    new constraint that pool must be 0 when VMDq is not configured. The
    function-level @param documentation and @return list should mention
    that -EINVAL is now returned when VMDq is disabled and pool != 0.

    Subtle behavior change: when VMDq is not configured, mac_pool_sel[index]
    is no longer set to RTE_BIT64(0) on add. Before this patch every added
    MAC ended up with bit 0 set; afterwards the array stays at 0. This is
    consistent with the new restore path (which now uses the VMDq flag
    rather than walking pool_mask in the !vmdq case), but the change in
    observable state of dev->data->mac_pool_sel[] could surprise an out-of-
    tree consumer. Worth a sentence in the API change note.

Patch 4 - net/iavf: accept up to 32k unicast MAC addresses
  Info
    Release notes wording: "secondary MAC addresses from 64 to 32k" is
    slightly imprecise. IAVF_NUM_MACADDR_MAX (64) was the total cap including
    the primary MAC; IAVF_UC_MACADDR_MAX (32768) is also a total cap
    including index 0. Either drop "secondary" or say "secondary MAC
    addresses from 63 to 32767".

    iavf_add_del_uc_addr_bulk() returns an int but the only caller
    (iavf_add_del_all_mac_addr) discards it. The pre-existing function was
    already void so this is no regression, but having the helper return an
    error code that is unconditionally dropped is misleading. Either
    propagate the error to the caller (and make iavf_add_del_all_mac_addr
    return int) or make the helper void.

    eth_dev_mac_restore() now iterates dev_info->max_mac_addrs which for
    iavf becomes 32768. With patch 5 applied this path is skipped via
    get_restore_flags, but any driver that increases max_mac_addrs into the
    thousands without setting RTE_ETH_RESTORE_MAC_ADDR off will pay a 32k
    rte_is_zero_ether_addr() scan on every dev_start. Not a bug here, but
    worth keeping in mind.

    sizeof(struct virtchnl_ether_addr_list) already accounts for the
    embedded list[1] slot, so
       in_args_size = sizeof(struct virtchnl_ether_addr_list) +
                      sizeof(struct virtchnl_ether_addr) * list->num_elements
    overcounts by sizeof(struct virtchnl_ether_addr). This matches what
    iavf_add_del_eth_addr() already does, and the buffer is sized to hold
    it, so it's not a bug -- noting it for completeness.

Patch 5 - net/iavf: fix duplicate MAC addresses install
  Info
    Recovery edge case: if iavf_add_del_eth_addr() for the primary fails
    during the first iavf_dev_start(), mac_primary_set stays false. On a
    subsequent VF reset, iavf_dev_start() (called from iavf_handle_hw_reset)
    will re-attempt the primary add, and the new
    iavf_add_del_all_mac_addr() call right after will add the primary again
    via VIRTCHNL_ETHER_ADDR_PRIMARY. This re-introduces a single-MAC
    double-install in a narrow failure path. Could be avoided by skipping
    the primary in iavf_add_del_all_mac_addr() when mac_primary_set is
    already true, or by having iavf_dev_start() skip the primary add when
    in_reset_recovery is set.

    No release notes entry. With Fixes: + Cc: stable this is acceptable as
    a bug fix, but the patch also introduces a new dev_op
    (get_restore_flags) for the driver and changes when MC addresses are
    re-pushed (only on VF reset, not on every dev_start). A one-line note
    in the iavf section of the release notes would help users tracking
    behaviour changes between stop/start cycles.

Patches 1 and 3 have no findings.