From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46428) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aY31J-0000PY-24 for qemu-devel@nongnu.org; Mon, 22 Feb 2016 21:48:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aY31E-00054b-8f for qemu-devel@nongnu.org; Mon, 22 Feb 2016 21:48:12 -0500 Received: from [59.151.112.132] (port=9897 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aY31C-00051v-CP for qemu-devel@nongnu.org; Mon, 22 Feb 2016 21:48:08 -0500 References: <1455783009-10017-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1455783009-10017-2-git-send-email-zhangchen.fnst@cn.fujitsu.com> <56CBBDEC.7000600@redhat.com> From: Zhang Chen Message-ID: <56CBC876.5050303@cn.fujitsu.com> Date: Tue, 23 Feb 2016 10:48:22 +0800 MIME-Version: 1.0 In-Reply-To: <56CBBDEC.7000600@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror:Add filter-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 02/23/2016 10:03 AM, Jason Wang wrote: > > On 02/18/2016 04:10 PM, Zhang Chen wrote: >> From: ZhangChen >> >> Filter-mirror is a netfilter plugin. >> It gives qemu the ability to mirror >> packets to a chardev. >> >> usage: >> >> -netdev tap,id=hn0 >> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait >> -filter-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0 >> >> Signed-off-by: ZhangChen >> Signed-off-by: Wen Congyang >> Reviewed-by: Yang Hongyang >> Reviewed-by: zhanghailiang >> --- >> net/Makefile.objs | 1 + >> net/filter-mirror.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> qemu-options.hx | 4 ++ >> vl.c | 3 +- >> 4 files changed, 185 insertions(+), 1 deletion(-) >> create mode 100644 net/filter-mirror.c >> >> diff --git a/net/Makefile.objs b/net/Makefile.objs >> index 5fa2f97..b7c22fd 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 += filter-mirror.o >> diff --git a/net/filter-mirror.c b/net/filter-mirror.c >> new file mode 100644 >> index 0000000..4a88e81 >> --- /dev/null >> +++ b/net/filter-mirror.c >> @@ -0,0 +1,178 @@ >> +/* >> + * 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" >> +#include "qemu/sockets.h" >> + >> +#define FILTER_MIRROR(obj) \ >> + OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR) >> + >> +#define TYPE_FILTER_MIRROR "filter-mirror" >> + >> +typedef struct MirrorState { >> + NetFilterState parent_obj; >> + char *outdev; >> + CharDriverState *chr_out; >> +} MirrorState; >> + >> +static int filter_mirror_send(NetFilterState *nf, >> + const struct iovec *iov, >> + int iovcnt) >> +{ >> + MirrorState *s = FILTER_MIRROR(nf); >> + int ret = 0; >> + ssize_t size = 0; >> + uint32_t len = 0; >> + char *buf; >> + >> + size = iov_size(iov, iovcnt); >> + if (!size) { >> + return 0; >> + } >> + >> + len = htonl(size); >> + ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)&len, sizeof(len)); >> + if (ret != sizeof(len)) { >> + goto err; >> + } >> + >> + buf = g_malloc0(size); > g_malloc() should be sufficient. OK, I will fix in next version. >> + iov_to_buf(iov, iovcnt, 0, buf, size); >> + ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)buf, size); >> + g_free(buf); >> + if (ret == size) { >> + return 0; >> + } > This looks odd, I'd rather have: > > if (ret != size) { > goto err; > } > > return 0; OK, I will fix it in next version. >> + >> +err: >> + return ret < 0 ? ret : -EIO; >> +} >> + >> +static ssize_t filter_mirror_receive_iov(NetFilterState *nf, >> + NetClientState *sender, >> + unsigned flags, >> + const struct iovec *iov, >> + int iovcnt, >> + NetPacketSent *sent_cb) >> +{ >> + int ret; >> + >> + ret = filter_mirror_send(nf, iov, iovcnt); >> + if (ret) { >> + error_report("filter_mirror_send failed(%s)", strerror(-ret)); >> + } >> + return 0; >> + /* >> + * FIXME： we don't hope this error interrupt the normal >> + * path of net packet, so we always return zero. >> + */ >> +} > Looks like it's better to place the above as a comment before 'return 0' > (and remove FIXME). I will fix it next version. >> + >> +static void filter_mirror_cleanup(NetFilterState *nf) >> +{ >> + MirrorState *s = FILTER_MIRROR(nf); >> + >> + if (s->chr_out) { >> + qemu_chr_fe_release(s->chr_out); >> + } >> +} >> + >> +static void filter_mirror_setup(NetFilterState *nf, Error **errp) >> +{ >> + MirrorState *s = FILTER_MIRROR(nf); >> + >> + if (!s->outdev) { >> + error_setg(errp, "filter filter mirror needs 'outdev' " >> + "property set"); >> + return; >> + } >> + >> + s->chr_out = qemu_chr_find(s->outdev); >> + if (s->chr_out == NULL) { >> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >> + "Device '%s' not found", s->outdev); >> + return; >> + } >> + >> + if (qemu_chr_fe_claim(s->chr_out) != 0) { >> + error_setg(errp, QERR_DEVICE_IN_USE, s->outdev); >> + return; >> + } >> +} >> + >> +static void filter_mirror_class_init(ObjectClass *oc, void *data) >> +{ >> + NetFilterClass *nfc = NETFILTER_CLASS(oc); >> + >> + nfc->setup = filter_mirror_setup; >> + nfc->cleanup = filter_mirror_cleanup; >> + nfc->receive_iov = filter_mirror_receive_iov; >> +} >> + >> +static char *filter_mirror_get_outdev(Object *obj, Error **errp) >> +{ >> + MirrorState *s = FILTER_MIRROR(obj); >> + >> + return g_strdup(s->outdev); >> +} >> + >> +static void >> +filter_mirror_set_outdev(Object *obj, const char *value, Error **errp) >> +{ >> + MirrorState *s = FILTER_MIRROR(obj); >> + >> + g_free(s->outdev); >> + s->outdev = g_strdup(value); >> + if (!s->outdev) { >> + error_setg(errp, "filter filter mirror needs 'outdev' " >> + "property set"); >> + return; >> + } >> +} >> + >> +static void filter_mirror_init(Object *obj) >> +{ >> + object_property_add_str(obj, "outdev", filter_mirror_get_outdev, >> + filter_mirror_set_outdev, NULL); >> +} >> + >> +static void filter_mirror_fini(Object *obj) >> +{ >> + MirrorState *s = FILTER_MIRROR(obj); >> + >> + g_free(s->outdev); >> +} >> + >> +static const TypeInfo filter_mirror_info = { >> + .name = TYPE_FILTER_MIRROR, >> + .parent = TYPE_NETFILTER, >> + .class_init = filter_mirror_class_init, >> + .instance_init = filter_mirror_init, >> + .instance_finalize = filter_mirror_fini, >> + .instance_size = sizeof(MirrorState), >> +}; >> + >> +static void register_types(void) >> +{ >> + type_register_static(&filter_mirror_info); >> +} >> + >> +type_init(register_types); >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 2f0465e..057e0e5 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -3759,6 +3759,10 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter. >> @option{tx}: the filter is attached to the transmit queue of the netdev, >> where it will receive packets sent by the netdev. >> >> +@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}] >> + >> +filter-mirror on netdev @var{netdevid},mirror net packet to outdev. > To be more accurate, better use "mirror net packet to chardev > @var{chardevid} OK, I will fix it in next version. Thanks zhangchen >> + >> @item -object filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}] >> >> Dump the network traffic on netdev @var{dev} to the file specified by >> diff --git a/vl.c b/vl.c >> index 175ebcc..d68533a 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2798,7 +2798,8 @@ static bool object_create_initial(const char *type) >> * they depend on netdevs already existing >> */ >> if (g_str_equal(type, "filter-buffer") || >> - g_str_equal(type, "filter-dump")) { >> + g_str_equal(type, "filter-dump") || >> + g_str_equal(type, "filter-mirror")) { >> return false; >> } >> > > > . > -- Thanks zhangchen