On 01/27/2016 10:40 AM, Zhang Chen wrote: > From: ZhangChen <zhangchen.f...@cn.fujitsu.com> > > 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 <zhangchen.f...@cn.fujitsu.com> > Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> > Reviewed-by: Yang Hongyang <hongyang.y...@easystack.cn>
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. - Is a reliable mirroring (e.g no packet drops during mirroring) is needed for COLO? If yes, this patch seems could not guarantee this. - Please consider to write a unit test for this patch. 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. > 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 <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 "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)? > + if (ret < 0) { This check is not sufficient, for some reason, only part of the packets maybe sent by the socket. Need to handle this properly, otherwise it may confuse receiver. > + g_free(buf); > + return ret; > + } > + > + ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)buf, size); > + g_free(buf); > + return ret; Ditto. > +} > + > +static ssize_t traffic_mirror_receive_iov(NetFilterState *nf, > + NetClientState *sender, > + unsigned flags, > + const struct iovec *iov, > + int iovcnt, > + NetPacketSent *sent_cb) > +{ > + /* > + * We copy and mirror packet to outdev, > + * then put back the packet. > + */ The code could explain itself, so the comment is unnecessary. > + ssize_t ret = 0; > + > + ret = traffic_mirror_send(nf, iov, iovcnt); > + if (ret < 0) { > + error_report("traffic_mirror_send failed"); Monitor could be flooded by this. > + } > + > + return 0; > +} > + Other looks good.