[Qemu-devel] [PATCH v1 0/3] Improvements to the netmap backend

[Qemu-devel] [PATCH v1 0/3] Improvements to the netmap backend

[Vincenzo Maffione]

Improvements to the netmap backend, mostly to fix the handling of
incomplete multi-slot packets.
This has been tested with and without jumbo frames, using pkt-gen
to send 60, 1500, 5000 or 9000 bytes packets between two VMs connected
through a VALE switch.

Vincenzo Maffione (3):
  net: netmap: small improvements netmap_send()
  net: netmap: simplify netmap_receive()
  net: netmap: improve netmap_receive_iov()

 net/netmap.c | 110 ++++++++++++++++++++++-----------------------------
 1 file changed, 47 insertions(+), 63 deletions(-)

-- 
2.19.2

[Qemu-devel] [PATCH v1 1/3] net: netmap: small improvements netmap_send()

[Vincenzo Maffione]

This change improves the handling of incomplete multi-slot packets
(e.g. with the NS_MOREFRAG set), by advancing ring->head only on
complete packets. The ring->cur pointer is advanced in any case in
order to acknowledge the kernel and move the wake-up point (thus
avoiding repeated wake-ups).
Also don't be verbose when incomplete packets are found.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 net/netmap.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/net/netmap.c b/net/netmap.c
index 2d11a8f4be..71a8122bdd 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -272,39 +272,46 @@ static void netmap_send(void *opaque)
 {
     NetmapState *s = opaque;
     struct netmap_ring *ring = s->rx;
+    unsigned int tail = ring->tail;
 
-    /* Keep sending while there are available packets into the netmap
+    /* Keep sending while there are available slots in the netmap
        RX ring and the forwarding path towards the peer is open. */
-    while (!nm_ring_empty(ring)) {
-        uint32_t i;
+    while (ring->head != tail) {
+        uint32_t i = ring->head;
         uint32_t idx;
         bool morefrag;
         int iovcnt = 0;
         int iovsize;
 
+        /* Get a (possibly multi-slot) packet. */
         do {
-            i = ring->cur;
             idx = ring->slot[i].buf_idx;
             morefrag = (ring->slot[i].flags & NS_MOREFRAG);
-            s->iov[iovcnt].iov_base = (u_char *)NETMAP_BUF(ring, idx);
+            s->iov[iovcnt].iov_base = (void *)NETMAP_BUF(ring, idx);
             s->iov[iovcnt].iov_len = ring->slot[i].len;
             iovcnt++;
+            i = nm_ring_next(ring, i);
+        } while (i != tail && morefrag);
 
-            ring->cur = ring->head = nm_ring_next(ring, i);
-        } while (!nm_ring_empty(ring) && morefrag);
+        /* Advance ring->cur to tell the kernel that we have seen the slots. */
+        ring->cur = i;
 
-        if (unlikely(nm_ring_empty(ring) && morefrag)) {
-            RD(5, "[netmap_send] ran out of slots, with a pending"
-                   "incomplete packet\n");
+        if (unlikely(morefrag)) {
+            /* This is a truncated packet, so we can stop without releasing the
+             * incomplete slots by updating ring->head. We will hopefully
+             * re-read the complete packet the next time we are called. */
+            break;
         }
 
         iovsize = qemu_sendv_packet_async(&s->nc, s->iov, iovcnt,
                                             netmap_send_completed);
 
+        /* Release the slots to the kernel. */
+        ring->head = i;
+
         if (iovsize == 0) {
             /* The peer does not receive anymore. Packet is queued, stop
-             * reading from the backend until netmap_send_completed()
-             */
+             * reading from the backend until netmap_send_completed(). */
             netmap_read_poll(s, false);
             break;
         }
-- 
2.19.2

[Qemu-devel] [PATCH v1 2/3] net: netmap: simplify netmap_receive()

[Vincenzo Maffione]

Improve code reuse by implementing netmap_receive() with a call
to netmap_receive_iov().

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 net/netmap.c | 50 +++++++++++---------------------------------------
 1 file changed, 11 insertions(+), 39 deletions(-)

diff --git a/net/netmap.c b/net/netmap.c
index 71a8122bdd..852106af29 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -154,45 +154,6 @@ static void netmap_writable(void *opaque)
     qemu_flush_queued_packets(&s->nc);
 }
 
-static ssize_t netmap_receive(NetClientState *nc,
-      const uint8_t *buf, size_t size)
-{
-    NetmapState *s = DO_UPCAST(NetmapState, nc, nc);
-    struct netmap_ring *ring = s->tx;
-    uint32_t i;
-    uint32_t idx;
-    uint8_t *dst;
-
-    if (unlikely(!ring)) {
-        /* Drop. */
-        return size;
-    }
-
-    if (unlikely(size > ring->nr_buf_size)) {
-        RD(5, "[netmap_receive] drop packet of size %d > %d\n",
-                                    (int)size, ring->nr_buf_size);
-        return size;
-    }
-
-    if (nm_ring_empty(ring)) {
-        /* No available slots in the netmap TX ring. */
-        netmap_write_poll(s, true);
-        return 0;
-    }
-
-    i = ring->cur;
-    idx = ring->slot[i].buf_idx;
-    dst = (uint8_t *)NETMAP_BUF(ring, idx);
-
-    ring->slot[i].len = size;
-    ring->slot[i].flags = 0;
-    pkt_copy(buf, dst, size);
-    ring->cur = ring->head = nm_ring_next(ring, i);
-    ioctl(s->nmd->fd, NIOCTXSYNC, NULL);
-
-    return size;
-}
-
 static ssize_t netmap_receive_iov(NetClientState *nc,
                     const struct iovec *iov, int iovcnt)
 {
@@ -259,6 +220,17 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
     return iov_size(iov, iovcnt);
 }
 
+static ssize_t netmap_receive(NetClientState *nc,
+      const uint8_t *buf, size_t size)
+{
+    struct iovec iov;
+
+    iov.iov_base = (void *)buf;
+    iov.iov_len = size;
+
+    return netmap_receive_iov(nc, &iov, 1);
+}
+
 /* Complete a previous send (backend --> guest) and enable the
    fd_read callback. */
 static void netmap_send_completed(NetClientState *nc, ssize_t len)
-- 
2.19.2

[Qemu-devel] [PATCH v1 3/3] net: netmap: improve netmap_receive_iov()

[Vincenzo Maffione]

Changes:
  - Save CPU cycles by computing the return value while scanning the
    input iovec, rather than calling iov_size() at the end.
  - Remove check for s->tx != NULL, because it cannot happen.
  - Cache ring->tail in a local variable and use it to check for
    space in the TX ring. The use of nm_ring_empty() was invalid,
    because nobody is updating ring->cur and ring->head at that point.
  - In case we run out of netmap slots in the middle of a packet,
    move the wake-up point by advancing ring->cur, but do not
    expose the incomplete packet (i.e., by updating also ring->head).

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 net/netmap.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/net/netmap.c b/net/netmap.c
index 852106af29..0cc8f545c5 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -159,21 +159,22 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
 {
     NetmapState *s = DO_UPCAST(NetmapState, nc, nc);
     struct netmap_ring *ring = s->tx;
+    unsigned int tail = ring->tail;
+    ssize_t totlen = 0;
     uint32_t last;
     uint32_t idx;
     uint8_t *dst;
     int j;
     uint32_t i;
 
-    if (unlikely(!ring)) {
-        /* Drop the packet. */
-        return iov_size(iov, iovcnt);
-    }
-
-    last = i = ring->cur;
+    last = i = ring->head;
 
     if (nm_ring_space(ring) < iovcnt) {
-        /* Not enough netmap slots. */
+        /* Not enough netmap slots. Tell the kernel that we have seen the new
+         * available slots (so that it notifies us again when it has more
+         * ones), but without publishing any new slots to be processed
+         * (e.g., we don't advance ring->head). */
+        ring->cur = tail;
         netmap_write_poll(s, true);
         return 0;
     }
@@ -183,14 +184,17 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
         int offset = 0;
         int nm_frag_size;
 
+        totlen += iov_frag_size;
+
         /* Split each iovec fragment over more netmap slots, if
            necessary. */
         while (iov_frag_size) {
             nm_frag_size = MIN(iov_frag_size, ring->nr_buf_size);
 
-            if (unlikely(nm_ring_empty(ring))) {
-                /* We run out of netmap slots while splitting the
+            if (unlikely(i == tail)) {
+                /* We ran out of netmap slots while splitting the
                    iovec fragments. */
+                ring->cur = tail;
                 netmap_write_poll(s, true);
                 return 0;
             }
@@ -212,12 +216,13 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
     /* The last slot must not have NS_MOREFRAG set. */
     ring->slot[last].flags &= ~NS_MOREFRAG;
 
-    /* Now update ring->cur and ring->head. */
-    ring->cur = ring->head = i;
+    /* Now update ring->head and ring->cur to publish the new slots and
+     * the new wakeup point. */
+    ring->head = ring->cur = i;
 
     ioctl(s->nmd->fd, NIOCTXSYNC, NULL);
 
-    return iov_size(iov, iovcnt);
+    return totlen;
 }
 
 static ssize_t netmap_receive(NetClientState *nc,
-- 
2.19.2

Re: [Qemu-devel] [PATCH v1 0/3] Improvements to the netmap backend

[Jason Wang]

On 2018/12/7 上午12:59, Vincenzo Maffione wrote:
> Improvements to the netmap backend, mostly to fix the handling of
> incomplete multi-slot packets.
> This has been tested with and without jumbo frames, using pkt-gen
> to send 60, 1500, 5000 or 9000 bytes packets between two VMs connected
> through a VALE switch.
>
> Vincenzo Maffione (3):
>    net: netmap: small improvements netmap_send()
>    net: netmap: simplify netmap_receive()
>    net: netmap: improve netmap_receive_iov()
>
>   net/netmap.c | 110 ++++++++++++++++++++++-----------------------------
>   1 file changed, 47 insertions(+), 63 deletions(-)
>

Applied.

Thanks