|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

# DPDK Patch Review

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

### Errors

None.

### Warnings

None.

### Info

This patch makes the `unaligned_uint*_t` types truly unaligned on all architectures, not just strict-alignment platforms. The change is well-motivated (UBSAN failures) and properly documented in the release notes. The API change note correctly warns that the compiler may generate narrower loads/stores.

---

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

### Errors

**Destination pointer cast without verification:**
The line `*(unaligned_uint64_t *)dst = *(unaligned_uint64_t *)src;` casts `dst` (which has type `uintptr_t`) to `unaligned_uint64_t *` without verifying that `dst` actually points to a properly typed object. While the commit message states the destination is at offset 4 of a 16-byte aligned structure (making it 4-byte aligned), casting arbitrary integer addresses to typed pointers and dereferencing them is fragile. If `dst` or `src` do not actually point to objects of the appropriate type, this is undefined behavior under C's strict aliasing rules.

**Suggested fix:**
Use `memcpy()` for the unaligned copy instead of pointer casts. Modern compilers optimize `memcpy()` to a single load/store when the size is known at compile time, and it is the standards-conformant way to perform unaligned or type-punned access:

```c
if (len & 0x08) {
	memcpy((void *)dst, (const void *)src, sizeof(uint64_t));
	dst += sizeof(uint64_t);
	src += sizeof(uint64_t);
}
```

This avoids both the strict aliasing issue and the need for the `unaligned_uint64_t` type at this call site.

### Warnings

**Removed assertion without explanation:**
The patch removes the assertion `MLX5_ASSERT(dst == RTE_PTR_ALIGN(dst, sizeof(uint32_t)));` that previously verified 4-byte alignment of the destination when `RTE_ARCH_STRICT_ALIGN` was defined. The commit message states "The debug assertion on the inline data offset goes away with the strict alignment path since the wider move has no such requirement," but a 64-bit store to a 4-byte-aligned address on some architectures may still benefit from an alignment check in debug builds. If the assertion was validating a structural invariant (that `inline_data` is always 4-byte aligned), consider retaining it or documenting why it is safe to remove.

---

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

### Errors

None.

### Warnings

**Incomplete removal verification:**
The patch removes `RTE_ARCH_STRICT_ALIGN` from the arm meson configuration, stating "The RTE_ARCH_STRICT_ALIGN flag is no longer used anywhere in the DPDK tree." However, the review only sees the patch itself, not a verification that all uses have been removed. Before merging, confirm:
- `git grep -r RTE_ARCH_STRICT_ALIGN` returns no matches outside of deleted code
- No driver base directories (which may use different conventions per the guidelines) still reference it
- Documentation and comments mentioning strict alignment are updated

---

## General Comments

**Series correctness:**
The series logically progresses: (1) changes the EAL type definitions, (2) adapts a consumer of those types, (3) removes the now-unused configuration flag. Each patch compiles independently assuming the changes in prior patches are applied.

**Portability:**
The commit message for patch 2/3 claims "ARMv8 does unaligned access in hardware" as justification for removing the split move. While ARMv8 AArch64 does support unaligned access by default, the removed code path was for `armv8_aarch32` (32-bit ARM on ARMv8 cores), and the patch should verify that all targeted ARMv8 AArch32 configurations also handle unaligned 64-bit loads/stores efficiently. The assertion removal and the switch from separate 32-bit moves to a single 64-bit move should be tested on actual AArch32 hardware if available.

**Documentation consistency:**
Patch 1/3 correctly updates the release notes for the API change. Patches 2/3 and 3/3 are internal cleanups and do not require release notes entries.