Re: [PATCH v5 08/10] net/mlx5: pass maximum number of unicast MAC to common code

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 23 Jul 2026 14:41:54 +0200
David Marchand <david.marchand@redhat.com> wrote:

> Isolate how the MAC addresses array is walked through in the common code
> by passing the max index at which a unicast MAC address is stored in
> dev->data->mac_addrs[].
> 
> In the sync callback, the size of the array allocated on the stack is
> known by the caller, treat the mac_n field as an input parameter too.
> 
> With this change, only net/mlx5 knows about the max number of
> unicast/multicast MAC addresses.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Deeper AI review found this:

[PATCH v5 08/10] net/mlx5: mlx5_nl_mac_addr_cb no longer accumulates across netlink messages

The refactor makes mac_n a local reset to 0 on every callback invocation and writes data->mac_n = mac_n (this-call count) on exit, while data->mac_n on entry is repurposed as the array capacity.

mlx5_nl_recv() calls the callback once per nlmsghdr, and a neighbor dump delivers one RTM_NEWNEIGH (one NDA_LLADDR) per message. So with N MAC addresses on the kernel netdev, the callback runs N times, each time writing to mac[0] and reporting count 1 — every address except the last is overwritten. mlx5_nl_mac_addr_list() then returns 1, and mlx5_nl_mac_addr_sync() syncs only the last address. The previous code kept data->mac_n as a persistent running counter, so it accumulated correctly.

Confirmed with a standalone harness (3 addresses across 3 invocations): new logic reports count=1, mac[0] = the 3rd address; old logic reports count=3 at indices 0/1/2. Works only when the netdev has a single MAC, which is why it can pass basic testing — and it's precisely the many-address path that 10/10 then widens to 4096.

The capacity and the running count need to be separate. Keep an input capacity field (e.g. add mac_cap to struct mlx5_nl_mac_addr, set from *mac_n) and leave mac_n as the persistent counter that survives across invocations, checking data->mac_n == data->mac_cap for the full condition — rather than folding both roles into one field.