[DPDK/ethdev Bug 1890] r8169: rtl_xmit_pkt corrupts TX descriptor ring on zero-length mid-chain segment

[bugzilla@dpdk.org]

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

            Bug ID: 1890
           Summary: r8169: rtl_xmit_pkt corrupts TX descriptor ring on
                    zero-length mid-chain segment
           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: ---

This is not urgent bug, since giving a driver an mbuf like this is likely
an application bug.

`rtl_xmit_pkt()` in `drivers/net/r8169/r8169_rxtx.c` breaks out of its
segment loop when it encounters a zero-length segment in a multi-segment
mbuf. If earlier segments have already been written to the descriptor ring
(with `FirstFrag` set but no `LastFrag`), the ring is left in a corrupted
state with an incomplete packet owned by the hardware.


In `rtl_xmit_pkt()` (r8169_rxtx.c, line ~1624), the per-segment loop
breaks immediately when it encounters a segment with `data_len == 0`:

```c
    for (m_seg = tx_pkt; m_seg; m_seg = m_seg->next) {
        opts1 = opts[0];
        opts2 = opts[1];

        len = m_seg->data_len;

        if (len == 0)
            break;

        txd = &txq->hw_ring[tail];
        ...
        opts1 |= len;
        if (m_seg == tx_pkt)
            opts1 |= FirstFrag;
        if (!m_seg->next)
            opts1 |= LastFrag;
        ...
        txd->opts1 = rte_cpu_to_le_32(opts1);

        tail = (tail + 1) % nb_tx_desc;
        desc_count++;
        ...
    }

    txq->tx_tail += desc_count;
    txq->tx_free -= desc_count;
```

Consider a 3-segment mbuf where segment 2 (the middle one) has
`data_len == 0`:

1. **Segment 1** (first): written to descriptor ring with `FirstFrag` set.
   `DescOwn` is set, so hardware considers it owned.
2. **Segment 2**: `data_len == 0`, loop breaks.
3. **Segment 3** (last): never processed — `LastFrag` is never written.

The result is:

- The descriptor ring contains a partial packet: one or more descriptors
  with `DescOwn` and `FirstFrag` but no corresponding `LastFrag`.
- `txq->tx_tail` and `txq->tx_free` are updated to reflect the partial
  write (desc_count > 0).
- When the doorbell is rung in `rtl_xmit_pkts()`, the hardware will
  attempt to process these descriptors. The behavior is undefined — the
  hardware may hang waiting for `LastFrag`, process garbage, or trigger
  a DMA error.
- The `tx_clean` path may also malfunction since the sw_ring entries and
  descriptor states are inconsistent.

This is a data path corruption bug that could cause the transmit queue to
hang or produce undefined hardware behavior.

## Steps to Reproduce

1. Construct a multi-segment mbuf (nb_segs >= 2) where a non-first
   segment has `data_len == 0`.
2. Submit it via `rte_eth_tx_burst()` on an r8169 port.
3. The hardware receives an incomplete descriptor chain.

## Suggested Fix

The zero-length segment check should be handled before any descriptors
are written, or alternatively, zero-length segments should be skipped
with `continue` rather than aborting the entire packet.

**Option A — reject the packet before writing any descriptors** (simplest,
pairs with Bug #1 fix):

Validate all segments up front in `rtl_xmit_pkts()` before calling
`rtl_xmit_pkt()`:

```c
        /* Validate all segments before committing to descriptor ring */
        bool valid = true;
        for (struct rte_mbuf *seg = tx_pkt; seg; seg = seg->next) {
            if (seg->data_len == 0) {
                valid = false;
                break;
            }
        }
        if (!valid) {
            rte_pktmbuf_free(tx_pkt);
            txq->sw_stats.tx_errors++;
            continue;
        }
```

**Option B — skip zero-length segments** (if they are considered legal):

```c
        if (len == 0)
            continue;  /* skip empty segments instead of aborting */
```

However, Option B still needs to ensure `LastFrag` is set on the actual
last non-empty segment, which requires restructuring the `LastFrag` logic.
Option A is safer and more straightforward.

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