Re: [PATCH v6 1/2] net/iavf: accept up to 32k unicast MAC addresses

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 9/4/2026 2:28 PM, David Marchand 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-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v5:
> - separated from series that went in next-net,
> - rebased,
> 
> Changes since v4:
> - rebased,
> 
> Changes since v2:
> - added an entry in release notes,
> - removed unneeded temp variable,
> 
> Changes since v1:
> - fixed buffer overflow on mailbox messages during port restart/VF reset,
> 

Hi David,

<snip>

>   
> -void
> -iavf_add_del_all_mac_addr(struct iavf_adapter *adapter, bool add)
> +static int
> +iavf_add_del_uc_addr_bulk(struct iavf_adapter *adapter, struct rte_ether_addr *addrs,
> +			  uint32_t nb_addrs, bool add)
>   {
> +#define IAVF_ETH_ADDR_PER_REQ \
> +	((IAVF_AQ_BUF_SZ - sizeof(struct virtchnl_ether_addr_list)) / \
> +	 sizeof(struct virtchnl_ether_addr))
>   	struct {
>   		struct virtchnl_ether_addr_list list;
> -		struct virtchnl_ether_addr addr[IAVF_NUM_MACADDR_MAX];
> -	} list_req = {0};
> -	struct virtchnl_ether_addr_list *list = &list_req.list;
> +		struct virtchnl_ether_addr addr[IAVF_ETH_ADDR_PER_REQ];
> +	} cmd_buffer;
> +#undef IAVF_ETH_ADDR_PER_REQ
> +	struct virtchnl_ether_addr_list *list = &cmd_buffer.list;
>   	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
>   	uint8_t msg_buf[IAVF_AQ_BUF_SZ] = {0};
> -	struct iavf_cmd_info args = {0};
> -	int err, i;
> -	size_t buf_len;
>   
> -	for (i = 0; i < IAVF_NUM_MACADDR_MAX; i++) {
> -		struct rte_ether_addr *addr = &adapter->dev_data->mac_addrs[i];
> -		struct virtchnl_ether_addr *vc_addr = &list->list[list->num_elements];
> +	for (uint32_t i = 0; i < nb_addrs; i++) {
> +		struct iavf_cmd_info args;
> +		uint32_t batch;
> +		int err;
>   
> -		/* ignore empty addresses */
> -		if (rte_is_zero_ether_addr(addr))
> -			continue;
> +		batch = i % RTE_DIM(cmd_buffer.addr);
> +
> +		if (batch == 0) {
> +			memset(&cmd_buffer, 0, sizeof(cmd_buffer));
> +			list->vsi_id = vf->vsi_res->vsi_id;
> +			list->num_elements = 0;
> +		}
> +
> +		memcpy(list->list[batch].addr, addrs[i].addr_bytes,
> +			sizeof(list->list[batch].addr));
> +		list->list[batch].type = VIRTCHNL_ETHER_ADDR_EXTRA;
>   		list->num_elements++;
>   
> -		memcpy(vc_addr->addr, addr->addr_bytes, sizeof(addr->addr_bytes));
> -		vc_addr->type = (list->num_elements == 1) ?
> -				VIRTCHNL_ETHER_ADDR_PRIMARY :
> -				VIRTCHNL_ETHER_ADDR_EXTRA;
> +		if (batch != RTE_DIM(cmd_buffer.addr) - 1 && i != nb_addrs - 1)
> +			continue;
> +
> +		memset(&args, 0, sizeof(args));
> +		args.ops = add ? VIRTCHNL_OP_ADD_ETH_ADDR : VIRTCHNL_OP_DEL_ETH_ADDR;
> +		args.in_args = (uint8_t *)list;
> +		args.in_args_size = sizeof(struct virtchnl_ether_addr_list) +
> +			sizeof(struct virtchnl_ether_addr) * list->num_elements;
> +		args.out_buffer = msg_buf;
> +		args.out_size = IAVF_AQ_BUF_SZ;
> +		err = iavf_execute_vf_cmd_safe(adapter, &args);
> +		if (err != 0) {
> +			PMD_DRV_LOG(ERR, "fail to execute command %s for %u macs",
> +				add ? "VIRTCHNL_OP_ADD_ETH_ADDR" : "VIRTCHNL_OP_DEL_ETH_ADDR",
> +				list->num_elements);
> +			return err;
> +		}
> +
> +		PMD_DRV_LOG(DEBUG, "executed command %s for %u macs",
> +			add ? "VIRTCHNL_OP_ADD_ETH_ADDR" : "VIRTCHNL_OP_DEL_ETH_ADDR",
> +			list->num_elements);
>   	}
>   
> -	/* for some reason PF side checks for buffer being too big, so adjust it down */
> -	buf_len = sizeof(struct virtchnl_ether_addr_list) +
> -		  sizeof(struct virtchnl_ether_addr) * list->num_elements;
> +	return 0;
> +}
>   
> -	list->vsi_id = vf->vsi_res->vsi_id;
> -	args.ops = add ? VIRTCHNL_OP_ADD_ETH_ADDR : VIRTCHNL_OP_DEL_ETH_ADDR;
> -	args.in_args = (uint8_t *)list;
> -	args.in_args_size = buf_len;
> -	args.out_buffer = msg_buf;
> -	args.out_size = IAVF_AQ_BUF_SZ;
> -	err = iavf_execute_vf_cmd_safe(adapter, &args);
> -	if (err)
> -		PMD_DRV_LOG(ERR, "fail to execute command %s",
> -				add ? "OP_ADD_ETHER_ADDRESS" : "OP_DEL_ETHER_ADDRESS");
> +void
> +iavf_add_del_all_mac_addr(struct iavf_adapter *adapter, bool add)
> +{
> +	int start = -1;
> +	int i;
> +
> +	/* Handle primary address (index 0) separately */
> +	if (!rte_is_zero_ether_addr(&adapter->dev_data->mac_addrs[0]))
> +		iavf_add_del_eth_addr(adapter, &adapter->dev_data->mac_addrs[0], add,
> +			VIRTCHNL_ETHER_ADDR_PRIMARY);
> +
> +	/* Process secondary addresses in contiguous blocks */
> +	for (i = 1; i < IAVF_UC_MACADDR_MAX; i++) {
> +		struct rte_ether_addr *addr = &adapter->dev_data->mac_addrs[i];
> +
> +		if (!rte_is_zero_ether_addr(addr)) {
> +			if (start == -1)
> +				start = i;
> +			continue;
> +		}
> +
> +		if (start != -1) {
> +			iavf_add_del_uc_addr_bulk(adapter, &adapter->dev_data->mac_addrs[start],
> +				i - start, add);
> +			start = -1;
> +		}
> +	}
> +
> +	if (start != -1) {
> +		iavf_add_del_uc_addr_bulk(adapter, &adapter->dev_data->mac_addrs[start],
> +			i - start, add);
> +	}
>   }
>   
>   int
> @@ -2304,7 +2353,7 @@ iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
>   	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
>   	uint8_t msg_buf[IAVF_AQ_BUF_SZ] = {0};
>   	uint8_t cmd_buffer[sizeof(struct virtchnl_ether_addr_list) +
> -		(IAVF_NUM_MACADDR_MAX * sizeof(struct virtchnl_ether_addr))];
> +		(IAVF_MC_MACADDR_MAX * sizeof(struct virtchnl_ether_addr))];
>   	struct virtchnl_ether_addr_list *list;
>   	struct iavf_cmd_info args;
>   	uint32_t i;

I would have preferred it if the caller managed the chunking, not the 
"add_del_addr_bulk" function. There is precedent for this style of 
refactor already [1], and I would like to keep things consistent - keep 
the loop simple (without memsets etc.), and make the caller manage how 
many addresses are being sent at once.

[1] 
https://patches.dpdk.org/project/dpdk/patch/5e6a55afa2b45e3ee5ec17af7a6c548c96e9698b.1771945933.git.anatoly.burakov@intel.com/

This specific refactor is more about removing rte_malloc, but it does 
also reorganize the loop in a way that I find to be more readable.

-- 
Thanks,
Anatoly