[Qemu-devel] [PATCH v2] net: Disable netmap backend when not supported

[Qemu-devel] [PATCH v2] net: Disable netmap backend when not supported

[Vincenzo Maffione]

This patch fixes configure so that the netmap backend is not compiled in if the
host doesn't support an API version >= 11. A version upper bound (15) has been
added so that the netmap API can be extended with some minor features without
requiring QEMU code modifications.

Moreover, some changes have been done to net/netmap.c in order to reflect the
current netmap API/ABI (11).

The NETMAP_WITH_LIBS macro makes possible to include some utilities (e.g.
netmap ring macros, D(), RD() and other high level functions) through the netmap
headers. In this way we get rid of the D and RD macro definitions in the QEMU
code, and we open the way for further code simplifications that will be
introduced by future patches.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
Note: This patch is against the net-next/net Stefan's branch.

Changes against the previous version:
   (1) more complete commit description
   (2) add comment in ./configure to explain version checks

 configure    | 10 +++++++++-
 net/netmap.c | 55 +++++++++++++------------------------------------------
 2 files changed, 22 insertions(+), 43 deletions(-)

diff --git a/configure b/configure
index 4648117..e399b3e 100755
--- a/configure
+++ b/configure
@@ -2118,13 +2118,21 @@ EOF
 fi
 
 ##########################################
-# netmap headers probe
+# netmap support probe
+# Apart from looking for netmap headers, we make sure that the host API version
+# supports the netmap backend (>=11). The upper bound (15) is meant to simulate
+# a minor/major version number. Minor new features will be marked with values up
+# to 15, and if something happens that requires a change to the backend we will
+# move above 15, submit the backend fixes and modify this two bounds.
 if test "$netmap" != "no" ; then
   cat > $TMPC << EOF
 #include <inttypes.h>
 #include <net/if.h>
 #include <net/netmap.h>
 #include <net/netmap_user.h>
+#if (NETMAP_API < 11) || (NETMAP_API > 15)
+#error
+#endif
 int main(void) { return 0; }
 EOF
   if compile_prog "" "" ; then
diff --git a/net/netmap.c b/net/netmap.c
index 73f6d7a..8213304 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -27,6 +27,8 @@
 #include <net/if.h>
 #include <sys/mman.h>
 #include <stdint.h>
+#include <stdio.h>
+#define NETMAP_WITH_LIBS
 #include <net/netmap.h>
 #include <net/netmap_user.h>
 
@@ -58,31 +60,6 @@ typedef struct NetmapState {
     int                 vnet_hdr_len;  /* Current virtio-net header length. */
 } NetmapState;
 
-#define D(format, ...)                                          \
-    do {                                                        \
-        struct timeval __xxts;                                  \
-        gettimeofday(&__xxts, NULL);                            \
-        printf("%03d.%06d %s [%d] " format "\n",                \
-                (int)__xxts.tv_sec % 1000, (int)__xxts.tv_usec, \
-                __func__, __LINE__, ##__VA_ARGS__);         \
-    } while (0)
-
-/* Rate limited version of "D", lps indicates how many per second */
-#define RD(lps, format, ...)                                    \
-    do {                                                        \
-        static int t0, __cnt;                                   \
-        struct timeval __xxts;                                  \
-        gettimeofday(&__xxts, NULL);                            \
-        if (t0 != __xxts.tv_sec) {                              \
-            t0 = __xxts.tv_sec;                                 \
-            __cnt = 0;                                          \
-        }                                                       \
-        if (__cnt++ < lps) {                                    \
-            D(format, ##__VA_ARGS__);                           \
-        }                                                       \
-    } while (0)
-
-
 #ifndef __FreeBSD__
 #define pkt_copy bcopy
 #else
@@ -239,7 +216,7 @@ static ssize_t netmap_receive(NetClientState *nc,
         return size;
     }
 
-    if (ring->avail == 0) {
+    if (nm_ring_empty(ring)) {
         /* No available slots in the netmap TX ring. */
         netmap_write_poll(s, true);
         return 0;
@@ -252,8 +229,7 @@ static ssize_t netmap_receive(NetClientState *nc,
     ring->slot[i].len = size;
     ring->slot[i].flags = 0;
     pkt_copy(buf, dst, size);
-    ring->cur = NETMAP_RING_NEXT(ring, i);
-    ring->avail--;
+    ring->cur = ring->head = nm_ring_next(ring, i);
     ioctl(s->me.fd, NIOCTXSYNC, NULL);
 
     return size;
@@ -269,7 +245,6 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
     uint8_t *dst;
     int j;
     uint32_t i;
-    uint32_t avail;
 
     if (unlikely(!ring)) {
         /* Drop the packet. */
@@ -277,9 +252,8 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
     }
 
     last = i = ring->cur;
-    avail = ring->avail;
 
-    if (avail < iovcnt) {
+    if (nm_ring_space(ring) < iovcnt) {
         /* Not enough netmap slots. */
         netmap_write_poll(s, true);
         return 0;
@@ -295,7 +269,7 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
         while (iov_frag_size) {
             nm_frag_size = MIN(iov_frag_size, ring->nr_buf_size);
 
-            if (unlikely(avail == 0)) {
+            if (unlikely(nm_ring_empty(ring))) {
                 /* We run out of netmap slots while splitting the
                    iovec fragments. */
                 netmap_write_poll(s, true);
@@ -310,8 +284,7 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
             pkt_copy(iov[j].iov_base + offset, dst, nm_frag_size);
 
             last = i;
-            i = NETMAP_RING_NEXT(ring, i);
-            avail--;
+            i = nm_ring_next(ring, i);
 
             offset += nm_frag_size;
             iov_frag_size -= nm_frag_size;
@@ -320,9 +293,8 @@ 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->avail. */
-    ring->cur = i;
-    ring->avail = avail;
+    /* Now update ring->cur and ring->head. */
+    ring->cur = ring->head = i;
 
     ioctl(s->me.fd, NIOCTXSYNC, NULL);
 
@@ -345,7 +317,7 @@ static void netmap_send(void *opaque)
 
     /* Keep sending while there are available packets into the netmap
        RX ring and the forwarding path towards the peer is open. */
-    while (ring->avail > 0 && qemu_can_send_packet(&s->nc)) {
+    while (!nm_ring_empty(ring) && qemu_can_send_packet(&s->nc)) {
         uint32_t i;
         uint32_t idx;
         bool morefrag;
@@ -360,11 +332,10 @@ static void netmap_send(void *opaque)
             s->iov[iovcnt].iov_len = ring->slot[i].len;
             iovcnt++;
 
-            ring->cur = NETMAP_RING_NEXT(ring, i);
-            ring->avail--;
-        } while (ring->avail && morefrag);
+            ring->cur = ring->head = nm_ring_next(ring, i);
+        } while (!nm_ring_empty(ring) && morefrag);
 
-        if (unlikely(!ring->avail && morefrag)) {
+        if (unlikely(nm_ring_empty(ring) && morefrag)) {
             RD(5, "[netmap_send] ran out of slots, with a pending"
                    "incomplete packet\n");
         }
-- 
1.9.0

Re: [Qemu-devel] [PATCH v2] net: Disable netmap backend when not supported

[Stefan Hajnoczi]

On Thu, Feb 20, 2014 at 03:40:43PM +0100, Vincenzo Maffione wrote:
> This patch fixes configure so that the netmap backend is not compiled in if the
> host doesn't support an API version >= 11. A version upper bound (15) has been
> added so that the netmap API can be extended with some minor features without
> requiring QEMU code modifications.
> 
> Moreover, some changes have been done to net/netmap.c in order to reflect the
> current netmap API/ABI (11).
> 
> The NETMAP_WITH_LIBS macro makes possible to include some utilities (e.g.
> netmap ring macros, D(), RD() and other high level functions) through the netmap
> headers. In this way we get rid of the D and RD macro definitions in the QEMU
> code, and we open the way for further code simplifications that will be
> introduced by future patches.
> 
> Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
> ---
> Note: This patch is against the net-next/net Stefan's branch.
> 
> Changes against the previous version:
>    (1) more complete commit description
>    (2) add comment in ./configure to explain version checks
> 
>  configure    | 10 +++++++++-
>  net/netmap.c | 55 +++++++++++++------------------------------------------
>  2 files changed, 22 insertions(+), 43 deletions(-)

Thanks for adding the explanations, it will make it easier for people
reading the code to understand this change in the future.

Applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

Re: [Qemu-devel] [PATCH v2] net: Disable netmap backend when not supported

[Vincenzo Maffione]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

Hi,
  I cannot see it in your net tree.

Thanks,
  Vincenzo


2014-02-21 11:05 GMT+01:00 Stefan Hajnoczi <stefanha@redhat.com>:

> On Thu, Feb 20, 2014 at 03:40:43PM +0100, Vincenzo Maffione wrote:
> > This patch fixes configure so that the netmap backend is not compiled in
> if the
> > host doesn't support an API version >= 11. A version upper bound (15)
> has been
> > added so that the netmap API can be extended with some minor features
> without
> > requiring QEMU code modifications.
> >
> > Moreover, some changes have been done to net/netmap.c in order to
> reflect the
> > current netmap API/ABI (11).
> >
> > The NETMAP_WITH_LIBS macro makes possible to include some utilities (e.g.
> > netmap ring macros, D(), RD() and other high level functions) through
> the netmap
> > headers. In this way we get rid of the D and RD macro definitions in the
> QEMU
> > code, and we open the way for further code simplifications that will be
> > introduced by future patches.
> >
> > Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
> > ---
> > Note: This patch is against the net-next/net Stefan's branch.
> >
> > Changes against the previous version:
> >    (1) more complete commit description
> >    (2) add comment in ./configure to explain version checks
> >
> >  configure    | 10 +++++++++-
> >  net/netmap.c | 55
> +++++++++++++------------------------------------------
> >  2 files changed, 22 insertions(+), 43 deletions(-)
>
> Thanks for adding the explanations, it will make it easier for people
> reading the code to understand this change in the future.
>
> Applied to my net tree:
> https://github.com/stefanha/qemu/commits/net
>
> Stefan
>



-- 
Vincenzo Maffione

[-- Attachment #2: Type: text/html, Size: 2438 bytes --]

Re: [Qemu-devel] [PATCH v2] net: Disable netmap backend when not supported

[Stefan Hajnoczi]

On Mon, Feb 24, 2014 at 10:54:43AM +0100, Vincenzo Maffione wrote:
>   I cannot see it in your net tree.

It's there now.  Sorry, I sometimes forget to push to my public repo.

Stefan