qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V6 0/6] Passthrough specific network traffic in COLO
@ 2021-04-20 15:15 Zhang Chen
  2021-04-20 15:15 ` [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough Zhang Chen
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Zhang Chen @ 2021-04-20 15:15 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Zhang Chen, Lukas Straub, 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.

  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     | 162 +++++++++++-----------------------------
 net/colo-compare.h     | 118 +++++++++++++++++++++++++++++
 net/net.c              | 166 +++++++++++++++++++++++++++++++++++++++++
 qapi/net.json          |  68 +++++++++++++++++
 util/qemu-sockets.c    |  14 ++++
 9 files changed, 519 insertions(+), 120 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough
  2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
@ 2021-04-20 15:15 ` Zhang Chen
  2021-05-17 20:34   ` Lukas Straub
  2021-04-20 15:15 ` [PATCH V6 2/6] util/qemu-sockets.c: Add inet_parse_base to handle InetSocketAddressBase Zhang Chen
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Zhang Chen @ 2021-04-20 15:15 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Zhang Chen, Lukas Straub, 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 edf9b95418..2a6e5f3886 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1196,6 +1196,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..f6e4e37526 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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V6 2/6] util/qemu-sockets.c: Add inet_parse_base to handle InetSocketAddressBase
  2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
  2021-04-20 15:15 ` [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough Zhang Chen
@ 2021-04-20 15:15 ` Zhang Chen
  2021-04-20 15:15 ` [PATCH V6 3/6] hmp-commands: Add new HMP command for COLO passthrough Zhang Chen
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Zhang Chen @ 2021-04-20 15:15 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Zhang Chen, Lukas Straub, 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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V6 3/6] hmp-commands: Add new HMP command for COLO passthrough
  2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
  2021-04-20 15:15 ` [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough Zhang Chen
  2021-04-20 15:15 ` [PATCH V6 2/6] util/qemu-sockets.c: Add inet_parse_base to handle InetSocketAddressBase Zhang Chen
@ 2021-04-20 15:15 ` Zhang Chen
  2021-04-20 15:15 ` [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file Zhang Chen
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Zhang Chen @ 2021-04-20 15:15 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Zhang Chen, Lukas Straub, 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 0ad5b77477..6991b03075 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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file.
  2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
                   ` (2 preceding siblings ...)
  2021-04-20 15:15 ` [PATCH V6 3/6] hmp-commands: Add new HMP command for COLO passthrough Zhang Chen
@ 2021-04-20 15:15 ` Zhang Chen
  2021-05-17 20:03   ` Lukas Straub
  2021-04-20 15:15 ` [PATCH V6 5/6] net/colo-compare: Add passthrough list to CompareState Zhang Chen
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Zhang Chen @ 2021-04-20 15:15 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Zhang Chen, Lukas Straub, 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 | 134 +++++----------------------------------------
 net/colo-compare.h | 106 +++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 120 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 9d1ad99941..b51b1437ef 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -17,44 +17,24 @@
 #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);
 
 static NotifierList colo_compare_notifiers =
     NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
 
-#define COMPARE_READ_LEN_MAX NET_BUFSIZE
-#define MAX_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 DEBUG_COLO_PACKETS */
 
 static QemuMutex colo_compare_mutex;
@@ -64,92 +44,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,
-};
-
 static const char *colo_mode[] = {
     [PRIMARY_IN] = "primary",
     [SECONDARY_IN] = "secondary",
@@ -737,19 +631,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 +660,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 +671,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 +684,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 +702,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 +715,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 +1168,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..ab649c9dbe 100644
--- a/net/colo-compare.h
+++ b/net/colo-compare.h
@@ -17,6 +17,112 @@
 #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)
+
+#define COMPARE_READ_LEN_MAX NET_BUFSIZE
+#define MAX_COLO_QUEUE_SIZE 1024
+
+#define COLO_COMPARE_FREE_PRIMARY     0x01
+#define COLO_COMPARE_FREE_SECONDARY   0x02
+
+#define COLO_REGULAR_PACKET_CHECK_MS 1000
+#define COLO_DEFAULT_TIME_OUT_MS 3000
+
+typedef struct COLOSendCo {
+    Coroutine *co;
+    struct CompareState *s;
+    CharBackend *chr;
+    GQueue send_list;
+    bool notify_remote_frame;
+    bool done;
+    int ret;
+} COLOSendCo;
+
+typedef struct COLOSendEntry {
+    uint32_t size;
+    uint32_t vnet_hdr_len;
+    uint8_t *buf;
+} COLOSendEntry;
+
+/*
+ *  + 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;
+
+enum {
+    PRIMARY_IN = 0,
+    SECONDARY_IN,
+};
+
 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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V6 5/6] net/colo-compare: Add passthrough list to CompareState
  2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
                   ` (3 preceding siblings ...)
  2021-04-20 15:15 ` [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file Zhang Chen
@ 2021-04-20 15:15 ` Zhang Chen
  2021-05-17 20:07   ` Lukas Straub
  2021-04-20 15:15 ` [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough connection Zhang Chen
  2021-04-28  3:26 ` [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang, Chen
  6 siblings, 1 reply; 17+ messages in thread
From: Zhang Chen @ 2021-04-20 15:15 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Zhang Chen, Lukas Straub, 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 b51b1437ef..7109e2ed30 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -141,6 +141,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) {
@@ -160,6 +161,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);
@@ -1225,6 +1251,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,
@@ -1237,6 +1264,7 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
     if (!colo_compare_active) {
         qemu_mutex_init(&event_mtx);
         qemu_cond_init(&event_complete_cond);
+        qemu_mutex_init(&s->passthroughlist_mutex);
         colo_compare_active = true;
     }
     QTAILQ_INSERT_TAIL(&net_compares, s, next);
diff --git a/net/colo-compare.h b/net/colo-compare.h
index ab649c9dbe..7ca74de840 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;
@@ -54,6 +55,15 @@ typedef struct COLOSendEntry {
     uint8_t *buf;
 } COLOSendEntry;
 
+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 ++
  *  |               |
@@ -110,6 +120,8 @@ struct CompareState {
 
     QEMUBH *event_bh;
     enum colo_event event;
+    QLIST_HEAD(, COLOPassthroughEntry) passthroughlist;
+    QemuMutex passthroughlist_mutex;
 
     QTAILQ_ENTRY(CompareState) next;
 };
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough connection
  2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
                   ` (4 preceding siblings ...)
  2021-04-20 15:15 ` [PATCH V6 5/6] net/colo-compare: Add passthrough list to CompareState Zhang Chen
@ 2021-04-20 15:15 ` Zhang Chen
  2021-05-17 20:38   ` Lukas Straub
  2021-04-28  3:26 ` [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang, Chen
  6 siblings, 1 reply; 17+ messages in thread
From: Zhang Chen @ 2021-04-20 15:15 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Zhang Chen, Lukas Straub, 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 | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 158 insertions(+), 2 deletions(-)

diff --git a/net/net.c b/net/net.c
index 2a6e5f3886..9b0de0f332 100644
--- a/net/net.c
+++ b/net/net.c
@@ -56,6 +56,8 @@
 #include "sysemu/sysemu.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)
@@ -1196,14 +1198,168 @@ 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 void compare_passthrough_add(CompareState *s,
+                                    IPFlowSpec *spec,
+                                    Error **errp)
+{
+    COLOPassthroughEntry *pass = NULL, *next = NULL, *origin = 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 (!QLIST_EMPTY(&s->passthroughlist)) {
+        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
+            if ((pass->l4_protocol->p_proto == origin->l4_protocol->p_proto) &&
+                (pass->src_port == origin->src_port) &&
+                (pass->dst_port == origin->dst_port) &&
+                (pass->src_ip.s_addr == origin->src_ip.s_addr) &&
+                (pass->dst_ip.s_addr == origin->dst_ip.s_addr)) {
+                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, *next = NULL, *origin = 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 (!QLIST_EMPTY(&s->passthroughlist)) {
+        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
+            if ((pass->l4_protocol->p_proto == origin->l4_protocol->p_proto) &&
+                (pass->src_port == origin->src_port) &&
+                (pass->dst_port == origin->dst_port) &&
+                (pass->src_ip.s_addr == origin->src_ip.s_addr) &&
+                (pass->dst_ip.s_addr == origin->dst_ip.s_addr)) {
+                QLIST_REMOVE(origin, node);
+                g_free(origin);
+                g_free(pass);
+                qemu_mutex_unlock(&s->passthroughlist_mutex);
+                return;
+            }
+        }
+        error_setg(errp, "The pass through list can't find the connection");
+    } else {
+        error_setg(errp, "The pass through connection list is empty");
+    }
+
+    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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* RE: [PATCH V6 0/6] Passthrough specific network traffic in COLO
  2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
                   ` (5 preceding siblings ...)
  2021-04-20 15:15 ` [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough connection Zhang Chen
@ 2021-04-28  3:26 ` Zhang, Chen
  2021-05-12  7:05   ` Zhang, Chen
  6 siblings, 1 reply; 17+ messages in thread
From: Zhang, Chen @ 2021-04-28  3:26 UTC (permalink / raw)
  To: Jason Wang, qemu-dev, Eric Blake, Dr. David Alan Gilbert,
	Markus Armbruster, Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Lukas Straub, Zhang Chen

Please give me for comments for this series, Ping....

Thanks
Chen

> -----Original Message-----
> From: Zhang, Chen <chen.zhang@intel.com>
> Sent: Tuesday, April 20, 2021 11:16 PM
> 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 V6 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.
> 
>   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     | 162 +++++++++++-----------------------------
>  net/colo-compare.h     | 118 +++++++++++++++++++++++++++++
>  net/net.c              | 166 +++++++++++++++++++++++++++++++++++++++++
>  qapi/net.json          |  68 +++++++++++++++++
>  util/qemu-sockets.c    |  14 ++++
>  9 files changed, 519 insertions(+), 120 deletions(-)
> 
> --
> 2.25.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V6 0/6] Passthrough specific network traffic in COLO
  2021-04-28  3:26 ` [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang, Chen
@ 2021-05-12  7:05   ` Zhang, Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Chen @ 2021-05-12  7:05 UTC (permalink / raw)
  To: Zhang, Chen, Jason Wang, qemu-dev, Eric Blake,
	Dr. David Alan Gilbert, Markus Armbruster,
	Daniel P. Berrangé,
	Gerd Hoffmann, Li Zhijian
  Cc: Lukas Straub, Zhang Chen

Hi Markus, Jason and Dave.

This series has been modified according to previous suggestions. 
Please review it again to make sure I fully understand comments.

Thanks
Chen

> -----Original Message-----
> From: Qemu-devel <qemu-devel-
> bounces+chen.zhang=intel.com@nongnu.org> On Behalf Of Zhang, Chen
> Sent: Wednesday, April 28, 2021 11:27 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: Lukas Straub <lukasstraub2@web.de>; Zhang Chen
> <zhangckid@gmail.com>
> Subject: RE: [PATCH V6 0/6] Passthrough specific network traffic in COLO
> 
> Please give me for comments for this series, Ping....
> 
> Thanks
> Chen
> 
> > -----Original Message-----
> > From: Zhang, Chen <chen.zhang@intel.com>
> > Sent: Tuesday, April 20, 2021 11:16 PM
> > 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 V6 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.
> >
> >   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     | 162 +++++++++++-----------------------------
> >  net/colo-compare.h     | 118 +++++++++++++++++++++++++++++
> >  net/net.c              | 166
> +++++++++++++++++++++++++++++++++++++++++
> >  qapi/net.json          |  68 +++++++++++++++++
> >  util/qemu-sockets.c    |  14 ++++
> >  9 files changed, 519 insertions(+), 120 deletions(-)
> >
> > --
> > 2.25.1
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file.
  2021-04-20 15:15 ` [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file Zhang Chen
@ 2021-05-17 20:03   ` Lukas Straub
  2021-05-20  1:50     ` Zhang, Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Lukas Straub @ 2021-05-17 20:03 UTC (permalink / raw)
  To: Zhang Chen
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert

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

On Tue, 20 Apr 2021 23:15:35 +0800
Zhang Chen <chen.zhang@intel.com> wrote:

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

Hi,
There are some definitions that don't need to be moved into the header,
more comments below.

In general I think the new passthrough feature can be exclusive to
colo-compare for now and that everything can remain there. If other net
filters implement the feature, we can still move it outside of
colo-compare later.

> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  net/colo-compare.c | 134 +++++----------------------------------------
>  net/colo-compare.h | 106 +++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+), 120 deletions(-)
> 
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 9d1ad99941..b51b1437ef 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -17,44 +17,24 @@
>  #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);
>  
>  static NotifierList colo_compare_notifiers =
>      NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
>  
> -#define COMPARE_READ_LEN_MAX NET_BUFSIZE
> -#define MAX_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
> -

These 6 defines should stay here.

>  /* #define DEBUG_COLO_PACKETS */
>  
>  static QemuMutex colo_compare_mutex;
> @@ -64,92 +44,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;

This struct should stay here.

> -typedef struct SendEntry {
> -    uint32_t size;
> -    uint32_t vnet_hdr_len;
> -    uint8_t *buf;
> -} SendEntry;

This struct should stay here.

> -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,
> -};

The enum should stay here.

>  static const char *colo_mode[] = {
>      [PRIMARY_IN] = "primary",
>      [SECONDARY_IN] = "secondary",
> @@ -737,19 +631,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 +660,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 +671,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 +684,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 +702,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 +715,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 +1168,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..ab649c9dbe 100644
> --- a/net/colo-compare.h
> +++ b/net/colo-compare.h
> @@ -17,6 +17,112 @@
>  #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)
> +
> +#define COMPARE_READ_LEN_MAX NET_BUFSIZE
> +#define MAX_COLO_QUEUE_SIZE 1024
> +
> +#define COLO_COMPARE_FREE_PRIMARY     0x01
> +#define COLO_COMPARE_FREE_SECONDARY   0x02
> +
> +#define COLO_REGULAR_PACKET_CHECK_MS 1000
> +#define COLO_DEFAULT_TIME_OUT_MS 3000
> +
> +typedef struct COLOSendCo {
> +    Coroutine *co;
> +    struct CompareState *s;
> +    CharBackend *chr;
> +    GQueue send_list;
> +    bool notify_remote_frame;
> +    bool done;
> +    int ret;
> +} COLOSendCo;
> +
> +typedef struct COLOSendEntry {
> +    uint32_t size;
> +    uint32_t vnet_hdr_len;
> +    uint8_t *buf;
> +} COLOSendEntry;
> +
> +/*
> + *  + 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;
> +
> +enum {
> +    PRIMARY_IN = 0,
> +    SECONDARY_IN,
> +};
> +
>  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);



-- 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V6 5/6] net/colo-compare: Add passthrough list to CompareState
  2021-04-20 15:15 ` [PATCH V6 5/6] net/colo-compare: Add passthrough list to CompareState Zhang Chen
@ 2021-05-17 20:07   ` Lukas Straub
  2021-05-20  3:38     ` Zhang, Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Lukas Straub @ 2021-05-17 20:07 UTC (permalink / raw)
  To: Zhang Chen
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert

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

On Tue, 20 Apr 2021 23:15:36 +0800
Zhang Chen <chen.zhang@intel.com> wrote:

> 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 b51b1437ef..7109e2ed30 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -141,6 +141,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) {
> @@ -160,6 +161,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);
> @@ -1225,6 +1251,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,
> @@ -1237,6 +1264,7 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
>      if (!colo_compare_active) {
>          qemu_mutex_init(&event_mtx);
>          qemu_cond_init(&event_complete_cond);
> +        qemu_mutex_init(&s->passthroughlist_mutex);

This initializes the mutex only for the first colo-compare object that is created. The mutex has to be initialized every time, as it separate for each
colo-compare object.

>          colo_compare_active = true;
>      }
>      QTAILQ_INSERT_TAIL(&net_compares, s, next);
> diff --git a/net/colo-compare.h b/net/colo-compare.h
> index ab649c9dbe..7ca74de840 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;
> @@ -54,6 +55,15 @@ typedef struct COLOSendEntry {
>      uint8_t *buf;
>  } COLOSendEntry;
>  
> +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 ++
>   *  |               |
> @@ -110,6 +120,8 @@ struct CompareState {
>  
>      QEMUBH *event_bh;
>      enum colo_event event;
> +    QLIST_HEAD(, COLOPassthroughEntry) passthroughlist;
> +    QemuMutex passthroughlist_mutex;
>  
>      QTAILQ_ENTRY(CompareState) next;
>  };



-- 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough
  2021-04-20 15:15 ` [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough Zhang Chen
@ 2021-05-17 20:34   ` Lukas Straub
  2021-05-20  5:49     ` Zhang, Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Lukas Straub @ 2021-05-17 20:34 UTC (permalink / raw)
  To: Zhang Chen
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert

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

On Tue, 20 Apr 2021 23:15:32 +0800
Zhang Chen <chen.zhang@intel.com> 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.
> 
> 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 edf9b95418..2a6e5f3886 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1196,6 +1196,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..f6e4e37526 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:

I think something like "@IPFilterRule" is clearer.

> +# IP flow specification.

"IP filter rule 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.

I think IPFlowSpec should be kept generic, so object-name should not be
part of it. It should move directly to 'colo-passthrough-add' and
'colo-passthrough-del'.

Also please use clearer wording. Proposal:
"@object-name: The id of the colo-compare object to add the filter to."

Again, if other net filters support the new feature in the future, the
wording can always be changed later.

> +# @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' } }

I think 'protocol' should be made optional too.

> +##
> +# @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' }



-- 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough connection
  2021-04-20 15:15 ` [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough connection Zhang Chen
@ 2021-05-17 20:38   ` Lukas Straub
  2021-05-20  5:50     ` Zhang, Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Lukas Straub @ 2021-05-17 20:38 UTC (permalink / raw)
  To: Zhang Chen
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert

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

On Tue, 20 Apr 2021 23:15:37 +0800
Zhang Chen <chen.zhang@intel.com> wrote:

> 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 | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 158 insertions(+), 2 deletions(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 2a6e5f3886..9b0de0f332 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -56,6 +56,8 @@
>  #include "sysemu/sysemu.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)
> @@ -1196,14 +1198,168 @@ 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 void compare_passthrough_add(CompareState *s,
> +                                    IPFlowSpec *spec,
> +                                    Error **errp)
> +{
> +    COLOPassthroughEntry *pass = NULL, *next = NULL, *origin = 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 (!QLIST_EMPTY(&s->passthroughlist)) {
> +        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
> +            if ((pass->l4_protocol->p_proto == origin->l4_protocol->p_proto) &&
> +                (pass->src_port == origin->src_port) &&
> +                (pass->dst_port == origin->dst_port) &&
> +                (pass->src_ip.s_addr == origin->src_ip.s_addr) &&
> +                (pass->dst_ip.s_addr == origin->dst_ip.s_addr)) {
> +                error_setg(errp, "The pass through connection already exists");
> +                g_free(pass);
> +                qemu_mutex_unlock(&s->passthroughlist_mutex);
> +                return;
> +            }
> +        }
> +    }

I think this searching for a existing passthrough rule should move into
a function. The function can then be used in compare_passthrough_del
too.

> +    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, *next = NULL, *origin = 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 (!QLIST_EMPTY(&s->passthroughlist)) {
> +        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
> +            if ((pass->l4_protocol->p_proto == origin->l4_protocol->p_proto) &&
> +                (pass->src_port == origin->src_port) &&
> +                (pass->dst_port == origin->dst_port) &&
> +                (pass->src_ip.s_addr == origin->src_ip.s_addr) &&
> +                (pass->dst_ip.s_addr == origin->dst_ip.s_addr)) {
> +                QLIST_REMOVE(origin, node);
> +                g_free(origin);
> +                g_free(pass);
> +                qemu_mutex_unlock(&s->passthroughlist_mutex);
> +                return;
> +            }
> +        }
> +        error_setg(errp, "The pass through list can't find the connection");
> +    } else {
> +        error_setg(errp, "The pass through connection list is empty");
> +    }
> +
> +    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)



-- 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file.
  2021-05-17 20:03   ` Lukas Straub
@ 2021-05-20  1:50     ` Zhang, Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Chen @ 2021-05-20  1:50 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert



> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Tuesday, May 18, 2021 4:04 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: 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>; Zhang Chen
> <zhangckid@gmail.com>
> Subject: Re: [PATCH V6 4/6] net/colo-compare: Move data structure and
> define to .h file.
> 
> On Tue, 20 Apr 2021 23:15:35 +0800
> Zhang Chen <chen.zhang@intel.com> wrote:
> 
> > Rename structure with COLO index and move it to .h file, It make other
> > modules can reuse COLO code.
> 
> Hi,
> There are some definitions that don't need to be moved into the header,
> more comments below.
> 

OK.

> In general I think the new passthrough feature can be exclusive to colo-
> compare for now and that everything can remain there. If other net filters
> implement the feature, we can still move it outside of colo-compare later.
> 

Agree, This patch move some code to colo-compare.h, it still in colo-compare.

Thanks
Chen

> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  net/colo-compare.c | 134
> > +++++----------------------------------------
> >  net/colo-compare.h | 106 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 120 insertions(+), 120 deletions(-)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 9d1ad99941..b51b1437ef 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -17,44 +17,24 @@
> >  #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);
> >
> >  static NotifierList colo_compare_notifiers =
> >      NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> >
> > -#define COMPARE_READ_LEN_MAX NET_BUFSIZE -#define
> MAX_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
> > -
> 
> These 6 defines should stay here.
> 
> >  /* #define DEBUG_COLO_PACKETS */
> >
> >  static QemuMutex colo_compare_mutex;
> > @@ -64,92 +44,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;
> 
> This struct should stay here.
> 
> > -typedef struct SendEntry {
> > -    uint32_t size;
> > -    uint32_t vnet_hdr_len;
> > -    uint8_t *buf;
> > -} SendEntry;
> 
> This struct should stay here.
> 
> > -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,
> > -};
> 
> The enum should stay here.
> 
> >  static const char *colo_mode[] = {
> >      [PRIMARY_IN] = "primary",
> >      [SECONDARY_IN] = "secondary",
> > @@ -737,19 +631,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 +660,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 +671,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 +684,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 +702,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 +715,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 +1168,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..ab649c9dbe 100644
> > --- a/net/colo-compare.h
> > +++ b/net/colo-compare.h
> > @@ -17,6 +17,112 @@
> >  #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)
> > +
> > +#define COMPARE_READ_LEN_MAX NET_BUFSIZE #define
> MAX_COLO_QUEUE_SIZE
> > +1024
> > +
> > +#define COLO_COMPARE_FREE_PRIMARY     0x01
> > +#define COLO_COMPARE_FREE_SECONDARY   0x02
> > +
> > +#define COLO_REGULAR_PACKET_CHECK_MS 1000 #define
> > +COLO_DEFAULT_TIME_OUT_MS 3000
> > +
> > +typedef struct COLOSendCo {
> > +    Coroutine *co;
> > +    struct CompareState *s;
> > +    CharBackend *chr;
> > +    GQueue send_list;
> > +    bool notify_remote_frame;
> > +    bool done;
> > +    int ret;
> > +} COLOSendCo;
> > +
> > +typedef struct COLOSendEntry {
> > +    uint32_t size;
> > +    uint32_t vnet_hdr_len;
> > +    uint8_t *buf;
> > +} COLOSendEntry;
> > +
> > +/*
> > + *  + 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;
> > +
> > +enum {
> > +    PRIMARY_IN = 0,
> > +    SECONDARY_IN,
> > +};
> > +
> >  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);
> 
> 
> 
> --



^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V6 5/6] net/colo-compare: Add passthrough list to CompareState
  2021-05-17 20:07   ` Lukas Straub
@ 2021-05-20  3:38     ` Zhang, Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Chen @ 2021-05-20  3:38 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert



> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Tuesday, May 18, 2021 4:07 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: 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>; Zhang Chen
> <zhangckid@gmail.com>
> Subject: Re: [PATCH V6 5/6] net/colo-compare: Add passthrough list to
> CompareState
> 
> On Tue, 20 Apr 2021 23:15:36 +0800
> Zhang Chen <chen.zhang@intel.com> wrote:
> 
> > 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
> > b51b1437ef..7109e2ed30 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -141,6 +141,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) {
> > @@ -160,6 +161,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); @@ -1225,6 +1251,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, @@ -1237,6 +1264,7 @@ static void
> colo_compare_complete(UserCreatable *uc, Error **errp)
> >      if (!colo_compare_active) {
> >          qemu_mutex_init(&event_mtx);
> >          qemu_cond_init(&event_complete_cond);
> > +        qemu_mutex_init(&s->passthroughlist_mutex);
> 
> This initializes the mutex only for the first colo-compare object that is created.
> The mutex has to be initialized every time, as it separate for each colo-
> compare object.

Good catch. I will fix it in the V7.

Thanks
Chen

> 
> >          colo_compare_active = true;
> >      }
> >      QTAILQ_INSERT_TAIL(&net_compares, s, next); diff --git
> > a/net/colo-compare.h b/net/colo-compare.h index
> ab649c9dbe..7ca74de840
> > 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; @@ -54,6 +55,15 @@
> typedef
> > struct COLOSendEntry {
> >      uint8_t *buf;
> >  } COLOSendEntry;
> >
> > +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 ++
> >   *  |               |
> > @@ -110,6 +120,8 @@ struct CompareState {
> >
> >      QEMUBH *event_bh;
> >      enum colo_event event;
> > +    QLIST_HEAD(, COLOPassthroughEntry) passthroughlist;
> > +    QemuMutex passthroughlist_mutex;
> >
> >      QTAILQ_ENTRY(CompareState) next;
> >  };
> 
> 
> 
> --



^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough
  2021-05-17 20:34   ` Lukas Straub
@ 2021-05-20  5:49     ` Zhang, Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Chen @ 2021-05-20  5:49 UTC (permalink / raw)
  To: Lukas Straub, Markus Armbruster
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert



> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Tuesday, May 18, 2021 4:35 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: 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>; Zhang Chen
> <zhangckid@gmail.com>
> Subject: Re: [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command
> for COLO passthrough
> 
> On Tue, 20 Apr 2021 23:15:32 +0800
> Zhang Chen <chen.zhang@intel.com> 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.
> >
> > 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 edf9b95418..2a6e5f3886 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1196,6 +1196,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..f6e4e37526 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:
> 
> I think something like "@IPFilterRule" is clearer.
> 
> > +# IP flow specification.
> 
> "IP filter rule 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.
> 
> I think IPFlowSpec should be kept generic, so object-name should not be
> part of it. It should move directly to 'colo-passthrough-add' and 'colo-
> passthrough-del'.
> 
> Also please use clearer wording. Proposal:
> "@object-name: The id of the colo-compare object to add the filter to."
> 
> Again, if other net filters support the new feature in the future, the wording
> can always be changed later.

We already discussed the name of the "IPFlowSpec" in this series V3/V4...
Current definition is a generic one. Both OK for me.
For the qapi/net.json, Hi Markus, which name do you think is better?


> 
> > +# @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' } }
> 
> I think 'protocol' should be made optional too.

Make protocol to optional is easy.
But for most cases, with a protocol is necessary.
If user unexpected input nothing, it will make the entire network unavailable.

Thanks
Chen

> 
> > +##
> > +# @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' }
> 
> 
> 
> --


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough connection
  2021-05-17 20:38   ` Lukas Straub
@ 2021-05-20  5:50     ` Zhang, Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Chen @ 2021-05-20  5:50 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Daniel P. Berrangé,
	Li Zhijian, Jason Wang, Markus Armbruster, qemu-dev,
	Gerd Hoffmann, Zhang Chen, Dr. David Alan Gilbert



> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Tuesday, May 18, 2021 4:39 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: 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>; Zhang Chen
> <zhangckid@gmail.com>
> Subject: Re: [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough
> connection
> 
> On Tue, 20 Apr 2021 23:15:37 +0800
> Zhang Chen <chen.zhang@intel.com> wrote:
> 
> > 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 | 160
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 158 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 2a6e5f3886..9b0de0f332 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -56,6 +56,8 @@
> >  #include "sysemu/sysemu.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) @@ -1196,14 +1198,168 @@ 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 void compare_passthrough_add(CompareState *s,
> > +                                    IPFlowSpec *spec,
> > +                                    Error **errp) {
> > +    COLOPassthroughEntry *pass = NULL, *next = NULL, *origin = 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 (!QLIST_EMPTY(&s->passthroughlist)) {
> > +        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
> > +            if ((pass->l4_protocol->p_proto == origin->l4_protocol->p_proto)
> &&
> > +                (pass->src_port == origin->src_port) &&
> > +                (pass->dst_port == origin->dst_port) &&
> > +                (pass->src_ip.s_addr == origin->src_ip.s_addr) &&
> > +                (pass->dst_ip.s_addr == origin->dst_ip.s_addr)) {
> > +                error_setg(errp, "The pass through connection already exists");
> > +                g_free(pass);
> > +                qemu_mutex_unlock(&s->passthroughlist_mutex);
> > +                return;
> > +            }
> > +        }
> > +    }
> 
> I think this searching for a existing passthrough rule should move into a
> function. The function can then be used in compare_passthrough_del too.

Good idea.
I will change it in next version.

Thanks
Chen

> 
> > +    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, *next = NULL, *origin = 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 (!QLIST_EMPTY(&s->passthroughlist)) {
> > +        QLIST_FOREACH_SAFE(origin, &s->passthroughlist, node, next) {
> > +            if ((pass->l4_protocol->p_proto == origin->l4_protocol->p_proto)
> &&
> > +                (pass->src_port == origin->src_port) &&
> > +                (pass->dst_port == origin->dst_port) &&
> > +                (pass->src_ip.s_addr == origin->src_ip.s_addr) &&
> > +                (pass->dst_ip.s_addr == origin->dst_ip.s_addr)) {
> > +                QLIST_REMOVE(origin, node);
> > +                g_free(origin);
> > +                g_free(pass);
> > +                qemu_mutex_unlock(&s->passthroughlist_mutex);
> > +                return;
> > +            }
> > +        }
> > +        error_setg(errp, "The pass through list can't find the connection");
> > +    } else {
> > +        error_setg(errp, "The pass through connection list is empty");
> > +    }
> > +
> > +    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)
> 
> 
> 
> --



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-05-20  5:51 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20 15:15 [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang Chen
2021-04-20 15:15 ` [PATCH V6 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough Zhang Chen
2021-05-17 20:34   ` Lukas Straub
2021-05-20  5:49     ` Zhang, Chen
2021-04-20 15:15 ` [PATCH V6 2/6] util/qemu-sockets.c: Add inet_parse_base to handle InetSocketAddressBase Zhang Chen
2021-04-20 15:15 ` [PATCH V6 3/6] hmp-commands: Add new HMP command for COLO passthrough Zhang Chen
2021-04-20 15:15 ` [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file Zhang Chen
2021-05-17 20:03   ` Lukas Straub
2021-05-20  1:50     ` Zhang, Chen
2021-04-20 15:15 ` [PATCH V6 5/6] net/colo-compare: Add passthrough list to CompareState Zhang Chen
2021-05-17 20:07   ` Lukas Straub
2021-05-20  3:38     ` Zhang, Chen
2021-04-20 15:15 ` [PATCH V6 6/6] net/net.c: Add handler for COLO passthrough connection Zhang Chen
2021-05-17 20:38   ` Lukas Straub
2021-05-20  5:50     ` Zhang, Chen
2021-04-28  3:26 ` [PATCH V6 0/6] Passthrough specific network traffic in COLO Zhang, Chen
2021-05-12  7:05   ` Zhang, Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).