On 04/28/2016 03:53 PM, Zhang Chen wrote:
>
>
> On 04/28/2016 02:53 PM, Jason Wang wrote:
>>
>> On 04/18/2016 07:11 PM, Zhang Chen wrote:
>>> packet come from primary char indev will be send to
>>> outdev - packet come from secondary char dev will be drop
>>>
>>> usage:
>>>
>>> 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 | 361
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> qemu-options.hx | 6 +
>>> vl.c | 3 +-
>>> 4 files changed, 370 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..c45b132
>>> --- /dev/null
>>> +++ b/net/colo-compare.c
>>> @@ -0,0 +1,361 @@
>>> +/*
>>> + * 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 "qapi/error.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"
>>> +#include "qapi-visit.h"
>>> +#include "trace.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);
>> Don't see any usage of this, if it was used in the following patches,
>> better move this to that patch.
>
> Hi~ jason~
>
> Thanks for review~~
>
> will fix it in next version.
>
>>
>>> +
>>> +typedef struct ReadState {
>>> + int state; /* 0 = getting length, 1 = getting data */
>>> + uint32_t index;
>>> + uint32_t packet_len;
>>> + 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;
>> What did 'rs' short for, ReadState? :)
>
> Yes,should I change the 'rs' to another name?
Probably not.
>
>>
>>> +} CompareState;
>>> +
>>> +typedef struct CompareClass {
>>> + ObjectClass parent_class;
>>> +} CompareClass;
>>> +
>>> +static int compare_chr_send(CharDriverState *out,
>>> + const uint8_t *buf,
>>> + uint32_t size)
>>> +{
>>> + int ret = 0;
>>> + uint32_t len = htonl(size);
>>> +
>>> + 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;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +err:
>>> + return ret < 0 ? ret : -EIO;
>>> +}
>>> +
>>> +static int compare_chr_can_read(void *opaque)
>>> +{
>>> + return COMPARE_READ_LEN_MAX;
>>> +}
>>> +
>>> +/*
>>> + * called from the main thread on the primary
>>> + * for get packets
>>> + * 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("colo-compare: "
>>> + "Received oversized packet over socket");
>>> + 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;
>>> +}
>> This looks rather similar to redirector_chr_read(), any chance to unify
>> the code?
>
> Yes,I think we can create 'colo-base.c' and 'colo-base.h'
> in net/ to share codes for colo-compare,filter-redirector
> and filter-rewriter. how about it?
You can, but need a generic name since it would be used by redirector
too. Looking at net/socket.c, I wonder whether or not we could reuse
NetSocketState directly.
>
>
>>
>>> +
>>> +/*
>>> + * called from the main thread on the primary for packets
>>> + * arriving over the socket from the primary.
>>> + */
>>> +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 */
>> Do you mean to use some internal data structure instead of chardev? If
>> yes, probably a "TODO", not "FIXME".
>
> Yes,will change to "TODO"
>
>>> + compare_chr_send(s->chr_out, s->pri_rs.buf,
>>> s->pri_rs.packet_len);
>>> + } else if (ret == -1) {
>>> + qemu_chr_add_handlers(s->chr_pri_in, NULL, NULL, NULL, NULL);
>> Should we add a warning here?
>
> OK~ I will add error_report()
>
>>
>>> + }
>>> +}
>>> +
>>> +/*
>>> + * called from the main thread on the primary for packets
>>> + * arriving over the socket from the secondary.
>>> + */
>>> +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*/
>>> + /* should we send sec arp pkt? */
>> What's the problem here?
>
> This comments will be move to patch 2/4
>
>
> The reason is I send secondary guest's arp and ipv6
> packet to ensure it can get mac addr for send other IP
> packet for compare.but currently I don't know this job
> may be affected to something?
Still not clear, I thought we don't need to do any trick for arp to work?
>
>>
>>> + compare_chr_send(s->chr_out, s->sec_rs.buf,
>>> s->sec_rs.packet_len);
>>> + } else if (ret == -1) {
>>> + qemu_chr_add_handlers(s->chr_sec_in, NULL, NULL, NULL, NULL);
>>> + }
>> This function is similar to primary version, may need to unify the
>> codes.
>
> Thanks
> zhangchen
[...]
