[Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Zhang Chen]

Filter-rewriter is a part of COLO project.
So this patch set depend on colo-compare.
It will rewrite some of secondary packet to make
secondary guest's connection established successfully.
In this module we will rewrite tcp packet's ack to the secondary
from primary,and rewrite tcp packet's seq to the primary from
secondary.

v3:
  - fix typo
  - add conn->syn_flag
  - add some comments
  - remove conn_list_lock
  - fix offset set bug

v2:
  - add more comments about packet flows
  - add some trace-event
  - add seq offset ( = secondary_seq - primary_seq)

v1:
  - initial patch

Zhang Chen (3):
  filter-rewriter: introduce filter-rewriter initialization
  filter-rewriter: track connection and parse packet
  filter-rewriter: rewrite tcp packet to keep secondary connection

 net/Makefile.objs     |   1 +
 net/colo-base.c       |   2 +
 net/colo-base.h       |   7 ++
 net/filter-rewriter.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx       |  13 +++
 trace-events          |   5 +
 vl.c                  |   3 +-
 7 files changed, 292 insertions(+), 1 deletion(-)
 create mode 100644 net/filter-rewriter.c

-- 
2.7.4

[Qemu-devel] [RFC PATCH V3 1/3] filter-rewriter: introduce filter-rewriter initialization

[Zhang Chen]

Filter-rewriter is a part of COLO project.
It will rewrite some of secondary packet to make
secondary guest's tcp connection established successfully.
In this module we will rewrite tcp packet's ack to the secondary
from primary,and rewrite tcp packet's seq to the primary from
secondary.

usage:

colo secondary:
-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=rew0,netdev=hn0,queue=all

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 net/Makefile.objs     |   1 +
 net/filter-rewriter.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx       |  13 ++++++
 vl.c                  |   3 +-
 4 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 net/filter-rewriter.c

diff --git a/net/Makefile.objs b/net/Makefile.objs
index 119589f..645bd10 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -18,3 +18,4 @@ common-obj-y += filter-buffer.o
 common-obj-y += filter-mirror.o
 common-obj-y += colo-compare.o
 common-obj-y += colo-base.o
+common-obj-y += filter-rewriter.o
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
new file mode 100644
index 0000000..c4f2739
--- /dev/null
+++ b/net/filter-rewriter.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ * Copyright (c) 2016 FUJITSU LIMITED
+ * Copyright (c) 2016 Intel Corporation
+ *
+ * Author: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "net/colo-base.h"
+#include "net/filter.h"
+#include "net/net.h"
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi-visit.h"
+#include "qom/object.h"
+#include "qemu/main-loop.h"
+#include "qemu/iov.h"
+#include "net/checksum.h"
+
+#define FILTER_COLO_REWRITER(obj) \
+    OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
+
+#define TYPE_FILTER_REWRITER "filter-rewriter"
+
+enum {
+    PRIMARY = 0,
+    SECONDARY,
+};
+
+typedef struct RewriterState {
+    NetFilterState parent_obj;
+    /* connection list: the connections belonged to this NIC could be found
+     * in this list.
+     * element type: Connection
+     */
+    GQueue conn_list;
+    NetQueue *incoming_queue;
+    /* hashtable to save connection */
+    GHashTable *connection_track_table;
+    /* to save unprocessed_connections */
+    GQueue unprocessed_connections;
+    /* current hash size */
+    uint32_t hashtable_size;
+} RewriterState;
+
+static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
+                                         NetClientState *sender,
+                                         unsigned flags,
+                                         const struct iovec *iov,
+                                         int iovcnt,
+                                         NetPacketSent *sent_cb)
+{
+    /*
+     * if we get tcp packet
+     * we will rewrite it to make secondary guest's
+     * connection established successfully
+     */
+    return 0;
+}
+
+static void colo_rewriter_cleanup(NetFilterState *nf)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(nf);
+
+    g_queue_free(&s->conn_list);
+}
+
+static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(nf);
+
+    g_queue_init(&s->conn_list);
+    s->hashtable_size = 0;
+
+    s->connection_track_table = g_hash_table_new_full(connection_key_hash,
+                                                      connection_key_equal,
+                                                      g_free,
+                                                      connection_destroy);
+    s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
+}
+
+static void colo_rewriter_class_init(ObjectClass *oc, void *data)
+{
+    NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+    nfc->setup = colo_rewriter_setup;
+    nfc->cleanup = colo_rewriter_cleanup;
+    nfc->receive_iov = colo_rewriter_receive_iov;
+}
+
+static const TypeInfo colo_rewriter_info = {
+    .name = TYPE_FILTER_REWRITER,
+    .parent = TYPE_NETFILTER,
+    .class_init = colo_rewriter_class_init,
+    .instance_size = sizeof(RewriterState),
+};
+
+static void register_types(void)
+{
+    type_register_static(&colo_rewriter_info);
+}
+
+type_init(register_types);
diff --git a/qemu-options.hx b/qemu-options.hx
index 14bade5..4afd511 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3859,6 +3859,19 @@ 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}]
+
+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.
+
+usage:
+colo secondary:
+-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=rew0,netdev=hn0,queue=all
+
 @item -object filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
 
 Dump the network traffic on netdev @var{dev} to the file specified by
diff --git a/vl.c b/vl.c
index c6b9a6f..b47be6a 100644
--- a/vl.c
+++ b/vl.c
@@ -2866,7 +2866,8 @@ static bool object_create_initial(const char *type)
         g_str_equal(type, "filter-dump") ||
         g_str_equal(type, "filter-mirror") ||
         g_str_equal(type, "filter-redirector") ||
-        g_str_equal(type, "colo-compare")) {
+        g_str_equal(type, "colo-compare") ||
+        g_str_equal(type, "filter-rewriter")) {
         return false;
     }
 
-- 
2.7.4

[Qemu-devel] [RFC PATCH V3 2/3] filter-rewriter: track connection and parse packet

[Zhang Chen]

We use colo-base.h to track connection and parse packet

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 net/filter-rewriter.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index c4f2739..7f0da2c 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -48,6 +48,20 @@ typedef struct RewriterState {
     uint32_t hashtable_size;
 } RewriterState;
 
+/*
+ * Return 1 on success, if return 0 means the pkt
+ * is not TCP packet
+ */
+static int is_tcp_packet(Packet *pkt)
+{
+    if (!parse_packet_early(pkt) &&
+        pkt->ip->ip_p == IPPROTO_TCP) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
 static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
                                          NetClientState *sender,
                                          unsigned flags,
@@ -55,11 +69,47 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
                                          int iovcnt,
                                          NetPacketSent *sent_cb)
 {
+    RewriterState *s = FILTER_COLO_REWRITER(nf);
+    Connection *conn;
+    ConnectionKey key = {{ 0 } };
+    Packet *pkt;
+    ssize_t size = iov_size(iov, iovcnt);
+    char *buf = g_malloc0(size);
+
+    iov_to_buf(iov, iovcnt, 0, buf, size);
+    pkt = packet_new(buf, size);
+
     /*
      * if we get tcp packet
      * we will rewrite it to make secondary guest's
      * connection established successfully
      */
+    if (is_tcp_packet(pkt)) {
+        if (sender == nf->netdev) {
+            fill_connection_key(pkt, &key, SECONDARY);
+        } else {
+            fill_connection_key(pkt, &key, PRIMARY);
+        }
+
+        conn = connection_get(s->connection_track_table,
+                              &key,
+                              &s->hashtable_size);
+        if (!conn->processing) {
+            g_queue_push_tail(&s->conn_list, conn);
+            conn->processing = true;
+        }
+
+        if (sender == nf->netdev) {
+            /* NET_FILTER_DIRECTION_TX */
+            /* handle_primary_tcp_pkt */
+        } else {
+            /* NET_FILTER_DIRECTION_RX */
+            /* handle_secondary_tcp_pkt */
+        }
+    }
+
+    packet_destroy(pkt, NULL);
+    pkt = NULL;
     return 0;
 }
 
-- 
2.7.4

[Qemu-devel] [RFC PATCH V3 3/3] filter-rewriter: rewrite tcp packet to keep secondary connection

[Zhang Chen]

We will rewrite tcp packet secondary received and sent.
When colo guest is a tcp server.

Firstly, client start a tcp handshake. the packet's seq=client_seq,
ack=0,flag=SYN. COLO primary guest get this pkt and mirror(filter-mirror)
to secondary guest, secondary get it use filter-redirector.
Then,primary guest response pkt
(seq=primary_seq,ack=client_seq+1,flag=ACK|SYN).
secondary guest response pkt
(seq=secondary_seq,ack=client_seq+1,flag=ACK|SYN).
In here,we use filter-rewriter save the secondary_seq to it's tcp connection.
Finally handshake,client send pkt
(seq=client_seq+1,ack=primary_seq+1,flag=ACK).
Here,filter-rewriter can get primary_seq, and rewrite ack from primary_seq+1
to secondary_seq+1, recalculate checksum. So the secondary tcp connection
kept good.

When we send/recv packet.
client send pkt(seq=client_seq+1+data_len,ack=primary_seq+1,flag=ACK|PSH).
filter-rewriter rewrite ack and send to secondary guest.

primary guest response pkt
(seq=primary_seq+1,ack=client_seq+1+data_len,flag=ACK)
secondary guest response pkt
(seq=secondary_seq+1,ack=client_seq+1+data_len,flag=ACK)
we rewrite secondary guest seq from secondary_seq+1 to primary_seq+1.
So tcp connection kept good.

In code We use offset( = secondary_seq - primary_seq )
to rewrite seq or ack.
handle_primary_tcp_pkt: tcp_pkt->th_ack += offset;
handle_secondary_tcp_pkt: tcp_pkt->th_seq -= offset;

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 net/colo-base.c       |   2 +
 net/colo-base.h       |   7 ++++
 net/filter-rewriter.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++-
 trace-events          |   5 +++
 4 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/net/colo-base.c b/net/colo-base.c
index 9673661..58fbd9d 100644
--- a/net/colo-base.c
+++ b/net/colo-base.c
@@ -123,6 +123,8 @@ Connection *connection_new(ConnectionKey *key)
 
     conn->ip_proto = key->ip_proto;
     conn->processing = false;
+    conn->offset = 0;
+    conn->syn_flag = 0;
     g_queue_init(&conn->primary_list);
     g_queue_init(&conn->secondary_list);
 
diff --git a/net/colo-base.h b/net/colo-base.h
index 62460c5..353bd55 100644
--- a/net/colo-base.h
+++ b/net/colo-base.h
@@ -71,6 +71,13 @@ typedef struct Connection {
     uint8_t ip_proto;
     /* be used by filter-rewriter */
     colo_conn_state state;
+    /* offset = secondary_seq - primary_seq */
+    tcp_seq  offset;
+    /*
+     * we use this flag update offset func
+     * run once in independent tcp connection
+     */
+    int syn_flag;
 } Connection;
 
 uint32_t connection_key_hash(const void *opaque);
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 7f0da2c..f911f99 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -21,6 +21,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/iov.h"
 #include "net/checksum.h"
+#include "trace.h"
 
 #define FILTER_COLO_REWRITER(obj) \
     OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
@@ -62,6 +63,89 @@ static int is_tcp_packet(Packet *pkt)
     }
 }
 
+/* handle tcp packet from primary guest */
+static int handle_primary_tcp_pkt(NetFilterState *nf,
+                                  Connection *conn,
+                                  Packet *pkt)
+{
+    struct tcphdr *tcp_pkt;
+
+    tcp_pkt = (struct tcphdr *)pkt->transport_layer;
+    if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
+        char *sdebug, *ddebug;
+        sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
+        ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
+        trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
+                    ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
+                    tcp_pkt->th_flags);
+        trace_colo_filter_rewriter_conn_offset(conn->offset);
+        g_free(sdebug);
+        g_free(ddebug);
+    }
+
+    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
+        /*
+         * we use this flag update offset func
+         * run once in independent tcp connection
+         */
+        conn->syn_flag = 1;
+    }
+
+    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK)) {
+        if (conn->syn_flag) {
+            /* offset = secondary_seq - primary seq */
+            conn->offset -= (ntohl(tcp_pkt->th_ack) - 1);
+            conn->syn_flag = 0;
+        }
+        /* handle packets to the secondary from the primary */
+        tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset);
+
+        net_checksum_calculate((uint8_t *)pkt->data, pkt->size);
+    }
+
+    return 0;
+}
+
+/* handle tcp packet from secondary guest */
+static int handle_secondary_tcp_pkt(NetFilterState *nf,
+                                    Connection *conn,
+                                    Packet *pkt)
+{
+    struct tcphdr *tcp_pkt;
+
+    tcp_pkt = (struct tcphdr *)pkt->transport_layer;
+
+    if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
+        char *sdebug, *ddebug;
+        sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
+        ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
+        trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
+                    ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
+                    tcp_pkt->th_flags);
+        trace_colo_filter_rewriter_conn_offset(conn->offset);
+        g_free(sdebug);
+        g_free(ddebug);
+    }
+
+    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) {
+        /*
+         * save offset = secondary_seq and then
+         * in handle_primary_tcp_pkt make offset
+         * = secondary_seq - primary_seq
+         */
+        conn->offset = ntohl(tcp_pkt->th_seq);
+    }
+
+    if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) {
+        /* handle packets to the primary from the secondary*/
+        tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset);
+
+        net_checksum_calculate((uint8_t *)pkt->data, pkt->size);
+    }
+
+    return 0;
+}
+
 static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
                                          NetClientState *sender,
                                          unsigned flags,
@@ -101,10 +185,30 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
 
         if (sender == nf->netdev) {
             /* NET_FILTER_DIRECTION_TX */
-            /* handle_primary_tcp_pkt */
+            if (!handle_primary_tcp_pkt(nf, conn, pkt)) {
+                qemu_net_queue_send(s->incoming_queue, sender, 0,
+                (const uint8_t *)pkt->data, pkt->size, NULL);
+                packet_destroy(pkt, NULL);
+                pkt = NULL;
+                /*
+                 * We block the packet here,after rewrite pkt
+                 * and will send it
+                 */
+                return 1;
+            }
         } else {
             /* NET_FILTER_DIRECTION_RX */
-            /* handle_secondary_tcp_pkt */
+            if (!handle_secondary_tcp_pkt(nf, conn, pkt)) {
+                qemu_net_queue_send(s->incoming_queue, sender, 0,
+                (const uint8_t *)pkt->data, pkt->size, NULL);
+                packet_destroy(pkt, NULL);
+                pkt = NULL;
+                /*
+                 * We block the packet here,after rewrite pkt
+                 * and will send it
+                 */
+                return 1;
+            }
         }
     }
 
diff --git a/trace-events b/trace-events
index 6686cdf..5ac56f6 100644
--- a/trace-events
+++ b/trace-events
@@ -1927,3 +1927,8 @@ colo_compare_icmp_miscompare_mtu(const char *sta, int size) ": %s  %d"
 colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
 colo_old_packet_check_found(int64_t old_time) "%" PRId64
 colo_compare_miscompare(void) ""
+
+# net/filter-rewriter.c
+colo_filter_rewriter_debug(void) ""
+colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=%x\n"
+colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u\n"
-- 
2.7.4

Re: [Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Zhang Chen]

Hi~ all~~

Anyone have some comments for this series?

We need more comments to improve COLO codes.


Thanks

Zhang Chen


On 07/05/2016 06:29 PM, Zhang Chen wrote:
> Filter-rewriter is a part of COLO project.
> So this patch set depend on colo-compare.
> It will rewrite some of secondary packet to make
> secondary guest's connection established successfully.
> In this module we will rewrite tcp packet's ack to the secondary
> from primary,and rewrite tcp packet's seq to the primary from
> secondary.
>
> v3:
>    - fix typo
>    - add conn->syn_flag
>    - add some comments
>    - remove conn_list_lock
>    - fix offset set bug
>
> v2:
>    - add more comments about packet flows
>    - add some trace-event
>    - add seq offset ( = secondary_seq - primary_seq)
>
> v1:
>    - initial patch
>
> Zhang Chen (3):
>    filter-rewriter: introduce filter-rewriter initialization
>    filter-rewriter: track connection and parse packet
>    filter-rewriter: rewrite tcp packet to keep secondary connection
>
>   net/Makefile.objs     |   1 +
>   net/colo-base.c       |   2 +
>   net/colo-base.h       |   7 ++
>   net/filter-rewriter.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   qemu-options.hx       |  13 +++
>   trace-events          |   5 +
>   vl.c                  |   3 +-
>   7 files changed, 292 insertions(+), 1 deletion(-)
>   create mode 100644 net/filter-rewriter.c
>

-- 
Thanks
zhangchen

Re: [Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Jason Wang]

On 2016年07月27日 10:32, Zhang Chen wrote:
> Hi~ all~~
>
> Anyone have some comments for this series?

Will have a look at this.

Btw. I wonder how this is tested, do we need a unit test for this?

>
> We need more comments to improve COLO codes.
>
>
> Thanks
>
> Zhang Chen
>
>
> On 07/05/2016 06:29 PM, Zhang Chen wrote:
>> Filter-rewriter is a part of COLO project.
>> So this patch set depend on colo-compare.

Interesting, I thought it can co-work with comparing, and the 
communication were done through socket now. So I think technically the 
filter itself could work without COLO?

>> It will rewrite some of secondary packet to make
>> secondary guest's connection established successfully.
>> In this module we will rewrite tcp packet's ack to the secondary
>> from primary,and rewrite tcp packet's seq to the primary from
>> secondary.
>>
>> v3:
>>    - fix typo
>>    - add conn->syn_flag
>>    - add some comments
>>    - remove conn_list_lock
>>    - fix offset set bug

I think you can remove RFC in the next version so we can try to merge it 
after one or several other iterations.

Thanks

>>
>> v2:
>>    - add more comments about packet flows
>>    - add some trace-event
>>    - add seq offset ( = secondary_seq - primary_seq)
>>
>> v1:
>>    - initial patch
>>
>> Zhang Chen (3):
>>    filter-rewriter: introduce filter-rewriter initialization
>>    filter-rewriter: track connection and parse packet
>>    filter-rewriter: rewrite tcp packet to keep secondary connection
>>
>>   net/Makefile.objs     |   1 +
>>   net/colo-base.c       |   2 +
>>   net/colo-base.h       |   7 ++
>>   net/filter-rewriter.c | 262 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-options.hx       |  13 +++
>>   trace-events          |   5 +
>>   vl.c                  |   3 +-
>>   7 files changed, 292 insertions(+), 1 deletion(-)
>>   create mode 100644 net/filter-rewriter.c
>>
>

Re: [Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Zhang Chen]

On 07/27/2016 10:50 AM, Jason Wang wrote:
>
>
> On 2016年07月27日 10:32, Zhang Chen wrote:
>> Hi~ all~~
>>
>> Anyone have some comments for this series?
>
> Will have a look at this.
>
> Btw. I wonder how this is tested, do we need a unit test for this?

COLO-compare and filter-rewriter works normal depend on colo-frame.
So, When colo-frame be merged I will send a colo-proxy unit test
for all filter-mirror,filter-redirector,colo-compare,filter-rewriter.


>
>>
>> We need more comments to improve COLO codes.
>>
>>
>> Thanks
>>
>> Zhang Chen
>>
>>
>> On 07/05/2016 06:29 PM, Zhang Chen wrote:
>>> Filter-rewriter is a part of COLO project.
>>> So this patch set depend on colo-compare.
>
> Interesting, I thought it can co-work with comparing, and the 
> communication were done through socket now. So I think technically the 
> filter itself could work without COLO?

Filter-rewriter can run without COLO, but it no sense for other people.
rewriter need TX tcp packet differ with RX tcp packet...
In normal qemu we can't get the environment same with COLO.


>
>>> It will rewrite some of secondary packet to make
>>> secondary guest's connection established successfully.
>>> In this module we will rewrite tcp packet's ack to the secondary
>>> from primary,and rewrite tcp packet's seq to the primary from
>>> secondary.
>>>
>>> v3:
>>>    - fix typo
>>>    - add conn->syn_flag
>>>    - add some comments
>>>    - remove conn_list_lock
>>>    - fix offset set bug
>
> I think you can remove RFC in the next version so we can try to merge 
> it after one or several other iterations.

OK~~ I will remove RFC in colo-compare and filter-rewriter.

Thanks for your review~~
Zhang Chen

>
> Thanks
>
>>>
>>> v2:
>>>    - add more comments about packet flows
>>>    - add some trace-event
>>>    - add seq offset ( = secondary_seq - primary_seq)
>>>
>>> v1:
>>>    - initial patch
>>>
>>> Zhang Chen (3):
>>>    filter-rewriter: introduce filter-rewriter initialization
>>>    filter-rewriter: track connection and parse packet
>>>    filter-rewriter: rewrite tcp packet to keep secondary connection
>>>
>>>   net/Makefile.objs     |   1 +
>>>   net/colo-base.c       |   2 +
>>>   net/colo-base.h       |   7 ++
>>>   net/filter-rewriter.c | 262 
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   qemu-options.hx       |  13 +++
>>>   trace-events          |   5 +
>>>   vl.c                  |   3 +-
>>>   7 files changed, 292 insertions(+), 1 deletion(-)
>>>   create mode 100644 net/filter-rewriter.c
>>>
>>
>
>
>
> .
>

-- 
Thanks
zhangchen

Re: [Qemu-devel] [RFC PATCH V3 1/3] filter-rewriter: introduce filter-rewriter initialization

[Jason Wang]

On 2016年07月05日 18:29, Zhang Chen wrote:
> Filter-rewriter is a part of COLO project.
> It will rewrite some of secondary packet to make
> secondary guest's tcp connection established successfully.
> In this module we will rewrite tcp packet's ack to the secondary
> from primary,and rewrite tcp packet's seq to the primary from
> secondary.
>
> usage:
>
> colo secondary:
> -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=rew0,netdev=hn0,queue=all
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>   net/Makefile.objs     |   1 +
>   net/filter-rewriter.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   qemu-options.hx       |  13 ++++++
>   vl.c                  |   3 +-
>   4 files changed, 124 insertions(+), 1 deletion(-)
>   create mode 100644 net/filter-rewriter.c
>
> diff --git a/net/Makefile.objs b/net/Makefile.objs
> index 119589f..645bd10 100644
> --- a/net/Makefile.objs
> +++ b/net/Makefile.objs
> @@ -18,3 +18,4 @@ common-obj-y += filter-buffer.o
>   common-obj-y += filter-mirror.o
>   common-obj-y += colo-compare.o
>   common-obj-y += colo-base.o
> +common-obj-y += filter-rewriter.o
> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
> new file mode 100644
> index 0000000..c4f2739
> --- /dev/null
> +++ b/net/filter-rewriter.c
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + * Copyright (c) 2016 FUJITSU LIMITED
> + * Copyright (c) 2016 Intel Corporation
> + *
> + * Author: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "net/colo-base.h"
> +#include "net/filter.h"
> +#include "net/net.h"
> +#include "qemu-common.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qapi-visit.h"
> +#include "qom/object.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/iov.h"
> +#include "net/checksum.h"
> +
> +#define FILTER_COLO_REWRITER(obj) \
> +    OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
> +
> +#define TYPE_FILTER_REWRITER "filter-rewriter"
> +
> +enum {
> +    PRIMARY = 0,
> +    SECONDARY,
> +};
> +
> +typedef struct RewriterState {
> +    NetFilterState parent_obj;
> +    /* connection list: the connections belonged to this NIC could be found
> +     * in this list.
> +     * element type: Connection
> +     */
> +    GQueue conn_list;

The connection were either in conntection_track_table or 
unprocessed_connections, why still need this list?

> +    NetQueue *incoming_queue;
> +    /* hashtable to save connection */
> +    GHashTable *connection_track_table;
> +    /* to save unprocessed_connections */
> +    GQueue unprocessed_connections;

Unused in this series.

> +    /* current hash size */
> +    uint32_t hashtable_size;
> +} RewriterState;
> +
> +static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
> +                                         NetClientState *sender,
> +                                         unsigned flags,
> +                                         const struct iovec *iov,
> +                                         int iovcnt,
> +                                         NetPacketSent *sent_cb)
> +{
> +    /*
> +     * if we get tcp packet
> +     * we will rewrite it to make secondary guest's
> +     * connection established successfully
> +     */
> +    return 0;
> +}
> +
> +static void colo_rewriter_cleanup(NetFilterState *nf)
> +{
> +    RewriterState *s = FILTER_COLO_REWRITER(nf);
> +
> +    g_queue_free(&s->conn_list);
> +}
> +
> +static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
> +{
> +    RewriterState *s = FILTER_COLO_REWRITER(nf);
> +
> +    g_queue_init(&s->conn_list);
> +    s->hashtable_size = 0;
> +
> +    s->connection_track_table = g_hash_table_new_full(connection_key_hash,
> +                                                      connection_key_equal,
> +                                                      g_free,
> +                                                      connection_destroy);
> +    s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
> +}
> +
> +static void colo_rewriter_class_init(ObjectClass *oc, void *data)
> +{
> +    NetFilterClass *nfc = NETFILTER_CLASS(oc);
> +
> +    nfc->setup = colo_rewriter_setup;
> +    nfc->cleanup = colo_rewriter_cleanup;
> +    nfc->receive_iov = colo_rewriter_receive_iov;
> +}
> +
> +static const TypeInfo colo_rewriter_info = {
> +    .name = TYPE_FILTER_REWRITER,
> +    .parent = TYPE_NETFILTER,
> +    .class_init = colo_rewriter_class_init,
> +    .instance_size = sizeof(RewriterState),
> +};
> +
> +static void register_types(void)
> +{
> +    type_register_static(&colo_rewriter_info);
> +}
> +
> +type_init(register_types);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 14bade5..4afd511 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3859,6 +3859,19 @@ 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}]
> +
> +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.
> +
> +usage:
> +colo secondary:
> +-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=rew0,netdev=hn0,queue=all
> +
>   @item -object filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
>   
>   Dump the network traffic on netdev @var{dev} to the file specified by
> diff --git a/vl.c b/vl.c
> index c6b9a6f..b47be6a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2866,7 +2866,8 @@ static bool object_create_initial(const char *type)
>           g_str_equal(type, "filter-dump") ||
>           g_str_equal(type, "filter-mirror") ||
>           g_str_equal(type, "filter-redirector") ||
> -        g_str_equal(type, "colo-compare")) {
> +        g_str_equal(type, "colo-compare") ||
> +        g_str_equal(type, "filter-rewriter")) {
>           return false;
>       }
>   

Re: [Qemu-devel] [RFC PATCH V3 3/3] filter-rewriter: rewrite tcp packet to keep secondary connection

[Jason Wang]

On 2016年07月05日 18:29, Zhang Chen wrote:
> We will rewrite tcp packet secondary received and sent.
> When colo guest is a tcp server.
>
> Firstly, client start a tcp handshake. the packet's seq=client_seq,
> ack=0,flag=SYN. COLO primary guest get this pkt and mirror(filter-mirror)
> to secondary guest, secondary get it use filter-redirector.
> Then,primary guest response pkt
> (seq=primary_seq,ack=client_seq+1,flag=ACK|SYN).
> secondary guest response pkt
> (seq=secondary_seq,ack=client_seq+1,flag=ACK|SYN).
> In here,we use filter-rewriter save the secondary_seq to it's tcp connection.
> Finally handshake,client send pkt
> (seq=client_seq+1,ack=primary_seq+1,flag=ACK).
> Here,filter-rewriter can get primary_seq, and rewrite ack from primary_seq+1
> to secondary_seq+1, recalculate checksum. So the secondary tcp connection
> kept good.
>
> When we send/recv packet.
> client send pkt(seq=client_seq+1+data_len,ack=primary_seq+1,flag=ACK|PSH).
> filter-rewriter rewrite ack and send to secondary guest.
>
> primary guest response pkt
> (seq=primary_seq+1,ack=client_seq+1+data_len,flag=ACK)
> secondary guest response pkt
> (seq=secondary_seq+1,ack=client_seq+1+data_len,flag=ACK)
> we rewrite secondary guest seq from secondary_seq+1 to primary_seq+1.
> So tcp connection kept good.
>
> In code We use offset( = secondary_seq - primary_seq )
> to rewrite seq or ack.
> handle_primary_tcp_pkt: tcp_pkt->th_ack += offset;
> handle_secondary_tcp_pkt: tcp_pkt->th_seq -= offset;
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>   net/colo-base.c       |   2 +
>   net/colo-base.h       |   7 ++++
>   net/filter-rewriter.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   trace-events          |   5 +++
>   4 files changed, 120 insertions(+), 2 deletions(-)
>
> diff --git a/net/colo-base.c b/net/colo-base.c
> index 9673661..58fbd9d 100644
> --- a/net/colo-base.c
> +++ b/net/colo-base.c
> @@ -123,6 +123,8 @@ Connection *connection_new(ConnectionKey *key)
>   
>       conn->ip_proto = key->ip_proto;
>       conn->processing = false;
> +    conn->offset = 0;
> +    conn->syn_flag = 0;
>       g_queue_init(&conn->primary_list);
>       g_queue_init(&conn->secondary_list);
>   
> diff --git a/net/colo-base.h b/net/colo-base.h
> index 62460c5..353bd55 100644
> --- a/net/colo-base.h
> +++ b/net/colo-base.h
> @@ -71,6 +71,13 @@ typedef struct Connection {
>       uint8_t ip_proto;
>       /* be used by filter-rewriter */
>       colo_conn_state state;
> +    /* offset = secondary_seq - primary_seq */
> +    tcp_seq  offset;
> +    /*
> +     * we use this flag update offset func
> +     * run once in independent tcp connection
> +     */
> +    int syn_flag;
>   } Connection;
>   
>   uint32_t connection_key_hash(const void *opaque);
> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
> index 7f0da2c..f911f99 100644
> --- a/net/filter-rewriter.c
> +++ b/net/filter-rewriter.c
> @@ -21,6 +21,7 @@
>   #include "qemu/main-loop.h"
>   #include "qemu/iov.h"
>   #include "net/checksum.h"
> +#include "trace.h"
>   
>   #define FILTER_COLO_REWRITER(obj) \
>       OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
> @@ -62,6 +63,89 @@ static int is_tcp_packet(Packet *pkt)
>       }
>   }
>   
> +/* handle tcp packet from primary guest */
> +static int handle_primary_tcp_pkt(NetFilterState *nf,
> +                                  Connection *conn,
> +                                  Packet *pkt)
> +{
> +    struct tcphdr *tcp_pkt;
> +
> +    tcp_pkt = (struct tcphdr *)pkt->transport_layer;
> +    if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
> +        char *sdebug, *ddebug;
> +        sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
> +        ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
> +        trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
> +                    ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
> +                    tcp_pkt->th_flags);
> +        trace_colo_filter_rewriter_conn_offset(conn->offset);
> +        g_free(sdebug);
> +        g_free(ddebug);
> +    }
> +
> +    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
> +        /*
> +         * we use this flag update offset func
> +         * run once in independent tcp connection
> +         */
> +        conn->syn_flag = 1;
> +    }
> +
> +    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK)) {
> +        if (conn->syn_flag) {
> +            /* offset = secondary_seq - primary seq */
> +            conn->offset -= (ntohl(tcp_pkt->th_ack) - 1);

The code here is conflict with your comment above. Why need - 1 here?

> +            conn->syn_flag = 0;
> +        }
> +        /* handle packets to the secondary from the primary */
> +        tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset);
> +
> +        net_checksum_calculate((uint8_t *)pkt->data, pkt->size);
> +    }
> +
> +    return 0;
> +}
> +
> +/* handle tcp packet from secondary guest */
> +static int handle_secondary_tcp_pkt(NetFilterState *nf,
> +                                    Connection *conn,
> +                                    Packet *pkt)
> +{
> +    struct tcphdr *tcp_pkt;
> +
> +    tcp_pkt = (struct tcphdr *)pkt->transport_layer;
> +
> +    if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
> +        char *sdebug, *ddebug;
> +        sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
> +        ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
> +        trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
> +                    ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
> +                    tcp_pkt->th_flags);
> +        trace_colo_filter_rewriter_conn_offset(conn->offset);
> +        g_free(sdebug);
> +        g_free(ddebug);
> +    }
> +
> +    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) {
> +        /*
> +         * save offset = secondary_seq and then
> +         * in handle_primary_tcp_pkt make offset
> +         * = secondary_seq - primary_seq
> +         */
> +        conn->offset = ntohl(tcp_pkt->th_seq);
> +    }
> +
> +    if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) {
> +        /* handle packets to the primary from the secondary*/
> +        tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset);
> +
> +        net_checksum_calculate((uint8_t *)pkt->data, pkt->size);
> +    }
> +
> +    return 0;
> +}
> +
>   static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>                                            NetClientState *sender,
>                                            unsigned flags,
> @@ -101,10 +185,30 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>   
>           if (sender == nf->netdev) {
>               /* NET_FILTER_DIRECTION_TX */
> -            /* handle_primary_tcp_pkt */
> +            if (!handle_primary_tcp_pkt(nf, conn, pkt)) {
> +                qemu_net_queue_send(s->incoming_queue, sender, 0,
> +                (const uint8_t *)pkt->data, pkt->size, NULL);
> +                packet_destroy(pkt, NULL);
> +                pkt = NULL;
> +                /*
> +                 * We block the packet here,after rewrite pkt
> +                 * and will send it
> +                 */
> +                return 1;
> +            }
>           } else {
>               /* NET_FILTER_DIRECTION_RX */
> -            /* handle_secondary_tcp_pkt */
> +            if (!handle_secondary_tcp_pkt(nf, conn, pkt)) {
> +                qemu_net_queue_send(s->incoming_queue, sender, 0,
> +                (const uint8_t *)pkt->data, pkt->size, NULL);
> +                packet_destroy(pkt, NULL);
> +                pkt = NULL;
> +                /*
> +                 * We block the packet here,after rewrite pkt
> +                 * and will send it
> +                 */
> +                return 1;
> +            }
>           }
>       }
>   
> diff --git a/trace-events b/trace-events
> index 6686cdf..5ac56f6 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1927,3 +1927,8 @@ colo_compare_icmp_miscompare_mtu(const char *sta, int size) ": %s  %d"
>   colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
>   colo_old_packet_check_found(int64_t old_time) "%" PRId64
>   colo_compare_miscompare(void) ""
> +
> +# net/filter-rewriter.c
> +colo_filter_rewriter_debug(void) ""
> +colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=%x\n"
> +colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u\n"

Re: [Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Jason Wang]

On 2016年07月27日 13:49, Zhang Chen wrote:
>
>
> On 07/27/2016 10:50 AM, Jason Wang wrote:
>>
>>
>> On 2016年07月27日 10:32, Zhang Chen wrote:
>>> Hi~ all~~
>>>
>>> Anyone have some comments for this series?
>>
>> Will have a look at this.
>>
>> Btw. I wonder how this is tested, do we need a unit test for this?
>
> COLO-compare and filter-rewriter works normal depend on colo-frame.
> So, When colo-frame be merged I will send a colo-proxy unit test
> for all filter-mirror,filter-redirector,colo-compare,filter-rewriter.

Sounds cool.

>
>
>>
>>>
>>> We need more comments to improve COLO codes.
>>>
>>>
>>> Thanks
>>>
>>> Zhang Chen
>>>
>>>
>>> On 07/05/2016 06:29 PM, Zhang Chen wrote:
>>>> Filter-rewriter is a part of COLO project.
>>>> So this patch set depend on colo-compare.
>>
>> Interesting, I thought it can co-work with comparing, and the 
>> communication were done through socket now. So I think technically 
>> the filter itself could work without COLO?
>
> Filter-rewriter can run without COLO, but it no sense for other people.
> rewriter need TX tcp packet differ with RX tcp packet...
> In normal qemu we can't get the environment same with COLO.
>

Then I suggest to add this to the colo comparing series.

>
>>
>>>> It will rewrite some of secondary packet to make
>>>> secondary guest's connection established successfully.
>>>> In this module we will rewrite tcp packet's ack to the secondary
>>>> from primary,and rewrite tcp packet's seq to the primary from
>>>> secondary.
>>>>
>>>> v3:
>>>>    - fix typo
>>>>    - add conn->syn_flag
>>>>    - add some comments
>>>>    - remove conn_list_lock
>>>>    - fix offset set bug
>>
>> I think you can remove RFC in the next version so we can try to merge 
>> it after one or several other iterations.
>
> OK~~ I will remove RFC in colo-compare and filter-rewriter.
>
> Thanks for your review~~
> Zhang Chen
>
>>
>> Thanks
>>
>>>>
>>>> v2:
>>>>    - add more comments about packet flows
>>>>    - add some trace-event
>>>>    - add seq offset ( = secondary_seq - primary_seq)
>>>>
>>>> v1:
>>>>    - initial patch
>>>>
>>>> Zhang Chen (3):
>>>>    filter-rewriter: introduce filter-rewriter initialization
>>>>    filter-rewriter: track connection and parse packet
>>>>    filter-rewriter: rewrite tcp packet to keep secondary connection
>>>>
>>>>   net/Makefile.objs     |   1 +
>>>>   net/colo-base.c       |   2 +
>>>>   net/colo-base.h       |   7 ++
>>>>   net/filter-rewriter.c | 262 
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   qemu-options.hx       |  13 +++
>>>>   trace-events          |   5 +
>>>>   vl.c                  |   3 +-
>>>>   7 files changed, 292 insertions(+), 1 deletion(-)
>>>>   create mode 100644 net/filter-rewriter.c
>>>>
>>>
>>
>>
>>
>> .
>>
>

Re: [Qemu-devel] [RFC PATCH V3 1/3] filter-rewriter: introduce filter-rewriter initialization

[Zhang Chen]

On 07/27/2016 03:02 PM, Jason Wang wrote:
>
>
> On 2016年07月05日 18:29, Zhang Chen wrote:
>> Filter-rewriter is a part of COLO project.
>> It will rewrite some of secondary packet to make
>> secondary guest's tcp connection established successfully.
>> In this module we will rewrite tcp packet's ack to the secondary
>> from primary,and rewrite tcp packet's seq to the primary from
>> secondary.
>>
>> usage:
>>
>> colo secondary:
>> -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=rew0,netdev=hn0,queue=all
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>   net/Makefile.objs     |   1 +
>>   net/filter-rewriter.c | 108 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-options.hx       |  13 ++++++
>>   vl.c                  |   3 +-
>>   4 files changed, 124 insertions(+), 1 deletion(-)
>>   create mode 100644 net/filter-rewriter.c
>>
>> diff --git a/net/Makefile.objs b/net/Makefile.objs
>> index 119589f..645bd10 100644
>> --- a/net/Makefile.objs
>> +++ b/net/Makefile.objs
>> @@ -18,3 +18,4 @@ common-obj-y += filter-buffer.o
>>   common-obj-y += filter-mirror.o
>>   common-obj-y += colo-compare.o
>>   common-obj-y += colo-base.o
>> +common-obj-y += filter-rewriter.o
>> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
>> new file mode 100644
>> index 0000000..c4f2739
>> --- /dev/null
>> +++ b/net/filter-rewriter.c
>> @@ -0,0 +1,108 @@
>> +/*
>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
>> + * Copyright (c) 2016 FUJITSU LIMITED
>> + * Copyright (c) 2016 Intel Corporation
>> + *
>> + * Author: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> + * later.  See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "net/colo-base.h"
>> +#include "net/filter.h"
>> +#include "net/net.h"
>> +#include "qemu-common.h"
>> +#include "qapi/error.h"
>> +#include "qapi/qmp/qerror.h"
>> +#include "qapi-visit.h"
>> +#include "qom/object.h"
>> +#include "qemu/main-loop.h"
>> +#include "qemu/iov.h"
>> +#include "net/checksum.h"
>> +
>> +#define FILTER_COLO_REWRITER(obj) \
>> +    OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
>> +
>> +#define TYPE_FILTER_REWRITER "filter-rewriter"
>> +
>> +enum {
>> +    PRIMARY = 0,
>> +    SECONDARY,
>> +};
>> +
>> +typedef struct RewriterState {
>> +    NetFilterState parent_obj;
>> +    /* connection list: the connections belonged to this NIC could 
>> be found
>> +     * in this list.
>> +     * element type: Connection
>> +     */
>> +    GQueue conn_list;
>
> The connection were either in conntection_track_table or 
> unprocessed_connections, why still need this list?

I will remove it in next version,filter-rewriter needn't flush packet 
before checkpoint.

>
>> +    NetQueue *incoming_queue;
>> +    /* hashtable to save connection */
>> +    GHashTable *connection_track_table;
>> +    /* to save unprocessed_connections */
>> +    GQueue unprocessed_connections;
>
> Unused in this series.

Yes,I will remove it.

Thanks
Zhang Chen

>
>> +    /* current hash size */
>> +    uint32_t hashtable_size;
>> +} RewriterState;
>> +
>> +static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>> +                                         NetClientState *sender,
>> +                                         unsigned flags,
>> +                                         const struct iovec *iov,
>> +                                         int iovcnt,
>> +                                         NetPacketSent *sent_cb)
>> +{
>> +    /*
>> +     * if we get tcp packet
>> +     * we will rewrite it to make secondary guest's
>> +     * connection established successfully
>> +     */
>> +    return 0;
>> +}
>> +
>> +static void colo_rewriter_cleanup(NetFilterState *nf)
>> +{
>> +    RewriterState *s = FILTER_COLO_REWRITER(nf);
>> +
>> +    g_queue_free(&s->conn_list);
>> +}
>> +
>> +static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
>> +{
>> +    RewriterState *s = FILTER_COLO_REWRITER(nf);
>> +
>> +    g_queue_init(&s->conn_list);
>> +    s->hashtable_size = 0;
>> +
>> +    s->connection_track_table = 
>> g_hash_table_new_full(connection_key_hash,
>> + connection_key_equal,
>> +                                                      g_free,
>> + connection_destroy);
>> +    s->incoming_queue = 
>> qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
>> +}
>> +
>> +static void colo_rewriter_class_init(ObjectClass *oc, void *data)
>> +{
>> +    NetFilterClass *nfc = NETFILTER_CLASS(oc);
>> +
>> +    nfc->setup = colo_rewriter_setup;
>> +    nfc->cleanup = colo_rewriter_cleanup;
>> +    nfc->receive_iov = colo_rewriter_receive_iov;
>> +}
>> +
>> +static const TypeInfo colo_rewriter_info = {
>> +    .name = TYPE_FILTER_REWRITER,
>> +    .parent = TYPE_NETFILTER,
>> +    .class_init = colo_rewriter_class_init,
>> +    .instance_size = sizeof(RewriterState),
>> +};
>> +
>> +static void register_types(void)
>> +{
>> +    type_register_static(&colo_rewriter_info);
>> +}
>> +
>> +type_init(register_types);
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 14bade5..4afd511 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3859,6 +3859,19 @@ 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}]
>> +
>> +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.
>> +
>> +usage:
>> +colo secondary:
>> +-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=rew0,netdev=hn0,queue=all
>> +
>>   @item -object 
>> filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
>>     Dump the network traffic on netdev @var{dev} to the file 
>> specified by
>> diff --git a/vl.c b/vl.c
>> index c6b9a6f..b47be6a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2866,7 +2866,8 @@ static bool object_create_initial(const char 
>> *type)
>>           g_str_equal(type, "filter-dump") ||
>>           g_str_equal(type, "filter-mirror") ||
>>           g_str_equal(type, "filter-redirector") ||
>> -        g_str_equal(type, "colo-compare")) {
>> +        g_str_equal(type, "colo-compare") ||
>> +        g_str_equal(type, "filter-rewriter")) {
>>           return false;
>>       }
>
>
>
> .
>

-- 
Thanks
zhangchen

Re: [Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Zhang Chen]

On 07/27/2016 03:05 PM, Jason Wang wrote:
>
>
> On 2016年07月27日 13:49, Zhang Chen wrote:
>>
>>
>> On 07/27/2016 10:50 AM, Jason Wang wrote:
>>>
>>>
>>> On 2016年07月27日 10:32, Zhang Chen wrote:
>>>> Hi~ all~~
>>>>
>>>> Anyone have some comments for this series?
>>>
>>> Will have a look at this.
>>>
>>> Btw. I wonder how this is tested, do we need a unit test for this?
>>
>> COLO-compare and filter-rewriter works normal depend on colo-frame.
>> So, When colo-frame be merged I will send a colo-proxy unit test
>> for all filter-mirror,filter-redirector,colo-compare,filter-rewriter.
>
> Sounds cool.
>
>>
>>
>>>
>>>>
>>>> We need more comments to improve COLO codes.
>>>>
>>>>
>>>> Thanks
>>>>
>>>> Zhang Chen
>>>>
>>>>
>>>> On 07/05/2016 06:29 PM, Zhang Chen wrote:
>>>>> Filter-rewriter is a part of COLO project.
>>>>> So this patch set depend on colo-compare.
>>>
>>> Interesting, I thought it can co-work with comparing, and the 
>>> communication were done through socket now. So I think technically 
>>> the filter itself could work without COLO?
>>
>> Filter-rewriter can run without COLO, but it no sense for other people.
>> rewriter need TX tcp packet differ with RX tcp packet...
>> In normal qemu we can't get the environment same with COLO.
>>
>
> Then I suggest to add this to the colo comparing series.

OK~~
BTW, Should we make independent the "qemu-char" patch to compare series?
This patch just add a API in qemu-char.c ....

Thanks
Zhang Chen

>
>>
>>>
>>>>> It will rewrite some of secondary packet to make
>>>>> secondary guest's connection established successfully.
>>>>> In this module we will rewrite tcp packet's ack to the secondary
>>>>> from primary,and rewrite tcp packet's seq to the primary from
>>>>> secondary.
>>>>>
>>>>> v3:
>>>>>    - fix typo
>>>>>    - add conn->syn_flag
>>>>>    - add some comments
>>>>>    - remove conn_list_lock
>>>>>    - fix offset set bug
>>>
>>> I think you can remove RFC in the next version so we can try to 
>>> merge it after one or several other iterations.
>>
>> OK~~ I will remove RFC in colo-compare and filter-rewriter.
>>
>> Thanks for your review~~
>> Zhang Chen
>>
>>>
>>> Thanks
>>>
>>>>>
>>>>> v2:
>>>>>    - add more comments about packet flows
>>>>>    - add some trace-event
>>>>>    - add seq offset ( = secondary_seq - primary_seq)
>>>>>
>>>>> v1:
>>>>>    - initial patch
>>>>>
>>>>> Zhang Chen (3):
>>>>>    filter-rewriter: introduce filter-rewriter initialization
>>>>>    filter-rewriter: track connection and parse packet
>>>>>    filter-rewriter: rewrite tcp packet to keep secondary connection
>>>>>
>>>>>   net/Makefile.objs     |   1 +
>>>>>   net/colo-base.c       |   2 +
>>>>>   net/colo-base.h       |   7 ++
>>>>>   net/filter-rewriter.c | 262 
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>   qemu-options.hx       |  13 +++
>>>>>   trace-events          |   5 +
>>>>>   vl.c                  |   3 +-
>>>>>   7 files changed, 292 insertions(+), 1 deletion(-)
>>>>>   create mode 100644 net/filter-rewriter.c
>>>>>
>>>>
>>>
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>

-- 
Thanks
zhangchen

Re: [Qemu-devel] [RFC PATCH V3 3/3] filter-rewriter: rewrite tcp packet to keep secondary connection

[Zhang Chen]

On 07/27/2016 03:03 PM, Jason Wang wrote:
>
>
> On 2016年07月05日 18:29, Zhang Chen wrote:
>> We will rewrite tcp packet secondary received and sent.
>> When colo guest is a tcp server.
>>
>> Firstly, client start a tcp handshake. the packet's seq=client_seq,
>> ack=0,flag=SYN. COLO primary guest get this pkt and 
>> mirror(filter-mirror)
>> to secondary guest, secondary get it use filter-redirector.
>> Then,primary guest response pkt
>> (seq=primary_seq,ack=client_seq+1,flag=ACK|SYN).
>> secondary guest response pkt
>> (seq=secondary_seq,ack=client_seq+1,flag=ACK|SYN).
>> In here,we use filter-rewriter save the secondary_seq to it's tcp 
>> connection.
>> Finally handshake,client send pkt
>> (seq=client_seq+1,ack=primary_seq+1,flag=ACK).
>> Here,filter-rewriter can get primary_seq, and rewrite ack from 
>> primary_seq+1
>> to secondary_seq+1, recalculate checksum. So the secondary tcp 
>> connection
>> kept good.
>>
>> When we send/recv packet.
>> client send 
>> pkt(seq=client_seq+1+data_len,ack=primary_seq+1,flag=ACK|PSH).
>> filter-rewriter rewrite ack and send to secondary guest.
>>
>> primary guest response pkt
>> (seq=primary_seq+1,ack=client_seq+1+data_len,flag=ACK)
>> secondary guest response pkt
>> (seq=secondary_seq+1,ack=client_seq+1+data_len,flag=ACK)
>> we rewrite secondary guest seq from secondary_seq+1 to primary_seq+1.
>> So tcp connection kept good.
>>
>> In code We use offset( = secondary_seq - primary_seq )
>> to rewrite seq or ack.
>> handle_primary_tcp_pkt: tcp_pkt->th_ack += offset;
>> handle_secondary_tcp_pkt: tcp_pkt->th_seq -= offset;
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>   net/colo-base.c       |   2 +
>>   net/colo-base.h       |   7 ++++
>>   net/filter-rewriter.c | 108 
>> +++++++++++++++++++++++++++++++++++++++++++++++++-
>>   trace-events          |   5 +++
>>   4 files changed, 120 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/colo-base.c b/net/colo-base.c
>> index 9673661..58fbd9d 100644
>> --- a/net/colo-base.c
>> +++ b/net/colo-base.c
>> @@ -123,6 +123,8 @@ Connection *connection_new(ConnectionKey *key)
>>         conn->ip_proto = key->ip_proto;
>>       conn->processing = false;
>> +    conn->offset = 0;
>> +    conn->syn_flag = 0;
>>       g_queue_init(&conn->primary_list);
>>       g_queue_init(&conn->secondary_list);
>>   diff --git a/net/colo-base.h b/net/colo-base.h
>> index 62460c5..353bd55 100644
>> --- a/net/colo-base.h
>> +++ b/net/colo-base.h
>> @@ -71,6 +71,13 @@ typedef struct Connection {
>>       uint8_t ip_proto;
>>       /* be used by filter-rewriter */
>>       colo_conn_state state;
>> +    /* offset = secondary_seq - primary_seq */
>> +    tcp_seq  offset;
>> +    /*
>> +     * we use this flag update offset func
>> +     * run once in independent tcp connection
>> +     */
>> +    int syn_flag;
>>   } Connection;
>>     uint32_t connection_key_hash(const void *opaque);
>> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
>> index 7f0da2c..f911f99 100644
>> --- a/net/filter-rewriter.c
>> +++ b/net/filter-rewriter.c
>> @@ -21,6 +21,7 @@
>>   #include "qemu/main-loop.h"
>>   #include "qemu/iov.h"
>>   #include "net/checksum.h"
>> +#include "trace.h"
>>     #define FILTER_COLO_REWRITER(obj) \
>>       OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
>> @@ -62,6 +63,89 @@ static int is_tcp_packet(Packet *pkt)
>>       }
>>   }
>>   +/* handle tcp packet from primary guest */
>> +static int handle_primary_tcp_pkt(NetFilterState *nf,
>> +                                  Connection *conn,
>> +                                  Packet *pkt)
>> +{
>> +    struct tcphdr *tcp_pkt;
>> +
>> +    tcp_pkt = (struct tcphdr *)pkt->transport_layer;
>> +    if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
>> +        char *sdebug, *ddebug;
>> +        sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
>> +        ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
>> +        trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
>> +                    ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
>> +                    tcp_pkt->th_flags);
>> + trace_colo_filter_rewriter_conn_offset(conn->offset);
>> +        g_free(sdebug);
>> +        g_free(ddebug);
>> +    }
>> +
>> +    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
>> +        /*
>> +         * we use this flag update offset func
>> +         * run once in independent tcp connection
>> +         */
>> +        conn->syn_flag = 1;
>> +    }
>> +
>> +    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK)) {
>> +        if (conn->syn_flag) {
>> +            /* offset = secondary_seq - primary seq */
>> +            conn->offset -= (ntohl(tcp_pkt->th_ack) - 1);
>
> The code here is conflict with your comment above. Why need - 1 here?

(ntohl(tcp_pkt->th_ack) - 1) is the primary seq
So, we need -1 here.

Thanks
Zhang Chen

>
>> +            conn->syn_flag = 0;
>> +        }
>> +        /* handle packets to the secondary from the primary */
>> +        tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset);
>> +
>> +        net_checksum_calculate((uint8_t *)pkt->data, pkt->size);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/* handle tcp packet from secondary guest */
>> +static int handle_secondary_tcp_pkt(NetFilterState *nf,
>> +                                    Connection *conn,
>> +                                    Packet *pkt)
>> +{
>> +    struct tcphdr *tcp_pkt;
>> +
>> +    tcp_pkt = (struct tcphdr *)pkt->transport_layer;
>> +
>> +    if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
>> +        char *sdebug, *ddebug;
>> +        sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
>> +        ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
>> +        trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
>> +                    ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
>> +                    tcp_pkt->th_flags);
>> + trace_colo_filter_rewriter_conn_offset(conn->offset);
>> +        g_free(sdebug);
>> +        g_free(ddebug);
>> +    }
>> +
>> +    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | 
>> TH_SYN))) {
>> +        /*
>> +         * save offset = secondary_seq and then
>> +         * in handle_primary_tcp_pkt make offset
>> +         * = secondary_seq - primary_seq
>> +         */
>> +        conn->offset = ntohl(tcp_pkt->th_seq);
>> +    }
>> +
>> +    if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) {
>> +        /* handle packets to the primary from the secondary*/
>> +        tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset);
>> +
>> +        net_checksum_calculate((uint8_t *)pkt->data, pkt->size);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>>                                            NetClientState *sender,
>>                                            unsigned flags,
>> @@ -101,10 +185,30 @@ static ssize_t 
>> colo_rewriter_receive_iov(NetFilterState *nf,
>>             if (sender == nf->netdev) {
>>               /* NET_FILTER_DIRECTION_TX */
>> -            /* handle_primary_tcp_pkt */
>> +            if (!handle_primary_tcp_pkt(nf, conn, pkt)) {
>> +                qemu_net_queue_send(s->incoming_queue, sender, 0,
>> +                (const uint8_t *)pkt->data, pkt->size, NULL);
>> +                packet_destroy(pkt, NULL);
>> +                pkt = NULL;
>> +                /*
>> +                 * We block the packet here,after rewrite pkt
>> +                 * and will send it
>> +                 */
>> +                return 1;
>> +            }
>>           } else {
>>               /* NET_FILTER_DIRECTION_RX */
>> -            /* handle_secondary_tcp_pkt */
>> +            if (!handle_secondary_tcp_pkt(nf, conn, pkt)) {
>> +                qemu_net_queue_send(s->incoming_queue, sender, 0,
>> +                (const uint8_t *)pkt->data, pkt->size, NULL);
>> +                packet_destroy(pkt, NULL);
>> +                pkt = NULL;
>> +                /*
>> +                 * We block the packet here,after rewrite pkt
>> +                 * and will send it
>> +                 */
>> +                return 1;
>> +            }
>>           }
>>       }
>>   diff --git a/trace-events b/trace-events
>> index 6686cdf..5ac56f6 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -1927,3 +1927,8 @@ colo_compare_icmp_miscompare_mtu(const char 
>> *sta, int size) ": %s  %d"
>>   colo_compare_ip_info(int psize, const char *sta, const char *stb, 
>> int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src 
>> = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
>>   colo_old_packet_check_found(int64_t old_time) "%" PRId64
>>   colo_compare_miscompare(void) ""
>> +
>> +# net/filter-rewriter.c
>> +colo_filter_rewriter_debug(void) ""
>> +colo_filter_rewriter_pkt_info(const char *func, const char *src, 
>> const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: 
>> src/dst: %s/%s p: seq/ack=%u/%u  flags=%x\n"
>> +colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u\n"
>
>
>
> .
>

-- 
Thanks
zhangchen

Re: [Qemu-devel] [RFC PATCH V3 0/3] filter-rewriter: introduce filter-rewriter

[Jason Wang]

On 2016年07月27日 15:52, Zhang Chen wrote:
>
>
> On 07/27/2016 03:05 PM, Jason Wang wrote:
>>
>>
>> On 2016年07月27日 13:49, Zhang Chen wrote:
>>>
>>>
>>> On 07/27/2016 10:50 AM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2016年07月27日 10:32, Zhang Chen wrote:
>>>>> Hi~ all~~
>>>>>
>>>>> Anyone have some comments for this series?
>>>>
>>>> Will have a look at this.
>>>>
>>>> Btw. I wonder how this is tested, do we need a unit test for this?
>>>
>>> COLO-compare and filter-rewriter works normal depend on colo-frame.
>>> So, When colo-frame be merged I will send a colo-proxy unit test
>>> for all filter-mirror,filter-redirector,colo-compare,filter-rewriter.
>>
>> Sounds cool.
>>
>>>
>>>
>>>>
>>>>>
>>>>> We need more comments to improve COLO codes.
>>>>>
>>>>>
>>>>> Thanks
>>>>>
>>>>> Zhang Chen
>>>>>
>>>>>
>>>>> On 07/05/2016 06:29 PM, Zhang Chen wrote:
>>>>>> Filter-rewriter is a part of COLO project.
>>>>>> So this patch set depend on colo-compare.
>>>>
>>>> Interesting, I thought it can co-work with comparing, and the 
>>>> communication were done through socket now. So I think technically 
>>>> the filter itself could work without COLO?
>>>
>>> Filter-rewriter can run without COLO, but it no sense for other people.
>>> rewriter need TX tcp packet differ with RX tcp packet...
>>> In normal qemu we can't get the environment same with COLO.
>>>
>>
>> Then I suggest to add this to the colo comparing series.
>
> OK~~
> BTW, Should we make independent the "qemu-char" patch to compare series?
> This patch just add a API in qemu-char.c ....
>
> Thanks
> Zhang Chen 

I think yes, and better make it patch 1.

Thanks

Re: [Qemu-devel] [RFC PATCH V3 3/3] filter-rewriter: rewrite tcp packet to keep secondary connection

[Jason Wang]

On 2016年07月27日 17:05, Zhang Chen wrote:
>>> +    if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK)) {
>>> +        if (conn->syn_flag) {
>>> +            /* offset = secondary_seq - primary seq */
>>> +            conn->offset -= (ntohl(tcp_pkt->th_ack) - 1);
>>
>> The code here is conflict with your comment above. Why need - 1 here?
>
> (ntohl(tcp_pkt->th_ack) - 1) is the primary seq
> So, we need -1 here.
>
> Thanks
> Zhang Chen

Aha, right. Since we can't get the syn,ack packet sent by guest from 
primary node.

Better add a comment to explain this.

Thanks