[DPDK/ethdev Bug 1889] r8169: rtl_xmit_pkts breaks tx_burst ownership contract on invalid mbufs

[bugzilla@dpdk.org]

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

            Bug ID: 1889
           Summary: r8169: rtl_xmit_pkts breaks tx_burst ownership
                    contract on invalid mbufs
           Product: DPDK
           Version: 25.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: minor
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: stephen@networkplumber.org
  Target Milestone: ---

`rtl_xmit_pkts()` in `drivers/net/r8169/r8169_rxtx.c` returns a short
count when it encounters an invalid mbuf, violating the `rte_eth_tx_burst()`
ownership contract. This can cause callers to retry an unfixable packet
indefinitely.


## Description

In `rtl_xmit_pkts()` (r8169_rxtx.c, line ~1974), the mbuf validation
check uses `break` to exit the transmit loop when it encounters an invalid
packet:

```c
    for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) {
        tx_pkt = *tx_pkts++;

        if (txq->tx_free < tx_pkt->nb_segs)
            break;

        /* Check mbuf is valid */
        if (tx_pkt->nb_segs == 0 || tx_pkt->pkt_len == 0 ||
            (tx_pkt->nb_segs > 1 && tx_pkt->next == NULL))
            break;

        rtl_xmit_pkt(hw, txq, tx_pkt);
    }
```

The `rte_eth_tx_burst()` contract requires:

- `tx_pkts[0..n-1]`: ownership transferred to the driver (consumed).
- `tx_pkts[n..nb_pkts-1]`: untouched, still owned by the caller.

When the driver encounters an invalid mbuf at index `nb_tx` and breaks,
that invalid packet becomes the first "unconsumed" packet returned to the
caller. Applications using the standard retry pattern will attempt to
re-transmit the unfixable packet indefinitely:

```c
    /* Common application pattern that becomes an infinite loop */
    do {
        n = rte_eth_tx_burst(port, txq, &mbufs[sent], nb_pkts - sent);
        sent += n;
    } while (sent < nb_pkts);
```

Per the clarified `rte_eth_tx_burst()` semantics: if a packet cannot be
transmitted due to an error, the driver must still consume it, free the
mbuf, and count it via `tx_errors`. Stopping at the error packet is
incorrect.

Note: the `txq->tx_free < tx_pkt->nb_segs` check (ring full) correctly
uses `break` — the driver genuinely cannot accept the packet, and the
caller should retry later.

## Steps to Reproduce

1. Construct an mbuf with `pkt_len == 0` or `nb_segs == 0` or a
   multi-segment mbuf with a NULL `next` pointer.
2. Submit it in the middle of a burst to `rte_eth_tx_burst()` on an r8169
   port.
3. Observe that the return value leaves the invalid mbuf as the first
   unconsumed packet.
4. A retry loop will stall forever on that packet.

## Suggested Fix

Replace the `break` on invalid mbufs with consume-and-continue:

```c
        /* Check mbuf is valid */
        if (tx_pkt->nb_segs == 0 || tx_pkt->pkt_len == 0 ||
            (tx_pkt->nb_segs > 1 && tx_pkt->next == NULL)) {
            rte_pktmbuf_free(tx_pkt);
            txq->sw_stats.tx_errors++;
            continue;
        }
```

This ensures the erroneous packet is consumed (freed), counted in
statistics, and does not block subsequent valid packets in the burst.

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