On 1/12/2022 11:00 PM, Michael S. Tsirkin wrote:
On Thu, Jan 13, 2022 at 12:10:51AM -0500, Si-Wei Liu wrote:
Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
Signed-off-by: Si-Wei Liu<si-wei.liu@xxxxxxxxxx>
Add motivation for change in the commit log.
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 46d4deb..491127f 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1563,11 +1563,21 @@ static virtio_net_ctrl_ack handle_ctrl_mq(struct mlx5_vdpa_dev *mvdev, u8 cmd)
switch (cmd) {
case VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET:
+ /* This mq feature check aligns with pre-existing userspace implementation,
+ * although the spec doesn't mandate so.
And so ... why do we bother? what breaks if we don't?
Without it, a malicious driver could fake a config multiqueue request
down to a non-mq backend that may cause kernel to panic due to
uninitialized resources for the queue. Even with a well behaving guest
driver, it is not expected to allow such kind of change.
+ */
+ if (!MLX5_FEATURE(mvdev, VIRTIO_NET_F_MQ))
+ break;
+
this part is not described in the commit log at all.
is it intentional?
No, I can add it.
read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, (void *)&mq, sizeof(mq));
if (read != sizeof(mq))
break;
newqps = mlx5vdpa16_to_cpu(mvdev, mq.virtqueue_pairs);
+ if (newqps < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
+ newqps > mlx5_vdpa_max_qps(mvdev->max_vqs))
+ break;
+
if (ndev->cur_num_vqs == 2 * newqps) {
status = VIRTIO_NET_OK;
break;
--
1.8.3.1
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
