|WARNING| net/vhost: preserve Tx ownership on VLAN failure

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# Review of net/vhost: preserve Tx ownership on VLAN failure

## Errors

### 1. Incorrect missed packet count calculation

**Issue:** `nb_missed = nb_send;` should be `nb_missed = nb_send - nb_tx;`

The missed packet count should represent packets that were prepared (software processed, VLAN inserted, checksummed) but not accepted by the vhost enqueue. The current code sets `nb_missed` to the total number of prepared packets, ignoring how many were successfully transmitted.

**Impact:** Statistics will incorrectly report all successfully transmitted packets as missed.

**Fix:**
```c
nb_missed = nb_send - nb_tx;
```

This matches the original logic (missed = total input - transmitted) but applied to the new `nb_send` instead of `nb_bufs`.

---

## Warnings

### 1. Missing variable reinitialization in loop rewrite

**Context:** The loop body changes from `bufs[nb_send] = m; ++nb_send;` to `bufs[i] = m;` with `nb_send = i;` after the loop.

**Issue:** The rewrite is correct for the success path. However, the variable `i` is reused as both the loop counter and the indicator of where to stop. While this works, it relies on loop counter value preservation after break, which is guaranteed by C but could be more explicit.

**Suggestion:** Consider a comment clarifying that `i` holds the count of successfully prepared packets after the loop (either via normal termination or break):

```c
	for (i = 0; i < nb_bufs; i++) {
		/* ... */
		if (unlikely(error))
			break;  /* i now marks first failed/unprocessed packet */

		/* ... */
		bufs[i] = m;
	}
	/* i is the count of packets successfully prepared */
	nb_send = i;
```

This is a clarity suggestion, not a functional issue.

---

## Info

### 1. Statistics behavior change

The patch changes what counts as a "missed" packet. Previously, `nb_missed` represented packets that were never enqueued to the vhost device (input count - transmitted). After this patch, it represents packets that passed software processing but were not transmitted by vhost.

This is **correct** given the new ownership model (caller owns the un-preprocessed suffix after a VLAN failure), but it is a subtle semantic change. The commit message correctly describes the new behavior ("Count only packets prepared for vhost enqueue but not accepted as missed"), which is good.

---

## Summary

One **Error** must be fixed: the missed packet count calculation is wrong and will produce incorrect statistics.

The patch correctly solves the ownership bug by stopping preprocessing at the first VLAN failure and leaving the failed mbuf in the array. The loop rewrite from compaction (`bufs[nb_send++]`) to in-place assignment (`bufs[i]`) is correct and preserves the original burst order.