|WARNING| test: add CRC length and alignment sweep

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Sun Nov 8 12:48:00 CET 2026


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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

None.

## WARNINGS

**Missing release notes (W1)**

This patch adds significant new test coverage (a length sweep test that exercises 0-512 byte inputs with 0-7 byte alignment offsets). While test-only changes typically do not require release notes, this represents a notable enhancement to CRC test coverage that validates SIMD implementations against scalar reference across a wide range of conditions. Consider adding a brief note in the test section of the release notes.

**Buffer allocation without hugepages (W2)**

Line: `uint8_t buf[512 + 8];`

A 520-byte stack array is used for test data. While acceptable for a test, consider using `rte_malloc()` for consistency with DPDK patterns, especially since the buffer is populated with random data via `rte_rand()`. Stack allocation is fine here, but heap allocation would be more typical for non-trivial buffers in DPDK tests.

**Nested loop depth (W3)**

Lines: `for (t = 0; t < RTE_DIM(types); t++) { ... for (a = 0; a < RTE_DIM(algs); a++) { ... for (off = 0; off < 8; off++) for (len = 0; len <= 512; len++)`

Four levels of nesting (t, a, off, len) reduce readability. Consider extracting the inner loops into a helper function:
```c
static int
check_alg_vs_ref(struct rte_net_crc *ctx, struct rte_net_crc *ref,
                 const uint8_t *buf, unsigned int alg, unsigned int type)
{
	unsigned int off, len;
	for (off = 0; off < 8; off++)
		for (len = 0; len <= 512; len++)
			if (rte_net_crc_calc(ctx, buf + off, len) !=
			    rte_net_crc_calc(ref, buf + off, len)) {
				printf("alg %u type %u off %u len %u\n",
				       alg, type, off, len);
				return TEST_FAILED;
			}
	return TEST_SUCCESS;
}
```

**Debug printf instead of TEST_ASSERT (W4)**

Line: `printf("alg %u type %u off %u len %u\n", algs[a], types[t], off, len);`

Per AGENTS.md guidelines, functional tests should use `TEST_ASSERT` macros for consistency with the DPDK test framework rather than bare `printf()` followed by manual `return TEST_FAILED`. The existing `crc_all_algs()` function uses `printf` as well, so this is consistent with the file's current pattern, but consider modernizing to:
```c
TEST_ASSERT_EQUAL(rte_net_crc_calc(ctx, buf + off, len),
                  rte_net_crc_calc(ref, buf + off, len),
                  "CRC mismatch: alg %u type %u off %u len %u",
                  algs[a], types[t], off, len);
```

## INFO

**Existing bug fix captured correctly (I1)**

The patch correctly fixes a pre-existing bug where Test 4's return value was not accumulated into `ret`, causing failures to be silently ignored. The fix (`ret |=` instead of bare call) is correct and matches the pattern used for other tests in the function.

**Random seed not controlled (I2)**

Line: `buf[i] = rte_rand();`

The test uses `rte_rand()` without explicitly seeding, relying on DPDK's automatic entropy initialization. This is acceptable for test purposes, but means test failures may not be reproducible unless the same random seed is used. If determinism is desired, consider adding a comment or using a fixed seed for regression testing.

**Algorithm availability not checked (I3)**

The test iterates over `RTE_NET_CRC_SSE42`, `RTE_NET_CRC_AVX512`, and `RTE_NET_CRC_NEON` without checking runtime availability. `rte_net_crc_set_alg()` will fall back to scalar if the requested algorithm is unavailable, so this is safe but may not test what was intended on platforms lacking those instructions. This matches the existing test pattern in `crc_all_algs()`, so it's consistent with the file's approach.

**Good practice: Resource cleanup on error paths (I4)**

The error path correctly frees both `ctx` and `ref` before returning `TEST_FAILED`. This prevents resource leaks when the test fails mid-iteration.

**Good practice: Test granularity (I5)**

The sweep covers alignment (0-7 byte offsets), short lengths (0 bytes), and longer buffers (up to 512 bytes), which exercises vector loop entry/exit, remainder handling, and various data patterns. This is valuable for catching off-by-one errors and alignment issues in SIMD implementations.

Review-Result: WARNING


More information about the test-report mailing list