[Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support

[Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support

[Vincenzo Maffione]

The purpose of this patch series is to add offloadings support
(TSO/UFO/CSUM) to the netmap network backend, and make it possible
for the paravirtual network frontends (virtio-net and vmxnet3) to
use it.
In order to achieve this, these patches extend the existing
net.h interface to add abstract operations through which a network
frontend can manipulate backend offloading features, instead of
directly calling TAP-specific functions.

Guest-to-guest performance before this patches for virtio-net + netmap:

    TCP_STREAM                  5.0 Gbps
    TCP_RR                      12.7 Gbps
    UDP_STREAM (64-bytes)       790 Kpps

Guest-to-guest performance after this patches for virtio-net + netmap:

    TCP_STREAM                  21.4 Gbps
    TCP_RR                      12.7 Gbps
    UDP_STREAM (64-bytes)       790 Kpps

Experiment details:
    - Processor: Intel i7-3770K CPU @ 3.50GHz (8 cores)
    - Memory @ 1333 MHz
    - Host O.S.: Archlinux with Linux 3.11
    - Guest O.S.: Archlinux with Linux 3.11

    - QEMU command line:
        qemu-system-x86_64 archdisk.qcow -snapshot -enable-kvm -device virtio-net-pci,ioeventfd=on,mac=00:AA:BB:CC:DD:01,netdev=mynet -netdev netmap,ifname=vale0:01,id=mynet -smp 2 -vga std -m 3G


******** Changes against the previous version ***********
(1) The first two commits were not included into the previous version. They
    are used to clean the TAP interface before the offloadings manipulation
    API is introduced.

(2) The fifth commit merges two commits from the previous version: 3/5 "virtio-net and
    vmxnet3 use offloading API" and 5/5 "virtio-net and vmxnet3 can use netmap offloadings".
    This because of (3).

(3) There is actually an important problem. In the previous patch version, TCP/UDP traffic was
    supported between two guests attached to a VALE switch if and only if both guests use (or
    don't) the same offloadings: e.g. two virtio-net guests or two e1000 guests. This is why
    in this version I put the commit which adds the support for netmap as the last one and
    I temporarily disable the offloading feature (e.g. netmap_has_vnet_hdr() returns false).
    We are working at adding proper support also in the general case in which there is
    a mismatch between the NIC offloadings capabilities. As soon as we have proper support,
    we can enable it in net/netmap.c.
    What do you think about that? What's the best way to manage this temporary lack of
    support?


Vincenzo Maffione (6):
  net: change vnet-hdr TAP prototypes
  net: removing tap_using_vnet_hdr() function
  net: extend NetClientInfo for offloading manipulations
  net: TAP uses NetClientInfo offloading callbacks
  net: virtio-net and vmxnet3 use offloading API
  net: add offloadings support to netmap backend

 hw/net/virtio-net.c | 18 +++++-----------
 hw/net/vmxnet3.c    | 12 ++++-------
 include/net/net.h   | 17 +++++++++++++++
 include/net/tap.h   |  5 ++---
 net/net.c           | 46 +++++++++++++++++++++++++++++++++++++++++
 net/netmap.c        | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 net/tap-win32.c     | 12 ++++-------
 net/tap.c           | 22 ++++++++------------
 8 files changed, 145 insertions(+), 46 deletions(-)

-- 
1.8.5.2

[Qemu-devel] [PATCH v2 1/6] net: change vnet-hdr TAP prototypes

[Vincenzo Maffione]

The tap_has_vnet_hdr() and tap_has_vnet_hdr_len() functions used
to return int, even though they only return true/false values.
This patch changes the prototypes to return bool.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 include/net/tap.h | 4 ++--
 net/tap-win32.c   | 8 ++++----
 net/tap.c         | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/net/tap.h b/include/net/tap.h
index a994f20..a3490a9 100644
--- a/include/net/tap.h
+++ b/include/net/tap.h
@@ -30,8 +30,8 @@
 #include "qapi-types.h"
 
 bool tap_has_ufo(NetClientState *nc);
-int tap_has_vnet_hdr(NetClientState *nc);
-int tap_has_vnet_hdr_len(NetClientState *nc, int len);
+bool tap_has_vnet_hdr(NetClientState *nc);
+bool tap_has_vnet_hdr_len(NetClientState *nc, int len);
 void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr);
 void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int ecn, int ufo);
 void tap_set_vnet_hdr_len(NetClientState *nc, int len);
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 91e9e84..edf26c4 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -727,9 +727,9 @@ bool tap_has_ufo(NetClientState *nc)
     return false;
 }
 
-int tap_has_vnet_hdr(NetClientState *nc)
+bool tap_has_vnet_hdr(NetClientState *nc)
 {
-    return 0;
+    return false;
 }
 
 int tap_probe_vnet_hdr_len(int fd, int len)
@@ -755,9 +755,9 @@ struct vhost_net *tap_get_vhost_net(NetClientState *nc)
     return NULL;
 }
 
-int tap_has_vnet_hdr_len(NetClientState *nc, int len)
+bool tap_has_vnet_hdr_len(NetClientState *nc, int len)
 {
-    return 0;
+    return false;
 }
 
 void tap_set_vnet_hdr_len(NetClientState *nc, int len)
diff --git a/net/tap.c b/net/tap.c
index 39c1cda..c805f3c 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -219,7 +219,7 @@ bool tap_has_ufo(NetClientState *nc)
     return s->has_ufo;
 }
 
-int tap_has_vnet_hdr(NetClientState *nc)
+bool tap_has_vnet_hdr(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
@@ -228,13 +228,13 @@ int tap_has_vnet_hdr(NetClientState *nc)
     return !!s->host_vnet_hdr_len;
 }
 
-int tap_has_vnet_hdr_len(NetClientState *nc, int len)
+bool tap_has_vnet_hdr_len(NetClientState *nc, int len)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
     assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
 
-    return tap_probe_vnet_hdr_len(s->fd, len);
+    return !!tap_probe_vnet_hdr_len(s->fd, len);
 }
 
 void tap_set_vnet_hdr_len(NetClientState *nc, int len)
-- 
1.8.5.2

[Qemu-devel] [PATCH v2 2/6] net: removing tap_using_vnet_hdr() function

[Vincenzo Maffione]

This function was used to set the using_vnet_hdr field into the
TAPState struct. However, it is always called immediately before
(see virtio-net.c) or immediately after (see vmxnet3.c) the function
tap_set_vnet_hdr_len(). It's therefore possible to set the
using_vnet_hdr field directly in tap_set_vnet_hdr_len() and remove
tap_using_vnet_hdr(), making the code simpler.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 hw/net/virtio-net.c |  4 ----
 hw/net/vmxnet3.c    |  2 --
 include/net/tap.h   |  1 -
 net/tap-win32.c     |  4 ----
 net/tap.c           | 11 +----------
 5 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3626608..ac8322d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1496,7 +1496,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIONet *n = VIRTIO_NET(dev);
     NetClientState *nc;
-    int i;
 
     virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
@@ -1543,9 +1542,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 
     peer_test_vnet_hdr(n);
     if (peer_has_vnet_hdr(n)) {
-        for (i = 0; i < n->max_queues; i++) {
-            tap_using_vnet_hdr(qemu_get_subqueue(n->nic, i)->peer, true);
-        }
         n->host_hdr_len = sizeof(struct virtio_net_hdr);
     } else {
         n->host_hdr_len = 0;
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 19687aa..096b598 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1937,8 +1937,6 @@ static void vmxnet3_net_init(VMXNET3State *s)
     if (s->peer_has_vhdr) {
         tap_set_vnet_hdr_len(qemu_get_queue(s->nic)->peer,
             sizeof(struct virtio_net_hdr));
-
-        tap_using_vnet_hdr(qemu_get_queue(s->nic)->peer, 1);
     }
 
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
diff --git a/include/net/tap.h b/include/net/tap.h
index a3490a9..54974dc 100644
--- a/include/net/tap.h
+++ b/include/net/tap.h
@@ -32,7 +32,6 @@
 bool tap_has_ufo(NetClientState *nc);
 bool tap_has_vnet_hdr(NetClientState *nc);
 bool tap_has_vnet_hdr_len(NetClientState *nc, int len);
-void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr);
 void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int ecn, int ufo);
 void tap_set_vnet_hdr_len(NetClientState *nc, int len);
 int tap_enable(NetClientState *nc);
diff --git a/net/tap-win32.c b/net/tap-win32.c
index edf26c4..f4cd002 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -741,10 +741,6 @@ void tap_fd_set_vnet_hdr_len(int fd, int len)
 {
 }
 
-void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
-{
-}
-
 void tap_set_offload(NetClientState *nc, int csum, int tso4,
                      int tso6, int ecn, int ufo)
 {
diff --git a/net/tap.c b/net/tap.c
index c805f3c..42f768c 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -247,16 +247,7 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int len)
 
     tap_fd_set_vnet_hdr_len(s->fd, len);
     s->host_vnet_hdr_len = len;
-}
-
-void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
-{
-    TAPState *s = DO_UPCAST(TAPState, nc, nc);
-
-    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
-    assert(!!s->host_vnet_hdr_len == using_vnet_hdr);
-
-    s->using_vnet_hdr = using_vnet_hdr;
+    s->using_vnet_hdr = true;
 }
 
 void tap_set_offload(NetClientState *nc, int csum, int tso4,
-- 
1.8.5.2

[Qemu-devel] [PATCH v2 3/6] net: extend NetClientInfo for offloading manipulations

[Vincenzo Maffione]

A set of new callbacks has been added to the NetClientInfo struct in
order to abstract the operations done by virtio-net and vmxnet3
frontends to manipulate TAP offloadings.

The net.h API has been extended with functions that access those
abstract operations, providing frontends with a way to manipulate
backend offloadings.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 include/net/net.h | 17 +++++++++++++++++
 net/net.c         | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 11e1468..29a0698 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -50,6 +50,11 @@ typedef void (NetCleanup) (NetClientState *);
 typedef void (LinkStatusChanged)(NetClientState *);
 typedef void (NetClientDestructor)(NetClientState *);
 typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
+typedef bool (HasUfo)(NetClientState *);
+typedef bool (HasVnetHdr)(NetClientState *);
+typedef bool (HasVnetHdrLen)(NetClientState *, int);
+typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
+typedef void (SetVnetHdrLen)(NetClientState *, int);
 
 typedef struct NetClientInfo {
     NetClientOptionsKind type;
@@ -62,6 +67,11 @@ typedef struct NetClientInfo {
     LinkStatusChanged *link_status_changed;
     QueryRxFilter *query_rx_filter;
     NetPoll *poll;
+    HasUfo *has_ufo;
+    HasVnetHdr *has_vnet_hdr;
+    HasVnetHdrLen *has_vnet_hdr_len;
+    SetOffload *set_offload;
+    SetVnetHdrLen *set_vnet_hdr_len;
 } NetClientInfo;
 
 struct NetClientState {
@@ -120,6 +130,13 @@ ssize_t qemu_send_packet_async(NetClientState *nc, const uint8_t *buf,
 void qemu_purge_queued_packets(NetClientState *nc);
 void qemu_flush_queued_packets(NetClientState *nc);
 void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
+bool qemu_peer_has_ufo(NetClientState *nc);
+bool qemu_peer_has_vnet_hdr(NetClientState *nc);
+bool qemu_peer_has_vnet_hdr_len(NetClientState *nc, int len);
+void qemu_peer_using_vnet_hdr(NetClientState *nc, bool enable);
+void qemu_peer_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
+                           int ecn, int ufo);
+void qemu_peer_set_vnet_hdr_len(NetClientState *nc, int len);
 void qemu_macaddr_default_if_unset(MACAddr *macaddr);
 int qemu_show_nic_models(const char *arg, const char *const *models);
 void qemu_check_nic_model(NICInfo *nd, const char *model);
diff --git a/net/net.c b/net/net.c
index f8db85f..bc90a27 100644
--- a/net/net.c
+++ b/net/net.c
@@ -381,6 +381,52 @@ void qemu_foreach_nic(qemu_nic_foreach func, void *opaque)
     }
 }
 
+bool qemu_peer_has_ufo(NetClientState *nc)
+{
+    if (!nc->peer || !nc->peer->info->has_ufo) {
+        return false;
+    }
+
+    return nc->peer->info->has_ufo(nc->peer);
+}
+
+bool qemu_peer_has_vnet_hdr(NetClientState *nc)
+{
+    if (!nc->peer || !nc->peer->info->has_vnet_hdr) {
+        return false;
+    }
+
+    return nc->peer->info->has_vnet_hdr(nc->peer);
+}
+
+bool qemu_peer_has_vnet_hdr_len(NetClientState *nc, int len)
+{
+    if (!nc->peer || !nc->peer->info->has_vnet_hdr_len) {
+        return false;
+    }
+
+    return nc->peer->info->has_vnet_hdr_len(nc->peer, len);
+}
+
+void qemu_peer_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
+                          int ecn, int ufo)
+{
+    if (!nc->peer || !nc->peer->info->set_offload) {
+        return;
+    }
+
+    nc->peer->info->set_offload(nc->peer, csum, tso4, tso6, ecn, ufo);
+}
+
+void qemu_peer_set_vnet_hdr_len(NetClientState *nc, int len)
+{
+    if (!nc->peer || !nc->peer->info->set_vnet_hdr_len) {
+        return;
+    }
+
+    nc->peer->info->set_vnet_hdr_len(nc->peer, len);
+}
+
 int qemu_can_send_packet(NetClientState *sender)
 {
     if (!sender->peer) {
-- 
1.8.5.2

[Qemu-devel] [PATCH v2 4/6] net: TAP uses NetClientInfo offloading callbacks

[Vincenzo Maffione]

The TAP NetClientInfo structure is inizialized with the TAP-specific
callbacks that manipulates backend offloading features.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 net/tap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/tap.c b/net/tap.c
index 42f768c..e6c598d 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -305,6 +305,11 @@ static NetClientInfo net_tap_info = {
     .receive_iov = tap_receive_iov,
     .poll = tap_poll,
     .cleanup = tap_cleanup,
+    .has_ufo = tap_has_ufo,
+    .has_vnet_hdr = tap_has_vnet_hdr,
+    .has_vnet_hdr_len = tap_has_vnet_hdr_len,
+    .set_offload = tap_set_offload,
+    .set_vnet_hdr_len = tap_set_vnet_hdr_len,
 };
 
 static TAPState *net_tap_fd_init(NetClientState *peer,
-- 
1.8.5.2

[Qemu-devel] [PATCH v2 5/6] net: virtio-net and vmxnet3 use offloading API

[Vincenzo Maffione]

With this patch, virtio-net and vmxnet3 frontends make
use of the qemu_peer_* API for backend offloadings manipulations,
instead of calling TAP-specific functions directly.
We also remove the existing checks which prevent those frontends
from using offloadings with backends different from TAP (e.g. netmap).

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 hw/net/virtio-net.c | 14 +++++---------
 hw/net/vmxnet3.c    | 10 ++++------
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index ac8322d..c850d5b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -325,11 +325,7 @@ static void peer_test_vnet_hdr(VirtIONet *n)
         return;
     }
 
-    if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) {
-        return;
-    }
-
-    n->has_vnet_hdr = tap_has_vnet_hdr(nc->peer);
+    n->has_vnet_hdr = qemu_peer_has_vnet_hdr(nc);
 }
 
 static int peer_has_vnet_hdr(VirtIONet *n)
@@ -342,7 +338,7 @@ static int peer_has_ufo(VirtIONet *n)
     if (!peer_has_vnet_hdr(n))
         return 0;
 
-    n->has_ufo = tap_has_ufo(qemu_get_queue(n->nic)->peer);
+    n->has_ufo = qemu_peer_has_ufo(qemu_get_queue(n->nic));
 
     return n->has_ufo;
 }
@@ -361,8 +357,8 @@ static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs)
         nc = qemu_get_subqueue(n->nic, i);
 
         if (peer_has_vnet_hdr(n) &&
-            tap_has_vnet_hdr_len(nc->peer, n->guest_hdr_len)) {
-            tap_set_vnet_hdr_len(nc->peer, n->guest_hdr_len);
+            qemu_peer_has_vnet_hdr_len(nc, n->guest_hdr_len)) {
+            qemu_peer_set_vnet_hdr_len(nc, n->guest_hdr_len);
             n->host_hdr_len = n->guest_hdr_len;
         }
     }
@@ -463,7 +459,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
 
 static void virtio_net_apply_guest_offloads(VirtIONet *n)
 {
-    tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
+    qemu_peer_set_offload(qemu_get_subqueue(n->nic, 0),
             !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_CSUM)),
             !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
             !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 096b598..ded53a4 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1290,7 +1290,7 @@ static void vmxnet3_update_features(VMXNET3State *s)
               s->lro_supported, rxcso_supported,
               s->rx_vlan_stripping);
     if (s->peer_has_vhdr) {
-        tap_set_offload(qemu_get_queue(s->nic)->peer,
+        qemu_peer_set_offload(qemu_get_queue(s->nic),
                         rxcso_supported,
                         s->lro_supported,
                         s->lro_supported,
@@ -1883,11 +1883,9 @@ static NetClientInfo net_vmxnet3_info = {
 
 static bool vmxnet3_peer_has_vnet_hdr(VMXNET3State *s)
 {
-    NetClientState *peer = qemu_get_queue(s->nic)->peer;
+    NetClientState *nc = qemu_get_queue(s->nic);
 
-    if ((NULL != peer)                              &&
-        (peer->info->type == NET_CLIENT_OPTIONS_KIND_TAP)   &&
-        tap_has_vnet_hdr(peer)) {
+    if (qemu_peer_has_vnet_hdr(nc)) {
         return true;
     }
 
@@ -1935,7 +1933,7 @@ static void vmxnet3_net_init(VMXNET3State *s)
     s->lro_supported = false;
 
     if (s->peer_has_vhdr) {
-        tap_set_vnet_hdr_len(qemu_get_queue(s->nic)->peer,
+        qemu_peer_set_vnet_hdr_len(qemu_get_queue(s->nic),
             sizeof(struct virtio_net_hdr));
     }
 
-- 
1.8.5.2

[Qemu-devel] [PATCH v2 6/6] net: add offloadings support to netmap backend

[Vincenzo Maffione]

Whit this patch, the netmap backend supports TSO/UFO/CSUM
offloadings, and accepts the virtio-net header, similarly to what
happens with TAP. The offloading callbacks in the NetClientInfo
interface have been implemented.

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

diff --git a/net/netmap.c b/net/netmap.c
index 0ccc497..d99bb29 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -54,6 +54,7 @@ typedef struct NetmapState {
     bool                read_poll;
     bool                write_poll;
     struct iovec        iov[IOV_MAX];
+    int                 vnet_hdr_len;  /* Current virtio-net header length. */
 } NetmapState;
 
 #define D(format, ...)                                          \
@@ -274,7 +275,7 @@ static ssize_t netmap_receive_iov(NetClientState *nc,
         return iov_size(iov, iovcnt);
     }
 
-    i = ring->cur;
+    last = i = ring->cur;
     avail = ring->avail;
 
     if (avail < iovcnt) {
@@ -394,6 +395,56 @@ static void netmap_cleanup(NetClientState *nc)
     s->me.fd = -1;
 }
 
+/* Offloading manipulation support callbacks.
+ *
+ * Currently we are simply able to pass the virtio-net header
+ * throughout the VALE switch, without modifying it or interpreting it.
+ * For this reason we are always able to support TSO/UFO/CSUM and
+ * different header lengths as long as two virtio-net-header-capable
+ * VMs are attached to the bridge.
+ */
+static bool netmap_has_ufo(NetClientState *nc)
+{
+    return false; /* Temporarily disabled. */
+}
+
+static bool netmap_has_vnet_hdr(NetClientState *nc)
+{
+    return false; /* Temporarily disabled. */
+}
+
+static bool netmap_has_vnet_hdr_len(NetClientState *nc, int len)
+{
+    return false; /* Temporarily disabled. */
+}
+
+static void netmap_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
+                               int ecn, int ufo)
+{
+}
+
+static void netmap_set_vnet_hdr_len(NetClientState *nc, int len)
+{
+    NetmapState *s = DO_UPCAST(NetmapState, nc, nc);
+    int err;
+    struct nmreq req;
+
+    /* Issue a NETMAP_BDG_OFFSET command to change the netmap adapter
+       offset of 'me->ifname'. */
+    memset(&req, 0, sizeof(req));
+    pstrcpy(req.nr_name, sizeof(req.nr_name), s->me.ifname);
+    req.nr_version = NETMAP_API;
+    req.nr_cmd = NETMAP_BDG_OFFSET;
+    req.nr_arg1 = len;
+    err = ioctl(s->me.fd, NIOCREGIF, &req);
+    if (err) {
+        error_report("Unable to execute NETMAP_BDG_OFFSET on %s: %s",
+                     s->me.ifname, strerror(errno));
+    } else {
+        /* Keep track of the current length, may be useful in the future. */
+        s->vnet_hdr_len = len;
+    }
+}
 
 /* NetClientInfo methods */
 static NetClientInfo net_netmap_info = {
@@ -403,6 +454,11 @@ static NetClientInfo net_netmap_info = {
     .receive_iov = netmap_receive_iov,
     .poll = netmap_poll,
     .cleanup = netmap_cleanup,
+    .has_ufo = netmap_has_ufo,
+    .has_vnet_hdr = netmap_has_vnet_hdr,
+    .has_vnet_hdr_len = netmap_has_vnet_hdr_len,
+    .set_offload = netmap_set_offload,
+    .set_vnet_hdr_len = netmap_set_vnet_hdr_len,
 };
 
 /* The exported init function
@@ -428,6 +484,7 @@ int net_init_netmap(const NetClientOptions *opts,
     nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", name);
     s = DO_UPCAST(NetmapState, nc, nc);
     s->me = me;
+    s->vnet_hdr_len = 0;
     netmap_read_poll(s, true); /* Initially only poll for reads. */
 
     return 0;
-- 
1.8.5.2

Re: [Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support

[Barak Wasserstrom]

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

Vincenzo,
I'm using a tap interface and in the guest virtual device i see all
offloading features are disabled, even though they are enabled in the
physical device.
Perhaps you can help? See below related information:

Bridge to the physical interface in the host:
---------------------------------------------------------------------------
brctl addbr br0
brctl  addif br0 eth3
---------------------------------------------------------------------------

/etc/qemu-ifup:
---------------------------------------------------------------------------
#!/bin/sh
set -x

switch=br0

if [ -n "$1" ];then
        /usr/bin/sudo /usr/sbin/tunctl -u `whoami` -t $1
        /usr/bin/sudo /sbin/ip link set $1 up
        sleep 0.5s
        /usr/bin/sudo /sbin/brctl addif $switch $1
        exit 0
else
        echo "Error: no interface specified"
        exit 1
fi
---------------------------------------------------------------------------

Activation command:
---------------------------------------------------------------------------
qemu-system-arm -enable-kvm  -M vexpress-a15  -serial /dev/ttyS1 -append
'root=/dev/vda rw console=ttyAMA0 rootwait earlyprintk' -nographic -kernel
/guest/zImage_vexpress -dtb /guest/vexpress-v2p-ca15_a7.dtb -drive
if=none,file=/guest/arm-wheezy.img,id=foo -device
virtio-blk-device,drive=foo -device
virtio-net-device,netdev=net0,mac=DE:AD:BE:EF:F4:E5 -netdev tap,id=net0
---------------------------------------------------------------------------

Physical interface features (ethtool -k eth3):
---------------------------------------------------------------------------
Features for eth3:
rx-checksumming: on
tx-checksumming: on
tx-checksum-ipv4: on
tx-checksum-ip-generic: off [fixed]
tx-checksum-ipv6: on
tx-checksum-fcoe-crc: off [fixed]
tx-checksum-sctp: off [fixed]
scatter-gather: on
tx-scatter-gather: on
tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: on
tx-tcp-segmentation: on
tx-tcp-ecn-segmentation: on
tx-tcp6-segmentation: on
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: off [fixed]
tx-vlan-offload: off [fixed]
ntuple-filters: on
receive-hashing: on
highdma: on
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: on
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
---------------------------------------------------------------------------

Virtual device features in the guest (ethtool -k eth0):
---------------------------------------------------------------------------
Features for eth0:

rx-checksumming: off [fixed]

tx-checksumming: off

        tx-checksum-ipv4: off [fixed]

        tx-checksum-ip-generic: off [fixed]

        tx-checksum-ipv6: off [fixed]

        tx-checksum-fcoe-crc: off [fixed]

        tx-checksum-sctp: off [fixed]

scatter-gather: off

        tx-scatter-gather: off [fixed]

        tx-scatter-gather-fraglist: off [fixed]

tcp-segmentation-offload: off

        tx-tcp-segmentation: off [fixed]

        tx-tcp-ecn-segmentation: off [fixed]

        tx-tcp6-segmentation: off [fixed]

udp-fragmentation-offload: off [fixed]

generic-segmentation-offload: off [requested on]

generic-receive-offload: on

large-receive-offload: off [fixed]

rx-vlan-offload: off [fixed]

tx-vlan-offload: off [fixed]

ntuple-filters: off [fixed]

receive-hashing: off [fixed]

highdma: on [fixed]

rx-vlan-filter: off [fixed]

vlan-challenged: off [fixed]

tx-lockless: off [fixed]

netns-local: off [fixed]

tx-gso-robust: off [fixed]

tx-fcoe-segmentation: off [fixed]

tx-gre-segmentation: off [fixed]

tx-ipip-segmentation: off [fixed]

tx-sit-segmentation: off [fixed]

tx-udp_tnl-segmentation: off [fixed]

tx-mpls-segmentation: off [fixed]

fcoe-mtu: off [fixed]

tx-nocache-copy: off

loopback: off [fixed]

rx-fcs: off [fixed]

rx-all: off [fixed]

tx-vlan-stag-hw-insert: off [fixed]

rx-vlan-stag-hw-parse: off [fixed]

rx-vlan-stag-filter: off [fixed]

l2-fwd-offload: off [fixed]

---------------------------------------------------------------------------

Regards,
Barak



On Tue, Jan 14, 2014 at 12:59 PM, Vincenzo Maffione <v.maffione@gmail.com>wrote:

> The purpose of this patch series is to add offloadings support
> (TSO/UFO/CSUM) to the netmap network backend, and make it possible
> for the paravirtual network frontends (virtio-net and vmxnet3) to
> use it.
> In order to achieve this, these patches extend the existing
> net.h interface to add abstract operations through which a network
> frontend can manipulate backend offloading features, instead of
> directly calling TAP-specific functions.
>
> Guest-to-guest performance before this patches for virtio-net + netmap:
>
>     TCP_STREAM                  5.0 Gbps
>     TCP_RR                      12.7 Gbps
>     UDP_STREAM (64-bytes)       790 Kpps
>
> Guest-to-guest performance after this patches for virtio-net + netmap:
>
>     TCP_STREAM                  21.4 Gbps
>     TCP_RR                      12.7 Gbps
>     UDP_STREAM (64-bytes)       790 Kpps
>
> Experiment details:
>     - Processor: Intel i7-3770K CPU @ 3.50GHz (8 cores)
>     - Memory @ 1333 MHz
>     - Host O.S.: Archlinux with Linux 3.11
>     - Guest O.S.: Archlinux with Linux 3.11
>
>     - QEMU command line:
>         qemu-system-x86_64 archdisk.qcow -snapshot -enable-kvm -device
> virtio-net-pci,ioeventfd=on,mac=00:AA:BB:CC:DD:01,netdev=mynet -netdev
> netmap,ifname=vale0:01,id=mynet -smp 2 -vga std -m 3G
>
>
> ******** Changes against the previous version ***********
> (1) The first two commits were not included into the previous version. They
>     are used to clean the TAP interface before the offloadings manipulation
>     API is introduced.
>
> (2) The fifth commit merges two commits from the previous version: 3/5
> "virtio-net and
>     vmxnet3 use offloading API" and 5/5 "virtio-net and vmxnet3 can use
> netmap offloadings".
>     This because of (3).
>
> (3) There is actually an important problem. In the previous patch version,
> TCP/UDP traffic was
>     supported between two guests attached to a VALE switch if and only if
> both guests use (or
>     don't) the same offloadings: e.g. two virtio-net guests or two e1000
> guests. This is why
>     in this version I put the commit which adds the support for netmap as
> the last one and
>     I temporarily disable the offloading feature (e.g.
> netmap_has_vnet_hdr() returns false).
>     We are working at adding proper support also in the general case in
> which there is
>     a mismatch between the NIC offloadings capabilities. As soon as we
> have proper support,
>     we can enable it in net/netmap.c.
>     What do you think about that? What's the best way to manage this
> temporary lack of
>     support?
>
>
> Vincenzo Maffione (6):
>   net: change vnet-hdr TAP prototypes
>   net: removing tap_using_vnet_hdr() function
>   net: extend NetClientInfo for offloading manipulations
>   net: TAP uses NetClientInfo offloading callbacks
>   net: virtio-net and vmxnet3 use offloading API
>   net: add offloadings support to netmap backend
>
>  hw/net/virtio-net.c | 18 +++++-----------
>  hw/net/vmxnet3.c    | 12 ++++-------
>  include/net/net.h   | 17 +++++++++++++++
>  include/net/tap.h   |  5 ++---
>  net/net.c           | 46 +++++++++++++++++++++++++++++++++++++++++
>  net/netmap.c        | 59
> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  net/tap-win32.c     | 12 ++++-------
>  net/tap.c           | 22 ++++++++------------
>  8 files changed, 145 insertions(+), 46 deletions(-)
>
> --
> 1.8.5.2
>
>
>

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

Re: [Qemu-devel] [PATCH v2 2/6] net: removing tap_using_vnet_hdr() function

[Stefan Hajnoczi]

On Tue, Jan 14, 2014 at 11:59:46AM +0100, Vincenzo Maffione wrote:
> This function was used to set the using_vnet_hdr field into the
> TAPState struct. However, it is always called immediately before
> (see virtio-net.c) or immediately after (see vmxnet3.c) the function
> tap_set_vnet_hdr_len(). It's therefore possible to set the
> using_vnet_hdr field directly in tap_set_vnet_hdr_len() and remove
> tap_using_vnet_hdr(), making the code simpler.
> 
> Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
> ---
>  hw/net/virtio-net.c |  4 ----
>  hw/net/vmxnet3.c    |  2 --
>  include/net/tap.h   |  1 -
>  net/tap-win32.c     |  4 ----
>  net/tap.c           | 11 +----------
>  5 files changed, 1 insertion(+), 21 deletions(-)

CCing Michael Tsirkin in case he has any concerns.  This looks safe to
me since virtio-net will call tap_set_vnet_hdr_len().

> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 3626608..ac8322d 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1496,7 +1496,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VirtIONet *n = VIRTIO_NET(dev);
>      NetClientState *nc;
> -    int i;
>  
>      virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
>  
> @@ -1543,9 +1542,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>  
>      peer_test_vnet_hdr(n);
>      if (peer_has_vnet_hdr(n)) {
> -        for (i = 0; i < n->max_queues; i++) {
> -            tap_using_vnet_hdr(qemu_get_subqueue(n->nic, i)->peer, true);
> -        }
>          n->host_hdr_len = sizeof(struct virtio_net_hdr);
>      } else {
>          n->host_hdr_len = 0;
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 19687aa..096b598 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1937,8 +1937,6 @@ static void vmxnet3_net_init(VMXNET3State *s)
>      if (s->peer_has_vhdr) {
>          tap_set_vnet_hdr_len(qemu_get_queue(s->nic)->peer,
>              sizeof(struct virtio_net_hdr));
> -
> -        tap_using_vnet_hdr(qemu_get_queue(s->nic)->peer, 1);
>      }
>  
>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
> diff --git a/include/net/tap.h b/include/net/tap.h
> index a3490a9..54974dc 100644
> --- a/include/net/tap.h
> +++ b/include/net/tap.h
> @@ -32,7 +32,6 @@
>  bool tap_has_ufo(NetClientState *nc);
>  bool tap_has_vnet_hdr(NetClientState *nc);
>  bool tap_has_vnet_hdr_len(NetClientState *nc, int len);
> -void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr);
>  void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int ecn, int ufo);
>  void tap_set_vnet_hdr_len(NetClientState *nc, int len);
>  int tap_enable(NetClientState *nc);
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index edf26c4..f4cd002 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -741,10 +741,6 @@ void tap_fd_set_vnet_hdr_len(int fd, int len)
>  {
>  }
>  
> -void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
> -{
> -}
> -
>  void tap_set_offload(NetClientState *nc, int csum, int tso4,
>                       int tso6, int ecn, int ufo)
>  {
> diff --git a/net/tap.c b/net/tap.c
> index c805f3c..42f768c 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -247,16 +247,7 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int len)
>  
>      tap_fd_set_vnet_hdr_len(s->fd, len);
>      s->host_vnet_hdr_len = len;
> -}
> -
> -void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
> -{
> -    TAPState *s = DO_UPCAST(TAPState, nc, nc);
> -
> -    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
> -    assert(!!s->host_vnet_hdr_len == using_vnet_hdr);
> -
> -    s->using_vnet_hdr = using_vnet_hdr;
> +    s->using_vnet_hdr = true;
>  }
>  
>  void tap_set_offload(NetClientState *nc, int csum, int tso4,
> -- 
> 1.8.5.2
> 
> 

Re: [Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support

[Stefan Hajnoczi]

On Tue, Jan 14, 2014 at 11:59:44AM +0100, Vincenzo Maffione wrote:
> (3) There is actually an important problem. In the previous patch version, TCP/UDP traffic was
>     supported between two guests attached to a VALE switch if and only if both guests use (or
>     don't) the same offloadings: e.g. two virtio-net guests or two e1000 guests. This is why
>     in this version I put the commit which adds the support for netmap as the last one and
>     I temporarily disable the offloading feature (e.g. netmap_has_vnet_hdr() returns false).
>     We are working at adding proper support also in the general case in which there is
>     a mismatch between the NIC offloadings capabilities. As soon as we have proper support,
>     we can enable it in net/netmap.c.
>     What do you think about that? What's the best way to manage this temporary lack of
>     support?

What you described is known problem in QEMU.  You cannot unplug a
vnet_hdr-enabled tap netdev and plug in a non-vnet_hdr socket netdev
since the socket netdev and its remote side don't understand vnet_hdr.
This has stopped us from supporting changing NetClient peers at runtime.

When this issue was raised we figured we'll have to add code to QEMU to
emulate the offload in software (i.e. TSO, checksums, etc).  But no one
has implemented that yet (although vmxnet3 has VMware offload software
emulation IIRC).

So maybe the answer is to finally implement vnet_hdr offload emulation
inside QEMU?  Then netmap can negotiate with its remote side and fall
back to offload emulation if the remote side doesn't understand
vnet_hdr.

Keep in mind that virtio-net does not allow the host to disable an
offload feature that was already enabled, except after the device has
been reset.  This precludes a simple solution where we just tell the
guest to stop using vnet_hdr.

I don't want to merge the tap -> net API changes and netmap offload
enablement until there is a solution to this.

> Vincenzo Maffione (6):
>   net: change vnet-hdr TAP prototypes
>   net: removing tap_using_vnet_hdr() function

Happy with these two patches, we can merge them regardless of the rest
of this series.  Still giving Michael Tsirkin a chance to review since
the special tap vnet_hdr interface is used closely by vhost_net.

Re: [Qemu-devel] [PATCH v2 2/6] net: removing tap_using_vnet_hdr() function

[Michael S. Tsirkin]

On Tue, Jan 14, 2014 at 11:59:46AM +0100, Vincenzo Maffione wrote:
> This function was used to set the using_vnet_hdr field into the
> TAPState struct. However, it is always called immediately before
> (see virtio-net.c) or immediately after (see vmxnet3.c) the function
> tap_set_vnet_hdr_len(). It's therefore possible to set the
> using_vnet_hdr field directly in tap_set_vnet_hdr_len() and remove
> tap_using_vnet_hdr(), making the code simpler.
> 
> Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>


virtio net only calls tap_set_vnet_hdr_len if tap_has_vnet_hdr_len
returns true.
I think this patch will break old linux hosts which don't support
modifying vnet_hdr_len.

Also, the current API is quite ugly, but I don't think adding side
effects to tap_set_vnet_hdr_len is an improvement.

> ---
>  hw/net/virtio-net.c |  4 ----
>  hw/net/vmxnet3.c    |  2 --
>  include/net/tap.h   |  1 -
>  net/tap-win32.c     |  4 ----
>  net/tap.c           | 11 +----------
>  5 files changed, 1 insertion(+), 21 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 3626608..ac8322d 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1496,7 +1496,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VirtIONet *n = VIRTIO_NET(dev);
>      NetClientState *nc;
> -    int i;
>  
>      virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
>  
> @@ -1543,9 +1542,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>  
>      peer_test_vnet_hdr(n);
>      if (peer_has_vnet_hdr(n)) {
> -        for (i = 0; i < n->max_queues; i++) {
> -            tap_using_vnet_hdr(qemu_get_subqueue(n->nic, i)->peer, true);
> -        }
>          n->host_hdr_len = sizeof(struct virtio_net_hdr);
>      } else {
>          n->host_hdr_len = 0;
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 19687aa..096b598 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1937,8 +1937,6 @@ static void vmxnet3_net_init(VMXNET3State *s)
>      if (s->peer_has_vhdr) {
>          tap_set_vnet_hdr_len(qemu_get_queue(s->nic)->peer,
>              sizeof(struct virtio_net_hdr));
> -
> -        tap_using_vnet_hdr(qemu_get_queue(s->nic)->peer, 1);
>      }
>  
>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
> diff --git a/include/net/tap.h b/include/net/tap.h
> index a3490a9..54974dc 100644
> --- a/include/net/tap.h
> +++ b/include/net/tap.h
> @@ -32,7 +32,6 @@
>  bool tap_has_ufo(NetClientState *nc);
>  bool tap_has_vnet_hdr(NetClientState *nc);
>  bool tap_has_vnet_hdr_len(NetClientState *nc, int len);
> -void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr);
>  void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int ecn, int ufo);
>  void tap_set_vnet_hdr_len(NetClientState *nc, int len);
>  int tap_enable(NetClientState *nc);
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index edf26c4..f4cd002 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -741,10 +741,6 @@ void tap_fd_set_vnet_hdr_len(int fd, int len)
>  {
>  }
>  
> -void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
> -{
> -}
> -
>  void tap_set_offload(NetClientState *nc, int csum, int tso4,
>                       int tso6, int ecn, int ufo)
>  {
> diff --git a/net/tap.c b/net/tap.c
> index c805f3c..42f768c 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -247,16 +247,7 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int len)
>  
>      tap_fd_set_vnet_hdr_len(s->fd, len);
>      s->host_vnet_hdr_len = len;
> -}
> -
> -void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
> -{
> -    TAPState *s = DO_UPCAST(TAPState, nc, nc);
> -
> -    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
> -    assert(!!s->host_vnet_hdr_len == using_vnet_hdr);
> -
> -    s->using_vnet_hdr = using_vnet_hdr;
> +    s->using_vnet_hdr = true;
>  }
>  
>  void tap_set_offload(NetClientState *nc, int csum, int tso4,
> -- 
> 1.8.5.2
> 

Re: [Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support

[Vincenzo Maffione]

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

2014/1/16 Stefan Hajnoczi <stefanha@gmail.com>

> On Tue, Jan 14, 2014 at 11:59:44AM +0100, Vincenzo Maffione wrote:
> > (3) There is actually an important problem. In the previous patch
> version, TCP/UDP traffic was
> >     supported between two guests attached to a VALE switch if and only
> if both guests use (or
> >     don't) the same offloadings: e.g. two virtio-net guests or two e1000
> guests. This is why
> >     in this version I put the commit which adds the support for netmap
> as the last one and
> >     I temporarily disable the offloading feature (e.g.
> netmap_has_vnet_hdr() returns false).
> >     We are working at adding proper support also in the general case in
> which there is
> >     a mismatch between the NIC offloadings capabilities. As soon as we
> have proper support,
> >     we can enable it in net/netmap.c.
> >     What do you think about that? What's the best way to manage this
> temporary lack of
> >     support?
>
> What you described is known problem in QEMU.  You cannot unplug a
> vnet_hdr-enabled tap netdev and plug in a non-vnet_hdr socket netdev
> since the socket netdev and its remote side don't understand vnet_hdr.
> This has stopped us from supporting changing NetClient peers at runtime.
>
> When this issue was raised we figured we'll have to add code to QEMU to
> emulate the offload in software (i.e. TSO, checksums, etc).  But no one
> has implemented that yet (although vmxnet3 has VMware offload software
> emulation IIRC).
>
> So maybe the answer is to finally implement vnet_hdr offload emulation
> inside QEMU?  Then netmap can negotiate with its remote side and fall
> back to offload emulation if the remote side doesn't understand
> vnet_hdr.
>
> Keep in mind that virtio-net does not allow the host to disable an
> offload feature that was already enabled, except after the device has
> been reset.  This precludes a simple solution where we just tell the
> guest to stop using vnet_hdr.
>

So what you are saying is that once you turn on an offload feature, it's
not possible to turn it off (apart from device reset).
By the way, let me ask a "sidechannel" question.
It's not clear to me how the virtio-net device decides which features to
enable and which features not. I guess there is a negotiation between the
guest virtio-net driver and the QEMU virtio-net frontend, and this is good.
But if I am not mistaken there is not a negotiation between the virtio-net
frontend and the netdev (the backend), apart from the UFO feature (I see a
tap_has_ufo() function). This means that the virtio-frontend will enable
the feature negotiated with the guest driver regardless of what the backend
is able to do. This is ok as long as only tap provides those offloadings,
but don't you see the need for a more general negotiation also between
netdev and the frontend?e.g. extend tap_has_ufo() to all the other
offloadings?


>
> I don't want to merge the tap -> net API changes and netmap offload
> enablement until there is a solution to this.
>
>
Ok, you pointed out a possible solution, but at the moment I don't think is
easy/convenient to add negotiation support to netmap (for example, in the
VALE switch there are more than two ports, so many possible pairs to keep
track...).

I'm currently working on adding offloading support inside netmap (so, no
QEMU code) and this will be enough to fix the problem for netmap, meaning
that two arbitrary netmap clients will be able to communicate regardless of
wether they understand or not the virtio-net-header, exactly how the tap
(and the associated in-kernel bridge) is currently able to do. Will you
accept the patches provided I complete the offloading support?

As a further step, I could try to convert the offloading code for QEMU, so
that you can solve the socket problem you described, regardless of netmap.

Does this sound good to you?





> > Vincenzo Maffione (6):
> >   net: change vnet-hdr TAP prototypes
> >   net: removing tap_using_vnet_hdr() function
>
> Happy with these two patches, we can merge them regardless of the rest
> of this series.  Still giving Michael Tsirkin a chance to review since
> the special tap vnet_hdr interface is used closely by vhost_net.
>



-- 
Vincenzo Maffione

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

Re: [Qemu-devel] [PATCH v2 0/6] Add netmap backend offloadings support

[Stefan Hajnoczi]

On Thu, Jan 16, 2014 at 04:00:36PM +0100, Vincenzo Maffione wrote:
> 2014/1/16 Stefan Hajnoczi <stefanha@gmail.com>
> 
> > On Tue, Jan 14, 2014 at 11:59:44AM +0100, Vincenzo Maffione wrote:
> > > (3) There is actually an important problem. In the previous patch
> > version, TCP/UDP traffic was
> > >     supported between two guests attached to a VALE switch if and only
> > if both guests use (or
> > >     don't) the same offloadings: e.g. two virtio-net guests or two e1000
> > guests. This is why
> > >     in this version I put the commit which adds the support for netmap
> > as the last one and
> > >     I temporarily disable the offloading feature (e.g.
> > netmap_has_vnet_hdr() returns false).
> > >     We are working at adding proper support also in the general case in
> > which there is
> > >     a mismatch between the NIC offloadings capabilities. As soon as we
> > have proper support,
> > >     we can enable it in net/netmap.c.
> > >     What do you think about that? What's the best way to manage this
> > temporary lack of
> > >     support?
> >
> > What you described is known problem in QEMU.  You cannot unplug a
> > vnet_hdr-enabled tap netdev and plug in a non-vnet_hdr socket netdev
> > since the socket netdev and its remote side don't understand vnet_hdr.
> > This has stopped us from supporting changing NetClient peers at runtime.
> >
> > When this issue was raised we figured we'll have to add code to QEMU to
> > emulate the offload in software (i.e. TSO, checksums, etc).  But no one
> > has implemented that yet (although vmxnet3 has VMware offload software
> > emulation IIRC).
> >
> > So maybe the answer is to finally implement vnet_hdr offload emulation
> > inside QEMU?  Then netmap can negotiate with its remote side and fall
> > back to offload emulation if the remote side doesn't understand
> > vnet_hdr.
> >
> > Keep in mind that virtio-net does not allow the host to disable an
> > offload feature that was already enabled, except after the device has
> > been reset.  This precludes a simple solution where we just tell the
> > guest to stop using vnet_hdr.
> >
> 
> So what you are saying is that once you turn on an offload feature, it's
> not possible to turn it off (apart from device reset).

Yes.

> By the way, let me ask a "sidechannel" question.
> It's not clear to me how the virtio-net device decides which features to
> enable and which features not. I guess there is a negotiation between the
> guest virtio-net driver and the QEMU virtio-net frontend, and this is good.
> But if I am not mistaken there is not a negotiation between the virtio-net
> frontend and the netdev (the backend), apart from the UFO feature (I see a
> tap_has_ufo() function). This means that the virtio-frontend will enable
> the feature negotiated with the guest driver regardless of what the backend
> is able to do. This is ok as long as only tap provides those offloadings,
> but don't you see the need for a more general negotiation also between
> netdev and the frontend?e.g. extend tap_has_ufo() to all the other
> offloadings?

Here is the code:

  if (!peer_has_vnet_hdr(n)) {
      features &= ~(0x1 << VIRTIO_NET_F_CSUM);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO6);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_ECN);

      features &= ~(0x1 << VIRTIO_NET_F_GUEST_CSUM);
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_TSO4);
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_TSO6);
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_ECN);
  }

  if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) {
      features &= ~(0x1 << VIRTIO_NET_F_GUEST_UFO);
      features &= ~(0x1 << VIRTIO_NET_F_HOST_UFO);
  }

It is assumed that most offloads are supported if vnet_hdr is available.
Only UFO is a separate feature on top of vnet_hdr.

> > I don't want to merge the tap -> net API changes and netmap offload
> > enablement until there is a solution to this.
> >
> >
> Ok, you pointed out a possible solution, but at the moment I don't think is
> easy/convenient to add negotiation support to netmap (for example, in the
> VALE switch there are more than two ports, so many possible pairs to keep
> track...).
> 
> I'm currently working on adding offloading support inside netmap (so, no
> QEMU code) and this will be enough to fix the problem for netmap, meaning
> that two arbitrary netmap clients will be able to communicate regardless of
> wether they understand or not the virtio-net-header, exactly how the tap
> (and the associated in-kernel bridge) is currently able to do. Will you
> accept the patches provided I complete the offloading support?

Yes.

> As a further step, I could try to convert the offloading code for QEMU, so
> that you can solve the socket problem you described, regardless of netmap.
> 
> Does this sound good to you?

Yes.

Stefan

Re: [Qemu-devel] [PATCH v2 2/6] net: removing tap_using_vnet_hdr() function

[Stefan Hajnoczi]

On Thu, Jan 16, 2014 at 11:29:34AM +0200, Michael S. Tsirkin wrote:
> On Tue, Jan 14, 2014 at 11:59:46AM +0100, Vincenzo Maffione wrote:
> > This function was used to set the using_vnet_hdr field into the
> > TAPState struct. However, it is always called immediately before
> > (see virtio-net.c) or immediately after (see vmxnet3.c) the function
> > tap_set_vnet_hdr_len(). It's therefore possible to set the
> > using_vnet_hdr field directly in tap_set_vnet_hdr_len() and remove
> > tap_using_vnet_hdr(), making the code simpler.
> > 
> > Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
> 
> 
> virtio net only calls tap_set_vnet_hdr_len if tap_has_vnet_hdr_len
> returns true.
> I think this patch will break old linux hosts which don't support
> modifying vnet_hdr_len.

This is a good point.  Let's leave the API alone then.

Stefan