On 03/31/2016 09:41 AM, Zhang Chen wrote:
On 03/30/2016 05:25 PM, Dr. David Alan Gilbert wrote:
* Zhang Chen (zhangchen.f...@cn.fujitsu.com) wrote:
packet come from primary char indev will be send to
outdev - packet come from secondary char dev will be drop
Please put in the description an example of how you invoke
the filter on the primary and secondary.
OK.
colo-compare get packet from chardev(primary_in,secondary_in),
and output to other chardev(outdev), so, we can use it with the
help of filter-mirror and filter-redirector. like that:
primary:
-netdev
tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
-device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
-chardev socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
-chardev socket,id=compare1,host=3.3.3.3,port=9004,server,nowait
-chardev socket,id=compare0,host=3.3.3.3,port=9001,server,nowait
-chardev socket,id=compare0-0,host=3.3.3.3,port=9001
-chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
-chardev socket,id=compare_out0,host=3.3.3.3,port=9005
-object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
-object
filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out
-object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0
-object
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0
secondary:
-netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down
script=/etc/qemu-ifdown
-device e1000,netdev=hn0,mac=52:a4:00:12:78:66
-chardev socket,id=red0,host=3.3.3.3,port=9003
-chardev socket,id=red1,host=3.3.3.3,port=9004
-object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0
-object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
Signed-off-by: Li Zhijian <lizhij...@cn.fujitsu.com>
Signed-off-by: Zhang Chen <zhangchen.f...@cn.fujitsu.com>
Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
---
net/Makefile.objs | 1 +
net/colo-compare.c | 344
+++++++++++++++++++++++++++++++++++++++++++++++++++++
vl.c | 3 +-
3 files changed, 347 insertions(+), 1 deletion(-)
create mode 100644 net/colo-compare.c
diff --git a/net/Makefile.objs b/net/Makefile.objs
index b7c22fd..ba92f73 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -16,3 +16,4 @@ common-obj-$(CONFIG_NETMAP) += netmap.o
common-obj-y += filter.o
common-obj-y += filter-buffer.o
common-obj-y += filter-mirror.o
+common-obj-y += colo-compare.o
diff --git a/net/colo-compare.c b/net/colo-compare.c
new file mode 100644
index 0000000..62c66df
--- /dev/null
+++ b/net/colo-compare.c
@@ -0,0 +1,344 @@
+/*
+ * Copyright (c) 2016 FUJITSU LIMITED
+ * Author: Zhang Chen <zhangchen.f...@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
+
+#include "net/net.h"
+#include "net/vhost_net.h"
+#include "qom/object_interfaces.h"
+#include "qemu/iov.h"
+#include "qom/object.h"
+#include "qemu/typedefs.h"
+#include "net/queue.h"
+#include "sysemu/char.h"
+#include "qemu/sockets.h"
+
+#define TYPE_COLO_COMPARE "colo-compare"
+#define COLO_COMPARE(obj) \
+ OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE)
+
+#define COMPARE_READ_LEN_MAX NET_BUFSIZE
+
+static QTAILQ_HEAD(, CompareState) net_compares =
+ QTAILQ_HEAD_INITIALIZER(net_compares);
+
+typedef struct ReadState {
+ int state; /* 0 = getting length, 1 = getting data */
+ unsigned int index;
+ unsigned int packet_len;
Please make packet_len and index uint32_t, since they're sent over
the wire
as 32bit.
OK~ will fix.
+ uint8_t buf[COMPARE_READ_LEN_MAX];
+} ReadState;
+
+typedef struct CompareState {
+ Object parent;
+
+ char *pri_indev;
+ char *sec_indev;
+ char *outdev;
+ CharDriverState *chr_pri_in;
+ CharDriverState *chr_sec_in;
+ CharDriverState *chr_out;
+ QTAILQ_ENTRY(CompareState) next;
+ ReadState pri_rs;
+ ReadState sec_rs;
+} CompareState;
+
+static int compare_chr_send(CharDriverState *out, const uint8_t
*buf, int size)
+{
+ int ret = 0;
+ uint32_t len = htonl(size);
+
Similarly, make 'size' uint32_t - everything that gets converted into
a uint32_t
it's probably best to make a uint32_t.
OK
+ if (!size) {
+ return 0;
+ }
+
+ ret = qemu_chr_fe_write_all(out, (uint8_t *)&len, sizeof(len));
+ if (ret != sizeof(len)) {
+ goto err;
+ }
+
+ ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size);
+ if (ret != size) {
+ goto err;
+ }
+
You can make this slightly simpler and save the return 0;
will fix~~
+ return 0;
+
+err:
+ return ret < 0 ? ret : -EIO;
err:
return ret <= 0 ? ret : -EIO;
+}
+
+static int compare_chr_can_read(void *opaque)
+{
+ return COMPARE_READ_LEN_MAX;
+}
+
+/* Returns
+ * 0: readstate is not ready
+ * 1: readstate is ready
+ * otherwise error occurs
+ */
+static int compare_chr_fill_rstate(ReadState *rs, const uint8_t
*buf, int size)
+{
+ unsigned int l;
+ while (size > 0) {
+ /* reassemble a packet from the network */
+ switch (rs->state) { /* 0 = getting length, 1 = getting
data */
+ case 0:
+ l = 4 - rs->index;
+ if (l > size) {
+ l = size;
+ }
+ memcpy(rs->buf + rs->index, buf, l);
+ buf += l;
+ size -= l;
+ rs->index += l;
+ if (rs->index == 4) {
+ /* got length */
+ rs->packet_len = ntohl(*(uint32_t *)rs->buf);
+ rs->index = 0;
+ rs->state = 1;
+ }
+ break;
+ case 1:
+ l = rs->packet_len - rs->index;
+ if (l > size) {
+ l = size;
+ }
+ if (rs->index + l <= sizeof(rs->buf)) {
+ memcpy(rs->buf + rs->index, buf, l);
+ } else {
+ error_report("serious error: oversized packet
received.");
Isn't it easier to do this check above in the 'got length' if ?
Instead of 'serious error' say where it's coming from;
'colo-compare: Received oversized packet over socket'
that makes it a lot easier when people see the error for the first time.
Also, should you check for the error case of receiving a packet where
packet_len == 0 ?
Yes , will fix it in next version.
+ rs->index = rs->state = 0;
+ return -1;
+ }
+
+ rs->index += l;
+ buf += l;
+ size -= l;
+ if (rs->index >= rs->packet_len) {
+ rs->index = 0;
+ rs->state = 0;
+ return 1;
+ }
+ break;
+ }
+ }
+ return 0;
+}
+
+static void compare_pri_chr_in(void *opaque, const uint8_t *buf,
int size)
+{
+ CompareState *s = COLO_COMPARE(opaque);
+ int ret;
+
+ ret = compare_chr_fill_rstate(&s->pri_rs, buf, size);
+ if (ret == 1) {
+ /* FIXME: enqueue to primary packet list */
+ compare_chr_send(s->chr_out, buf, size);
+ } else if (ret == -1) {
+ qemu_chr_add_handlers(s->chr_pri_in, NULL, NULL, NULL, NULL);
+ }
+}
+
+static void compare_sec_chr_in(void *opaque, const uint8_t *buf,
int size)
+{
+ CompareState *s = COLO_COMPARE(opaque);
+ int ret;
+
+ ret = compare_chr_fill_rstate(&s->sec_rs, buf, size);
+ if (ret == 1) {
+ /* TODO: enqueue to secondary packet list*/
+ } else if (ret == -1) {
+ qemu_chr_add_handlers(s->chr_sec_in, NULL, NULL, NULL, NULL);
+ }
+}
+
+static char *compare_get_pri_indev(Object *obj, Error **errp)
+{
+ CompareState *s = COLO_COMPARE(obj);
+
+ return g_strdup(s->pri_indev);
+}
+
+static void compare_set_pri_indev(Object *obj, const char *value,
Error **errp)
+{
+ CompareState *s = COLO_COMPARE(obj);
+
+ g_free(s->pri_indev);
+ s->pri_indev = g_strdup(value);
+}
+
+static char *compare_get_sec_indev(Object *obj, Error **errp)
+{
+ CompareState *s = COLO_COMPARE(obj);
+
+ return g_strdup(s->sec_indev);
+}
+
+static void compare_set_sec_indev(Object *obj, const char *value,
Error **errp)
+{
+ CompareState *s = COLO_COMPARE(obj);
+
+ g_free(s->sec_indev);
+ s->sec_indev = g_strdup(value);
+}
+
+static char *compare_get_outdev(Object *obj, Error **errp)
+{
+ CompareState *s = COLO_COMPARE(obj);
+
+ return g_strdup(s->outdev);
+}
+
+static void compare_set_outdev(Object *obj, const char *value,
Error **errp)
+{
+ CompareState *s = COLO_COMPARE(obj);
+
+ g_free(s->outdev);
+ s->outdev = g_strdup(value);
+}
+
+static void colo_compare_complete(UserCreatable *uc, Error **errp)
+{
+ CompareState *s = COLO_COMPARE(uc);
+
+ if (!s->pri_indev || !s->sec_indev || !s->outdev) {
+ error_setg(errp, "colo compare needs 'primary_in' ,"
+ "'secondary_in','outdev' property set");
+ return;
+ } else if (!strcmp(s->pri_indev, s->outdev) ||
+ !strcmp(s->sec_indev, s->outdev) ||
+ !strcmp(s->pri_indev, s->sec_indev)) {
+ error_setg(errp, "'indev' and 'outdev' could not be same "
+ "for compare module");
+ return;
+ }
+
+ s->chr_pri_in = qemu_chr_find(s->pri_indev);
+ if (s->chr_pri_in == NULL) {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
I think error_set seems to be discouraged these days, just use
error_setg
(see the comment in include/qapi/error.h just above error_set).
OK
+ "IN Device '%s' not found", s->pri_indev);
+ goto out;
+ }
+
+ qemu_chr_fe_claim_no_fail(s->chr_pri_in);
+ qemu_chr_add_handlers(s->chr_pri_in, compare_chr_can_read,
+ compare_pri_chr_in, NULL, s);
+
+ s->chr_sec_in = qemu_chr_find(s->sec_indev);
+ if (s->chr_sec_in == NULL) {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+ "IN Device '%s' not found", s->sec_indev);
+ goto out;
+ }
Can you explain/give an example of the way you create one of these
filters?
Yes, the way on top of this mail.
Would you ever have a pri_indev and sec_indev on the same filter?
Not same one.
If not, then why not just have an 'indev' option rather than the
two separate configs.
If you cna have both then you need to change hte error 'IN Device'
to say either 'Primary IN device' or secondary.
will fix
+ qemu_chr_fe_claim_no_fail(s->chr_sec_in);
+ qemu_chr_add_handlers(s->chr_sec_in, compare_chr_can_read,
+ compare_sec_chr_in, NULL, s);
+
+ s->chr_out = qemu_chr_find(s->outdev);
+ if (s->chr_out == NULL) {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+ "OUT Device '%s' not found", s->outdev);
+ goto out;
+ }
+ qemu_chr_fe_claim_no_fail(s->chr_out);
+
+ QTAILQ_INSERT_TAIL(&net_compares, s, next);
+
+ return;
+
+out:
+ if (s->chr_pri_in) {
+ qemu_chr_add_handlers(s->chr_pri_in, NULL, NULL, NULL, NULL);
+ qemu_chr_fe_release(s->chr_pri_in);
+ s->chr_pri_in = NULL;
+ }
+ if (s->chr_sec_in) {
+ qemu_chr_add_handlers(s->chr_sec_in, NULL, NULL, NULL, NULL);
+ qemu_chr_fe_release(s->chr_sec_in);
+ s->chr_pri_in = NULL;
+ }
Can't you avoid this by making the code:
s->chr_pri_in = qemu_chr_find(...)
if (s->chr_pri_in == NULL) {
error (...)
}
s->chr_sec_in = qemu_chr_find(...)
if (s->chr_sec_in == NULL) {
error (...)
}
s->chr_out = qemu_chr_find(...)
if (s->chr_out == NULL) {
error (...)
}
qemu_chr_fe_claim_no_fail(pri)
add_handlers(pri...)
qemu_chr_fe_claim_no_fail(sec)
add_handlers(sec...)
qemu_chr_fe_claim_no_fail(out)
add_handlers(out...)
so you don't have to clean up the handlers/release in the out: ?
I will fix code style to this.
we don't need set a handler for out, because we just send packet to out.
so we don't have to clean up out.
Thanks
Zhang Chen
+}
+
+static void colo_compare_class_init(ObjectClass *oc, void *data)
+{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+
+ ucc->complete = colo_compare_complete;
+}
+
+static void colo_compare_class_finalize(ObjectClass *oc, void *data)
+{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ CompareState *s = COLO_COMPARE(ucc);
+
+ if (s->chr_pri_in) {
+ qemu_chr_add_handlers(s->chr_pri_in, NULL, NULL, NULL, NULL);
+ qemu_chr_fe_release(s->chr_pri_in);
+ }
+ if (s->chr_sec_in) {
+ qemu_chr_add_handlers(s->chr_sec_in, NULL, NULL, NULL, NULL);
+ qemu_chr_fe_release(s->chr_sec_in);
+ }
+ if (s->chr_out) {
+ qemu_chr_fe_release(s->chr_out);
+ }
+
+ if (!QTAILQ_EMPTY(&net_compares)) {
+ QTAILQ_REMOVE(&net_compares, s, next);
+ }
+}
+
+static void colo_compare_init(Object *obj)
+{
+ object_property_add_str(obj, "primary_in",
+ compare_get_pri_indev,
compare_set_pri_indev,
+ NULL);
+ object_property_add_str(obj, "secondary_in",
+ compare_get_sec_indev,
compare_set_sec_indev,
+ NULL);
+ object_property_add_str(obj, "outdev",
+ compare_get_outdev, compare_set_outdev,
+ NULL);
+}
+
+static void colo_compare_finalize(Object *obj)
+{
+ CompareState *s = COLO_COMPARE(obj);
+
+ g_free(s->pri_indev);
+ g_free(s->sec_indev);
+ g_free(s->outdev);
+}
+
+static const TypeInfo colo_compare_info = {
+ .name = TYPE_COLO_COMPARE,
+ .parent = TYPE_OBJECT,
+ .instance_size = sizeof(CompareState),
+ .instance_init = colo_compare_init,
+ .instance_finalize = colo_compare_finalize,
+ .class_size = sizeof(CompareState),
+ .class_init = colo_compare_class_init,
+ .class_finalize = colo_compare_class_finalize,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_USER_CREATABLE },
+ { }
+ }
+};
+
+static void register_types(void)
+{
+ type_register_static(&colo_compare_info);
+}
+
+type_init(register_types);
diff --git a/vl.c b/vl.c
index dc6e63a..70064ad 100644
--- a/vl.c
+++ b/vl.c
@@ -2842,7 +2842,8 @@ static bool object_create_initial(const char
*type)
if (g_str_equal(type, "filter-buffer") ||
g_str_equal(type, "filter-dump") ||
g_str_equal(type, "filter-mirror") ||
- g_str_equal(type, "filter-redirector")) {
+ g_str_equal(type, "filter-redirector") ||
+ g_str_equal(type, "colo-compare")) {
return false;
}
--
1.9.1
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
.
--
Thanks
zhangchen