|SUCCESS| net/intel/common: track all RS buckets used by a Tx packet

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172697

_AI Code Review_

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

# DPDK Patch Review: net/intel/common: track all RS buckets used by a Tx packet

## ERRORS

None identified.

## WARNINGS

None identified.

## INFO

**1. Loop structure in RS bucket tracking**

Lines 631-634 use a do-while loop to record the completion descriptor ID for every RS bucket crossed by a TSO packet:

```c
do {
    txq->rs_last_id[rs_idx] = tx_last;
    if (++rs_idx == nb_rs_buckets)
        rs_idx = 0;
} while (rs_idx != next_rs_idx);
```

This pattern is correct for the stated purpose (crossing multiple buckets), but the loop body executes at least once unconditionally even when `pkt_rs_idx == next_rs_idx` (single-bucket case). The outer `if (next_rs_idx != pkt_rs_idx)` at line 627 prevents execution when no bucket boundary is crossed, so the unconditional first iteration is safe.

An alternative would be a for-loop with explicit iteration count:
```c
uint16_t rs_idx = pkt_rs_idx;
for (uint16_t i = 0; rs_idx != next_rs_idx; i++) {
    txq->rs_last_id[rs_idx] = tx_last;
    if (++rs_idx == nb_rs_buckets)
        rs_idx = 0;
}
```

However, the existing do-while is clear and correct. This is a stylistic observation, not a defect.

**2. Wrapping behavior at ring end**

Lines 625-626 handle the case where incrementing `tx_last` wraps the ring by resetting `next_rs_idx` to 0:

```c
if (next_rs_idx == nb_rs_buckets)
    next_rs_idx = 0;
```

This is correct: `tx_last` is a descriptor index in a ring of `nb_tx_desc` descriptors. The RS bucket index is `descriptor_index >> log2_rs_thresh`, which ranges from 0 to `nb_rs_buckets - 1`. When `tx_last + 1 == nb_tx_desc`, the bucket index wraps from `nb_rs_buckets - 1` to 0, not `nb_rs_buckets`. The reset is the correct wrap handling.

The wrapping inside the loop (lines 633-634) similarly ensures `rs_idx` stays in range [0, nb_rs_buckets).

**3. Patch scope**

This fix addresses a correctness bug (Tx ring stall) introduced by commit 8f4396b9f63c. The original code recorded only one `rs_last_id` entry per packet, causing cleanup failure when a TSO packet spanned multiple RS buckets. The new code correctly records the completion descriptor for every crossed bucket.

The change is minimal and targeted to the specific issue, which is appropriate for a bugfix.

Review-Result: CLEAN