[PATCH V7 0/6] Passthrough specific network traffic in COLO

[PATCH V7 0/6] Passthrough specific network traffic in COLO

[Zhang Chen]

Due to some real user scenarios don't need to monitor all traffic.
And qemu net-filter also need function to more detailed flow control.
This series give user ability to passthrough kinds of COLO network stream.

For example, windows guest user want to enable windows remote desktop
to touch guest(UDP/TCP 3389), This case use UDP and TCP mixed, and the
tcp part payload always different caused by real desktop display
data(for guest time/ mouse display....).

Another case is some real user application will actively transmit information
include guest time part, primary guest send data with time 10:01.000,
At the same time secondary guest send data with time 10:01.001,
it will always trigger COLO checkpoint(live migrate) to drop guest performance.

  V7:
    - Keep some data structure stay in .c (patch 4/6).
    - Fix mutex init issue (patch 5/6).
    - Make the IPFlowSpec 'protocol' field optional (patch 1/6).
    - Add compare_passthrough_find function in net.c (patch 6/6).

  V6:
    - Change QAPI IPFlowSpec protocol from enum to str.
    - Use getprotobyname to handle the protocols.
    - Optimize code in net.

  V5:
    - Squash original 1-3 QAPI patches together.
    - Rename some data structures to avoid misunderstanding.
    - Reuse InetSocketAddressBase in IPFlowSpec.
    - Add new function in util/qemu-sockets.c to parse
      InetSocketAddressBase.
    - Update HMP command define to reuse current code.
    - Add more comments.

  V4:
    - Fix QAPI code conflict for V6.0 merged patches.
    - Note this feature for V6.1.

  V3:
    - Add COLO passthrough list lock.
    - Add usage demo and more comments.

  V2:
    - Add the n-tuple support.
    - Add some qapi definitions.
    - Support multi colo-compare objects.
    - Support setup each rules for each objects individually.
    - Clean up COLO compare definition to .h file.
    - Rebase HMP command for stable tree.
    - Add redundant rules check.


Zhang Chen (6):
  qapi/net: Add IPFlowSpec and QMP command for COLO passthrough
  util/qemu-sockets.c: Add inet_parse_base to handle
    InetSocketAddressBase
  hmp-commands: Add new HMP command for COLO passthrough
  net/colo-compare: Move data structure and define to .h file.
  net/colo-compare: Add passthrough list to CompareState
  net/net.c: Add handler for COLO passthrough connection

 hmp-commands.hx        |  26 +++++++
 include/monitor/hmp.h  |   2 +
 include/qemu/sockets.h |   1 +
 monitor/hmp-cmds.c     |  82 ++++++++++++++++++++
 net/colo-compare.c     | 160 +++++++++++++--------------------------
 net/colo-compare.h     |  98 ++++++++++++++++++++++++
 net/net.c              | 168 +++++++++++++++++++++++++++++++++++++++++
 qapi/net.json          |  68 +++++++++++++++++
 util/qemu-sockets.c    |  14 ++++
 9 files changed, 510 insertions(+), 109 deletions(-)

-- 
2.25.1

[PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough

[Zhang Chen]

Since the real user scenario does not need COLO to monitor all traffic.
Add colo-passthrough-add and colo-passthrough-del to maintain
a COLO network passthrough list. Add IPFlowSpec struct for all QMP commands.
Except protocol field is necessary, other fields are optional.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/net.c     | 10 ++++++++
 qapi/net.json | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/net/net.c b/net/net.c
index 2a472604ec..a4bee86b24 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1194,6 +1194,16 @@ void qmp_netdev_del(const char *id, Error **errp)
     }
 }
 
+void qmp_colo_passthrough_add(IPFlowSpec *spec, Error **errp)
+{
+    /* TODO implement setup passthrough rule */
+}
+
+void qmp_colo_passthrough_del(IPFlowSpec *spec, Error **errp)
+{
+    /* TODO implement delete passthrough rule */
+}
+
 static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
 {
     char *str;
diff --git a/qapi/net.json b/qapi/net.json
index af3f5b0fda..aca3b54278 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -7,6 +7,7 @@
 ##
 
 { 'include': 'common.json' }
+{ 'include': 'sockets.json' }
 
 ##
 # @set_link:
@@ -694,3 +695,70 @@
 ##
 { 'event': 'FAILOVER_NEGOTIATED',
   'data': {'device-id': 'str'} }
+
+##
+# @IPFlowSpec:
+#
+# IP flow specification.
+#
+# @protocol: Transport layer protocol like TCP/UDP...
+#
+# @object-name: Point out the IPflow spec effective range of object,
+#               If there is no such part, it means global spec.
+#
+# @source: Source address and port.
+#
+# @destination: Destination address and port.
+#
+# Since: 6.1
+##
+{ 'struct': 'IPFlowSpec',
+  'data': { '*protocol': 'str', '*object-name': 'str',
+    '*source': 'InetSocketAddressBase',
+    '*destination': 'InetSocketAddressBase' } }
+
+##
+# @colo-passthrough-add:
+#
+# Add passthrough entry according to user's needs in COLO-compare.
+# Source IP/port and destination IP/port both optional, If user just
+# input parts of infotmation, it will match all.
+#
+# Returns: Nothing on success
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute": "colo-passthrough-add",
+#      "arguments": { "protocol": "tcp", "object-name": "object0",
+#      "source": {"host": "192.168.1.1", "port": "1234"},
+#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'colo-passthrough-add', 'boxed': true,
+     'data': 'IPFlowSpec' }
+
+##
+# @colo-passthrough-del:
+#
+# Delete passthrough entry according to user's needs in COLO-compare.
+# Source IP/port and destination IP/port both optional, If user just
+# input parts of infotmation, it will match all.
+#
+# Returns: Nothing on success
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute": "colo-passthrough-del",
+#      "arguments": { "protocol": "tcp", "object-name": "object0",
+#      "source": {"host": "192.168.1.1", "port": "1234"},
+#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'colo-passthrough-del', 'boxed': true,
+     'data': 'IPFlowSpec' }
-- 
2.25.1

[PATCH V7 2/6] util/qemu-sockets.c: Add inet_parse_base to handle InetSocketAddressBase

[Zhang Chen]

No need to carry the flag all the time in many scenarios.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 include/qemu/sockets.h |  1 +
 util/qemu-sockets.c    | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 7d1f813576..d5abc227eb 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -32,6 +32,7 @@ int socket_set_fast_reuse(int fd);
 int inet_ai_family_from_address(InetSocketAddress *addr,
                                 Error **errp);
 int inet_parse(InetSocketAddress *addr, const char *str, Error **errp);
+int inet_parse_base(InetSocketAddressBase *addr, const char *str, Error **errp);
 int inet_connect(const char *str, Error **errp);
 int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
 
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 8af0278f15..c16eba1917 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -690,6 +690,20 @@ int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
     return 0;
 }
 
+int inet_parse_base(InetSocketAddressBase *base, const char *str, Error **errp)
+{
+    InetSocketAddress *addr;
+    int ret = 0;
+
+    addr = g_new0(InetSocketAddress, 1);
+    ret = inet_parse(addr, str, errp);
+
+    base->host = addr->host;
+    base->port = addr->port;
+
+    g_free(addr);
+    return ret;
+}
 
 /**
  * Create a blocking socket and connect it to an address.
-- 
2.25.1

[PATCH V7 3/6] hmp-commands: Add new HMP command for COLO passthrough

[Zhang Chen]

Add hmp_colo_passthrough_add and hmp_colo_passthrough_del make user
can maintain COLO network passthrough list in human monitor

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 hmp-commands.hx       | 26 ++++++++++++++
 include/monitor/hmp.h |  2 ++
 monitor/hmp-cmds.c    | 82 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 435c591a1c..cbb08623c7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1290,6 +1290,32 @@ SRST
   Remove host network device.
 ERST
 
+    {
+        .name       = "colo_passthrough_add",
+        .args_type  = "protocol:s,object-name:s?,src:s?,dst:s?",
+        .params     = "protocol [object-name] [src] [dst]",
+        .help       = "Add network stream to colo passthrough list",
+        .cmd        = hmp_colo_passthrough_add,
+    },
+
+SRST
+``colo_passthrough_add``
+  Add network stream to colo passthrough list.
+ERST
+
+    {
+        .name       = "colo_passthrough_del",
+        .args_type  = "protocol:s,object-name:s?,src:s?,dst:s?",
+        .params     = "protocol [object-name] [src] [dst]",
+        .help       = "Delete network stream from colo passthrough list",
+        .cmd        = hmp_colo_passthrough_del,
+    },
+
+SRST
+``colo_passthrough_del``
+  Delete network stream from colo passthrough list.
+ERST
+
     {
         .name       = "object_add",
         .args_type  = "object:S",
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 605d57287a..a784f98531 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -77,6 +77,8 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_colo_passthrough_add(Monitor *mon, const QDict *qdict);
+void hmp_colo_passthrough_del(Monitor *mon, const QDict *qdict);
 void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_sendkey(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index d9bef63373..01b467e306 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1634,6 +1634,88 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);
 }
 
+void hmp_colo_passthrough_add(Monitor *mon, const QDict *qdict)
+{
+    IPFlowSpec *spec = g_new0(IPFlowSpec, 1);
+    char *src, *dst;
+    Error *err = NULL;
+
+    spec->protocol = g_strdup(qdict_get_try_str(qdict, "protocol"));
+    spec->object_name = g_strdup(qdict_get_try_str(qdict, "object-name"));
+
+    src = g_strdup(qdict_get_try_str(qdict, "src"));
+    if (src) {
+        spec->source = g_new0(InetSocketAddressBase, 1);
+
+        if (inet_parse_base(spec->source, src, NULL)) {
+            monitor_printf(mon, "bad colo passthrough src address");
+            goto out;
+        }
+    }
+
+    dst = g_strdup(qdict_get_try_str(qdict, "dst"));
+    if (dst) {
+        spec->destination = g_new0(InetSocketAddressBase, 1);
+
+        if (inet_parse_base(spec->destination, dst, NULL)) {
+            monitor_printf(mon, "bad colo passthrough dst address");
+            goto out;
+        }
+    }
+
+    qmp_colo_passthrough_add(spec, &err);
+
+out:
+    g_free(src);
+    src = NULL;
+
+    g_free(dst);
+    dst = NULL;
+
+    hmp_handle_error(mon, err);
+}
+
+void hmp_colo_passthrough_del(Monitor *mon, const QDict *qdict)
+{
+    IPFlowSpec *spec = g_new0(IPFlowSpec, 1);
+    char *src, *dst;
+    Error *err = NULL;
+
+    spec->protocol = g_strdup(qdict_get_try_str(qdict, "protocol"));
+    spec->object_name = g_strdup(qdict_get_try_str(qdict, "object-name"));
+
+    src = g_strdup(qdict_get_try_str(qdict, "src"));
+    if (src) {
+        spec->source = g_new0(InetSocketAddressBase, 1);
+
+        if (inet_parse_base(spec->source, src, NULL)) {
+            monitor_printf(mon, "bad colo passthrough src address");
+            goto out;
+        }
+    }
+
+    dst = g_strdup(qdict_get_try_str(qdict, "dst"));
+    if (dst) {
+        spec->destination = g_new0(InetSocketAddressBase, 1);
+
+        if (inet_parse_base(spec->destination, dst, NULL)) {
+            monitor_printf(mon, "bad colo passthrough dst address");
+            goto out;
+        }
+    }
+
+    qmp_colo_passthrough_del(spec, &err);
+
+out:
+    g_free(src);
+    src = NULL;
+
+    g_free(dst);
+    dst = NULL;
+
+    hmp_handle_error(mon, err);
+}
+
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     const char *options = qdict_get_str(qdict, "object");
-- 
2.25.1

[PATCH V7 4/6] net/colo-compare: Move data structure and define to .h file.

[Zhang Chen]

Rename structure with COLO index and move it to .h file,
It make other modules can reuse COLO code.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/colo-compare.c | 132 ++++++++-------------------------------------
 net/colo-compare.h |  86 +++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 109 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 9d1ad99941..8175612c5f 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -17,29 +17,18 @@
 #include "qemu/error-report.h"
 #include "trace.h"
 #include "qapi/error.h"
-#include "net/net.h"
 #include "net/eth.h"
 #include "qom/object_interfaces.h"
 #include "qemu/iov.h"
 #include "qom/object.h"
 #include "net/queue.h"
-#include "chardev/char-fe.h"
 #include "qemu/sockets.h"
-#include "colo.h"
-#include "sysemu/iothread.h"
 #include "net/colo-compare.h"
-#include "migration/colo.h"
-#include "migration/migration.h"
 #include "util.h"
 
 #include "block/aio-wait.h"
 #include "qemu/coroutine.h"
 
-#define TYPE_COLO_COMPARE "colo-compare"
-typedef struct CompareState CompareState;
-DECLARE_INSTANCE_CHECKER(CompareState, COLO_COMPARE,
-                         TYPE_COLO_COMPARE)
-
 static QTAILQ_HEAD(, CompareState) net_compares =
        QTAILQ_HEAD_INITIALIZER(net_compares);
 
@@ -47,13 +36,13 @@ static NotifierList colo_compare_notifiers =
     NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
 
 #define COMPARE_READ_LEN_MAX NET_BUFSIZE
-#define MAX_QUEUE_SIZE 1024
+#define MAX_COLO_QUEUE_SIZE 1024
 
 #define COLO_COMPARE_FREE_PRIMARY     0x01
 #define COLO_COMPARE_FREE_SECONDARY   0x02
 
-#define REGULAR_PACKET_CHECK_MS 1000
-#define DEFAULT_TIME_OUT_MS 3000
+#define COLO_REGULAR_PACKET_CHECK_MS 1000
+#define COLO_DEFAULT_TIME_OUT_MS 3000
 
 /* #define DEBUG_COLO_PACKETS */
 
@@ -64,87 +53,6 @@ static QemuCond event_complete_cond;
 static int event_unhandled_count;
 static uint32_t max_queue_size;
 
-/*
- *  + CompareState ++
- *  |               |
- *  +---------------+   +---------------+         +---------------+
- *  |   conn list   + - >      conn     + ------- >      conn     + -- > ......
- *  +---------------+   +---------------+         +---------------+
- *  |               |     |           |             |          |
- *  +---------------+ +---v----+  +---v----+    +---v----+ +---v----+
- *                    |primary |  |secondary    |primary | |secondary
- *                    |packet  |  |packet  +    |packet  | |packet  +
- *                    +--------+  +--------+    +--------+ +--------+
- *                        |           |             |          |
- *                    +---v----+  +---v----+    +---v----+ +---v----+
- *                    |primary |  |secondary    |primary | |secondary
- *                    |packet  |  |packet  +    |packet  | |packet  +
- *                    +--------+  +--------+    +--------+ +--------+
- *                        |           |             |          |
- *                    +---v----+  +---v----+    +---v----+ +---v----+
- *                    |primary |  |secondary    |primary | |secondary
- *                    |packet  |  |packet  +    |packet  | |packet  +
- *                    +--------+  +--------+    +--------+ +--------+
- */
-
-typedef struct SendCo {
-    Coroutine *co;
-    struct CompareState *s;
-    CharBackend *chr;
-    GQueue send_list;
-    bool notify_remote_frame;
-    bool done;
-    int ret;
-} SendCo;
-
-typedef struct SendEntry {
-    uint32_t size;
-    uint32_t vnet_hdr_len;
-    uint8_t *buf;
-} SendEntry;
-
-struct CompareState {
-    Object parent;
-
-    char *pri_indev;
-    char *sec_indev;
-    char *outdev;
-    char *notify_dev;
-    CharBackend chr_pri_in;
-    CharBackend chr_sec_in;
-    CharBackend chr_out;
-    CharBackend chr_notify_dev;
-    SocketReadState pri_rs;
-    SocketReadState sec_rs;
-    SocketReadState notify_rs;
-    SendCo out_sendco;
-    SendCo notify_sendco;
-    bool vnet_hdr;
-    uint64_t compare_timeout;
-    uint32_t expired_scan_cycle;
-
-    /*
-     * Record the connection that through the NIC
-     * Element type: Connection
-     */
-    GQueue conn_list;
-    /* Record the connection without repetition */
-    GHashTable *connection_track_table;
-
-    IOThread *iothread;
-    GMainContext *worker_context;
-    QEMUTimer *packet_check_timer;
-
-    QEMUBH *event_bh;
-    enum colo_event event;
-
-    QTAILQ_ENTRY(CompareState) next;
-};
-
-typedef struct CompareClass {
-    ObjectClass parent_class;
-} CompareClass;
-
 enum {
     PRIMARY_IN = 0,
     SECONDARY_IN,
@@ -155,6 +63,12 @@ static const char *colo_mode[] = {
     [SECONDARY_IN] = "secondary",
 };
 
+typedef struct COLOSendEntry {
+    uint32_t size;
+    uint32_t vnet_hdr_len;
+    uint8_t *buf;
+} COLOSendEntry;
+
 static int compare_chr_send(CompareState *s,
                             uint8_t *buf,
                             uint32_t size,
@@ -737,19 +651,19 @@ static void colo_compare_connection(void *opaque, void *user_data)
 
 static void coroutine_fn _compare_chr_send(void *opaque)
 {
-    SendCo *sendco = opaque;
+    COLOSendCo *sendco = opaque;
     CompareState *s = sendco->s;
     int ret = 0;
 
     while (!g_queue_is_empty(&sendco->send_list)) {
-        SendEntry *entry = g_queue_pop_tail(&sendco->send_list);
+        COLOSendEntry *entry = g_queue_pop_tail(&sendco->send_list);
         uint32_t len = htonl(entry->size);
 
         ret = qemu_chr_fe_write_all(sendco->chr, (uint8_t *)&len, sizeof(len));
 
         if (ret != sizeof(len)) {
             g_free(entry->buf);
-            g_slice_free(SendEntry, entry);
+            g_slice_free(COLOSendEntry, entry);
             goto err;
         }
 
@@ -766,7 +680,7 @@ static void coroutine_fn _compare_chr_send(void *opaque)
 
             if (ret != sizeof(len)) {
                 g_free(entry->buf);
-                g_slice_free(SendEntry, entry);
+                g_slice_free(COLOSendEntry, entry);
                 goto err;
             }
         }
@@ -777,12 +691,12 @@ static void coroutine_fn _compare_chr_send(void *opaque)
 
         if (ret != entry->size) {
             g_free(entry->buf);
-            g_slice_free(SendEntry, entry);
+            g_slice_free(COLOSendEntry, entry);
             goto err;
         }
 
         g_free(entry->buf);
-        g_slice_free(SendEntry, entry);
+        g_slice_free(COLOSendEntry, entry);
     }
 
     sendco->ret = 0;
@@ -790,9 +704,9 @@ static void coroutine_fn _compare_chr_send(void *opaque)
 
 err:
     while (!g_queue_is_empty(&sendco->send_list)) {
-        SendEntry *entry = g_queue_pop_tail(&sendco->send_list);
+        COLOSendEntry *entry = g_queue_pop_tail(&sendco->send_list);
         g_free(entry->buf);
-        g_slice_free(SendEntry, entry);
+        g_slice_free(COLOSendEntry, entry);
     }
     sendco->ret = ret < 0 ? ret : -EIO;
 out:
@@ -808,8 +722,8 @@ static int compare_chr_send(CompareState *s,
                             bool notify_remote_frame,
                             bool zero_copy)
 {
-    SendCo *sendco;
-    SendEntry *entry;
+    COLOSendCo *sendco;
+    COLOSendEntry *entry;
 
     if (notify_remote_frame) {
         sendco = &s->notify_sendco;
@@ -821,7 +735,7 @@ static int compare_chr_send(CompareState *s,
         return 0;
     }
 
-    entry = g_slice_new(SendEntry);
+    entry = g_slice_new(COLOSendEntry);
     entry->size = size;
     entry->vnet_hdr_len = vnet_hdr_len;
     if (zero_copy) {
@@ -1274,17 +1188,17 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
 
     if (!s->compare_timeout) {
         /* Set default value to 3000 MS */
-        s->compare_timeout = DEFAULT_TIME_OUT_MS;
+        s->compare_timeout = COLO_DEFAULT_TIME_OUT_MS;
     }
 
     if (!s->expired_scan_cycle) {
         /* Set default value to 3000 MS */
-        s->expired_scan_cycle = REGULAR_PACKET_CHECK_MS;
+        s->expired_scan_cycle = COLO_REGULAR_PACKET_CHECK_MS;
     }
 
     if (!max_queue_size) {
         /* Set default queue size to 1024 */
-        max_queue_size = MAX_QUEUE_SIZE;
+        max_queue_size = MAX_COLO_QUEUE_SIZE;
     }
 
     if (find_and_check_chardev(&chr, s->pri_indev, errp) ||
diff --git a/net/colo-compare.h b/net/colo-compare.h
index 22ddd512e2..a481df8ce6 100644
--- a/net/colo-compare.h
+++ b/net/colo-compare.h
@@ -17,6 +17,92 @@
 #ifndef QEMU_COLO_COMPARE_H
 #define QEMU_COLO_COMPARE_H
 
+#include "net/net.h"
+#include "chardev/char-fe.h"
+#include "migration/colo.h"
+#include "migration/migration.h"
+#include "sysemu/iothread.h"
+#include "colo.h"
+
+#define TYPE_COLO_COMPARE "colo-compare"
+typedef struct CompareState CompareState;
+DECLARE_INSTANCE_CHECKER(CompareState, COLO_COMPARE,
+                         TYPE_COLO_COMPARE)
+
+typedef struct COLOSendCo {
+    Coroutine *co;
+    struct CompareState *s;
+    CharBackend *chr;
+    GQueue send_list;
+    bool notify_remote_frame;
+    bool done;
+    int ret;
+} COLOSendCo;
+
+/*
+ *  + CompareState ++
+ *  |               |
+ *  +---------------+   +---------------+         +---------------+
+ *  |   conn list   + - >      conn     + ------- >      conn     + -- > ......
+ *  +---------------+   +---------------+         +---------------+
+ *  |               |     |           |             |          |
+ *  +---------------+ +---v----+  +---v----+    +---v----+ +---v----+
+ *                    |primary |  |secondary    |primary | |secondary
+ *                    |packet  |  |packet  +    |packet  | |packet  +
+ *                    +--------+  +--------+    +--------+ +--------+
+ *                        |           |             |          |
+ *                    +---v----+  +---v----+    +---v----+ +---v----+
+ *                    |primary |  |secondary    |primary | |secondary
+ *                    |packet  |  |packet  +    |packet  | |packet  +
+ *                    +--------+  +--------+    +--------+ +--------+
+ *                        |           |             |          |
+ *                    +---v----+  +---v----+    +---v----+ +---v----+
+ *                    |primary |  |secondary    |primary | |secondary
+ *                    |packet  |  |packet  +    |packet  | |packet  +
+ *                    +--------+  +--------+    +--------+ +--------+
+ */
+struct CompareState {
+    Object parent;
+
+    char *pri_indev;
+    char *sec_indev;
+    char *outdev;
+    char *notify_dev;
+    CharBackend chr_pri_in;
+    CharBackend chr_sec_in;
+    CharBackend chr_out;
+    CharBackend chr_notify_dev;
+    SocketReadState pri_rs;
+    SocketReadState sec_rs;
+    SocketReadState notify_rs;
+    COLOSendCo out_sendco;
+    COLOSendCo notify_sendco;
+    bool vnet_hdr;
+    uint64_t compare_timeout;
+    uint32_t expired_scan_cycle;
+
+    /*
+     * Record the connection that through the NIC
+     * Element type: Connection
+     */
+    GQueue conn_list;
+    /* Record the connection without repetition */
+    GHashTable *connection_track_table;
+
+    IOThread *iothread;
+    GMainContext *worker_context;
+    QEMUTimer *packet_check_timer;
+
+    QEMUBH *event_bh;
+    enum colo_event event;
+
+    QTAILQ_ENTRY(CompareState) next;
+};
+
+typedef struct CompareClass {
+    ObjectClass parent_class;
+} CompareClass;
+
 void colo_notify_compares_event(void *opaque, int event, Error **errp);
 void colo_compare_register_notifier(Notifier *notify);
 void colo_compare_unregister_notifier(Notifier *notify);
-- 
2.25.1

[PATCH V7 5/6] net/colo-compare: Add passthrough list to CompareState

[Zhang Chen]

Add passthrough list for each CompareState.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/colo-compare.c | 28 ++++++++++++++++++++++++++++
 net/colo-compare.h | 12 ++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 8175612c5f..2250efd40d 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -161,6 +161,7 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
     ConnectionKey key;
     Packet *pkt = NULL;
     Connection *conn;
+    COLOPassthroughEntry *pass, *next;
     int ret;
 
     if (mode == PRIMARY_IN) {
@@ -180,6 +181,31 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
     }
     fill_connection_key(pkt, &key);
 
+    /* Check COLO passthrough specifications */
+    qemu_mutex_lock(&s->passthroughlist_mutex);
+    if (!QLIST_EMPTY(&s->passthroughlist)) {
+        QLIST_FOREACH_SAFE(pass, &s->passthroughlist, node, next) {
+            if (key.ip_proto == pass->l4_protocol->p_proto) {
+                if (pass->src_port == 0 || pass->src_port == key.dst_port) {
+                    if (pass->src_ip.s_addr == 0 ||
+                        pass->src_ip.s_addr == key.src.s_addr) {
+                        if (pass->dst_port == 0 ||
+                            pass->dst_port == key.src_port) {
+                            if (pass->dst_ip.s_addr == 0 ||
+                                pass->dst_ip.s_addr == key.dst.s_addr) {
+                                packet_destroy(pkt, NULL);
+                                pkt = NULL;
+                                qemu_mutex_unlock(&s->passthroughlist_mutex);
+                                return -1;
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+    qemu_mutex_unlock(&s->passthroughlist_mutex);
+
     conn = connection_get(s->connection_track_table,
                           &key,
                           &s->conn_list);
@@ -1245,6 +1271,7 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
     }
 
     g_queue_init(&s->conn_list);
+    QLIST_INIT(&s->passthroughlist);
 
     s->connection_track_table = g_hash_table_new_full(connection_key_hash,
                                                       connection_key_equal,
@@ -1259,6 +1286,7 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
         qemu_cond_init(&event_complete_cond);
         colo_compare_active = true;
     }
+    qemu_mutex_init(&s->passthroughlist_mutex);
     QTAILQ_INSERT_TAIL(&net_compares, s, next);
     qemu_mutex_unlock(&colo_compare_mutex);
 
diff --git a/net/colo-compare.h b/net/colo-compare.h
index a481df8ce6..1980df4487 100644
--- a/net/colo-compare.h
+++ b/net/colo-compare.h
@@ -23,6 +23,7 @@
 #include "migration/migration.h"
 #include "sysemu/iothread.h"
 #include "colo.h"
+#include <netdb.h>
 
 #define TYPE_COLO_COMPARE "colo-compare"
 typedef struct CompareState CompareState;
@@ -39,6 +40,15 @@ typedef struct COLOSendCo {
     int ret;
 } COLOSendCo;
 
+typedef struct COLOPassthroughEntry {
+    struct protoent *l4_protocol;
+    int src_port;
+    int dst_port;
+    struct in_addr src_ip;
+    struct in_addr dst_ip;
+    QLIST_ENTRY(COLOPassthroughEntry) node;
+} COLOPassthroughEntry;
+
 /*
  *  + CompareState ++
  *  |               |
@@ -95,6 +105,8 @@ struct CompareState {
 
     QEMUBH *event_bh;
     enum colo_event event;
+    QLIST_HEAD(, COLOPassthroughEntry) passthroughlist;
+    QemuMutex passthroughlist_mutex;
 
     QTAILQ_ENTRY(CompareState) next;
 };
-- 
2.25.1

[PATCH V7 6/6] net/net.c: Add handler for COLO passthrough connection

[Zhang Chen]

Use connection protocol,src port,dst port,src ip,dst ip as the key
to bypass certain network traffic in COLO compare.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/net.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 160 insertions(+), 2 deletions(-)

diff --git a/net/net.c b/net/net.c
index a4bee86b24..6cb0387199 100644
--- a/net/net.c
+++ b/net/net.c
@@ -54,6 +54,8 @@
 #include "sysemu/runstate.h"
 #include "net/filter.h"
 #include "qapi/string-output-visitor.h"
+#include "net/colo-compare.h"
+#include "qom/object_interfaces.h"
 
 /* Net bridge is currently not supported for W32. */
 #if !defined(_WIN32)
@@ -1194,14 +1196,170 @@ void qmp_netdev_del(const char *id, Error **errp)
     }
 }
 
+static CompareState *colo_passthrough_check(IPFlowSpec *spec, Error **errp)
+{
+    Object *container;
+    Object *obj;
+    CompareState *s;
+
+    if (!spec->object_name) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "object-name",
+                   "Need input colo-compare object name");
+        return NULL;
+    }
+
+    container = object_get_objects_root();
+    obj = object_resolve_path_component(container, spec->object_name);
+    if (!obj) {
+        error_setg(errp, "colo-compare '%s' not found", spec->object_name);
+        return NULL;
+    }
+
+    s = COLO_COMPARE(obj);
+
+    if (!getprotobyname(spec->protocol)) {
+        error_setg(errp, "COLO pass through get wrong protocol");
+        return NULL;
+    }
+
+    if ((spec->source->host && !qemu_isdigit(spec->source->host[0])) ||
+        (spec->destination->host &&
+        !qemu_isdigit(spec->destination->host[0]))) {
+        error_setg(errp, "COLO pass through get wrong IP");
+        return NULL;
+    }
+
+    if (atoi(spec->source->port) > 65536 || atoi(spec->source->port) < 0 ||
+        atoi(spec->destination->port) > 65536 ||
+        atoi(spec->destination->port) < 0) {
+        error_setg(errp, "COLO pass through get wrong port");
+        return NULL;
+    }
+
+    return s;
+}
+
+static COLOPassthroughEntry *compare_passthrough_find(CompareState *s,
+                                                      COLOPassthroughEntry *ent)
+{
+    COLOPassthroughEntry *next = NULL, *origin = NULL;
+
+    if (!QLIST_EMPTY(&s->passthroughlist)) {
+        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
+            if ((ent->l4_protocol->p_proto == origin->l4_protocol->p_proto) &&
+                (ent->src_port == origin->src_port) &&
+                (ent->dst_port == origin->dst_port) &&
+                (ent->src_ip.s_addr == origin->src_ip.s_addr) &&
+                (ent->dst_ip.s_addr == origin->dst_ip.s_addr)) {
+                return origin;
+            }
+        }
+    }
+
+    return NULL;
+}
+
+static void compare_passthrough_add(CompareState *s,
+                                    IPFlowSpec *spec,
+                                    Error **errp)
+{
+    COLOPassthroughEntry *pass = NULL;
+
+    pass = g_new0(COLOPassthroughEntry, 1);
+
+    pass->l4_protocol = getprotobyname(spec->protocol);
+    pass->src_port = atoi(spec->source->port);
+    pass->dst_port = atoi(spec->destination->port);
+
+    if (!inet_aton(spec->source->host, &pass->src_ip)) {
+        pass->src_ip.s_addr = 0;
+    }
+
+    if (!inet_aton(spec->destination->host, &pass->dst_ip)) {
+        pass->dst_ip.s_addr = 0;
+    }
+
+    qemu_mutex_lock(&s->passthroughlist_mutex);
+    if (compare_passthrough_find(s, pass)) {
+        error_setg(errp, "The pass through connection already exists");
+        g_free(pass);
+        qemu_mutex_unlock(&s->passthroughlist_mutex);
+        return;
+    }
+
+    QLIST_INSERT_HEAD(&s->passthroughlist, pass, node);
+    qemu_mutex_unlock(&s->passthroughlist_mutex);
+}
+
+static void compare_passthrough_del(CompareState *s,
+                                    IPFlowSpec *spec,
+                                    Error **errp)
+{
+    COLOPassthroughEntry *pass = NULL, *result = NULL;
+
+    pass = g_new0(COLOPassthroughEntry, 1);
+
+    pass->l4_protocol = getprotobyname(spec->protocol);
+    pass->src_port = atoi(spec->source->port);
+    pass->dst_port = atoi(spec->destination->port);
+
+    if (!inet_aton(spec->source->host, &pass->src_ip)) {
+        pass->src_ip.s_addr = 0;
+    }
+
+    if (!inet_aton(spec->destination->host, &pass->dst_ip)) {
+        pass->dst_ip.s_addr = 0;
+    }
+
+    qemu_mutex_lock(&s->passthroughlist_mutex);
+
+    result = compare_passthrough_find(s, pass);
+    if (result) {
+        QLIST_REMOVE(result, node);
+        g_free(result);
+    } else {
+        error_setg(errp, "Can't find the IP flow Spec");
+    }
+
+    g_free(pass);
+    qemu_mutex_unlock(&s->passthroughlist_mutex);
+}
+
+
 void qmp_colo_passthrough_add(IPFlowSpec *spec, Error **errp)
 {
-    /* TODO implement setup passthrough rule */
+    CompareState *s;
+    Error *err = NULL;
+
+    s = colo_passthrough_check(spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    compare_passthrough_add(s, spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
 }
 
 void qmp_colo_passthrough_del(IPFlowSpec *spec, Error **errp)
 {
-    /* TODO implement delete passthrough rule */
+    CompareState *s;
+    Error *err = NULL;
+
+    s = colo_passthrough_check(spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    compare_passthrough_del(s, spec, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
 }
 
 static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
-- 
2.25.1

Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough

[Eric Blake]

On Wed, May 26, 2021 at 10:54:19AM +0800, Zhang Chen wrote:
> Since the real user scenario does not need COLO to monitor all traffic.
> Add colo-passthrough-add and colo-passthrough-del to maintain
> a COLO network passthrough list. Add IPFlowSpec struct for all QMP commands.
> Except protocol field is necessary, other fields are optional.

That last sentence reads awkwardly, and I don't see a protocol field
in the patch below.

> 
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  net/net.c     | 10 ++++++++
>  qapi/net.json | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
> 

> +++ b/qapi/net.json
> @@ -7,6 +7,7 @@
>  ##
>  
>  { 'include': 'common.json' }
> +{ 'include': 'sockets.json' }
>  
>  ##
>  # @set_link:
> @@ -694,3 +695,70 @@
>  ##
>  { 'event': 'FAILOVER_NEGOTIATED',
>    'data': {'device-id': 'str'} }
> +
> +##
> +# @IPFlowSpec:
> +#
> +# IP flow specification.
> +#
> +# @protocol: Transport layer protocol like TCP/UDP...

Why is this open-coded as 'str' instead of an enum?

> +#
> +# @object-name: Point out the IPflow spec effective range of object,
> +#               If there is no such part, it means global spec.
> +#
> +# @source: Source address and port.
> +#
> +# @destination: Destination address and port.
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'IPFlowSpec',
> +  'data': { '*protocol': 'str', '*object-name': 'str',
> +    '*source': 'InetSocketAddressBase',
> +    '*destination': 'InetSocketAddressBase' } }
> +
> +##
> +# @colo-passthrough-add:
> +#
> +# Add passthrough entry according to user's needs in COLO-compare.
> +# Source IP/port and destination IP/port both optional, If user just
> +# input parts of infotmation, it will match all.

information

Grammar suggestion:

The source and destination IP/ports are both optional; if the user
only inputs part of the information, this will match all traffic.

except I'm not sure if my rewrite conveys the actual intent.

> +#
> +# Returns: Nothing on success
> +#
> +# Since: 6.1
> +#
> +# Example:
> +#
> +# -> { "execute": "colo-passthrough-add",
> +#      "arguments": { "protocol": "tcp", "object-name": "object0",
> +#      "source": {"host": "192.168.1.1", "port": "1234"},
> +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'colo-passthrough-add', 'boxed': true,
> +     'data': 'IPFlowSpec' }
> +
> +##
> +# @colo-passthrough-del:
> +#
> +# Delete passthrough entry according to user's needs in COLO-compare.
> +# Source IP/port and destination IP/port both optional, If user just
> +# input parts of infotmation, it will match all.

Same problems as above.

> +#
> +# Returns: Nothing on success
> +#
> +# Since: 6.1
> +#
> +# Example:
> +#
> +# -> { "execute": "colo-passthrough-del",
> +#      "arguments": { "protocol": "tcp", "object-name": "object0",
> +#      "source": {"host": "192.168.1.1", "port": "1234"},
> +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'colo-passthrough-del', 'boxed': true,
> +     'data': 'IPFlowSpec' }
> -- 
> 2.25.1
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

RE: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Eric Blake <eblake@redhat.com>
> Sent: Friday, June 4, 2021 10:35 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>; Markus
> Armbruster <armbru@redhat.com>; Daniel P. Berrangé
> <berrange@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Li Zhijian
> <lizhijian@cn.fujitsu.com>; Zhang Chen <zhangckid@gmail.com>; Lukas
> Straub <lukasstraub2@web.de>
> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command
> for COLO passthrough
> 
> On Wed, May 26, 2021 at 10:54:19AM +0800, Zhang Chen wrote:
> > Since the real user scenario does not need COLO to monitor all traffic.
> > Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
> > network passthrough list. Add IPFlowSpec struct for all QMP commands.
> > Except protocol field is necessary, other fields are optional.
> 
> That last sentence reads awkwardly, and I don't see a protocol field in the
> patch below.

Oh, We move the protocol field to optional by Lukas's comments in V6.
I will remove this comments here.

> 
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  net/net.c     | 10 ++++++++
> >  qapi/net.json | 68
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 78 insertions(+)
> >
> 
> > +++ b/qapi/net.json
> > @@ -7,6 +7,7 @@
> >  ##
> >
> >  { 'include': 'common.json' }
> > +{ 'include': 'sockets.json' }
> >
> >  ##
> >  # @set_link:
> > @@ -694,3 +695,70 @@
> >  ##
> >  { 'event': 'FAILOVER_NEGOTIATED',
> >    'data': {'device-id': 'str'} }
> > +
> > +##
> > +# @IPFlowSpec:
> > +#
> > +# IP flow specification.
> > +#
> > +# @protocol: Transport layer protocol like TCP/UDP...
> 
> Why is this open-coded as 'str' instead of an enum?

The original code use enum, but we change it by Dave and Markus's comments.
Please check the history:
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg03919.html

> 
> > +#
> > +# @object-name: Point out the IPflow spec effective range of object,
> > +#               If there is no such part, it means global spec.
> > +#
> > +# @source: Source address and port.
> > +#
> > +# @destination: Destination address and port.
> > +#
> > +# Since: 6.1
> > +##
> > +{ 'struct': 'IPFlowSpec',
> > +  'data': { '*protocol': 'str', '*object-name': 'str',
> > +    '*source': 'InetSocketAddressBase',
> > +    '*destination': 'InetSocketAddressBase' } }
> > +
> > +##
> > +# @colo-passthrough-add:
> > +#
> > +# Add passthrough entry according to user's needs in COLO-compare.
> > +# Source IP/port and destination IP/port both optional, If user just
> > +# input parts of infotmation, it will match all.
> 
> information
> 
> Grammar suggestion:
> 
> The source and destination IP/ports are both optional; if the user only inputs
> part of the information, this will match all traffic.
> 
> except I'm not sure if my rewrite conveys the actual intent.

Looks good to me, It should add the "protocol" to optional too.
Sorry, I'm not a native speaker, I will fix it in next version.

> 
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 6.1
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-add",
> > +#      "arguments": { "protocol": "tcp", "object-name": "object0",
> > +#      "source": {"host": "192.168.1.1", "port": "1234"},
> > +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-add', 'boxed': true,
> > +     'data': 'IPFlowSpec' }
> > +
> > +##
> > +# @colo-passthrough-del:
> > +#
> > +# Delete passthrough entry according to user's needs in COLO-compare.
> > +# Source IP/port and destination IP/port both optional, If user just
> > +# input parts of infotmation, it will match all.
> 
> Same problems as above.

OK.

Thanks
Chen

> 
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 6.1
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-del",
> > +#      "arguments": { "protocol": "tcp", "object-name": "object0",
> > +#      "source": {"host": "192.168.1.1", "port": "1234"},
> > +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-del', 'boxed': true,
> > +     'data': 'IPFlowSpec' }
> > --
> > 2.25.1
> >
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

RE: [PATCH V7 0/6] Passthrough specific network traffic in COLO

[Zhang, Chen]

Hi Jason and Markus,

No news for a while.
If you have time, please give me more comments for this series.

Thanks
Chen


> -----Original Message-----
> From: Zhang, Chen <chen.zhang@intel.com>
> Sent: Wednesday, May 26, 2021 10:54 AM
> To: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>;
> Daniel P. Berrangé <berrange@redhat.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>
> Cc: Zhang Chen <zhangckid@gmail.com>; Zhang, Chen
> <chen.zhang@intel.com>; Lukas Straub <lukasstraub2@web.de>
> Subject: [PATCH V7 0/6] Passthrough specific network traffic in COLO
> 
> Due to some real user scenarios don't need to monitor all traffic.
> And qemu net-filter also need function to more detailed flow control.
> This series give user ability to passthrough kinds of COLO network stream.
> 
> For example, windows guest user want to enable windows remote desktop
> to touch guest(UDP/TCP 3389), This case use UDP and TCP mixed, and the tcp
> part payload always different caused by real desktop display data(for guest
> time/ mouse display....).
> 
> Another case is some real user application will actively transmit information
> include guest time part, primary guest send data with time 10:01.000, At the
> same time secondary guest send data with time 10:01.001, it will always
> trigger COLO checkpoint(live migrate) to drop guest performance.
> 
>   V7:
>     - Keep some data structure stay in .c (patch 4/6).
>     - Fix mutex init issue (patch 5/6).
>     - Make the IPFlowSpec 'protocol' field optional (patch 1/6).
>     - Add compare_passthrough_find function in net.c (patch 6/6).
> 
>   V6:
>     - Change QAPI IPFlowSpec protocol from enum to str.
>     - Use getprotobyname to handle the protocols.
>     - Optimize code in net.
> 
>   V5:
>     - Squash original 1-3 QAPI patches together.
>     - Rename some data structures to avoid misunderstanding.
>     - Reuse InetSocketAddressBase in IPFlowSpec.
>     - Add new function in util/qemu-sockets.c to parse
>       InetSocketAddressBase.
>     - Update HMP command define to reuse current code.
>     - Add more comments.
> 
>   V4:
>     - Fix QAPI code conflict for V6.0 merged patches.
>     - Note this feature for V6.1.
> 
>   V3:
>     - Add COLO passthrough list lock.
>     - Add usage demo and more comments.
> 
>   V2:
>     - Add the n-tuple support.
>     - Add some qapi definitions.
>     - Support multi colo-compare objects.
>     - Support setup each rules for each objects individually.
>     - Clean up COLO compare definition to .h file.
>     - Rebase HMP command for stable tree.
>     - Add redundant rules check.
> 
> 
> Zhang Chen (6):
>   qapi/net: Add IPFlowSpec and QMP command for COLO passthrough
>   util/qemu-sockets.c: Add inet_parse_base to handle
>     InetSocketAddressBase
>   hmp-commands: Add new HMP command for COLO passthrough
>   net/colo-compare: Move data structure and define to .h file.
>   net/colo-compare: Add passthrough list to CompareState
>   net/net.c: Add handler for COLO passthrough connection
> 
>  hmp-commands.hx        |  26 +++++++
>  include/monitor/hmp.h  |   2 +
>  include/qemu/sockets.h |   1 +
>  monitor/hmp-cmds.c     |  82 ++++++++++++++++++++
>  net/colo-compare.c     | 160 +++++++++++++--------------------------
>  net/colo-compare.h     |  98 ++++++++++++++++++++++++
>  net/net.c              | 168 +++++++++++++++++++++++++++++++++++++++++
>  qapi/net.json          |  68 +++++++++++++++++
>  util/qemu-sockets.c    |  14 ++++
>  9 files changed, 510 insertions(+), 109 deletions(-)
> 
> --
> 2.25.1

Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough

[Markus Armbruster]

"Zhang, Chen" <chen.zhang@intel.com> writes:

>> -----Original Message-----
>> From: Eric Blake <eblake@redhat.com>
>> Sent: Friday, June 4, 2021 10:35 PM
>> To: Zhang, Chen <chen.zhang@intel.com>
>> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
>> devel@nongnu.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>; Markus
>> Armbruster <armbru@redhat.com>; Daniel P. Berrangé
>> <berrange@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Li Zhijian
>> <lizhijian@cn.fujitsu.com>; Zhang Chen <zhangckid@gmail.com>; Lukas
>> Straub <lukasstraub2@web.de>
>> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command
>> for COLO passthrough
>> 
>> On Wed, May 26, 2021 at 10:54:19AM +0800, Zhang Chen wrote:
>> > Since the real user scenario does not need COLO to monitor all traffic.
>> > Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
>> > network passthrough list. Add IPFlowSpec struct for all QMP commands.
>> > Except protocol field is necessary, other fields are optional.
>> 
>> That last sentence reads awkwardly, and I don't see a protocol field in the
>> patch below.
>
> Oh, We move the protocol field to optional by Lukas's comments in V6.
> I will remove this comments here.
>
>> 
>> >
>> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
>> > ---
>> >  net/net.c     | 10 ++++++++
>> >  qapi/net.json | 68
>> > +++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 78 insertions(+)
>> >
>> 
>> > +++ b/qapi/net.json
>> > @@ -7,6 +7,7 @@
>> >  ##
>> >
>> >  { 'include': 'common.json' }
>> > +{ 'include': 'sockets.json' }
>> >
>> >  ##
>> >  # @set_link:
>> > @@ -694,3 +695,70 @@
>> >  ##
>> >  { 'event': 'FAILOVER_NEGOTIATED',
>> >    'data': {'device-id': 'str'} }
>> > +
>> > +##
>> > +# @IPFlowSpec:
>> > +#
>> > +# IP flow specification.
>> > +#
>> > +# @protocol: Transport layer protocol like TCP/UDP...
>> 
>> Why is this open-coded as 'str' instead of an enum?
>
> The original code use enum, but we change it by Dave and Markus's comments.
> Please check the history:
> https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg03919.html

It's a string to be passed to getprotobyname(3).  Please mention that in
the doc string.

It's not an enum, because we don't want to duplicate /etc/protocols in
the QAPI schema.

>> > +#
>> > +# @object-name: Point out the IPflow spec effective range of object,

I have no idea what that means :)

It might be what was called @id in v4.  There, you explained

    The @id means packet hander in Qemu. Because not all the guest network packet into the colo-compare module, the net-filters are same cases.
    There modules attach to NIC or chardev socket to work, VM maybe have multi modules running. So we use the ID to set the rule to the specific module. 

and I asked you to work it into the doc comment.

If you want help with working it into the doc comment, please explain
its intended use for dummies :)

>> > +#               If there is no such part, it means global spec.
>> > +#
>> > +# @source: Source address and port.
>> > +#
>> > +# @destination: Destination address and port.
>> > +#
>> > +# Since: 6.1
>> > +##
>> > +{ 'struct': 'IPFlowSpec',
>> > +  'data': { '*protocol': 'str', '*object-name': 'str',
>> > +    '*source': 'InetSocketAddressBase',
>> > +    '*destination': 'InetSocketAddressBase' } }
>> > +
>> > +##
>> > +# @colo-passthrough-add:
>> > +#
>> > +# Add passthrough entry according to user's needs in COLO-compare.
>> > +# Source IP/port and destination IP/port both optional, If user just
>> > +# input parts of infotmation, it will match all.
>> 
>> information
>> 
>> Grammar suggestion:
>> 
>> The source and destination IP/ports are both optional; if the user only inputs
>> part of the information, this will match all traffic.
>> 
>> except I'm not sure if my rewrite conveys the actual intent.
>
> Looks good to me, It should add the "protocol" to optional too.
> Sorry, I'm not a native speaker, I will fix it in next version.
>
>> 
>> > +#
>> > +# Returns: Nothing on success
>> > +#
>> > +# Since: 6.1
>> > +#
>> > +# Example:
>> > +#
>> > +# -> { "execute": "colo-passthrough-add",
>> > +#      "arguments": { "protocol": "tcp", "object-name": "object0",
>> > +#      "source": {"host": "192.168.1.1", "port": "1234"},
>> > +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
>> > +# <- { "return": {} }
>> > +#
>> > +##
>> > +{ 'command': 'colo-passthrough-add', 'boxed': true,
>> > +     'data': 'IPFlowSpec' }
>> > +
>> > +##
>> > +# @colo-passthrough-del:
>> > +#
>> > +# Delete passthrough entry according to user's needs in COLO-compare.
>> > +# Source IP/port and destination IP/port both optional, If user just
>> > +# input parts of infotmation, it will match all.
>> 
>> Same problems as above.
>
> OK.
>
> Thanks
> Chen
>
>> 
>> > +#
>> > +# Returns: Nothing on success
>> > +#
>> > +# Since: 6.1
>> > +#
>> > +# Example:
>> > +#
>> > +# -> { "execute": "colo-passthrough-del",
>> > +#      "arguments": { "protocol": "tcp", "object-name": "object0",
>> > +#      "source": {"host": "192.168.1.1", "port": "1234"},
>> > +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
>> > +# <- { "return": {} }
>> > +#
>> > +##
>> > +{ 'command': 'colo-passthrough-del', 'boxed': true,
>> > +     'data': 'IPFlowSpec' }
>> > --
>> > 2.25.1
>> >
>> 
>> --
>> Eric Blake, Principal Software Engineer
>> Red Hat, Inc.           +1-919-301-3266
>> Virtualization:  qemu.org | libvirt.org

RE: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: Friday, June 11, 2021 5:37 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Eric Blake <eblake@redhat.com>; Lukas Straub <lukasstraub2@web.de>;
> Daniel P.Berrangé <berrange@redhat.com>; Li Zhijian
> <lizhijian@cn.fujitsu.com>; Jason Wang <jasowang@redhat.com>; Markus
> Armbruster <armbru@redhat.com>; qemu-dev <qemu-devel@nongnu.org>;
> Gerd Hoffmann <kraxel@redhat.com>; Zhang Chen <zhangckid@gmail.com>;
> Dr. David Alan Gilbert <dgilbert@redhat.com>
> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command
> for COLO passthrough
> 
> "Zhang, Chen" <chen.zhang@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Eric Blake <eblake@redhat.com>
> >> Sent: Friday, June 4, 2021 10:35 PM
> >> To: Zhang, Chen <chen.zhang@intel.com>
> >> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> >> devel@nongnu.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> >> Markus Armbruster <armbru@redhat.com>; Daniel P. Berrangé
> >> <berrange@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Li
> Zhijian
> >> <lizhijian@cn.fujitsu.com>; Zhang Chen <zhangckid@gmail.com>; Lukas
> >> Straub <lukasstraub2@web.de>
> >> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP
> command
> >> for COLO passthrough
> >>
> >> On Wed, May 26, 2021 at 10:54:19AM +0800, Zhang Chen wrote:
> >> > Since the real user scenario does not need COLO to monitor all traffic.
> >> > Add colo-passthrough-add and colo-passthrough-del to maintain a
> >> > COLO network passthrough list. Add IPFlowSpec struct for all QMP
> commands.
> >> > Except protocol field is necessary, other fields are optional.
> >>
> >> That last sentence reads awkwardly, and I don't see a protocol field
> >> in the patch below.
> >
> > Oh, We move the protocol field to optional by Lukas's comments in V6.
> > I will remove this comments here.
> >
> >>
> >> >
> >> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> >> > ---
> >> >  net/net.c     | 10 ++++++++
> >> >  qapi/net.json | 68
> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >  2 files changed, 78 insertions(+)
> >> >
> >>
> >> > +++ b/qapi/net.json
> >> > @@ -7,6 +7,7 @@
> >> >  ##
> >> >
> >> >  { 'include': 'common.json' }
> >> > +{ 'include': 'sockets.json' }
> >> >
> >> >  ##
> >> >  # @set_link:
> >> > @@ -694,3 +695,70 @@
> >> >  ##
> >> >  { 'event': 'FAILOVER_NEGOTIATED',
> >> >    'data': {'device-id': 'str'} }
> >> > +
> >> > +##
> >> > +# @IPFlowSpec:
> >> > +#
> >> > +# IP flow specification.
> >> > +#
> >> > +# @protocol: Transport layer protocol like TCP/UDP...
> >>
> >> Why is this open-coded as 'str' instead of an enum?
> >
> > The original code use enum, but we change it by Dave and Markus's
> comments.
> > Please check the history:
> > https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg03919.html
> 
> It's a string to be passed to getprotobyname(3).  Please mention that in the
> doc string.

OK, I will add this to commit log and qapi/net.json  in next version.

> 
> It's not an enum, because we don't want to duplicate /etc/protocols in the
> QAPI schema.

Yes.

> 
> >> > +#
> >> > +# @object-name: Point out the IPflow spec effective range of
> >> > +object,
> 
> I have no idea what that means :)
> 
> It might be what was called @id in v4.  There, you explained
> 
>     The @id means packet hander in Qemu. Because not all the guest network
> packet into the colo-compare module, the net-filters are same cases.
>     There modules attach to NIC or chardev socket to work, VM maybe have
> multi modules running. So we use the ID to set the rule to the specific
> module.
> 
> and I asked you to work it into the doc comment.
> 

Yes, As we discussed In V4, already changed the "id" to "object-name".
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01088.html 

And sorry I missed another mail thread to add the comments, I will add the explanation and quick update to V8 for this series.
Thanks for your help~

Thanks
Chen

> If you want help with working it into the doc comment, please explain its
> intended use for dummies :)
> 
> >> > +#               If there is no such part, it means global spec.
> >> > +#
> >> > +# @source: Source address and port.
> >> > +#
> >> > +# @destination: Destination address and port.
> >> > +#
> >> > +# Since: 6.1
> >> > +##
> >> > +{ 'struct': 'IPFlowSpec',
> >> > +  'data': { '*protocol': 'str', '*object-name': 'str',
> >> > +    '*source': 'InetSocketAddressBase',
> >> > +    '*destination': 'InetSocketAddressBase' } }
> >> > +
> >> > +##
> >> > +# @colo-passthrough-add:
> >> > +#
> >> > +# Add passthrough entry according to user's needs in COLO-compare.
> >> > +# Source IP/port and destination IP/port both optional, If user
> >> > +just # input parts of infotmation, it will match all.
> >>
> >> information
> >>
> >> Grammar suggestion:
> >>
> >> The source and destination IP/ports are both optional; if the user
> >> only inputs part of the information, this will match all traffic.
> >>
> >> except I'm not sure if my rewrite conveys the actual intent.
> >
> > Looks good to me, It should add the "protocol" to optional too.
> > Sorry, I'm not a native speaker, I will fix it in next version.
> >
> >>
> >> > +#
> >> > +# Returns: Nothing on success
> >> > +#
> >> > +# Since: 6.1
> >> > +#
> >> > +# Example:
> >> > +#
> >> > +# -> { "execute": "colo-passthrough-add",
> >> > +#      "arguments": { "protocol": "tcp", "object-name": "object0",
> >> > +#      "source": {"host": "192.168.1.1", "port": "1234"},
> >> > +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
> >> > +# <- { "return": {} }
> >> > +#
> >> > +##
> >> > +{ 'command': 'colo-passthrough-add', 'boxed': true,
> >> > +     'data': 'IPFlowSpec' }
> >> > +
> >> > +##
> >> > +# @colo-passthrough-del:
> >> > +#
> >> > +# Delete passthrough entry according to user's needs in COLO-
> compare.
> >> > +# Source IP/port and destination IP/port both optional, If user
> >> > +just # input parts of infotmation, it will match all.
> >>
> >> Same problems as above.
> >
> > OK.
> >
> > Thanks
> > Chen
> >
> >>
> >> > +#
> >> > +# Returns: Nothing on success
> >> > +#
> >> > +# Since: 6.1
> >> > +#
> >> > +# Example:
> >> > +#
> >> > +# -> { "execute": "colo-passthrough-del",
> >> > +#      "arguments": { "protocol": "tcp", "object-name": "object0",
> >> > +#      "source": {"host": "192.168.1.1", "port": "1234"},
> >> > +#      "destination": {"host": "192.168.1.2", "port": "4321"} } }
> >> > +# <- { "return": {} }
> >> > +#
> >> > +##
> >> > +{ 'command': 'colo-passthrough-del', 'boxed': true,
> >> > +     'data': 'IPFlowSpec' }
> >> > --
> >> > 2.25.1
> >> >
> >>
> >> --
> >> Eric Blake, Principal Software Engineer
> >> Red Hat, Inc.           +1-919-301-3266
> >> Virtualization:  qemu.org | libvirt.org