|WARNING| net/ice: fix ACL entry limit on devices with 8 ports

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Nov 9 17:22:44 CET 2026


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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

**W1: Potential array access beyond runtime bounds**

Location: `drivers/net/intel/ice/ice_acl_filter.c`, line 467
```c
pf->acl.hw_entry_id[slot_id] = hw_entry;
```

The `hw_entry_id` array is declared as `uint64_t hw_entry_id[MAX_ACL_NORMAL_ENTRIES]` (fixed-size),
but the check now allows `slot_id < pf->acl.max_entries`.
If `max_entries` exceeds `MAX_ACL_NORMAL_ENTRIES` (256) on a system where the depth calculation
produces a larger value, this write will be out of bounds.

The check at line 454 (`if (slot_id < pf->acl.max_entries)`) gates the array access,
but there is no guarantee that `max_entries <= MAX_ACL_NORMAL_ENTRIES`.
The assignment at line 135 is `pf->acl.max_entries = params.depth / 2;`,
and `params.depth` is device-determined.

**Suggested fix:** Either:
1. Clamp `max_entries` to the array size:
```c
pf->acl.max_entries = RTE_MIN(params.depth / 2, MAX_ACL_NORMAL_ENTRIES);
```
2. Or add a check before the array access:
```c
if (slot_id >= MAX_ACL_NORMAL_ENTRIES) {
	PMD_DRV_LOG(ERR, "slot_id %u exceeds array bounds", slot_id);
	rte_bitmap_set(pf->acl.slots, slot_id);
	return -1;
}
pf->acl.hw_entry_id[slot_id] = hw_entry;
```

The first approach (clamping at setup time) is cleaner,
as it ensures `max_entries` is always a safe bound for both the bitmap and the array.

---

**W2: Inconsistent data types for entry count**

Location: `drivers/net/intel/ice/ice_ethdev.h`, line 501
```c
uint16_t max_entries;
```

The existing `MAX_ACL_NORMAL_ENTRIES` is 256 (an `int` constant).
Using `uint16_t` for `max_entries` is safe for that value,
but if the intent is to support larger depths in the future,
a wider type (`uint32_t`) would avoid potential truncation.

Given that `params.depth` (from the base driver) is likely a wider type,
and the assignment `pf->acl.max_entries = params.depth / 2;` could truncate,
consider using `uint32_t` for `max_entries`.

This is a minor future-proofing concern, not an immediate bug,
since current hardware appears to cap depth well below 65535.

**Suggested fix:**
```c
uint32_t max_entries;
```

And update the log format specifier at line 469 to `%u` (already done in the patch).

---

## INFO

**I1: Comment accuracy**

Location: `drivers/net/intel/ice/ice_acl_filter.c`, line 137
```c
/* Only the NORMAL priority band (50% of the scenario) is used. */
```

The comment correctly explains the 50% factor,
but "scenario" is base-driver jargon that may not be clear to readers unfamiliar with the ICE ACL subsystem.
Consider clarifying: "50% of the ACL table depth" or "50% of `params.depth`".

This is a readability suggestion, not a defect.

Review-Result: WARNING


More information about the test-report mailing list