[DPDK/ethdev Bug 1887] bnxt: single bad packet in tx burst stalls transmission of remaining valid packets

[DPDK/ethdev Bug 1887] bnxt: single bad packet in tx burst stalls transmission of remaining valid packets

[bugzilla]

http://bugs.dpdk.org/show_bug.cgi?id=1887

            Bug ID: 1887
           Summary: bnxt: single bad packet in tx burst stalls
                    transmission of remaining valid packets
           Product: DPDK
           Version: 25.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: stephen@networkplumber.org
  Target Milestone: ---

Recent audit of tx_pkt_burst semantics in drivers found this bug.

he bnxt scalar transmit path (`_bnxt_xmit_pkts` in `bnxt_txr.c`) has two
error handling problems that violate the `rte_eth_tx_burst()` contract.

## Bug 1: break-on-error stops processing valid packets

When `bnxt_start_xmit()` returns `-EINVAL` for a malformed packet (bad IOVA,
mismatched `nb_segs`, zero `data_len`, too many TSO segments), the driver
frees the mbuf and increments a drop counter, but then breaks out of the
transmit loop. The dropped packet is correctly included in the return value,
so the immediate caller won't leak or double-free it. However, every valid
packet after the bad one is returned as unconsumed even though there may be
plenty of ring space. The caller must re-submit them in a subsequent call.
In a burst of 32 packets where packet 2 is malformed, 29 valid packets get
needlessly bounced back.

The `break` should be `continue` for the `-EINVAL` case. The `break` is only
appropriate for `-EIO` (device error) and `-ENOMEM` (ring full), where
continuing would be pointless.

```c
/* current code */
if (unlikely(rc)) {
    if (rc == -EINVAL) {
        rte_atomic_fetch_add_explicit(&txq->tx_mbuf_drop, 1,
                                       rte_memory_order_relaxed);
        dropped++;
    }
    break;
}

/* proposed fix */
if (unlikely(rc)) {
    if (rc == -EINVAL) {
        rte_atomic_fetch_add_explicit(&txq->tx_mbuf_drop, 1,
                                       rte_memory_order_relaxed);
        dropped++;
        continue;
    }
    break;
}
```

## Bug 2: partial descriptor ring state on multi-segment drop

`bnxt_start_xmit()` writes transmit descriptors and advances `tx_raw_prod`
incrementally as it walks a multi-segment packet. If validation fails partway
through the segment chain (the zero `data_len` check at line 474), execution
jumps to `drop:` which frees the mbuf but does not roll back `tx_raw_prod` or
clean up the descriptors already written for the earlier segments of that
packet. This leaves orphaned descriptors in the ring that reference a
now-freed mbuf's memory.

This second issue is harder to hit in practice since the failing check
(`data_len == 0` on a middle segment) is unusual, but when it does occur the
consequences — stale DMA addresses in the descriptor ring — are severe.

## Scope

Both bugs are in the scalar path only. The vector paths (`bnxt_xmit_pkts_vec`
and AVX2 variant) skip per-packet validation entirely and are not affected.

-- 
You are receiving this mail because:
You are the assignee for the bug.