|SUCCESS| net/hns3: fix hard-coded default value of UMV space
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Nov 4 14:11:28 CET 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172636
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-04
# DPDK Patch Review
## ERRORS
None identified.
## WARNINGS
None identified.
## INFO
### Variable Naming Consistency
The patch renames `wanted_umv_size` to `cfg_umv_size` and introduces `def_umv_size`.
While this improves clarity about the source of each value, the local variable
`wanted_umv_size` in `hns3_init_umv_space()` retains the old name.
Consider renaming it to `requested_umv_size` or similar for consistency:
```c
uint16_t requested_umv_size;
requested_umv_size = pf->cfg_umv_size;
if (requested_umv_size == 0)
requested_umv_size = pf->def_umv_size;
if (requested_umv_size == 0)
requested_umv_size = HNS3_DEFAULT_UMV_SPACE_PER_PF;
```
This is a minor style preference and does not affect correctness.
### Reserved Field Renaming
In `hns3_dev_specs_1_cmd`, the patch splits `rsv1[11]` into `rsv1[2]`, extracts
`min_tx_pkt_len`, and adds `rsv2[11]`.
The structure layout is preserved (offsets unchanged), but the renaming of `rsv1`
to `rsv1[2]` followed by `rsv2[11]` when the original was `rsv1[11]` is slightly
unusual naming.
More conventional would be to keep the numbering sequential or describe the split:
`rsv1[2]`, then `min_tx_pkt_len`, then `rsv3[11]` (skipping `rsv2` to avoid
implying it is related to the earlier `rsv0`).
This is a style observation and does not impact correctness or ABI.
### Fallback Chain Clarity
The three-level fallback in `hns3_init_umv_space()` is correct:
1. User-specified (`cfg_umv_size`)
2. Firmware default (`def_umv_size`)
3. Hard-coded constant
The code is clear as written.
An alternative pattern would be a single assignment with ternary operators,
but the current form is more readable.
---
**Summary**: The patch correctly addresses the reported bug by reading the firmware's
default UMV size from the device specifications command and using it as a fallback
before the hard-coded constant.
The logic prevents the over-allocation issue on chips with smaller UMV tables.
Code style is compliant, and no correctness issues were found.
Review-Result: CLEAN
More information about the test-report
mailing list