|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