|SUCCESS| [13/13] net/mlx5: allow multiport auxiliary device probe

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172096

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-26

# DPDK Patch Review

This series reworks mlx5 driver to support probing ports from multiple rte_device's sharing a single IB context. The motivation is to support embedded switch topologies where ports belong to different PCI/auxiliary devices but share the same switch domain.

---

## PATCH 01/13: common/mlx5: rework IB port number query

**No findings.**

Clean refactoring: `mlx5_nl_portnum()` split into `mlx5_dev_info_get_portnum()` and `mlx5_dev_info_set_portnum()` to allow use without cache.

---

## PATCH 02/13: common/mlx5: allow ifindex query without cache

**No findings.**

`mlx5_nl_ifindex()` updated to accept `NULL` dev_info and skip caching when unavailable.

---

## PATCH 03/13: common/mlx5: separate netdev info from switch info

**No findings.**

`mlx5_nl_switch_info()` refactored to call new `mlx5_nl_netdev_info_get()` which handles netlink queries. Switch info derived from returned data.

---

## PATCH 04/13: common/mlx5: query parent device over Netlink

**No findings.**

Adds `IFLA_PARENT_DEV_NAME`/`IFLA_PARENT_DEV_BUS_NAME` parsing to `mlx5_nl_netdev_info_get()`. Used later to match associated devices.

---

## PATCH 05/13: common/mlx5: add check for auxiliary device

**No findings.**

Adds `mlx5_dev_is_auxiliary()` helper.

---

## PATCH 06/13: common/mlx5: gather associated devices

### Errors

**Missing error path resource cleanup (Error):**  
Line 146 in `drivers/common/mlx5/linux/mlx5_common_os.c`:
```c
assoc = associated_device_alloc(netdev_info.parent_dev_bus_name,
                                netdev_info.parent_dev_name);
if (assoc == NULL) {
    ret = -ENOMEM;
    goto release_return;
}
```
The `goto release_return` label at line 150 (added lines) frees all associated devices and the netlink sockets, which is correct. However, the code opens `nl_route` at line 112 (`mlx5_nl_init(NETLINK_ROUTE, 0)`) only if `portnum > 1`. If `portnum == 1`, `nl_route` remains `-1`, and the cleanup at line 161 (`if (nl_route >= 0) close(nl_route);`) is safe because of the conditional.

Re-reading: The resource cleanup at `release_return` is correct. All allocations (`assoc` entries) are freed via the `while` loop, and both fds are closed conditionally. No leak.

Retract this item.

---

## PATCH 07/13: common/mlx5: always use EAL device for probing

**No findings.**

Adds `probe_associated` flag to `mlx5_class_driver` and plumbs `dev` argument through `drivers_probe()` and probe callbacks.

---

## PATCH 08/13: common/mlx5: query devlink port for netdevs

**No findings.**

Adds parsing of `IFLA_DEVLINK_PORT` (bus/dev/port_index) in netdev info. Devlink port flavour query added for later use.

---

## PATCH 09/13: common/mlx5: get devlink port by port index

**No findings.**

`mlx5_nl_devlink_port_info_get()` queries devlink port by bus/dev/index. Currently parses flavour only.

---

## PATCH 10/13: common/mlx5: add check for managed device

**No findings.**

`mlx5_dev_is_managed()` checks if a device is owned by mlx5 PCI or auxiliary driver.

---

## PATCH 11/13: net/mlx5: add sibling device iteration macro

**No findings.**

`MLX5_ETH_FOREACH_SIBLING(port_id, cdev)` iterates over all ethdev ports sharing the same common device. Existing iteration macros replaced where appropriate.

---

## PATCH 12/13: net/mlx5: allow associated devices

### Errors

**`mlx5_nl_check_switch_info()` added `parent_dev_match` parameter but no callers on this patch:**  
At line 1684 in `drivers/common/mlx5/linux/mlx5_nl.c`, signature changes to:
```c
static void
mlx5_nl_check_switch_info(bool parent_dev_match,
                          bool num_vf_set,
                          struct mlx5_switch_info *switch_info)
```
This patch adds the parameter and updates the single caller at line 1846 to compute `parent_dev_match` from `mlx5_dev_matches()`. The code is correct.

Retract.

**Potential race in `check_virtual_port()` on `info` mutation (Error):**  
Line 1730-1770 in `drivers/common/mlx5/linux/mlx5_nl.c` (added):
```c
static void
check_virtual_port(struct mlx5_netdev_info *netdev_info,
                   struct mlx5_switch_info *switch_info)
{
    struct mlx5_devlink_port_info devlink_port_info = { ... };
    int nl;
    int family;
    int ret = 0;

    nl = mlx5_nl_init(NETLINK_GENERIC, 0);
    if (nl < 0) {
        DRV_LOG(DEBUG, ...);
        return;
    }
    ...
    if (ret < 0) {
        DRV_LOG(DEBUG, ...);
        goto exit_end;
    }
    if ((devlink_port_info.flags & MLX5_DEVLINK_PORT_INFO_FLAG_FLAVOUR) &&
        devlink_port_info.flavour == MLX5_DEVLINK_PORT_FLAVOUR_VIRTUAL)
        switch_info->name_type = MLX5_PHYS_PORT_NAME_TYPE_VIRTUAL;
exit_end:
    close(nl);
}
```
`switch_info` is written only when `ret >= 0` and flavour is VIRTUAL. On any failure, `switch_info` is unchanged. No race.

Retract.

**Unused ret assignment (Info, pre-existing pattern):**  
Line 1738: `int ret = 0;` is overwritten at line 1745 (`ret = mlx5_nl_devlink_family_id_get(nl);`). The initial assignment is harmless (function returns void).

---

## PATCH 13/13: net/mlx5: allow multiport auxiliary device probe

**No findings.**

`mlx5_auxiliary_fill_spawn_data()` queries portnum and fills spawn data for single-port or switch cases. Multiport SF E-Switches now supported.

---

## Summary

All patches compile individually (assumed from context).  
No correctness bugs identified after final verification pass.

---

Review-Result: CLEAN