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

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

On 9/10/2026 12:24 PM, Burakov, Anatoly wrote:
> 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.
> 

I tried prototyping a loop, and realized that the fact that MAC address 
list has holes in it is making things a little difficult, but here's 
what I came up with as an alternative implementation, I think it's a 
little clearer:

```
#define IAVF_ETH_ADDR_PER_REQ \
	((IAVF_AQ_BUF_SZ - sizeof(struct virtchnl_ether_addr_list)) / \
	 sizeof(struct virtchnl_ether_addr))

struct iavf_eth_addr_cmd {
	struct virtchnl_ether_addr_list list;
	struct virtchnl_ether_addr extra[IAVF_ETH_ADDR_PER_REQ];
};

static int
iavf_send_uc_addr_list(struct iavf_adapter *adapter,
		       struct virtchnl_ether_addr_list *list, bool add)
{
	const char *opname = add ? "VIRTCHNL_OP_ADD_ETH_ADDR" : 
"VIRTCHNL_OP_DEL_ETH_ADDR";
	uint8_t msg_buf[IAVF_AQ_BUF_SZ] = {0};
	struct iavf_cmd_info args = {0};
	int err;

	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",
			opname, list->num_elements);
	else
		PMD_DRV_LOG(DEBUG, "executed command %s for %u macs",
			opname, list->num_elements);

	return err;
}

void
iavf_add_del_all_mac_addr(struct iavf_adapter *adapter, bool add)
{
	struct rte_ether_addr *addrs = adapter->dev_data->mac_addrs;
	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
	uint32_t idx = 1;

	/* Handle primary address (index 0) separately */
	if (!rte_is_zero_ether_addr(&addrs[0]))
		iavf_add_del_eth_addr(adapter, &addrs[0], add,
			VIRTCHNL_ETHER_ADDR_PRIMARY);

	/* the secondary address list is sparse, so gather it into full batches */
	while (idx < IAVF_UC_MACADDR_MAX) {
		struct iavf_eth_addr_cmd cmd = {0};
		uint16_t nb_addrs = 0;

		for (; idx < IAVF_UC_MACADDR_MAX && nb_addrs < IAVF_ETH_ADDR_PER_REQ; 
idx++) {
			if (rte_is_zero_ether_addr(&addrs[idx]))
				continue;

			memcpy(cmd.list.list[nb_addrs].addr, addrs[idx].addr_bytes,
				sizeof(cmd.list.list[nb_addrs].addr));
			cmd.list.list[nb_addrs].type = VIRTCHNL_ETHER_ADDR_EXTRA;
			nb_addrs++;
		}

		if (nb_addrs == 0)
			break;

		cmd.list.vsi_id = vf->vsi_res->vsi_id;
		cmd.list.num_elements = nb_addrs;
		if (iavf_send_uc_addr_list(adapter, &cmd.list, add) != 0)
			break;
	}
}
```

-- 
Thanks,
Anatoly