Re: [virtio-comment] Re: [PATCH RESEND] virtio-net: support setting coalescing params for multiple vqs

[Heng Qi <hengqi@linux.alibaba.com>]

在 2023/12/21 上午11:37, Jason Wang 写道:
> On Wed, Dec 20, 2023 at 10:40 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>> Currently, when each time the driver attempts to update the coalescing parameters
>> for a vq, it needs to kick the device and wait for the ctrlq response to return.
>>
>> If a command can only update one vq parameters, when the parameters are updated
>> frequently (such as netdim), ctrlq on the device is kicked frequently, which will
>> increase the device CPU scheduling overhead, and the number and overhead of device
>> DMA will also increase.
>>
>> Merging multiple vq updated parameters into one command can effectively reduce
>> the number of kick devices and device DMA times.
>>
>> Test results show that this greatly improves the efficiency of the ctrlq in
>> responding to multiple vq coalescing parameter updates issued by the driver.
> So netdim is per virtqueue, to make use of this, you need to batch the
> netdim requests first.

Yes. I think the code looks more intuitive.
Please view the following PoC test code combined with the current code 
of upstream virtio netdim

+struct ctrl_coal_vq_entry {
+       u16 vqn;
+       u16 reserved;
+       int usecs;
+       int packets;
+};
+
+struct virtio_net_ctrl_mrg_coal_vq {
+       int num_entries;
+       struct ctrl_coal_vq_entry coal_vqs[];
+};
+

@@ -313,12 +335,18 @@ struct virtnet_info {

         struct control_buf *ctrl;

+       struct virtio_net_ctrl_mrg_coal_vq *ctrl_coal_vqs;
+


@@ -4390,6 +4586,7 @@ static int virtnet_alloc_queues(struct 
virtnet_info *vi)
         if (!vi->rq)
                 goto err_rq;

+       vi->ctrl_coal_vqs = kzalloc(sizeof(struct 
virtio_net_ctrl_mrg_coal_vq) + vi->max_queue_pairs * sizeof(struct 
ctrl_coal_vq_entry), GFP_KERNEL);


+       struct scatterlist sg;
+       for (i = 0; i < vi->curr_queue_pairs; i++) {
+                vi->ctrl_coal_vqs->coal_vqs[i].vqn = rxq2vq(i);
+                vi->ctrl_coal_vqs->coal_vqs[i].usecs = 8;
+                vi->ctrl_coal_vqs->coal_vqs[i].packets = 16;
+        }
+        vi->ctrl_coal_vqs->num_entries = i;
+        sg_init_one(&sg, vi->ctrl_coal_vqs, sizeof(struct 
virtio_net_ctrl_mrg_coal_vq) + vi->ctrl_coal_vqs->num_entries * 
sizeof(struct ctrl_coal_vq_entry));
+        vi->req_num++;
+        if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
+                              VIRTIO_NET_CTRL_NOTF_COAL_*VQS*_SET, &sg))

  #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET               3
+#define VIRTIO_NET_CTRL_NOTF_COAL_VQS_SET              4

>   And if you do that, you can batch the commands
> as well.


Batch commands we have tried:

+               for (i = 0; i < vi->curr_queue_pairs; i++) {
+                       err = virtnet_pre_send_cmd(vi, &vi->vq_ctrls[i], 
8, 16);
+               }
+
+               if (unlikely(!virtqueue_kick(vi->cvq)))
+                       return vi->ctrl->status == VIRTIO_NET_OK;
+
+               for (i = 0; i < vi->curr_queue_pairs; i++) {
+                       while (!virtqueue_is_broken(vi->cvq) && 
!virtqueue_get_buf(vi->cvq, &tmp))
+                               cpu_relax();
+               }
+       }


+
+static bool virtnet_pre_send_cmd(struct virtnet_info *vi, struct 
vq_coal *vq_ctrls,
+                                u32 max_usecs, u32 max_packets)
+{
+       struct scatterlist hdr, out, stat;
+       struct scatterlist *sgs[4];
+       unsigned out_num = 0;
+       int ret;
+
+       BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
+
+       vq_ctrls->hdr.class = VIRTIO_NET_CTRL_NOTF_COAL;
+       vq_ctrls->hdr.cmd = VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET;
+       sg_init_one(&hdr, &vq_ctrls->hdr, sizeof(vq_ctrls->hdr));
+       sgs[out_num++] = &hdr;
+
+       vq_ctrls->coal_vq.vqn = cpu_to_le16(0);
+       vq_ctrls->coal_vq.coal.max_usecs = cpu_to_le32(max_usecs);
+       vq_ctrls->coal_vq.coal.max_packets = cpu_to_le32(max_packets);
+       sg_init_one(&out, &vq_ctrls->ho, sizeof(vq_ctrls->ho));
+
+       if (&out)
+               sgs[out_num++] = &out;
+
+       vq_ctrls->status = ~0;
+       sg_init_one(&stat, &vq_ctrls->status, sizeof(vq_ctrls->status));
+       sgs[out_num] = &stat;
+
+       BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
+       ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
+       if (ret < 0) {
+               dev_warn(&vi->vdev->dev,
+                        "Failed to add sgs for command vq: %d\n.", ret);
+               return false;
+       }


Overall, batch reqs are sufficient. Because the current major overhead 
is the number of DMAs.
For example, for a device with 256 queues,

For the current upstream code, the overhead is 256 kicks + 256*8 DMA times.
The overhead of batch cmds is 1 kick + 256*8 DMA times.
The overhead of batch reqs is 1 kick + 8 DMA times.

Below is 8 DMA times:
- get avail idx 1 time
- Pull avail ring information 1 time
- Pull the desc pointed to by the avail ring 3 times
- Pull the hdr and out bufs pointed to by avail ring desc 2 times
- Write once to the status buf pointed to by status 1 time

Thanks!

> Then you get one kick for sevreal coal requests?
>
> Or are you saying you run out of ctrl vq?
>
> Thanks
>
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/