|WARNING| [RFC,3/3] arm: remove no longer used RTE_ARCH_STRICT_ALIGN

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-04

# Review of DPDK Patch Series

## Patch 1/3: "eal: make unaligned really unaligned"

### Errors

None.

### Warnings

**Release notes formatting:**
The release notes entry uses first-person plural ("are now") which is acceptable, but DPDK style typically uses imperative or past tense. Consider rephrasing to match existing entries. However, this is a minor stylistic preference and the current wording is clear.

### Info

The change makes `unaligned_uint*_t` types consistently have alignment 1 on all architectures. This is a behavior change that could affect code generation (as noted in the release notes). The API change is properly documented.

---

## Patch 2/3: "net/mlx5: drop unnecessary STRICT_ALIGN"

### Errors

None.

### Warnings

**Misaligned store correctness:**
The change from `*(uint64_t *)dst` to `*(unaligned_uint64_t *)dst` is correct and necessary -- the destination `dst` is at `&dseg->inline_data[0]`, which is at offset 4 within a 16-byte aligned `dseg`, so the store is always misaligned. Using `unaligned_uint64_t` makes this well-defined. The old code was undefined behavior (unaligned write through a `uint64_t *` cast).

**Removal of `MLX5_ASSERT`:**
The assertion on `dst` alignment is removed. The comment explains this is intentional because the wider move no longer requires the alignment check (the destination is naturally misaligned). This is consistent with using `unaligned_uint64_t`.

However, **verify that the source `buf` pointer alignment is acceptable.** The code casts `src` (derived from `buf`) to `unaligned_uint64_t *` without validating that `buf` itself permits this. If `buf` originates from packet data or user input, this is fine. If `buf` has stricter alignment requirements in the caller contract, document that the function now relaxes this. (This is likely fine since the code already used `unaligned_uint64_t` on the source side.)

### Info

The removal of the `RTE_ARCH_STRICT_ALIGN` conditional simplifies the code. The explanation that ARMv8 handles unaligned access in hardware is correct, so splitting the move was unnecessary.

---

## Patch 3/3: "arm: remove no longer used RTE_ARCH_STRICT_ALIGN"

### Errors

None.

### Warnings

**Release notes missing:**
This patch removes a build configuration flag (`RTE_ARCH_STRICT_ALIGN`) that was part of the public build system configuration. This is an API-level change (build-time API) and should be documented in the release notes under "Removed Items" or similar.

The release notes for patch 1/3 mention the behavior change to `unaligned_uint*_t` but do not mention that `RTE_ARCH_STRICT_ALIGN` is being removed entirely. Add a note that this flag has been removed from the ARM build configuration.

### Info

The removal is safe given that patch 2/3 was the last consumer of `RTE_ARCH_STRICT_ALIGN` in the codebase (per the commit message stating "no longer used anywhere"). Verify this claim by searching for any remaining references to `RTE_ARCH_STRICT_ALIGN` in `lib/`, `drivers/`, and `app/`.

---

## Series-Level Comments

**Testing:**
The series makes a potentially performance-impacting change (forcing alignment-1 on all platforms, which may cause compilers to generate narrower/slower memory ops). The commit message for patch 1/3 notes this ("The compiler may generate narrower loads and stores than before"). Ensure performance validation has been done, particularly on x86 where the previous definition allowed natural alignment.

**ABI:**
The `unaligned_uint*_t` types are used in inline functions and macros but not directly in structure layouts or ABI-exposed function signatures, so this should not be an ABI break. However, if any structures use these types and are part of the public ABI, this could change alignment and break compatibility. Review usages of `unaligned_uint*_t` in public headers to confirm no ABI impact.

---

## Summary

**Correctness:** No blocking bugs found. The changes appear sound from a correctness perspective.

**Style/Process:**
- Missing release notes entry for removal of `RTE_ARCH_STRICT_ALIGN` flag (Warning)
- Release notes formatting could match DPDK style more closely (Info-level suggestion)

**Recommendation:** Address the missing release notes entry for patch 3/3. Verify no remaining usages of `RTE_ARCH_STRICT_ALIGN` exist. Consider performance testing on x86 if not already done.