[PATCH AUTOSEL 6.12 2/6] virtio_net: Enforce minimum TX ring size for reliability

[PATCH AUTOSEL 6.12 2/6] virtio_net: Enforce minimum TX ring size for reliability

[Sasha Levin]

From: Laurent Vivier <lvivier@redhat.com>

[ Upstream commit 24b2f5df86aaebbe7bac40304eaf5a146c02367c ]

The `tx_may_stop()` logic stops TX queues if free descriptors
(`sq->vq->num_free`) fall below the threshold of (`MAX_SKB_FRAGS` + 2).
If the total ring size (`ring_num`) is not strictly greater than this
value, queues can become persistently stopped or stop after minimal
use, severely degrading performance.

A single sk_buff transmission typically requires descriptors for:
- The virtio_net_hdr (1 descriptor)
- The sk_buff's linear data (head) (1 descriptor)
- Paged fragments (up to MAX_SKB_FRAGS descriptors)

This patch enforces that the TX ring size ('ring_num') must be strictly
greater than (MAX_SKB_FRAGS + 2). This ensures that the ring is
always large enough to hold at least one maximally-fragmented packet
plus at least one additional slot.

Reported-by: Lei Yang <leiyang@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Link: https://patch.msgid.link/20250521092236.661410-4-lvivier@redhat.com
Tested-by: Lei Yang <leiyang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

**YES**

This commit should be backported to stable kernel trees. Here's my
extensive analysis:

## 1. **Critical Bug Fix**
The commit fixes a severe bug where TX queues can become permanently
stopped, causing complete network transmission failure. Looking at the
code change, it adds a crucial validation in `virtnet_tx_resize()`:

```c
if (ring_num <= MAX_SKB_FRAGS + 2) {
    netdev_err(vi->dev, "tx size (%d) cannot be smaller than %d\n",
               ring_num, MAX_SKB_FRAGS + 2);
    return -EINVAL;
}
```

This prevents a configuration that would make the network interface
essentially unusable.

## 2. **Root Cause Analysis**
The bug occurs because the `tx_may_stop()` logic (used throughout
virtio_net for flow control) stops the TX queue when free descriptors
fall below `MAX_SKB_FRAGS + 2`. If the total ring size is not strictly
greater than this threshold, the queue can:
- Stop after transmitting just one packet
- Never have enough free slots to wake up again
- Result in a permanently stalled TX queue

## 3. **User Impact**
This affects real users who:
- Use virtio_net in virtualized environments (very common in cloud/VM
  deployments)
- Configure TX ring sizes via ethtool or other management tools
- May unknowingly set ring sizes that trigger this bug

The commit message explicitly mentions a report from Lei Yang at Red
Hat, indicating this was discovered through real-world usage.

## 4. **Minimal and Safe Fix**
The fix is:
- **Small**: Only 6 lines of code added
- **Contained**: Only affects ring resize operations
- **Safe**: Adds validation without changing existing behavior for valid
  configurations
- **Clear**: Returns -EINVAL with a descriptive error message

## 5. **Meets Stable Criteria**
According to stable kernel rules, this commit qualifies because it:
- Fixes a bug that affects users (network stalls)
- Is relatively small and contained
- Has no major side effects
- Doesn't introduce new features
- Has been tested (by Lei Yang) and reviewed by maintainers (Jason Wang,
  Michael S. Tsirkin)

## 6. **Comparison with Similar Commits**
Looking at the historical commits:
- Similar commit #3 (virtio_net: ensure netdev_tx_reset_queue) was
  backported because it fixed TX queue handling issues
- This commit is even more critical as it prevents complete TX failure
- The fix is simpler than commit #3 (just validation vs. callback
  handling)

## 7. **No Architectural Changes**
Unlike commits #1 and #2 which made architectural changes to ring
sizing, this commit:
- Only adds input validation
- Doesn't change data structures
- Doesn't modify existing workflows
- Simply prevents an invalid configuration

The fact that experienced maintainers like Jason Wang and Michael S.
Tsirkin acked this change further supports its importance for stable
backporting. This is a textbook example of a fix that should go to
stable: it addresses a real bug with minimal risk.

 drivers/net/virtio_net.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6d36cb204f9bc..384aabca39964 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3231,6 +3231,12 @@ static int virtnet_tx_resize(struct virtnet_info *vi, struct send_queue *sq,
 {
 	int qindex, err;
 
+	if (ring_num <= MAX_SKB_FRAGS + 2) {
+		netdev_err(vi->dev, "tx size (%d) cannot be smaller than %d\n",
+			   ring_num, MAX_SKB_FRAGS + 2);
+		return -EINVAL;
+	}
+
 	qindex = sq - vi->sq;
 
 	virtnet_tx_pause(vi, sq);
-- 
2.39.5

[PATCH AUTOSEL 6.12 3/6] virtio_ring: Fix error reporting in virtqueue_resize

[Sasha Levin]

From: Laurent Vivier <lvivier@redhat.com>

[ Upstream commit 45ebc7e6c125ce93d2ddf82cd5bea20121bb0258 ]

The virtqueue_resize() function was not correctly propagating error codes
from its internal resize helper functions, specifically
virtqueue_resize_packet() and virtqueue_resize_split(). If these helpers
returned an error, but the subsequent call to virtqueue_enable_after_reset()
succeeded, the original error from the resize operation would be masked.
Consequently, virtqueue_resize() could incorrectly report success to its
caller despite an underlying resize failure.

This change restores the original code behavior:

       if (vdev->config->enable_vq_after_reset(_vq))
               return -EBUSY;

       return err;

Fix: commit ad48d53b5b3f ("virtio_ring: separate the logic of reset/enable from virtqueue_resize")
Cc: xuanzhuo@linux.alibaba.com
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Link: https://patch.msgid.link/20250521092236.661410-2-lvivier@redhat.com
Tested-by: Lei Yang <leiyang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

## Analysis Summary

**YES**, this commit should be backported to stable kernel trees.

Here's my extensive analysis:

### 1. **This is a clear bug fix for incorrect error reporting**

The commit fixes a critical error handling bug in `virtqueue_resize()`
where error codes from the resize operation (`virtqueue_resize_packed()`
or `virtqueue_resize_split()`) were being masked if
`virtqueue_enable_after_reset()` succeeded.

The specific bug pattern:
```c
// Before fix (buggy code):
err = virtqueue_resize_packed/_split(_vq, num);  // May return error
return virtqueue_enable_after_reset(_vq);        // Returns 0 on
success, masking 'err'

// After fix (correct code):
err = virtqueue_resize_packed/_split(_vq, num);
err_reset = virtqueue_enable_after_reset(_vq);
if (err_reset)
    return err_reset;
return err;  // Correctly returns the resize error
```

### 2. **The bug affects users and can cause silent failures**

According to the function documentation at lines 2787-2788, when
`-ENOMEM` is returned from resize, "vq can still work normally" with the
original ring size. However, with the bug, the caller would receive
success (0) instead of `-ENOMEM`, leading them to incorrectly believe
the resize succeeded when it actually failed. This could cause:
- Incorrect assumptions about queue capacity
- Performance issues if the application expected a different queue size
- Potential resource allocation mismatches

### 3. **The fix is small, contained, and low-risk**

The change is minimal - only 6 lines of code:
- Introduces a new local variable `err_reset`
- Properly preserves and returns the original error code
- No architectural changes or new features
- Only affects error propagation logic

### 4. **The bug exists in stable kernels**

- Bug introduced in v6.6-rc1 (commit ad48d53b5b3f)
- The feature (virtqueue_resize) exists since v6.0-rc1
- Therefore, stable kernels 6.6.x and later contain this bug

### 5. **Clear regression from refactoring**

The commit message explicitly states this "restores the original code
behavior" and includes a "Fix:" tag pointing to the commit that
introduced the regression. The original correct pattern was:
```c
if (vdev->config->enable_vq_after_reset(_vq))
    return -EBUSY;
return err;
```

### 6. **Meets stable kernel criteria**

Per stable kernel rules, this fix:
- Fixes a real bug that affects users (incorrect error reporting)
- Is already in Linus' tree (merged by Paolo Abeni)
- Is small and easily reviewable
- Has been tested (Tested-by: Lei Yang)
- Has multiple maintainer acks (Jason Wang, Michael S. Tsirkin)
- Does not add new features or make risky changes

### 7. **Similar commits context**

While the similar commits shown are feature additions (introducing
virtqueue_resize functionality), this commit is fundamentally different
- it's a bug fix for error handling, not a feature addition.

The fix ensures that callers of `virtqueue_resize()` receive accurate
error information, which is critical for proper error handling and
recovery in virtio drivers that use queue resizing functionality.

 drivers/virtio/virtio_ring.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 147926c8bae09..c0276979675df 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2741,7 +2741,7 @@ int virtqueue_resize(struct virtqueue *_vq, u32 num,
 		     void (*recycle_done)(struct virtqueue *vq))
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	int err;
+	int err, err_reset;
 
 	if (num > vq->vq.num_max)
 		return -E2BIG;
@@ -2763,7 +2763,11 @@ int virtqueue_resize(struct virtqueue *_vq, u32 num,
 	else
 		err = virtqueue_resize_split(_vq, num);
 
-	return virtqueue_enable_after_reset(_vq);
+	err_reset = virtqueue_enable_after_reset(_vq);
+	if (err_reset)
+		return err_reset;
+
+	return err;
 }
 EXPORT_SYMBOL_GPL(virtqueue_resize);
 
-- 
2.39.5