On Tue, Jul 30, 2024 at 09:47:32PM GMT, Luigi Leonardi via B4 Relay wrote:
From: Luigi Leonardi <luigi.leona...@outlook.com>

When the driver needs to send new packets to the device, it always
queues the new sk_buffs into an intermediate queue (send_pkt_queue)
and schedules a worker (send_pkt_work) to then queue them into the
virtqueue exposed to the device.

This increases the chance of batching, but also introduces a lot of
latency into the communication. So we can optimize this path by
adding a fast path to be taken when there is no element in the
intermediate queue, there is space available in the virtqueue,
and no other process that is sending packets (tx_lock held).

The following benchmarks were run to check improvements in latency and
throughput. The test bed is a host with Intel i7-10700KF CPU @ 3.80GHz
and L1 guest running on QEMU/KVM with vhost process and all vCPUs
pinned individually to pCPUs.

- Latency
  Tool: Fio version 3.37-56
  Mode: pingpong (h-g-h)
  Test runs: 50
  Runtime-per-test: 50s
  Type: SOCK_STREAM

In the following fio benchmark (pingpong mode) the host sends
a payload to the guest and waits for the same payload back.

fio process pinned both inside the host and the guest system.

Before: Linux 6.9.8

Payload 64B:

        1st perc.       overall         99th perc.
Before  12.91           16.78           42.24           us
After   9.77            13.57           39.17           us

Payload 512B:

        1st perc.       overall         99th perc.
Before  13.35           17.35           41.52           us
After   10.25           14.11           39.58           us

Payload 4K:

        1st perc.       overall         99th perc.
Before  14.71           19.87           41.52           us
After   10.51           14.96           40.81           us

- Throughput
  Tool: iperf-vsock

The size represents the buffer length (-l) to read/write
P represents the number of parallel streams

P=1
        4K      64K     128K
Before  6.87    29.3    29.5 Gb/s
After   10.5    39.4    39.9 Gb/s

P=2
        4K      64K     128K
Before  10.5    32.8    33.2 Gb/s
After   17.8    47.7    48.5 Gb/s

P=4
        4K      64K     128K
Before  12.7    33.6    34.2 Gb/s
After   16.9    48.1    50.5 Gb/s

Great improvement! Thanks again for this work!


The performance improvement is related to this optimization,
I used a ebpf kretprobe on virtio_transport_send_skb to check
that each packet was sent directly to the virtqueue

Co-developed-by: Marco Pinna <marco.pin...@gmail.com>
Signed-off-by: Marco Pinna <marco.pin...@gmail.com>
Signed-off-by: Luigi Leonardi <luigi.leona...@outlook.com>
---
net/vmw_vsock/virtio_transport.c | 39 +++++++++++++++++++++++++++++++++++----
1 file changed, 35 insertions(+), 4 deletions(-)

All my comments have been resolved. I let iperf run bidirectionally for a long time and saw no problems, so:

Reviewed-by: Stefano Garzarella <sgarz...@redhat.com>



diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index f641e906f351..f992f9a216f0 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -208,6 +208,28 @@ virtio_transport_send_pkt_work(struct work_struct *work)
                queue_work(virtio_vsock_workqueue, &vsock->rx_work);
}

+/* Caller need to hold RCU for vsock.
+ * Returns 0 if the packet is successfully put on the vq.
+ */
+static int virtio_transport_send_skb_fast_path(struct virtio_vsock *vsock, 
struct sk_buff *skb)
+{
+       struct virtqueue *vq = vsock->vqs[VSOCK_VQ_TX];
+       int ret;
+
+       /* Inside RCU, can't sleep! */
+       ret = mutex_trylock(&vsock->tx_lock);
+       if (unlikely(ret == 0))
+               return -EBUSY;
+
+       ret = virtio_transport_send_skb(skb, vq, vsock);
+       if (ret == 0)
+               virtqueue_kick(vq);
+
+       mutex_unlock(&vsock->tx_lock);
+
+       return ret;
+}
+
static int
virtio_transport_send_pkt(struct sk_buff *skb)
{
@@ -231,11 +253,20 @@ virtio_transport_send_pkt(struct sk_buff *skb)
                goto out_rcu;
        }

-       if (virtio_vsock_skb_reply(skb))
-               atomic_inc(&vsock->queued_replies);
+       /* If send_pkt_queue is empty, we can safely bypass this queue
+        * because packet order is maintained and (try) to put the packet
+        * on the virtqueue using virtio_transport_send_skb_fast_path.
+        * If this fails we simply put the packet on the intermediate
+        * queue and schedule the worker.
+        */
+       if (!skb_queue_empty_lockless(&vsock->send_pkt_queue) ||
+           virtio_transport_send_skb_fast_path(vsock, skb)) {
+               if (virtio_vsock_skb_reply(skb))
+                       atomic_inc(&vsock->queued_replies);

-       virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb);
-       queue_work(virtio_vsock_workqueue, &vsock->send_pkt_work);
+               virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb);
+               queue_work(virtio_vsock_workqueue, &vsock->send_pkt_work);
+       }

out_rcu:
        rcu_read_unlock();

--
2.45.2




Reply via email to