[PATCH V6 00/10] Add COLO-proxy virtio-net support

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 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.

V6:
 - p1: Remove the using_vnet_hdr, I will send a independent
       patchset about it.
 - p2: Change option input way from vnet_hdr=on/off to vnet_hdr_support.
       Use nf->direction to decide check nf->netdev->vnet_hdr_len or nf->netdev->peer->vnet_hdr_len.
 - p3: Change option input way from vnet_hdr=on/off to vnet_hdr_support.
 - p4: No change.
 - p5: No change.
 - p6: Change option input way from vnet_hdr=on/off to vnet_hdr_support.
       Fix commit log.
 - p7: No change.
 - p8: No change.
 - p9: Change option input way from vnet_hdr=on/off to vnet_hdr_support.
       Use nf->direction to decide check nf->netdev->vnet_hdr_len or nf->netdev->peer->vnet_hdr_len.
 - p10: New patch to add new example for the case needs vnet hdr.

V5:
 - patch1: No change.
 - patch2: Keep the long line in qemu-option.hx.
           Squash patch2 into old patch3.
           Add more comments.
           Fix the return value.
 - patch3: Add more comments in commit log.
 - patch4: Add Suggested-by tag.
           Fix commit log.
           Move vnet_hdr to SocketReadState.
 - patch5: No change.
 - patch6: Squash old patch6 into this patch.
 - patch7: No change.
 - patch8: Remove the offset_all.
 - patch9: Squash old patch11 and the patch12.


V4:
 - Add vnet_hdr option for filter-mirror, filter-redirector,
   filter-rewriter,colo-compare.
 - Use new design to impliment virtio-net support for colo-proxy.
 - Fix codestyle.
 - Remove unused option for filter-rewriter.
 - Add filter-rewriter virtio-net support.
 - Address other comments.


Zhang Chen (10):
  net: Add vnet_hdr_len arguments in NetClientState
  net/filter-mirror.c: Make filter mirror support vnet support.
  net/filter-mirror.c: Add new option to enable vnet support for
    filter-redirector
  net/net.c: Add vnet_hdr support in SocketReadState
  net/colo.c: Make vnet_hdr_len as packet property
  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
  net/filter-rewriter.c: Make filter-rewriter support vnet_hdr_len
  docs/colo-proxy.txt: Update colo-proxy usage of net driver with
    vnet_header

 docs/colo-proxy.txt   | 26 ++++++++++++++
 include/net/net.h     |  7 +++-
 net/colo-compare.c    | 86 +++++++++++++++++++++++++++++++++++++-------
 net/colo.c            |  9 ++---
 net/colo.h            |  4 ++-
 net/filter-mirror.c   | 98 ++++++++++++++++++++++++++++++++++++++++++++++++---
 net/filter-rewriter.c | 51 ++++++++++++++++++++++++++-
 net/net.c             | 34 ++++++++++++++++--
 qemu-options.hx       | 19 +++++-----
 9 files changed, 296 insertions(+), 38 deletions(-)

--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 01/10] net: Add vnet_hdr_len arguments in NetClientState

Zhang Chen
Add vnet_hdr_len arguments in NetClientState
that make other module get real vnet_hdr_len easily.

Signed-off-by: Zhang Chen <[hidden email]>
---
 include/net/net.h | 1 +
 net/net.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 99b28d5..9a92c70 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -100,6 +100,7 @@ struct NetClientState {
     unsigned int queue_index;
     unsigned rxfilter_notify_enabled:1;
     int vring_enable;
+    int vnet_hdr_len;
     QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
 };
 
diff --git a/net/net.c b/net/net.c
index 0ac3b9e..4e7a305 100644
--- a/net/net.c
+++ b/net/net.c
@@ -491,6 +491,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
         return;
     }
 
+    nc->vnet_hdr_len = len;
     nc->info->set_vnet_hdr_len(nc, len);
 }
 
--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 02/10] net/filter-mirror.c: Make filter mirror support vnet support.

Zhang Chen
In reply to this post by Zhang Chen
We add the vnet_hdr_support option for filter-mirror, default is disable.
If you use virtio-net-pci net driver, please enable it.
You can use it for example:
-object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr_support

If have vnet_hdr_support flag, we will 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 <[hidden email]>
---
 net/filter-mirror.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++----
 qemu-options.hx     |  5 ++--
 2 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 72fa7c2..50aa81b 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -38,15 +38,17 @@ typedef struct MirrorState {
     NetFilterState parent_obj;
     char *indev;
     char *outdev;
+    bool vnet_hdr;
     CharBackend chr_in;
     CharBackend chr_out;
     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;
     uint32_t len = 0;
@@ -58,14 +60,43 @@ 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;
     }
 
+    if (s->vnet_hdr) {
+        /*
+         * If vnet_hdr = on, we send vnet header len to make other
+         * module(like colo-compare) know how to parse net
+         * packet correctly.
+         */
+        ssize_t vnet_hdr_len;
+
+        /*
+         * 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.
+         */
+        if (nf->direction == NET_FILTER_DIRECTION_RX ||
+            nf->direction == NET_FILTER_DIRECTION_ALL) {
+            vnet_hdr_len = nf->netdev->vnet_hdr_len;
+        } else if (nf->direction == NET_FILTER_DIRECTION_TX) {
+            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;
+        } else {
+            return 0;
+        }
+
+        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 +172,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 +195,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));
         }
@@ -308,6 +339,13 @@ static char *filter_mirror_get_outdev(Object *obj, Error **errp)
     return g_strdup(s->outdev);
 }
 
+static bool filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
+{
+    MirrorState *s = FILTER_MIRROR(obj);
+
+    return s->vnet_hdr;
+}
+
 static void
 filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
 {
@@ -322,6 +360,15 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
     }
 }
 
+static void filter_mirror_set_vnet_hdr(Object *obj,
+                                       bool value,
+                                       Error **errp)
+{
+    MirrorState *s = FILTER_MIRROR(obj);
+
+    s->vnet_hdr = value;
+}
+
 static char *filter_redirector_get_outdev(Object *obj, Error **errp)
 {
     MirrorState *s = FILTER_REDIRECTOR(obj);
@@ -340,8 +387,20 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
 
 static void filter_mirror_init(Object *obj)
 {
+    MirrorState *s = FILTER_MIRROR(obj);
+
     object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
                             filter_mirror_set_outdev, NULL);
+
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_bool(obj, "vnet_hdr_support",
+                             filter_mirror_get_vnet_hdr,
+                             filter_mirror_set_vnet_hdr, NULL);
 }
 
 static void filter_redirector_init(Object *obj)
diff --git a/qemu-options.hx b/qemu-options.hx
index 70c0ded..5c09fae 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4024,10 +4024,9 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
 @option{tx}: the filter is attached to the transmit queue of the netdev,
              where it will receive packets sent by the netdev.
 
-@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
+@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
 
-filter-mirror on netdev @var{netdevid},mirror net packet to chardev
-@var{chardevid}
+filter-mirror on netdev @var{netdevid},mirror net packet to chardev@var{chardevid}, if have the vnet_hdr_support flag, filter-mirror will mirror packet with vnet_hdr_len.
 
 @item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
 outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 03/10] net/filter-mirror.c: Add new option to enable vnet support for filter-redirector

Zhang Chen
In reply to this post by Zhang Chen
We add the vnet_hdr_support option for filter-redirector, default is disable.
If you use virtio-net-pci net driver, please enable it.
Because colo-compare or other modules needs the vnet_hdr_len to parse
packet, so we add this new option send the len to others.
You can use it for example:
-object filter-redirector,id=r0,netdev=hn0,queue=tx,outdev=red0,vnet_hdr_support

Signed-off-by: Zhang Chen <[hidden email]>
---
 net/filter-mirror.c | 28 ++++++++++++++++++++++++++++
 qemu-options.hx     |  6 +++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 50aa81b..3413e82 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -376,6 +376,13 @@ static char *filter_redirector_get_outdev(Object *obj, Error **errp)
     return g_strdup(s->outdev);
 }
 
+static bool filter_redirector_get_vnet_hdr(Object *obj, Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    return s->vnet_hdr;
+}
+
 static void
 filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
 {
@@ -385,6 +392,15 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
     s->outdev = g_strdup(value);
 }
 
+static void filter_redirector_set_vnet_hdr(Object *obj,
+                                           bool value,
+                                           Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    s->vnet_hdr = value;
+}
+
 static void filter_mirror_init(Object *obj)
 {
     MirrorState *s = FILTER_MIRROR(obj);
@@ -405,10 +421,22 @@ static void filter_mirror_init(Object *obj)
 
 static void filter_redirector_init(Object *obj)
 {
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
     object_property_add_str(obj, "indev", filter_redirector_get_indev,
                             filter_redirector_set_indev, NULL);
     object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
                             filter_redirector_set_outdev, NULL);
+
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_bool(obj, "vnet_hdr_support",
+                             filter_redirector_get_vnet_hdr,
+                             filter_redirector_set_vnet_hdr, NULL);
 }
 
 static void filter_mirror_fini(Object *obj)
diff --git a/qemu-options.hx b/qemu-options.hx
index 5c09fae..e78b942 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4028,11 +4028,11 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
 
 filter-mirror on netdev @var{netdevid},mirror net packet to chardev@var{chardevid}, if have the vnet_hdr_support flag, filter-mirror will mirror packet with vnet_hdr_len.
 
-@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
-outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
+@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
 
 filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
-@var{chardevid},and redirect indev's packet to filter.
+@var{chardevid},and redirect indev's packet to filter.if have the vnet_hdr_support flag,
+filter-redirector will redirect packet with vnet_hdr_len.
 Create a filter-redirector we need to differ outdev id from indev id, id can not
 be the same. we can just use indev or outdev, but at least one of indev or outdev
 need to be specified.
--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 04/10] net/net.c: Add vnet_hdr support in SocketReadState

Zhang Chen
In reply to this post by Zhang Chen
We add a flag to dicide whether net_fill_rstate() to read
the vnet_hdr_len or not.

Signed-off-by: Zhang Chen <[hidden email]>
Suggested-by: Jason Wang <[hidden email]>
---
 include/net/net.h   |  6 +++++-
 net/filter-mirror.c |  1 +
 net/net.c           | 33 ++++++++++++++++++++++++++++++---
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 9a92c70..b2167ae 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -112,9 +112,13 @@ 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;
+    /* This flag decide whether to read the vnet_hdr_len field */
+    bool vnet_hdr;
     uint32_t index;
     uint32_t packet_len;
+    uint32_t vnet_hdr_len;
     uint8_t buf[NET_BUFSIZE];
     SocketReadStateFinalize *finalize;
 };
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 3413e82..4b03dda 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -267,6 +267,7 @@ static void filter_redirector_setup(NetFilterState *nf, Error **errp)
     }
 
     net_socket_rs_init(&s->rs, redirector_rs_finalize);
+    s->rs.vnet_hdr = s->vnet_hdr;
 
     if (s->indev) {
         chr = qemu_chr_find(s->indev);
diff --git a/net/net.c b/net/net.c
index 4e7a305..b9b90c9 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1606,8 +1606,10 @@ void net_socket_rs_init(SocketReadState *rs,
                         SocketReadStateFinalize *finalize)
 {
     rs->state = 0;
+    rs->vnet_hdr = false;
     rs->index = 0;
     rs->packet_len = 0;
+    rs->vnet_hdr_len = 0;
     memset(rs->buf, 0, sizeof(rs->buf));
     rs->finalize = finalize;
 }
@@ -1622,8 +1624,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) {
@@ -1637,10 +1643,31 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
                 /* got length */
                 rs->packet_len = ntohl(*(uint32_t *)rs->buf);
                 rs->index = 0;
-                rs->state = 1;
+                if (rs->vnet_hdr) {
+                    rs->state = 1;
+                } else {
+                    rs->state = 2;
+                    rs->vnet_hdr_len = 0;
+                }
             }
             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




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 05/10] net/colo.c: Make vnet_hdr_len as packet property

Zhang Chen
In reply to this post by Zhang Chen
We can use this property flush and send packet with vnet_hdr_len.

Signed-off-by: Zhang Chen <[hidden email]>
---
 net/colo-compare.c    | 8 ++++++--
 net/colo.c            | 3 ++-
 net/colo.h            | 4 +++-
 net/filter-rewriter.c | 2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 4ab80b1..bf0b856 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -121,9 +121,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)) {
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




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 06/10] net/colo-compare.c: Make colo-compare support vnet_hdr_len

Zhang Chen
In reply to this post by Zhang Chen
We add the vnet_hdr_support option for colo-compare, default is disable.
If you use virtio-net-pci or other driver needs vnet_hdr, please enable it.
You can use it for example:
-object colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr_support

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 <[hidden email]>
---
 net/colo-compare.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++-------
 qemu-options.hx    |  4 ++--
 2 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index bf0b856..e33cf7e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -73,6 +73,7 @@ typedef struct CompareState {
     CharBackend chr_out;
     SocketReadState pri_rs;
     SocketReadState sec_rs;
+    bool vnet_hdr;
 
     /* connection list: the connections belonged to this NIC could be found
      * in this list.
@@ -97,9 +98,10 @@ enum {
     SECONDARY_IN,
 };
 
-static int compare_chr_send(CharBackend *out,
+static int compare_chr_send(CompareState *s,
                             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)
 {
@@ -472,7 +474,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,
+                                   pkt->data,
+                                   pkt->size,
+                                   pkt->vnet_hdr_len);
             if (ret < 0) {
                 error_report("colo_send_primary_packet failed");
             }
@@ -493,9 +498,10 @@ static void colo_compare_connection(void *opaque, void *user_data)
     }
 }
 
-static int compare_chr_send(CharBackend *out,
+static int compare_chr_send(CompareState *s,
                             const uint8_t *buf,
-                            uint32_t size)
+                            uint32_t size,
+                            uint32_t vnet_hdr_len)
 {
     int ret = 0;
     uint32_t len = htonl(size);
@@ -504,12 +510,24 @@ static int compare_chr_send(CharBackend *out,
         return 0;
     }
 
-    ret = qemu_chr_fe_write_all(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;
     }
 
-    ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size);
+    if (s->vnet_hdr) {
+        /*
+         * We send vnet header len make other module(like filter-redirector)
+         * know how to parse net packet correctly.
+         */
+        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;
+        }
+    }
+
+    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
     if (ret != size) {
         goto err;
     }
@@ -646,13 +664,32 @@ static void compare_set_outdev(Object *obj, const char *value, Error **errp)
     s->outdev = g_strdup(value);
 }
 
+static bool compare_get_vnet_hdr(Object *obj, Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+
+    return s->vnet_hdr;
+}
+
+static void compare_set_vnet_hdr(Object *obj,
+                                 bool value,
+                                 Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+
+    s->vnet_hdr = value;
+}
+
 static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 {
     CompareState *s = container_of(pri_rs, CompareState, 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,
+                         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);
@@ -735,7 +772,9 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
     }
 
     net_socket_rs_init(&s->pri_rs, compare_pri_rs_finalize);
+    s->pri_rs.vnet_hdr = s->vnet_hdr;
     net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize);
+    s->sec_rs.vnet_hdr = s->vnet_hdr;
 
     g_queue_init(&s->conn_list);
 
@@ -761,7 +800,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,
+                         pkt->data,
+                         pkt->size,
+                         pkt->vnet_hdr_len);
         packet_destroy(pkt, NULL);
     }
     while (!g_queue_is_empty(&conn->secondary_list)) {
@@ -779,6 +821,8 @@ static void colo_compare_class_init(ObjectClass *oc, void *data)
 
 static void colo_compare_init(Object *obj)
 {
+    CompareState *s = COLO_COMPARE(obj);
+
     object_property_add_str(obj, "primary_in",
                             compare_get_pri_indev, compare_set_pri_indev,
                             NULL);
@@ -788,6 +832,14 @@ static void colo_compare_init(Object *obj)
     object_property_add_str(obj, "outdev",
                             compare_get_outdev, compare_set_outdev,
                             NULL);
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,
+                             compare_set_vnet_hdr, NULL);
 }
 
 static void colo_compare_finalize(Object *obj)
diff --git a/qemu-options.hx b/qemu-options.hx
index e78b942..fbfd604 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4057,13 +4057,13 @@ Dump the network traffic on netdev @var{dev} to the file specified by
 The file format is libpcap, so it can be analyzed with tools such as tcpdump
 or Wireshark.
 
-@item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},
-outdev=@var{chardevid}
+@item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid}[,vnet_hdr_support]
 
 Colo-compare gets packet from primary_in@var{chardevid} and secondary_in@var{chardevid}, than compare primary packet with
 secondary packet. If the packets are same, we will output primary
 packet to outdev@var{chardevid}, else we will notify colo-frame
 do checkpoint and send primary packet to outdev@var{chardevid}.
+if have the vnet_hdr_support flag, colo compare will send/recv packet with vnet_hdr_len.
 
 we must use it with the help of filter-mirror and filter-redirector.
 
--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 07/10] net/colo.c: Add vnet packet parse feature in colo-proxy

Zhang Chen
In reply to this post by Zhang Chen
Make colo-compare and filter-rewriter can parse vnet packet.

Signed-off-by: Zhang Chen <[hidden email]>
---
 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




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Zhang Chen
In reply to this post by Zhang Chen
COLO-Proxy just focus on packet payload, So we skip vnet header.

Signed-off-by: Zhang Chen <[hidden email]>
---
 net/colo-compare.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index e33cf7e..ad1c3d5 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -201,8 +201,11 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
                                    sec_ip_src, sec_ip_dst);
     }
 
+    offset = ppkt->vnet_hdr_len + offset;
+
     if (ppkt->size == spkt->size) {
-        return memcmp(ppkt->data + offset, spkt->data + offset,
+        return memcmp(ppkt->data + offset,
+                      spkt->data + offset,
                       spkt->size - offset);
     } else {
         trace_colo_compare_main("Net packet size are not the same");
@@ -261,8 +264,9 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
      */
     if (ptcp->th_off > 5) {
         ptrdiff_t tcp_offset;
+
         tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
-                     + (ptcp->th_off * 4);
+                     + (ptcp->th_off * 4) - ppkt->vnet_hdr_len;
         res = colo_packet_compare_common(ppkt, spkt, tcp_offset);
     } else if (ptcp->th_sum == stcp->th_sum) {
         res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 09/10] net/filter-rewriter.c: Make filter-rewriter support vnet_hdr_len

Zhang Chen
In reply to this post by Zhang Chen
We add the vnet_hdr_support option for filter-rewriter, default is disable.
If you use virtio-net-pci net driver, please enable it.
You can use it for example:
-object filter-rewriter,id=rew0,netdev=hn0,queue=all,vnet_hdr_support

We get the vnet_hdr_len from NetClientState that make us
parse net packet correctly.

Signed-off-by: Zhang Chen <[hidden email]>
---
 net/filter-rewriter.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 qemu-options.hx       |  4 ++--
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 63256c7..8eaf0e8 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -17,6 +17,7 @@
 #include "qemu-common.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
 #include "qapi-visit.h"
 #include "qom/object.h"
 #include "qemu/main-loop.h"
@@ -33,6 +34,7 @@ typedef struct RewriterState {
     NetQueue *incoming_queue;
     /* hashtable to save connection */
     GHashTable *connection_track_table;
+    bool vnet_hdr;
 } RewriterState;
 
 static void filter_rewriter_flush(NetFilterState *nf)
@@ -155,10 +157,25 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
     ConnectionKey key;
     Packet *pkt;
     ssize_t size = iov_size(iov, iovcnt);
+    ssize_t vnet_hdr_len = 0;
     char *buf = g_malloc0(size);
 
     iov_to_buf(iov, iovcnt, 0, buf, size);
-    pkt = packet_new(buf, size, 0);
+
+    if (s->vnet_hdr) {
+        if (nf->direction == NET_FILTER_DIRECTION_RX ||
+            nf->direction == NET_FILTER_DIRECTION_ALL) {
+            vnet_hdr_len = nf->netdev->vnet_hdr_len;
+        } else if (nf->direction == NET_FILTER_DIRECTION_TX) {
+            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;
+        } else {
+            error_report("filter-rewriter get vnet_hdr_len failed");
+            /* When error occurred we drop the packet  */
+            return 1;
+        }
+    }
+
+    pkt = packet_new(buf, size, vnet_hdr_len);
     g_free(buf);
 
     /*
@@ -237,6 +254,37 @@ static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
     s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
 }
 
+static bool filter_rewriter_get_vnet_hdr(Object *obj, Error **errp)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(obj);
+
+    return s->vnet_hdr;
+}
+
+static void filter_rewriter_set_vnet_hdr(Object *obj,
+                                         bool value,
+                                         Error **errp)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(obj);
+
+    s->vnet_hdr = value;
+}
+
+static void filter_rewriter_init(Object *obj)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(obj);
+
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_bool(obj, "vnet_hdr_support",
+                             filter_rewriter_get_vnet_hdr,
+                             filter_rewriter_set_vnet_hdr, NULL);
+}
+
 static void colo_rewriter_class_init(ObjectClass *oc, void *data)
 {
     NetFilterClass *nfc = NETFILTER_CLASS(oc);
@@ -250,6 +298,7 @@ static const TypeInfo colo_rewriter_info = {
     .name = TYPE_FILTER_REWRITER,
     .parent = TYPE_NETFILTER,
     .class_init = colo_rewriter_class_init,
+    .instance_init = filter_rewriter_init,
     .instance_size = sizeof(RewriterState),
 };
 
diff --git a/qemu-options.hx b/qemu-options.hx
index fbfd604..8655842 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4037,12 +4037,12 @@ Create a filter-redirector we need to differ outdev id from indev id, id can not
 be the same. we can just use indev or outdev, but at least one of indev or outdev
 need to be specified.
 
-@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},rewriter-mode=@var{mode}[,queue=@var{all|rx|tx}]
+@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},rewriter-mode=@var{mode},queue=@var{all|rx|tx},[vnet_hdr_support]
 
 Filter-rewriter is a part of COLO project.It will rewrite tcp packet to
 secondary from primary to keep secondary tcp connection,and rewrite
 tcp packet to primary from secondary make tcp packet can be handled by
-client.
+client.if have the vnet_hdr_support flag, we can parse packet with vnet header.
 
 usage:
 colo secondary:
--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH V6 10/10] docs/colo-proxy.txt: Update colo-proxy usage of net driver with vnet_header

Zhang Chen
In reply to this post by Zhang Chen
Signed-off-by: Zhang Chen <[hidden email]>
---
 docs/colo-proxy.txt | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt
index c4941de..f6a624f 100644
--- a/docs/colo-proxy.txt
+++ b/docs/colo-proxy.txt
@@ -182,6 +182,32 @@ Secondary(ip:3.3.3.8):
 -chardev socket,id=red1,host=3.3.3.3,port=9004
 -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0
 -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
+-object filter-rewriter,id=f3,netdev=hn0,queue=all
+
+If you want to use virtio-net-pci or other driver with vnet_header:
+
+Primary(ip:3.3.3.3):
+-netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
+-device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
+-chardev socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
+-chardev socket,id=compare1,host=3.3.3.3,port=9004,server,nowait
+-chardev socket,id=compare0,host=3.3.3.3,port=9001,server,nowait
+-chardev socket,id=compare0-0,host=3.3.3.3,port=9001
+-chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
+-chardev socket,id=compare_out0,host=3.3.3.3,port=9005
+-object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr_support
+-object filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out,vnet_hdr_support
+-object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0,vnet_hdr_support
+-object colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr_support
+
+Secondary(ip:3.3.3.8):
+-netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
+-device e1000,netdev=hn0,mac=52:a4:00:12:78:66
+-chardev socket,id=red0,host=3.3.3.3,port=9003
+-chardev socket,id=red1,host=3.3.3.3,port=9004
+-object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0,vnet_hdr_support
+-object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1,vnet_hdr_support
+-object filter-rewriter,id=f3,netdev=hn0,queue=all,vnet_hdr_support
 
 Note:
   a.COLO-proxy must work with COLO-frame and Block-replication.
--
2.7.4




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 02/10] net/filter-mirror.c: Make filter mirror support vnet support.

Jason Wang-8
In reply to this post by Zhang Chen


On 2017年06月07日 17:55, Zhang Chen wrote:
> We add the vnet_hdr_support option for filter-mirror, default is disable.

s/disable/disabled/

> If you use virtio-net-pci net driver, please enable it.
> You can use it for example:
> -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr_support
>
> If have vnet_hdr_support flag,

If it has

>   we will change the send packet format from

s/send/sending/

> 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.

Please double check the commit logs, some have obvious grammar issues.

>
> Signed-off-by: Zhang Chen <[hidden email]>
> ---
>   net/filter-mirror.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++----
>   qemu-options.hx     |  5 ++--
>   2 files changed, 66 insertions(+), 8 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 72fa7c2..50aa81b 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -38,15 +38,17 @@ typedef struct MirrorState {
>       NetFilterState parent_obj;
>       char *indev;
>       char *outdev;
> +    bool vnet_hdr;
>       CharBackend chr_in;
>       CharBackend chr_out;
>       SocketReadState rs;
>   } MirrorState;
>  
> -static int filter_mirror_send(CharBackend *chr_out,
> +static int filter_mirror_send(MirrorState *s,
>                                 const struct iovec *iov,
>                                 int iovcnt)

This function were renamed to filter_send in your commit
e05dc4cf56d70225fc76225a3fd7df9f7b8ca5f9 ("net/filter-mirror.c: Rename
filter_mirror_send() and fix codestyle")

>   {
> +    NetFilterState *nf = NETFILTER(s);
>       int ret = 0;
>       ssize_t size = 0;
>       uint32_t len = 0;
> @@ -58,14 +60,43 @@ 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));

This looks a independent change, please do it in another patch.

>       if (ret != sizeof(len)) {
>           goto err;
>       }
>  
> +    if (s->vnet_hdr) {
> +        /*
> +         * If vnet_hdr = on, we send vnet header len to make other
> +         * module(like colo-compare) know how to parse net
> +         * packet correctly.
> +         */
> +        ssize_t vnet_hdr_len;
> +
> +        /*
> +         * 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.
> +         */

Let's remove the comment and let code explain itself.

> +        if (nf->direction == NET_FILTER_DIRECTION_RX ||
> +            nf->direction == NET_FILTER_DIRECTION_ALL) {
> +            vnet_hdr_len = nf->netdev->vnet_hdr_len;

This can only work if e.g virtio-net set its own vnet_hdr_len. But looks
like it use guest_hdr_len instead.

And using NET_FILTER_DIRECTION_ALL looks suspicious. Maybe we should
accept sender from its caller and compare it with nf->netdev to get the
correct direction.

> +        } else if (nf->direction == NET_FILTER_DIRECTION_TX) {
> +            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;

This looks wrong, TX means the packet were sent from netdev, we should
care nf->netdev's hdrlen.

> +        } else {
> +            return 0;
> +        }
> +
> +        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 +172,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 +195,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));
>           }
> @@ -308,6 +339,13 @@ static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>       return g_strdup(s->outdev);
>   }
>  
> +static bool filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
> +    return s->vnet_hdr;
> +}
> +
>   static void
>   filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>   {
> @@ -322,6 +360,15 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>       }
>   }
>  
> +static void filter_mirror_set_vnet_hdr(Object *obj,
> +                                       bool value,
> +                                       Error **errp)
> +{
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
> +    s->vnet_hdr = value;
> +}
> +
>   static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>   {
>       MirrorState *s = FILTER_REDIRECTOR(obj);
> @@ -340,8 +387,20 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>  
>   static void filter_mirror_init(Object *obj)
>   {
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>                               filter_mirror_set_outdev, NULL);
> +
> +    /*
> +     * The vnet_hdr is disabled by default, if you want to enable
> +     * this option, you must enable all the option on related modules
> +     * (like other filter or colo-compare).
> +     */

This comment makes sense for docs but is useless here, let code explain
itself.

Thanks

> +    s->vnet_hdr = false;
> +    object_property_add_bool(obj, "vnet_hdr_support",
> +                             filter_mirror_get_vnet_hdr,
> +                             filter_mirror_set_vnet_hdr, NULL);
>   }
>  
>   static void filter_redirector_init(Object *obj)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 70c0ded..5c09fae 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4024,10 +4024,9 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>   @option{tx}: the filter is attached to the transmit queue of the netdev,
>                where it will receive packets sent by the netdev.
>  
> -@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
> +@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
>  
> -filter-mirror on netdev @var{netdevid},mirror net packet to chardev
> -@var{chardevid}
> +filter-mirror on netdev @var{netdevid},mirror net packet to chardev@var{chardevid}, if have the vnet_hdr_support flag, filter-mirror will mirror packet with vnet_hdr_len.
>  
>   @item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>   outdev=@var{chardevid}[,queue=@var{all|rx|tx}]


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 03/10] net/filter-mirror.c: Add new option to enable vnet support for filter-redirector

Jason Wang-8
In reply to this post by Zhang Chen


On 2017年06月07日 17:55, Zhang Chen wrote:
> We add the vnet_hdr_support option for filter-redirector, default is disable.

s/disable/disabled/

> If you use virtio-net-pci net driver, please enable it.
> Because colo-compare or other modules needs the vnet_hdr_len to parse
> packet, so we add this new option send the len to others.

s/so//

> You can use it for example:
> -object filter-redirector,id=r0,netdev=hn0,queue=tx,outdev=red0,vnet_hdr_support
>
> Signed-off-by: Zhang Chen <[hidden email]>
> ---
>   net/filter-mirror.c | 28 ++++++++++++++++++++++++++++
>   qemu-options.hx     |  6 +++---
>   2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 50aa81b..3413e82 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -376,6 +376,13 @@ static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>       return g_strdup(s->outdev);
>   }
>  
> +static bool filter_redirector_get_vnet_hdr(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    return s->vnet_hdr;
> +}
> +
>   static void
>   filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>   {
> @@ -385,6 +392,15 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>       s->outdev = g_strdup(value);
>   }
>  
> +static void filter_redirector_set_vnet_hdr(Object *obj,
> +                                           bool value,
> +                                           Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    s->vnet_hdr = value;
> +}
> +
>   static void filter_mirror_init(Object *obj)
>   {
>       MirrorState *s = FILTER_MIRROR(obj);
> @@ -405,10 +421,22 @@ static void filter_mirror_init(Object *obj)
>  
>   static void filter_redirector_init(Object *obj)
>   {
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
>       object_property_add_str(obj, "indev", filter_redirector_get_indev,
>                               filter_redirector_set_indev, NULL);
>       object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
>                               filter_redirector_set_outdev, NULL);
> +
> +    /*
> +     * The vnet_hdr is disabled by default, if you want to enable
> +     * this option, you must enable all the option on related modules
> +     * (like other filter or colo-compare).
> +     */

Let's remove the comment here.

> +    s->vnet_hdr = false;
> +    object_property_add_bool(obj, "vnet_hdr_support",
> +                             filter_redirector_get_vnet_hdr,
> +                             filter_redirector_set_vnet_hdr, NULL);
>   }
>  
>   static void filter_mirror_fini(Object *obj)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5c09fae..e78b942 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4028,11 +4028,11 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>  
>   filter-mirror on netdev @var{netdevid},mirror net packet to chardev@var{chardevid}, if have the vnet_hdr_support flag, filter-mirror will mirror packet with vnet_hdr_len.
>  
> -@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
> -outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
> +@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
>  
>   filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
> -@var{chardevid},and redirect indev's packet to filter.
> +@var{chardevid},and redirect indev's packet to filter.if have the vnet_hdr_support flag,
> +filter-redirector will redirect packet with vnet_hdr_len.
>   Create a filter-redirector we need to differ outdev id from indev id, id can not
>   be the same. we can just use indev or outdev, but at least one of indev or outdev
>   need to be specified.


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 04/10] net/net.c: Add vnet_hdr support in SocketReadState

Jason Wang-8
In reply to this post by Zhang Chen


On 2017年06月07日 17:55, Zhang Chen wrote:
> We add a flag to dicide whether net_fill_rstate() to read

s/dicide/decide/ and s/to/needs/

> the vnet_hdr_len or not.
>
> Signed-off-by: Zhang Chen <[hidden email]>
> Suggested-by: Jason Wang <[hidden email]>
> ---
>   include/net/net.h   |  6 +++++-
>   net/filter-mirror.c |  1 +
>   net/net.c           | 33 ++++++++++++++++++++++++++++++---
>   3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 9a92c70..b2167ae 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -112,9 +112,13 @@ 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;
> +    /* This flag decide whether to read the vnet_hdr_len field */
> +    bool vnet_hdr;
>       uint32_t index;
>       uint32_t packet_len;
> +    uint32_t vnet_hdr_len;
>       uint8_t buf[NET_BUFSIZE];
>       SocketReadStateFinalize *finalize;
>   };
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 3413e82..4b03dda 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -267,6 +267,7 @@ static void filter_redirector_setup(NetFilterState *nf, Error **errp)
>       }
>  
>       net_socket_rs_init(&s->rs, redirector_rs_finalize);
> +    s->rs.vnet_hdr = s->vnet_hdr;

This looks like a follow up patch on top for this. And I would like to
reoder the series and make this patch as 02/10.

>  
>       if (s->indev) {
>           chr = qemu_chr_find(s->indev);
> diff --git a/net/net.c b/net/net.c
> index 4e7a305..b9b90c9 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1606,8 +1606,10 @@ void net_socket_rs_init(SocketReadState *rs,
>                           SocketReadStateFinalize *finalize)
>   {
>       rs->state = 0;
> +    rs->vnet_hdr = false;

Let's introduce a parameter for this function and let's caller set it
instead of manually toggling it like above.

>       rs->index = 0;
>       rs->packet_len = 0;
> +    rs->vnet_hdr_len = 0;
>       memset(rs->buf, 0, sizeof(rs->buf));
>       rs->finalize = finalize;
>   }
> @@ -1622,8 +1624,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) {
> @@ -1637,10 +1643,31 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
>                   /* got length */
>                   rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>                   rs->index = 0;
> -                rs->state = 1;
> +                if (rs->vnet_hdr) {
> +                    rs->state = 1;
> +                } else {
> +                    rs->state = 2;
> +                    rs->vnet_hdr_len = 0;
> +                }
>               }
>               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;


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 08/10] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare

Jason Wang-8
In reply to this post by Zhang Chen


On 2017年06月07日 17:55, Zhang Chen wrote:
> COLO-Proxy just focus on packet payload, So we skip vnet header.

s/So/so/

>
> Signed-off-by: Zhang Chen <[hidden email]>
> ---
>   net/colo-compare.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index e33cf7e..ad1c3d5 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -201,8 +201,11 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
>                                      sec_ip_src, sec_ip_dst);
>       }
>  
> +    offset = ppkt->vnet_hdr_len + offset;
> +
>       if (ppkt->size == spkt->size) {
> -        return memcmp(ppkt->data + offset, spkt->data + offset,
> +        return memcmp(ppkt->data + offset,
> +                      spkt->data + offset,
>                         spkt->size - offset);
>       } else {
>           trace_colo_compare_main("Net packet size are not the same");
> @@ -261,8 +264,9 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>        */
>       if (ptcp->th_off > 5) {
>           ptrdiff_t tcp_offset;
> +
>           tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
> -                     + (ptcp->th_off * 4);
> +                     + (ptcp->th_off * 4) - ppkt->vnet_hdr_len;
>           res = colo_packet_compare_common(ppkt, spkt, tcp_offset);
>       } else if (ptcp->th_sum == stcp->th_sum) {
>           res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 09/10] net/filter-rewriter.c: Make filter-rewriter support vnet_hdr_len

Jason Wang-8
In reply to this post by Zhang Chen


On 2017年06月07日 17:55, Zhang Chen wrote:

> We add the vnet_hdr_support option for filter-rewriter, default is disable.
> If you use virtio-net-pci net driver, please enable it.
> You can use it for example:
> -object filter-rewriter,id=rew0,netdev=hn0,queue=all,vnet_hdr_support
>
> We get the vnet_hdr_len from NetClientState that make us
> parse net packet correctly.
>
> Signed-off-by: Zhang Chen <[hidden email]>
> ---
>   net/filter-rewriter.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>   qemu-options.hx       |  4 ++--
>   2 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
> index 63256c7..8eaf0e8 100644
> --- a/net/filter-rewriter.c
> +++ b/net/filter-rewriter.c
> @@ -17,6 +17,7 @@
>   #include "qemu-common.h"
>   #include "qapi/error.h"
>   #include "qapi/qmp/qerror.h"
> +#include "qemu/error-report.h"
>   #include "qapi-visit.h"
>   #include "qom/object.h"
>   #include "qemu/main-loop.h"
> @@ -33,6 +34,7 @@ typedef struct RewriterState {
>       NetQueue *incoming_queue;
>       /* hashtable to save connection */
>       GHashTable *connection_track_table;
> +    bool vnet_hdr;
>   } RewriterState;
>  
>   static void filter_rewriter_flush(NetFilterState *nf)
> @@ -155,10 +157,25 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>       ConnectionKey key;
>       Packet *pkt;
>       ssize_t size = iov_size(iov, iovcnt);
> +    ssize_t vnet_hdr_len = 0;
>       char *buf = g_malloc0(size);
>  
>       iov_to_buf(iov, iovcnt, 0, buf, size);
> -    pkt = packet_new(buf, size, 0);
> +
> +    if (s->vnet_hdr) {
> +        if (nf->direction == NET_FILTER_DIRECTION_RX ||
> +            nf->direction == NET_FILTER_DIRECTION_ALL) {
> +            vnet_hdr_len = nf->netdev->vnet_hdr_len;
> +        } else if (nf->direction == NET_FILTER_DIRECTION_TX) {
> +            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;
> +        } else {
> +            error_report("filter-rewriter get vnet_hdr_len failed");
> +            /* When error occurred we drop the packet  */
> +            return 1;
> +        }
> +    }

Similar issue as patch 02, and we can probably use a helper to avoid
duplicating codes.

Thanks

> +
> +    pkt = packet_new(buf, size, vnet_hdr_len);
>       g_free(buf);
>  
>       /*
> @@ -237,6 +254,37 @@ static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
>       s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
>   }
>  
> +static bool filter_rewriter_get_vnet_hdr(Object *obj, Error **errp)
> +{
> +    RewriterState *s = FILTER_COLO_REWRITER(obj);
> +
> +    return s->vnet_hdr;
> +}
> +
> +static void filter_rewriter_set_vnet_hdr(Object *obj,
> +                                         bool value,
> +                                         Error **errp)
> +{
> +    RewriterState *s = FILTER_COLO_REWRITER(obj);
> +
> +    s->vnet_hdr = value;
> +}
> +
> +static void filter_rewriter_init(Object *obj)
> +{
> +    RewriterState *s = FILTER_COLO_REWRITER(obj);
> +
> +    /*
> +     * The vnet_hdr is disabled by default, if you want to enable
> +     * this option, you must enable all the option on related modules
> +     * (like other filter or colo-compare).
> +     */
> +    s->vnet_hdr = false;
> +    object_property_add_bool(obj, "vnet_hdr_support",
> +                             filter_rewriter_get_vnet_hdr,
> +                             filter_rewriter_set_vnet_hdr, NULL);
> +}
> +
>   static void colo_rewriter_class_init(ObjectClass *oc, void *data)
>   {
>       NetFilterClass *nfc = NETFILTER_CLASS(oc);
> @@ -250,6 +298,7 @@ static const TypeInfo colo_rewriter_info = {
>       .name = TYPE_FILTER_REWRITER,
>       .parent = TYPE_NETFILTER,
>       .class_init = colo_rewriter_class_init,
> +    .instance_init = filter_rewriter_init,
>       .instance_size = sizeof(RewriterState),
>   };
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index fbfd604..8655842 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4037,12 +4037,12 @@ Create a filter-redirector we need to differ outdev id from indev id, id can not
>   be the same. we can just use indev or outdev, but at least one of indev or outdev
>   need to be specified.
>  
> -@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},rewriter-mode=@var{mode}[,queue=@var{all|rx|tx}]
> +@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},rewriter-mode=@var{mode},queue=@var{all|rx|tx},[vnet_hdr_support]
>  
>   Filter-rewriter is a part of COLO project.It will rewrite tcp packet to
>   secondary from primary to keep secondary tcp connection,and rewrite
>   tcp packet to primary from secondary make tcp packet can be handled by
> -client.
> +client.if have the vnet_hdr_support flag, we can parse packet with vnet header.
>  
>   usage:
>   colo secondary:


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 10/10] docs/colo-proxy.txt: Update colo-proxy usage of net driver with vnet_header

Jason Wang-8
In reply to this post by Zhang Chen


On 2017年06月07日 17:55, Zhang Chen wrote:

> Signed-off-by: Zhang Chen <[hidden email]>
> ---
>   docs/colo-proxy.txt | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt
> index c4941de..f6a624f 100644
> --- a/docs/colo-proxy.txt
> +++ b/docs/colo-proxy.txt
> @@ -182,6 +182,32 @@ Secondary(ip:3.3.3.8):
>   -chardev socket,id=red1,host=3.3.3.3,port=9004
>   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0
>   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
> +-object filter-rewriter,id=f3,netdev=hn0,queue=all
> +
> +If you want to use virtio-net-pci or other driver with vnet_header:

s/virtio-net-pci/virtio-net/ and s/driver/device/

Thanks

> +
> +Primary(ip:3.3.3.3):
> +-netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
> +-device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
> +-chardev socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
> +-chardev socket,id=compare1,host=3.3.3.3,port=9004,server,nowait
> +-chardev socket,id=compare0,host=3.3.3.3,port=9001,server,nowait
> +-chardev socket,id=compare0-0,host=3.3.3.3,port=9001
> +-chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
> +-chardev socket,id=compare_out0,host=3.3.3.3,port=9005
> +-object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr_support
> +-object filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out,vnet_hdr_support
> +-object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0,vnet_hdr_support
> +-object colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr_support
> +
> +Secondary(ip:3.3.3.8):
> +-netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
> +-device e1000,netdev=hn0,mac=52:a4:00:12:78:66
> +-chardev socket,id=red0,host=3.3.3.3,port=9003
> +-chardev socket,id=red1,host=3.3.3.3,port=9004
> +-object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0,vnet_hdr_support
> +-object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1,vnet_hdr_support
> +-object filter-rewriter,id=f3,netdev=hn0,queue=all,vnet_hdr_support
>  
>   Note:
>     a.COLO-proxy must work with COLO-frame and Block-replication.


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 02/10] net/filter-mirror.c: Make filter mirror support vnet support.

Zhang Chen
In reply to this post by Jason Wang-8


On 06/09/2017 02:08 PM, Jason Wang wrote:

>
>
> On 2017年06月07日 17:55, Zhang Chen wrote:
>> We add the vnet_hdr_support option for filter-mirror, default is
>> disable.
>
> s/disable/disabled/
>
>> If you use virtio-net-pci net driver, please enable it.
>> You can use it for example:
>> -object
>> filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr_support
>>
>> If have vnet_hdr_support flag,
>
> If it has
>
>>   we will change the send packet format from
>
> s/send/sending/
>
>> 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.
>
> Please double check the commit logs, some have obvious grammar issues.

OK, I will fix the garmmar issues.

>
>>
>> Signed-off-by: Zhang Chen <[hidden email]>
>> ---
>>   net/filter-mirror.c | 69
>> +++++++++++++++++++++++++++++++++++++++++++++++++----
>>   qemu-options.hx     |  5 ++--
>>   2 files changed, 66 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 72fa7c2..50aa81b 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -38,15 +38,17 @@ typedef struct MirrorState {
>>       NetFilterState parent_obj;
>>       char *indev;
>>       char *outdev;
>> +    bool vnet_hdr;
>>       CharBackend chr_in;
>>       CharBackend chr_out;
>>       SocketReadState rs;
>>   } MirrorState;
>>   -static int filter_mirror_send(CharBackend *chr_out,
>> +static int filter_mirror_send(MirrorState *s,
>>                                 const struct iovec *iov,
>>                                 int iovcnt)
>
> This function were renamed to filter_send in your commit
> e05dc4cf56d70225fc76225a3fd7df9f7b8ca5f9 ("net/filter-mirror.c: Rename
> filter_mirror_send() and fix codestyle")

I will rebase the code for upstream.

>
>>   {
>> +    NetFilterState *nf = NETFILTER(s);
>>       int ret = 0;
>>       ssize_t size = 0;
>>       uint32_t len = 0;
>> @@ -58,14 +60,43 @@ 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));
>
> This looks a independent change, please do it in another patch.

OK.

>
>>       if (ret != sizeof(len)) {
>>           goto err;
>>       }
>>   +    if (s->vnet_hdr) {
>> +        /*
>> +         * If vnet_hdr = on, we send vnet header len to make other
>> +         * module(like colo-compare) know how to parse net
>> +         * packet correctly.
>> +         */
>> +        ssize_t vnet_hdr_len;
>> +
>> +        /*
>> +         * 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.
>> +         */
>
> Let's remove the comment and let code explain itself.

I will remove it in next version.

>
>> +        if (nf->direction == NET_FILTER_DIRECTION_RX ||
>> +            nf->direction == NET_FILTER_DIRECTION_ALL) {
>> +            vnet_hdr_len = nf->netdev->vnet_hdr_len;
>
> This can only work if e.g virtio-net set its own vnet_hdr_len. But
> looks like it use guest_hdr_len instead.

I see in hw/net/virtio-net.c use the "qemu_set_vnet_hdr_len(nc->peer,
n->guest_hdr_len);" to
set the nf->netdev->vnet_hdr_len, any case not include here?

>
> And using NET_FILTER_DIRECTION_ALL looks suspicious. Maybe we should
> accept sender from its caller and compare it with nf->netdev to get
> the correct direction.
>
>> +        } else if (nf->direction == NET_FILTER_DIRECTION_TX) {
>> +            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;
>
> This looks wrong, TX means the packet were sent from netdev, we should
> care nf->netdev's hdrlen.

I rethink and test the code, you are right. I will remove it.

>
>> +        } else {
>> +            return 0;
>> +        }
>> +
>> +        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 +172,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 +195,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));
>>           }
>> @@ -308,6 +339,13 @@ static char *filter_mirror_get_outdev(Object
>> *obj, Error **errp)
>>       return g_strdup(s->outdev);
>>   }
>>   +static bool filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>> +    return s->vnet_hdr;
>> +}
>> +
>>   static void
>>   filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>>   {
>> @@ -322,6 +360,15 @@ filter_mirror_set_outdev(Object *obj, const char
>> *value, Error **errp)
>>       }
>>   }
>>   +static void filter_mirror_set_vnet_hdr(Object *obj,
>> +                                       bool value,
>> +                                       Error **errp)
>> +{
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>> +    s->vnet_hdr = value;
>> +}
>> +
>>   static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>>   {
>>       MirrorState *s = FILTER_REDIRECTOR(obj);
>> @@ -340,8 +387,20 @@ filter_redirector_set_outdev(Object *obj, const
>> char *value, Error **errp)
>>     static void filter_mirror_init(Object *obj)
>>   {
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>>                               filter_mirror_set_outdev, NULL);
>> +
>> +    /*
>> +     * The vnet_hdr is disabled by default, if you want to enable
>> +     * this option, you must enable all the option on related modules
>> +     * (like other filter or colo-compare).
>> +     */
>
> This comment makes sense for docs but is useless here, let code
> explain itself.

OK.

Thanks
Zhang Chen

>
> Thanks
>
>> +    s->vnet_hdr = false;
>> +    object_property_add_bool(obj, "vnet_hdr_support",
>> +                             filter_mirror_get_vnet_hdr,
>> +                             filter_mirror_set_vnet_hdr, NULL);
>>   }
>>     static void filter_redirector_init(Object *obj)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 70c0ded..5c09fae 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4024,10 +4024,9 @@ queue @var{all|rx|tx} is an option that can be
>> applied to any netfilter.
>>   @option{tx}: the filter is attached to the transmit queue of the
>> netdev,
>>                where it will receive packets sent by the netdev.
>>   -@item -object
>> filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>> +@item -object
>> filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
>>   -filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>> -@var{chardevid}
>> +filter-mirror on netdev @var{netdevid},mirror net packet to
>> chardev@var{chardevid}, if have the vnet_hdr_support flag,
>> filter-mirror will mirror packet with vnet_hdr_len.
>>     @item -object
>> filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>>   outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>
>
>
> .
>

--
Thanks
Zhang Chen




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 02/10] net/filter-mirror.c: Make filter mirror support vnet support.

Jason Wang-8


On 2017年06月12日 17:27, Zhang Chen wrote:

>>
>>> +        if (nf->direction == NET_FILTER_DIRECTION_RX ||
>>> +            nf->direction == NET_FILTER_DIRECTION_ALL) {
>>> +            vnet_hdr_len = nf->netdev->vnet_hdr_len;
>>
>> This can only work if e.g virtio-net set its own vnet_hdr_len. But
>> looks like it use guest_hdr_len instead.
>
> I see in hw/net/virtio-net.c use the "qemu_set_vnet_hdr_len(nc->peer,
> n->guest_hdr_len);" to
> set the nf->netdev->vnet_hdr_len, any case not include here?

You mean:

         if (peer_has_vnet_hdr(n) &&
             qemu_has_vnet_hdr_len(nc->peer, n->guest_hdr_len)) {
             qemu_set_vnet_hdr_len(nc->peer, n->guest_hdr_len);
             n->host_hdr_len = n->guest_hdr_len;
         }

?

 From the code, it only set peer's vnet header when peer supports
vnet_hdr up to n->guest_hdr_len. If peer can't, virtio-net will strip
the header.

Thanks

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH V6 02/10] net/filter-mirror.c: Make filter mirror support vnet support.

Zhang Chen


On 06/13/2017 05:14 PM, Jason Wang wrote:

>
>
> On 2017年06月12日 17:27, Zhang Chen wrote:
>>>
>>>> +        if (nf->direction == NET_FILTER_DIRECTION_RX ||
>>>> +            nf->direction == NET_FILTER_DIRECTION_ALL) {
>>>> +            vnet_hdr_len = nf->netdev->vnet_hdr_len;
>>>
>>> This can only work if e.g virtio-net set its own vnet_hdr_len. But
>>> looks like it use guest_hdr_len instead.
>>
>> I see in hw/net/virtio-net.c use the "qemu_set_vnet_hdr_len(nc->peer,
>> n->guest_hdr_len);" to
>> set the nf->netdev->vnet_hdr_len, any case not include here?
>
> You mean:
>
>         if (peer_has_vnet_hdr(n) &&
>             qemu_has_vnet_hdr_len(nc->peer, n->guest_hdr_len)) {
>             qemu_set_vnet_hdr_len(nc->peer, n->guest_hdr_len);
>             n->host_hdr_len = n->guest_hdr_len;
>         }
>
> ?
>
> From the code, it only set peer's vnet header when peer supports
> vnet_hdr up to n->guest_hdr_len. If peer can't, virtio-net will strip
> the header.

So, We should get the n->guest_hdr_len in another way? like set a new
parameter in NetClientState?
Any suggestion about it?

Thanks
Zhang Chen

>
> Thanks
>
>
>

--
Thanks
Zhang Chen




12
Loading...