|WARNING| net/af_packet: move size check to after vlan insert

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Correctness Analysis

**Error: Resource leak on VLAN insert failure**

Line 321: `if (rte_vlan_insert(&mbuf)) {`

The code calls `rte_vlan_insert()` which can fail and internally frees the mbuf on failure (per `rte_vlan.h` documentation). However, the subsequent `rte_pktmbuf_free(mbuf)` at line 323 attempts to free an already-freed mbuf, causing a **double-free**.

```c
/* insert vlan info if necessary */
if (mbuf->ol_flags & RTE_MBUF_F_TX_VLAN) {
    if (rte_vlan_insert(&mbuf)) {
        rte_pktmbuf_free(mbuf);  /* DOUBLE-FREE: mbuf already freed by rte_vlan_insert */
        continue;
    }
}
```

Suggested fix: Remove the explicit free, as `rte_vlan_insert()` handles cleanup on failure:

```c
/* insert vlan info if necessary */
if (mbuf->ol_flags & RTE_MBUF_F_TX_VLAN) {
    if (rte_vlan_insert(&mbuf)) {
        /* rte_vlan_insert frees mbuf on failure */
        continue;
    }
}
```

---

## Style and Process

**Info: Pre-existing comparison style**

Line 329: `if (mbuf->pkt_len > pkt_q->frame_data_size) {`

This is an explicit comparison and follows DPDK style guidelines correctly. The patch moves this check but does not change the comparison style. No action required.

Line 334: `if (!tx_ring_status_available(ppd->tp_status)) {`

The negation of a function call result is acceptable when the function returns a boolean-like status. No issue here.

---

## Patch Logic Review

The patch correctly addresses the corner case where VLAN insertion increases packet size beyond the frame data size limit. Moving the size check to after VLAN insertion ensures the final packet size is validated. However, the double-free bug introduced at the error path must be fixed.

---

Review-Result: ERROR