diff mbox series

[v1,19/21] net/virtio-user: forward control messages to shadow queue

Message ID 20221130155639.150553-20-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series Add control queue & MQ support to Virtio-user vDPA |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin Nov. 30, 2022, 3:56 p.m. UTC 
  This patch implements control messages forwarding from the
regular control queue to the shadow control queue.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++++++++++---
 .../net/virtio/virtio_user/virtio_user_dev.h  |  3 --
 drivers/net/virtio/virtio_user_ethdev.c       |  6 +--
 3 files changed, 33 insertions(+), 13 deletions(-)

Comments

Stephen Hemminger Nov. 30, 2022, 4:54 p.m. UTC | #1 
On Wed, 30 Nov 2022 16:56:37 +0100
Maxime Coquelin <maxime.coquelin@redhat.com> wrote:

> +	if (status != 0 || !dev->scvq)
> +		goto out;
> +
> +	status = virtio_send_command(&dev->scvq->cq, (struct virtio_pmd_ctrl *)hdr, dlen, nb_dlen);
> +out:

Maybe I am looking at the diff only, and not seeing something but.

This looks like just an if statement, why the goto here?
Maxime Coquelin Dec. 6, 2022, 12:58 p.m. UTC | #2 
On 11/30/22 17:54, Stephen Hemminger wrote:
> On Wed, 30 Nov 2022 16:56:37 +0100
> Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> 
>> +	if (status != 0 || !dev->scvq)
>> +		goto out;
>> +
>> +	status = virtio_send_command(&dev->scvq->cq, (struct virtio_pmd_ctrl *)hdr, dlen, nb_dlen);
>> +out:
> 
> Maybe I am looking at the diff only, and not seeing something but.
> 
> This looks like just an if statement, why the goto here?
> 

The code was a bit more complex initially, but now it has been
simplified, I agree the goto does not make sense anymore.

I will rework it in v2.

Thanks,
Maxime
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index b0d603ee12..7c48c9bb29 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -750,7 +750,7 @@  virtio_user_dev_uninit(struct virtio_user_dev *dev)
 	dev->ops->destroy(dev);
 }
 
-uint8_t
+static uint8_t
 virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs)
 {
 	uint16_t i;
@@ -775,14 +775,17 @@  virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs)
 	return ret;
 }
 
+#define CVQ_MAX_DATA_DESCS 32
+
 static uint32_t
-virtio_user_handle_ctrl_msg(struct virtio_user_dev *dev, struct vring *vring,
+virtio_user_handle_ctrl_msg_split(struct virtio_user_dev *dev, struct vring *vring,
 			    uint16_t idx_hdr)
 {
 	struct virtio_net_ctrl_hdr *hdr;
 	virtio_net_ctrl_ack status = ~0;
 	uint16_t i, idx_data, idx_status;
 	uint32_t n_descs = 0;
+	int dlen[CVQ_MAX_DATA_DESCS], nb_dlen = 0;
 
 	/* locate desc for header, data, and status */
 	idx_data = vring->desc[idx_hdr].next;
@@ -790,6 +793,7 @@  virtio_user_handle_ctrl_msg(struct virtio_user_dev *dev, struct vring *vring,
 
 	i = idx_data;
 	while (vring->desc[i].flags == VRING_DESC_F_NEXT) {
+		dlen[nb_dlen++] = vring->desc[i].len;
 		i = vring->desc[i].next;
 		n_descs++;
 	}
@@ -811,6 +815,11 @@  virtio_user_handle_ctrl_msg(struct virtio_user_dev *dev, struct vring *vring,
 		status = 0;
 	}
 
+	if (status != 0 || !dev->scvq)
+		goto out;
+
+	status = virtio_send_command(&dev->scvq->cq, (struct virtio_pmd_ctrl *)hdr, dlen, nb_dlen);
+out:
 	/* Update status */
 	*(virtio_net_ctrl_ack *)(uintptr_t)vring->desc[idx_status].addr = status;
 
@@ -836,6 +845,7 @@  virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
 	uint16_t idx_data, idx_status;
 	/* initialize to one, header is first */
 	uint32_t n_descs = 1;
+	int dlen[CVQ_MAX_DATA_DESCS], nb_dlen = 0;
 
 	/* locate desc for header, data, and status */
 	idx_data = idx_hdr + 1;
@@ -846,6 +856,7 @@  virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
 
 	idx_status = idx_data;
 	while (vring->desc[idx_status].flags & VRING_DESC_F_NEXT) {
+		dlen[nb_dlen++] = vring->desc[idx_status].len;
 		idx_status++;
 		if (idx_status >= dev->queue_size)
 			idx_status -= dev->queue_size;
@@ -866,6 +877,11 @@  virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
 		status = 0;
 	}
 
+	if (status != 0 || !dev->scvq)
+		goto out;
+
+	status = virtio_send_command(&dev->scvq->cq, (struct virtio_pmd_ctrl *)hdr, dlen, nb_dlen);
+out:
 	/* Update status */
 	*(virtio_net_ctrl_ack *)(uintptr_t)
 		vring->desc[idx_status].addr = status;
@@ -877,7 +893,7 @@  virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
 	return n_descs;
 }
 
-void
+static void
 virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx)
 {
 	struct virtio_user_queue *vq = &dev->packed_queues[queue_idx];
@@ -909,8 +925,8 @@  virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx)
 	}
 }
 
-void
-virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
+static void
+virtio_user_handle_cq_split(struct virtio_user_dev *dev, uint16_t queue_idx)
 {
 	uint16_t avail_idx, desc_idx;
 	struct vring_used_elem *uep;
@@ -924,7 +940,7 @@  virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
 			    & (vring->num - 1);
 		desc_idx = vring->avail->ring[avail_idx];
 
-		n_descs = virtio_user_handle_ctrl_msg(dev, vring, desc_idx);
+		n_descs = virtio_user_handle_ctrl_msg_split(dev, vring, desc_idx);
 
 		/* Update used ring */
 		uep = &vring->used->ring[avail_idx];
@@ -935,6 +951,15 @@  virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
 	}
 }
 
+void
+virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
+{
+	if (virtio_with_packed_queue(&dev->hw))
+		virtio_user_handle_cq_packed(dev, queue_idx);
+	else
+		virtio_user_handle_cq_split(dev, queue_idx);
+}
+
 static void
 virtio_user_control_queue_notify(struct virtqueue *vq, void *cookie)
 {
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index e0db4faf3f..e8753f6019 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -74,9 +74,6 @@  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queue
 			 enum virtio_user_backend_type backend_type);
 void virtio_user_dev_uninit(struct virtio_user_dev *dev);
 void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
-void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
-				  uint16_t queue_idx);
-uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
 int virtio_user_dev_create_shadow_cvq(struct virtio_user_dev *dev, struct virtqueue *vq);
 void virtio_user_dev_destroy_shadow_cvq(struct virtio_user_dev *dev);
 int virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status);
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 626bd95b62..d23959e836 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -266,10 +266,8 @@  virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	if (hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq)) {
-		if (virtio_with_packed_queue(vq->hw))
-			virtio_user_handle_cq_packed(dev, vq->vq_queue_index);
-		else
-			virtio_user_handle_cq(dev, vq->vq_queue_index);
+		virtio_user_handle_cq(dev, vq->vq_queue_index);
+
 		return;
 	}