[Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support

[Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support

[Zhang Chen]

If user use -device virtio-net-pci, virtio-net driver will add a header
to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to compare
the sent packet that primary guest's to secondary guest's.

Zhang Chen (10):
  net: Add vnet_hdr_len related callback in NetClientInfo
  net/tap.c: Add tap_get_vnet_hdr_len and tap_get_using_vnet_hdr
    function
  net/netmap.c: Add netmap_get_vnet_hdr_len function
  net/filter-mirror.c: Add filter-mirror and filter-redirector vnet
    support.
  net/net.c: Add vnet header length to SocketReadState
  tests/e1000e-test.c : change e1000e test case send data format
  tests/virtio-net-test.c : change virtio-net test case iov send data
    format
  net/colo-compare.c: Make colo-compare support vnet_hdr_len
  net/colo.c: Add vnet packet parse feature in colo-proxy
  net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare

 include/net/net.h       | 10 +++++++++-
 net/colo-compare.c      | 48 +++++++++++++++++++++++++++++++++++++++---------
 net/colo.c              |  9 +++++----
 net/colo.h              |  4 +++-
 net/filter-mirror.c     | 28 +++++++++++++++++++++++-----
 net/filter-rewriter.c   |  2 +-
 net/net.c               | 42 ++++++++++++++++++++++++++++++++++++++++--
 net/netmap.c            |  8 ++++++++
 net/tap-win32.c         | 12 ++++++++++++
 net/tap.c               | 20 ++++++++++++++++++++
 tests/e1000e-test.c     | 10 ++++++++--
 tests/virtio-net-test.c | 18 ++++++++++++++----
 12 files changed, 182 insertions(+), 29 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo

[Zhang Chen]

Add get_vnet_hdr_len and get_using_vnet_hdr callback
that make we get vnet_hdr_len easily.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 include/net/net.h |  6 ++++++
 net/net.c         | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 99b28d5..402d913 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -57,7 +57,9 @@ typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
 typedef bool (HasUfo)(NetClientState *);
 typedef bool (HasVnetHdr)(NetClientState *);
 typedef bool (HasVnetHdrLen)(NetClientState *, int);
+typedef int (GetVnetHdrLen)(NetClientState *);
 typedef void (UsingVnetHdr)(NetClientState *, bool);
+typedef bool (GetUsingVnetHdr)(NetClientState *);
 typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
 typedef void (SetVnetHdrLen)(NetClientState *, int);
 typedef int (SetVnetLE)(NetClientState *, bool);
@@ -79,7 +81,9 @@ typedef struct NetClientInfo {
     HasUfo *has_ufo;
     HasVnetHdr *has_vnet_hdr;
     HasVnetHdrLen *has_vnet_hdr_len;
+    GetVnetHdrLen *get_vnet_hdr_len;
     UsingVnetHdr *using_vnet_hdr;
+    GetUsingVnetHdr *get_using_vnet_hdr;
     SetOffload *set_offload;
     SetVnetHdrLen *set_vnet_hdr_len;
     SetVnetLE *set_vnet_le;
@@ -155,7 +159,9 @@ void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
 bool qemu_has_ufo(NetClientState *nc);
 bool qemu_has_vnet_hdr(NetClientState *nc);
 bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
+int qemu_get_vnet_hdr_len(NetClientState *nc);
 void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
+bool qemu_get_using_vnet_hdr(NetClientState *nc);
 void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
                       int ecn, int ufo);
 void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
diff --git a/net/net.c b/net/net.c
index 0ac3b9e..f69260f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -466,6 +466,15 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)
     return nc->info->has_vnet_hdr_len(nc, len);
 }
 
+int qemu_get_vnet_hdr_len(NetClientState *nc)
+{
+    if (!nc || !nc->info->get_vnet_hdr_len) {
+        return false;
+    }
+
+    return nc->info->get_vnet_hdr_len(nc);
+}
+
 void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
 {
     if (!nc || !nc->info->using_vnet_hdr) {
@@ -475,6 +484,15 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
     nc->info->using_vnet_hdr(nc, enable);
 }
 
+bool qemu_get_using_vnet_hdr(NetClientState *nc)
+{
+    if (!nc || !nc->info->get_using_vnet_hdr) {
+        return false;
+    }
+
+    return nc->info->get_using_vnet_hdr(nc);
+}
+
 void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
                           int ecn, int ufo)
 {
-- 
2.7.4

[Qemu-devel] [PATCH V3 02/10] net/tap.c: Add tap_get_vnet_hdr_len and tap_get_using_vnet_hdr function

[Zhang Chen]

Make tap backend support get_vnet_hdr_len.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/tap-win32.c | 12 ++++++++++++
 net/tap.c       | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/net/tap-win32.c b/net/tap-win32.c
index 662f9b6..337e8ea 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -729,6 +729,11 @@ static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
 {
 }
 
+static void tap_get_using_vnet_hdr(NetClientState *nc)
+{
+    return false;
+}
+
 static void tap_set_offload(NetClientState *nc, int csum, int tso4,
                      int tso6, int ecn, int ufo)
 {
@@ -744,6 +749,11 @@ static bool tap_has_vnet_hdr_len(NetClientState *nc, int len)
     return false;
 }
 
+static int tap_get_vnet_hdr_len(NetClientState *nc)
+{
+    return 0;
+}
+
 static void tap_set_vnet_hdr_len(NetClientState *nc, int len)
 {
     abort();
@@ -757,7 +767,9 @@ static NetClientInfo net_tap_win32_info = {
     .has_ufo = tap_has_ufo,
     .has_vnet_hdr = tap_has_vnet_hdr,
     .has_vnet_hdr_len = tap_has_vnet_hdr_len,
+    .get_vnet_hdr_len = tap_get_vnet_hdr_len,
     .using_vnet_hdr = tap_using_vnet_hdr,
+    .get_using_vnet_hdr = tap_get_using_vnet_hdr,
     .set_offload = tap_set_offload,
     .set_vnet_hdr_len = tap_set_vnet_hdr_len,
 };
diff --git a/net/tap.c b/net/tap.c
index 979e622..214c83d 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -246,6 +246,15 @@ static bool tap_has_vnet_hdr_len(NetClientState *nc, int len)
     return !!tap_probe_vnet_hdr_len(s->fd, len);
 }
 
+static int tap_get_vnet_hdr_len(NetClientState *nc)
+{
+    TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
+
+    return s->host_vnet_hdr_len;
+}
+
 static void tap_set_vnet_hdr_len(NetClientState *nc, int len)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
@@ -268,6 +277,15 @@ static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
     s->using_vnet_hdr = using_vnet_hdr;
 }
 
+static bool tap_get_using_vnet_hdr(NetClientState *nc)
+{
+    TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
+
+    return s->using_vnet_hdr;
+}
+
 static int tap_set_vnet_le(NetClientState *nc, bool is_le)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
@@ -354,7 +372,9 @@ static NetClientInfo net_tap_info = {
     .has_ufo = tap_has_ufo,
     .has_vnet_hdr = tap_has_vnet_hdr,
     .has_vnet_hdr_len = tap_has_vnet_hdr_len,
+    .get_vnet_hdr_len = tap_get_vnet_hdr_len,
     .using_vnet_hdr = tap_using_vnet_hdr,
+    .get_using_vnet_hdr = tap_get_using_vnet_hdr,
     .set_offload = tap_set_offload,
     .set_vnet_hdr_len = tap_set_vnet_hdr_len,
     .set_vnet_le = tap_set_vnet_le,
-- 
2.7.4

[Qemu-devel] [PATCH V3 03/10] net/netmap.c: Add netmap_get_vnet_hdr_len function

[Zhang Chen]

Make netmap support get_vnet_hdr_len.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/netmap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/netmap.c b/net/netmap.c
index 2d11a8f..694c340 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -360,6 +360,13 @@ static bool netmap_has_vnet_hdr_len(NetClientState *nc, int len)
     return true;
 }
 
+static int netmap_get_vnet_hdr_len(NetClientState *nc)
+{
+    NetmapState *s = DO_UPCAST(NetmapState, nc, nc);
+
+    return s->vnet_hdr_len;
+}
+
 /* A netmap interface that supports virtio-net headers always
  * supports UFO, so we use this callback also for the has_ufo hook. */
 static bool netmap_has_vnet_hdr(NetClientState *nc)
@@ -409,6 +416,7 @@ static NetClientInfo net_netmap_info = {
     .has_ufo = netmap_has_vnet_hdr,
     .has_vnet_hdr = netmap_has_vnet_hdr,
     .has_vnet_hdr_len = netmap_has_vnet_hdr_len,
+    .get_vnet_hdr_len = netmap_get_vnet_hdr_len,
     .using_vnet_hdr = netmap_using_vnet_hdr,
     .set_offload = netmap_set_offload,
     .set_vnet_hdr_len = netmap_set_vnet_hdr_len,
-- 
2.7.4

[Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Zhang Chen]

In this patch, we change the send packet format from
struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
make other module(like colo-compare) know how to parse net packet correctly.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/filter-mirror.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 72fa7c2..bb9ecf3 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -43,12 +43,14 @@ typedef struct MirrorState {
     SocketReadState rs;
 } MirrorState;
 
-static int filter_mirror_send(CharBackend *chr_out,
+static int filter_mirror_send(MirrorState *s,
                               const struct iovec *iov,
                               int iovcnt)
 {
+    NetFilterState *nf = NETFILTER(s);
     int ret = 0;
     ssize_t size = 0;
+    ssize_t vnet_hdr_len;
     uint32_t len = 0;
     char *buf;
 
@@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend *chr_out,
     }
 
     len = htonl(size);
-    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, sizeof(len));
+    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
+    if (ret != sizeof(len)) {
+        goto err;
+    }
+
+    /*
+     * We send vnet header len make other module(like colo-compare)
+     * know how to parse net packet correctly.
+     */
+    if (qemu_get_using_vnet_hdr(nf->netdev)) {
+        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
+    } else {
+        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
+    }
+
+    len = htonl(vnet_hdr_len);
+    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
     if (ret != sizeof(len)) {
         goto err;
     }
 
     buf = g_malloc(size);
     iov_to_buf(iov, iovcnt, 0, buf, size);
-    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
+    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
     g_free(buf);
     if (ret != size) {
         goto err;
@@ -141,7 +159,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
     MirrorState *s = FILTER_MIRROR(nf);
     int ret;
 
-    ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
+    ret = filter_mirror_send(s, iov, iovcnt);
     if (ret) {
         error_report("filter_mirror_send failed(%s)", strerror(-ret));
     }
@@ -164,7 +182,7 @@ static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
     int ret;
 
     if (qemu_chr_fe_get_driver(&s->chr_out)) {
-        ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
+        ret = filter_mirror_send(s, iov, iovcnt);
         if (ret) {
             error_report("filter_mirror_send failed(%s)", strerror(-ret));
         }
-- 
2.7.4

[Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Zhang Chen]

Address Jason Wang's comments add vnet header length to SocketReadState.
So we change net_fill_rstate() to read
struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 include/net/net.h |  4 +++-
 net/net.c         | 24 ++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 402d913..865cb98 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -115,9 +115,11 @@ typedef struct NICState {
 } NICState;
 
 struct SocketReadState {
-    int state; /* 0 = getting length, 1 = getting data */
+    /* 0 = getting length, 1 = getting vnet header length, 2 = getting data */
+    int state;
     uint32_t index;
     uint32_t packet_len;
+    uint32_t vnet_hdr_len;
     uint8_t buf[NET_BUFSIZE];
     SocketReadStateFinalize *finalize;
 };
diff --git a/net/net.c b/net/net.c
index f69260f..5a6b5ac 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
     unsigned int l;
 
     while (size > 0) {
-        /* reassemble a packet from the network */
-        switch (rs->state) { /* 0 = getting length, 1 = getting data */
+        /* Reassemble a packet from the network.
+         * 0 = getting length.
+         * 1 = getting vnet header length.
+         * 2 = getting data.
+         */
+        switch (rs->state) {
         case 0:
             l = 4 - rs->index;
             if (l > size) {
@@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
             }
             break;
         case 1:
+            l = 4 - rs->index;
+            if (l > size) {
+                l = size;
+            }
+            memcpy(rs->buf + rs->index, buf, l);
+            buf += l;
+            size -= l;
+            rs->index += l;
+            if (rs->index == 4) {
+                /* got vnet header length */
+                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
+                rs->index = 0;
+                rs->state = 2;
+            }
+            break;
+        case 2:
             l = rs->packet_len - rs->index;
             if (l > size) {
                 l = size;
-- 
2.7.4

[Qemu-devel] [PATCH V3 06/10] tests/e1000e-test.c : change e1000e test case send data format

[Zhang Chen]

Because we change net_fill_rstate() date format from
struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
So, we add fake vnet_hdr_len in e1000e test case.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 tests/e1000e-test.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/e1000e-test.c b/tests/e1000e-test.c
index c612dc6..fb3bd77 100644
--- a/tests/e1000e-test.c
+++ b/tests/e1000e-test.c
@@ -329,11 +329,15 @@ static void e1000e_receive_verify(e1000e_device *d)
 
     char test[] = "TEST";
     int len = htonl(sizeof(test));
+    int vnet_hdr_len = 0;
     struct iovec iov[] = {
         {
             .iov_base = &len,
             .iov_len = sizeof(len),
         },{
+            .iov_base = &vnet_hdr_len,
+            .iov_len = sizeof(vnet_hdr_len),
+        },{
             .iov_base = test,
             .iov_len = sizeof(test),
         },
@@ -344,8 +348,10 @@ static void e1000e_receive_verify(e1000e_device *d)
     int ret;
 
     /* Send a dummy packet to device's socket*/
-    ret = iov_send(test_sockets[0], iov, 2, 0, sizeof(len) + sizeof(test));
-    g_assert_cmpint(ret, == , sizeof(test) + sizeof(len));
+    ret = iov_send(test_sockets[0], iov, 3, 0, sizeof(len)
+                   + sizeof(vnet_hdr_len) + sizeof(test));
+    g_assert_cmpint(ret, == , sizeof(test) + sizeof(len)
+                    + sizeof(vnet_hdr_len));
 
     /* Prepare test data buffer */
     uint64_t data = guest_alloc(test_alloc, data_len);
-- 
2.7.4

[Qemu-devel] [PATCH V3 07/10] tests/virtio-net-test.c : change virtio-net test case iov send data format

[Zhang Chen]

Because we change net_fill_rstate() date format from
struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
So, we add fake vnet_hdr_len in virtio-net test case.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 tests/virtio-net-test.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 8f94360..93f4fe0 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -89,11 +89,15 @@ static void rx_test(QVirtioDevice *dev,
     char test[] = "TEST";
     char buffer[64];
     int len = htonl(sizeof(test));
+    int vnet_hdr_len = 0;
     struct iovec iov[] = {
         {
             .iov_base = &len,
             .iov_len = sizeof(len),
         }, {
+            .iov_base = &vnet_hdr_len,
+            .iov_len = sizeof(vnet_hdr_len),
+        }, {
             .iov_base = test,
             .iov_len = sizeof(test),
         },
@@ -105,8 +109,9 @@ static void rx_test(QVirtioDevice *dev,
     free_head = qvirtqueue_add(vq, req_addr, 64, true, false);
     qvirtqueue_kick(dev, vq, free_head);
 
-    ret = iov_send(socket, iov, 2, 0, sizeof(len) + sizeof(test));
-    g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len));
+    ret = iov_send(socket, iov, 3, 0, sizeof(len)
+                   + sizeof(test) + sizeof(vnet_hdr_len));
+    g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len) + sizeof(vnet_hdr_len));
 
     qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
     memread(req_addr + VNET_HDR_SIZE, buffer, sizeof(test));
@@ -151,12 +156,16 @@ static void rx_stop_cont_test(QVirtioDevice *dev,
     char test[] = "TEST";
     char buffer[64];
     int len = htonl(sizeof(test));
+    int vnet_hdr_len = 0;
     QDict *rsp;
     struct iovec iov[] = {
         {
             .iov_base = &len,
             .iov_len = sizeof(len),
         }, {
+            .iov_base = &vnet_hdr_len,
+            .iov_len = sizeof(vnet_hdr_len),
+        }, {
             .iov_base = test,
             .iov_len = sizeof(test),
         },
@@ -171,8 +180,9 @@ static void rx_stop_cont_test(QVirtioDevice *dev,
     rsp = qmp("{ 'execute' : 'stop'}");
     QDECREF(rsp);
 
-    ret = iov_send(socket, iov, 2, 0, sizeof(len) + sizeof(test));
-    g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len));
+    ret = iov_send(socket, iov, 3, 0, sizeof(len)
+                   + sizeof(test) + sizeof(vnet_hdr_len));
+    g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len) + sizeof(vnet_hdr_len));
 
     /* We could check the status, but this command is more importantly to
      * ensure the packet data gets queued in QEMU, before we do 'cont'.
-- 
2.7.4

[Qemu-devel] [PATCH V3 08/10] net/colo-compare.c: Make colo-compare support vnet_hdr_len

[Zhang Chen]

COLO-compare can get vnet header length from filter,
Add vnet_hdr_len to struct packet and output packet with
the vnet_hdr_len.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c    | 39 ++++++++++++++++++++++++++++++++-------
 net/colo.c            |  3 ++-
 net/colo.h            |  4 +++-
 net/filter-rewriter.c |  2 +-
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 54e6d40..b3e933c 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -99,7 +99,8 @@ enum {
 
 static int compare_chr_send(CharBackend *out,
                             const uint8_t *buf,
-                            uint32_t size);
+                            uint32_t size,
+                            uint32_t vnet_hdr_len);
 
 static gint seq_sorter(Packet *a, Packet *b, gpointer data)
 {
@@ -121,9 +122,13 @@ static int packet_enqueue(CompareState *s, int mode)
     Connection *conn;
 
     if (mode == PRIMARY_IN) {
-        pkt = packet_new(s->pri_rs.buf, s->pri_rs.packet_len);
+        pkt = packet_new(s->pri_rs.buf,
+                         s->pri_rs.packet_len,
+                         s->pri_rs.vnet_hdr_len);
     } else {
-        pkt = packet_new(s->sec_rs.buf, s->sec_rs.packet_len);
+        pkt = packet_new(s->sec_rs.buf,
+                         s->sec_rs.packet_len,
+                         s->sec_rs.vnet_hdr_len);
     }
 
     if (parse_packet_early(pkt)) {
@@ -436,7 +441,10 @@ static void colo_compare_connection(void *opaque, void *user_data)
         }
 
         if (result) {
-            ret = compare_chr_send(&s->chr_out, pkt->data, pkt->size);
+            ret = compare_chr_send(&s->chr_out,
+                                   pkt->data,
+                                   pkt->size,
+                                   pkt->vnet_hdr_len);
             if (ret < 0) {
                 error_report("colo_send_primary_packet failed");
             }
@@ -459,7 +467,8 @@ static void colo_compare_connection(void *opaque, void *user_data)
 
 static int compare_chr_send(CharBackend *out,
                             const uint8_t *buf,
-                            uint32_t size)
+                            uint32_t size,
+                            uint32_t vnet_hdr_len)
 {
     int ret = 0;
     uint32_t len = htonl(size);
@@ -473,6 +482,16 @@ static int compare_chr_send(CharBackend *out,
         goto err;
     }
 
+    /*
+     * We send vnet header len make other module(like colo-compare)
+     * know how to parse net packet correctly.
+     */
+    len = htonl(vnet_hdr_len);
+    ret = qemu_chr_fe_write_all(out, (uint8_t *)&len, sizeof(len));
+    if (ret != sizeof(len)) {
+        goto err;
+    }
+
     ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size);
     if (ret != size) {
         goto err;
@@ -616,7 +635,10 @@ static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 
     if (packet_enqueue(s, PRIMARY_IN)) {
         trace_colo_compare_main("primary: unsupported packet in");
-        compare_chr_send(&s->chr_out, pri_rs->buf, pri_rs->packet_len);
+        compare_chr_send(&s->chr_out,
+                         pri_rs->buf,
+                         pri_rs->packet_len,
+                         pri_rs->vnet_hdr_len);
     } else {
         /* compare connection */
         g_queue_foreach(&s->conn_list, colo_compare_connection, s);
@@ -725,7 +747,10 @@ static void colo_flush_packets(void *opaque, void *user_data)
 
     while (!g_queue_is_empty(&conn->primary_list)) {
         pkt = g_queue_pop_head(&conn->primary_list);
-        compare_chr_send(&s->chr_out, pkt->data, pkt->size);
+        compare_chr_send(&s->chr_out,
+                         pkt->data,
+                         pkt->size,
+                         pkt->vnet_hdr_len);
         packet_destroy(pkt, NULL);
     }
     while (!g_queue_is_empty(&conn->secondary_list)) {
diff --git a/net/colo.c b/net/colo.c
index 8cc166b..180eaed 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -153,13 +153,14 @@ void connection_destroy(void *opaque)
     g_slice_free(Connection, conn);
 }
 
-Packet *packet_new(const void *data, int size)
+Packet *packet_new(const void *data, int size, int vnet_hdr_len)
 {
     Packet *pkt = g_slice_new(Packet);
 
     pkt->data = g_memdup(data, size);
     pkt->size = size;
     pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST);
+    pkt->vnet_hdr_len = vnet_hdr_len;
 
     return pkt;
 }
diff --git a/net/colo.h b/net/colo.h
index 7c524f3..caedb0d 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -43,6 +43,8 @@ typedef struct Packet {
     int size;
     /* Time of packet creation, in wall clock ms */
     int64_t creation_ms;
+    /* Get vnet_hdr_len from filter */
+    uint32_t vnet_hdr_len;
 } Packet;
 
 typedef struct ConnectionKey {
@@ -82,7 +84,7 @@ Connection *connection_get(GHashTable *connection_track_table,
                            ConnectionKey *key,
                            GQueue *conn_list);
 void connection_hashtable_reset(GHashTable *connection_track_table);
-Packet *packet_new(const void *data, int size);
+Packet *packet_new(const void *data, int size, int vnet_hdr_len);
 void packet_destroy(void *opaque, void *user_data);
 
 #endif /* QEMU_COLO_PROXY_H */
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index afa06e8..63256c7 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -158,7 +158,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
     char *buf = g_malloc0(size);
 
     iov_to_buf(iov, iovcnt, 0, buf, size);
-    pkt = packet_new(buf, size);
+    pkt = packet_new(buf, size, 0);
     g_free(buf);
 
     /*
-- 
2.7.4

[Qemu-devel] [PATCH V3 09/10] net/colo.c: Add vnet packet parse feature in colo-proxy

[Zhang Chen]

Make colo-compare and filter-rewriter can parse vnet packet.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/colo.c b/net/colo.c
index 180eaed..28ce7c8 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -43,11 +43,11 @@ int parse_packet_early(Packet *pkt)
 {
     int network_length;
     static const uint8_t vlan[] = {0x81, 0x00};
-    uint8_t *data = pkt->data;
+    uint8_t *data = pkt->data + pkt->vnet_hdr_len;
     uint16_t l3_proto;
     ssize_t l2hdr_len = eth_get_l2_hdr_length(data);
 
-    if (pkt->size < ETH_HLEN) {
+    if (pkt->size < ETH_HLEN + pkt->vnet_hdr_len) {
         trace_colo_proxy_main("pkt->size < ETH_HLEN");
         return 1;
     }
@@ -73,7 +73,7 @@ int parse_packet_early(Packet *pkt)
     }
 
     network_length = pkt->ip->ip_hl * 4;
-    if (pkt->size < l2hdr_len + network_length) {
+    if (pkt->size < l2hdr_len + network_length + pkt->vnet_hdr_len) {
         trace_colo_proxy_main("pkt->size < network_header + network_length");
         return 1;
     }
-- 
2.7.4

[Qemu-devel] [PATCH V3 10/10] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare

[Zhang Chen]

COLO-Proxy just focus on packet payload, So we skip vnet header.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index b3e933c..1941ad9 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -187,6 +187,8 @@ static int packet_enqueue(CompareState *s, int mode)
  */
 static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
 {
+    int offset_all;
+
     if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
         char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
 
@@ -200,9 +202,12 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
                                    sec_ip_src, sec_ip_dst);
     }
 
+    offset_all = ppkt->vnet_hdr_len + offset;
+
     if (ppkt->size == spkt->size) {
-        return memcmp(ppkt->data + offset, spkt->data + offset,
-                      spkt->size - offset);
+        return memcmp(ppkt->data + offset_all,
+                      spkt->data + offset_all,
+                      spkt->size - offset_all);
     } else {
         trace_colo_compare_main("Net packet size are not the same");
         return -1;
-- 
2.7.4

Re: [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo

[Jason Wang]

On 2017年04月28日 17:47, Zhang Chen wrote:
> Add get_vnet_hdr_len and get_using_vnet_hdr callback
> that make we get vnet_hdr_len easily.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   include/net/net.h |  6 ++++++
>   net/net.c         | 18 ++++++++++++++++++
>   2 files changed, 24 insertions(+)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 99b28d5..402d913 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -57,7 +57,9 @@ typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
>   typedef bool (HasUfo)(NetClientState *);
>   typedef bool (HasVnetHdr)(NetClientState *);
>   typedef bool (HasVnetHdrLen)(NetClientState *, int);
> +typedef int (GetVnetHdrLen)(NetClientState *);
>   typedef void (UsingVnetHdr)(NetClientState *, bool);
> +typedef bool (GetUsingVnetHdr)(NetClientState *);
>   typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
>   typedef void (SetVnetHdrLen)(NetClientState *, int);
>   typedef int (SetVnetLE)(NetClientState *, bool);
> @@ -79,7 +81,9 @@ typedef struct NetClientInfo {
>       HasUfo *has_ufo;
>       HasVnetHdr *has_vnet_hdr;
>       HasVnetHdrLen *has_vnet_hdr_len;
> +    GetVnetHdrLen *get_vnet_hdr_len;
>       UsingVnetHdr *using_vnet_hdr;
> +    GetUsingVnetHdr *get_using_vnet_hdr;
>       SetOffload *set_offload;
>       SetVnetHdrLen *set_vnet_hdr_len;
>       SetVnetLE *set_vnet_le;
> @@ -155,7 +159,9 @@ void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
>   bool qemu_has_ufo(NetClientState *nc);
>   bool qemu_has_vnet_hdr(NetClientState *nc);
>   bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
> +int qemu_get_vnet_hdr_len(NetClientState *nc);
>   void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
> +bool qemu_get_using_vnet_hdr(NetClientState *nc);
>   void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
>                         int ecn, int ufo);
>   void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
> diff --git a/net/net.c b/net/net.c
> index 0ac3b9e..f69260f 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -466,6 +466,15 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)
>       return nc->info->has_vnet_hdr_len(nc, len);
>   }
>   
> +int qemu_get_vnet_hdr_len(NetClientState *nc)
> +{
> +    if (!nc || !nc->info->get_vnet_hdr_len) {
> +        return false;
> +    }
> +
> +    return nc->info->get_vnet_hdr_len(nc);
> +}
> +
>   void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
>   {
>       if (!nc || !nc->info->using_vnet_hdr) {
> @@ -475,6 +484,15 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
>       nc->info->using_vnet_hdr(nc, enable);
>   }
>   
> +bool qemu_get_using_vnet_hdr(NetClientState *nc)
> +{
> +    if (!nc || !nc->info->get_using_vnet_hdr) {
> +        return false;
> +    }
> +
> +    return nc->info->get_using_vnet_hdr(nc);
> +}

Looks like we can do this simply by:

Introduce two common fields in NetClientState:

bool using_vnet_hdr;
int vnet_hdr_len;

And set them during qemu_using_vnet_hdr() and qemu_set_vnet_hdr_len(). 
Then we can query them directly without introducing any new callbacks.

Thanks

> +
>   void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
>                             int ecn, int ufo)
>   {

Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Jason Wang]

On 2017年04月28日 17:47, Zhang Chen wrote:
> In this patch, we change the send packet format from
> struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
> make other module(like colo-compare) know how to parse net packet correctly.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>   1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 72fa7c2..bb9ecf3 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>       SocketReadState rs;
>   } MirrorState;
>   
> -static int filter_mirror_send(CharBackend *chr_out,
> +static int filter_mirror_send(MirrorState *s,
>                                 const struct iovec *iov,
>                                 int iovcnt)
>   {
> +    NetFilterState *nf = NETFILTER(s);
>       int ret = 0;
>       ssize_t size = 0;
> +    ssize_t vnet_hdr_len;
>       uint32_t len = 0;
>       char *buf;
>   
> @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend *chr_out,
>       }
>   
>       len = htonl(size);
> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, sizeof(len));
> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
> +    if (ret != sizeof(len)) {
> +        goto err;
> +    }
> +
> +    /*
> +     * We send vnet header len make other module(like colo-compare)
> +     * know how to parse net packet correctly.
> +     */
> +    if (qemu_get_using_vnet_hdr(nf->netdev)) {
> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
> +    } else {
> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
> +    }

Any reason to query peer here?

> +
> +    len = htonl(vnet_hdr_len);
> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
>       if (ret != sizeof(len)) {
>           goto err;
>       }
>   
>       buf = g_malloc(size);
>       iov_to_buf(iov, iovcnt, 0, buf, size);
> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
>       g_free(buf);
>       if (ret != size) {
>           goto err;
> @@ -141,7 +159,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>       MirrorState *s = FILTER_MIRROR(nf);
>       int ret;
>   
> -    ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
> +    ret = filter_mirror_send(s, iov, iovcnt);
>       if (ret) {
>           error_report("filter_mirror_send failed(%s)", strerror(-ret));
>       }
> @@ -164,7 +182,7 @@ static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
>       int ret;
>   
>       if (qemu_chr_fe_get_driver(&s->chr_out)) {
> -        ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
> +        ret = filter_mirror_send(s, iov, iovcnt);
>           if (ret) {
>               error_report("filter_mirror_send failed(%s)", strerror(-ret));
>           }

Do we need to strip vnet_hdr_len in redirector_to_filter() ?

Thanks

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Jason Wang]

On 2017年04月28日 17:47, Zhang Chen wrote:
> Address Jason Wang's comments add vnet header length to SocketReadState.

Instead of saying this, you can add "Suggested-by: Jason Wang 
<jasowang@redhat.com>" above your sign-off.

> So we change net_fill_rstate() to read
> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.

This makes me thinking about the backward compatibility. I think we'd 
better try to keep it as much as possible. E.g how about pack 
vnet_hdr_len into higher bits of size?

Thanks

>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   include/net/net.h |  4 +++-
>   net/net.c         | 24 ++++++++++++++++++++++--
>   2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 402d913..865cb98 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -115,9 +115,11 @@ typedef struct NICState {
>   } NICState;
>   
>   struct SocketReadState {
> -    int state; /* 0 = getting length, 1 = getting data */
> +    /* 0 = getting length, 1 = getting vnet header length, 2 = getting data */
> +    int state;
>       uint32_t index;
>       uint32_t packet_len;
> +    uint32_t vnet_hdr_len;
>       uint8_t buf[NET_BUFSIZE];
>       SocketReadStateFinalize *finalize;
>   };
> diff --git a/net/net.c b/net/net.c
> index f69260f..5a6b5ac 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
>       unsigned int l;
>   
>       while (size > 0) {
> -        /* reassemble a packet from the network */
> -        switch (rs->state) { /* 0 = getting length, 1 = getting data */
> +        /* Reassemble a packet from the network.
> +         * 0 = getting length.
> +         * 1 = getting vnet header length.
> +         * 2 = getting data.
> +         */
> +        switch (rs->state) {
>           case 0:
>               l = 4 - rs->index;
>               if (l > size) {
> @@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
>               }
>               break;
>           case 1:
> +            l = 4 - rs->index;
> +            if (l > size) {
> +                l = size;
> +            }
> +            memcpy(rs->buf + rs->index, buf, l);
> +            buf += l;
> +            size -= l;
> +            rs->index += l;
> +            if (rs->index == 4) {
> +                /* got vnet header length */
> +                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
> +                rs->index = 0;
> +                rs->state = 2;
> +            }
> +            break;
> +        case 2:
>               l = rs->packet_len - rs->index;
>               if (l > size) {
>                   l = size;

Re: [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo

[Zhang Chen]

On 05/02/2017 12:46 PM, Jason Wang wrote:
>
>
> On 2017年04月28日 17:47, Zhang Chen wrote:
>> Add get_vnet_hdr_len and get_using_vnet_hdr callback
>> that make we get vnet_hdr_len easily.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   include/net/net.h |  6 ++++++
>>   net/net.c         | 18 ++++++++++++++++++
>>   2 files changed, 24 insertions(+)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 99b28d5..402d913 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -57,7 +57,9 @@ typedef RxFilterInfo 
>> *(QueryRxFilter)(NetClientState *);
>>   typedef bool (HasUfo)(NetClientState *);
>>   typedef bool (HasVnetHdr)(NetClientState *);
>>   typedef bool (HasVnetHdrLen)(NetClientState *, int);
>> +typedef int (GetVnetHdrLen)(NetClientState *);
>>   typedef void (UsingVnetHdr)(NetClientState *, bool);
>> +typedef bool (GetUsingVnetHdr)(NetClientState *);
>>   typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
>>   typedef void (SetVnetHdrLen)(NetClientState *, int);
>>   typedef int (SetVnetLE)(NetClientState *, bool);
>> @@ -79,7 +81,9 @@ typedef struct NetClientInfo {
>>       HasUfo *has_ufo;
>>       HasVnetHdr *has_vnet_hdr;
>>       HasVnetHdrLen *has_vnet_hdr_len;
>> +    GetVnetHdrLen *get_vnet_hdr_len;
>>       UsingVnetHdr *using_vnet_hdr;
>> +    GetUsingVnetHdr *get_using_vnet_hdr;
>>       SetOffload *set_offload;
>>       SetVnetHdrLen *set_vnet_hdr_len;
>>       SetVnetLE *set_vnet_le;
>> @@ -155,7 +159,9 @@ void qemu_format_nic_info_str(NetClientState *nc, 
>> uint8_t macaddr[6]);
>>   bool qemu_has_ufo(NetClientState *nc);
>>   bool qemu_has_vnet_hdr(NetClientState *nc);
>>   bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
>> +int qemu_get_vnet_hdr_len(NetClientState *nc);
>>   void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
>> +bool qemu_get_using_vnet_hdr(NetClientState *nc);
>>   void qemu_set_offload(NetClientState *nc, int csum, int tso4, int 
>> tso6,
>>                         int ecn, int ufo);
>>   void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
>> diff --git a/net/net.c b/net/net.c
>> index 0ac3b9e..f69260f 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -466,6 +466,15 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, 
>> int len)
>>       return nc->info->has_vnet_hdr_len(nc, len);
>>   }
>>   +int qemu_get_vnet_hdr_len(NetClientState *nc)
>> +{
>> +    if (!nc || !nc->info->get_vnet_hdr_len) {
>> +        return false;
>> +    }
>> +
>> +    return nc->info->get_vnet_hdr_len(nc);
>> +}
>> +
>>   void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
>>   {
>>       if (!nc || !nc->info->using_vnet_hdr) {
>> @@ -475,6 +484,15 @@ void qemu_using_vnet_hdr(NetClientState *nc, 
>> bool enable)
>>       nc->info->using_vnet_hdr(nc, enable);
>>   }
>>   +bool qemu_get_using_vnet_hdr(NetClientState *nc)
>> +{
>> +    if (!nc || !nc->info->get_using_vnet_hdr) {
>> +        return false;
>> +    }
>> +
>> +    return nc->info->get_using_vnet_hdr(nc);
>> +}
>
> Looks like we can do this simply by:
>
> Introduce two common fields in NetClientState:
>
> bool using_vnet_hdr;
> int vnet_hdr_len;
>
> And set them during qemu_using_vnet_hdr() and qemu_set_vnet_hdr_len(). 
> Then we can query them directly without introducing any new callbacks.

Good idea~
I will implement in next version.

Thanks
Zhang Chen

>
> Thanks
>
>> +
>>   void qemu_set_offload(NetClientState *nc, int csum, int tso4, int 
>> tso6,
>>                             int ecn, int ufo)
>>   {
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Zhang Chen]

On 05/02/2017 12:47 PM, Jason Wang wrote:
>
>
> On 2017年04月28日 17:47, Zhang Chen wrote:
>> In this patch, we change the send packet format from
>> struct {int size; const uint8_t buf[];} to {int size; int 
>> vnet_hdr_len; const uint8_t buf[];}.
>> make other module(like colo-compare) know how to parse net packet 
>> correctly.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>   1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 72fa7c2..bb9ecf3 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>       SocketReadState rs;
>>   } MirrorState;
>>   -static int filter_mirror_send(CharBackend *chr_out,
>> +static int filter_mirror_send(MirrorState *s,
>>                                 const struct iovec *iov,
>>                                 int iovcnt)
>>   {
>> +    NetFilterState *nf = NETFILTER(s);
>>       int ret = 0;
>>       ssize_t size = 0;
>> +    ssize_t vnet_hdr_len;
>>       uint32_t len = 0;
>>       char *buf;
>>   @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend 
>> *chr_out,
>>       }
>>         len = htonl(size);
>> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, sizeof(len));
>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
>> sizeof(len));
>> +    if (ret != sizeof(len)) {
>> +        goto err;
>> +    }
>> +
>> +    /*
>> +     * We send vnet header len make other module(like colo-compare)
>> +     * know how to parse net packet correctly.
>> +     */
>> +    if (qemu_get_using_vnet_hdr(nf->netdev)) {
>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>> +    } else {
>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>> +    }
>
> Any reason to query peer here?

That's depend on using NetClientState, If we using nf->netdev that need 
to query,
Otherwise we query nf->netdev->peer, then we can get the real 
vnet_hdr_len in my test.

Thanks
Zhang Chen

>
>> +
>> +    len = htonl(vnet_hdr_len);
>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
>> sizeof(len));
>>       if (ret != sizeof(len)) {
>>           goto err;
>>       }
>>         buf = g_malloc(size);
>>       iov_to_buf(iov, iovcnt, 0, buf, size);
>> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
>>       g_free(buf);
>>       if (ret != size) {
>>           goto err;
>> @@ -141,7 +159,7 @@ static ssize_t 
>> filter_mirror_receive_iov(NetFilterState *nf,
>>       MirrorState *s = FILTER_MIRROR(nf);
>>       int ret;
>>   -    ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
>> +    ret = filter_mirror_send(s, iov, iovcnt);
>>       if (ret) {
>>           error_report("filter_mirror_send failed(%s)", strerror(-ret));
>>       }
>> @@ -164,7 +182,7 @@ static ssize_t 
>> filter_redirector_receive_iov(NetFilterState *nf,
>>       int ret;
>>         if (qemu_chr_fe_get_driver(&s->chr_out)) {
>> -        ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
>> +        ret = filter_mirror_send(s, iov, iovcnt);
>>           if (ret) {
>>               error_report("filter_mirror_send failed(%s)", 
>> strerror(-ret));
>>           }
>
> Do we need to strip vnet_hdr_len in redirector_to_filter() ?
>
> Thanks
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Zhang Chen]

On 05/02/2017 12:53 PM, Jason Wang wrote:
>
>
> On 2017年04月28日 17:47, Zhang Chen wrote:
>> Address Jason Wang's comments add vnet header length to SocketReadState.
>
> Instead of saying this, you can add "Suggested-by: Jason Wang 
> <jasowang@redhat.com>" above your sign-off.

OK.

>
>> So we change net_fill_rstate() to read
>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>
> This makes me thinking about the backward compatibility. I think we'd 
> better try to keep it as much as possible. E.g how about pack 
> vnet_hdr_len into higher bits of size?
>

Do you means split uint32_t size to uint16_t size and uint16_t 
vnet_hdr_len ?
If yes, we also can't keep compatibility with old version.
Old code send a uint32_t size, we read it as uint16_t size is always wrong.

Thanks
Zhang Chen


> Thanks
>
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   include/net/net.h |  4 +++-
>>   net/net.c         | 24 ++++++++++++++++++++++--
>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 402d913..865cb98 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -115,9 +115,11 @@ typedef struct NICState {
>>   } NICState;
>>     struct SocketReadState {
>> -    int state; /* 0 = getting length, 1 = getting data */
>> +    /* 0 = getting length, 1 = getting vnet header length, 2 = 
>> getting data */
>> +    int state;
>>       uint32_t index;
>>       uint32_t packet_len;
>> +    uint32_t vnet_hdr_len;
>>       uint8_t buf[NET_BUFSIZE];
>>       SocketReadStateFinalize *finalize;
>>   };
>> diff --git a/net/net.c b/net/net.c
>> index f69260f..5a6b5ac 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const 
>> uint8_t *buf, int size)
>>       unsigned int l;
>>         while (size > 0) {
>> -        /* reassemble a packet from the network */
>> -        switch (rs->state) { /* 0 = getting length, 1 = getting data */
>> +        /* Reassemble a packet from the network.
>> +         * 0 = getting length.
>> +         * 1 = getting vnet header length.
>> +         * 2 = getting data.
>> +         */
>> +        switch (rs->state) {
>>           case 0:
>>               l = 4 - rs->index;
>>               if (l > size) {
>> @@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const 
>> uint8_t *buf, int size)
>>               }
>>               break;
>>           case 1:
>> +            l = 4 - rs->index;
>> +            if (l > size) {
>> +                l = size;
>> +            }
>> +            memcpy(rs->buf + rs->index, buf, l);
>> +            buf += l;
>> +            size -= l;
>> +            rs->index += l;
>> +            if (rs->index == 4) {
>> +                /* got vnet header length */
>> +                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
>> +                rs->index = 0;
>> +                rs->state = 2;
>> +            }
>> +            break;
>> +        case 2:
>>               l = rs->packet_len - rs->index;
>>               if (l > size) {
>>                   l = size;
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Jason Wang]

On 2017年05月03日 11:18, Zhang Chen wrote:
>
>
> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>
>>
>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>> In this patch, we change the send packet format from
>>> struct {int size; const uint8_t buf[];} to {int size; int 
>>> vnet_hdr_len; const uint8_t buf[];}.
>>> make other module(like colo-compare) know how to parse net packet 
>>> correctly.
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> ---
>>>   net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>>   1 file changed, 23 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>> index 72fa7c2..bb9ecf3 100644
>>> --- a/net/filter-mirror.c
>>> +++ b/net/filter-mirror.c
>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>>       SocketReadState rs;
>>>   } MirrorState;
>>>   -static int filter_mirror_send(CharBackend *chr_out,
>>> +static int filter_mirror_send(MirrorState *s,
>>>                                 const struct iovec *iov,
>>>                                 int iovcnt)
>>>   {
>>> +    NetFilterState *nf = NETFILTER(s);
>>>       int ret = 0;
>>>       ssize_t size = 0;
>>> +    ssize_t vnet_hdr_len;
>>>       uint32_t len = 0;
>>>       char *buf;
>>>   @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend 
>>> *chr_out,
>>>       }
>>>         len = htonl(size);
>>> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, 
>>> sizeof(len));
>>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
>>> sizeof(len));
>>> +    if (ret != sizeof(len)) {
>>> +        goto err;
>>> +    }
>>> +
>>> +    /*
>>> +     * We send vnet header len make other module(like colo-compare)
>>> +     * know how to parse net packet correctly.
>>> +     */
>>> +    if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>> +    } else {
>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>> +    }
>>
>> Any reason to query peer here?
>
> That's depend on using NetClientState, If we using nf->netdev that 
> need to query,
> Otherwise we query nf->netdev->peer, then we can get the real 
> vnet_hdr_len in my test.
>
> Thanks
> Zhang Chen 

Confused, I think nf->netdev won't be a nic?

Thanks

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Jason Wang]

On 2017年05月03日 11:43, Zhang Chen wrote:
>
>
> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>
>>
>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>> Address Jason Wang's comments add vnet header length to 
>>> SocketReadState.
>>
>> Instead of saying this, you can add "Suggested-by: Jason Wang 
>> <jasowang@redhat.com>" above your sign-off.
>
> OK.
>
>>
>>> So we change net_fill_rstate() to read
>>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>
>> This makes me thinking about the backward compatibility. I think we'd 
>> better try to keep it as much as possible. E.g how about pack 
>> vnet_hdr_len into higher bits of size?
>>
>
> Do you means split uint32_t size to uint16_t size and uint16_t 
> vnet_hdr_len ?
> If yes, we also can't keep compatibility with old version.
> Old code send a uint32_t size, we read it as uint16_t size is always 
> wrong.
>
> Thanks
> Zhang Chen

Consider it's unlikely to send or receive packet >= 64K, we can reuse 
higher bits (e.g highest 8 bits). Then we can still read uint32_t and 
then check its highest 8 bits. If it was zero, we know vnet header is 
zero, if not it could be read as vnet header length.

Thanks

Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Zhang Chen]

On 05/03/2017 06:19 PM, Jason Wang wrote:
>
>
> On 2017年05月03日 11:18, Zhang Chen wrote:
>>
>>
>> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>> In this patch, we change the send packet format from
>>>> struct {int size; const uint8_t buf[];} to {int size; int 
>>>> vnet_hdr_len; const uint8_t buf[];}.
>>>> make other module(like colo-compare) know how to parse net packet 
>>>> correctly.
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> ---
>>>>   net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>>>   1 file changed, 23 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>> index 72fa7c2..bb9ecf3 100644
>>>> --- a/net/filter-mirror.c
>>>> +++ b/net/filter-mirror.c
>>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>>>       SocketReadState rs;
>>>>   } MirrorState;
>>>>   -static int filter_mirror_send(CharBackend *chr_out,
>>>> +static int filter_mirror_send(MirrorState *s,
>>>>                                 const struct iovec *iov,
>>>>                                 int iovcnt)
>>>>   {
>>>> +    NetFilterState *nf = NETFILTER(s);
>>>>       int ret = 0;
>>>>       ssize_t size = 0;
>>>> +    ssize_t vnet_hdr_len;
>>>>       uint32_t len = 0;
>>>>       char *buf;
>>>>   @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend 
>>>> *chr_out,
>>>>       }
>>>>         len = htonl(size);
>>>> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, 
>>>> sizeof(len));
>>>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
>>>> sizeof(len));
>>>> +    if (ret != sizeof(len)) {
>>>> +        goto err;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * We send vnet header len make other module(like colo-compare)
>>>> +     * know how to parse net packet correctly.
>>>> +     */
>>>> +    if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>>> +    } else {
>>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>>> +    }
>>>
>>> Any reason to query peer here?
>>
>> That's depend on using NetClientState, If we using nf->netdev that 
>> need to query,
>> Otherwise we query nf->netdev->peer, then we can get the real 
>> vnet_hdr_len in my test.
>>
>> Thanks
>> Zhang Chen 
>
> Confused, I think nf->netdev won't be a nic?

I don't know whether I fully understand.
I think it's depend on the sender, we must query sender to get real 
vnet_hdr_len ,
like that in filter.c:

         if (sender == nf->netdev) {
             /* This packet is sent by netdev itself */
             direction = NET_FILTER_DIRECTION_TX;
         } else {
             direction = NET_FILTER_DIRECTION_RX;
         }

Thanks
Zhang Chen


>
> Thanks
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Jason Wang]

On 2017年05月05日 16:44, Zhang Chen wrote:
>
>
> On 05/03/2017 06:19 PM, Jason Wang wrote:
>>
>>
>> On 2017年05月03日 11:18, Zhang Chen wrote:
>>>
>>>
>>> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>> In this patch, we change the send packet format from
>>>>> struct {int size; const uint8_t buf[];} to {int size; int 
>>>>> vnet_hdr_len; const uint8_t buf[];}.
>>>>> make other module(like colo-compare) know how to parse net packet 
>>>>> correctly.
>>>>>
>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>> ---
>>>>>   net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>>>>   1 file changed, 23 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>>> index 72fa7c2..bb9ecf3 100644
>>>>> --- a/net/filter-mirror.c
>>>>> +++ b/net/filter-mirror.c
>>>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>>>>       SocketReadState rs;
>>>>>   } MirrorState;
>>>>>   -static int filter_mirror_send(CharBackend *chr_out,
>>>>> +static int filter_mirror_send(MirrorState *s,
>>>>>                                 const struct iovec *iov,
>>>>>                                 int iovcnt)
>>>>>   {
>>>>> +    NetFilterState *nf = NETFILTER(s);
>>>>>       int ret = 0;
>>>>>       ssize_t size = 0;
>>>>> +    ssize_t vnet_hdr_len;
>>>>>       uint32_t len = 0;
>>>>>       char *buf;
>>>>>   @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend 
>>>>> *chr_out,
>>>>>       }
>>>>>         len = htonl(size);
>>>>> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, 
>>>>> sizeof(len));
>>>>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
>>>>> sizeof(len));
>>>>> +    if (ret != sizeof(len)) {
>>>>> +        goto err;
>>>>> +    }
>>>>> +
>>>>> +    /*
>>>>> +     * We send vnet header len make other module(like colo-compare)
>>>>> +     * know how to parse net packet correctly.
>>>>> +     */
>>>>> +    if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>>>> +    } else {
>>>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>>>> +    }
>>>>
>>>> Any reason to query peer here?
>>>
>>> That's depend on using NetClientState, If we using nf->netdev that 
>>> need to query,
>>> Otherwise we query nf->netdev->peer, then we can get the real 
>>> vnet_hdr_len in my test.
>>>
>>> Thanks
>>> Zhang Chen 
>>
>> Confused, I think nf->netdev won't be a nic?
>
> I don't know whether I fully understand.
> I think it's depend on the sender, we must query sender to get real 
> vnet_hdr_len ,
> like that in filter.c:
>
>         if (sender == nf->netdev) {
>             /* This packet is sent by netdev itself */
>             direction = NET_FILTER_DIRECTION_TX;
>         } else {
>             direction = NET_FILTER_DIRECTION_RX;
>         }
>
> Thanks
> Zhang Chen

The problem is nf->netdev->peer should be a nic. But we don't care about 
its vnet_hdr_len. Take virtio-net as an example, we only care about 
host_hdr_len, since guest will strip the part that host does not care.

Thanks

>
>
>>
>> Thanks
>>
>>
>> .
>>
>

Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Zhang Chen]

On 05/05/2017 05:25 PM, Jason Wang wrote:
>
>
> On 2017年05月05日 16:44, Zhang Chen wrote:
>>
>>
>> On 05/03/2017 06:19 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月03日 11:18, Zhang Chen wrote:
>>>>
>>>>
>>>> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>> In this patch, we change the send packet format from
>>>>>> struct {int size; const uint8_t buf[];} to {int size; int 
>>>>>> vnet_hdr_len; const uint8_t buf[];}.
>>>>>> make other module(like colo-compare) know how to parse net packet 
>>>>>> correctly.
>>>>>>
>>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>>   net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>>>>>   1 file changed, 23 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>>>> index 72fa7c2..bb9ecf3 100644
>>>>>> --- a/net/filter-mirror.c
>>>>>> +++ b/net/filter-mirror.c
>>>>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>>>>>       SocketReadState rs;
>>>>>>   } MirrorState;
>>>>>>   -static int filter_mirror_send(CharBackend *chr_out,
>>>>>> +static int filter_mirror_send(MirrorState *s,
>>>>>>                                 const struct iovec *iov,
>>>>>>                                 int iovcnt)
>>>>>>   {
>>>>>> +    NetFilterState *nf = NETFILTER(s);
>>>>>>       int ret = 0;
>>>>>>       ssize_t size = 0;
>>>>>> +    ssize_t vnet_hdr_len;
>>>>>>       uint32_t len = 0;
>>>>>>       char *buf;
>>>>>>   @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend 
>>>>>> *chr_out,
>>>>>>       }
>>>>>>         len = htonl(size);
>>>>>> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, 
>>>>>> sizeof(len));
>>>>>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
>>>>>> sizeof(len));
>>>>>> +    if (ret != sizeof(len)) {
>>>>>> +        goto err;
>>>>>> +    }
>>>>>> +
>>>>>> +    /*
>>>>>> +     * We send vnet header len make other module(like colo-compare)
>>>>>> +     * know how to parse net packet correctly.
>>>>>> +     */
>>>>>> +    if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>>>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>>>>> +    } else {
>>>>>> +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>>>>> +    }
>>>>>
>>>>> Any reason to query peer here?
>>>>
>>>> That's depend on using NetClientState, If we using nf->netdev that 
>>>> need to query,
>>>> Otherwise we query nf->netdev->peer, then we can get the real 
>>>> vnet_hdr_len in my test.
>>>>
>>>> Thanks
>>>> Zhang Chen 
>>>
>>> Confused, I think nf->netdev won't be a nic?
>>
>> I don't know whether I fully understand.
>> I think it's depend on the sender, we must query sender to get real 
>> vnet_hdr_len ,
>> like that in filter.c:
>>
>>         if (sender == nf->netdev) {
>>             /* This packet is sent by netdev itself */
>>             direction = NET_FILTER_DIRECTION_TX;
>>         } else {
>>             direction = NET_FILTER_DIRECTION_RX;
>>         }
>>
>> Thanks
>> Zhang Chen
>
> The problem is nf->netdev->peer should be a nic. But we don't care 
> about its vnet_hdr_len. Take virtio-net as an example, we only care 
> about host_hdr_len, since guest will strip the part that host does not 
> care.

Yes, you are right. In anytime, nf->netdev and nf->netdev->peer both 
have a vnet_hdr_len,
Here we just find out which is we need. When filter set RX or TX that 
the real vnet_hdr_len are different.
We need query different netdev to get in my test.

Thanks
Zhang Chen

>
> Thanks
>
>>
>>
>>>
>>> Thanks
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Zhang Chen]

On 05/03/2017 06:42 PM, Jason Wang wrote:
>
>
> On 2017年05月03日 11:43, Zhang Chen wrote:
>>
>>
>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>> Address Jason Wang's comments add vnet header length to 
>>>> SocketReadState.
>>>
>>> Instead of saying this, you can add "Suggested-by: Jason Wang 
>>> <jasowang@redhat.com>" above your sign-off.
>>
>> OK.
>>
>>>
>>>> So we change net_fill_rstate() to read
>>>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>
>>> This makes me thinking about the backward compatibility. I think 
>>> we'd better try to keep it as much as possible. E.g how about pack 
>>> vnet_hdr_len into higher bits of size?
>>>
>>
>> Do you means split uint32_t size to uint16_t size and uint16_t 
>> vnet_hdr_len ?
>> If yes, we also can't keep compatibility with old version.
>> Old code send a uint32_t size, we read it as uint16_t size is always 
>> wrong.
>>
>> Thanks
>> Zhang Chen
>
> Consider it's unlikely to send or receive packet >= 64K, we can reuse 
> higher bits (e.g highest 8 bits). Then we can still read uint32_t and 
> then check its highest 8 bits. If it was zero, we know vnet header is 
> zero, if not it could be read as vnet header length.

I got your point, but in this way, packet size must < 64K, if size >=64K 
we still can't maintain the backward compatibility,
For the packet sender that didn't know the potential packet size limit, 
I think we should make code explicitly declared like
currently code. Otherwise we will make other people confused and make 
code difficult to maintain.

Thanks
Zhang Chen


>
> Thanks
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Jason Wang]

On 2017年05月08日 11:47, Zhang Chen wrote:
>
>
> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>
>>
>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>
>>>
>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>> Address Jason Wang's comments add vnet header length to 
>>>>> SocketReadState.
>>>>
>>>> Instead of saying this, you can add "Suggested-by: Jason Wang 
>>>> <jasowang@redhat.com>" above your sign-off.
>>>
>>> OK.
>>>
>>>>
>>>>> So we change net_fill_rstate() to read
>>>>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>
>>>> This makes me thinking about the backward compatibility. I think 
>>>> we'd better try to keep it as much as possible. E.g how about pack 
>>>> vnet_hdr_len into higher bits of size?
>>>>
>>>
>>> Do you means split uint32_t size to uint16_t size and uint16_t 
>>> vnet_hdr_len ?
>>> If yes, we also can't keep compatibility with old version.
>>> Old code send a uint32_t size, we read it as uint16_t size is always 
>>> wrong.
>>>
>>> Thanks
>>> Zhang Chen
>>
>> Consider it's unlikely to send or receive packet >= 64K, we can reuse 
>> higher bits (e.g highest 8 bits). Then we can still read uint32_t and 
>> then check its highest 8 bits. If it was zero, we know vnet header is 
>> zero, if not it could be read as vnet header length.
>
> I got your point, but in this way, packet size must < 64K, if size 
> >=64K we still can't maintain the backward compatibility,
> For the packet sender that didn't know the potential packet size 
> limit, I think we should make code explicitly declared like
> currently code. Otherwise we will make other people confused and make 
> code difficult to maintain.
>
> Thanks
> Zhang Chen
>

Yes, this is an possible issue in the future. Rethink about this, what 
if we just compare vnet header? Is there any issue of doing this? 
(Sorry, I remember this is a reason, but forget now).

Thanks

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Zhang Chen]

On 05/09/2017 10:40 AM, Jason Wang wrote:
>
>
> On 2017年05月08日 11:47, Zhang Chen wrote:
>>
>>
>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>
>>>>
>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>> Address Jason Wang's comments add vnet header length to 
>>>>>> SocketReadState.
>>>>>
>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang 
>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>
>>>> OK.
>>>>
>>>>>
>>>>>> So we change net_fill_rstate() to read
>>>>>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>
>>>>> This makes me thinking about the backward compatibility. I think 
>>>>> we'd better try to keep it as much as possible. E.g how about pack 
>>>>> vnet_hdr_len into higher bits of size?
>>>>>
>>>>
>>>> Do you means split uint32_t size to uint16_t size and uint16_t 
>>>> vnet_hdr_len ?
>>>> If yes, we also can't keep compatibility with old version.
>>>> Old code send a uint32_t size, we read it as uint16_t size is 
>>>> always wrong.
>>>>
>>>> Thanks
>>>> Zhang Chen
>>>
>>> Consider it's unlikely to send or receive packet >= 64K, we can 
>>> reuse higher bits (e.g highest 8 bits). Then we can still read 
>>> uint32_t and then check its highest 8 bits. If it was zero, we know 
>>> vnet header is zero, if not it could be read as vnet header length.
>>
>> I got your point, but in this way, packet size must < 64K, if size 
>> >=64K we still can't maintain the backward compatibility,
>> For the packet sender that didn't know the potential packet size 
>> limit, I think we should make code explicitly declared like
>> currently code. Otherwise we will make other people confused and make 
>> code difficult to maintain.
>>
>> Thanks
>> Zhang Chen
>>
>
> Yes, this is an possible issue in the future. Rethink about this, what 
> if we just compare vnet header? Is there any issue of doing this? 
> (Sorry, I remember this is a reason, but forget now).

Yes, we can compare all packet with vnet header, the compare performance 
is very low. but we can't parse raw packet to tcp/udp/icmp packet, 
because we didn't know the vnet_hdr_len as the offset.

Thanks
Zhang Chen

>
> Thanks
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Jason Wang]

On 2017年05月09日 12:03, Zhang Chen wrote:
>
>
> On 05/09/2017 10:40 AM, Jason Wang wrote:
>>
>>
>> On 2017年05月08日 11:47, Zhang Chen wrote:
>>>
>>>
>>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>>
>>>>>
>>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>>
>>>>>>
>>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>>> Address Jason Wang's comments add vnet header length to 
>>>>>>> SocketReadState.
>>>>>>
>>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang 
>>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>>
>>>>> OK.
>>>>>
>>>>>>
>>>>>>> So we change net_fill_rstate() to read
>>>>>>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>>
>>>>>> This makes me thinking about the backward compatibility. I think 
>>>>>> we'd better try to keep it as much as possible. E.g how about 
>>>>>> pack vnet_hdr_len into higher bits of size?
>>>>>>
>>>>>
>>>>> Do you means split uint32_t size to uint16_t size and uint16_t 
>>>>> vnet_hdr_len ?
>>>>> If yes, we also can't keep compatibility with old version.
>>>>> Old code send a uint32_t size, we read it as uint16_t size is 
>>>>> always wrong.
>>>>>
>>>>> Thanks
>>>>> Zhang Chen
>>>>
>>>> Consider it's unlikely to send or receive packet >= 64K, we can 
>>>> reuse higher bits (e.g highest 8 bits). Then we can still read 
>>>> uint32_t and then check its highest 8 bits. If it was zero, we know 
>>>> vnet header is zero, if not it could be read as vnet header length.
>>>
>>> I got your point, but in this way, packet size must < 64K, if size 
>>> >=64K we still can't maintain the backward compatibility,
>>> For the packet sender that didn't know the potential packet size 
>>> limit, I think we should make code explicitly declared like
>>> currently code. Otherwise we will make other people confused and 
>>> make code difficult to maintain.
>>>
>>> Thanks
>>> Zhang Chen
>>>
>>
>> Yes, this is an possible issue in the future. Rethink about this, 
>> what if we just compare vnet header? Is there any issue of doing 
>> this? (Sorry, I remember this is a reason, but forget now).
>
> Yes, we can compare all packet with vnet header, the compare 
> performance is very low. but we can't parse raw packet to tcp/udp/icmp 
> packet, because we didn't know the vnet_hdr_len as the offset.
>
> Thanks
> Zhang Chen

Aha, so I think it's time to use the new format but:

- probably need a new option to enable this support for filter
- let's don't touch socket backend, and make it still can work with 
filters whose vnet_hder is off

Thanks

>
>>
>> Thanks
>>
>>
>> .
>>
>

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Zhang Chen]

On 05/09/2017 01:36 PM, Jason Wang wrote:
>
>
> On 2017年05月09日 12:03, Zhang Chen wrote:
>>
>>
>> On 05/09/2017 10:40 AM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月08日 11:47, Zhang Chen wrote:
>>>>
>>>>
>>>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>>>
>>>>>>
>>>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>>>> Address Jason Wang's comments add vnet header length to 
>>>>>>>> SocketReadState.
>>>>>>>
>>>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang 
>>>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>>>
>>>>>>>> So we change net_fill_rstate() to read
>>>>>>>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>>>
>>>>>>> This makes me thinking about the backward compatibility. I think 
>>>>>>> we'd better try to keep it as much as possible. E.g how about 
>>>>>>> pack vnet_hdr_len into higher bits of size?
>>>>>>>
>>>>>>
>>>>>> Do you means split uint32_t size to uint16_t size and uint16_t 
>>>>>> vnet_hdr_len ?
>>>>>> If yes, we also can't keep compatibility with old version.
>>>>>> Old code send a uint32_t size, we read it as uint16_t size is 
>>>>>> always wrong.
>>>>>>
>>>>>> Thanks
>>>>>> Zhang Chen
>>>>>
>>>>> Consider it's unlikely to send or receive packet >= 64K, we can 
>>>>> reuse higher bits (e.g highest 8 bits). Then we can still read 
>>>>> uint32_t and then check its highest 8 bits. If it was zero, we 
>>>>> know vnet header is zero, if not it could be read as vnet header 
>>>>> length.
>>>>
>>>> I got your point, but in this way, packet size must < 64K, if size 
>>>> >=64K we still can't maintain the backward compatibility,
>>>> For the packet sender that didn't know the potential packet size 
>>>> limit, I think we should make code explicitly declared like
>>>> currently code. Otherwise we will make other people confused and 
>>>> make code difficult to maintain.
>>>>
>>>> Thanks
>>>> Zhang Chen
>>>>
>>>
>>> Yes, this is an possible issue in the future. Rethink about this, 
>>> what if we just compare vnet header? Is there any issue of doing 
>>> this? (Sorry, I remember this is a reason, but forget now).
>>
>> Yes, we can compare all packet with vnet header, the compare 
>> performance is very low. but we can't parse raw packet to 
>> tcp/udp/icmp packet, because we didn't know the vnet_hdr_len as the 
>> offset.
>>
>> Thanks
>> Zhang Chen
>
> Aha, so I think it's time to use the new format but:
>
> - probably need a new option to enable this support for filter

Do you means we should add the new option for every filter object like that:
Now:
-object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
Feature:
-object 
filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,has_vnet_hdr_len=on

And colo-compare also need this option to get the vnet_hdr_len.


> - let's don't touch socket backend, and make it still can work with 
> filters whose vnet_hder is off

OK, Maybe we can add a new variable in net_fill_rstate().

Thanks
Zhang Chen

>
> Thanks
>
>>
>>>
>>> Thanks
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Jason Wang]

On 2017年05月09日 14:59, Zhang Chen wrote:
>
>
> On 05/09/2017 01:36 PM, Jason Wang wrote:
>>
>>
>> On 2017年05月09日 12:03, Zhang Chen wrote:
>>>
>>>
>>> On 05/09/2017 10:40 AM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年05月08日 11:47, Zhang Chen wrote:
>>>>>
>>>>>
>>>>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>>>>
>>>>>>
>>>>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>>>>> Address Jason Wang's comments add vnet header length to 
>>>>>>>>> SocketReadState.
>>>>>>>>
>>>>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang 
>>>>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>>>>
>>>>>>> OK.
>>>>>>>
>>>>>>>>
>>>>>>>>> So we change net_fill_rstate() to read
>>>>>>>>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>>>>
>>>>>>>> This makes me thinking about the backward compatibility. I 
>>>>>>>> think we'd better try to keep it as much as possible. E.g how 
>>>>>>>> about pack vnet_hdr_len into higher bits of size?
>>>>>>>>
>>>>>>>
>>>>>>> Do you means split uint32_t size to uint16_t size and uint16_t 
>>>>>>> vnet_hdr_len ?
>>>>>>> If yes, we also can't keep compatibility with old version.
>>>>>>> Old code send a uint32_t size, we read it as uint16_t size is 
>>>>>>> always wrong.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Zhang Chen
>>>>>>
>>>>>> Consider it's unlikely to send or receive packet >= 64K, we can 
>>>>>> reuse higher bits (e.g highest 8 bits). Then we can still read 
>>>>>> uint32_t and then check its highest 8 bits. If it was zero, we 
>>>>>> know vnet header is zero, if not it could be read as vnet header 
>>>>>> length.
>>>>>
>>>>> I got your point, but in this way, packet size must < 64K, if size 
>>>>> >=64K we still can't maintain the backward compatibility,
>>>>> For the packet sender that didn't know the potential packet size 
>>>>> limit, I think we should make code explicitly declared like
>>>>> currently code. Otherwise we will make other people confused and 
>>>>> make code difficult to maintain.
>>>>>
>>>>> Thanks
>>>>> Zhang Chen
>>>>>
>>>>
>>>> Yes, this is an possible issue in the future. Rethink about this, 
>>>> what if we just compare vnet header? Is there any issue of doing 
>>>> this? (Sorry, I remember this is a reason, but forget now).
>>>
>>> Yes, we can compare all packet with vnet header, the compare 
>>> performance is very low. but we can't parse raw packet to 
>>> tcp/udp/icmp packet, because we didn't know the vnet_hdr_len as the 
>>> offset.
>>>
>>> Thanks
>>> Zhang Chen
>>
>> Aha, so I think it's time to use the new format but:
>>
>> - probably need a new option to enable this support for filter
>
> Do you means we should add the new option for every filter object like 
> that:
> Now:
> -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
> Feature:
> -object 
> filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,has_vnet_hdr_len=on
>
> And colo-compare also need this option to get the vnet_hdr_len.

Yes, and you can use make it short like "vnet_hdr".

>
>
>> - let's don't touch socket backend, and make it still can work with 
>> filters whose vnet_hder is off
>
> OK, Maybe we can add a new variable in net_fill_rstate().

Right.

Thanks

>
> Thanks
> Zhang Chen
>
>>
>> Thanks
>>
>>>
>>>>
>>>> Thanks
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>>
>>
>> .
>>
>