|WARNING| event/cnxk: fix new-event burst enqueue with LMTST

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Correctness Issues

### Error: Missing error check propagation

**cn10k_worker.c line 401:**
```c
if (!cn10k_sso_hws_new_event(ws, &ev[idx]))
    break;
```

**cn20k_worker.c line 357:**
```c
if (!cn20k_sso_hws_new_event(ws, &ev[idx]))
    break;
```

The loop breaks on the first failed event but does not verify that at least one event succeeded. If `cn10k_sso_hws_new_event()` fails on the very first iteration (`idx == 0`), the function returns 0, which may be indistinguishable from a case where space is exhausted. The caller cannot tell whether the failure was due to back-pressure (retriable) or a per-event error (non-retriable).

**Suggested fix:** This pattern may be intentional if the caller is expected to retry the entire burst. However, if `cn10k_sso_hws_new_event()` can fail for reasons other than back-pressure, consider distinguishing the error cases or documenting the contract that a return of 0 means "retry later."

## Style and Process Issues

### Warning: Patch modifies two files but lacks detail in commit message

The commit message states "Add an I/O write barrier after LMTST submissions" and "Enqueue small bursts... event by event instead of using the LMTST path," but does not explain:

1. What observable bug the barrier fixes (e.g., "events lost under high load" or "descriptor corruption observed")
2. Why `BIT(ROC_SSO_AW_PER_LMT_LINE_LOG2)` is the correct threshold for switching to the event-by-event path
3. Whether the two changes (barrier + small-burst bypass) are independent fixes or interdependent

**Suggested improvement:** Expand the commit message to describe the failure mode that necessitated the barrier and the rationale for the small-burst threshold. This helps reviewers verify the fix is complete and helps future maintainers understand the constraints.

### Info: Release notes missing

The patch fixes two issues (`Fixes:` tags reference commits introducing bugs), but does not update release notes. If this is a bug fix targeting a stable branch or the next release, the fix should be documented in `doc/guides/rel_notes/release_XX_XX.rst` under "Fixed Issues."

**Suggested action:** Add a release note entry summarizing the fix:
```rst
* **event/cnxk: Fixed event loss in new-event burst enqueue.**

  Added memory barrier after LMTST submission to prevent reordering.
  Small bursts now bypass LMTST path to avoid partial submission errors.
```

### Info: Memory barrier ordering - verify sufficiency of `rte_io_wmb()`

**cn10k_worker.c line 353, cn20k_worker.c line 311:**
```c
rte_io_wmb();
```

`rte_io_wmb()` orders stores to I/O memory (MMIO) but is a no-op for normal memory on most architectures. If the LMTST operation writes to device MMIO and the barrier is intended to flush pending stores to that MMIO region before the next iteration begins, this is correct.

However, if the barrier is meant to order stores to *normal memory* (e.g., descriptor ring entries in DRAM) before the LMTST operation itself, `rte_wmb()` or a release-ordered atomic store would be more appropriate.

**Verification needed:** Confirm that LMTST writes go to MMIO and that the intent is to flush those MMIO writes. If LMTST writes to memory, the barrier type may need adjustment.

## Positive Observations

- The small-burst bypass (lines 400-407 in cn10k, 355-362 in cn20k) is a reasonable optimization: for bursts smaller than one LMTST line, the overhead of setting up LMTST may exceed the cost of individual event submissions.
- The barrier placement after all LMTST submissions in the loop is appropriate if the goal is to ensure prior stores complete before the next iteration.

---

**Summary:**
- One **Error**: Missing clarity on error vs. back-pressure when small-burst loop returns 0.
- Two **Warnings**: Commit message lacks failure-mode detail; missing release notes.
- One **Info**: Verify `rte_io_wmb()` is the correct barrier for LMTST ordering.

Review-Result: ERROR