From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57258) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOjFq-0007sk-FZ for qemu-devel@nongnu.org; Thu, 28 Jan 2016 04:52:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOjFm-0003Cs-Et for qemu-devel@nongnu.org; Thu, 28 Jan 2016 04:52:42 -0500 Received: from [59.151.112.132] (port=27312 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOjFl-00037Y-HA for qemu-devel@nongnu.org; Thu, 28 Jan 2016 04:52:38 -0500 References: <1453862428-25570-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <56A9AAC8.4010803@redhat.com> <56A9C6E4.2050309@cn.fujitsu.com> <56A9D349.6090809@redhat.com> From: Zhang Chen Message-ID: <56A9E4C8.4050203@cn.fujitsu.com> Date: Thu, 28 Jan 2016 17:52:08 +0800 MIME-Version: 1.0 In-Reply-To: <56A9D349.6090809@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V2] net/traffic-mirror:Add traffic-mirror List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu devel Cc: zhanghailiang , Li Zhijian , Gui jianfeng , "eddie.dong" , "Dr. David Alan Gilbert" , Yang Hongyang On 01/28/2016 04:37 PM, Jason Wang wrote: > > On 01/28/2016 03:44 PM, Zhang Chen wrote: >> >> On 01/28/2016 01:44 PM, Jason Wang wrote: >>> On 01/27/2016 10:40 AM, Zhang Chen wrote: >>>> From: ZhangChen >>>> >>>> Traffic-mirror is a netfilter plugin. >>>> It gives qemu the ability to copy and mirror guest's >>>> net packet. we output packet to chardev. >>>> >>>> usage: >>>> >>>> -netdev tap,id=hn0 >>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait >>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0 >>>> >>>> Signed-off-by: ZhangChen >>>> Signed-off-by: Wen Congyang >>>> Reviewed-by: Yang Hongyang >>> Thanks for the patch. Several questions: >>> >>> - I'm curious about how the patch was tested? Simple setup e.g: >>> >>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev >>> socket,id=c0,host=localhost,port=4444,server,nowait -object >>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev >>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0 >>> >>> does not works for me. >> I test it in this way. >> primary: >> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev >> socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait >> -object traffic-mirror,id=f0,netdev=hn0,queue=tx,outdev=mirror0 >> >> secondary: >> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev >> socket,id=mirror0,host=3.3.3.3,port=9003 -object >> traffic-reader,id=f1,netdev=hn0,queue=rx,indev=mirror0 >> >> I write a traffic-reader demo to read chardev socket and print it in >> monitor. > Ok, but maybe you can try socket backend. I think the protocol should be > at least compatible with it. Yes,I will try it. >> >>> - Is a reliable mirroring (e.g no packet drops during mirroring) is >>> needed for COLO? If yes, this patch seems could not guarantee this. >> I will fix it in V3 >> >>> - Please consider to write a unit test for this patch. >> write a unit test like tests/test-netfilter.c ? > Even more for its basic function to work. E.g, start qemu with: > > -netdev socket,id=s0,listen=localhost:X -chardev > socket,id=c0,host=localhost,pory=Y,server,nowait -object > filter-mirror,netdev=hn0,outdev=c0 > > Then you can inject packet from the socket connected to s0 and see if > you can read it from socket that connected from c0 (or your traffic reader). I got it~ will add test in next version. >>> And see comments below. >>> >>> Thanks >>> >>> >>>> --- >>>> net/Makefile.objs | 1 + >>>> net/traffic-mirror.c | 173 >>>> +++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> qemu-options.hx | 5 ++ >>>> vl.c | 3 +- >>>> 4 files changed, 181 insertions(+), 1 deletion(-) >>>> create mode 100644 net/traffic-mirror.c >>>> >>>> diff --git a/net/Makefile.objs b/net/Makefile.objs >>>> index 5fa2f97..de06ebe 100644 >>>> --- a/net/Makefile.objs >>>> +++ b/net/Makefile.objs >>>> @@ -15,3 +15,4 @@ common-obj-$(CONFIG_VDE) += vde.o >>>> common-obj-$(CONFIG_NETMAP) += netmap.o >>>> common-obj-y += filter.o >>>> common-obj-y += filter-buffer.o >>>> +common-obj-y += traffic-mirror.o >>> Let's s/traffic-mirror/filter-mirror/g to be consistent with other >>> filters. >>> >> OK~ I will fix it in V3 >> >>>> diff --git a/net/traffic-mirror.c b/net/traffic-mirror.c >>>> new file mode 100644 >>>> index 0000000..bed915c >>>> --- /dev/null >>>> +++ b/net/traffic-mirror.c >>>> @@ -0,0 +1,173 @@ >>>> +/* >>>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. >>>> + * Copyright (c) 2016 FUJITSU LIMITED >>>> + * Copyright (c) 2016 Intel Corporation >>>> + * >>>> + * Author: Zhang Chen >>>> + * >>>> + * 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 "net/filter.h" >>>> +#include "net/net.h" >>>> +#include "qemu-common.h" >>>> +#include "qapi/qmp/qerror.h" >>>> +#include "qapi-visit.h" >>>> +#include "qom/object.h" >>>> +#include "qemu/main-loop.h" >>>> +#include "qemu/error-report.h" >>>> +#include "trace.h" >>>> +#include "sysemu/char.h" >>>> +#include "qemu/iov.h" >>>> + >>>> +#define FILTER_TRAFFIC_MIRROR(obj) \ >>>> + OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_TRAFFIC_MIRROR) >>>> + >>>> +#define TYPE_FILTER_TRAFFIC_MIRROR "traffic-mirror" >>>> + >>>> +typedef struct MirrorState { >>>> + NetFilterState parent_obj; >>>> + char *outdev; >>>> + CharDriverState *chr_out; >>>> + >>>> +} MirrorState; >>>> + >>>> +static ssize_t traffic_mirror_send(NetFilterState *nf, >>>> + const struct iovec *iov, >>>> + int iovcnt) >>>> +{ >>>> + MirrorState *s = FILTER_TRAFFIC_MIRROR(nf); >>>> + ssize_t ret = 0; >>>> + ssize_t size = 0; >>>> + char *buf; >>>> + >>>> + size = iov_size(iov, iovcnt); >>>> + if (!size) { >>>> + return 0; >>>> + } >>>> + >>>> + buf = g_malloc0(size); >>>> + iov_to_buf(iov, iovcnt, 0, buf, size); >>>> + ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)&size, >>>> sizeof(size)); >>> htonl(size)? >> We do not need this. >> > Why? Did you test your mirroring on the wire? Oh, you are right. I will fix it. I have test it in same endian computer.so it work. but difference endian not. Thanks zhangchen > Thanks > > > . > -- Thanks zhangchen