From: Lukas Straub <lukasstraub2@web.de>
To: Zhang Chen <chen.zhang@intel.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
"Li Zhijian" <lizhijian@cn.fujitsu.com>,
"Jason Wang" <jasowang@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
qemu-dev <qemu-devel@nongnu.org>,
"Gerd Hoffmann" <kraxel@redhat.com>,
"Zhang Chen" <zhangckid@gmail.com>,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file.
Date: Mon, 17 May 2021 22:03:40 +0200 [thread overview]
Message-ID: <20210517220340.387bd21e@gecko.fritz.box> (raw)
In-Reply-To: <20210420151537.64360-5-chen.zhang@intel.com>
[-- 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 --]
next prev parent reply other threads:[~2021-05-17 20:05 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210517220340.387bd21e@gecko.fritz.box \
--to=lukasstraub2@web.de \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=chen.zhang@intel.com \
--cc=dgilbert@redhat.com \
--cc=jasowang@redhat.com \
--cc=kraxel@redhat.com \
--cc=lizhijian@cn.fujitsu.com \
--cc=qemu-devel@nongnu.org \
--cc=zhangckid@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).