|WARNING| [2/2] test/af_packet: add test for fanout modes

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-23

# DPDK Patch Review

## PATCH 1/2: net/af_packet: do not set rollover flag in rollover mode

### Errors

None identified.

### Warnings

None identified.

### Info

**Clarity of comment**: The comment "setting both is rejected by kernel" is terse. Consider expanding slightly for future maintainers:

```c
/* The kernel rejects configurations with both PACKET_FANOUT_ROLLOVER mode
 * and PACKET_FANOUT_FLAG_ROLLOVER flag set, so clear the flag when using
 * rollover mode.
 */
load_balance &= ~PACKET_FANOUT_FLAG_ROLLOVER;
load_balance |= PACKET_FANOUT_ROLLOVER;
```

This would make the reason clearer without requiring readers to consult git history or kernel documentation.

---

## PATCH 2/2: test/af_packet: add test for fanout modes

### Errors

**Resource leak on error path**: The test allocates a vdev with `rte_vdev_init()` but does not clean it up if `rte_eth_dev_get_port_by_name()` fails.

```c
ret = rte_vdev_init("net_af_packet_fanout", devargs);
TEST_ASSERT(ret == 0,
            "Failed to create af_packet sockets with fanout_mode=%s: %d",
            modes[i], ret);

ret = rte_eth_dev_get_port_by_name("net_af_packet_fanout", &test_port);
/* ERROR: if ret != 0, TEST_ASSERT below returns without calling uninit */
if (ret == 0)
    rte_eth_dev_close(test_port);
rte_vdev_uninit("net_af_packet_fanout");
TEST_ASSERT(ret == 0,
            "af_packet port with fanout_mode=%s has no port id: %d",
            modes[i], ret);
```

If `rte_eth_dev_get_port_by_name()` returns non-zero, the `TEST_ASSERT` fires and returns immediately, skipping `rte_vdev_uninit()`. This leaks the vdev.

**Suggested fix**: Move the assertion before any early return path, or ensure cleanup happens before returning:

```c
ret = rte_vdev_init("net_af_packet_fanout", devargs);
TEST_ASSERT(ret == 0,
            "Failed to create af_packet sockets with fanout_mode=%s: %d",
            modes[i], ret);

ret = rte_eth_dev_get_port_by_name("net_af_packet_fanout", &test_port);
TEST_ASSERT(ret == 0,
            "af_packet port with fanout_mode=%s has no port id: %d",
            modes[i], ret);

rte_eth_dev_close(test_port);
rte_vdev_uninit("net_af_packet_fanout");
```

Or wrap the cleanup:

```c
ret = rte_eth_dev_get_port_by_name("net_af_packet_fanout", &test_port);
if (ret == 0)
    rte_eth_dev_close(test_port);
rte_vdev_uninit("net_af_packet_fanout");  /* Always uninit before checking */
TEST_ASSERT(ret == 0,
            "af_packet port with fanout_mode=%s has no port id: %d",
            modes[i], ret);
```

### Warnings

None identified.

### Info

None.

---

## Summary

Patch 1/2 is correct but could benefit from a more detailed comment.

Patch 2/2 has a resource leak on the error path where `rte_eth_dev_get_port_by_name()` fails -- the vdev is not uninitialized before `TEST_ASSERT` returns.

Review-Result: ERROR