[PATCH 0/3] Bypass specific network traffic in COLO

[PATCH 0/3] Bypass specific network traffic in COLO

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

Since the real user scenario does not need to monitor all traffic.
This series give user ability to bypass kinds of network stream.

Zhang Chen (3):
  qapi/net: Add new QMP command for COLO passthrough
  hmp-commands: Add new HMP command for COLO passthrough
  net/colo-compare: Add handler for passthrough connection

 hmp-commands.hx       | 26 +++++++++++++++++++++++
 include/monitor/hmp.h |  2 ++
 monitor/hmp-cmds.c    | 20 ++++++++++++++++++
 net/colo-compare.c    | 49 +++++++++++++++++++++++++++++++++++++++++++
 net/colo-compare.h    |  2 ++
 net/net.c             | 39 ++++++++++++++++++++++++++++++++++
 qapi/net.json         | 46 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 184 insertions(+)

-- 
2.17.1

[PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

Since the real user scenario does not need to monitor all traffic.
Add colo-passthrough-add and colo-passthrough-del to maintain
a COLO network passthrough list.

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

diff --git a/net/net.c b/net/net.c
index e1035f21d1..eac7a92618 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1151,6 +1151,18 @@ void qmp_netdev_del(const char *id, Error **errp)
     qemu_del_net_client(nc);
 }
 
+void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
+                              Error **errp)
+{
+    /* Setup passthrough connection */
+}
+
+void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
+                              Error **errp)
+{
+    /* Delete passthrough connection */
+}
+
 static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
 {
     char *str;
diff --git a/qapi/net.json b/qapi/net.json
index c31748c87f..466c29714e 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -714,3 +714,49 @@
 ##
 { 'event': 'FAILOVER_NEGOTIATED',
   'data': {'device-id': 'str'} }
+
+##
+# @colo-passthrough-add:
+#
+# Add passthrough entry according to customer's needs in COLO-compare.
+#
+# @protocol: COLO passthrough just support TCP and UDP.
+#
+# @port: TCP or UDP port number.
+#
+# Returns: Nothing on success
+#
+# Since: 5.3
+#
+# Example:
+#
+# -> { "execute": "colo-passthrough-add",
+#      "arguments": { "protocol": "tcp", "port": 3389 } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'colo-passthrough-add',
+     'data': {'protocol': 'str', 'port': 'uint32'} }
+
+##
+# @colo-passthrough-del:
+#
+# Delete passthrough entry according to customer's needs in COLO-compare.
+#
+# @protocol: COLO passthrough just support TCP and UDP.
+#
+# @port: TCP or UDP port number.
+#
+# Returns: Nothing on success
+#
+# Since: 5.3
+#
+# Example:
+#
+# -> { "execute": "colo-passthrough-del",
+#      "arguments": { "protocol": "tcp", "port": 3389 } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'colo-passthrough-del',
+     'data': {'protocol': 'str', 'port': 'uint32'} }
-- 
2.17.1

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

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

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    | 20 ++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 470a420c2d..f5790782d6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1341,6 +1341,32 @@ SRST
   Remove host network device.
 ERST
 
+    {
+        .name       = "colo_passthrough_add",
+        .args_type  = "protocol:s,port:i",
+        .params     = "protocol port",
+        .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,port:i",
+        .params     = "protocol port",
+        .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:O",
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index ed2913fd18..3c4943b09f 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -81,6 +81,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 65d8ff4849..ab98a6b77d 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1661,6 +1661,26 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);
 }
 
+void hmp_colo_passthrough_add(Monitor *mon, const QDict *qdict)
+{
+    const char *prot = qdict_get_str(qdict, "protocol");
+    uint32_t port = qdict_get_int(qdict, "port");
+    Error *err = NULL;
+
+    qmp_colo_passthrough_add(prot, port, &err);
+    hmp_handle_error(mon, err);
+}
+
+void hmp_colo_passthrough_del(Monitor *mon, const QDict *qdict)
+{
+    const char *prot = qdict_get_str(qdict, "protocol");
+    uint32_t port = qdict_get_int(qdict, "port");
+    Error *err = NULL;
+
+    qmp_colo_passthrough_del(prot, port, &err);
+    hmp_handle_error(mon, err);
+}
+
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
-- 
2.17.1

[PATCH 3/3] net/colo-compare: Add handler for passthrough connection

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

Currently, we just use guest's TCP/UDP source port as the key
to bypass certain network traffic.

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

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 337025b44f..11a32caa9b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
 static NotifierList colo_compare_notifiers =
     NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
 
+static QLIST_HEAD(, PassthroughEntry) passthroughlist =
+    QLIST_HEAD_INITIALIZER(passthroughlist);
+
 #define COMPARE_READ_LEN_MAX NET_BUFSIZE
 #define MAX_QUEUE_SIZE 1024
 
@@ -103,6 +106,12 @@ typedef struct SendEntry {
     uint8_t *buf;
 } SendEntry;
 
+typedef struct PassthroughEntry {
+    bool is_tcp;
+    uint16_t port;
+    QLIST_ENTRY(PassthroughEntry) node;
+} PassthroughEntry;
+
 struct CompareState {
     Object parent;
 
@@ -247,6 +256,7 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
     ConnectionKey key;
     Packet *pkt = NULL;
     Connection *conn;
+    PassthroughEntry *bypass, *next;
     int ret;
 
     if (mode == PRIMARY_IN) {
@@ -264,8 +274,23 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
         pkt = NULL;
         return -1;
     }
+
     fill_connection_key(pkt, &key);
 
+    /* Check COLO passthrough connenction */
+    if (!QLIST_EMPTY(&passthroughlist)) {
+        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
+            if (((key.ip_proto == IPPROTO_TCP) && bypass->is_tcp) ||
+                ((key.ip_proto == IPPROTO_UDP) && !bypass->is_tcp)) {
+                if (bypass->port == key.src_port) {
+                    packet_destroy(pkt, NULL);
+                    pkt = NULL;
+                    return -1;
+                }
+            }
+        }
+    }
+
     conn = connection_get(s->connection_track_table,
                           &key,
                           &s->conn_list);
@@ -1373,6 +1398,30 @@ static void colo_flush_packets(void *opaque, void *user_data)
     }
 }
 
+void colo_compare_passthrough_add(bool is_tcp, const uint16_t port)
+{
+    PassthroughEntry *bypass = NULL;
+
+    bypass = g_new0(PassthroughEntry, 1);
+    bypass->is_tcp = is_tcp;
+    bypass->port = port;
+    QLIST_INSERT_HEAD(&passthroughlist, bypass, node);
+}
+
+void colo_compare_passthrough_del(bool is_tcp, const uint16_t port)
+{
+    PassthroughEntry *bypass = NULL, *next = NULL;
+
+    if (!QLIST_EMPTY(&passthroughlist)) {
+        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
+            if ((bypass->is_tcp == is_tcp) && (bypass->port == port)) {
+                QLIST_REMOVE(bypass, node);
+                g_free(bypass);
+            }
+        }
+    }
+}
+
 static void colo_compare_class_init(ObjectClass *oc, void *data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
diff --git a/net/colo-compare.h b/net/colo-compare.h
index 22ddd512e2..1fa026c85e 100644
--- a/net/colo-compare.h
+++ b/net/colo-compare.h
@@ -20,5 +20,7 @@
 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);
+void colo_compare_passthrough_add(bool is_tcp, const uint16_t port);
+void colo_compare_passthrough_del(bool is_tcp, const uint16_t port);
 
 #endif /* QEMU_COLO_COMPARE_H */
diff --git a/net/net.c b/net/net.c
index eac7a92618..1f303e8309 100644
--- a/net/net.c
+++ b/net/net.c
@@ -55,6 +55,7 @@
 #include "sysemu/sysemu.h"
 #include "net/filter.h"
 #include "qapi/string-output-visitor.h"
+#include "net/colo-compare.h"
 
 /* Net bridge is currently not supported for W32. */
 #if !defined(_WIN32)
@@ -1155,12 +1156,38 @@ void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
                               Error **errp)
 {
     /* Setup passthrough connection */
+    if (port > 65536) {
+        error_setg(errp, "COLO pass through get wrong port");
+        return;
+    }
+
+    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
+        colo_compare_passthrough_add(true, (uint16_t)port);
+    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
+        colo_compare_passthrough_add(false, (uint16_t)port);
+    } else {
+        error_setg(errp, "COLO pass through just support tcp or udp protocol");
+        return;
+    }
 }
 
 void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
                               Error **errp)
 {
     /* Delete passthrough connection */
+    if (port > 65536) {
+        error_setg(errp, "COLO pass through get wrong port");
+        return;
+    }
+
+    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
+        colo_compare_passthrough_del(true, (uint16_t)port);
+    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
+        colo_compare_passthrough_del(false, (uint16_t)port);
+    } else {
+        error_setg(errp, "COLO pass through just support tcp or udp protocol");
+        return;
+    }
 }
 
 static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
-- 
2.17.1

Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Jason Wang]

On 2020/12/24 上午9:09, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
>
> Since the real user scenario does not need to monitor all traffic.
> Add colo-passthrough-add and colo-passthrough-del to maintain
> a COLO network passthrough list.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>   net/net.c     | 12 ++++++++++++
>   qapi/net.json | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 58 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index e1035f21d1..eac7a92618 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1151,6 +1151,18 @@ void qmp_netdev_del(const char *id, Error **errp)
>       qemu_del_net_client(nc);
>   }
>   
> +void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
> +                              Error **errp)
> +{
> +    /* Setup passthrough connection */
> +}
> +
> +void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
> +                              Error **errp)
> +{
> +    /* Delete passthrough connection */
> +}
> +
>   static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
>   {
>       char *str;
> diff --git a/qapi/net.json b/qapi/net.json
> index c31748c87f..466c29714e 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -714,3 +714,49 @@
>   ##
>   { 'event': 'FAILOVER_NEGOTIATED',
>     'data': {'device-id': 'str'} }
> +
> +##
> +# @colo-passthrough-add:
> +#
> +# Add passthrough entry according to customer's needs in COLO-compare.
> +#
> +# @protocol: COLO passthrough just support TCP and UDP.
> +#
> +# @port: TCP or UDP port number.
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 5.3
> +#
> +# Example:
> +#
> +# -> { "execute": "colo-passthrough-add",
> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'colo-passthrough-add',
> +     'data': {'protocol': 'str', 'port': 'uint32'} }


Do we plan to support 4-tuple (src ip,src port, dst ip, dst port) in the 
future? If yes, let's add them now.

And do we plan to support wildcard here?

Thanks


> +
> +##
> +# @colo-passthrough-del:
> +#
> +# Delete passthrough entry according to customer's needs in COLO-compare.
> +#
> +# @protocol: COLO passthrough just support TCP and UDP.
> +#
> +# @port: TCP or UDP port number.
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 5.3
> +#
> +# Example:
> +#
> +# -> { "execute": "colo-passthrough-del",
> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'colo-passthrough-del',
> +     'data': {'protocol': 'str', 'port': 'uint32'} }

Re: [PATCH 0/3] Bypass specific network traffic in COLO

[Jason Wang]

On 2020/12/24 上午9:09, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
>
> Since the real user scenario does not need to monitor all traffic.


Hi Chen:

It would be better to elaborate more on this. E.g what scenario and who 
will use those new QMP/HMP commands.

Thanks


> This series give user ability to bypass kinds of network stream.
>
> Zhang Chen (3):
>    qapi/net: Add new QMP command for COLO passthrough
>    hmp-commands: Add new HMP command for COLO passthrough
>    net/colo-compare: Add handler for passthrough connection
>
>   hmp-commands.hx       | 26 +++++++++++++++++++++++
>   include/monitor/hmp.h |  2 ++
>   monitor/hmp-cmds.c    | 20 ++++++++++++++++++
>   net/colo-compare.c    | 49 +++++++++++++++++++++++++++++++++++++++++++
>   net/colo-compare.h    |  2 ++
>   net/net.c             | 39 ++++++++++++++++++++++++++++++++++
>   qapi/net.json         | 46 ++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 184 insertions(+)
>

RE: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Friday, December 25, 2020 2:20 PM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> passthrough
> 
> 
> On 2020/12/24 上午9:09, Zhang Chen wrote:
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Since the real user scenario does not need to monitor all traffic.
> > Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
> > network passthrough list.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >   net/net.c     | 12 ++++++++++++
> >   qapi/net.json | 46
> ++++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 58 insertions(+)
> >
> > diff --git a/net/net.c b/net/net.c
> > index e1035f21d1..eac7a92618 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1151,6 +1151,18 @@ void qmp_netdev_del(const char *id, Error
> **errp)
> >       qemu_del_net_client(nc);
> >   }
> >
> > +void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
> > +                              Error **errp) {
> > +    /* Setup passthrough connection */ }
> > +
> > +void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
> > +                              Error **errp) {
> > +    /* Delete passthrough connection */ }
> > +
> >   static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
> >   {
> >       char *str;
> > diff --git a/qapi/net.json b/qapi/net.json index
> > c31748c87f..466c29714e 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -714,3 +714,49 @@
> >   ##
> >   { 'event': 'FAILOVER_NEGOTIATED',
> >     'data': {'device-id': 'str'} }
> > +
> > +##
> > +# @colo-passthrough-add:
> > +#
> > +# Add passthrough entry according to customer's needs in COLO-compare.
> > +#
> > +# @protocol: COLO passthrough just support TCP and UDP.
> > +#
> > +# @port: TCP or UDP port number.
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 5.3
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-add",
> > +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-add',
> > +     'data': {'protocol': 'str', 'port': 'uint32'} }
> 
> 
> Do we plan to support 4-tuple (src ip,src port, dst ip, dst port) in the future? If
> yes, let's add them now.
> 
> And do we plan to support wildcard here?

I think just using the port is enough for COLO compare.
Because in this case, users need passthrough some guest services are distinguished by static ports.
And for support 4-tuple and wildcard are a good question, do you think we should add some passthrough
Mechanism for all Qemu net filter? If yes, we should support that in the future.

Thanks
Chen

> 
> Thanks
> 
> 
> > +
> > +##
> > +# @colo-passthrough-del:
> > +#
> > +# Delete passthrough entry according to customer's needs in COLO-
> compare.
> > +#
> > +# @protocol: COLO passthrough just support TCP and UDP.
> > +#
> > +# @port: TCP or UDP port number.
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 5.3
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-del",
> > +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-del',
> > +     'data': {'protocol': 'str', 'port': 'uint32'} }

RE: [PATCH 0/3] Bypass specific network traffic in COLO

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Friday, December 25, 2020 2:23 PM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 0/3] Bypass specific network traffic in COLO
> 
> 
> On 2020/12/24 上午9:09, Zhang Chen wrote:
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Since the real user scenario does not need to monitor all traffic.
> 
> 
> Hi Chen:
> 
> It would be better to elaborate more on this. E.g what scenario and who will
> use those new QMP/HMP commands.

OK, I will add more commit log in next version.

Thanks
Chen

> 
> Thanks
> 
> 
> > This series give user ability to bypass kinds of network stream.
> >
> > Zhang Chen (3):
> >    qapi/net: Add new QMP command for COLO passthrough
> >    hmp-commands: Add new HMP command for COLO passthrough
> >    net/colo-compare: Add handler for passthrough connection
> >
> >   hmp-commands.hx       | 26 +++++++++++++++++++++++
> >   include/monitor/hmp.h |  2 ++
> >   monitor/hmp-cmds.c    | 20 ++++++++++++++++++
> >   net/colo-compare.c    | 49
> +++++++++++++++++++++++++++++++++++++++++++
> >   net/colo-compare.h    |  2 ++
> >   net/net.c             | 39 ++++++++++++++++++++++++++++++++++
> >   qapi/net.json         | 46
> ++++++++++++++++++++++++++++++++++++++++
> >   7 files changed, 184 insertions(+)
> >

Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Jason Wang]

On 2020/12/28 上午8:38, Zhang, Chen wrote:
>
>> -----Original Message-----
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Friday, December 25, 2020 2:20 PM
>> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
>> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
>> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
>> Cc: Zhang Chen <zhangckid@gmail.com>
>> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
>> passthrough
>>
>>
>> On 2020/12/24 上午9:09, Zhang Chen wrote:
>>> From: Zhang Chen <chen.zhang@intel.com>
>>>
>>> Since the real user scenario does not need to monitor all traffic.
>>> Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
>>> network passthrough list.
>>>
>>> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
>>> ---
>>>    net/net.c     | 12 ++++++++++++
>>>    qapi/net.json | 46
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>    2 files changed, 58 insertions(+)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index e1035f21d1..eac7a92618 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -1151,6 +1151,18 @@ void qmp_netdev_del(const char *id, Error
>> **errp)
>>>        qemu_del_net_client(nc);
>>>    }
>>>
>>> +void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
>>> +                              Error **errp) {
>>> +    /* Setup passthrough connection */ }
>>> +
>>> +void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
>>> +                              Error **errp) {
>>> +    /* Delete passthrough connection */ }
>>> +
>>>    static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
>>>    {
>>>        char *str;
>>> diff --git a/qapi/net.json b/qapi/net.json index
>>> c31748c87f..466c29714e 100644
>>> --- a/qapi/net.json
>>> +++ b/qapi/net.json
>>> @@ -714,3 +714,49 @@
>>>    ##
>>>    { 'event': 'FAILOVER_NEGOTIATED',
>>>      'data': {'device-id': 'str'} }
>>> +
>>> +##
>>> +# @colo-passthrough-add:
>>> +#
>>> +# Add passthrough entry according to customer's needs in COLO-compare.
>>> +#
>>> +# @protocol: COLO passthrough just support TCP and UDP.
>>> +#
>>> +# @port: TCP or UDP port number.
>>> +#
>>> +# Returns: Nothing on success
>>> +#
>>> +# Since: 5.3
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "colo-passthrough-add",
>>> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
>>> +# <- { "return": {} }
>>> +#
>>> +##
>>> +{ 'command': 'colo-passthrough-add',
>>> +     'data': {'protocol': 'str', 'port': 'uint32'} }
>>
>> Do we plan to support 4-tuple (src ip,src port, dst ip, dst port) in the future? If
>> yes, let's add them now.
>>
>> And do we plan to support wildcard here?
> I think just using the port is enough for COLO compare.
> Because in this case, users need passthrough some guest services are distinguished by static ports.
> And for support 4-tuple and wildcard are a good question, do you think we should add some passthrough
> Mechanism for all Qemu net filter? If yes, we should support that in the future.


I think we can start form COLO. To avoid QMP compatibility issues, I 
would like to add the n tuple and wildcard support now.

Thanks


>
> Thanks
> Chen
>
>> Thanks
>>
>>
>>> +
>>> +##
>>> +# @colo-passthrough-del:
>>> +#
>>> +# Delete passthrough entry according to customer's needs in COLO-
>> compare.
>>> +#
>>> +# @protocol: COLO passthrough just support TCP and UDP.
>>> +#
>>> +# @port: TCP or UDP port number.
>>> +#
>>> +# Returns: Nothing on success
>>> +#
>>> +# Since: 5.3
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "colo-passthrough-del",
>>> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
>>> +# <- { "return": {} }
>>> +#
>>> +##
>>> +{ 'command': 'colo-passthrough-del',
>>> +     'data': {'protocol': 'str', 'port': 'uint32'} }

RE: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, December 28, 2020 3:11 PM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> passthrough
> 
> 
> On 2020/12/28 上午8:38, Zhang, Chen wrote:
> >
> >> -----Original Message-----
> >> From: Jason Wang <jasowang@redhat.com>
> >> Sent: Friday, December 25, 2020 2:20 PM
> >> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> >> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> >> Gilbert <dgilbert@redhat.com>; Markus Armbruster
> <armbru@redhat.com>
> >> Cc: Zhang Chen <zhangckid@gmail.com>
> >> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> >> passthrough
> >>
> >>
> >> On 2020/12/24 上午9:09, Zhang Chen wrote:
> >>> From: Zhang Chen <chen.zhang@intel.com>
> >>>
> >>> Since the real user scenario does not need to monitor all traffic.
> >>> Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
> >>> network passthrough list.
> >>>
> >>> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> >>> ---
> >>>    net/net.c     | 12 ++++++++++++
> >>>    qapi/net.json | 46
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >>>    2 files changed, 58 insertions(+)
> >>>
> >>> diff --git a/net/net.c b/net/net.c
> >>> index e1035f21d1..eac7a92618 100644
> >>> --- a/net/net.c
> >>> +++ b/net/net.c
> >>> @@ -1151,6 +1151,18 @@ void qmp_netdev_del(const char *id, Error
> >> **errp)
> >>>        qemu_del_net_client(nc);
> >>>    }
> >>>
> >>> +void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
> >>> +                              Error **errp) {
> >>> +    /* Setup passthrough connection */ }
> >>> +
> >>> +void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
> >>> +                              Error **errp) {
> >>> +    /* Delete passthrough connection */ }
> >>> +
> >>>    static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
> >>>    {
> >>>        char *str;
> >>> diff --git a/qapi/net.json b/qapi/net.json index
> >>> c31748c87f..466c29714e 100644
> >>> --- a/qapi/net.json
> >>> +++ b/qapi/net.json
> >>> @@ -714,3 +714,49 @@
> >>>    ##
> >>>    { 'event': 'FAILOVER_NEGOTIATED',
> >>>      'data': {'device-id': 'str'} }
> >>> +
> >>> +##
> >>> +# @colo-passthrough-add:
> >>> +#
> >>> +# Add passthrough entry according to customer's needs in COLO-
> compare.
> >>> +#
> >>> +# @protocol: COLO passthrough just support TCP and UDP.
> >>> +#
> >>> +# @port: TCP or UDP port number.
> >>> +#
> >>> +# Returns: Nothing on success
> >>> +#
> >>> +# Since: 5.3
> >>> +#
> >>> +# Example:
> >>> +#
> >>> +# -> { "execute": "colo-passthrough-add",
> >>> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> >>> +# <- { "return": {} }
> >>> +#
> >>> +##
> >>> +{ 'command': 'colo-passthrough-add',
> >>> +     'data': {'protocol': 'str', 'port': 'uint32'} }
> >>
> >> Do we plan to support 4-tuple (src ip,src port, dst ip, dst port) in
> >> the future? If yes, let's add them now.
> >>
> >> And do we plan to support wildcard here?
> > I think just using the port is enough for COLO compare.
> > Because in this case, users need passthrough some guest services are
> distinguished by static ports.
> > And for support 4-tuple and wildcard are a good question, do you think
> > we should add some passthrough Mechanism for all Qemu net filter? If yes,
> we should support that in the future.
> 
> 
> I think we can start form COLO. To avoid QMP compatibility issues, I would
> like to add the n tuple and wildcard support now.

OK, I will do this job in next version.
For the QMP compatibility issues, please give me a demo of what we want to see, Like some existing commands.

Thanks
Chen

> 
> Thanks
> 
> 
> >
> > Thanks
> > Chen
> >
> >> Thanks
> >>
> >>
> >>> +
> >>> +##
> >>> +# @colo-passthrough-del:
> >>> +#
> >>> +# Delete passthrough entry according to customer's needs in COLO-
> >> compare.
> >>> +#
> >>> +# @protocol: COLO passthrough just support TCP and UDP.
> >>> +#
> >>> +# @port: TCP or UDP port number.
> >>> +#
> >>> +# Returns: Nothing on success
> >>> +#
> >>> +# Since: 5.3
> >>> +#
> >>> +# Example:
> >>> +#
> >>> +# -> { "execute": "colo-passthrough-del",
> >>> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> >>> +# <- { "return": {} }
> >>> +#
> >>> +##
> >>> +{ 'command': 'colo-passthrough-del',
> >>> +     'data': {'protocol': 'str', 'port': 'uint32'} }

Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Jason Wang]

On 2020/12/29 上午10:56, Zhang, Chen wrote:
>> I think we can start form COLO. To avoid QMP compatibility issues, I would
>> like to add the n tuple and wildcard support now.
> OK, I will do this job in next version.
> For the QMP compatibility issues, please give me a demo of what we want to see, Like some existing commands.


I meant if we start from port and then want to add e.g n-tuple support. 
Do we need to introduce another command? Or is there any introspection 
that can let management layer know about this?

Thanks


>
> Thanks
> Chen
>

Re: [PATCH 0/3] Bypass specific network traffic in COLO

[Dr. David Alan Gilbert]

* Zhang Chen (chen.zhang@intel.com) wrote:
> From: Zhang Chen <chen.zhang@intel.com>
> 
> Since the real user scenario does not need to monitor all traffic.

Can you explain the type of real user case where they only need to
compare some connections?

Dave

> This series give user ability to bypass kinds of network stream.
> 
> Zhang Chen (3):
>   qapi/net: Add new QMP command for COLO passthrough
>   hmp-commands: Add new HMP command for COLO passthrough
>   net/colo-compare: Add handler for passthrough connection
> 
>  hmp-commands.hx       | 26 +++++++++++++++++++++++
>  include/monitor/hmp.h |  2 ++
>  monitor/hmp-cmds.c    | 20 ++++++++++++++++++
>  net/colo-compare.c    | 49 +++++++++++++++++++++++++++++++++++++++++++
>  net/colo-compare.h    |  2 ++
>  net/net.c             | 39 ++++++++++++++++++++++++++++++++++
>  qapi/net.json         | 46 ++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 184 insertions(+)
> 
> -- 
> 2.17.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

RE: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, December 30, 2020 11:57 AM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> passthrough
> 
> 
> On 2020/12/29 上午10:56, Zhang, Chen wrote:
> >> I think we can start form COLO. To avoid QMP compatibility issues, I
> >> would like to add the n tuple and wildcard support now.
> > OK, I will do this job in next version.
> > For the QMP compatibility issues, please give me a demo of what we want
> to see, Like some existing commands.
> 
> 
> I meant if we start from port and then want to add e.g n-tuple support.
> Do we need to introduce another command? Or is there any introspection
> that can let management layer know about this?

OK, I will add the n-tuple support.
It looks basic command are add/del connection, Do you think something needs to be introduced?
For the management layer, I don't know the detail process of how to add new Qemu command support for example libvirt.
Maybe depend on libvirt community's plan?

Thanks
Chen 

> 
> Thanks
> 
> 
> >
> > Thanks
> > Chen
> >

RE: [PATCH 0/3] Bypass specific network traffic in COLO

[Zhang, Chen]

> -----Original Message-----
> From: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Sent: Monday, January 4, 2021 9:07 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Markus Armbruster
> <armbru@redhat.com>; Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 0/3] Bypass specific network traffic in COLO
> 
> * Zhang Chen (chen.zhang@intel.com) wrote:
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Since the real user scenario does not need to monitor all traffic.
> 
> Can you explain the type of real user case where they only need to compare
> some connections?

Sure.
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 to drop guest performance.

Thanks
Chen


> 
> Dave
> 
> > This series give user ability to bypass kinds of network stream.
> >
> > Zhang Chen (3):
> >   qapi/net: Add new QMP command for COLO passthrough
> >   hmp-commands: Add new HMP command for COLO passthrough
> >   net/colo-compare: Add handler for passthrough connection
> >
> >  hmp-commands.hx       | 26 +++++++++++++++++++++++
> >  include/monitor/hmp.h |  2 ++
> >  monitor/hmp-cmds.c    | 20 ++++++++++++++++++
> >  net/colo-compare.c    | 49
> +++++++++++++++++++++++++++++++++++++++++++
> >  net/colo-compare.h    |  2 ++
> >  net/net.c             | 39 ++++++++++++++++++++++++++++++++++
> >  qapi/net.json         | 46
> ++++++++++++++++++++++++++++++++++++++++
> >  7 files changed, 184 insertions(+)
> >
> > --
> > 2.17.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Jason Wang]

On 2021/1/5 上午11:28, Zhang, Chen wrote:
>
>> -----Original Message-----
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Wednesday, December 30, 2020 11:57 AM
>> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
>> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
>> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
>> Cc: Zhang Chen <zhangckid@gmail.com>
>> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
>> passthrough
>>
>>
>> On 2020/12/29 上午10:56, Zhang, Chen wrote:
>>>> I think we can start form COLO. To avoid QMP compatibility issues, I
>>>> would like to add the n tuple and wildcard support now.
>>> OK, I will do this job in next version.
>>> For the QMP compatibility issues, please give me a demo of what we want
>> to see, Like some existing commands.
>>
>>
>> I meant if we start from port and then want to add e.g n-tuple support.
>> Do we need to introduce another command? Or is there any introspection
>> that can let management layer know about this?
> OK, I will add the n-tuple support.
> It looks basic command are add/del connection, Do you think something needs to be introduced?


It looks to me it's fine to start with them.


> For the management layer, I don't know the detail process of how to add new Qemu command support for example libvirt.
> Maybe depend on libvirt community's plan?


So a question here, how COLO is being used now. Is it expected to be 
managed by libvirt or not?

Thanks


>
> Thanks
> Chen
>
>> Thanks
>>
>>
>>> Thanks
>>> Chen
>>>

RE: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Tuesday, January 5, 2021 12:17 PM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> passthrough
> 
> 
> On 2021/1/5 上午11:28, Zhang, Chen wrote:
> >
> >> -----Original Message-----
> >> From: Jason Wang <jasowang@redhat.com>
> >> Sent: Wednesday, December 30, 2020 11:57 AM
> >> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> >> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> >> Gilbert <dgilbert@redhat.com>; Markus Armbruster
> <armbru@redhat.com>
> >> Cc: Zhang Chen <zhangckid@gmail.com>
> >> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> >> passthrough
> >>
> >>
> >> On 2020/12/29 上午10:56, Zhang, Chen wrote:
> >>>> I think we can start form COLO. To avoid QMP compatibility issues,
> >>>> I would like to add the n tuple and wildcard support now.
> >>> OK, I will do this job in next version.
> >>> For the QMP compatibility issues, please give me a demo of what we
> >>> want
> >> to see, Like some existing commands.
> >>
> >>
> >> I meant if we start from port and then want to add e.g n-tuple support.
> >> Do we need to introduce another command? Or is there any
> >> introspection that can let management layer know about this?
> > OK, I will add the n-tuple support.
> > It looks basic command are add/del connection, Do you think something
> needs to be introduced?
> 
> 
> It looks to me it's fine to start with them.
> 
> 
> > For the management layer, I don't know the detail process of how to add
> new Qemu command support for example libvirt.
> > Maybe depend on libvirt community's plan?
> 
> 
> So a question here, how COLO is being used now. Is it expected to be
> managed by libvirt or not?

Yes, they use libvirt to manage COLO.
Currently, China CSP integrated COLO on edge computing product, they need change a small part of the code on libvirt,
Most of COLO parameter use libvirt bypass mechanism direct input to Qemu.
For our side, we focus on Qemu COLO code.

Thanks
Chen

> 
> Thanks
> 
> 
> >
> > Thanks
> > Chen
> >
> >> Thanks
> >>
> >>
> >>> Thanks
> >>> Chen
> >>>

Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

[Lukas Straub]

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

On Thu, 24 Dec 2020 09:09:18 +0800
Zhang Chen <chen.zhang@intel.com > wrote:

> From: Zhang Chen <chen.zhang@intel.com>
> 
> Currently, we just use guest's TCP/UDP source port as the key
> to bypass certain network traffic.
> 
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  net/colo-compare.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>  net/colo-compare.h |  2 ++
>  net/net.c          | 27 +++++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
> 
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 337025b44f..11a32caa9b 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
>  static NotifierList colo_compare_notifiers =
>      NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
>  
> +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> +    QLIST_HEAD_INITIALIZER(passthroughlist);
> +

Hi,
I think this should be per colo-compare instance e.g. inside 'struct CompareState'.

>  #define COMPARE_READ_LEN_MAX NET_BUFSIZE
>  #define MAX_QUEUE_SIZE 1024
>  
> @@ -103,6 +106,12 @@ typedef struct SendEntry {
>      uint8_t *buf;
>  } SendEntry;
>  
> +typedef struct PassthroughEntry {
> +    bool is_tcp;
> +    uint16_t port;
> +    QLIST_ENTRY(PassthroughEntry) node;
> +} PassthroughEntry;
> +
>  struct CompareState {
>      Object parent;
>  
> @@ -247,6 +256,7 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
>      ConnectionKey key;
>      Packet *pkt = NULL;
>      Connection *conn;
> +    PassthroughEntry *bypass, *next;
>      int ret;
>  
>      if (mode == PRIMARY_IN) {
> @@ -264,8 +274,23 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
>          pkt = NULL;
>          return -1;
>      }
> +
>      fill_connection_key(pkt, &key);
>  
> +    /* Check COLO passthrough connenction */
> +    if (!QLIST_EMPTY(&passthroughlist)) {
> +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> +            if (((key.ip_proto == IPPROTO_TCP) && bypass->is_tcp) ||
> +                ((key.ip_proto == IPPROTO_UDP) && !bypass->is_tcp)) {
> +                if (bypass->port == key.src_port) {
> +                    packet_destroy(pkt, NULL);
> +                    pkt = NULL;
> +                    return -1;
> +                }
> +            }
> +        }
> +    }
> +
>      conn = connection_get(s->connection_track_table,
>                            &key,
>                            &s->conn_list);
> @@ -1373,6 +1398,30 @@ static void colo_flush_packets(void *opaque, void *user_data)
>      }
>  }
>  
> +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port)
> +{
> +    PassthroughEntry *bypass = NULL;
> +
> +    bypass = g_new0(PassthroughEntry, 1);
> +    bypass->is_tcp = is_tcp;
> +    bypass->port = port;
> +    QLIST_INSERT_HEAD(&passthroughlist, bypass, node);
> +}
> +
> +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port)
> +{
> +    PassthroughEntry *bypass = NULL, *next = NULL;
> +
> +    if (!QLIST_EMPTY(&passthroughlist)) {
> +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> +            if ((bypass->is_tcp == is_tcp) && (bypass->port == port)) {
> +                QLIST_REMOVE(bypass, node);
> +                g_free(bypass);
> +            }
> +        }
> +    }
> +}
> +
>  static void colo_compare_class_init(ObjectClass *oc, void *data)
>  {
>      UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> diff --git a/net/colo-compare.h b/net/colo-compare.h
> index 22ddd512e2..1fa026c85e 100644
> --- a/net/colo-compare.h
> +++ b/net/colo-compare.h
> @@ -20,5 +20,7 @@
>  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);
> +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port);
> +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port);
>  
>  #endif /* QEMU_COLO_COMPARE_H */
> diff --git a/net/net.c b/net/net.c
> index eac7a92618..1f303e8309 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -55,6 +55,7 @@
>  #include "sysemu/sysemu.h"
>  #include "net/filter.h"
>  #include "qapi/string-output-visitor.h"
> +#include "net/colo-compare.h"
>  
>  /* Net bridge is currently not supported for W32. */
>  #if !defined(_WIN32)
> @@ -1155,12 +1156,38 @@ void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
>                                Error **errp)
>  {
>      /* Setup passthrough connection */
> +    if (port > 65536) {
> +        error_setg(errp, "COLO pass through get wrong port");
> +        return;
> +    }
> +
> +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> +        colo_compare_passthrough_add(true, (uint16_t)port);
> +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> +        colo_compare_passthrough_add(false, (uint16_t)port);
> +    } else {
> +        error_setg(errp, "COLO pass through just support tcp or udp protocol");
> +        return;
> +    }
>  }
>  
>  void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
>                                Error **errp)
>  {
>      /* Delete passthrough connection */
> +    if (port > 65536) {
> +        error_setg(errp, "COLO pass through get wrong port");
> +        return;
> +    }
> +
> +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> +        colo_compare_passthrough_del(true, (uint16_t)port);
> +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> +        colo_compare_passthrough_del(false, (uint16_t)port);
> +    } else {
> +        error_setg(errp, "COLO pass through just support tcp or udp protocol");
> +        return;
> +    }
>  }
>  
>  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)



-- 


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

Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

[Lukas Straub]

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

On Thu, 24 Dec 2020 09:09:18 +0800
Zhang Chen <chen.zhang@intel.com > wrote:

> From: Zhang Chen <chen.zhang@intel.com>
> 
> Currently, we just use guest's TCP/UDP source port as the key
> to bypass certain network traffic.
> 
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  net/colo-compare.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>  net/colo-compare.h |  2 ++
>  net/net.c          | 27 +++++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
> 
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 337025b44f..11a32caa9b 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
>  static NotifierList colo_compare_notifiers =
>      NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
>  
> +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> +    QLIST_HEAD_INITIALIZER(passthroughlist);
> +
>  #define COMPARE_READ_LEN_MAX NET_BUFSIZE
>  #define MAX_QUEUE_SIZE 1024
>  
> @@ -103,6 +106,12 @@ typedef struct SendEntry {
>      uint8_t *buf;
>  } SendEntry;
>  
> +typedef struct PassthroughEntry {
> +    bool is_tcp;
> +    uint16_t port;
> +    QLIST_ENTRY(PassthroughEntry) node;
> +} PassthroughEntry;
> +
>  struct CompareState {
>      Object parent;
>  
> @@ -247,6 +256,7 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
>      ConnectionKey key;
>      Packet *pkt = NULL;
>      Connection *conn;
> +    PassthroughEntry *bypass, *next;
>      int ret;
>  
>      if (mode == PRIMARY_IN) {
> @@ -264,8 +274,23 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con)
>          pkt = NULL;
>          return -1;
>      }
> +
>      fill_connection_key(pkt, &key);
>  
> +    /* Check COLO passthrough connenction */
> +    if (!QLIST_EMPTY(&passthroughlist)) {
> +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> +            if (((key.ip_proto == IPPROTO_TCP) && bypass->is_tcp) ||
> +                ((key.ip_proto == IPPROTO_UDP) && !bypass->is_tcp)) {
> +                if (bypass->port == key.src_port) {
> +                    packet_destroy(pkt, NULL);
> +                    pkt = NULL;
> +                    return -1;
> +                }
> +            }
> +        }
> +    }
> +
>      conn = connection_get(s->connection_track_table,
>                            &key,
>                            &s->conn_list);
> @@ -1373,6 +1398,30 @@ static void colo_flush_packets(void *opaque, void *user_data)
>      }
>  }
>  
> +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port)
> +{
> +    PassthroughEntry *bypass = NULL;
> +
> +    bypass = g_new0(PassthroughEntry, 1);
> +    bypass->is_tcp = is_tcp;
> +    bypass->port = port;
> +    QLIST_INSERT_HEAD(&passthroughlist, bypass, node);
> +}
> +
> +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port)
> +{
> +    PassthroughEntry *bypass = NULL, *next = NULL;
> +
> +    if (!QLIST_EMPTY(&passthroughlist)) {
> +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> +            if ((bypass->is_tcp == is_tcp) && (bypass->port == port)) {
> +                QLIST_REMOVE(bypass, node);
> +                g_free(bypass);
> +            }
> +        }
> +    }
> +}
> +

Access to "passtroughlist" needs to be protected by a lock, as "packet_enqueue" is called from a different iothread.

>  static void colo_compare_class_init(ObjectClass *oc, void *data)
>  {
>      UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> diff --git a/net/colo-compare.h b/net/colo-compare.h
> index 22ddd512e2..1fa026c85e 100644
> --- a/net/colo-compare.h
> +++ b/net/colo-compare.h
> @@ -20,5 +20,7 @@
>  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);
> +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port);
> +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port);
>  
>  #endif /* QEMU_COLO_COMPARE_H */
> diff --git a/net/net.c b/net/net.c
> index eac7a92618..1f303e8309 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -55,6 +55,7 @@
>  #include "sysemu/sysemu.h"
>  #include "net/filter.h"
>  #include "qapi/string-output-visitor.h"
> +#include "net/colo-compare.h"
>  
>  /* Net bridge is currently not supported for W32. */
>  #if !defined(_WIN32)
> @@ -1155,12 +1156,38 @@ void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
>                                Error **errp)
>  {
>      /* Setup passthrough connection */
> +    if (port > 65536) {
> +        error_setg(errp, "COLO pass through get wrong port");
> +        return;
> +    }
> +
> +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> +        colo_compare_passthrough_add(true, (uint16_t)port);
> +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> +        colo_compare_passthrough_add(false, (uint16_t)port);
> +    } else {
> +        error_setg(errp, "COLO pass through just support tcp or udp protocol");
> +        return;
> +    }
>  }
>  
>  void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
>                                Error **errp)
>  {
>      /* Delete passthrough connection */
> +    if (port > 65536) {
> +        error_setg(errp, "COLO pass through get wrong port");
> +        return;
> +    }
> +
> +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> +        colo_compare_passthrough_del(true, (uint16_t)port);
> +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> +        colo_compare_passthrough_del(false, (uint16_t)port);
> +    } else {
> +        error_setg(errp, "COLO pass through just support tcp or udp protocol");
> +        return;
> +    }
>  }
>  
>  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)



-- 


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

RE: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

[Zhang, Chen]

> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Thursday, January 14, 2021 9:45 PM
> 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>;
> Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough
> connection
> 
> On Thu, 24 Dec 2020 09:09:18 +0800
> Zhang Chen <chen.zhang@intel.com > wrote:
> 
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Currently, we just use guest's TCP/UDP source port as the key to
> > bypass certain network traffic.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  net/colo-compare.c | 49
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  net/colo-compare.h |  2 ++
> >  net/net.c          | 27 +++++++++++++++++++++++++
> >  3 files changed, 78 insertions(+)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 337025b44f..11a32caa9b 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
> > static NotifierList colo_compare_notifiers =
> >      NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> >
> > +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> > +    QLIST_HEAD_INITIALIZER(passthroughlist);
> > +
> 
> Hi,
> I think this should be per colo-compare instance e.g. inside 'struct
> CompareState'.

It looks QMP and HMP also need to add colo-compare object ID to control it.
Do we need make this command more general?

Thanks
Chen

> 
> >  #define COMPARE_READ_LEN_MAX NET_BUFSIZE  #define
> MAX_QUEUE_SIZE 1024
> >
> > @@ -103,6 +106,12 @@ typedef struct SendEntry {
> >      uint8_t *buf;
> >  } SendEntry;
> >
> > +typedef struct PassthroughEntry {
> > +    bool is_tcp;
> > +    uint16_t port;
> > +    QLIST_ENTRY(PassthroughEntry) node; } PassthroughEntry;
> > +
> >  struct CompareState {
> >      Object parent;
> >
> > @@ -247,6 +256,7 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >      ConnectionKey key;
> >      Packet *pkt = NULL;
> >      Connection *conn;
> > +    PassthroughEntry *bypass, *next;
> >      int ret;
> >
> >      if (mode == PRIMARY_IN) {
> > @@ -264,8 +274,23 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >          pkt = NULL;
> >          return -1;
> >      }
> > +
> >      fill_connection_key(pkt, &key);
> >
> > +    /* Check COLO passthrough connenction */
> > +    if (!QLIST_EMPTY(&passthroughlist)) {
> > +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +            if (((key.ip_proto == IPPROTO_TCP) && bypass->is_tcp) ||
> > +                ((key.ip_proto == IPPROTO_UDP) && !bypass->is_tcp)) {
> > +                if (bypass->port == key.src_port) {
> > +                    packet_destroy(pkt, NULL);
> > +                    pkt = NULL;
> > +                    return -1;
> > +                }
> > +            }
> > +        }
> > +    }
> > +
> >      conn = connection_get(s->connection_track_table,
> >                            &key,
> >                            &s->conn_list); @@ -1373,6 +1398,30 @@
> > static void colo_flush_packets(void *opaque, void *user_data)
> >      }
> >  }
> >
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port) {
> > +    PassthroughEntry *bypass = NULL;
> > +
> > +    bypass = g_new0(PassthroughEntry, 1);
> > +    bypass->is_tcp = is_tcp;
> > +    bypass->port = port;
> > +    QLIST_INSERT_HEAD(&passthroughlist, bypass, node); }
> > +
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port) {
> > +    PassthroughEntry *bypass = NULL, *next = NULL;
> > +
> > +    if (!QLIST_EMPTY(&passthroughlist)) {
> > +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +            if ((bypass->is_tcp == is_tcp) && (bypass->port == port)) {
> > +                QLIST_REMOVE(bypass, node);
> > +                g_free(bypass);
> > +            }
> > +        }
> > +    }
> > +}
> > +
> >  static void colo_compare_class_init(ObjectClass *oc, void *data)  {
> >      UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); diff --git
> > a/net/colo-compare.h b/net/colo-compare.h index
> 22ddd512e2..1fa026c85e
> > 100644
> > --- a/net/colo-compare.h
> > +++ b/net/colo-compare.h
> > @@ -20,5 +20,7 @@
> >  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);
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port);
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port);
> >
> >  #endif /* QEMU_COLO_COMPARE_H */
> > diff --git a/net/net.c b/net/net.c
> > index eac7a92618..1f303e8309 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -55,6 +55,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "net/filter.h"
> >  #include "qapi/string-output-visitor.h"
> > +#include "net/colo-compare.h"
> >
> >  /* Net bridge is currently not supported for W32. */  #if
> > !defined(_WIN32) @@ -1155,12 +1156,38 @@ void
> > qmp_colo_passthrough_add(const char *prot, const uint32_t port,
> >                                Error **errp)  {
> >      /* Setup passthrough connection */
> > +    if (port > 65536) {
> > +        error_setg(errp, "COLO pass through get wrong port");
> > +        return;
> > +    }
> > +
> > +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> > +        colo_compare_passthrough_add(true, (uint16_t)port);
> > +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> > +        colo_compare_passthrough_add(false, (uint16_t)port);
> > +    } else {
> > +        error_setg(errp, "COLO pass through just support tcp or udp
> protocol");
> > +        return;
> > +    }
> >  }
> >
> >  void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
> >                                Error **errp)  {
> >      /* Delete passthrough connection */
> > +    if (port > 65536) {
> > +        error_setg(errp, "COLO pass through get wrong port");
> > +        return;
> > +    }
> > +
> > +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> > +        colo_compare_passthrough_del(true, (uint16_t)port);
> > +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> > +        colo_compare_passthrough_del(false, (uint16_t)port);
> > +    } else {
> > +        error_setg(errp, "COLO pass through just support tcp or udp
> protocol");
> > +        return;
> > +    }
> >  }
> >
> >  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
> 
> 
> 
> --

RE: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

[Zhang, Chen]

> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Thursday, January 14, 2021 9:51 PM
> 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>;
> Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough
> connection
> 
> On Thu, 24 Dec 2020 09:09:18 +0800
> Zhang Chen <chen.zhang@intel.com > wrote:
> 
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Currently, we just use guest's TCP/UDP source port as the key to
> > bypass certain network traffic.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  net/colo-compare.c | 49
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  net/colo-compare.h |  2 ++
> >  net/net.c          | 27 +++++++++++++++++++++++++
> >  3 files changed, 78 insertions(+)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 337025b44f..11a32caa9b 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
> > static NotifierList colo_compare_notifiers =
> >      NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> >
> > +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> > +    QLIST_HEAD_INITIALIZER(passthroughlist);
> > +
> >  #define COMPARE_READ_LEN_MAX NET_BUFSIZE  #define
> MAX_QUEUE_SIZE 1024
> >
> > @@ -103,6 +106,12 @@ typedef struct SendEntry {
> >      uint8_t *buf;
> >  } SendEntry;
> >
> > +typedef struct PassthroughEntry {
> > +    bool is_tcp;
> > +    uint16_t port;
> > +    QLIST_ENTRY(PassthroughEntry) node; } PassthroughEntry;
> > +
> >  struct CompareState {
> >      Object parent;
> >
> > @@ -247,6 +256,7 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >      ConnectionKey key;
> >      Packet *pkt = NULL;
> >      Connection *conn;
> > +    PassthroughEntry *bypass, *next;
> >      int ret;
> >
> >      if (mode == PRIMARY_IN) {
> > @@ -264,8 +274,23 @@ static int packet_enqueue(CompareState *s, int
> mode, Connection **con)
> >          pkt = NULL;
> >          return -1;
> >      }
> > +
> >      fill_connection_key(pkt, &key);
> >
> > +    /* Check COLO passthrough connenction */
> > +    if (!QLIST_EMPTY(&passthroughlist)) {
> > +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +            if (((key.ip_proto == IPPROTO_TCP) && bypass->is_tcp) ||
> > +                ((key.ip_proto == IPPROTO_UDP) && !bypass->is_tcp)) {
> > +                if (bypass->port == key.src_port) {
> > +                    packet_destroy(pkt, NULL);
> > +                    pkt = NULL;
> > +                    return -1;
> > +                }
> > +            }
> > +        }
> > +    }
> > +
> >      conn = connection_get(s->connection_track_table,
> >                            &key,
> >                            &s->conn_list); @@ -1373,6 +1398,30 @@
> > static void colo_flush_packets(void *opaque, void *user_data)
> >      }
> >  }
> >
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port) {
> > +    PassthroughEntry *bypass = NULL;
> > +
> > +    bypass = g_new0(PassthroughEntry, 1);
> > +    bypass->is_tcp = is_tcp;
> > +    bypass->port = port;
> > +    QLIST_INSERT_HEAD(&passthroughlist, bypass, node); }
> > +
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port) {
> > +    PassthroughEntry *bypass = NULL, *next = NULL;
> > +
> > +    if (!QLIST_EMPTY(&passthroughlist)) {
> > +        QLIST_FOREACH_SAFE(bypass, &passthroughlist, node, next) {
> > +            if ((bypass->is_tcp == is_tcp) && (bypass->port == port)) {
> > +                QLIST_REMOVE(bypass, node);
> > +                g_free(bypass);
> > +            }
> > +        }
> > +    }
> > +}
> > +
> 
> Access to "passtroughlist" needs to be protected by a lock, as
> "packet_enqueue" is called from a different iothread.

OK, I will add the lock in next version.

Thanks
Chen

> 
> >  static void colo_compare_class_init(ObjectClass *oc, void *data)  {
> >      UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); diff --git
> > a/net/colo-compare.h b/net/colo-compare.h index
> 22ddd512e2..1fa026c85e
> > 100644
> > --- a/net/colo-compare.h
> > +++ b/net/colo-compare.h
> > @@ -20,5 +20,7 @@
> >  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);
> > +void colo_compare_passthrough_add(bool is_tcp, const uint16_t port);
> > +void colo_compare_passthrough_del(bool is_tcp, const uint16_t port);
> >
> >  #endif /* QEMU_COLO_COMPARE_H */
> > diff --git a/net/net.c b/net/net.c
> > index eac7a92618..1f303e8309 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -55,6 +55,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "net/filter.h"
> >  #include "qapi/string-output-visitor.h"
> > +#include "net/colo-compare.h"
> >
> >  /* Net bridge is currently not supported for W32. */  #if
> > !defined(_WIN32) @@ -1155,12 +1156,38 @@ void
> > qmp_colo_passthrough_add(const char *prot, const uint32_t port,
> >                                Error **errp)  {
> >      /* Setup passthrough connection */
> > +    if (port > 65536) {
> > +        error_setg(errp, "COLO pass through get wrong port");
> > +        return;
> > +    }
> > +
> > +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> > +        colo_compare_passthrough_add(true, (uint16_t)port);
> > +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> > +        colo_compare_passthrough_add(false, (uint16_t)port);
> > +    } else {
> > +        error_setg(errp, "COLO pass through just support tcp or udp
> protocol");
> > +        return;
> > +    }
> >  }
> >
> >  void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
> >                                Error **errp)  {
> >      /* Delete passthrough connection */
> > +    if (port > 65536) {
> > +        error_setg(errp, "COLO pass through get wrong port");
> > +        return;
> > +    }
> > +
> > +    if (!strcmp(prot, "tcp") || !strcmp(prot, "TCP")) {
> > +        colo_compare_passthrough_del(true, (uint16_t)port);
> > +    } else if (!strcmp(prot, "udp") || !strcmp(prot, "UDP")) {
> > +        colo_compare_passthrough_del(false, (uint16_t)port);
> > +    } else {
> > +        error_setg(errp, "COLO pass through just support tcp or udp
> protocol");
> > +        return;
> > +    }
> >  }
> >
> >  static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
> 
> 
> 
> --

Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

[Lukas Straub]

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

On Fri, 15 Jan 2021 09:07:47 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Lukas Straub <lukasstraub2@web.de>
> > Sent: Thursday, January 14, 2021 9:45 PM
> > 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>;
> > Zhang Chen <zhangckid@gmail.com>
> > Subject: Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough
> > connection
> > 
> > On Thu, 24 Dec 2020 09:09:18 +0800
> > Zhang Chen <chen.zhang@intel.com > wrote:
> >   
> > > From: Zhang Chen <chen.zhang@intel.com>
> > >
> > > Currently, we just use guest's TCP/UDP source port as the key to
> > > bypass certain network traffic.
> > >
> > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > > ---
> > >  net/colo-compare.c | 49
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > >  net/colo-compare.h |  2 ++
> > >  net/net.c          | 27 +++++++++++++++++++++++++
> > >  3 files changed, 78 insertions(+)
> > >
> > > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > > 337025b44f..11a32caa9b 100644
> > > --- a/net/colo-compare.c
> > > +++ b/net/colo-compare.c
> > > @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
> > > static NotifierList colo_compare_notifiers =
> > >      NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> > >
> > > +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> > > +    QLIST_HEAD_INITIALIZER(passthroughlist);
> > > +  
> > 
> > Hi,
> > I think this should be per colo-compare instance e.g. inside 'struct
> > CompareState'.  
> 
> It looks QMP and HMP also need to add colo-compare object ID to control it.
> Do we need make this command more general?

Yes, it gives more flexibility. For example if the VM a "public" and a separate "management" network interface, passthrough can then be enabled just on the "management" interface.

Regards,
Lukas Straub

> Thanks
> Chen
> 
> >   
> > >  [...]

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

Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Eric Blake]

On 12/23/20 7:09 PM, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
> 
> Since the real user scenario does not need to monitor all traffic.
> Add colo-passthrough-add and colo-passthrough-del to maintain
> a COLO network passthrough list.
> 
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>

> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -714,3 +714,49 @@
>  ##
>  { 'event': 'FAILOVER_NEGOTIATED',
>    'data': {'device-id': 'str'} }
> +
> +##
> +# @colo-passthrough-add:
> +#
> +# Add passthrough entry according to customer's needs in COLO-compare.
> +#
> +# @protocol: COLO passthrough just support TCP and UDP.
> +#
> +# @port: TCP or UDP port number.
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 5.3

The next release is 6.0, not 5.3.

> +#
> +# Example:
> +#
> +# -> { "execute": "colo-passthrough-add",
> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'colo-passthrough-add',
> +     'data': {'protocol': 'str', 'port': 'uint32'} }

Should 'protocol' be an enum (finite set of values) rather than an
open-coded string (infinite number of values, even though you mentioned
in the docs above that only 'tcp' or 'udp' make sense)?  In fact, do we
already have existing QAPI types representing tcp/udp and a port number
that could be reused here, rather than open-coding yet another one?

> +
> +##
> +# @colo-passthrough-del:
> +#
> +# Delete passthrough entry according to customer's needs in COLO-compare.
> +#
> +# @protocol: COLO passthrough just support TCP and UDP.
> +#
> +# @port: TCP or UDP port number.
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 5.3

another 6.0 spot

> +#
> +# Example:
> +#
> +# -> { "execute": "colo-passthrough-del",
> +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'colo-passthrough-del',
> +     'data': {'protocol': 'str', 'port': 'uint32'} }
> 

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

RE: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough

[Zhang, Chen]

> -----Original Message-----
> From: Eric Blake <eblake@redhat.com>
> Sent: Wednesday, January 20, 2021 3:33 AM
> To: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> <jasowang@redhat.com>; qemu-dev <qemu-devel@nongnu.org>; Dr. David
> Alan Gilbert <dgilbert@redhat.com>; Markus Armbruster
> <armbru@redhat.com>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> passthrough
> 
> On 12/23/20 7:09 PM, Zhang Chen wrote:
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Since the real user scenario does not need to monitor all traffic.
> > Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
> > network passthrough list.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> 
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -714,3 +714,49 @@
> >  ##
> >  { 'event': 'FAILOVER_NEGOTIATED',
> >    'data': {'device-id': 'str'} }
> > +
> > +##
> > +# @colo-passthrough-add:
> > +#
> > +# Add passthrough entry according to customer's needs in COLO-compare.
> > +#
> > +# @protocol: COLO passthrough just support TCP and UDP.
> > +#
> > +# @port: TCP or UDP port number.
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 5.3
> 
> The next release is 6.0, not 5.3.

Missed the plan, I will fix it in next version.

> 
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-add",
> > +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-add',
> > +     'data': {'protocol': 'str', 'port': 'uint32'} }
> 
> Should 'protocol' be an enum (finite set of values) rather than an open-
> coded string (infinite number of values, even though you mentioned in the
> docs above that only 'tcp' or 'udp' make sense)?  In fact, do we already have
> existing QAPI types representing tcp/udp and a port number that could be
> reused here, rather than open-coding yet another one?
> 

I checked current QAPI code, looks no existing enum defined.
I will add the new general QAPI types in next version.

Thanks
Chen

> > +
> > +##
> > +# @colo-passthrough-del:
> > +#
> > +# Delete passthrough entry according to customer's needs in COLO-
> compare.
> > +#
> > +# @protocol: COLO passthrough just support TCP and UDP.
> > +#
> > +# @port: TCP or UDP port number.
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 5.3
> 
> another 6.0 spot
> 

OK.

> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-del",
> > +#      "arguments": { "protocol": "tcp", "port": 3389 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-del',
> > +     'data': {'protocol': 'str', 'port': 'uint32'} }
> >
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org