|WARNING| [v3, 6/6] net/iavf: fix leak of IPsec crypto capabilities array

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/6: net/iavf: fix local memory leaks in TM hierarchy commit

### Errors

None.

### Warnings

None.

### Info

The patch correctly identifies and fixes resource leaks in `iavf_hierarchy_commit()`. The changes are appropriate:
- Local temporary variables `q_tc_mapping` and `q_bw` are freed on both success and error paths
- The switch from `rte_zmalloc` to `calloc` is correct per the guidelines (these are not DMA buffers or shared with secondary processes)
- Error path cleanup is consistent

## Patch 2/6: net/iavf: fix leak of queue to traffic class mapping data

### Errors

None.

### Warnings

None.

### Info

Good fix for a resource leak. The patch:
- Correctly frees `vf->qtc_map` in `iavf_uninit_vf()` and `iavf_tm_conf_uninit()`
- Properly handles replacement of old mapping with new (frees old mapping after new one is assigned)
- Adds `qtc_map` to the error path cleanup
- The switch from `rte_zmalloc` to `calloc` is appropriate (not a DMA buffer)

## Patch 3/6: net/iavf: fix memory leak on error when adding flow parser

### Errors

None.

### Warnings

None.

### Info

Clean fix. The patch adds `rte_free(parser_node)` in the error branch where the node is not added to any TAILQ, preventing a memory leak.

## Patch 4/6: net/ice: fix buffer leak in config of Tx queue TM node

### Errors

**Error 1: Missing error check on `calloc()`**

```c
buf = calloc(1, buf_size);
if (buf == NULL)
    return -ENOMEM;
```

The existing NULL check is correct, but the function returns `-ENOMEM` which is not a standard errno value. This should be `ENOMEM` (positive) or the code should use a driver-specific error code. However, this is pre-existing and not introduced by this patch, so it's acceptable to not fix it here.

### Warnings

None.

### Info

The patch correctly:
- Moves `free(buf)` to execute unconditionally after `ice_aq_move_recfg_lan_txq()`
- Replaces `ice_malloc()` with `calloc()` and `ice_free()` with `free()`

## Patch 5/6: net/iavf: fix leak of flex metadata extraction field

### Errors

None.

### Warnings

**Warning 1: Redundant free in `iavf_dev_close()`**

The patch adds:
```c
rte_free(vf->proto_xtr);
vf->proto_xtr = NULL;
```
in both `iavf_uninit_vf()` and `iavf_dev_close()`.

Looking at the code flow, `iavf_dev_close()` appears to handle normal port shutdown, while `iavf_uninit_vf()` is called during error cleanup and device removal. The double-free protection (`vf->proto_xtr = NULL`) prevents actual corruption, but this suggests either:
1. The free in `iavf_dev_close()` is redundant if `iavf_uninit_vf()` is always called after close, OR
2. The free in `iavf_uninit_vf()` is redundant if close is always called first

Without seeing the full call graph, this pattern is acceptable (defensive programming with NULL protection), but consider verifying that both call sites are necessary.

### Info

The patch fixes the memory leak of `vf->proto_xtr` allocated in `iavf_init_proto_xtr()`.

## Patch 6/6: net/iavf: fix leak of IPsec crypto capabilities array

### Errors

None.

### Warnings

None.

### Info

The patch correctly frees `iavf_sctx->crypto_capabilities` before freeing the containing structure `iavf_sctx` in `iavf_security_ctx_destroy()`. The NULL assignment after free is good defensive practice.

---

## Summary

**Correctness:** All patches correctly fix resource leaks. No use-after-free issues introduced. Error paths are properly updated.

**Style:** All patches follow DPDK coding style. The conversions from `rte_zmalloc`/`rte_free` to `calloc`/`free` are appropriate per the guidelines (these are control-path allocations, not DMA buffers or hugepage-backed memory).

**API/Documentation:** No API changes. Release notes are not required for these internal fixes per the guidelines (they are bug fixes, not API changes or new features).

**Overall Assessment:** This is a solid patch series. All identified leaks are real and the fixes are correct. The only minor note is the potential redundancy in Patch 5, which is acceptable defensive programming but could be optimized if the call graph confirms one of the free sites is always sufficient.