|WARNING| [8/8] ethdev: check unknown Rx/Tx multi-queue modes

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171539

_AI Code Review_

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

# DPDK Patch Review

## PATCH 1/8: net/bnxt: remove leftover VMDq code in MAC helpers

### Correctness

**Error**: Resource leak on error path in `bnxt_mac_addr_remove_op()`.

Line 2008 in the patched code:
```c
filter = STAILQ_FIRST(&vnic->filter);
while (filter) {
    temp_filter = STAILQ_NEXT(filter, next);
    if (filter->mac_index == index) {
        STAILQ_REMOVE(&vnic->filter, filter,
                bnxt_filter_info, next);
        bnxt_hwrm_clear_l2_filter(bp, filter);
        bnxt_free_filter(bp, filter);
    }
    filter = temp_filter;
}
```

If `bnxt_hwrm_clear_l2_filter()` fails, the function continues and calls `bnxt_free_filter()`, but the filter has already been removed from the STAILQ. If a subsequent iteration also fails, the first filter's resources may be partially leaked. The original code had the same issue, so this is pre-existing, but worth noting.

**Info (pre-existing)**: No error code is returned to the caller if `bnxt_hwrm_clear_l2_filter()` fails. The function signature is `void`, so errors are silently dropped. This is pre-existing behavior not introduced by this patch.

## PATCH 2/8: drivers/net: fix reported VMDq capability

### Correctness

**Error**: Inconsistent feature documentation in failsafe driver.

Line added to `doc/guides/nics/features/failsafe.ini`:
```ini
VMDq                 = P
```

The failsafe driver is a proxy that forwards operations to its sub-devices. It does not implement VMDq itself, nor does it populate `max_vmdq_pools`. Marking it as "P" (Partial support) is misleading. The failsafe driver should not advertise VMDq support in its feature matrix, as it performs no VMDq-specific logic. If a sub-device supports VMDq, that device's own feature file should reflect it.

### Style

Clean.

## PATCH 3/8: drivers/net: remove check on VMDq modes

### Correctness

Clean.

### Style

Clean.

## PATCH 4/8: devtools: check VMDq capability

### Correctness

**Warning**: Shell script pattern may produce false positives.

Line 82 in `devtools/check-doc-vs-code.sh`:
```bash
git grep -qE 'max_vmdq_pools[[:space:]]*=' -- "$code" ||
```

This regex matches assignment but not compound assignment operators like `+=`. If a driver uses `dev_info->max_vmdq_pools += N;` (unlikely but syntactically valid), the check would miss it. Consider `max_vmdq_pools[[:space:]]*\|?=` to match both `=` and `|=` like the RSS check below.

### Style

Clean.

## PATCH 5/8: ethdev: check DCB availability

### Correctness

Clean.

### Style

Clean.

## PATCH 6/8: drivers/net: fix reported RSS capability

### Correctness

Clean -- the changes correctly gate `flow_type_rss_offloads` on the presence of actual RSS capability (`rss_caps` or equivalent).

### Style

Clean.

## PATCH 7/8: ethdev: check RSS availability

### Correctness

Clean.

### Style

Clean.

## PATCH 8/8: ethdev: check unknown Rx/Tx multi-queue modes

### Correctness

**Warning**: Macro definition inside function scope.

Line 1529 in `lib/ethdev/rte_ethdev.c`:
```c
#define RX_MQ_MODE_KNOWN_FLAGS \
    (RTE_ETH_MQ_RX_RSS_FLAG | RTE_ETH_MQ_RX_DCB_FLAG | RTE_ETH_MQ_RX_VMDQ_FLAG)

if ((dev_conf->rxmode.mq_mode & ~RX_MQ_MODE_KNOWN_FLAGS) != 0) {
```

The macro `RX_MQ_MODE_KNOWN_FLAGS` is defined inside the function `rte_eth_dev_configure()`. While valid C, this pollutes the global preprocessor namespace for the remainder of the translation unit. Prefer moving the macro to file scope or using a `static const uint32_t` variable instead.

Suggested fix:
```c
static const uint32_t rx_mq_mode_known_flags =
    RTE_ETH_MQ_RX_RSS_FLAG | RTE_ETH_MQ_RX_DCB_FLAG | RTE_ETH_MQ_RX_VMDQ_FLAG;

if ((dev_conf->rxmode.mq_mode & ~rx_mq_mode_known_flags) != 0) {
```

Or place the `#define` before the function at file scope.

### Style

Clean otherwise.

---

## Cross-Patch Observations

**Info**: The series removes driver-specific validation of multi-queue modes that is now centralized in `rte_eth_dev_configure()`. This is architecturally sound and reduces redundant code. Ensure that the centralized checks cover all cases previously handled by individual drivers -- a manual audit of removed checks versus new ethdev checks suggests coverage is complete.

---

## Summary

- **Patch 1**: Resource leak on hwrm failure (pre-existing, Info).
- **Patch 2**: Incorrect VMDq documentation for failsafe driver (Error).
- **Patch 4**: Regex may miss `+=` operators (Warning).
- **Patch 8**: Macro defined in function scope (Warning).

Review-Result: ERROR