* [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 14:36 ` Stefan Hajnoczi
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 02/10] init/cleanup of netfilter object Yang Hongyang
` (8 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Paolo Bonzini, Yang Hongyang
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.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
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 */ }
+ },
+};
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)
+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
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter Yang Hongyang
@ 2015-09-01 14:36 ` Stefan Hajnoczi
2015-09-02 1:39 ` Yang Hongyang
0 siblings, 1 reply; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-01 14:36 UTC (permalink / raw)
To: Markus Armbruster, Andreas Faerber
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Paolo Bonzini, Yang Hongyang
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.
It seems you have followed how the net subsystem adds -netdev. I think
that approach is not the preferred way of adding new types of objects...
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
> 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.
> +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
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter
2015-09-01 14:36 ` Stefan Hajnoczi
@ 2015-09-02 1:39 ` Yang Hongyang
2015-09-02 12:58 ` Stefan Hajnoczi
2015-09-02 13:06 ` Stefan Hajnoczi
0 siblings, 2 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-02 1:39 UTC (permalink / raw)
To: Stefan Hajnoczi, Markus Armbruster, Andreas Faerber
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, 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 <yanghy@cn.fujitsu.com>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter
2015-09-02 1:39 ` Yang Hongyang
@ 2015-09-02 12:58 ` Stefan Hajnoczi
2015-09-02 13:04 ` Daniel P. Berrange
2015-09-02 13:06 ` Stefan Hajnoczi
1 sibling, 1 reply; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-02 12:58 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Paolo Bonzini, Andreas Faerber
On Wed, Sep 02, 2015 at 09:39:11AM +0800, Yang Hongyang wrote:
> On 09/01/2015 10:36 PM, Stefan Hajnoczi wrote:
> >On Tue, Sep 01, 2015 at 05:06:14PM +0800, Yang Hongyang wrote:
> >>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.
The advantage of QOM and -object is that it eliminates code for
command-line options, object instantiation, etc. They introduce a
single object model that all types within QEMU can use instead of
inventing their own.
Personally I'm not that involved in QAPI or command-line but I thought
-object was the new preferred way to do things.
This is where I guess Markus and Andreas might have opinions.
Stefan
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter
2015-09-02 12:58 ` Stefan Hajnoczi
@ 2015-09-02 13:04 ` Daniel P. Berrange
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2015-09-02 13:04 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Paolo Bonzini, Yang Hongyang,
Andreas Faerber
On Wed, Sep 02, 2015 at 01:58:58PM +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 02, 2015 at 09:39:11AM +0800, Yang Hongyang wrote:
> > On 09/01/2015 10:36 PM, Stefan Hajnoczi wrote:
> > >On Tue, Sep 01, 2015 at 05:06:14PM +0800, Yang Hongyang wrote:
> > >>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.
>
> The advantage of QOM and -object is that it eliminates code for
> command-line options, object instantiation, etc. They introduce a
> single object model that all types within QEMU can use instead of
> inventing their own.
>
> Personally I'm not that involved in QAPI or command-line but I thought
> -object was the new preferred way to do things.
Yep, you're right - using QOM would be a better idea as it avoids
all the QemuOpts boilerplate command line handling and being part
of the standard object framework makes introspection easier for
apps too.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter
2015-09-02 1:39 ` Yang Hongyang
2015-09-02 12:58 ` Stefan Hajnoczi
@ 2015-09-02 13:06 ` Stefan Hajnoczi
1 sibling, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-02 13:06 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Paolo Bonzini, Andreas Faerber
On Wed, Sep 02, 2015 at 09:39:11AM +0800, Yang Hongyang wrote:
> 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 :)
QAPI is about mapping to C structs, I think the real question I'm
raising is whether QOM should be used to avoid duplicating an object
model, lifecycle, etc.
Stefan
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v9 02/10] init/cleanup of netfilter object
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 03/10] netfilter: add netfilter_{add|del} commands Yang Hongyang
` (7 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Yang Hongyang
QTAILQ_ENTRY global_list but used by filter layer, so that we can
manage all filters together.
QTAILQ_ENTRY next used by netdev, filter belongs to the specific netdev is
in this queue.
This is mostly the same with init/cleanup of netdev object.
The dummy filter is to work around with the empty union of QAPI, and will
be repleced later by buffer filter.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v9: use flat union instead of simple union in QAPI schema
v8: include vhost_net header
v7: add check for vhost
fix error propagate bug
v6: add multiqueue support (net_filter_init1)
v5: remove model from NetFilterState
add a sent_cb param to receive_iov API
---
include/net/filter.h | 42 ++++++++++++
include/net/net.h | 1 +
include/qemu/typedefs.h | 1 +
net/filter.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++++
net/filters.h | 23 +++++++
net/net.c | 1 +
qapi-schema.json | 60 +++++++++++++++++
7 files changed, 301 insertions(+)
create mode 100644 net/filters.h
diff --git a/include/net/filter.h b/include/net/filter.h
index 4242ded..ce15f15 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -9,7 +9,49 @@
#define QEMU_NET_FILTER_H
#include "qemu-common.h"
+#include "qemu/typedefs.h"
+#include "net/queue.h"
+
+/* the netfilter chain */
+enum {
+ NET_FILTER_IN,
+ NET_FILTER_OUT,
+ NET_FILTER_ALL,
+};
+
+typedef void (FilterCleanup) (NetFilterState *);
+/*
+ * Return:
+ * 0: finished handling the packet, we should continue
+ * size: filter stolen this packet, we stop pass this packet further
+ */
+typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb);
+
+typedef struct NetFilterInfo {
+ NetFilterType type;
+ size_t size;
+ FilterCleanup *cleanup;
+ FilterReceiveIOV *receive_iov;
+} NetFilterInfo;
+
+struct NetFilterState {
+ NetFilterInfo *info;
+ char *name;
+ NetClientState *netdev;
+ int chain;
+ QTAILQ_ENTRY(NetFilterState) global_list;
+ QTAILQ_ENTRY(NetFilterState) next;
+};
int net_init_filters(void);
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+ NetClientState *netdev,
+ const char *name,
+ int chain);
#endif /* QEMU_NET_FILTER_H */
diff --git a/include/net/net.h b/include/net/net.h
index 6a6cbef..36e5fab 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -92,6 +92,7 @@ struct NetClientState {
NetClientDestructor *destructor;
unsigned int queue_index;
unsigned rxfilter_notify_enabled:1;
+ QTAILQ_HEAD(, NetFilterState) filters;
};
typedef struct NICState {
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index f8a9dd6..2c0648f 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -45,6 +45,7 @@ typedef struct Monitor Monitor;
typedef struct MouseTransformInfo MouseTransformInfo;
typedef struct MSIMessage MSIMessage;
typedef struct NetClientState NetClientState;
+typedef struct NetFilterState NetFilterState;
typedef struct NICInfo NICInfo;
typedef struct PcGuestInfo PcGuestInfo;
typedef struct PCIBridge PCIBridge;
diff --git a/net/filter.c b/net/filter.c
index 4e40f08..89fb089 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -6,10 +6,183 @@
*/
#include "qemu-common.h"
+#include "qapi-visit.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/dealloc-visitor.h"
+#include "qemu/config-file.h"
+
#include "net/filter.h"
+#include "net/net.h"
+#include "net/vhost_net.h"
+#include "filters.h"
+
+static QTAILQ_HEAD(, NetFilterState) net_filters;
+
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+ NetClientState *netdev,
+ const char *name,
+ int chain)
+{
+ NetFilterState *nf;
+
+ assert(info->size >= sizeof(NetFilterState));
+ assert(info->receive_iov);
+
+ nf = g_malloc0(info->size);
+ nf->info = info;
+ nf->name = g_strdup(name);
+ nf->netdev = netdev;
+ nf->chain = chain;
+ QTAILQ_INSERT_TAIL(&net_filters, nf, global_list);
+ QTAILQ_INSERT_TAIL(&netdev->filters, nf, next);
+
+ return nf;
+}
+
+static inline void qemu_cleanup_net_filter(NetFilterState *nf)
+{
+ QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
+ QTAILQ_REMOVE(&net_filters, nf, global_list);
+
+ if (nf->info->cleanup) {
+ nf->info->cleanup(nf);
+ }
+
+ g_free(nf->name);
+ g_free(nf);
+}
+
+static NetFilterState *qemu_find_netfilter(const char *id)
+{
+ NetFilterState *nf;
+
+ QTAILQ_FOREACH(nf, &net_filters, global_list) {
+ if (!strcmp(nf->name, id)) {
+ return nf;
+ }
+ }
+
+ return NULL;
+}
+
+typedef int (NetFilterInit)(const NetFilter *netfilter,
+ const char *name, int chain,
+ NetClientState *netdev, Error **errp);
+
+static
+NetFilterInit * const net_filter_init_fun[NET_FILTER_TYPE_MAX] = {
+ [NET_FILTER_TYPE_DUMMY] = net_init_filter_dummy,
+};
+
+static int net_filter_init1(const NetFilter *netfilter, Error **errp)
+{
+ NetClientState *ncs[MAX_QUEUE_NUM];
+ const char *name = netfilter->id;
+ const char *netdev_id = netfilter->netdev;
+ const char *chain_str = NULL;
+ int chain, queues, i;
+ NetFilterState *nf;
+
+ if (!net_filter_init_fun[netfilter->kind]) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+ "a net filter type");
+ return -1;
+ }
+
+ nf = qemu_find_netfilter(netfilter->id);
+ if (nf) {
+ error_setg(errp, "Filter '%s' already exists", netfilter->id);
+ return -1;
+ }
+
+ if (netfilter->has_chain) {
+ chain_str = netfilter->chain;
+ if (!strcmp(chain_str, "in")) {
+ chain = NET_FILTER_IN;
+ } else if (!strcmp(chain_str, "out")) {
+ chain = NET_FILTER_OUT;
+ } else if (!strcmp(chain_str, "all")) {
+ chain = NET_FILTER_ALL;
+ } else {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "chain",
+ "netfilter chain (in/out/all)");
+ return -1;
+ }
+ } else {
+ /* default */
+ chain = NET_FILTER_ALL;
+ }
+
+ queues = qemu_find_net_clients_except(netdev_id, ncs,
+ NET_CLIENT_OPTIONS_KIND_NIC,
+ MAX_QUEUE_NUM);
+ if (queues < 1) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev",
+ "a network backend id");
+ return -1;
+ }
+
+ if (get_vhost_net(ncs[0])) {
+ error_setg(errp, "vhost is not supported");
+ return -1;
+ }
+
+ for (i = 0; i < queues; i++) {
+ if (net_filter_init_fun[netfilter->kind](netfilter, name,
+ chain, ncs[i], errp) < 0) {
+ if (errp && !*errp) {
+ error_setg(errp, QERR_DEVICE_INIT_FAILED,
+ NetFilterType_lookup[netfilter->kind]);
+ }
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp)
+{
+ NetFilter *object = NULL;
+ Error *err = NULL;
+ int ret = -1;
+ OptsVisitor *ov = opts_visitor_new(opts);
+
+ visit_type_NetFilter(opts_get_visitor(ov), &object, NULL, &err);
+ opts_visitor_cleanup(ov);
+
+ if (!err) {
+ ret = net_filter_init1(object, &err);
+ }
+
+ if (object) {
+ QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
+
+ visit_type_NetFilter(qapi_dealloc_get_visitor(dv), &object, NULL, NULL);
+ qapi_dealloc_visitor_cleanup(dv);
+ }
+
+ if (errp) {
+ error_propagate(errp, err);
+ } else if (err) {
+ error_report_err(err);
+ }
+
+ return ret;
+}
int net_init_filters(void)
{
+ QTAILQ_INIT(&net_filters);
+
+ if (qemu_opts_foreach(qemu_find_opts("netfilter"),
+ net_init_filter, NULL, NULL)) {
+ return -1;
+ }
+
return 0;
}
diff --git a/net/filters.h b/net/filters.h
new file mode 100644
index 0000000..7c1f30d
--- /dev/null
+++ b/net/filters.h
@@ -0,0 +1,23 @@
+/*
+ * 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_FILTERS_H
+#define QEMU_NET_FILTERS_H
+
+#include "net/net.h"
+#include "net/filter.h"
+
+int net_init_filter_dummy(const NetFilter *netfilter, const char *name,
+ int chain, NetClientState *netdev, Error **errp);
+
+int net_init_filter_dummy(const NetFilter *netfilter, const char *name,
+ int chain, NetClientState *netdev, Error **errp)
+{
+ return 0;
+}
+
+#endif /* QEMU_NET_FILTERS_H */
diff --git a/net/net.c b/net/net.c
index 28a5597..d9b70cd 100644
--- a/net/net.c
+++ b/net/net.c
@@ -287,6 +287,7 @@ static void qemu_net_client_setup(NetClientState *nc,
nc->incoming_queue = qemu_new_net_queue(nc);
nc->destructor = destructor;
+ QTAILQ_INIT(&nc->filters);
}
NetClientState *qemu_new_net_client(NetClientInfo *info,
diff --git a/qapi-schema.json b/qapi-schema.json
index 4342a08..750fc00 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2537,6 +2537,66 @@
'opts': 'NetClientOptions' } }
##
+# @NetFilterDummyOptions:
+#
+# An dummy filter for network backend
+#
+# Since: 2.5
+##
+{ 'struct': 'NetFilterDummyOptions',
+ 'data': { } }
+
+##
+# @NetFilterType:
+#
+# An enumeration of netfilter types.
+#
+# Since: 2.5
+##
+{ 'enum': 'NetFilterType',
+ 'data': ['dummy'] }
+
+##
+# @NetFilterBase
+#
+# Network filters which captures the packets of a network backend.
+#
+# @id: identifier for monitor commands
+#
+# @netdev: the network backend it attached to
+#
+# @chain: #optional accept "in","out","all", if not specified, default is "all"
+# "in" means this filter will receive packets sent to the @netdev
+# "out" means this filter will receive packets sent from the @netdev
+# "all" means this filter will receive packets both sent to/from
+# the @netdev
+#
+# @opts: filter type specific properties
+#
+# Since 2.5
+##
+{ 'struct': 'NetFilterBase',
+ 'data': {
+ 'id': 'str',
+ 'netdev': 'str',
+ '*chain': 'str',
+ 'type': 'NetFilterType' } }
+
+##
+# @NetFilter
+#
+# A discriminated record of network filters.
+#
+# Since 2.5
+#
+##
+{ 'union': 'NetFilter',
+ 'base': 'NetFilterBase',
+ 'discriminator': 'type',
+ 'data': {
+ 'dummy': 'NetFilterDummyOptions' } }
+
+##
# @InetSocketAddress
#
# Captures a socket address or address range in the Internet namespace.
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH v9 03/10] netfilter: add netfilter_{add|del} commands
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 01/10] net: add a new object netfilter Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 02/10] init/cleanup of netfilter object Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 14:37 ` Stefan Hajnoczi
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 04/10] netfilter: hook packets before net queue send Yang Hongyang
` (6 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
Luiz Capitulino, stefanha, Yang Hongyang
add netfilter_{add|del} commands
This is mostly the same with netdev_{add|del} commands.
When we delete the netdev, we also delete the netfilter object
attached to it, because if the netdev is removed, the filters
which attached to it is useless.
Note that the buffer filter will be implemented in the following
patches.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Luiz Capitulino <lcapitulino@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v9: use buffer filter as an example in the command help
cleanup qapi qmp command according to Markus&Eric's comment
v7: error msg fix
move qmp_opts_del() into qemu_del_net_filter()
v6: add multiqueue support (qemu_del_net_filter)
v5: squash "net: delete netfilter object when delete netdev"
---
hmp-commands.hx | 30 ++++++++++++++++++++++++
hmp.c | 29 +++++++++++++++++++++++
hmp.h | 4 ++++
include/net/filter.h | 2 ++
monitor.c | 33 ++++++++++++++++++++++++++
net/filter.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
net/net.c | 7 ++++++
qapi-schema.json | 29 +++++++++++++++++++++++
qapi/opts-visitor.c | 16 +++++++++++++
qmp-commands.hx | 55 ++++++++++++++++++++++++++++++++++++++++++++
10 files changed, 269 insertions(+), 1 deletion(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..9e5f39d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1253,6 +1253,36 @@ Remove host network device.
ETEXI
{
+ .name = "netfilter_add",
+ .args_type = "netfilter:O",
+ .params = "buffer,id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
+ .help = "add netfilter",
+ .mhandler.cmd = hmp_netfilter_add,
+ .command_completion = netfilter_add_completion,
+ },
+
+STEXI
+@item netfilter_add
+@findex netfilter_add
+Add a netfilter to @var{netdev}, which captures the network packets on @var{netdev}.
+ETEXI
+
+ {
+ .name = "netfilter_del",
+ .args_type = "id:s",
+ .params = "id",
+ .help = "remove netfilter",
+ .mhandler.cmd = hmp_netfilter_del,
+ .command_completion = netfilter_del_completion,
+ },
+
+STEXI
+@item netfilter_del
+@findex netfilter_del
+Remove the netfilter which named @var{id}.
+ETEXI
+
+ {
.name = "object_add",
.args_type = "object:O",
.params = "[qom-type=]type,id=str[,prop=value][,...]",
diff --git a/hmp.c b/hmp.c
index dcc66f1..09e3cda 100644
--- a/hmp.c
+++ b/hmp.c
@@ -15,6 +15,7 @@
#include "hmp.h"
#include "net/net.h"
+#include "net/filter.h"
#include "net/eth.h"
#include "sysemu/char.h"
#include "sysemu/block-backend.h"
@@ -1599,6 +1600,34 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_netfilter_add(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+ QemuOpts *opts;
+
+ opts = qemu_opts_from_qdict(qemu_find_opts("netfilter"), qdict, &err);
+ if (err) {
+ goto out;
+ }
+
+ netfilter_add(opts, &err);
+ if (err) {
+ qemu_opts_del(opts);
+ }
+
+out:
+ hmp_handle_error(mon, &err);
+}
+
+void hmp_netfilter_del(Monitor *mon, const QDict *qdict)
+{
+ const char *id = qdict_get_str(qdict, "id");
+ Error *err = NULL;
+
+ qmp_netfilter_del(id, &err);
+ hmp_handle_error(mon, &err);
+}
+
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 0cf4f2a..a21dbbb 100644
--- a/hmp.h
+++ b/hmp.h
@@ -85,6 +85,8 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
void hmp_netdev_add(Monitor *mon, const QDict *qdict);
void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_netfilter_add(Monitor *mon, const QDict *qdict);
+void hmp_netfilter_del(Monitor *mon, const QDict *qdict);
void hmp_getfd(Monitor *mon, const QDict *qdict);
void hmp_closefd(Monitor *mon, const QDict *qdict);
void hmp_sendkey(Monitor *mon, const QDict *qdict);
@@ -112,6 +114,8 @@ void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str);
void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void netfilter_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void netfilter_del_completion(ReadLineState *rs, int nb_args, const char *str);
void ringbuf_write_completion(ReadLineState *rs, int nb_args, const char *str);
void watchdog_action_completion(ReadLineState *rs, int nb_args,
const char *str);
diff --git a/include/net/filter.h b/include/net/filter.h
index ce15f15..083083b 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -53,5 +53,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
NetClientState *netdev,
const char *name,
int chain);
+void qemu_del_net_filter(NetFilterState *nf);
+void netfilter_add(QemuOpts *opts, Error **errp);
#endif /* QEMU_NET_FILTER_H */
diff --git a/monitor.c b/monitor.c
index fc32f12..10b1f77 100644
--- a/monitor.c
+++ b/monitor.c
@@ -31,6 +31,7 @@
#include "hw/loader.h"
#include "exec/gdbstub.h"
#include "net/net.h"
+#include "net/filter.h"
#include "net/slirp.h"
#include "sysemu/char.h"
#include "ui/qemu-spice.h"
@@ -4193,6 +4194,21 @@ void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str)
}
}
+void netfilter_add_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+ size_t len;
+ int i;
+
+ if (nb_args != 2) {
+ return;
+ }
+ len = strlen(str);
+ readline_set_completion_index(rs, len);
+ for (i = 0; NetFilterType_lookup[i]; i++) {
+ add_completion_option(rs, str, NetFilterType_lookup[i]);
+ }
+}
+
void device_add_completion(ReadLineState *rs, int nb_args, const char *str)
{
GSList *list, *elt;
@@ -4429,6 +4445,23 @@ void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
}
}
+void netfilter_del_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+ int len;
+ QemuOpts *opts;
+
+ if (nb_args != 2) {
+ return;
+ }
+
+ len = strlen(str);
+ readline_set_completion_index(rs, len);
+ opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), str);
+ if (opts) {
+ readline_add_completion(rs, str);
+ }
+}
+
void watchdog_action_completion(ReadLineState *rs, int nb_args, const char *str)
{
int i;
diff --git a/net/filter.c b/net/filter.c
index 89fb089..904e5c7 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -13,6 +13,7 @@
#include "qapi/opts-visitor.h"
#include "qapi/dealloc-visitor.h"
#include "qemu/config-file.h"
+#include "qmp-commands.h"
#include "net/filter.h"
#include "net/net.h"
@@ -42,7 +43,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
return nf;
}
-static inline void qemu_cleanup_net_filter(NetFilterState *nf)
+static void qemu_cleanup_net_filter(NetFilterState *nf)
{
QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
QTAILQ_REMOVE(&net_filters, nf, global_list);
@@ -55,6 +56,42 @@ static inline void qemu_cleanup_net_filter(NetFilterState *nf)
g_free(nf);
}
+static int qemu_find_netfilters_by_name(const char *id, NetFilterState **nfs,
+ int max)
+{
+ NetFilterState *nf;
+ int ret = 0;
+
+ QTAILQ_FOREACH(nf, &net_filters, global_list) {
+ if (!strcmp(nf->name, id)) {
+ if (ret < max) {
+ nfs[ret] = nf;
+ }
+ ret++;
+ }
+ }
+
+ return ret;
+}
+
+void qemu_del_net_filter(NetFilterState *nf)
+{
+ NetFilterState *nfs[MAX_QUEUE_NUM];
+ int queues, i;
+ QemuOpts *opts;
+
+ opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), nf->name);
+
+ queues = qemu_find_netfilters_by_name(nf->name, nfs, MAX_QUEUE_NUM);
+ assert(queues != 0);
+
+ for (i = 0; i < queues; i++) {
+ qemu_cleanup_net_filter(nfs[i]);
+ }
+
+ qemu_opts_del(opts);
+}
+
static NetFilterState *qemu_find_netfilter(const char *id)
{
NetFilterState *nf;
@@ -68,6 +105,32 @@ static NetFilterState *qemu_find_netfilter(const char *id)
return NULL;
}
+static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp);
+void netfilter_add(QemuOpts *opts, Error **errp)
+{
+ net_init_filter(NULL, opts, errp);
+}
+
+static int net_filter_init1(const NetFilter *netfilter, Error **errp);
+void qmp_netfilter_add(NetFilter *data, Error **errp)
+{
+ net_filter_init1(data, errp);
+}
+
+void qmp_netfilter_del(const char *id, Error **errp)
+{
+ NetFilterState *nf;
+
+ nf = qemu_find_netfilter(id);
+ if (!nf) {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+ "Filter '%s' not found", id);
+ return;
+ }
+
+ qemu_del_net_filter(nf);
+}
+
typedef int (NetFilterInit)(const NetFilter *netfilter,
const char *name, int chain,
NetClientState *netdev, Error **errp);
diff --git a/net/net.c b/net/net.c
index d9b70cd..74f3592 100644
--- a/net/net.c
+++ b/net/net.c
@@ -28,6 +28,7 @@
#include "hub.h"
#include "net/slirp.h"
#include "net/eth.h"
+#include "net/filter.h"
#include "util.h"
#include "monitor/monitor.h"
@@ -385,6 +386,7 @@ void qemu_del_net_client(NetClientState *nc)
{
NetClientState *ncs[MAX_QUEUE_NUM];
int queues, i;
+ NetFilterState *nf, *next;
assert(nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC);
@@ -396,6 +398,11 @@ void qemu_del_net_client(NetClientState *nc)
MAX_QUEUE_NUM);
assert(queues != 0);
+ /* qemu_del_net_filter() will handle the multiqueue case */
+ QTAILQ_FOREACH_SAFE(nf, &nc->filters, next, next) {
+ qemu_del_net_filter(nf);
+ }
+
/* If there is a peer NIC, delete and cleanup client, but do not free. */
if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
NICState *nic = qemu_get_nic(nc->peer);
diff --git a/qapi-schema.json b/qapi-schema.json
index 750fc00..d961ac8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2537,6 +2537,35 @@
'opts': 'NetClientOptions' } }
##
+# @netfilter-add:
+#
+# Add a netfilter.
+#
+# @data: see NetFilterBase options and specific type's options.
+#
+# Since: 2.5
+#
+# Returns: Nothing on success
+# If it is not a valid netfilter, DeviceNotFound
+##
+{ 'command': 'netfilter-add',
+ 'data': { 'data': 'NetFilter' } }
+
+##
+# @netfilter-del:
+#
+# Remove a netfilter.
+#
+# @id: the name of the netfilter to remove
+#
+# Returns: Nothing on success
+# If @id is not a valid netfilter, DeviceNotFound
+#
+# Since: 2.5
+##
+{ 'command': 'netfilter-del', 'data': {'id': 'str'} }
+
+##
# @NetFilterDummyOptions:
#
# An dummy filter for network backend
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 7ae33b3..1271fab 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -186,6 +186,20 @@ opts_end_struct(Visitor *v, Error **errp)
}
+static void opts_start_implicit_struct(Visitor *v, void **obj,
+ size_t size, Error **errp)
+{
+ if (obj) {
+ *obj = g_malloc0(size);
+ }
+}
+
+
+static void opts_end_implicit_struct(Visitor *v, Error **errp)
+{
+}
+
+
static GQueue *
lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp)
{
@@ -507,6 +521,8 @@ opts_visitor_new(const QemuOpts *opts)
ov->visitor.start_struct = &opts_start_struct;
ov->visitor.end_struct = &opts_end_struct;
+ ov->visitor.start_implicit_struct = &opts_start_implicit_struct;
+ ov->visitor.end_implicit_struct = &opts_end_implicit_struct;
ov->visitor.start_list = &opts_start_list;
ov->visitor.next_list = &opts_next_list;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..bb73806 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -926,6 +926,61 @@ Example:
EQMP
{
+ .name = "netfilter-add",
+ .args_type = "data:q",
+ .mhandler.cmd_new = qmp_marshal_input_netfilter_add,
+ },
+
+SQMP
+netfilter-add
+----------
+
+Add a netfilter.
+
+Arguments:
+
+- "data": filter options (json-string)
+
+Example:
+
+-> { "execute": "netfilter-add",
+ "arguments": { "data": { "type": "buffer", "id": "nf0",
+ "netdev": "bn",
+ "chain": "in",
+ "interval": 1000 } } }
+<- { "return": {} }
+
+Note: The supported filter options are the same ones supported by the
+ '-netfilter' command-line argument, which are listed in the '-help'
+ output or QEMU's manual
+
+EQMP
+
+ {
+ .name = "netfilter-del",
+ .args_type = "id:s",
+ .mhandler.cmd_new = qmp_marshal_input_netfilter_del,
+ },
+
+SQMP
+netfilter-del
+----------
+
+Remove a netfilter.
+
+Arguments:
+
+- "id": the netfilter's ID, must be unique (json-string)
+
+Example:
+
+-> { "execute": "netfilter_del", "arguments": { "id": "nf0" } }
+<- { "return": {} }
+
+
+EQMP
+
+ {
.name = "object-add",
.args_type = "qom-type:s,id:s,props:q?",
.mhandler.cmd_new = qmp_object_add,
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 03/10] netfilter: add netfilter_{add|del} commands
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 03/10] netfilter: add netfilter_{add|del} commands Yang Hongyang
@ 2015-09-01 14:37 ` Stefan Hajnoczi
0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-01 14:37 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, armbru, Luiz Capitulino
On Tue, Sep 01, 2015 at 05:06:16PM +0800, Yang Hongyang wrote:
> add netfilter_{add|del} commands
> This is mostly the same with netdev_{add|del} commands.
The QOM equivalent here is object-add/object-del.
> When we delete the netdev, we also delete the netfilter object
> attached to it, because if the netdev is removed, the filters
> which attached to it is useless.
>
> Note that the buffer filter will be implemented in the following
> patches.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Luiz Capitulino <lcapitulino@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
> v9: use buffer filter as an example in the command help
> cleanup qapi qmp command according to Markus&Eric's comment
> v7: error msg fix
> move qmp_opts_del() into qemu_del_net_filter()
> v6: add multiqueue support (qemu_del_net_filter)
> v5: squash "net: delete netfilter object when delete netdev"
> ---
> hmp-commands.hx | 30 ++++++++++++++++++++++++
> hmp.c | 29 +++++++++++++++++++++++
> hmp.h | 4 ++++
> include/net/filter.h | 2 ++
> monitor.c | 33 ++++++++++++++++++++++++++
> net/filter.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> net/net.c | 7 ++++++
> qapi-schema.json | 29 +++++++++++++++++++++++
> qapi/opts-visitor.c | 16 +++++++++++++
> qmp-commands.hx | 55 ++++++++++++++++++++++++++++++++++++++++++++
> 10 files changed, 269 insertions(+), 1 deletion(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..9e5f39d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1253,6 +1253,36 @@ Remove host network device.
> ETEXI
>
> {
> + .name = "netfilter_add",
> + .args_type = "netfilter:O",
> + .params = "buffer,id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
> + .help = "add netfilter",
> + .mhandler.cmd = hmp_netfilter_add,
> + .command_completion = netfilter_add_completion,
> + },
> +
> +STEXI
> +@item netfilter_add
> +@findex netfilter_add
> +Add a netfilter to @var{netdev}, which captures the network packets on @var{netdev}.
> +ETEXI
> +
> + {
> + .name = "netfilter_del",
> + .args_type = "id:s",
> + .params = "id",
> + .help = "remove netfilter",
> + .mhandler.cmd = hmp_netfilter_del,
> + .command_completion = netfilter_del_completion,
> + },
> +
> +STEXI
> +@item netfilter_del
> +@findex netfilter_del
> +Remove the netfilter which named @var{id}.
> +ETEXI
> +
> + {
> .name = "object_add",
> .args_type = "object:O",
> .params = "[qom-type=]type,id=str[,prop=value][,...]",
> diff --git a/hmp.c b/hmp.c
> index dcc66f1..09e3cda 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -15,6 +15,7 @@
>
> #include "hmp.h"
> #include "net/net.h"
> +#include "net/filter.h"
> #include "net/eth.h"
> #include "sysemu/char.h"
> #include "sysemu/block-backend.h"
> @@ -1599,6 +1600,34 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, &err);
> }
>
> +void hmp_netfilter_add(Monitor *mon, const QDict *qdict)
> +{
> + Error *err = NULL;
> + QemuOpts *opts;
> +
> + opts = qemu_opts_from_qdict(qemu_find_opts("netfilter"), qdict, &err);
> + if (err) {
> + goto out;
> + }
> +
> + netfilter_add(opts, &err);
> + if (err) {
> + qemu_opts_del(opts);
> + }
> +
> +out:
> + hmp_handle_error(mon, &err);
> +}
> +
> +void hmp_netfilter_del(Monitor *mon, const QDict *qdict)
> +{
> + const char *id = qdict_get_str(qdict, "id");
> + Error *err = NULL;
> +
> + qmp_netfilter_del(id, &err);
> + hmp_handle_error(mon, &err);
> +}
> +
> void hmp_object_add(Monitor *mon, const QDict *qdict)
> {
> Error *err = NULL;
> diff --git a/hmp.h b/hmp.h
> index 0cf4f2a..a21dbbb 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -85,6 +85,8 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_netfilter_add(Monitor *mon, const QDict *qdict);
> +void hmp_netfilter_del(Monitor *mon, const QDict *qdict);
> void hmp_getfd(Monitor *mon, const QDict *qdict);
> void hmp_closefd(Monitor *mon, const QDict *qdict);
> void hmp_sendkey(Monitor *mon, const QDict *qdict);
> @@ -112,6 +114,8 @@ void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str);
> void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
> void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
> void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);
> +void netfilter_add_completion(ReadLineState *rs, int nb_args, const char *str);
> +void netfilter_del_completion(ReadLineState *rs, int nb_args, const char *str);
> void ringbuf_write_completion(ReadLineState *rs, int nb_args, const char *str);
> void watchdog_action_completion(ReadLineState *rs, int nb_args,
> const char *str);
> diff --git a/include/net/filter.h b/include/net/filter.h
> index ce15f15..083083b 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -53,5 +53,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
> NetClientState *netdev,
> const char *name,
> int chain);
> +void qemu_del_net_filter(NetFilterState *nf);
> +void netfilter_add(QemuOpts *opts, Error **errp);
>
> #endif /* QEMU_NET_FILTER_H */
> diff --git a/monitor.c b/monitor.c
> index fc32f12..10b1f77 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -31,6 +31,7 @@
> #include "hw/loader.h"
> #include "exec/gdbstub.h"
> #include "net/net.h"
> +#include "net/filter.h"
> #include "net/slirp.h"
> #include "sysemu/char.h"
> #include "ui/qemu-spice.h"
> @@ -4193,6 +4194,21 @@ void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str)
> }
> }
>
> +void netfilter_add_completion(ReadLineState *rs, int nb_args, const char *str)
> +{
> + size_t len;
> + int i;
> +
> + if (nb_args != 2) {
> + return;
> + }
> + len = strlen(str);
> + readline_set_completion_index(rs, len);
> + for (i = 0; NetFilterType_lookup[i]; i++) {
> + add_completion_option(rs, str, NetFilterType_lookup[i]);
> + }
> +}
> +
> void device_add_completion(ReadLineState *rs, int nb_args, const char *str)
> {
> GSList *list, *elt;
> @@ -4429,6 +4445,23 @@ void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
> }
> }
>
> +void netfilter_del_completion(ReadLineState *rs, int nb_args, const char *str)
> +{
> + int len;
> + QemuOpts *opts;
> +
> + if (nb_args != 2) {
> + return;
> + }
> +
> + len = strlen(str);
> + readline_set_completion_index(rs, len);
> + opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), str);
> + if (opts) {
> + readline_add_completion(rs, str);
> + }
> +}
> +
> void watchdog_action_completion(ReadLineState *rs, int nb_args, const char *str)
> {
> int i;
> diff --git a/net/filter.c b/net/filter.c
> index 89fb089..904e5c7 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -13,6 +13,7 @@
> #include "qapi/opts-visitor.h"
> #include "qapi/dealloc-visitor.h"
> #include "qemu/config-file.h"
> +#include "qmp-commands.h"
>
> #include "net/filter.h"
> #include "net/net.h"
> @@ -42,7 +43,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
> return nf;
> }
>
> -static inline void qemu_cleanup_net_filter(NetFilterState *nf)
> +static void qemu_cleanup_net_filter(NetFilterState *nf)
> {
> QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
> QTAILQ_REMOVE(&net_filters, nf, global_list);
> @@ -55,6 +56,42 @@ static inline void qemu_cleanup_net_filter(NetFilterState *nf)
> g_free(nf);
> }
>
> +static int qemu_find_netfilters_by_name(const char *id, NetFilterState **nfs,
> + int max)
> +{
> + NetFilterState *nf;
> + int ret = 0;
> +
> + QTAILQ_FOREACH(nf, &net_filters, global_list) {
> + if (!strcmp(nf->name, id)) {
> + if (ret < max) {
> + nfs[ret] = nf;
> + }
> + ret++;
> + }
> + }
> +
> + return ret;
> +}
> +
> +void qemu_del_net_filter(NetFilterState *nf)
> +{
> + NetFilterState *nfs[MAX_QUEUE_NUM];
> + int queues, i;
> + QemuOpts *opts;
> +
> + opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), nf->name);
> +
> + queues = qemu_find_netfilters_by_name(nf->name, nfs, MAX_QUEUE_NUM);
> + assert(queues != 0);
> +
> + for (i = 0; i < queues; i++) {
> + qemu_cleanup_net_filter(nfs[i]);
> + }
> +
> + qemu_opts_del(opts);
> +}
> +
> static NetFilterState *qemu_find_netfilter(const char *id)
> {
> NetFilterState *nf;
> @@ -68,6 +105,32 @@ static NetFilterState *qemu_find_netfilter(const char *id)
> return NULL;
> }
>
> +static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp);
> +void netfilter_add(QemuOpts *opts, Error **errp)
> +{
> + net_init_filter(NULL, opts, errp);
> +}
> +
> +static int net_filter_init1(const NetFilter *netfilter, Error **errp);
> +void qmp_netfilter_add(NetFilter *data, Error **errp)
> +{
> + net_filter_init1(data, errp);
> +}
> +
> +void qmp_netfilter_del(const char *id, Error **errp)
> +{
> + NetFilterState *nf;
> +
> + nf = qemu_find_netfilter(id);
> + if (!nf) {
> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> + "Filter '%s' not found", id);
> + return;
> + }
> +
> + qemu_del_net_filter(nf);
> +}
> +
> typedef int (NetFilterInit)(const NetFilter *netfilter,
> const char *name, int chain,
> NetClientState *netdev, Error **errp);
> diff --git a/net/net.c b/net/net.c
> index d9b70cd..74f3592 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -28,6 +28,7 @@
> #include "hub.h"
> #include "net/slirp.h"
> #include "net/eth.h"
> +#include "net/filter.h"
> #include "util.h"
>
> #include "monitor/monitor.h"
> @@ -385,6 +386,7 @@ void qemu_del_net_client(NetClientState *nc)
> {
> NetClientState *ncs[MAX_QUEUE_NUM];
> int queues, i;
> + NetFilterState *nf, *next;
>
> assert(nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC);
>
> @@ -396,6 +398,11 @@ void qemu_del_net_client(NetClientState *nc)
> MAX_QUEUE_NUM);
> assert(queues != 0);
>
> + /* qemu_del_net_filter() will handle the multiqueue case */
> + QTAILQ_FOREACH_SAFE(nf, &nc->filters, next, next) {
> + qemu_del_net_filter(nf);
> + }
> +
> /* If there is a peer NIC, delete and cleanup client, but do not free. */
> if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> NICState *nic = qemu_get_nic(nc->peer);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 750fc00..d961ac8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2537,6 +2537,35 @@
> 'opts': 'NetClientOptions' } }
>
> ##
> +# @netfilter-add:
> +#
> +# Add a netfilter.
> +#
> +# @data: see NetFilterBase options and specific type's options.
> +#
> +# Since: 2.5
> +#
> +# Returns: Nothing on success
> +# If it is not a valid netfilter, DeviceNotFound
> +##
> +{ 'command': 'netfilter-add',
> + 'data': { 'data': 'NetFilter' } }
> +
> +##
> +# @netfilter-del:
> +#
> +# Remove a netfilter.
> +#
> +# @id: the name of the netfilter to remove
> +#
> +# Returns: Nothing on success
> +# If @id is not a valid netfilter, DeviceNotFound
> +#
> +# Since: 2.5
> +##
> +{ 'command': 'netfilter-del', 'data': {'id': 'str'} }
> +
> +##
> # @NetFilterDummyOptions:
> #
> # An dummy filter for network backend
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 7ae33b3..1271fab 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -186,6 +186,20 @@ opts_end_struct(Visitor *v, Error **errp)
> }
>
>
> +static void opts_start_implicit_struct(Visitor *v, void **obj,
> + size_t size, Error **errp)
> +{
> + if (obj) {
> + *obj = g_malloc0(size);
> + }
> +}
> +
> +
> +static void opts_end_implicit_struct(Visitor *v, Error **errp)
> +{
> +}
> +
> +
> static GQueue *
> lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp)
> {
> @@ -507,6 +521,8 @@ opts_visitor_new(const QemuOpts *opts)
>
> ov->visitor.start_struct = &opts_start_struct;
> ov->visitor.end_struct = &opts_end_struct;
> + ov->visitor.start_implicit_struct = &opts_start_implicit_struct;
> + ov->visitor.end_implicit_struct = &opts_end_implicit_struct;
>
> ov->visitor.start_list = &opts_start_list;
> ov->visitor.next_list = &opts_next_list;
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ba630b1..bb73806 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -926,6 +926,61 @@ Example:
> EQMP
>
> {
> + .name = "netfilter-add",
> + .args_type = "data:q",
> + .mhandler.cmd_new = qmp_marshal_input_netfilter_add,
> + },
> +
> +SQMP
> +netfilter-add
> +----------
> +
> +Add a netfilter.
> +
> +Arguments:
> +
> +- "data": filter options (json-string)
> +
> +Example:
> +
> +-> { "execute": "netfilter-add",
> + "arguments": { "data": { "type": "buffer", "id": "nf0",
> + "netdev": "bn",
> + "chain": "in",
> + "interval": 1000 } } }
> +<- { "return": {} }
> +
> +Note: The supported filter options are the same ones supported by the
> + '-netfilter' command-line argument, which are listed in the '-help'
> + output or QEMU's manual
> +
> +EQMP
> +
> + {
> + .name = "netfilter-del",
> + .args_type = "id:s",
> + .mhandler.cmd_new = qmp_marshal_input_netfilter_del,
> + },
> +
> +SQMP
> +netfilter-del
> +----------
> +
> +Remove a netfilter.
> +
> +Arguments:
> +
> +- "id": the netfilter's ID, must be unique (json-string)
> +
> +Example:
> +
> +-> { "execute": "netfilter_del", "arguments": { "id": "nf0" } }
> +<- { "return": {} }
> +
> +
> +EQMP
> +
> + {
> .name = "object-add",
> .args_type = "qom-type:s,id:s,props:q?",
> .mhandler.cmd_new = qmp_object_add,
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v9 04/10] netfilter: hook packets before net queue send
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
` (2 preceding siblings ...)
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 03/10] netfilter: add netfilter_{add|del} commands Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 05/10] move out net queue structs define Yang Hongyang
` (5 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Yang Hongyang
Capture packets that will be sent.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v5: do not check ret against iov_size
pass sent_cb to filters
---
net/net.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/net/net.c b/net/net.c
index 74f3592..00cca83 100644
--- a/net/net.c
+++ b/net/net.c
@@ -562,6 +562,44 @@ int qemu_can_send_packet(NetClientState *sender)
return 1;
}
+static ssize_t filter_receive_iov(NetClientState *nc, int chain,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
+{
+ ssize_t ret = 0;
+ NetFilterState *nf = NULL;
+
+ QTAILQ_FOREACH(nf, &nc->filters, next) {
+ if (nf->chain == chain || nf->chain == NET_FILTER_ALL) {
+ ret = nf->info->receive_iov(nf, sender, flags,
+ iov, iovcnt, sent_cb);
+ if (ret) {
+ return ret;
+ }
+ }
+ }
+
+ return ret;
+}
+
+static ssize_t filter_receive(NetClientState *nc, int chain,
+ NetClientState *sender,
+ unsigned flags,
+ const uint8_t *data,
+ size_t size,
+ NetPacketSent *sent_cb)
+{
+ struct iovec iov = {
+ .iov_base = (void *)data,
+ .iov_len = size
+ };
+
+ return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb);
+}
+
ssize_t qemu_deliver_packet(NetClientState *sender,
unsigned flags,
const uint8_t *data,
@@ -633,6 +671,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+ int ret;
#ifdef DEBUG_NET
printf("qemu_send_packet_async:\n");
@@ -643,6 +682,19 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
return size;
}
+ /* Let filters handle the packet first */
+ ret = filter_receive(sender, NET_FILTER_OUT,
+ sender, flags, buf, size, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
+ ret = filter_receive(sender->peer, NET_FILTER_IN,
+ sender, flags, buf, size, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
queue = sender->peer->incoming_queue;
return qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
@@ -713,11 +765,25 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+ int ret;
if (sender->link_down || !sender->peer) {
return iov_size(iov, iovcnt);
}
+ /* Let filters handle the packet first */
+ ret = filter_receive_iov(sender, NET_FILTER_OUT, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
+ ret = filter_receive_iov(sender->peer, NET_FILTER_IN, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
+ if (ret) {
+ return ret;
+ }
+
queue = sender->peer->incoming_queue;
return qemu_net_queue_send_iov(queue, sender,
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
` (3 preceding siblings ...)
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 04/10] netfilter: hook packets before net queue send Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 14:43 ` Stefan Hajnoczi
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 06/10] netfilter: add an API to pass the packet to next filter Yang Hongyang
` (4 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Yang Hongyang
This will be used by the next patch in this series.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
include/net/queue.h | 19 +++++++++++++++++++
net/queue.c | 19 -------------------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/include/net/queue.h b/include/net/queue.h
index fc02b33..1d65e47 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
+struct NetPacket {
+ QTAILQ_ENTRY(NetPacket) entry;
+ NetClientState *sender;
+ unsigned flags;
+ int size;
+ NetPacketSent *sent_cb;
+ uint8_t data[0];
+};
+
+struct NetQueue {
+ void *opaque;
+ uint32_t nq_maxlen;
+ uint32_t nq_count;
+
+ QTAILQ_HEAD(packets, NetPacket) packets;
+
+ unsigned delivering:1;
+};
+
#define QEMU_NET_PACKET_FLAG_NONE 0
#define QEMU_NET_PACKET_FLAG_RAW (1<<0)
diff --git a/net/queue.c b/net/queue.c
index ebbe2bb..1499479 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -39,25 +39,6 @@
* unbounded queueing.
*/
-struct NetPacket {
- QTAILQ_ENTRY(NetPacket) entry;
- NetClientState *sender;
- unsigned flags;
- int size;
- NetPacketSent *sent_cb;
- uint8_t data[0];
-};
-
-struct NetQueue {
- void *opaque;
- uint32_t nq_maxlen;
- uint32_t nq_count;
-
- QTAILQ_HEAD(packets, NetPacket) packets;
-
- unsigned delivering : 1;
-};
-
NetQueue *qemu_new_net_queue(void *opaque)
{
NetQueue *queue;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 05/10] move out net queue structs define Yang Hongyang
@ 2015-09-01 14:43 ` Stefan Hajnoczi
2015-09-02 1:49 ` Yang Hongyang
0 siblings, 1 reply; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-01 14:43 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, armbru
On Tue, Sep 01, 2015 at 05:06:18PM +0800, Yang Hongyang wrote:
> This will be used by the next patch in this series.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
> include/net/queue.h | 19 +++++++++++++++++++
> net/queue.c | 19 -------------------
> 2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/include/net/queue.h b/include/net/queue.h
> index fc02b33..1d65e47 100644
> --- a/include/net/queue.h
> +++ b/include/net/queue.h
> @@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
>
> typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
>
> +struct NetPacket {
> + QTAILQ_ENTRY(NetPacket) entry;
> + NetClientState *sender;
> + unsigned flags;
> + int size;
> + NetPacketSent *sent_cb;
> + uint8_t data[0];
> +};
> +
> +struct NetQueue {
> + void *opaque;
> + uint32_t nq_maxlen;
> + uint32_t nq_count;
> +
> + QTAILQ_HEAD(packets, NetPacket) packets;
> +
> + unsigned delivering:1;
> +};
> +
Why is it necessary to expose both structs?
Normally functions would be added to maintain the abstraction. Instead,
you have chosen to access the fields directly - this is probably a bad
idea.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-01 14:43 ` Stefan Hajnoczi
@ 2015-09-02 1:49 ` Yang Hongyang
2015-09-02 13:02 ` Stefan Hajnoczi
0 siblings, 1 reply; 32+ messages in thread
From: Yang Hongyang @ 2015-09-02 1:49 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, armbru
On 09/01/2015 10:43 PM, Stefan Hajnoczi wrote:
> On Tue, Sep 01, 2015 at 05:06:18PM +0800, Yang Hongyang wrote:
>> This will be used by the next patch in this series.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> ---
>> include/net/queue.h | 19 +++++++++++++++++++
>> net/queue.c | 19 -------------------
>> 2 files changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/net/queue.h b/include/net/queue.h
>> index fc02b33..1d65e47 100644
>> --- a/include/net/queue.h
>> +++ b/include/net/queue.h
>> @@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
>>
>> typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
>>
>> +struct NetPacket {
>> + QTAILQ_ENTRY(NetPacket) entry;
>> + NetClientState *sender;
>> + unsigned flags;
>> + int size;
>> + NetPacketSent *sent_cb;
>> + uint8_t data[0];
>> +};
>> +
>> +struct NetQueue {
>> + void *opaque;
>> + uint32_t nq_maxlen;
>> + uint32_t nq_count;
>> +
>> + QTAILQ_HEAD(packets, NetPacket) packets;
>> +
>> + unsigned delivering:1;
>> +};
>> +
>
> Why is it necessary to expose both structs?
In next patch, we add a API to pass the packet to next filter:
ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
which will access the internals of NetPacket.
and in buffer filter, we need the NetQueue to Queue packets, and also to
access queue->packets(pass this packets to next filter).
>
> Normally functions would be added to maintain the abstraction. Instead,
> you have chosen to access the fields directly - this is probably a bad
> idea.
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-02 1:49 ` Yang Hongyang
@ 2015-09-02 13:02 ` Stefan Hajnoczi
2015-09-02 16:18 ` Yang Hongyang
0 siblings, 1 reply; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-02 13:02 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, armbru
On Wed, Sep 02, 2015 at 09:49:18AM +0800, Yang Hongyang wrote:
> On 09/01/2015 10:43 PM, Stefan Hajnoczi wrote:
> >On Tue, Sep 01, 2015 at 05:06:18PM +0800, Yang Hongyang wrote:
> >>This will be used by the next patch in this series.
> >>
> >>Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> >>Reviewed-by: Thomas Huth <thuth@redhat.com>
> >>---
> >> include/net/queue.h | 19 +++++++++++++++++++
> >> net/queue.c | 19 -------------------
> >> 2 files changed, 19 insertions(+), 19 deletions(-)
> >>
> >>diff --git a/include/net/queue.h b/include/net/queue.h
> >>index fc02b33..1d65e47 100644
> >>--- a/include/net/queue.h
> >>+++ b/include/net/queue.h
> >>@@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
> >>
> >> typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
> >>
> >>+struct NetPacket {
> >>+ QTAILQ_ENTRY(NetPacket) entry;
> >>+ NetClientState *sender;
> >>+ unsigned flags;
> >>+ int size;
> >>+ NetPacketSent *sent_cb;
> >>+ uint8_t data[0];
> >>+};
> >>+
> >>+struct NetQueue {
> >>+ void *opaque;
> >>+ uint32_t nq_maxlen;
> >>+ uint32_t nq_count;
> >>+
> >>+ QTAILQ_HEAD(packets, NetPacket) packets;
> >>+
> >>+ unsigned delivering:1;
> >>+};
> >>+
> >
> >Why is it necessary to expose both structs?
>
> In next patch, we add a API to pass the packet to next filter:
> ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
> which will access the internals of NetPacket.
>
> and in buffer filter, we need the NetQueue to Queue packets, and also to
> access queue->packets(pass this packets to next filter).
Can you do these things by introducing net queue APIs instead of
exposing the struct?
It's a C anti-pattern to expose structs. It makes code complex because
now the net queue code no longer contains all the assumptions about
NetQueue/NetPacket fields. People modifying the code now also need to
go into the net filter code to figure out how this struct works.
Exposing the fields also leads to code duplication since every user will
reimplement similar stuff if they access the fields directly instead of
using a function interface.
Stefan
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-02 13:02 ` Stefan Hajnoczi
@ 2015-09-02 16:18 ` Yang Hongyang
2015-09-04 10:32 ` Stefan Hajnoczi
0 siblings, 1 reply; 32+ messages in thread
From: Yang Hongyang @ 2015-09-02 16:18 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, armbru
On 09/02/2015 09:02 PM, Stefan Hajnoczi wrote:
> On Wed, Sep 02, 2015 at 09:49:18AM +0800, Yang Hongyang wrote:
>> On 09/01/2015 10:43 PM, Stefan Hajnoczi wrote:
>>> On Tue, Sep 01, 2015 at 05:06:18PM +0800, Yang Hongyang wrote:
>>>> This will be used by the next patch in this series.
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> include/net/queue.h | 19 +++++++++++++++++++
>>>> net/queue.c | 19 -------------------
>>>> 2 files changed, 19 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/include/net/queue.h b/include/net/queue.h
>>>> index fc02b33..1d65e47 100644
>>>> --- a/include/net/queue.h
>>>> +++ b/include/net/queue.h
>>>> @@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
>>>>
>>>> typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
>>>>
>>>> +struct NetPacket {
>>>> + QTAILQ_ENTRY(NetPacket) entry;
>>>> + NetClientState *sender;
>>>> + unsigned flags;
>>>> + int size;
>>>> + NetPacketSent *sent_cb;
>>>> + uint8_t data[0];
>>>> +};
>>>> +
>>>> +struct NetQueue {
>>>> + void *opaque;
>>>> + uint32_t nq_maxlen;
>>>> + uint32_t nq_count;
>>>> +
>>>> + QTAILQ_HEAD(packets, NetPacket) packets;
>>>> +
>>>> + unsigned delivering:1;
>>>> +};
>>>> +
>>>
>>> Why is it necessary to expose both structs?
>>
>> In next patch, we add a API to pass the packet to next filter:
>> ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
>> which will access the internals of NetPacket.
>>
>> and in buffer filter, we need the NetQueue to Queue packets, and also to
>> access queue->packets(pass this packets to next filter).
>
> Can you do these things by introducing net queue APIs instead of
> exposing the struct?
>
> It's a C anti-pattern to expose structs. It makes code complex because
> now the net queue code no longer contains all the assumptions about
> NetQueue/NetPacket fields. People modifying the code now also need to
> go into the net filter code to figure out how this struct works.
>
> Exposing the fields also leads to code duplication since every user will
> reimplement similar stuff if they access the fields directly instead of
> using a function interface.
I can't think of a interface that no need to access the NetPacket internals
to archive the needs. Except I do not reuse the NetPacket and NetQueue,
implement a Queue myself. But that will be more complex.
So in order to not expose those structs, I shall add lots of public get
functions to use these internals, like:
Qemu_NetQueue_get_xxx
Qemu_NetPacket_get_xxx
I'd be very happy if you could give me a better suggestion on this.
Thanks!
>
> Stefan
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-02 16:18 ` Yang Hongyang
@ 2015-09-04 10:32 ` Stefan Hajnoczi
2015-09-07 7:37 ` Yang Hongyang
0 siblings, 1 reply; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-04 10:32 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, armbru
On Thu, Sep 03, 2015 at 12:18:18AM +0800, Yang Hongyang wrote:
>
>
> On 09/02/2015 09:02 PM, Stefan Hajnoczi wrote:
> >On Wed, Sep 02, 2015 at 09:49:18AM +0800, Yang Hongyang wrote:
> >>On 09/01/2015 10:43 PM, Stefan Hajnoczi wrote:
> >>>On Tue, Sep 01, 2015 at 05:06:18PM +0800, Yang Hongyang wrote:
> >>>>This will be used by the next patch in this series.
> >>>>
> >>>>Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> >>>>Reviewed-by: Thomas Huth <thuth@redhat.com>
> >>>>---
> >>>> include/net/queue.h | 19 +++++++++++++++++++
> >>>> net/queue.c | 19 -------------------
> >>>> 2 files changed, 19 insertions(+), 19 deletions(-)
> >>>>
> >>>>diff --git a/include/net/queue.h b/include/net/queue.h
> >>>>index fc02b33..1d65e47 100644
> >>>>--- a/include/net/queue.h
> >>>>+++ b/include/net/queue.h
> >>>>@@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
> >>>>
> >>>> typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
> >>>>
> >>>>+struct NetPacket {
> >>>>+ QTAILQ_ENTRY(NetPacket) entry;
> >>>>+ NetClientState *sender;
> >>>>+ unsigned flags;
> >>>>+ int size;
> >>>>+ NetPacketSent *sent_cb;
> >>>>+ uint8_t data[0];
> >>>>+};
> >>>>+
> >>>>+struct NetQueue {
> >>>>+ void *opaque;
> >>>>+ uint32_t nq_maxlen;
> >>>>+ uint32_t nq_count;
> >>>>+
> >>>>+ QTAILQ_HEAD(packets, NetPacket) packets;
> >>>>+
> >>>>+ unsigned delivering:1;
> >>>>+};
> >>>>+
> >>>
> >>>Why is it necessary to expose both structs?
> >>
> >>In next patch, we add a API to pass the packet to next filter:
> >>ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
> >>which will access the internals of NetPacket.
> >>
> >>and in buffer filter, we need the NetQueue to Queue packets, and also to
> >>access queue->packets(pass this packets to next filter).
> >
> >Can you do these things by introducing net queue APIs instead of
> >exposing the struct?
> >
> >It's a C anti-pattern to expose structs. It makes code complex because
> >now the net queue code no longer contains all the assumptions about
> >NetQueue/NetPacket fields. People modifying the code now also need to
> >go into the net filter code to figure out how this struct works.
> >
> >Exposing the fields also leads to code duplication since every user will
> >reimplement similar stuff if they access the fields directly instead of
> >using a function interface.
>
> I can't think of a interface that no need to access the NetPacket internals
> to archive the needs. Except I do not reuse the NetPacket and NetQueue,
> implement a Queue myself. But that will be more complex.
> So in order to not expose those structs, I shall add lots of public get
> functions to use these internals, like:
> Qemu_NetQueue_get_xxx
> Qemu_NetPacket_get_xxx
>
> I'd be very happy if you could give me a better suggestion on this.
net/queue.c has logic to send/queue/flush packets but a
qemu_deliver_packet() call is hardcoded.
Maybe you can extend qemu_new_net_queue() like this:
/* Returns:
* >0 - success
* 0 - queue packet for future redelivery
* <0 - failure (discard packet)
*/
typedef ssize_t NetQueueDeliverFunc(NetClientState *sender,
unsigned flags,
const struct iovec *iov,
int iovcnt,
void *opaque);
NetQueue *qemu_new_net_queue(NetQueueDeliverFunc deliver,
void *opaque);
Now net/net.c:qemu_net_client_setup() needs to call:
nc->incoming_queue = qemu_new_net_queue(qemu_deliver_packet_iov, nc);
And the filter code can use qemu_net_queue_send_iov() and
qemu_net_queue_flush(). The filter just needs to provide its own
NetQueueDeliveryFunc.
I haven't checked the details (e.g. non-iov delivery, etc) but the idea
is to use the net/queue.c API instead of duplicating similar logic in
the filter code.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-04 10:32 ` Stefan Hajnoczi
@ 2015-09-07 7:37 ` Yang Hongyang
2015-09-07 9:06 ` Markus Armbruster
2015-09-07 9:11 ` Stefan Hajnoczi
0 siblings, 2 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-07 7:37 UTC (permalink / raw)
To: Stefan Hajnoczi, Markus Armbruster, Andreas Faerber
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines
Hi Stefan,
On 09/04/2015 06:32 PM, Stefan Hajnoczi wrote:
[...]
>
> net/queue.c has logic to send/queue/flush packets but a
> qemu_deliver_packet() call is hardcoded.
>
> Maybe you can extend qemu_new_net_queue() like this:
>
> /* Returns:
> * >0 - success
> * 0 - queue packet for future redelivery
> * <0 - failure (discard packet)
> */
> typedef ssize_t NetQueueDeliverFunc(NetClientState *sender,
> unsigned flags,
> const struct iovec *iov,
> int iovcnt,
> void *opaque);
>
> NetQueue *qemu_new_net_queue(NetQueueDeliverFunc deliver,
> void *opaque);
>
> Now net/net.c:qemu_net_client_setup() needs to call:
>
> nc->incoming_queue = qemu_new_net_queue(qemu_deliver_packet_iov, nc);
>
> And the filter code can use qemu_net_queue_send_iov() and
> qemu_net_queue_flush(). The filter just needs to provide its own
> NetQueueDeliveryFunc.
>
> I haven't checked the details (e.g. non-iov delivery, etc) but the idea
> is to use the net/queue.c API instead of duplicating similar logic in
> the filter code.
Thanks very much for the suggestion, I've already implemented it and tested,
the code looks cleaner now.
The last issue is the QOM thing, do Markus and Andreas have more input
about that?
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 7:37 ` Yang Hongyang
@ 2015-09-07 9:06 ` Markus Armbruster
2015-09-07 9:21 ` Yang Hongyang
2015-09-07 9:11 ` Stefan Hajnoczi
1 sibling, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2015-09-07 9:06 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Stefan Hajnoczi, Andreas Faerber
Yang Hongyang <yanghy@cn.fujitsu.com> writes:
> Hi Stefan,
>
> On 09/04/2015 06:32 PM, Stefan Hajnoczi wrote:
> [...]
>>
>> net/queue.c has logic to send/queue/flush packets but a
>> qemu_deliver_packet() call is hardcoded.
>>
>> Maybe you can extend qemu_new_net_queue() like this:
>>
>> /* Returns:
>> * >0 - success
>> * 0 - queue packet for future redelivery
>> * <0 - failure (discard packet)
>> */
>> typedef ssize_t NetQueueDeliverFunc(NetClientState *sender,
>> unsigned flags,
>> const struct iovec *iov,
>> int iovcnt,
>> void *opaque);
>>
>> NetQueue *qemu_new_net_queue(NetQueueDeliverFunc deliver,
>> void *opaque);
>>
>> Now net/net.c:qemu_net_client_setup() needs to call:
>>
>> nc->incoming_queue = qemu_new_net_queue(qemu_deliver_packet_iov, nc);
>>
>> And the filter code can use qemu_net_queue_send_iov() and
>> qemu_net_queue_flush(). The filter just needs to provide its own
>> NetQueueDeliveryFunc.
>>
>> I haven't checked the details (e.g. non-iov delivery, etc) but the idea
>> is to use the net/queue.c API instead of duplicating similar logic in
>> the filter code.
>
> Thanks very much for the suggestion, I've already implemented it and tested,
> the code looks cleaner now.
>
> The last issue is the QOM thing, do Markus and Andreas have more input
> about that?
This series is in my review queue. I'm struggling with clearing my
queue, and apologize for the delay.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 9:06 ` Markus Armbruster
@ 2015-09-07 9:21 ` Yang Hongyang
0 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-07 9:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Stefan Hajnoczi, Andreas Faerber
On 09/07/2015 05:06 PM, Markus Armbruster wrote:
> Yang Hongyang <yanghy@cn.fujitsu.com> writes:
>
>> Hi Stefan,
>>
>> On 09/04/2015 06:32 PM, Stefan Hajnoczi wrote:
>> [...]
>>>
>>> net/queue.c has logic to send/queue/flush packets but a
>>> qemu_deliver_packet() call is hardcoded.
>>>
>>> Maybe you can extend qemu_new_net_queue() like this:
>>>
>>> /* Returns:
>>> * >0 - success
>>> * 0 - queue packet for future redelivery
>>> * <0 - failure (discard packet)
>>> */
>>> typedef ssize_t NetQueueDeliverFunc(NetClientState *sender,
>>> unsigned flags,
>>> const struct iovec *iov,
>>> int iovcnt,
>>> void *opaque);
>>>
>>> NetQueue *qemu_new_net_queue(NetQueueDeliverFunc deliver,
>>> void *opaque);
>>>
>>> Now net/net.c:qemu_net_client_setup() needs to call:
>>>
>>> nc->incoming_queue = qemu_new_net_queue(qemu_deliver_packet_iov, nc);
>>>
>>> And the filter code can use qemu_net_queue_send_iov() and
>>> qemu_net_queue_flush(). The filter just needs to provide its own
>>> NetQueueDeliveryFunc.
>>>
>>> I haven't checked the details (e.g. non-iov delivery, etc) but the idea
>>> is to use the net/queue.c API instead of duplicating similar logic in
>>> the filter code.
>>
>> Thanks very much for the suggestion, I've already implemented it and tested,
>> the code looks cleaner now.
>>
>> The last issue is the QOM thing, do Markus and Andreas have more input
>> about that?
>
> This series is in my review queue. I'm struggling with clearing my
> queue, and apologize for the delay.
Thanks, I'm going to rebase this series on QOM in v10, the QAPI part will
be the same as in this series unless you're about to review this series,
so if you are still struggling on the long queue, you might want to review the
next version :)
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 7:37 ` Yang Hongyang
2015-09-07 9:06 ` Markus Armbruster
@ 2015-09-07 9:11 ` Stefan Hajnoczi
2015-09-07 9:26 ` Yang Hongyang
2015-09-07 10:53 ` Yang Hongyang
1 sibling, 2 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2015-09-07 9:11 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Andreas Faerber
On Mon, Sep 07, 2015 at 03:37:20PM +0800, Yang Hongyang wrote:
> Hi Stefan,
>
> On 09/04/2015 06:32 PM, Stefan Hajnoczi wrote:
> [...]
> >
> >net/queue.c has logic to send/queue/flush packets but a
> >qemu_deliver_packet() call is hardcoded.
> >
> >Maybe you can extend qemu_new_net_queue() like this:
> >
> >/* Returns:
> > * >0 - success
> > * 0 - queue packet for future redelivery
> > * <0 - failure (discard packet)
> > */
> >typedef ssize_t NetQueueDeliverFunc(NetClientState *sender,
> > unsigned flags,
> > const struct iovec *iov,
> > int iovcnt,
> > void *opaque);
> >
> >NetQueue *qemu_new_net_queue(NetQueueDeliverFunc deliver,
> > void *opaque);
> >
> >Now net/net.c:qemu_net_client_setup() needs to call:
> >
> > nc->incoming_queue = qemu_new_net_queue(qemu_deliver_packet_iov, nc);
> >
> >And the filter code can use qemu_net_queue_send_iov() and
> >qemu_net_queue_flush(). The filter just needs to provide its own
> >NetQueueDeliveryFunc.
> >
> >I haven't checked the details (e.g. non-iov delivery, etc) but the idea
> >is to use the net/queue.c API instead of duplicating similar logic in
> >the filter code.
>
> Thanks very much for the suggestion, I've already implemented it and tested,
> the code looks cleaner now.
>
> The last issue is the QOM thing, do Markus and Andreas have more input
> about that?
If you would like to see examples of QOM usage, take a look at
iothread.c and/or backends/hostmem.c.
The key things are:
1. They use include/qom/object.h to register a type based on
TYPE_OBJECT and their properties are registered using
object_property_add_*().
2. They implement the TYPE_USER_CREATABLE interface so the -object
command-line option can be used to instantiate them. See
object_interfaces.h.
As a result, a lot of code becomes unnecessary and iothread.c, in
particular, is quite short.
Stefan
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 9:11 ` Stefan Hajnoczi
@ 2015-09-07 9:26 ` Yang Hongyang
2015-09-07 10:53 ` Yang Hongyang
1 sibling, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-07 9:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Andreas Faerber
On 09/07/2015 05:11 PM, Stefan Hajnoczi wrote:
> On Mon, Sep 07, 2015 at 03:37:20PM +0800, Yang Hongyang wrote:
>> Hi Stefan,
>>
>> On 09/04/2015 06:32 PM, Stefan Hajnoczi wrote:
>> [...]
>>>
>>> net/queue.c has logic to send/queue/flush packets but a
>>> qemu_deliver_packet() call is hardcoded.
>>>
>>> Maybe you can extend qemu_new_net_queue() like this:
>>>
>>> /* Returns:
>>> * >0 - success
>>> * 0 - queue packet for future redelivery
>>> * <0 - failure (discard packet)
>>> */
>>> typedef ssize_t NetQueueDeliverFunc(NetClientState *sender,
>>> unsigned flags,
>>> const struct iovec *iov,
>>> int iovcnt,
>>> void *opaque);
>>>
>>> NetQueue *qemu_new_net_queue(NetQueueDeliverFunc deliver,
>>> void *opaque);
>>>
>>> Now net/net.c:qemu_net_client_setup() needs to call:
>>>
>>> nc->incoming_queue = qemu_new_net_queue(qemu_deliver_packet_iov, nc);
>>>
>>> And the filter code can use qemu_net_queue_send_iov() and
>>> qemu_net_queue_flush(). The filter just needs to provide its own
>>> NetQueueDeliveryFunc.
>>>
>>> I haven't checked the details (e.g. non-iov delivery, etc) but the idea
>>> is to use the net/queue.c API instead of duplicating similar logic in
>>> the filter code.
>>
>> Thanks very much for the suggestion, I've already implemented it and tested,
>> the code looks cleaner now.
>>
>> The last issue is the QOM thing, do Markus and Andreas have more input
>> about that?
>
> If you would like to see examples of QOM usage, take a look at
> iothread.c and/or backends/hostmem.c.
>
> The key things are:
>
> 1. They use include/qom/object.h to register a type based on
> TYPE_OBJECT and their properties are registered using
> object_property_add_*().
>
> 2. They implement the TYPE_USER_CREATABLE interface so the -object
> command-line option can be used to instantiate them. See
> object_interfaces.h.
>
> As a result, a lot of code becomes unnecessary and iothread.c, in
> particular, is quite short.
Thanks a lot, this is what I need, will look into this and rebase this
series on top of QOM.
>
> Stefan
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 9:11 ` Stefan Hajnoczi
2015-09-07 9:26 ` Yang Hongyang
@ 2015-09-07 10:53 ` Yang Hongyang
2015-09-07 11:00 ` Daniel P. Berrange
1 sibling, 1 reply; 32+ messages in thread
From: Yang Hongyang @ 2015-09-07 10:53 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Andreas Faerber
On 09/07/2015 05:11 PM, Stefan Hajnoczi wrote:
[...]
>> Thanks very much for the suggestion, I've already implemented it and tested,
>> the code looks cleaner now.
>>
>> The last issue is the QOM thing, do Markus and Andreas have more input
>> about that?
>
> If you would like to see examples of QOM usage, take a look at
> iothread.c and/or backends/hostmem.c.
>
> The key things are:
>
> 1. They use include/qom/object.h to register a type based on
> TYPE_OBJECT and their properties are registered using
> object_property_add_*().
>
> 2. They implement the TYPE_USER_CREATABLE interface so the -object
> command-line option can be used to instantiate them. See
> object_interfaces.h.
>
> As a result, a lot of code becomes unnecessary and iothread.c, in
> particular, is quite short.
After looking into this, I have some questions on the implement, could you
please help me on this because I don't know much about the object mechanism:
The netfilter need to be initialized after the net_init_clients, because we
need to attach the filter to the net client. But currently, net client is not
using QOM, and seems that it is initialized after objects been created. So here
comes the problem: how can I initialize a certain object later, is it possiable?
>
> Stefan
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 10:53 ` Yang Hongyang
@ 2015-09-07 11:00 ` Daniel P. Berrange
2015-09-07 11:41 ` Yang Hongyang
0 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2015-09-07 11:00 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Stefan Hajnoczi, Andreas Faerber
On Mon, Sep 07, 2015 at 06:53:49PM +0800, Yang Hongyang wrote:
> On 09/07/2015 05:11 PM, Stefan Hajnoczi wrote:
> [...]
> >>Thanks very much for the suggestion, I've already implemented it and tested,
> >>the code looks cleaner now.
> >>
> >>The last issue is the QOM thing, do Markus and Andreas have more input
> >>about that?
> >
> >If you would like to see examples of QOM usage, take a look at
> >iothread.c and/or backends/hostmem.c.
> >
> >The key things are:
> >
> >1. They use include/qom/object.h to register a type based on
> > TYPE_OBJECT and their properties are registered using
> > object_property_add_*().
> >
> >2. They implement the TYPE_USER_CREATABLE interface so the -object
> > command-line option can be used to instantiate them. See
> > object_interfaces.h.
> >
> >As a result, a lot of code becomes unnecessary and iothread.c, in
> >particular, is quite short.
>
> After looking into this, I have some questions on the implement, could you
> please help me on this because I don't know much about the object mechanism:
>
> The netfilter need to be initialized after the net_init_clients, because we
> need to attach the filter to the net client. But currently, net client is not
> using QOM, and seems that it is initialized after objects been created. So here
> comes the problem: how can I initialize a certain object later, is it possiable?
It is currently a bit hacky - most objects are initialized very early,
but we have a similar problem with rng-egd which must be created /after/
chardevs. To deal with this in vl.c main() we have two helper methods
object_create_initial and object_create_delayed. The delayed method though
still happens before the net clients are created. We could probably just
move the objec_create_delayed method invokation to later on, after net
clients are created, but if that doesn't work just add an extra helper
object_create_very_delayed :-)
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 11:00 ` Daniel P. Berrange
@ 2015-09-07 11:41 ` Yang Hongyang
2015-09-07 11:43 ` Daniel P. Berrange
0 siblings, 1 reply; 32+ messages in thread
From: Yang Hongyang @ 2015-09-07 11:41 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Stefan Hajnoczi, Andreas Faerber
On 09/07/2015 07:00 PM, Daniel P. Berrange wrote:
> On Mon, Sep 07, 2015 at 06:53:49PM +0800, Yang Hongyang wrote:
>> On 09/07/2015 05:11 PM, Stefan Hajnoczi wrote:
>> [...]
>>>> Thanks very much for the suggestion, I've already implemented it and tested,
>>>> the code looks cleaner now.
>>>>
>>>> The last issue is the QOM thing, do Markus and Andreas have more input
>>>> about that?
>>>
>>> If you would like to see examples of QOM usage, take a look at
>>> iothread.c and/or backends/hostmem.c.
>>>
>>> The key things are:
>>>
>>> 1. They use include/qom/object.h to register a type based on
>>> TYPE_OBJECT and their properties are registered using
>>> object_property_add_*().
>>>
>>> 2. They implement the TYPE_USER_CREATABLE interface so the -object
>>> command-line option can be used to instantiate them. See
>>> object_interfaces.h.
>>>
>>> As a result, a lot of code becomes unnecessary and iothread.c, in
>>> particular, is quite short.
>>
>> After looking into this, I have some questions on the implement, could you
>> please help me on this because I don't know much about the object mechanism:
>>
>> The netfilter need to be initialized after the net_init_clients, because we
>> need to attach the filter to the net client. But currently, net client is not
>> using QOM, and seems that it is initialized after objects been created. So here
>> comes the problem: how can I initialize a certain object later, is it possiable?
>
> It is currently a bit hacky - most objects are initialized very early,
> but we have a similar problem with rng-egd which must be created /after/
> chardevs. To deal with this in vl.c main() we have two helper methods
> object_create_initial and object_create_delayed. The delayed method though
> still happens before the net clients are created. We could probably just
> move the objec_create_delayed method invokation to later on, after net
> clients are created, but if that doesn't work just add an extra helper
> object_create_very_delayed :-)
If it's ok to move creation of "rng-egd" later, then "move the
objec_create_delayed method invokation after net clients are created" should
solve the problem.
Thank you for the help!
>
> Regards,
> Daniel
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 11:41 ` Yang Hongyang
@ 2015-09-07 11:43 ` Daniel P. Berrange
2015-09-07 11:46 ` Yang Hongyang
0 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2015-09-07 11:43 UTC (permalink / raw)
To: Yang Hongyang
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Stefan Hajnoczi, Andreas Faerber
On Mon, Sep 07, 2015 at 07:41:26PM +0800, Yang Hongyang wrote:
>
>
> On 09/07/2015 07:00 PM, Daniel P. Berrange wrote:
> >On Mon, Sep 07, 2015 at 06:53:49PM +0800, Yang Hongyang wrote:
> >>On 09/07/2015 05:11 PM, Stefan Hajnoczi wrote:
> >>[...]
> >>>>Thanks very much for the suggestion, I've already implemented it and tested,
> >>>>the code looks cleaner now.
> >>>>
> >>>>The last issue is the QOM thing, do Markus and Andreas have more input
> >>>>about that?
> >>>
> >>>If you would like to see examples of QOM usage, take a look at
> >>>iothread.c and/or backends/hostmem.c.
> >>>
> >>>The key things are:
> >>>
> >>>1. They use include/qom/object.h to register a type based on
> >>> TYPE_OBJECT and their properties are registered using
> >>> object_property_add_*().
> >>>
> >>>2. They implement the TYPE_USER_CREATABLE interface so the -object
> >>> command-line option can be used to instantiate them. See
> >>> object_interfaces.h.
> >>>
> >>>As a result, a lot of code becomes unnecessary and iothread.c, in
> >>>particular, is quite short.
> >>
> >>After looking into this, I have some questions on the implement, could you
> >>please help me on this because I don't know much about the object mechanism:
> >>
> >>The netfilter need to be initialized after the net_init_clients, because we
> >>need to attach the filter to the net client. But currently, net client is not
> >>using QOM, and seems that it is initialized after objects been created. So here
> >>comes the problem: how can I initialize a certain object later, is it possiable?
> >
> >It is currently a bit hacky - most objects are initialized very early,
> >but we have a similar problem with rng-egd which must be created /after/
> >chardevs. To deal with this in vl.c main() we have two helper methods
> >object_create_initial and object_create_delayed. The delayed method though
> >still happens before the net clients are created. We could probably just
> >move the objec_create_delayed method invokation to later on, after net
> >clients are created, but if that doesn't work just add an extra helper
> >object_create_very_delayed :-)
>
> If it's ok to move creation of "rng-egd" later, then "move the
> objec_create_delayed method invokation after net clients are created" should
> solve the problem.
I think it should be ok, because IIRC, the only requirement from rng-egd
was that it be created /after/ -device is processed, so moving it even
later shouldn't hurt it
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
2015-09-07 11:43 ` Daniel P. Berrange
@ 2015-09-07 11:46 ` Yang Hongyang
0 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-07 11:46 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, qemu-devel,
mrhines, Markus Armbruster, Stefan Hajnoczi, Andreas Faerber
On 09/07/2015 07:43 PM, Daniel P. Berrange wrote:
> On Mon, Sep 07, 2015 at 07:41:26PM +0800, Yang Hongyang wrote:
>>
>>
>> On 09/07/2015 07:00 PM, Daniel P. Berrange wrote:
>>> On Mon, Sep 07, 2015 at 06:53:49PM +0800, Yang Hongyang wrote:
>>>> On 09/07/2015 05:11 PM, Stefan Hajnoczi wrote:
>>>> [...]
>>>>>> Thanks very much for the suggestion, I've already implemented it and tested,
>>>>>> the code looks cleaner now.
>>>>>>
>>>>>> The last issue is the QOM thing, do Markus and Andreas have more input
>>>>>> about that?
>>>>>
>>>>> If you would like to see examples of QOM usage, take a look at
>>>>> iothread.c and/or backends/hostmem.c.
>>>>>
>>>>> The key things are:
>>>>>
>>>>> 1. They use include/qom/object.h to register a type based on
>>>>> TYPE_OBJECT and their properties are registered using
>>>>> object_property_add_*().
>>>>>
>>>>> 2. They implement the TYPE_USER_CREATABLE interface so the -object
>>>>> command-line option can be used to instantiate them. See
>>>>> object_interfaces.h.
>>>>>
>>>>> As a result, a lot of code becomes unnecessary and iothread.c, in
>>>>> particular, is quite short.
>>>>
>>>> After looking into this, I have some questions on the implement, could you
>>>> please help me on this because I don't know much about the object mechanism:
>>>>
>>>> The netfilter need to be initialized after the net_init_clients, because we
>>>> need to attach the filter to the net client. But currently, net client is not
>>>> using QOM, and seems that it is initialized after objects been created. So here
>>>> comes the problem: how can I initialize a certain object later, is it possiable?
>>>
>>> It is currently a bit hacky - most objects are initialized very early,
>>> but we have a similar problem with rng-egd which must be created /after/
>>> chardevs. To deal with this in vl.c main() we have two helper methods
>>> object_create_initial and object_create_delayed. The delayed method though
>>> still happens before the net clients are created. We could probably just
>>> move the objec_create_delayed method invokation to later on, after net
>>> clients are created, but if that doesn't work just add an extra helper
>>> object_create_very_delayed :-)
>>
>> If it's ok to move creation of "rng-egd" later, then "move the
>> objec_create_delayed method invokation after net clients are created" should
>> solve the problem.
>
> I think it should be ok, because IIRC, the only requirement from rng-egd
> was that it be created /after/ -device is processed, so moving it even
> later shouldn't hurt it
Got it, thanks!
>
> Regards,
> Daniel
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v9 06/10] netfilter: add an API to pass the packet to next filter
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
` (4 preceding siblings ...)
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 05/10] move out net queue structs define Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 07/10] netfilter: print filter info associate with the netdev Yang Hongyang
` (3 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Yang Hongyang
add an API qemu_netfilter_pass_to_next() to pass the packet
to next filter.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
v9: fix a bug when curr filter chain is all
v5: fold params to NetPacket struct
---
include/net/filter.h | 3 +++
net/filter.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index 083083b..d1aafbe 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -56,4 +56,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
void qemu_del_net_filter(NetFilterState *nf);
void netfilter_add(QemuOpts *opts, Error **errp);
+/* pass the packet to the next filter */
+ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
+
#endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index 904e5c7..53bb8c4 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -14,11 +14,13 @@
#include "qapi/dealloc-visitor.h"
#include "qemu/config-file.h"
#include "qmp-commands.h"
+#include "qemu/iov.h"
#include "net/filter.h"
#include "net/net.h"
#include "net/vhost_net.h"
#include "filters.h"
+#include "net/queue.h"
static QTAILQ_HEAD(, NetFilterState) net_filters;
@@ -131,6 +133,59 @@ void qmp_netfilter_del(const char *id, Error **errp)
qemu_del_net_filter(nf);
}
+ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet)
+{
+ int ret = 0;
+ int chain;
+ NetFilterState *next = QTAILQ_NEXT(nf, next);
+ struct iovec iov = {
+ .iov_base = (void *)packet->data,
+ .iov_len = packet->size
+ };
+
+ if (!packet->sender || !packet->sender->peer) {
+ /* no receiver, or sender been deleted, no need to pass it further */
+ goto out;
+ }
+
+ if (nf->chain == NET_FILTER_ALL) {
+ if (packet->sender == nf->netdev) {
+ /* This packet is sent by netdev itself */
+ chain = NET_FILTER_OUT;
+ } else {
+ chain = NET_FILTER_IN;
+ }
+ } else {
+ chain = nf->chain;
+ }
+
+ while (next) {
+ if (next->chain == chain || next->chain == NET_FILTER_ALL) {
+ ret = next->info->receive_iov(next, packet->sender, packet->flags,
+ &iov, 1, packet->sent_cb);
+ if (ret) {
+ return ret;
+ }
+ }
+ next = QTAILQ_NEXT(next, next);
+ }
+
+ /*
+ * We have gone through all filters, pass it to receiver.
+ * Do the valid check again incase sender or receiver been
+ * deleted while we go through filters.
+ */
+ if (packet->sender && packet->sender->peer) {
+ return qemu_net_queue_send_iov(packet->sender->peer->incoming_queue,
+ packet->sender, packet->flags,
+ &iov, 1, packet->sent_cb);
+ }
+
+out:
+ /* no receiver, or sender been deleted */
+ return packet->size;
+}
+
typedef int (NetFilterInit)(const NetFilter *netfilter,
const char *name, int chain,
NetClientState *netdev, Error **errp);
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH v9 07/10] netfilter: print filter info associate with the netdev
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
` (5 preceding siblings ...)
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 06/10] netfilter: add an API to pass the packet to next filter Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 08/10] net/queue: export qemu_net_queue_append_iov Yang Hongyang
` (2 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
Yang Hongyang, stefanha, Yang Hongyang
From: Yang Hongyang <burnef@gmail.com>
When execute "info network", print filter info also.
current info printed is simple, can add more info later.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v9: tiny cleanup according to Tomas's comment.
v7: initial patch
---
include/net/filter.h | 1 +
net/filter.c | 22 ++++++++++++++++++++++
net/net.c | 11 +++++++++++
3 files changed, 34 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index d1aafbe..e29c8a4 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -55,6 +55,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
int chain);
void qemu_del_net_filter(NetFilterState *nf);
void netfilter_add(QemuOpts *opts, Error **errp);
+const char *qemu_netfilter_get_chain_str(int chain);
/* pass the packet to the next filter */
ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
diff --git a/net/filter.c b/net/filter.c
index 53bb8c4..570813c 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -24,6 +24,28 @@
static QTAILQ_HEAD(, NetFilterState) net_filters;
+const char *qemu_netfilter_get_chain_str(int chain)
+{
+ const char *str;
+
+ switch (chain) {
+ case NET_FILTER_IN:
+ str = "in";
+ break;
+ case NET_FILTER_OUT:
+ str = "out";
+ break;
+ case NET_FILTER_ALL:
+ str = "all";
+ break;
+ default:
+ str = "unknown";
+ break;
+ }
+
+ return str;
+}
+
NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
NetClientState *netdev,
const char *name,
diff --git a/net/net.c b/net/net.c
index 00cca83..06f2cce 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1199,10 +1199,21 @@ void qmp_netdev_del(const char *id, Error **errp)
void print_net_client(Monitor *mon, NetClientState *nc)
{
+ NetFilterState *nf;
+
monitor_printf(mon, "%s: index=%d,type=%s,%s\n", nc->name,
nc->queue_index,
NetClientOptionsKind_lookup[nc->info->type],
nc->info_str);
+ if (!QTAILQ_EMPTY(&nc->filters)) {
+ monitor_printf(mon, "filters:\n");
+ }
+ QTAILQ_FOREACH(nf, &nc->filters, next) {
+ monitor_printf(mon, " - %s: type=%s,netdev=%s,chain=%s\n", nf->name,
+ NetFilterType_lookup[nf->info->type],
+ nf->netdev->name,
+ qemu_netfilter_get_chain_str(nf->chain));
+ }
}
RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH v9 08/10] net/queue: export qemu_net_queue_append_iov
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
` (6 preceding siblings ...)
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 07/10] netfilter: print filter info associate with the netdev Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 09/10] netfilter: add a netbuffer filter Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 10/10] tests: add test cases for netfilter object Yang Hongyang
9 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Yang Hongyang
This will be used by buffer filter implementation later to
queue packets.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
include/net/queue.h | 7 +++++++
net/queue.c | 12 ++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/net/queue.h b/include/net/queue.h
index 1d65e47..e139cc7 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -55,6 +55,13 @@ struct NetQueue {
NetQueue *qemu_new_net_queue(void *opaque);
+void qemu_net_queue_append_iov(NetQueue *queue,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb);
+
void qemu_del_net_queue(NetQueue *queue);
ssize_t qemu_net_queue_send(NetQueue *queue,
diff --git a/net/queue.c b/net/queue.c
index 1499479..428fdd6 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -91,12 +91,12 @@ static void qemu_net_queue_append(NetQueue *queue,
QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
}
-static void qemu_net_queue_append_iov(NetQueue *queue,
- NetClientState *sender,
- unsigned flags,
- const struct iovec *iov,
- int iovcnt,
- NetPacketSent *sent_cb)
+void qemu_net_queue_append_iov(NetQueue *queue,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
{
NetPacket *packet;
size_t max_len = 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH v9 09/10] netfilter: add a netbuffer filter
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
` (7 preceding siblings ...)
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 08/10] net/queue: export qemu_net_queue_append_iov Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 10/10] tests: add test cases for netfilter object Yang Hongyang
9 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Yang Hongyang
This filter is to buffer/release packets, this feature can be used
when using MicroCheckpointing, or other Remus like VM FT solutions, you
can also use it to simulate the network delay.
It has an interval option, if supplied, this filter will release
packets by interval.
The dummy filter is replaced by this filter.
Usage:
-netdev tap,id=bn0
-netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
NOTE:
the scale of interval is microsecond.
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v9: adjustment due to the qapi change
v7: use QTAILQ_FOREACH_SAFE() when flush packets
v6: move the interval check earlier and some comment adjust
v5: remove dummy sent_cb
change interval type from int64 to uint32
check interval!=0 when initialise
rename FILTERBUFFERState to FilterBufferState
v4: remove bh
pass the packet to next filter instead of receiver
v3: check packet's sender and sender->peer when flush it
---
net/Makefile.objs | 1 +
net/filter-buffer.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++
net/filter.c | 3 +-
net/filters.h | 10 +----
qapi-schema.json | 20 ++++++---
5 files changed, 143 insertions(+), 16 deletions(-)
create mode 100644 net/filter-buffer.c
diff --git a/net/Makefile.objs b/net/Makefile.objs
index 914aec0..5fa2f97 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -14,3 +14,4 @@ common-obj-$(CONFIG_SLIRP) += slirp.o
common-obj-$(CONFIG_VDE) += vde.o
common-obj-$(CONFIG_NETMAP) += netmap.o
common-obj-y += filter.o
+common-obj-y += filter-buffer.o
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
new file mode 100644
index 0000000..93f31c6
--- /dev/null
+++ b/net/filter-buffer.c
@@ -0,0 +1,125 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ * Author: Yang Hongyang <yanghy@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/queue.h"
+#include "filters.h"
+#include "qemu-common.h"
+#include "qemu/timer.h"
+#include "qemu/iov.h"
+#include "qapi/qmp/qerror.h"
+
+typedef struct FilterBufferState {
+ NetFilterState nf;
+ NetQueue *incoming_queue;
+ uint32_t interval;
+ QEMUTimer release_timer;
+} FilterBufferState;
+
+static void filter_buffer_flush(NetFilterState *nf)
+{
+ FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+ NetQueue *queue = s->incoming_queue;
+ NetPacket *packet, *next;
+
+ QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
+ QTAILQ_REMOVE(&queue->packets, packet, entry);
+ queue->nq_count--;
+
+ if (packet->sender && packet->sender->peer) {
+ qemu_netfilter_pass_to_next(nf, packet);
+ }
+
+ /*
+ * now that we have passed the packet to next filter (or there's
+ * no receiver). If it's queued by receiver's incoming_queue, there
+ * will be a copy of the packet->data, so simply free this packet
+ * now.
+ */
+ g_free(packet);
+ }
+}
+
+static void filter_buffer_release_timer(void *opaque)
+{
+ FilterBufferState *s = opaque;
+ filter_buffer_flush(&s->nf);
+ timer_mod(&s->release_timer,
+ qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
+}
+
+/* filter APIs */
+static ssize_t filter_buffer_receive_iov(NetFilterState *nf,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
+{
+ FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+ NetQueue *queue = s->incoming_queue;
+
+ qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, sent_cb);
+ return iov_size(iov, iovcnt);
+}
+
+static void filter_buffer_cleanup(NetFilterState *nf)
+{
+ FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+
+ if (s->interval) {
+ timer_del(&s->release_timer);
+ }
+
+ /* flush packets */
+ filter_buffer_flush(nf);
+ g_free(s->incoming_queue);
+ return;
+}
+
+static NetFilterInfo net_filter_buffer_info = {
+ .type = NET_FILTER_TYPE_BUFFER,
+ .size = sizeof(FilterBufferState),
+ .receive_iov = filter_buffer_receive_iov,
+ .cleanup = filter_buffer_cleanup,
+};
+
+int net_init_filter_buffer(const NetFilter *netfilter, const char *name,
+ int chain, NetClientState *netdev, Error **errp)
+{
+ NetFilterState *nf;
+ FilterBufferState *s;
+ const NetFilterBufferOptions *bufferopt;
+ int interval;
+
+ assert(netfilter->kind == NET_FILTER_TYPE_BUFFER);
+ bufferopt = netfilter->buffer;
+ interval = bufferopt->has_interval ? bufferopt->interval : 0;
+ /*
+ * this check should be dropped when there're VM FT solutions like MC
+ * or COLO use this filter to release packets on demand.
+ */
+ if (!interval) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "interval",
+ "a non-zero interval");
+ return -1;
+ }
+
+ nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, name, chain);
+ s = DO_UPCAST(FilterBufferState, nf, nf);
+ s->incoming_queue = qemu_new_net_queue(nf);
+ s->interval = interval;
+ if (s->interval) {
+ timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL,
+ filter_buffer_release_timer, s);
+ timer_mod(&s->release_timer,
+ qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
+ }
+
+ return 0;
+}
diff --git a/net/filter.c b/net/filter.c
index 570813c..12d52e5 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -21,6 +21,7 @@
#include "net/vhost_net.h"
#include "filters.h"
#include "net/queue.h"
+#include "filters.h"
static QTAILQ_HEAD(, NetFilterState) net_filters;
@@ -214,7 +215,7 @@ typedef int (NetFilterInit)(const NetFilter *netfilter,
static
NetFilterInit * const net_filter_init_fun[NET_FILTER_TYPE_MAX] = {
- [NET_FILTER_TYPE_DUMMY] = net_init_filter_dummy,
+ [NET_FILTER_TYPE_BUFFER] = net_init_filter_buffer,
};
static int net_filter_init1(const NetFilter *netfilter, Error **errp)
diff --git a/net/filters.h b/net/filters.h
index 7c1f30d..8dd9970 100644
--- a/net/filters.h
+++ b/net/filters.h
@@ -11,13 +11,7 @@
#include "net/net.h"
#include "net/filter.h"
-int net_init_filter_dummy(const NetFilter *netfilter, const char *name,
- int chain, NetClientState *netdev, Error **errp);
-
-int net_init_filter_dummy(const NetFilter *netfilter, const char *name,
- int chain, NetClientState *netdev, Error **errp)
-{
- return 0;
-}
+int net_init_filter_buffer(const NetFilter *netfilter, const char *name,
+ int chain, NetClientState *netdev, Error **errp);
#endif /* QEMU_NET_FILTERS_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index d961ac8..0e8dc17 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2566,14 +2566,20 @@
{ 'command': 'netfilter-del', 'data': {'id': 'str'} }
##
-# @NetFilterDummyOptions:
+# @NetFilterBufferOptions
#
-# An dummy filter for network backend
+# a netbuffer filter for network backend.
#
-# Since: 2.5
+# @interval: #optional release packets by interval, if no interval supplied,
+# packets will be buffered forever unless internal APIs been called
+# to release the packets or filter been removed. Use with caution.
+# scale: microsecond
+#
+# Since 2.5
##
-{ 'struct': 'NetFilterDummyOptions',
- 'data': { } }
+{ 'struct': 'NetFilterBufferOptions',
+ 'data': {
+ '*interval': 'uint32' } }
##
# @NetFilterType:
@@ -2583,7 +2589,7 @@
# Since: 2.5
##
{ 'enum': 'NetFilterType',
- 'data': ['dummy'] }
+ 'data': ['buffer'] }
##
# @NetFilterBase
@@ -2623,7 +2629,7 @@
'base': 'NetFilterBase',
'discriminator': 'type',
'data': {
- 'dummy': 'NetFilterDummyOptions' } }
+ 'buffer': 'NetFilterBufferOptions' } }
##
# @InetSocketAddress
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH v9 10/10] tests: add test cases for netfilter object
2015-09-01 9:06 [Qemu-devel] [PATCH v9 00/10] Add a netfilter object and netbuffer filter Yang Hongyang
` (8 preceding siblings ...)
2015-09-01 9:06 ` [Qemu-devel] [PATCH v9 09/10] netfilter: add a netbuffer filter Yang Hongyang
@ 2015-09-01 9:06 ` Yang Hongyang
9 siblings, 0 replies; 32+ messages in thread
From: Yang Hongyang @ 2015-09-01 9:06 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, zhang.zhanghailiang, lizhijian, jasowang, armbru, mrhines,
stefanha, Yang Hongyang
Using qtest qmp interface to implement following cases:
1) add/remove netfilter
2) add a netfilter then delete the netdev
3) add/remove more than one netfilters
4) add more than one netfilters and then delete the netdev
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
v9: qmp command change due to the qapi change
---
tests/.gitignore | 1 +
tests/Makefile | 2 +
tests/test-netfilter.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 197 insertions(+)
create mode 100644 tests/test-netfilter.c
diff --git a/tests/.gitignore b/tests/.gitignore
index ccc92e4..395962b 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -41,5 +41,6 @@ test-vmstate
test-write-threshold
test-x86-cpuid
test-xbzrle
+test-netfilter
*-test
qapi-schema/*.test.*
diff --git a/tests/Makefile b/tests/Makefile
index 5271123..bc59ad8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -185,6 +185,7 @@ check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
check-qtest-i386-y += tests/q35-test$(EXESUF)
gcov-files-i386-y += hw/pci-host/q35.c
check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF)
+check-qtest-i386-y += tests/test-netfilter$(EXESUF)
check-qtest-x86_64-y = $(check-qtest-i386-y)
gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -409,6 +410,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o
tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(block-obj-y) libqemuutil.a libqemustub.a
+tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
ifeq ($(CONFIG_POSIX),y)
LIBS += -lutil
diff --git a/tests/test-netfilter.c b/tests/test-netfilter.c
new file mode 100644
index 0000000..1bbcb77
--- /dev/null
+++ b/tests/test-netfilter.c
@@ -0,0 +1,194 @@
+/*
+ * QTest testcase for netfilter
+ *
+ * Copyright (c) 2015 FUJITSU LIMITED
+ * Author: Yang Hongyang <yanghy@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 <glib.h>
+#include "libqtest.h"
+
+/* add a netfilter to a netdev and then remove it */
+static void add_one_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{'execute': 'netfilter-add',"
+ " 'arguments': { data: {"
+ " 'type': 'buffer',"
+ " 'id': 'qtest-f0',"
+ " 'netdev': 'qtest-bn0',"
+ " 'chain': 'in',"
+ " 'interval': '1000'"
+ "}}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{'execute': 'netfilter-del',"
+ " 'arguments': {"
+ " 'id': 'qtest-f0'"
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+/* add a netfilter to a netdev and then remove the netdev */
+static void remove_netdev_with_one_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{'execute': 'netfilter-add',"
+ " 'arguments': { data: {"
+ " 'type': 'buffer',"
+ " 'id': 'qtest-f0',"
+ " 'netdev': 'qtest-bn0',"
+ " 'chain': 'in',"
+ " 'interval': '1000'"
+ "}}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{'execute': 'netdev_del',"
+ " 'arguments': {"
+ " 'id': 'qtest-bn0'"
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ /* add back the netdev */
+ response = qmp("{'execute': 'netdev_add',"
+ " 'arguments': {"
+ " 'type': 'user',"
+ " 'id': 'qtest-bn0'"
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+/* add multi(2) netfilters to a netdev and then remove them */
+static void add_multi_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{'execute': 'netfilter-add',"
+ " 'arguments': { data: {"
+ " 'type': 'buffer',"
+ " 'id': 'qtest-f0',"
+ " 'netdev': 'qtest-bn0',"
+ " 'chain': 'in',"
+ " 'interval': '1000'"
+ "}}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{'execute': 'netfilter-add',"
+ " 'arguments': { data: {"
+ " 'type': 'buffer',"
+ " 'id': 'qtest-f1',"
+ " 'netdev': 'qtest-bn0',"
+ " 'chain': 'in',"
+ " 'interval': '1000'"
+ "}}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{'execute': 'netfilter-del',"
+ " 'arguments': {"
+ " 'id': 'qtest-f0'"
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{'execute': 'netfilter-del',"
+ " 'arguments': {"
+ " 'id': 'qtest-f1'"
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+/* add multi(2) netfilters to a netdev and then remove the netdev */
+static void remove_netdev_with_multi_netfilter(void)
+{
+ QDict *response;
+
+ response = qmp("{'execute': 'netfilter-add',"
+ " 'arguments': { data: {"
+ " 'type': 'buffer',"
+ " 'id': 'qtest-f0',"
+ " 'netdev': 'qtest-bn0',"
+ " 'chain': 'in',"
+ " 'interval': '1000'"
+ "}}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{'execute': 'netfilter-add',"
+ " 'arguments': { data: {"
+ " 'type': 'buffer',"
+ " 'id': 'qtest-f1',"
+ " 'netdev': 'qtest-bn0',"
+ " 'chain': 'in',"
+ " 'interval': '1000'"
+ "}}}");
+
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ response = qmp("{'execute': 'netdev_del',"
+ " 'arguments': {"
+ " 'id': 'qtest-bn0'"
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+
+ /* add back the netdev */
+ response = qmp("{'execute': 'netdev_add',"
+ " 'arguments': {"
+ " 'type': 'user',"
+ " 'id': 'qtest-bn0'"
+ "}}");
+ g_assert(response);
+ g_assert(!qdict_haskey(response, "error"));
+ QDECREF(response);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+ qtest_add_func("/netfilter/addremove_one", add_one_netfilter);
+ qtest_add_func("/netfilter/remove_netdev_one",
+ remove_netdev_with_one_netfilter);
+ qtest_add_func("/netfilter/addremove_multi", add_multi_netfilter);
+ qtest_add_func("/netfilter/remove_netdev_multi",
+ remove_netdev_with_multi_netfilter);
+
+ qtest_start("-netdev user,id=qtest-bn0 -device e1000,netdev=qtest-bn0");
+ ret = g_test_run();
+
+ qtest_end();
+
+ return ret;
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread