|WARNING| [v2] gro: fix returned packet count after UDP fragment merge

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Fri Nov 6 09:44:44 CET 2026


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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

**Missing release notes update**

The patch fixes a correctness bug (mempool corruption and incorrect packet count) in the GRO library's public API `rte_gro_reassemble_burst()`. This is a significant bug fix that affects the behavior of a core library function and should be documented in the release notes.

**Suggested fix:**
Add an entry to `doc/guides/rel_notes/release_25_XX.rst` (or the appropriate current release notes file) under the "Fixed Issues" section:

```rst
* **gro: fixed returned packet count after UDP fragment merge**

  Fixed a bug in ``rte_gro_reassemble_burst()`` where flush-time fragment
  merges were not reflected in the returned packet count, causing mempool
  corruption when the caller freed the returned burst.
```

## Correctness Analysis

The patch addresses a real correctness bug:

**The bug:** `rte_gro_reassemble_burst()` maintains two counters:
- `nb_after_gro`: decremented only when packets merge during insertion
- `i`: the actual index tracking packets written to `pkts[]`

When `gro_udp4_merge_items()` merges fragments during the flush phase (fragments that became adjacent after an intermediate fragment arrived), those merges reduce `i` but do not update `nb_after_gro`. Returning `nb_after_gro` causes the function to report more packets than were actually written, leading to:
1. Double-free when the caller frees the surplus entries (they point to mbufs already chained into earlier packets)
2. Returning stripped fragment payloads as standalone packets

**The fix:** Return `i` instead of `nb_after_gro`. The analysis that `i == nb_pkts == nb_after_gro` when no flush occurs is correct - the change only affects the behavior when flush-time merges happen, which is exactly when the count needs correction.

**Evidence:** The commit message clearly describes the code path (flush - `gro_udp4_merge_items()` - mbufs chained - `i` decremented but `nb_after_gro` unchanged), and the one-line change directly addresses it.

## Code Quality

- The fix is minimal and surgical - changes only the return value
- In the no-flush path, `i` equals `nb_after_gro` so behavior is unchanged
- The Fixes tag correctly identifies the commit that introduced the issue (verified by the v2 changelog)
- Cc: stable is present

## Style and Format

No style issues. The change touches only a return statement.

## Testing

No test case added. While the bug is clearly a correctness issue, adding a test case that triggers flush-time fragment merging would strengthen the patch. However, this is not strictly required for a bug fix of this nature.

---

**Summary:** The patch correctly fixes a mempool corruption bug. The only issue is the missing release notes update, which is required for a bug fix that affects public API behavior.

Review-Result: WARNING


More information about the test-report mailing list