|WARNING| net/bonding: fix the log MAC address restore failure
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Jun 29 16:26:53 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166398
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-29
# DPDK Patch Review
## Patch Summary
This patch adds error logging when restoring a member device's MAC address fails during removal from a bonding device. The change is a simple improvement to visibility without altering control flow.
---
## Findings
### Errors
None.
### Warnings
**1. Missing error check on `member_remove_mac_addresses()`**
The patch adds error logging for `rte_eth_dev_default_mac_addr_set()` but does not check the return value of `member_remove_mac_addresses()` on the next line. If consistency is the goal, both operations should have error handling. However, this is pre-existing and not introduced by this patch, so it is a suggestion for follow-up rather than a blocking issue.
```c
/* Current code after patch */
if (rte_eth_dev_default_mac_addr_set(member_port_id,
&internals->members[member_idx].persisted_mac_addr)) {
RTE_BOND_LOG(ERR,
"Failed to restore MAC address on member port %u",
member_port_id);
}
/* member_remove_mac_addresses() return value not checked */
member_remove_mac_addresses(bonding_eth_dev, member_port_id);
/* Suggested (for future patch) */
if (member_remove_mac_addresses(bonding_eth_dev, member_port_id) != 0) {
RTE_BOND_LOG(ERR,
"Failed to remove MAC addresses from member port %u",
member_port_id);
}
```
**2. Implicit comparison**
The error check uses implicit truthiness: `if (rte_eth_dev_default_mac_addr_set(...))`. DPDK style requires explicit comparison against zero for integer return values.
```c
/* Current */
if (rte_eth_dev_default_mac_addr_set(member_port_id,
&internals->members[member_idx].persisted_mac_addr)) {
/* Preferred DPDK style */
if (rte_eth_dev_default_mac_addr_set(member_port_id,
&internals->members[member_idx].persisted_mac_addr) != 0) {
```
### Info
**1. Release notes not required**
This is an internal diagnostic improvement (adding a log message) without API or behavioral changes. Release notes are not needed per the guidelines.
**2. .mailmap addition**
The .mailmap entry is appropriate for a first-time contributor.
---
## Summary
The patch correctly identifies and logs a previously silent failure. The two warnings are minor style issues:
- Use explicit `!= 0` comparison (DPDK style)
- Consider checking `member_remove_mac_addresses()` for consistency (optional, not introduced by this patch)
Both are low-priority and do not block acceptance of the fix.
More information about the test-report
mailing list