Mark McLoughlin wrote:
From: Herbert Xu <[EMAIL PROTECTED]>

Currently, in order to receive large packets, the guest must allocate
max-sized packet buffers and pass them to the host. Each of these
max-sized packets occupy 20 ring entries, which means we can only
transfer a maximum of 12 packets in a single batch with a 256 entry
ring.

When receiving packets from external networks, we only receive MTU
sized packets and so the throughput observed is throttled by the
number of packets the ring can hold.

Implement the VIRTIO_NET_F_MRG_RXBUF feature to let guests know that
we can merge smaller buffers together in order to handle large packets.

This scheme allows us to be efficient in our use of ring entries
while still supporting large packets. Benchmarking using netperf from
an external machine to a guest over a 10Gb/s network shows a 100%
improvement from ~1Gb/s to ~2Gb/s. With a local host->guest benchmark
with GSO disabled on the host side, throughput was seen to increase
from 700Mb/s to 1.7Gb/s.

Based on a patch from Herbert, with the feature renamed from
"datahead" and some re-factoring for readability.


diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 403247b..afa5fe5 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -34,9 +34,13 @@
 #define VIRTIO_NET_F_HOST_TSO6 12      /* Host can handle TSOv6 in. */
 #define VIRTIO_NET_F_HOST_ECN  13      /* Host can handle TSO[6] w/ ECN in. */
 #define VIRTIO_NET_F_HOST_UFO  14      /* Host can handle UFO in. */
+#define VIRTIO_NET_F_MRG_RXBUF 15      /* Host can merge receive buffers. */

What's the status of the guest side of this feature?

 #define TX_TIMER_INTERVAL 150000 /* 150 us */
+/* Should be the largest MAX_SKB_FRAGS supported. */
+#define VIRTIO_NET_MAX_FRAGS   18
+

This should be advertised by the host to the guest (or vice-versa?). We're embedding Linux-specific magic numbers in a guest-OS-agnostic ABI.

Perfereably, there shouldn't be a limit at all.

@@ -209,7 +220,12 @@ static void virtio_net_receive(void *opaque, const uint8_t 
*buf, int size)
     if (virtqueue_pop(n->rx_vq, &elem) == 0)
        return;
- if (elem.in_num < 1 || elem.in_sg[0].iov_len != sizeof(*hdr)) {
+    if (n->mergeable_rx_bufs) {
+       if (elem.in_num < 1 || elem.in_sg[0].iov_len < TARGET_PAGE_SIZE) {
+           fprintf(stderr, "virtio-net IOV is irregular\n");
+           exit(1);
+       }

Again, this is burying details of the current Linux stack into the ABI. The Linux stack may change not to be page oriented, or maybe this won't fit will to how Windows views things. Can this be made not to depend on the size of the iov elements?

+    } else if (elem.in_num < 1 || elem.in_sg[0].iov_len != sizeof(*hdr)) {
        fprintf(stderr, "virtio-net header not in first element\n");
        exit(1);
     }
@@ -229,11 +245,49 @@ static void virtio_net_receive(void *opaque, const 
uint8_t *buf, int size)
     }
/* copy in packet. ugh */
-    iov_fill(&elem.in_sg[1], elem.in_num - 1,
-            buf + offset, size - offset);
- /* signal other side */
-    virtqueue_push(n->rx_vq, &elem, total);
+    if (n->mergeable_rx_bufs) {
+       int i = 0;
+
+       elem.in_sg[0].iov_base += sizeof(*hdr);
+       elem.in_sg[0].iov_len  -= sizeof(*hdr);
+
+       offset += iov_fill(&elem.in_sg[0], elem.in_num,
+                          buf + offset, size - offset);
+
+       /* signal other side */
+       virtqueue_fill(n->rx_vq, &elem, total, i++);
+
+       while (offset < size) {
+           int len;
+
+           if (virtqueue_pop(n->rx_vq, &elem) == 0) {
+               fprintf(stderr, "virtio-net truncating packet\n");
+               exit(1);
+           }
+
+           if (elem.in_num < 1 || elem.in_sg[0].iov_len < TARGET_PAGE_SIZE) {
+               fprintf(stderr, "virtio-net IOV is irregular\n");
+               exit(1);
+           }
+
+           len = iov_fill(&elem.in_sg[0], elem.in_num,
+                          buf + offset, size - offset);
+
+           virtqueue_fill(n->rx_vq, &elem, len, i++);
+
+           offset += len;
+       }
+
+       virtqueue_flush(n->rx_vq, i);
+    } else {
+       iov_fill(&elem.in_sg[1], elem.in_num - 1,
+                buf + offset, size - offset);
+
+       /* signal other side */
+       virtqueue_push(n->rx_vq, &elem, total);
+    }
+

Can we merge the two sides of the if () so that the only difference is the number of times we go through the loop?

Anthony, please review this as well, my virtio-foo is pretty superficial.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to