|WARNING| net/bnxt: fix promiscuous mode set

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review: net/bnxt: fix promiscuous mode set

## Summary
This patch adds a platform check to skip promiscuous mode configuration on unsupported Whitney platforms, allowing only Thor (P5) and Thor2 (P7) platforms to proceed.

---

## Errors

None identified.

---

## Warnings

**1. Inappropriate return value for skipped operation**

The function `bnxt_ulp_promisc_mode_set()` returns `rc` (which is initialized to 0) when the platform is unsupported. This silently succeeds, making the operation appear successful to the caller when it was actually skipped.

**Why it matters:** Callers cannot distinguish between "operation succeeded" and "operation was not performed because platform doesn't support it". This could mask configuration issues.

**Suggested fix:**

Either:
- Return `-ENOTSUP` to indicate the operation is not supported on this platform:
  ```c
  /* This mode set is supported only on thor and thor2 */
  if (!BNXT_CHIP_P5_P7(bp))
      return -ENOTSUP;
  ```

Or:
- Add a log message if returning 0 is intentional (so operators know the operation was skipped):
  ```c
  /* This mode set is supported only on thor and thor2 */
  if (!BNXT_CHIP_P5_P7(bp)) {
      PMD_DRV_LOG_LINE(DEBUG, "Promiscuous mode set not supported on this platform");
      return rc;
  }
  ```

---

**2. Missing context in commit message**

The commit message states "Skip the operation on unsupported platforms instead of proceeding" but does not explain what the previous behavior was or why it was problematic (did it cause errors? fail silently? corrupt state?).

**Suggested improvement:** Add one sentence explaining the bug being fixed, e.g.:
```
Promiscuous mode set is only supported on Thor and Thor2 platforms.
The code previously attempted the operation on Whitney, causing
[firmware errors / HWRM command failures / incorrect behavior].
Skip the operation on unsupported platforms instead.
```

---

## Info

**1. Code location and maintainability**

The existing early-return check (`if (bp == NULL || !BNXT_TRUFLOW_EN(bp) || !bp->ulp_ctx)`) already establishes the pattern of silently returning 0 for unsupported configurations. The new platform check follows this pattern consistently.

If the intention is for unsupported operations to be silent (common in PMD code where applications may call operations that aren't universally supported), the current implementation is acceptable. The warning about return value is only relevant if the caller needs to know the operation was skipped.

**2. Style compliance**

The patch follows DPDK C coding style:
- Comment style is correct
- Indentation and brace placement are consistent with surrounding code
- Explicit comparison `!BNXT_CHIP_P5_P7(bp)` is appropriate

---

## Final Assessment

The patch is functionally correct if silent skipping of unsupported operations is the intended behavior for this driver. The primary concern is whether callers should be informed that the operation was not performed (via return code or log message). This is a design decision that depends on the broader driver architecture and how this function is used.