From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44089) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWx1L-0007H6-4n for qemu-devel@nongnu.org; Tue, 01 Sep 2015 21:39:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWx1H-000545-0E for qemu-devel@nongnu.org; Tue, 01 Sep 2015 21:39:27 -0400 Received: from [59.151.112.132] (port=56043 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWx1F-00053T-IQ for qemu-devel@nongnu.org; Tue, 01 Sep 2015 21:39:22 -0400 Message-ID: <55E6533F.30600@cn.fujitsu.com> Date: Wed, 2 Sep 2015 09:39:11 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <1441098383-22585-1-git-send-email-yanghy@cn.fujitsu.com> <1441098383-22585-2-git-send-email-yanghy@cn.fujitsu.com> <20150901143622.GC2407@stefanha-thinkpad.redhat.com> In-Reply-To: <20150901143622.GC2407@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , Markus Armbruster , Andreas Faerber Cc: thuth@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, jasowang@redhat.com, qemu-devel@nongnu.org, mrhines@linux.vnet.ibm.com, Paolo Bonzini Hi Stefan, On 09/01/2015 10:36 PM, Stefan Hajnoczi wrote: > On Tue, Sep 01, 2015 at 05:06:14PM +0800, Yang Hongyang wrote: >> Add the framework for a new netfilter object and a new >> -netfilter CLI option as a basis for the following patches. >> Note that the new added document in qemu-options.hx indicate that >> there's a buffer filter. This type of filter will be implemented >> in the following patches. > > Adding Markus and Andreas for the command-line and QAPI perspective on > adding a new type of object to QEMU. I think I've already addressed Markus's comment of QAPI part on v8 :) Thanks! > > It seems you have followed how the net subsystem adds -netdev. I think Yes, but for v9, it's different, we now use flat union instead of simple union as in -netdev. According to Markus's comment. > that approach is not the preferred way of adding new types of objects... > >> >> Signed-off-by: Yang Hongyang >> CC: Paolo Bonzini >> CC: Eric Blake >> Reviewed-by: Thomas Huth >> --- >> v9: Add documentation of buffer filter which will be implemented later >> in this series. >> --- >> include/net/filter.h | 15 +++++++++++++++ >> include/sysemu/sysemu.h | 1 + >> net/Makefile.objs | 1 + >> net/filter.c | 27 +++++++++++++++++++++++++++ >> qemu-options.hx | 21 +++++++++++++++++++++ >> vl.c | 13 +++++++++++++ >> 6 files changed, 78 insertions(+) >> create mode 100644 include/net/filter.h >> create mode 100644 net/filter.c >> >> diff --git a/include/net/filter.h b/include/net/filter.h >> new file mode 100644 >> index 0000000..4242ded >> --- /dev/null >> +++ b/include/net/filter.h >> @@ -0,0 +1,15 @@ >> +/* >> + * Copyright (c) 2015 FUJITSU LIMITED >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or >> + * later. See the COPYING file in the top-level directory. >> + */ >> + >> +#ifndef QEMU_NET_FILTER_H >> +#define QEMU_NET_FILTER_H >> + >> +#include "qemu-common.h" >> + >> +int net_init_filters(void); >> + >> +#endif /* QEMU_NET_FILTER_H */ >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index 44570d1..15d6d00 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts; >> extern QemuOptsList qemu_device_opts; >> extern QemuOptsList qemu_netdev_opts; >> extern QemuOptsList qemu_net_opts; >> +extern QemuOptsList qemu_netfilter_opts; >> extern QemuOptsList qemu_global_opts; >> extern QemuOptsList qemu_mon_opts; >> >> diff --git a/net/Makefile.objs b/net/Makefile.objs >> index ec19cb3..914aec0 100644 >> --- a/net/Makefile.objs >> +++ b/net/Makefile.objs >> @@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o >> common-obj-$(CONFIG_SLIRP) += slirp.o >> common-obj-$(CONFIG_VDE) += vde.o >> common-obj-$(CONFIG_NETMAP) += netmap.o >> +common-obj-y += filter.o >> diff --git a/net/filter.c b/net/filter.c >> new file mode 100644 >> index 0000000..4e40f08 >> --- /dev/null >> +++ b/net/filter.c >> @@ -0,0 +1,27 @@ >> +/* >> + * Copyright (c) 2015 FUJITSU LIMITED >> + * >> + * 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-common.h" >> +#include "net/filter.h" >> + >> +int net_init_filters(void) >> +{ >> + return 0; >> +} >> + >> +QemuOptsList qemu_netfilter_opts = { >> + .name = "netfilter", >> + .implied_opt_name = "type", >> + .head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head), >> + .desc = { >> + /* >> + * no elements => accept any params >> + * validation will happen later >> + */ >> + { /* end of list */ } >> + }, >> +}; > > ...because catch-alls like this make introspection impossible. > >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 77f5853..f1d42a1 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -1991,6 +1991,27 @@ override the default configuration (@option{-net nic -net user}) which >> is activated if no @option{-net} options are provided. >> ETEXI >> >> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, >> + "-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=t]\n" >> + " buffer network packets on netdev. if interval provided, will release\n" >> + " packets by interval. Interval scale: microsecond\n", QEMU_ARCH_ALL) > > Perhaps the -object option should be used: > > -object netfilter-buffer,id=str,netdev=str[,chain=in|out|all,interval=t] > > That is how IOThread and memory backends were recently added. > > They are QOM objects (see include/qom/object.h) and eliminate the need > to write boilerplate code that adds new command-line options and > instantiates objects. I thought -netfilter is more obvious for an object name, -object is kind of abstract name... but I'm not maintainer, if you think it's really need to change to -object, I can do that. Thanks! > >> +STEXI >> +@item -netfilter buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{t}] >> +Buffer network packets on netdev @var{netdevid}. >> +If interval @var{t} provided, will release packets by interval. Interval scale: microsecond. >> +If interval @var{t} not provided, you have to make sure the packets can be released, >> +either by manually remove this filter or call the release buffer API, otherwise, >> +the packets will be buffered forever. Use with caution. >> + >> +chain @var{in/out/all} is an option that can be applied to any netfilter, default is @option{all}. >> + >> +@option{in} means this filter will receive packets sent to the netdev >> + >> +@option{out} means this filter will receive packets sent from the netdev >> + >> +@option{all} means this filter will receive packets both sent to/from the netdev >> +ETEXI >> + >> STEXI >> @end table >> ETEXI >> diff --git a/vl.c b/vl.c >> index 584ca88..aee931a 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -75,6 +75,7 @@ int main(int argc, char **argv) >> #include "monitor/qdev.h" >> #include "sysemu/bt.h" >> #include "net/net.h" >> +#include "net/filter.h" >> #include "net/slirp.h" >> #include "monitor/monitor.h" >> #include "ui/console.h" >> @@ -2998,6 +2999,7 @@ int main(int argc, char **argv, char **envp) >> qemu_add_opts(&qemu_device_opts); >> qemu_add_opts(&qemu_netdev_opts); >> qemu_add_opts(&qemu_net_opts); >> + qemu_add_opts(&qemu_netfilter_opts); >> qemu_add_opts(&qemu_rtc_opts); >> qemu_add_opts(&qemu_global_opts); >> qemu_add_opts(&qemu_mon_opts); >> @@ -3284,6 +3286,13 @@ int main(int argc, char **argv, char **envp) >> exit(1); >> } >> break; >> + case QEMU_OPTION_netfilter: >> + opts = qemu_opts_parse_noisily(qemu_find_opts("netfilter"), >> + optarg, true); >> + if (!opts) { >> + exit(1); >> + } >> + break; >> #ifdef CONFIG_LIBISCSI >> case QEMU_OPTION_iscsi: >> opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"), >> @@ -4413,6 +4422,10 @@ int main(int argc, char **argv, char **envp) >> exit(1); >> } >> >> + if (net_init_filters() < 0) { >> + exit(1); >> + } >> + >> #ifdef CONFIG_TPM >> if (tpm_init() < 0) { >> exit(1); >> -- >> 1.9.1 >> > . > -- Thanks, Yang.