[Qemu-devel] [PATCH 00/12] For QEMU 2.5: Add a netfilter object and netbuffer filter

[Qemu-devel] [PATCH 00/12] For QEMU 2.5: Add a netfilter object and netbuffer filter

[Yang Hongyang]

I do not take this as v2 because the design of this has been changed,
and code is rewritten. according to thread:
http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg05445.html

This patch add a new object netfilter, capture all network packets.
Also implement a netbuffer based on this object.
the "buffer" netfilter could be used by VM FT solutions like
Macrocheckpointing, to buffer/release packets. Or to simulate
packet delay.

Usage:
 -netdev tap,id=bn0
 -netfilter buffer,id=f0,netdev=bn0,interval=1000
 -device e1000,netdev=bn0

dynamically add/remove netfilters:
 netfilter_add buffer,id=f0,netdev=bn0,interval=1000
 netfilter_del f0

TODO:
 - multiqueue support
 - dump

Yang Hongyang (12):
  net: add a new object netfilter
  init/cleanup of netfilter object
  netfilter: add netfilter_{add|del} commands
  net: add/remove filters from network backend
  netfilter: hook packets before receive
  netfilter: provide a compat receive_iov
  net/queue: export qemu_net_queue_append
  move out net queue structs define
  netfilter: add a netbuffer filter
  netbuffer: add a public api filter_buffer_release_all
  filter/buffer: add an interval option to buffer filter
  filter/buffer: update command description and help

 hmp-commands.hx         |  30 ++++++
 hmp.c                   |  29 ++++++
 hmp.h                   |   4 +
 include/net/filter.h    |  59 +++++++++++
 include/net/net.h       |   8 ++
 include/net/queue.h     |  26 +++++
 include/qemu/typedefs.h |   1 +
 include/sysemu/sysemu.h |   1 +
 monitor.c               |  33 ++++++
 net/Makefile.objs       |   2 +
 net/filter-buffer.c     | 192 ++++++++++++++++++++++++++++++++++
 net/filter.c            | 270 ++++++++++++++++++++++++++++++++++++++++++++++++
 net/filters.h           |  17 +++
 net/net.c               | 103 +++++++++++++++++-
 net/queue.c             |  31 ++----
 qapi-schema.json        |  86 +++++++++++++++
 qemu-options.hx         |   4 +
 qmp-commands.hx         |  55 ++++++++++
 vl.c                    |  13 +++
 19 files changed, 938 insertions(+), 26 deletions(-)
 create mode 100644 include/net/filter.h
 create mode 100644 net/filter-buffer.c
 create mode 100644 net/filter.c
 create mode 100644 net/filters.h

-- 
1.9.1

[Qemu-devel] [PATCH 01/12] net: add a new object netfilter

[Yang Hongyang]

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 include/net/filter.h    | 15 +++++++++++++++
 include/sysemu/sysemu.h |  1 +
 net/Makefile.objs       |  1 +
 net/filter.c            | 27 +++++++++++++++++++++++++++
 qemu-options.hx         |  1 +
 vl.c                    | 13 +++++++++++++
 6 files changed, 58 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 8c9add9..8c1eb30 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "socket][,vlan=n][,option][,option][,...]\n"
     "                old way to initialize a host network interface\n"
     "                (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
+DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
 STEXI
 @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
 @findex -net
diff --git a/vl.c b/vl.c
index 5856396..1a0ebe1 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

Re: [Qemu-devel] [PATCH 01/12] net: add a new object netfilter

[Thomas Huth]

On Wednesday, July 29, 2015 12:51:45 PM,
"Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

Maybe add at least a very short patch description? You're doing
a little bit more than just "add a new object netfilter" as
mentioned in the title...

> ---
>  include/net/filter.h    | 15 +++++++++++++++
>  include/sysemu/sysemu.h |  1 +
>  net/Makefile.objs       |  1 +
>  net/filter.c            | 27 +++++++++++++++++++++++++++
>  qemu-options.hx         |  1 +
>  vl.c                    | 13 +++++++++++++
>  6 files changed, 58 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 8c9add9..8c1eb30 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "socket][,vlan=n][,option][,option][,...]\n"
>      "                old way to initialize a host network interface\n"
>      "                (use the -netdev option if possible instead)\n",
>      QEMU_ARCH_ALL)
> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>  STEXI
>  @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}]
>  [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>  @findex -net
> diff --git a/vl.c b/vl.c
> index 5856396..1a0ebe1 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);

Looks good to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 01/12] net: add a new object netfilter

[Yang Hongyang]

On 07/29/2015 09:53 PM, Thomas Huth wrote:
> On Wednesday, July 29, 2015 12:51:45 PM,
> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>
> Maybe add at least a very short patch description? You're doing
> a little bit more than just "add a new object netfilter" as
> mentioned in the title...

How about:
Add a new object netfilter, used to capture packets send/receive
by network backend, init it after init net clients.

>
>> ---
>>   include/net/filter.h    | 15 +++++++++++++++
>>   include/sysemu/sysemu.h |  1 +
>>   net/Makefile.objs       |  1 +
>>   net/filter.c            | 27 +++++++++++++++++++++++++++
>>   qemu-options.hx         |  1 +
>>   vl.c                    | 13 +++++++++++++
>>   6 files changed, 58 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 8c9add9..8c1eb30 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>>       "socket][,vlan=n][,option][,option][,...]\n"
>>       "                old way to initialize a host network interface\n"
>>       "                (use the -netdev option if possible instead)\n",
>>       QEMU_ARCH_ALL)
>> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>>   STEXI
>>   @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}]
>>   [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
>>   @findex -net
>> diff --git a/vl.c b/vl.c
>> index 5856396..1a0ebe1 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);
>
> Looks good to me.
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 01/12] net: add a new object netfilter

[Thomas Huth]

On Wednesday, July 29, 2015 4:05:38 PM,
"Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> 
> On 07/29/2015 09:53 PM, Thomas Huth wrote:
> > On Wednesday, July 29, 2015 12:51:45 PM,
> > "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> >>
> >> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> >
> > Maybe add at least a very short patch description? You're doing
> > a little bit more than just "add a new object netfilter" as
> > mentioned in the title...
> 
> How about:
> Add a new object netfilter, used to capture packets send/receive
> by network backend, init it after init net clients.

I rather thought about a short remark about the fact that this
patch also adds the "-netfilter" option. So maybe something
like this:

"Add the framework for a new netfilter object and a new
-netfilter CLI option as a basis for the following patches"

?

 Thomas

Re: [Qemu-devel] [PATCH 01/12] net: add a new object netfilter

[Yang Hongyang]

On 07/29/2015 10:20 PM, Thomas Huth wrote:
> On Wednesday, July 29, 2015 4:05:38 PM,
> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>
>> On 07/29/2015 09:53 PM, Thomas Huth wrote:
>>> On Wednesday, July 29, 2015 12:51:45 PM,
>>> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>
>>> Maybe add at least a very short patch description? You're doing
>>> a little bit more than just "add a new object netfilter" as
>>> mentioned in the title...
>>
>> How about:
>> Add a new object netfilter, used to capture packets send/receive
>> by network backend, init it after init net clients.
>
> I rather thought about a short remark about the fact that this
> patch also adds the "-netfilter" option. So maybe something
> like this:
>
> "Add the framework for a new netfilter object and a new
> -netfilter CLI option as a basis for the following patches"
>
> ?

Seems better, thank you, will update!

>
>   Thomas
> .
>

-- 
Thanks,
Yang.

[Qemu-devel] [PATCH 02/12] init/cleanup of netfilter object

[Yang Hongyang]

This is mostly the same with init/cleanup of netdev object.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 include/net/filter.h    |  21 ++++++++
 include/qemu/typedefs.h |   1 +
 net/filter.c            | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json        |  30 +++++++++++
 4 files changed, 183 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 4242ded..fa813c4 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -9,7 +9,28 @@
 #define QEMU_NET_FILTER_H
 
 #include "qemu-common.h"
+#include "qemu/typedefs.h"
+
+typedef void (FilterCleanup) (NetFilterState *);
+
+typedef struct NetFilterInfo {
+    NetFilterOptionsKind type;
+    size_t size;
+    FilterCleanup *cleanup;
+} NetFilterInfo;
+
+struct NetFilterState {
+    NetFilterInfo *info;
+    char *model;
+    char *name;
+    NetClientState *netdev;
+    QTAILQ_ENTRY(NetFilterState) next;
+};
 
 int net_init_filters(void);
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+                                    NetClientState *netdev,
+                                    const char *model,
+                                    const char *name);
 
 #endif /* QEMU_NET_FILTER_H */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 6fdcbcd..2f75109 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..e6fdc26 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -6,10 +6,141 @@
  */
 
 #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"
+
+static QTAILQ_HEAD(, NetFilterState) net_filters;
+
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+                                    NetClientState *netdev,
+                                    const char *model,
+                                    const char *name)
+{
+    NetFilterState *nf;
+
+    assert(info->size >= sizeof(NetFilterState));
+
+    nf = g_malloc0(info->size);
+    nf->info = info;
+    nf->model = g_strdup(model);
+    nf->name = g_strdup(name);
+    nf->netdev = netdev;
+    QTAILQ_INSERT_TAIL(&net_filters, nf, next);
+    /* TODO: attach netfilter to netdev */
+
+    return nf;
+}
+
+static __attribute__((unused)) void qemu_cleanup_net_filter(NetFilterState *nf)
+{
+    /* TODO: remove netfilter from netdev */
+
+    QTAILQ_REMOVE(&net_filters, nf, next);
+
+    if (nf->info->cleanup) {
+        nf->info->cleanup(nf);
+    }
+
+    g_free(nf->name);
+    g_free(nf->model);
+    g_free(nf);
+}
+
+typedef int (NetFilterInit)(const NetFilterOptions *opts,
+                            const char *name,
+                            NetClientState *netdev, Error **errp);
+
+static
+NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
+};
+
+static int net_filter_init1(const NetFilter *netfilter, Error **errp)
+{
+    NetClientState *netdev = NULL;
+    NetClientState *ncs[MAX_QUEUE_NUM];
+    const char *name = netfilter->id;
+    const char *netdev_id = netfilter->netdev;
+    const NetFilterOptions *opts = netfilter->opts;
+    int queues;
+
+    if (!net_filter_init_fun[opts->kind]) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+                   "a net filter type");
+        return -1;
+    }
+
+    queues = qemu_find_net_clients_except(netdev_id, ncs,
+                                          NET_CLIENT_OPTIONS_KIND_NIC,
+                                          MAX_QUEUE_NUM);
+    if (queues > 1) {
+        error_setg(errp, "multiqueues is not supported by now");
+        return -1;
+    } else if (queues < 1) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev",
+                   "a network backend id");
+        return -1;
+    }
+
+    netdev = ncs[0];
+
+
+    if (net_filter_init_fun[opts->kind](opts, name, netdev, errp) < 0) {
+        if (errp && !*errp) {
+            error_setg(errp, QERR_DEVICE_INIT_FAILED,
+                       NetFilterOptionsKind_lookup[opts->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);
+    }
+
+    error_propagate(errp, 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/qapi-schema.json b/qapi-schema.json
index a0a45f7..9a7c107 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2537,6 +2537,36 @@
     'opts': 'NetClientOptions' } }
 
 ##
+# @NetFilterOptions
+#
+# A discriminated record of network filters.
+#
+# Since 2.5
+#
+##
+{ 'union': 'NetFilterOptions',
+  'data': { } }
+
+##
+# @NetFilter
+#
+# Captures the packets of a network backend.
+#
+# @id: identifier for monitor commands.
+#
+# @netdev: the network backend it attached to.
+#
+# @opts: filter type specific properties
+#
+# Since 2.5
+##
+{ 'struct': 'NetFilter',
+  'data': {
+    'id':   'str',
+    'netdev': 'str',
+    'opts': 'NetFilterOptions' } }
+
+##
 # @InetSocketAddress
 #
 # Captures a socket address or address range in the Internet namespace.
-- 
1.9.1

Re: [Qemu-devel] [PATCH 02/12] init/cleanup of netfilter object

[Thomas Huth]

On Wednesday, July 29, 2015 12:51:46 PM,
"Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>
> This is mostly the same with init/cleanup of netdev object.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  include/net/filter.h    |  21 ++++++++
>  include/qemu/typedefs.h |   1 +
>  net/filter.c            | 131
>  ++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json        |  30 +++++++++++
>  4 files changed, 183 insertions(+)
> 
[...]
> diff --git a/net/filter.c b/net/filter.c
> index 4e40f08..e6fdc26 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -6,10 +6,141 @@
>   */
>  
>  #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"
> +
> +static QTAILQ_HEAD(, NetFilterState) net_filters;
> +
> +NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
> +                                    NetClientState *netdev,
> +                                    const char *model,
> +                                    const char *name)
> +{
> +    NetFilterState *nf;
> +
> +    assert(info->size >= sizeof(NetFilterState));
> +
> +    nf = g_malloc0(info->size);
> +    nf->info = info;
> +    nf->model = g_strdup(model);
> +    nf->name = g_strdup(name);
> +    nf->netdev = netdev;
> +    QTAILQ_INSERT_TAIL(&net_filters, nf, next);
> +    /* TODO: attach netfilter to netdev */
> +
> +    return nf;
> +}
> +
> +static __attribute__((unused)) void qemu_cleanup_net_filter(NetFilterState
> *nf)

Maybe rather add this function in the patch you really need it? Then you could
avoid the ugly attribute-unused here.

> +{
> +    /* TODO: remove netfilter from netdev */
> +
> +    QTAILQ_REMOVE(&net_filters, nf, next);
> +
> +    if (nf->info->cleanup) {
> +        nf->info->cleanup(nf);
> +    }
> +
> +    g_free(nf->name);
> +    g_free(nf->model);
> +    g_free(nf);
> +}
> +
> +typedef int (NetFilterInit)(const NetFilterOptions *opts,
> +                            const char *name,
> +                            NetClientState *netdev, Error **errp);
> +
> +static
> +NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
> +};
> +
> +static int net_filter_init1(const NetFilter *netfilter, Error **errp)
> +{
> +    NetClientState *netdev = NULL;
> +    NetClientState *ncs[MAX_QUEUE_NUM];
> +    const char *name = netfilter->id;
> +    const char *netdev_id = netfilter->netdev;
> +    const NetFilterOptions *opts = netfilter->opts;
> +    int queues;
> +
> +    if (!net_filter_init_fun[opts->kind]) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
> +                   "a net filter type");
> +        return -1;
> +    }
> +
> +    queues = qemu_find_net_clients_except(netdev_id, ncs,
> +                                          NET_CLIENT_OPTIONS_KIND_NIC,
> +                                          MAX_QUEUE_NUM);
> +    if (queues > 1) {
> +        error_setg(errp, "multiqueues is not supported by now");
> +        return -1;
> +    } else if (queues < 1) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev",
> +                   "a network backend id");
> +        return -1;
> +    }
> +
> +    netdev = ncs[0];
> +
> +

One empty line should be enough.

> +    if (net_filter_init_fun[opts->kind](opts, name, netdev, errp) < 0) {
> +        if (errp && !*errp) {
> +            error_setg(errp, QERR_DEVICE_INIT_FAILED,
> +                       NetFilterOptionsKind_lookup[opts->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;
> +
> +    {

Why the extra curly brackets here? Looks somewhat strange, can you also
do it without?

> +        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);
> +    }
> +
> +    error_propagate(errp, 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/qapi-schema.json b/qapi-schema.json
> index a0a45f7..9a7c107 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2537,6 +2537,36 @@
>      'opts': 'NetClientOptions' } }
>  
>  ##
> +# @NetFilterOptions
> +#
> +# A discriminated record of network filters.
> +#
> +# Since 2.5
> +#
> +##
> +{ 'union': 'NetFilterOptions',
> +  'data': { } }
> +
> +##
> +# @NetFilter
> +#
> +# Captures the packets of a network backend.
> +#
> +# @id: identifier for monitor commands.
> +#
> +# @netdev: the network backend it attached to.
> +#
> +# @opts: filter type specific properties
> +#
> +# Since 2.5
> +##
> +{ 'struct': 'NetFilter',
> +  'data': {
> +    'id':   'str',
> +    'netdev': 'str',
> +    'opts': 'NetFilterOptions' } }
> +
> +##
>  # @InetSocketAddress
>  #
>  # Captures a socket address or address range in the Internet namespace.

Apart from the nits, the patch looks ok to me.

 Thomas

Re: [Qemu-devel] [PATCH 02/12] init/cleanup of netfilter object

[Yang Hongyang]

Hi Thomas,
   Thanks for the review.

On 07/29/2015 09:33 PM, Thomas Huth wrote:
> On Wednesday, July 29, 2015 12:51:46 PM,
> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>
>> This is mostly the same with init/cleanup of netdev object.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> ---
>>   include/net/filter.h    |  21 ++++++++
>>   include/qemu/typedefs.h |   1 +
>>   net/filter.c            | 131
>>   ++++++++++++++++++++++++++++++++++++++++++++++++
>>   qapi-schema.json        |  30 +++++++++++
>>   4 files changed, 183 insertions(+)
>>
> [...]
>> diff --git a/net/filter.c b/net/filter.c
>> index 4e40f08..e6fdc26 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -6,10 +6,141 @@
>>    */
>>
>>   #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"
>> +
>> +static QTAILQ_HEAD(, NetFilterState) net_filters;
>> +
>> +NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
>> +                                    NetClientState *netdev,
>> +                                    const char *model,
>> +                                    const char *name)
>> +{
>> +    NetFilterState *nf;
>> +
>> +    assert(info->size >= sizeof(NetFilterState));
>> +
>> +    nf = g_malloc0(info->size);
>> +    nf->info = info;
>> +    nf->model = g_strdup(model);
>> +    nf->name = g_strdup(name);
>> +    nf->netdev = netdev;
>> +    QTAILQ_INSERT_TAIL(&net_filters, nf, next);
>> +    /* TODO: attach netfilter to netdev */
>> +
>> +    return nf;
>> +}
>> +
>> +static __attribute__((unused)) void qemu_cleanup_net_filter(NetFilterState
>> *nf)
>
> Maybe rather add this function in the patch you really need it? Then you could
> avoid the ugly attribute-unused here.

It will be removed in the next patch. I put cleanup here in order to pair with
the new function, and could be easy to review...

>
>> +{
>> +    /* TODO: remove netfilter from netdev */
>> +
>> +    QTAILQ_REMOVE(&net_filters, nf, next);
>> +
>> +    if (nf->info->cleanup) {
>> +        nf->info->cleanup(nf);
>> +    }
>> +
>> +    g_free(nf->name);
>> +    g_free(nf->model);
>> +    g_free(nf);
>> +}
>> +
>> +typedef int (NetFilterInit)(const NetFilterOptions *opts,
>> +                            const char *name,
>> +                            NetClientState *netdev, Error **errp);
>> +
>> +static
>> +NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
>> +};
>> +
>> +static int net_filter_init1(const NetFilter *netfilter, Error **errp)
>> +{
>> +    NetClientState *netdev = NULL;
>> +    NetClientState *ncs[MAX_QUEUE_NUM];
>> +    const char *name = netfilter->id;
>> +    const char *netdev_id = netfilter->netdev;
>> +    const NetFilterOptions *opts = netfilter->opts;
>> +    int queues;
>> +
>> +    if (!net_filter_init_fun[opts->kind]) {
>> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
>> +                   "a net filter type");
>> +        return -1;
>> +    }
>> +
>> +    queues = qemu_find_net_clients_except(netdev_id, ncs,
>> +                                          NET_CLIENT_OPTIONS_KIND_NIC,
>> +                                          MAX_QUEUE_NUM);
>> +    if (queues > 1) {
>> +        error_setg(errp, "multiqueues is not supported by now");
>> +        return -1;
>> +    } else if (queues < 1) {
>> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev",
>> +                   "a network backend id");
>> +        return -1;
>> +    }
>> +
>> +    netdev = ncs[0];
>> +
>> +
>
> One empty line should be enough.

Got, thanks!

>
>> +    if (net_filter_init_fun[opts->kind](opts, name, netdev, errp) < 0) {
>> +        if (errp && !*errp) {
>> +            error_setg(errp, QERR_DEVICE_INIT_FAILED,
>> +                       NetFilterOptionsKind_lookup[opts->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;
>> +
>> +    {
>
> Why the extra curly brackets here? Looks somewhat strange, can you also
> do it without?

actually it is almost the same with net.c, but sure, can remove it in
the next version.

>
>> +        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);
>> +    }
>> +
>> +    error_propagate(errp, 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/qapi-schema.json b/qapi-schema.json
>> index a0a45f7..9a7c107 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2537,6 +2537,36 @@
>>       'opts': 'NetClientOptions' } }
>>
>>   ##
>> +# @NetFilterOptions
>> +#
>> +# A discriminated record of network filters.
>> +#
>> +# Since 2.5
>> +#
>> +##
>> +{ 'union': 'NetFilterOptions',
>> +  'data': { } }
>> +
>> +##
>> +# @NetFilter
>> +#
>> +# Captures the packets of a network backend.
>> +#
>> +# @id: identifier for monitor commands.
>> +#
>> +# @netdev: the network backend it attached to.
>> +#
>> +# @opts: filter type specific properties
>> +#
>> +# Since 2.5
>> +##
>> +{ 'struct': 'NetFilter',
>> +  'data': {
>> +    'id':   'str',
>> +    'netdev': 'str',
>> +    'opts': 'NetFilterOptions' } }
>> +
>> +##
>>   # @InetSocketAddress
>>   #
>>   # Captures a socket address or address range in the Internet namespace.
>
> Apart from the nits, the patch looks ok to me.

Thank you.

>
>   Thomas
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 02/12] init/cleanup of netfilter object

[Thomas Huth]

On Wednesday, July 29, 2015 3:50:02 PM,
"Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> 
> On 07/29/2015 09:33 PM, Thomas Huth wrote:
> > On Wednesday, July 29, 2015 12:51:46 PM,
> > "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> >>
> >> This is mostly the same with init/cleanup of netdev object.
> >>
> >> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> >> ---
> >>   include/net/filter.h    |  21 ++++++++
> >>   include/qemu/typedefs.h |   1 +
> >>   net/filter.c            | 131
> >>   ++++++++++++++++++++++++++++++++++++++++++++++++
> >>   qapi-schema.json        |  30 +++++++++++
> >>   4 files changed, 183 insertions(+)
> >>
> > [...]
> >> diff --git a/net/filter.c b/net/filter.c
> >> index 4e40f08..e6fdc26 100644
> >> --- a/net/filter.c
> >> +++ b/net/filter.c
> >> @@ -6,10 +6,141 @@
> >>    */
> >>
> >>   #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"
> >> +
> >> +static QTAILQ_HEAD(, NetFilterState) net_filters;
> >> +
> >> +NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
> >> +                                    NetClientState *netdev,
> >> +                                    const char *model,
> >> +                                    const char *name)
> >> +{
> >> +    NetFilterState *nf;
> >> +
> >> +    assert(info->size >= sizeof(NetFilterState));
> >> +
> >> +    nf = g_malloc0(info->size);
> >> +    nf->info = info;
> >> +    nf->model = g_strdup(model);
> >> +    nf->name = g_strdup(name);
> >> +    nf->netdev = netdev;
> >> +    QTAILQ_INSERT_TAIL(&net_filters, nf, next);
> >> +    /* TODO: attach netfilter to netdev */
> >> +
> >> +    return nf;
> >> +}
> >> +
> >> +static __attribute__((unused)) void
> >> qemu_cleanup_net_filter(NetFilterState
> >> *nf)
> >
> > Maybe rather add this function in the patch you really need it? Then you
> > could
> > avoid the ugly attribute-unused here.
> 
> It will be removed in the next patch. I put cleanup here in order to pair
> with the new function, and could be easy to review...

You could also use another trick: Use "static inline" instead of "static
__attribute__((unused))" ... then GCC also does not complain about unused
static functions!

 Thomas

Re: [Qemu-devel] [PATCH 02/12] init/cleanup of netfilter object

[Yang Hongyang]

On 07/29/2015 09:58 PM, Thomas Huth wrote:
> On Wednesday, July 29, 2015 3:50:02 PM,
> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>
>> On 07/29/2015 09:33 PM, Thomas Huth wrote:
>>> On Wednesday, July 29, 2015 12:51:46 PM,
>>> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>>>
>>>> This is mostly the same with init/cleanup of netdev object.
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>> ---
>>>>    include/net/filter.h    |  21 ++++++++
>>>>    include/qemu/typedefs.h |   1 +
>>>>    net/filter.c            | 131
>>>>    ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    qapi-schema.json        |  30 +++++++++++
>>>>    4 files changed, 183 insertions(+)
>>>>
>>> [...]
>>>> diff --git a/net/filter.c b/net/filter.c
>>>> index 4e40f08..e6fdc26 100644
>>>> --- a/net/filter.c
>>>> +++ b/net/filter.c
>>>> @@ -6,10 +6,141 @@
>>>>     */
>>>>
>>>>    #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"
>>>> +
>>>> +static QTAILQ_HEAD(, NetFilterState) net_filters;
>>>> +
>>>> +NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
>>>> +                                    NetClientState *netdev,
>>>> +                                    const char *model,
>>>> +                                    const char *name)
>>>> +{
>>>> +    NetFilterState *nf;
>>>> +
>>>> +    assert(info->size >= sizeof(NetFilterState));
>>>> +
>>>> +    nf = g_malloc0(info->size);
>>>> +    nf->info = info;
>>>> +    nf->model = g_strdup(model);
>>>> +    nf->name = g_strdup(name);
>>>> +    nf->netdev = netdev;
>>>> +    QTAILQ_INSERT_TAIL(&net_filters, nf, next);
>>>> +    /* TODO: attach netfilter to netdev */
>>>> +
>>>> +    return nf;
>>>> +}
>>>> +
>>>> +static __attribute__((unused)) void
>>>> qemu_cleanup_net_filter(NetFilterState
>>>> *nf)
>>>
>>> Maybe rather add this function in the patch you really need it? Then you
>>> could
>>> avoid the ugly attribute-unused here.
>>
>> It will be removed in the next patch. I put cleanup here in order to pair
>> with the new function, and could be easy to review...
>
> You could also use another trick: Use "static inline" instead of "static
> __attribute__((unused))" ... then GCC also does not complain about unused
> static functions!

Ok, thanks.

>
>   Thomas
> .
>

-- 
Thanks,
Yang.

[Qemu-devel] [PATCH 03/12] netfilter: add netfilter_{add|del} commands

[Yang Hongyang]

add netfilter_{add|del} commands
This is mostly the same with netdev_{add|del} commands.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 hmp-commands.hx      | 30 +++++++++++++++++++++
 hmp.c                | 29 ++++++++++++++++++++
 hmp.h                |  4 +++
 include/net/filter.h |  2 ++
 monitor.c            | 33 +++++++++++++++++++++++
 net/filter.c         | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 qapi-schema.json     | 40 +++++++++++++++++++++++++++
 qmp-commands.hx      | 55 +++++++++++++++++++++++++++++++++++++
 8 files changed, 268 insertions(+), 1 deletion(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..5326a82 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     = "[type],id=str,netdev=str[,prop=value][,...]",
+        .help       = "add netfilter",
+        .mhandler.cmd = hmp_netfilter_add,
+        .command_completion = netfilter_add_completion,
+    },
+
+STEXI
+@item netfilter_add
+@findex netfilter_add
+Add netfilter.
+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 netfilter.
+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 fa813c4..1dd86cf 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -32,5 +32,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
                                     NetClientState *netdev,
                                     const char *model,
                                     const char *name);
+void netfilter_add(QemuOpts *opts, Error **errp);
+void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
 
 #endif /* QEMU_NET_FILTER_H */
diff --git a/monitor.c b/monitor.c
index aeea2b5..d6b8f24 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; NetFilterOptionsKind_lookup[i]; i++) {
+        add_completion_option(rs, str, NetFilterOptionsKind_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 e6fdc26..1685fbc 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"
@@ -39,7 +40,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
     return nf;
 }
 
-static __attribute__((unused)) void qemu_cleanup_net_filter(NetFilterState *nf)
+static void qemu_cleanup_net_filter(NetFilterState *nf)
 {
     /* TODO: remove netfilter from netdev */
 
@@ -54,6 +55,79 @@ static __attribute__((unused)) void qemu_cleanup_net_filter(NetFilterState *nf)
     g_free(nf);
 }
 
+static void qemu_del_net_filter(NetFilterState *nf)
+{
+    /* handle multi queue? */
+    qemu_cleanup_net_filter(nf);
+}
+
+static NetFilterState *qemu_find_netfilter(const char *id)
+{
+    NetFilterState *nf;
+
+    QTAILQ_FOREACH(nf, &net_filters, next) {
+        if (!strcmp(nf->name, id)) {
+            return nf;
+        }
+    }
+
+    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);
+}
+
+void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp)
+{
+    Error *local_err = NULL;
+    QemuOptsList *opts_list;
+    QemuOpts *opts;
+
+    opts_list = qemu_find_opts_err("netfilter", &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    opts = qemu_opts_from_qdict(opts_list, qdict, &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    netfilter_add(opts, &local_err);
+    if (local_err) {
+        qemu_opts_del(opts);
+        goto out;
+    }
+
+out:
+    error_propagate(errp, local_err);
+}
+
+void qmp_netfilter_del(const char *id, Error **errp)
+{
+    NetFilterState *nf;
+    QemuOpts *opts;
+
+    nf = qemu_find_netfilter(id);
+    if (!nf) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", id);
+        return;
+    }
+
+    opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), id);
+    if (!opts) {
+        error_setg(errp, "Device '%s' is not a netfilter", id);
+        return;
+    }
+
+    qemu_del_net_filter(nf);
+    qemu_opts_del(opts);
+}
+
 typedef int (NetFilterInit)(const NetFilterOptions *opts,
                             const char *name,
                             NetClientState *netdev, Error **errp);
diff --git a/qapi-schema.json b/qapi-schema.json
index 9a7c107..1fc6390 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2537,6 +2537,46 @@
     'opts': 'NetClientOptions' } }
 
 ##
+# @netfilter_add:
+#
+# Add a netfilter.
+#
+# @type: the type of netfilter.
+#
+# @id: the name of the new netfilter.
+#
+# @netdev: the name of the netdev which this filter will be attached to.
+#
+# @props: #optional a list of properties to be passed to the netfilter in
+#         the format of 'name=value'
+#
+# Since: 2.5
+#
+# Returns: Nothing on success
+#          If @type is not a valid netfilter, DeviceNotFound
+##
+{ 'command': 'netfilter_add',
+  'data': {
+    'type': 'str',
+    'id':   'str',
+    'netdev': 'str',
+    '*props': '**'}, 'gen': false }
+
+##
+# @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'} }
+
+##
 # @NetFilterOptions
 #
 # A discriminated record of network filters.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..9e62e0b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -926,6 +926,61 @@ Example:
 EQMP
 
     {
+        .name       = "netfilter_add",
+        .args_type  = "netfilter:O",
+        .mhandler.cmd_new = qmp_netfilter_add,
+    },
+
+SQMP
+netfilter_add
+----------
+
+Add netfilter.
+
+Arguments:
+
+- "type": the filter type (json-string)
+- "id": the netfilter's ID, must be unique (json-string)
+- "netdev": the netdev's ID which this filter will be attached to(json-string)
+- filter options
+
+Example:
+
+-> { "execute": "netfilter_add",
+                "arguments": { "type": "type", "id": "nf0", "netdev": "bn" } }
+<- { "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 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

Re: [Qemu-devel] [PATCH 03/12] netfilter: add netfilter_{add|del} commands

[Thomas Huth]

On Wednesday, July 29, 2015 12:51:47 PM,
"Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> 
> add netfilter_{add|del} commands
> This is mostly the same with netdev_{add|del} commands.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
...
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..5326a82 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     = "[type],id=str,netdev=str[,prop=value][,...]",

I'm not fully sure about the syntax, but writing "[type]" in square
brackets sounds like this parameter is optional to me. But if
I've got this right, it is not optional, is it? So maybe use
"<type>" instead?

 Thomas

Re: [Qemu-devel] [PATCH 03/12] netfilter: add netfilter_{add|del} commands

[Yang Hongyang]

On 07/29/2015 10:15 PM, Thomas Huth wrote:
> On Wednesday, July 29, 2015 12:51:47 PM,
> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>
>> add netfilter_{add|del} commands
>> This is mostly the same with netdev_{add|del} commands.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> ---
> ...
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index d3b7932..5326a82 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     = "[type],id=str,netdev=str[,prop=value][,...]",
>
> I'm not fully sure about the syntax, but writing "[type]" in square
> brackets sounds like this parameter is optional to me. But if
> I've got this right, it is not optional, is it? So maybe use
> "<type>" instead?

Yes, <type> could be better here, it will be updated in the last
patch when we have added a type "buffer"!

>
>   Thomas
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 03/12] netfilter: add netfilter_{add|del} commands

[Yang Hongyang]

On 07/29/2015 10:28 PM, Yang Hongyang wrote:
>
>
> On 07/29/2015 10:15 PM, Thomas Huth wrote:
>> On Wednesday, July 29, 2015 12:51:47 PM,
>> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>>
>>> add netfilter_{add|del} commands
>>> This is mostly the same with netdev_{add|del} commands.
>>>
>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>> ---
>> ...
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index d3b7932..5326a82 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     = "[type],id=str,netdev=str[,prop=value][,...]",
>>
>> I'm not fully sure about the syntax, but writing "[type]" in square
>> brackets sounds like this parameter is optional to me. But if
>> I've got this right, it is not optional, is it? So maybe use
>> "<type>" instead?
>
>  Yes, <type> could be better here,

but I'm not sure if we can use <> syntax here...

> it will be updated in the last
> patch when we have added a type "buffer"!
>
>>
>>   Thomas
>> .
>>
>

-- 
Thanks,
Yang.

[Qemu-devel] [PATCH 04/12] net: add/remove filters from network backend

[Yang Hongyang]

add/remove filters from network backend

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 include/net/net.h |  8 ++++++++
 net/filter.c      |  4 ++--
 net/net.c         | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 6a6cbef..5c5c109 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -40,6 +40,11 @@ typedef struct NICConf {
 
 
 /* Net clients */
+typedef struct Filter Filter;
+struct Filter {
+    NetFilterState *nf;
+    QTAILQ_ENTRY(Filter) next;
+};
 
 typedef void (NetPoll)(NetClientState *, bool enable);
 typedef int (NetCanReceive)(NetClientState *);
@@ -92,6 +97,7 @@ struct NetClientState {
     NetClientDestructor *destructor;
     unsigned int queue_index;
     unsigned rxfilter_notify_enabled:1;
+    QTAILQ_HEAD(, Filter) filters;
 };
 
 typedef struct NICState {
@@ -109,6 +115,8 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
                                     NetClientState *peer,
                                     const char *model,
                                     const char *name);
+int qemu_netdev_add_filter(NetClientState *nc, NetFilterState *nf);
+void qemu_netdev_remove_filter(NetClientState *nc, NetFilterState *nf);
 NICState *qemu_new_nic(NetClientInfo *info,
                        NICConf *conf,
                        const char *model,
diff --git a/net/filter.c b/net/filter.c
index 1685fbc..3d3b7bc 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -35,14 +35,14 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
     nf->name = g_strdup(name);
     nf->netdev = netdev;
     QTAILQ_INSERT_TAIL(&net_filters, nf, next);
-    /* TODO: attach netfilter to netdev */
+    qemu_netdev_add_filter(netdev, nf);
 
     return nf;
 }
 
 static void qemu_cleanup_net_filter(NetFilterState *nf)
 {
-    /* TODO: remove netfilter from netdev */
+    qemu_netdev_remove_filter(nf->netdev, nf);
 
     QTAILQ_REMOVE(&net_filters, nf, next);
 
diff --git a/net/net.c b/net/net.c
index 28a5597..22748e0 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,
@@ -305,6 +306,38 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
     return nc;
 }
 
+int qemu_netdev_add_filter(NetClientState *nc, NetFilterState *nf)
+{
+    Filter *filter = g_malloc0(sizeof(*filter));
+
+    filter->nf = nf;
+    QTAILQ_INSERT_TAIL(&nc->filters, filter, next);
+    return 0;
+}
+
+static void remove_filter(NetClientState *nc, Filter *filter)
+{
+    if (!filter) {
+        return;
+    }
+
+    QTAILQ_REMOVE(&nc->filters, filter, next);
+    g_free(filter);
+}
+
+void qemu_netdev_remove_filter(NetClientState *nc, NetFilterState *nf)
+{
+    Filter *filter = NULL;
+
+    QTAILQ_FOREACH(filter, &nc->filters, next) {
+        if (filter->nf == nf) {
+            break;
+        }
+    }
+
+    remove_filter(nc, filter);
+}
+
 NICState *qemu_new_nic(NetClientInfo *info,
                        NICConf *conf,
                        const char *model,
@@ -367,6 +400,8 @@ static void qemu_cleanup_net_client(NetClientState *nc)
 
 static void qemu_free_net_client(NetClientState *nc)
 {
+    Filter *filter, *next;
+
     if (nc->incoming_queue) {
         qemu_del_net_queue(nc->incoming_queue);
     }
@@ -375,6 +410,9 @@ static void qemu_free_net_client(NetClientState *nc)
     }
     g_free(nc->name);
     g_free(nc->model);
+    QTAILQ_FOREACH_SAFE(filter, &nc->filters, next, next) {
+        remove_filter(nc, filter);
+    }
     if (nc->destructor) {
         nc->destructor(nc);
     }
-- 
1.9.1

[Qemu-devel] [PATCH 05/12] netfilter: hook packets before receive

[Yang Hongyang]

Capture packets that will be sent.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 include/net/filter.h | 16 +++++++++++++
 net/net.c            | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/include/net/filter.h b/include/net/filter.h
index 1dd86cf..5292563 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -12,11 +12,27 @@
 #include "qemu/typedefs.h"
 
 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 (FilterReceive)(NetFilterState *, NetClientState *sender,
+                                unsigned flags, const uint8_t *, size_t);
+/*
+ * 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 *, NetClientState *sender,
+                                   unsigned flags, const struct iovec *, int);
 
 typedef struct NetFilterInfo {
     NetFilterOptionsKind type;
     size_t size;
     FilterCleanup *cleanup;
+    FilterReceive *receive;
+    FilterReceiveIOV *receive_iov;
 } NetFilterInfo;
 
 struct NetFilterState {
diff --git a/net/net.c b/net/net.c
index 22748e0..e4c822c 100644
--- a/net/net.c
+++ b/net/net.c
@@ -24,6 +24,7 @@
 #include "config-host.h"
 
 #include "net/net.h"
+#include "net/filter.h"
 #include "clients.h"
 #include "hub.h"
 #include "net/slirp.h"
@@ -592,6 +593,25 @@ int qemu_can_send_packet(NetClientState *sender)
     return 1;
 }
 
+static ssize_t filter_receive(NetClientState *nc, NetClientState *sender,
+                              unsigned flags,
+                              const uint8_t *data,
+                              size_t size) {
+    ssize_t ret = 0;
+    Filter *filter = NULL;
+    NetFilterState *nf = NULL;
+
+    QTAILQ_FOREACH(filter, &nc->filters, next) {
+        nf = filter->nf;
+        ret = nf->info->receive(nf, sender, flags, data, size);
+        if (ret == size) {
+            return ret;
+        }
+    }
+
+    return ret;
+}
+
 ssize_t qemu_deliver_packet(NetClientState *sender,
                             unsigned flags,
                             const uint8_t *data,
@@ -609,6 +629,17 @@ ssize_t qemu_deliver_packet(NetClientState *sender,
         return 0;
     }
 
+    /* Let filters handle the packet first */
+    ret = filter_receive(sender, sender, flags, data, size);
+    if (ret == size) {
+        return size;
+    }
+
+    ret = filter_receive(nc, sender, flags, data, size);
+    if (ret == size) {
+        return size;
+    }
+
     if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) {
         ret = nc->info->receive_raw(nc, data, size);
     } else {
@@ -697,6 +728,26 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size)
                                              buf, size, NULL);
 }
 
+static ssize_t filter_receive_iov(NetClientState *nc, NetClientState *sender,
+                                  unsigned flags,
+                                  const struct iovec *iov,
+                                  int iovcnt) {
+    ssize_t ret = 0;
+    Filter *filter = NULL;
+    NetFilterState *nf = NULL;
+    ssize_t size = iov_size(iov, iovcnt);
+
+    QTAILQ_FOREACH(filter, &nc->filters, next) {
+        nf = filter->nf;
+        ret = nf->info->receive_iov(nf, sender, flags, iov, iovcnt);
+        if (ret == size) {
+            return ret;
+        }
+    }
+
+    return ret;
+}
+
 static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov,
                                int iovcnt)
 {
@@ -716,15 +767,27 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
 {
     NetClientState *nc = opaque;
     int ret;
+    ssize_t size = iov_size(iov, iovcnt);
 
     if (nc->link_down) {
-        return iov_size(iov, iovcnt);
+        return size;
     }
 
     if (nc->receive_disabled) {
         return 0;
     }
 
+    /* Let filters handle the packet first */
+    ret = filter_receive_iov(sender, sender, flags, iov, iovcnt);
+    if (ret == size) {
+        return size;
+    }
+
+    ret = filter_receive_iov(nc, sender, flags, iov, iovcnt);
+    if (ret == size) {
+        return size;
+    }
+
     if (nc->info->receive_iov) {
         ret = nc->info->receive_iov(nc, iov, iovcnt);
     } else {
-- 
1.9.1

Re: [Qemu-devel] [PATCH 05/12] netfilter: hook packets before receive

[Jason Wang]

On 07/29/2015 06:51 PM, Yang Hongyang wrote:
> Capture packets that will be sent.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  include/net/filter.h | 16 +++++++++++++
>  net/net.c            | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 1dd86cf..5292563 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -12,11 +12,27 @@
>  #include "qemu/typedefs.h"
>  
>  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 (FilterReceive)(NetFilterState *, NetClientState *sender,
> +                                unsigned flags, const uint8_t *, size_t);

Looks like there's no need for this. Just add a wrapper and pretend a
single iov should be ok?

Re: [Qemu-devel] [PATCH 05/12] netfilter: hook packets before receive

[Yang Hongyang]

On 07/30/2015 12:51 PM, Jason Wang wrote:
>
>
> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>> Capture packets that will be sent.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> ---
>>   include/net/filter.h | 16 +++++++++++++
>>   net/net.c            | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 80 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index 1dd86cf..5292563 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -12,11 +12,27 @@
>>   #include "qemu/typedefs.h"
>>
>>   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 (FilterReceive)(NetFilterState *, NetClientState *sender,
>> +                                unsigned flags, const uint8_t *, size_t);
>
> Looks like there's no need for this. Just add a wrapper and pretend a
> single iov should be ok?

Yes, should be ok, will also make concrete filter implement simpler, only
implement receive_iov is enough.

>
> .
>

-- 
Thanks,
Yang.

[Qemu-devel] [PATCH 06/12] netfilter: provide a compat receive_iov

[Yang Hongyang]

if a filter do not provide a receive_iov, use a compat
receive_iov instead.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 net/filter.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/net/filter.c b/net/filter.c
index 3d3b7bc..50fb837 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -14,12 +14,27 @@
 #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"
 
 static QTAILQ_HEAD(, NetFilterState) net_filters;
 
+static ssize_t filter_receive_iov_compat(NetFilterState *nf,
+                                         NetClientState *sender,
+                                         unsigned flags,
+                                         const struct iovec *iov,
+                                         int iovcnt)
+{
+    uint8_t buffer[NET_BUFSIZE];
+    size_t offset;
+
+    offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer));
+
+    return nf->info->receive(nf, sender, flags, buffer, offset);
+}
+
 NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
                                     NetClientState *netdev,
                                     const char *model,
@@ -34,6 +49,9 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
     nf->model = g_strdup(model);
     nf->name = g_strdup(name);
     nf->netdev = netdev;
+    if (!nf->info->receive_iov) {
+        nf->info->receive_iov = filter_receive_iov_compat;
+    }
     QTAILQ_INSERT_TAIL(&net_filters, nf, next);
     qemu_netdev_add_filter(netdev, nf);
 
-- 
1.9.1

[Qemu-devel] [PATCH 07/12] net/queue: export qemu_net_queue_append

[Yang Hongyang]

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.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 fc02b33..fea6c51 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -36,6 +36,13 @@ typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
 
 NetQueue *qemu_new_net_queue(void *opaque);
 
+void qemu_net_queue_append(NetQueue *queue,
+                           NetClientState *sender,
+                           unsigned flags,
+                           const uint8_t *buf,
+                           size_t size,
+                           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 ebbe2bb..3d733dc 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -87,12 +87,12 @@ void qemu_del_net_queue(NetQueue *queue)
     g_free(queue);
 }
 
-static void qemu_net_queue_append(NetQueue *queue,
-                                  NetClientState *sender,
-                                  unsigned flags,
-                                  const uint8_t *buf,
-                                  size_t size,
-                                  NetPacketSent *sent_cb)
+void qemu_net_queue_append(NetQueue *queue,
+                           NetClientState *sender,
+                           unsigned flags,
+                           const uint8_t *buf,
+                           size_t size,
+                           NetPacketSent *sent_cb)
 {
     NetPacket *packet;
 
-- 
1.9.1

[Qemu-devel] [PATCH 08/12] move out net queue structs define

[Yang Hongyang]

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.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 fea6c51..ee92caf 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 3d733dc..2bab882 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

[Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

This filter is to buffer/release packets, this feature can be used
when using macrocheckpoing, 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.

Usage:
 -netdev tap,id=bn0
 -netfilter buffer,id=f0,netdev=bn0,interval=1000

NOTE:
 the scale of interval is microsecond.
 API to release packets and interval support will be added
 by the following 2 patches.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 net/Makefile.objs   |   1 +
 net/filter-buffer.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/filter.c        |   2 +
 net/filters.h       |  17 +++++++
 qapi-schema.json    |  13 +++++-
 5 files changed, 159 insertions(+), 1 deletion(-)
 create mode 100644 net/filter-buffer.c
 create mode 100644 net/filters.h

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..628e66f
--- /dev/null
+++ b/net/filter-buffer.c
@@ -0,0 +1,127 @@
+/*
+ * 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/error-report.h"
+
+typedef struct FILTERBUFFERState {
+    NetFilterState nf;
+    NetClientState dummy; /* used to send buffered packets */
+    NetQueue *incoming_queue;
+    NetQueue *inflight_queue;
+} FILTERBUFFERState;
+
+static void packet_send_completed(NetClientState *nc, ssize_t len)
+{
+    return;
+}
+
+static void filter_buffer_flush(NetFilterState *nf)
+{
+    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
+    NetQueue *queue = s->inflight_queue;
+    NetPacket *packet;
+
+    while (queue && !QTAILQ_EMPTY(&queue->packets)) {
+        packet = QTAILQ_FIRST(&queue->packets);
+        QTAILQ_REMOVE(&queue->packets, packet, entry);
+        queue->nq_count--;
+
+        if (packet->flags & QEMU_NET_PACKET_FLAG_RAW) {
+            qemu_send_packet_raw(&s->dummy, packet->data, packet->size);
+        } else {
+            qemu_send_packet_async(&s->dummy, packet->data, packet->size,
+                                   packet->sent_cb);
+        }
+
+        /*
+         * now that we pass the packet to sender->peer->incoming_queue, we
+         * don't care the reture value here, because the peer's queue will
+         * take care of this packet
+         */
+        g_free(packet);
+    }
+
+    if (queue && QTAILQ_EMPTY(&queue->packets)) {
+        g_free(queue);
+        s->inflight_queue = NULL;
+    }
+}
+
+/* filter APIs */
+static ssize_t filter_buffer_receive(NetFilterState *nf,
+                                     NetClientState *sender,
+                                     unsigned flags,
+                                     const uint8_t *data,
+                                     size_t size)
+{
+    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
+    NetQueue *queue = s->incoming_queue;
+
+    if (!sender->info) {
+        /* This must be a dummy NetClientState, do nothing */
+        return 0;
+    }
+
+    if (sender->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
+        /* we only buffer guest output packets */
+        qemu_net_queue_append(queue, sender, flags, data, size,
+                              packet_send_completed);
+        /* Now that we have buffered the packet, return sucess */
+        return size;
+    }
+
+    return 0;
+}
+
+static void filter_buffer_cleanup(NetFilterState *nf)
+{
+    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
+
+    /* flush inflight packets */
+    filter_buffer_flush(nf);
+    /* flush incoming packets */
+    s->inflight_queue = s->incoming_queue;
+    s->incoming_queue = NULL;
+    filter_buffer_flush(nf);
+
+    return;
+}
+
+
+static NetFilterInfo net_filter_buffer_info = {
+    .type = NET_FILTER_OPTIONS_KIND_BUFFER,
+    .size = sizeof(FILTERBUFFERState),
+    .receive = filter_buffer_receive,
+    .cleanup = filter_buffer_cleanup,
+};
+
+int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
+                           NetClientState *netdev, Error **errp)
+{
+    NetFilterState *nf;
+    FILTERBUFFERState *s;
+
+    assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
+
+    nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, "buffer", name);
+    s = DO_UPCAST(FILTERBUFFERState, nf, nf);
+    /*
+     * we need the dummy NetClientState to send packets in order to avoid
+     * receive packets again.
+     * we are buffering guest output packets, our buffered packets should be
+     * sent to real network backend, so dummy's peer should be that backend.
+     */
+    s->dummy.peer = netdev;
+    s->incoming_queue = qemu_new_net_queue(nf);
+
+    return 0;
+}
diff --git a/net/filter.c b/net/filter.c
index 50fb837..e741e2a 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -18,6 +18,7 @@
 
 #include "net/filter.h"
 #include "net/net.h"
+#include "filters.h"
 
 static QTAILQ_HEAD(, NetFilterState) net_filters;
 
@@ -152,6 +153,7 @@ typedef int (NetFilterInit)(const NetFilterOptions *opts,
 
 static
 NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
+    [NET_FILTER_OPTIONS_KIND_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
new file mode 100644
index 0000000..6c249b8
--- /dev/null
+++ b/net/filters.h
@@ -0,0 +1,17 @@
+/*
+ * 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_buffer(const NetFilterOptions *opts, const char *name,
+                           NetClientState *netdev, Error **errp);
+
+#endif /* QEMU_NET_FILTERS_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index 1fc6390..67e00a0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2577,6 +2577,16 @@
 { 'command': 'netfilter_del', 'data': {'id': 'str'} }
 
 ##
+# @NetFilterBufferOptions
+#
+# a netbuffer filter for network backend.
+#
+# Since 2.5
+##
+{ 'struct': 'NetFilterBufferOptions',
+  'data': { } }
+
+##
 # @NetFilterOptions
 #
 # A discriminated record of network filters.
@@ -2585,7 +2595,8 @@
 #
 ##
 { 'union': 'NetFilterOptions',
-  'data': { } }
+  'data': {
+    'buffer':     'NetFilterBufferOptions'} }
 
 ##
 # @NetFilter
-- 
1.9.1

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Li Zhijian]

On 07/29/2015 06:51 PM, Yang Hongyang wrote:
> This filter is to buffer/release packets, this feature can be used
> when using macrocheckpoing, or other Remus like VM FT solutions, you

s/macrocheckpoing/MicroCheckpointing/

the cover letter have the same typo


-- 
Best regards.
Li Zhijian

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 09:45 AM, Li Zhijian wrote:
> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>> This filter is to buffer/release packets, this feature can be used
>> when using macrocheckpoing, or other Remus like VM FT solutions, you
>
> s/macrocheckpoing/MicroCheckpointing/
>
> the cover letter have the same typo

Fixed, thanks.

>
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Jason Wang]

On 07/29/2015 06:51 PM, Yang Hongyang wrote:
> This filter is to buffer/release packets, this feature can be used
> when using macrocheckpoing, 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.
>
> Usage:
>  -netdev tap,id=bn0
>  -netfilter buffer,id=f0,netdev=bn0,interval=1000
>
> NOTE:
>  the scale of interval is microsecond.
>  API to release packets and interval support will be added
>  by the following 2 patches.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  net/Makefile.objs   |   1 +
>  net/filter-buffer.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/filter.c        |   2 +
>  net/filters.h       |  17 +++++++
>  qapi-schema.json    |  13 +++++-
>  5 files changed, 159 insertions(+), 1 deletion(-)
>  create mode 100644 net/filter-buffer.c
>  create mode 100644 net/filters.h
>
> 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..628e66f
> --- /dev/null
> +++ b/net/filter-buffer.c
> @@ -0,0 +1,127 @@
> +/*
> + * 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/error-report.h"
> +
> +typedef struct FILTERBUFFERState {
> +    NetFilterState nf;
> +    NetClientState dummy; /* used to send buffered packets */

Why need this? Couldn't we just infer this from NetFilterState?

> +    NetQueue *incoming_queue;
> +    NetQueue *inflight_queue;
> +} FILTERBUFFERState;
> +
> +static void packet_send_completed(NetClientState *nc, ssize_t len)
> +{
> +    return;
> +}
> +
> +static void filter_buffer_flush(NetFilterState *nf)
> +{
> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
> +    NetQueue *queue = s->inflight_queue;
> +    NetPacket *packet;
> +
> +    while (queue && !QTAILQ_EMPTY(&queue->packets)) {
> +        packet = QTAILQ_FIRST(&queue->packets);
> +        QTAILQ_REMOVE(&queue->packets, packet, entry);
> +        queue->nq_count--;
> +
> +        if (packet->flags & QEMU_NET_PACKET_FLAG_RAW) {
> +            qemu_send_packet_raw(&s->dummy, packet->data, packet->size);
> +        } else {
> +            qemu_send_packet_async(&s->dummy, packet->data, packet->size,
> +                                   packet->sent_cb);
> +        }
> +
> +        /*
> +         * now that we pass the packet to sender->peer->incoming_queue, we
> +         * don't care the reture value here, because the peer's queue will
> +         * take care of this packet
> +         */
> +        g_free(packet);
> +    }
> +
> +    if (queue && QTAILQ_EMPTY(&queue->packets)) {

Under what condition queue->packet could be not empty here? And I don't
get the point that why a inflight_queue is needed.

> +        g_free(queue);
> +        s->inflight_queue = NULL;
> +    }
> +}
> +
> +/* filter APIs */
> +static ssize_t filter_buffer_receive(NetFilterState *nf,
> +                                     NetClientState *sender,
> +                                     unsigned flags,
> +                                     const uint8_t *data,
> +                                     size_t size)
> +{
> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
> +    NetQueue *queue = s->incoming_queue;
> +
> +    if (!sender->info) {
> +        /* This must be a dummy NetClientState, do nothing */
> +        return 0;
> +    }
> +
> +    if (sender->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> +        /* we only buffer guest output packets */
> +        qemu_net_queue_append(queue, sender, flags, data, size,
> +                              packet_send_completed);

This may brings some confusion for user. Since the name is 'netbuffer'.
Maybe we can change the filter to be ingress or out or both? Like:

-device virtio-net-pci,id=virtio0
-netfilter buffer,id=filter0,dev=virtio0,interval=1000,type=out

Then we can just try to enqueue the packet when virtio-net-pci is sender?

> +        /* Now that we have buffered the packet, return sucess */
> +        return size;
> +    }
> +
> +    return 0;
> +}
> +
> +static void filter_buffer_cleanup(NetFilterState *nf)
> +{
> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
> +
> +    /* flush inflight packets */
> +    filter_buffer_flush(nf);
> +    /* flush incoming packets */
> +    s->inflight_queue = s->incoming_queue;
> +    s->incoming_queue = NULL;
> +    filter_buffer_flush(nf);
> +
> +    return;
> +}
> +
> +
> +static NetFilterInfo net_filter_buffer_info = {
> +    .type = NET_FILTER_OPTIONS_KIND_BUFFER,
> +    .size = sizeof(FILTERBUFFERState),
> +    .receive = filter_buffer_receive,
> +    .cleanup = filter_buffer_cleanup,
> +};
> +
> +int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
> +                           NetClientState *netdev, Error **errp)
> +{
> +    NetFilterState *nf;
> +    FILTERBUFFERState *s;
> +
> +    assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
> +
> +    nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, "buffer", name);
> +    s = DO_UPCAST(FILTERBUFFERState, nf, nf);
> +    /*
> +     * we need the dummy NetClientState to send packets in order to avoid
> +     * receive packets again.
> +     * we are buffering guest output packets, our buffered packets should be
> +     * sent to real network backend, so dummy's peer should be that backend.
> +     */
> +    s->dummy.peer = netdev;
> +    s->incoming_queue = qemu_new_net_queue(nf);
> +
> +    return 0;
> +}
> diff --git a/net/filter.c b/net/filter.c
> index 50fb837..e741e2a 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -18,6 +18,7 @@
>  
>  #include "net/filter.h"
>  #include "net/net.h"
> +#include "filters.h"
>  
>  static QTAILQ_HEAD(, NetFilterState) net_filters;
>  
> @@ -152,6 +153,7 @@ typedef int (NetFilterInit)(const NetFilterOptions *opts,
>  
>  static
>  NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
> +    [NET_FILTER_OPTIONS_KIND_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
> new file mode 100644
> index 0000000..6c249b8
> --- /dev/null
> +++ b/net/filters.h
> @@ -0,0 +1,17 @@
> +/*
> + * 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_buffer(const NetFilterOptions *opts, const char *name,
> +                           NetClientState *netdev, Error **errp);
> +
> +#endif /* QEMU_NET_FILTERS_H */
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 1fc6390..67e00a0 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2577,6 +2577,16 @@
>  { 'command': 'netfilter_del', 'data': {'id': 'str'} }
>  
>  ##
> +# @NetFilterBufferOptions
> +#
> +# a netbuffer filter for network backend.
> +#
> +# Since 2.5
> +##
> +{ 'struct': 'NetFilterBufferOptions',
> +  'data': { } }
> +
> +##
>  # @NetFilterOptions
>  #
>  # A discriminated record of network filters.
> @@ -2585,7 +2595,8 @@
>  #
>  ##
>  { 'union': 'NetFilterOptions',
> -  'data': { } }
> +  'data': {
> +    'buffer':     'NetFilterBufferOptions'} }
>  
>  ##
>  # @NetFilter

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 01:13 PM, Jason Wang wrote:
[...]
>> +
>> +#include "net/filter.h"
>> +#include "net/queue.h"
>> +#include "filters.h"
>> +#include "qemu-common.h"
>> +#include "qemu/error-report.h"
>> +
>> +typedef struct FILTERBUFFERState {
>> +    NetFilterState nf;
>> +    NetClientState dummy; /* used to send buffered packets */
>
> Why need this? Couldn't we just infer this from NetFilterState?

Because we use existing API qemu_send_packet_async/raw to send
packet, it takes an NetClientState as the first argument sender,
and use sender->peer->incoming_queue as the dest queue, so in order to
make this API work, we need to use this dummy NC and init it's
peer to our dest(which is the network backend)
Another way is to call qemu_net_queue_send(netdev->incoming_queue,...)
directly, we still need a NetClientState *sender param, can not
use NetFilterState.
This dummy NC also been checked in filter_buffer_receive to avoid buffering
packet been sent by ourself.

>
>> +    NetQueue *incoming_queue;
>> +    NetQueue *inflight_queue;
>> +} FILTERBUFFERState;
>> +
>> +static void packet_send_completed(NetClientState *nc, ssize_t len)
>> +{
>> +    return;
>> +}
>> +
>> +static void filter_buffer_flush(NetFilterState *nf)
>> +{
>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>> +    NetQueue *queue = s->inflight_queue;
>> +    NetPacket *packet;
>> +
>> +    while (queue && !QTAILQ_EMPTY(&queue->packets)) {
>> +        packet = QTAILQ_FIRST(&queue->packets);
>> +        QTAILQ_REMOVE(&queue->packets, packet, entry);
>> +        queue->nq_count--;
>> +
>> +        if (packet->flags & QEMU_NET_PACKET_FLAG_RAW) {
>> +            qemu_send_packet_raw(&s->dummy, packet->data, packet->size);
>> +        } else {
>> +            qemu_send_packet_async(&s->dummy, packet->data, packet->size,
>> +                                   packet->sent_cb);
>> +        }
>> +
>> +        /*
>> +         * now that we pass the packet to sender->peer->incoming_queue, we
>> +         * don't care the reture value here, because the peer's queue will
>> +         * take care of this packet
>> +         */
>> +        g_free(packet);
>> +    }
>> +
>> +    if (queue && QTAILQ_EMPTY(&queue->packets)) {
>
> Under what condition queue->packet could be not empty here?

You're right, this check is nonsense. can be dropped.

> And I don't
> get the point that why a inflight_queue is needed.

maybe squash the interval patch here will make it clear?

>
>> +        g_free(queue);
>> +        s->inflight_queue = NULL;
>> +    }
>> +}
>> +
>> +/* filter APIs */
>> +static ssize_t filter_buffer_receive(NetFilterState *nf,
>> +                                     NetClientState *sender,
>> +                                     unsigned flags,
>> +                                     const uint8_t *data,
>> +                                     size_t size)
>> +{
>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>> +    NetQueue *queue = s->incoming_queue;
>> +
>> +    if (!sender->info) {
>> +        /* This must be a dummy NetClientState, do nothing */
>> +        return 0;
>> +    }
>> +
>> +    if (sender->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
>> +        /* we only buffer guest output packets */
>> +        qemu_net_queue_append(queue, sender, flags, data, size,
>> +                              packet_send_completed);
>
> This may brings some confusion for user. Since the name is 'netbuffer'.
> Maybe we can change the filter to be ingress or out or both? Like:
>
> -device virtio-net-pci,id=virtio0
> -netfilter buffer,id=filter0,dev=virtio0,interval=1000,type=out
>
> Then we can just try to enqueue the packet when virtio-net-pci is sender?
>
>> +        /* Now that we have buffered the packet, return sucess */
>> +        return size;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void filter_buffer_cleanup(NetFilterState *nf)
>> +{
>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>> +
>> +    /* flush inflight packets */
>> +    filter_buffer_flush(nf);
>> +    /* flush incoming packets */
>> +    s->inflight_queue = s->incoming_queue;
>> +    s->incoming_queue = NULL;
>> +    filter_buffer_flush(nf);
>> +
>> +    return;
>> +}
>> +
>> +
>> +static NetFilterInfo net_filter_buffer_info = {
>> +    .type = NET_FILTER_OPTIONS_KIND_BUFFER,
>> +    .size = sizeof(FILTERBUFFERState),
>> +    .receive = filter_buffer_receive,
>> +    .cleanup = filter_buffer_cleanup,
>> +};
>> +
>> +int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
>> +                           NetClientState *netdev, Error **errp)
>> +{
>> +    NetFilterState *nf;
>> +    FILTERBUFFERState *s;
>> +
>> +    assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
>> +
>> +    nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, "buffer", name);
>> +    s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>> +    /*
>> +     * we need the dummy NetClientState to send packets in order to avoid
>> +     * receive packets again.
>> +     * we are buffering guest output packets, our buffered packets should be
>> +     * sent to real network backend, so dummy's peer should be that backend.
>> +     */
>> +    s->dummy.peer = netdev;
>> +    s->incoming_queue = qemu_new_net_queue(nf);
>> +
>> +    return 0;
>> +}
>> diff --git a/net/filter.c b/net/filter.c
>> index 50fb837..e741e2a 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -18,6 +18,7 @@
>>
>>   #include "net/filter.h"
>>   #include "net/net.h"
>> +#include "filters.h"
>>
>>   static QTAILQ_HEAD(, NetFilterState) net_filters;
>>
>> @@ -152,6 +153,7 @@ typedef int (NetFilterInit)(const NetFilterOptions *opts,
>>
>>   static
>>   NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
>> +    [NET_FILTER_OPTIONS_KIND_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
>> new file mode 100644
>> index 0000000..6c249b8
>> --- /dev/null
>> +++ b/net/filters.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * 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_buffer(const NetFilterOptions *opts, const char *name,
>> +                           NetClientState *netdev, Error **errp);
>> +
>> +#endif /* QEMU_NET_FILTERS_H */
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 1fc6390..67e00a0 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2577,6 +2577,16 @@
>>   { 'command': 'netfilter_del', 'data': {'id': 'str'} }
>>
>>   ##
>> +# @NetFilterBufferOptions
>> +#
>> +# a netbuffer filter for network backend.
>> +#
>> +# Since 2.5
>> +##
>> +{ 'struct': 'NetFilterBufferOptions',
>> +  'data': { } }
>> +
>> +##
>>   # @NetFilterOptions
>>   #
>>   # A discriminated record of network filters.
>> @@ -2585,7 +2595,8 @@
>>   #
>>   ##
>>   { 'union': 'NetFilterOptions',
>> -  'data': { } }
>> +  'data': {
>> +    'buffer':     'NetFilterBufferOptions'} }
>>
>>   ##
>>   # @NetFilter
>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Jason Wang]

On 07/30/2015 02:47 PM, Yang Hongyang wrote:
> On 07/30/2015 01:13 PM, Jason Wang wrote:
> [...]
>>> +
>>> +#include "net/filter.h"
>>> +#include "net/queue.h"
>>> +#include "filters.h"
>>> +#include "qemu-common.h"
>>> +#include "qemu/error-report.h"
>>> +
>>> +typedef struct FILTERBUFFERState {
>>> +    NetFilterState nf;
>>> +    NetClientState dummy; /* used to send buffered packets */
>>
>> Why need this? Couldn't we just infer this from NetFilterState?
>
> Because we use existing API qemu_send_packet_async/raw to send
> packet, it takes an NetClientState as the first argument sender,
> and use sender->peer->incoming_queue as the dest queue, so in order to
> make this API work, we need to use this dummy NC and init it's
> peer to our dest(which is the network backend)
> Another way is to call qemu_net_queue_send(netdev->incoming_queue,...)
> directly, we still need a NetClientState *sender param, can not
> use NetFilterState.

I think this is my meaning. Use NetFilterState->netdev.

> This dummy NC also been checked in filter_buffer_receive to avoid
> buffering
> packet been sent by ourself.
>

I don't get why this is needed. Who is going to queue a packet in dummy
NC, consider it was not peered by any others?
>>
>>> +    NetQueue *incoming_queue;
>>> +    NetQueue *inflight_queue;
>>> +} FILTERBUFFERState;
>>> +
>>> +static void packet_send_completed(NetClientState *nc, ssize_t len)
>>> +{
>>> +    return;
>>> +}
>>> +
>>> +static void filter_buffer_flush(NetFilterState *nf)
>>> +{
>>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>> +    NetQueue *queue = s->inflight_queue;
>>> +    NetPacket *packet;
>>> +
>>> +    while (queue && !QTAILQ_EMPTY(&queue->packets)) {
>>> +        packet = QTAILQ_FIRST(&queue->packets);
>>> +        QTAILQ_REMOVE(&queue->packets, packet, entry);
>>> +        queue->nq_count--;
>>> +
>>> +        if (packet->flags & QEMU_NET_PACKET_FLAG_RAW) {
>>> +            qemu_send_packet_raw(&s->dummy, packet->data,
>>> packet->size);
>>> +        } else {
>>> +            qemu_send_packet_async(&s->dummy, packet->data,
>>> packet->size,
>>> +                                   packet->sent_cb);
>>> +        }
>>> +
>>> +        /*
>>> +         * now that we pass the packet to
>>> sender->peer->incoming_queue, we
>>> +         * don't care the reture value here, because the peer's
>>> queue will
>>> +         * take care of this packet
>>> +         */
>>> +        g_free(packet);
>>> +    }
>>> +
>>> +    if (queue && QTAILQ_EMPTY(&queue->packets)) {
>>
>> Under what condition queue->packet could be not empty here?
>
> You're right, this check is nonsense. can be dropped.
>
>> And I don't
>> get the point that why a inflight_queue is needed.
>
> maybe squash the interval patch here will make it clear?
>

Yes, maybe.

>>
>>> +        g_free(queue);
>>> +        s->inflight_queue = NULL;
>>> +    }
>>> +}
>>> +
>>> +/* filter APIs */
>>> +static ssize_t filter_buffer_receive(NetFilterState *nf,
>>> +                                     NetClientState *sender,
>>> +                                     unsigned flags,
>>> +                                     const uint8_t *data,
>>> +                                     size_t size)
>>> +{
>>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>> +    NetQueue *queue = s->incoming_queue;
>>> +
>>> +    if (!sender->info) {
>>> +        /* This must be a dummy NetClientState, do nothing */
>>> +        return 0;
>>> +    }
>>> +
>>> +    if (sender->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
>>> +        /* we only buffer guest output packets */
>>> +        qemu_net_queue_append(queue, sender, flags, data, size,
>>> +                              packet_send_completed);
>>
>> This may brings some confusion for user. Since the name is 'netbuffer'.
>> Maybe we can change the filter to be ingress or out or both? Like:
>>
>> -device virtio-net-pci,id=virtio0
>> -netfilter buffer,id=filter0,dev=virtio0,interval=1000,type=out
>>
>> Then we can just try to enqueue the packet when virtio-net-pci is
>> sender?
>>
>>> +        /* Now that we have buffered the packet, return sucess */
>>> +        return size;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void filter_buffer_cleanup(NetFilterState *nf)
>>> +{
>>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>> +
>>> +    /* flush inflight packets */
>>> +    filter_buffer_flush(nf);
>>> +    /* flush incoming packets */
>>> +    s->inflight_queue = s->incoming_queue;
>>> +    s->incoming_queue = NULL;
>>> +    filter_buffer_flush(nf);
>>> +
>>> +    return;
>>> +}
>>> +
>>> +
>>> +static NetFilterInfo net_filter_buffer_info = {
>>> +    .type = NET_FILTER_OPTIONS_KIND_BUFFER,
>>> +    .size = sizeof(FILTERBUFFERState),
>>> +    .receive = filter_buffer_receive,
>>> +    .cleanup = filter_buffer_cleanup,
>>> +};
>>> +
>>> +int net_init_filter_buffer(const NetFilterOptions *opts, const char
>>> *name,
>>> +                           NetClientState *netdev, Error **errp)
>>> +{
>>> +    NetFilterState *nf;
>>> +    FILTERBUFFERState *s;
>>> +
>>> +    assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
>>> +
>>> +    nf = qemu_new_net_filter(&net_filter_buffer_info, netdev,
>>> "buffer", name);
>>> +    s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>> +    /*
>>> +     * we need the dummy NetClientState to send packets in order to
>>> avoid
>>> +     * receive packets again.
>>> +     * we are buffering guest output packets, our buffered packets
>>> should be
>>> +     * sent to real network backend, so dummy's peer should be that
>>> backend.
>>> +     */
>>> +    s->dummy.peer = netdev;
>>> +    s->incoming_queue = qemu_new_net_queue(nf);
>>> +
>>> +    return 0;
>>> +}
>>> diff --git a/net/filter.c b/net/filter.c
>>> index 50fb837..e741e2a 100644
>>> --- a/net/filter.c
>>> +++ b/net/filter.c
>>> @@ -18,6 +18,7 @@
>>>
>>>   #include "net/filter.h"
>>>   #include "net/net.h"
>>> +#include "filters.h"
>>>
>>>   static QTAILQ_HEAD(, NetFilterState) net_filters;
>>>
>>> @@ -152,6 +153,7 @@ typedef int (NetFilterInit)(const
>>> NetFilterOptions *opts,
>>>
>>>   static
>>>   NetFilterInit * const
>>> net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
>>> +    [NET_FILTER_OPTIONS_KIND_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
>>> new file mode 100644
>>> index 0000000..6c249b8
>>> --- /dev/null
>>> +++ b/net/filters.h
>>> @@ -0,0 +1,17 @@
>>> +/*
>>> + * 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_buffer(const NetFilterOptions *opts, const char
>>> *name,
>>> +                           NetClientState *netdev, Error **errp);
>>> +
>>> +#endif /* QEMU_NET_FILTERS_H */
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index 1fc6390..67e00a0 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -2577,6 +2577,16 @@
>>>   { 'command': 'netfilter_del', 'data': {'id': 'str'} }
>>>
>>>   ##
>>> +# @NetFilterBufferOptions
>>> +#
>>> +# a netbuffer filter for network backend.
>>> +#
>>> +# Since 2.5
>>> +##
>>> +{ 'struct': 'NetFilterBufferOptions',
>>> +  'data': { } }
>>> +
>>> +##
>>>   # @NetFilterOptions
>>>   #
>>>   # A discriminated record of network filters.
>>> @@ -2585,7 +2595,8 @@
>>>   #
>>>   ##
>>>   { 'union': 'NetFilterOptions',
>>> -  'data': { } }
>>> +  'data': {
>>> +    'buffer':     'NetFilterBufferOptions'} }
>>>
>>>   ##
>>>   # @NetFilter
>>
>> .
>>
>

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 04:40 PM, Jason Wang wrote:
>
>
> On 07/30/2015 02:47 PM, Yang Hongyang wrote:
>> On 07/30/2015 01:13 PM, Jason Wang wrote:
>> [...]
>>>> +
>>>> +#include "net/filter.h"
>>>> +#include "net/queue.h"
>>>> +#include "filters.h"
>>>> +#include "qemu-common.h"
>>>> +#include "qemu/error-report.h"
>>>> +
>>>> +typedef struct FILTERBUFFERState {
>>>> +    NetFilterState nf;
>>>> +    NetClientState dummy; /* used to send buffered packets */
>>>
>>> Why need this? Couldn't we just infer this from NetFilterState?
>>
>> Because we use existing API qemu_send_packet_async/raw to send
>> packet, it takes an NetClientState as the first argument sender,
>> and use sender->peer->incoming_queue as the dest queue, so in order to
>> make this API work, we need to use this dummy NC and init it's
>> peer to our dest(which is the network backend)
>> Another way is to call qemu_net_queue_send(netdev->incoming_queue,...)
>> directly, we still need a NetClientState *sender param, can not
>> use NetFilterState.
>
> I think this is my meaning. Use NetFilterState->netdev.

Problem is NetFilterState->netdev is our destination, we need a sender...
if we use this, packet will be sent back to NIC...

>
>> This dummy NC also been checked in filter_buffer_receive to avoid
>> buffering
>> packet been sent by ourself.
>>
>
> I don't get why this is needed. Who is going to queue a packet in dummy
> NC, consider it was not peered by any others?

There's nothing in the dummy NC except the dummy->peer = NetFilterState->netdev
This dummy NC only used to as a sender param of the existing APIs which send
packets. When a buffered packet been sent, we shouldn't buffer it again, we
cann't use any existing NC (packet->sender or NetFilterState->netdev) as the 
sender because otherwise we can't distinguish if the packet is a buffered
packet sent by ourself.

>>>
>>>> +    NetQueue *incoming_queue;
>>>> +    NetQueue *inflight_queue;
>>>> +} FILTERBUFFERState;
>>>> +
>>>> +static void packet_send_completed(NetClientState *nc, ssize_t len)
>>>> +{
>>>> +    return;
>>>> +}
>>>> +
>>>> +static void filter_buffer_flush(NetFilterState *nf)
>>>> +{
>>>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>>> +    NetQueue *queue = s->inflight_queue;
>>>> +    NetPacket *packet;
>>>> +
>>>> +    while (queue && !QTAILQ_EMPTY(&queue->packets)) {
>>>> +        packet = QTAILQ_FIRST(&queue->packets);
>>>> +        QTAILQ_REMOVE(&queue->packets, packet, entry);
>>>> +        queue->nq_count--;
>>>> +
>>>> +        if (packet->flags & QEMU_NET_PACKET_FLAG_RAW) {
>>>> +            qemu_send_packet_raw(&s->dummy, packet->data,
>>>> packet->size);
>>>> +        } else {
>>>> +            qemu_send_packet_async(&s->dummy, packet->data,
>>>> packet->size,
>>>> +                                   packet->sent_cb);
>>>> +        }
>>>> +
>>>> +        /*
>>>> +         * now that we pass the packet to
>>>> sender->peer->incoming_queue, we
>>>> +         * don't care the reture value here, because the peer's
>>>> queue will
>>>> +         * take care of this packet
>>>> +         */
>>>> +        g_free(packet);
>>>> +    }
>>>> +
>>>> +    if (queue && QTAILQ_EMPTY(&queue->packets)) {
>>>
>>> Under what condition queue->packet could be not empty here?
>>
>> You're right, this check is nonsense. can be dropped.
>>
>>> And I don't
>>> get the point that why a inflight_queue is needed.
>>
>> maybe squash the interval patch here will make it clear?
>>
>
> Yes, maybe.
>
>>>
>>>> +        g_free(queue);
>>>> +        s->inflight_queue = NULL;
>>>> +    }
>>>> +}
>>>> +
>>>> +/* filter APIs */
>>>> +static ssize_t filter_buffer_receive(NetFilterState *nf,
>>>> +                                     NetClientState *sender,
>>>> +                                     unsigned flags,
>>>> +                                     const uint8_t *data,
>>>> +                                     size_t size)
>>>> +{
>>>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>>> +    NetQueue *queue = s->incoming_queue;
>>>> +
>>>> +    if (!sender->info) {
>>>> +        /* This must be a dummy NetClientState, do nothing */
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    if (sender->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
>>>> +        /* we only buffer guest output packets */
>>>> +        qemu_net_queue_append(queue, sender, flags, data, size,
>>>> +                              packet_send_completed);
>>>
>>> This may brings some confusion for user. Since the name is 'netbuffer'.
>>> Maybe we can change the filter to be ingress or out or both? Like:
>>>
>>> -device virtio-net-pci,id=virtio0
>>> -netfilter buffer,id=filter0,dev=virtio0,interval=1000,type=out
>>>
>>> Then we can just try to enqueue the packet when virtio-net-pci is
>>> sender?
>>>
>>>> +        /* Now that we have buffered the packet, return sucess */
>>>> +        return size;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void filter_buffer_cleanup(NetFilterState *nf)
>>>> +{
>>>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>>> +
>>>> +    /* flush inflight packets */
>>>> +    filter_buffer_flush(nf);
>>>> +    /* flush incoming packets */
>>>> +    s->inflight_queue = s->incoming_queue;
>>>> +    s->incoming_queue = NULL;
>>>> +    filter_buffer_flush(nf);
>>>> +
>>>> +    return;
>>>> +}
>>>> +
>>>> +
>>>> +static NetFilterInfo net_filter_buffer_info = {
>>>> +    .type = NET_FILTER_OPTIONS_KIND_BUFFER,
>>>> +    .size = sizeof(FILTERBUFFERState),
>>>> +    .receive = filter_buffer_receive,
>>>> +    .cleanup = filter_buffer_cleanup,
>>>> +};
>>>> +
>>>> +int net_init_filter_buffer(const NetFilterOptions *opts, const char
>>>> *name,
>>>> +                           NetClientState *netdev, Error **errp)
>>>> +{
>>>> +    NetFilterState *nf;
>>>> +    FILTERBUFFERState *s;
>>>> +
>>>> +    assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
>>>> +
>>>> +    nf = qemu_new_net_filter(&net_filter_buffer_info, netdev,
>>>> "buffer", name);
>>>> +    s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>>> +    /*
>>>> +     * we need the dummy NetClientState to send packets in order to
>>>> avoid
>>>> +     * receive packets again.
>>>> +     * we are buffering guest output packets, our buffered packets
>>>> should be
>>>> +     * sent to real network backend, so dummy's peer should be that
>>>> backend.
>>>> +     */
>>>> +    s->dummy.peer = netdev;
>>>> +    s->incoming_queue = qemu_new_net_queue(nf);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> diff --git a/net/filter.c b/net/filter.c
>>>> index 50fb837..e741e2a 100644
>>>> --- a/net/filter.c
>>>> +++ b/net/filter.c
>>>> @@ -18,6 +18,7 @@
>>>>
>>>>    #include "net/filter.h"
>>>>    #include "net/net.h"
>>>> +#include "filters.h"
>>>>
>>>>    static QTAILQ_HEAD(, NetFilterState) net_filters;
>>>>
>>>> @@ -152,6 +153,7 @@ typedef int (NetFilterInit)(const
>>>> NetFilterOptions *opts,
>>>>
>>>>    static
>>>>    NetFilterInit * const
>>>> net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
>>>> +    [NET_FILTER_OPTIONS_KIND_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
>>>> new file mode 100644
>>>> index 0000000..6c249b8
>>>> --- /dev/null
>>>> +++ b/net/filters.h
>>>> @@ -0,0 +1,17 @@
>>>> +/*
>>>> + * 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_buffer(const NetFilterOptions *opts, const char
>>>> *name,
>>>> +                           NetClientState *netdev, Error **errp);
>>>> +
>>>> +#endif /* QEMU_NET_FILTERS_H */
>>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>>> index 1fc6390..67e00a0 100644
>>>> --- a/qapi-schema.json
>>>> +++ b/qapi-schema.json
>>>> @@ -2577,6 +2577,16 @@
>>>>    { 'command': 'netfilter_del', 'data': {'id': 'str'} }
>>>>
>>>>    ##
>>>> +# @NetFilterBufferOptions
>>>> +#
>>>> +# a netbuffer filter for network backend.
>>>> +#
>>>> +# Since 2.5
>>>> +##
>>>> +{ 'struct': 'NetFilterBufferOptions',
>>>> +  'data': { } }
>>>> +
>>>> +##
>>>>    # @NetFilterOptions
>>>>    #
>>>>    # A discriminated record of network filters.
>>>> @@ -2585,7 +2595,8 @@
>>>>    #
>>>>    ##
>>>>    { 'union': 'NetFilterOptions',
>>>> -  'data': { } }
>>>> +  'data': {
>>>> +    'buffer':     'NetFilterBufferOptions'} }
>>>>
>>>>    ##
>>>>    # @NetFilter
>>>
>>> .
>>>
>>
>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Jason Wang]

On 07/30/2015 05:04 PM, Yang Hongyang wrote:
>
>
> On 07/30/2015 04:40 PM, Jason Wang wrote:
>>
>>
>> On 07/30/2015 02:47 PM, Yang Hongyang wrote:
>>> On 07/30/2015 01:13 PM, Jason Wang wrote:
>>> [...]
>>>>> +
>>>>> +#include "net/filter.h"
>>>>> +#include "net/queue.h"
>>>>> +#include "filters.h"
>>>>> +#include "qemu-common.h"
>>>>> +#include "qemu/error-report.h"
>>>>> +
>>>>> +typedef struct FILTERBUFFERState {
>>>>> +    NetFilterState nf;
>>>>> +    NetClientState dummy; /* used to send buffered packets */
>>>>
>>>> Why need this? Couldn't we just infer this from NetFilterState?
>>>
>>> Because we use existing API qemu_send_packet_async/raw to send
>>> packet, it takes an NetClientState as the first argument sender,
>>> and use sender->peer->incoming_queue as the dest queue, so in order to
>>> make this API work, we need to use this dummy NC and init it's
>>> peer to our dest(which is the network backend)
>>> Another way is to call qemu_net_queue_send(netdev->incoming_queue,...)
>>> directly, we still need a NetClientState *sender param, can not
>>> use NetFilterState.
>>
>> I think this is my meaning. Use NetFilterState->netdev.
>
> Problem is NetFilterState->netdev is our destination, we need a sender...
> if we use this, packet will be sent back to NIC...
>

I see, then NetFilterState->netdev->peer is sender. But I think it's
better to track sender instead of destination in this case. Something
like dummy NC is not elegant.

>>
>>> This dummy NC also been checked in filter_buffer_receive to avoid
>>> buffering
>>> packet been sent by ourself.
>>>
>>
>> I don't get why this is needed. Who is going to queue a packet in dummy
>> NC, consider it was not peered by any others?
>
> There's nothing in the dummy NC except the dummy->peer =
> NetFilterState->netdev
> This dummy NC only used to as a sender param of the existing APIs
> which send
> packets. When a buffered packet been sent, we shouldn't buffer it
> again, we
> cann't use any existing NC (packet->sender or NetFilterState->netdev)
> as the sender because otherwise we can't distinguish if the packet is
> a buffered
> packet sent by ourself. 

I see, so the reason is you are using qemu_deliver_packet() for both
enqueuing packet to filter and delivering packet to destination. How
about something like:

E.g for qemu_send_packet_async(), move the hook before
qemu_send_packet_async_with_flags(). Then flush method can call
qemu_send_packet_async_with_flags() without any issue?

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 05:33 PM, Jason Wang wrote:
> On 07/30/2015 05:04 PM, Yang Hongyang wrote:
>>
>>
>> On 07/30/2015 04:40 PM, Jason Wang wrote:
>>>
>>>
>>> On 07/30/2015 02:47 PM, Yang Hongyang wrote:
>>>> On 07/30/2015 01:13 PM, Jason Wang wrote:
>>>> [...]
>>>>>> +
>>>>>> +#include "net/filter.h"
>>>>>> +#include "net/queue.h"
>>>>>> +#include "filters.h"
>>>>>> +#include "qemu-common.h"
>>>>>> +#include "qemu/error-report.h"
>>>>>> +
>>>>>> +typedef struct FILTERBUFFERState {
>>>>>> +    NetFilterState nf;
>>>>>> +    NetClientState dummy; /* used to send buffered packets */
>>>>>
>>>>> Why need this? Couldn't we just infer this from NetFilterState?
>>>>
>>>> Because we use existing API qemu_send_packet_async/raw to send
>>>> packet, it takes an NetClientState as the first argument sender,
>>>> and use sender->peer->incoming_queue as the dest queue, so in order to
>>>> make this API work, we need to use this dummy NC and init it's
>>>> peer to our dest(which is the network backend)
>>>> Another way is to call qemu_net_queue_send(netdev->incoming_queue,...)
>>>> directly, we still need a NetClientState *sender param, can not
>>>> use NetFilterState.
>>>
>>> I think this is my meaning. Use NetFilterState->netdev.
>>
>> Problem is NetFilterState->netdev is our destination, we need a sender...
>> if we use this, packet will be sent back to NIC...
>>
>
> I see, then NetFilterState->netdev->peer is sender. But I think it's
> better to track sender instead of destination in this case. Something
> like dummy NC is not elegant.
>
>>>
>>>> This dummy NC also been checked in filter_buffer_receive to avoid
>>>> buffering
>>>> packet been sent by ourself.
>>>>
>>>
>>> I don't get why this is needed. Who is going to queue a packet in dummy
>>> NC, consider it was not peered by any others?
>>
>> There's nothing in the dummy NC except the dummy->peer =
>> NetFilterState->netdev
>> This dummy NC only used to as a sender param of the existing APIs
>> which send
>> packets. When a buffered packet been sent, we shouldn't buffer it
>> again, we
>> cann't use any existing NC (packet->sender or NetFilterState->netdev)
>> as the sender because otherwise we can't distinguish if the packet is
>> a buffered
>> packet sent by ourself.
>
> I see, so the reason is you are using qemu_deliver_packet() for both
> enqueuing packet to filter and delivering packet to destination. How
> about something like:
>
> E.g for qemu_send_packet_async(), move the hook before
> qemu_send_packet_async_with_flags(). Then flush method can call
> qemu_send_packet_async_with_flags() without any issue?

I think we can't move the hook earlier, because filters only deal
with the packets will actually been sent. for example, a dump filter.
dump packet that probably won't been sent is wrong. calling
qemu_send_packet_async() or qemu_send_packet_async_with_flags()
doesn't mean the packet is sent, if the sent_cb is not provided and
the other peer is not able to receive, the packet will be dropped.

>
>
>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Jason Wang]

On 07/30/2015 05:49 PM, Yang Hongyang wrote:
> On 07/30/2015 05:33 PM, Jason Wang wrote:
>> On 07/30/2015 05:04 PM, Yang Hongyang wrote:
>>>
>>>
>>> On 07/30/2015 04:40 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 07/30/2015 02:47 PM, Yang Hongyang wrote:
>>>>> On 07/30/2015 01:13 PM, Jason Wang wrote:
>>>>> [...]
>>>>>>> +
>>>>>>> +#include "net/filter.h"
>>>>>>> +#include "net/queue.h"
>>>>>>> +#include "filters.h"
>>>>>>> +#include "qemu-common.h"
>>>>>>> +#include "qemu/error-report.h"
>>>>>>> +
>>>>>>> +typedef struct FILTERBUFFERState {
>>>>>>> +    NetFilterState nf;
>>>>>>> +    NetClientState dummy; /* used to send buffered packets */
>>>>>>
>>>>>> Why need this? Couldn't we just infer this from NetFilterState?
>>>>>
>>>>> Because we use existing API qemu_send_packet_async/raw to send
>>>>> packet, it takes an NetClientState as the first argument sender,
>>>>> and use sender->peer->incoming_queue as the dest queue, so in
>>>>> order to
>>>>> make this API work, we need to use this dummy NC and init it's
>>>>> peer to our dest(which is the network backend)
>>>>> Another way is to call
>>>>> qemu_net_queue_send(netdev->incoming_queue,...)
>>>>> directly, we still need a NetClientState *sender param, can not
>>>>> use NetFilterState.
>>>>
>>>> I think this is my meaning. Use NetFilterState->netdev.
>>>
>>> Problem is NetFilterState->netdev is our destination, we need a
>>> sender...
>>> if we use this, packet will be sent back to NIC...
>>>
>>
>> I see, then NetFilterState->netdev->peer is sender. But I think it's
>> better to track sender instead of destination in this case. Something
>> like dummy NC is not elegant.
>>
>>>>
>>>>> This dummy NC also been checked in filter_buffer_receive to avoid
>>>>> buffering
>>>>> packet been sent by ourself.
>>>>>
>>>>
>>>> I don't get why this is needed. Who is going to queue a packet in
>>>> dummy
>>>> NC, consider it was not peered by any others?
>>>
>>> There's nothing in the dummy NC except the dummy->peer =
>>> NetFilterState->netdev
>>> This dummy NC only used to as a sender param of the existing APIs
>>> which send
>>> packets. When a buffered packet been sent, we shouldn't buffer it
>>> again, we
>>> cann't use any existing NC (packet->sender or NetFilterState->netdev)
>>> as the sender because otherwise we can't distinguish if the packet is
>>> a buffered
>>> packet sent by ourself.
>>
>> I see, so the reason is you are using qemu_deliver_packet() for both
>> enqueuing packet to filter and delivering packet to destination. How
>> about something like:
>>
>> E.g for qemu_send_packet_async(), move the hook before
>> qemu_send_packet_async_with_flags(). Then flush method can call
>> qemu_send_packet_async_with_flags() without any issue?
>
> I think we can't move the hook earlier, because filters only deal
> with the packets will actually been sent. for example, a dump filter.
> dump packet that probably won't been sent is wrong. calling
> qemu_send_packet_async() or qemu_send_packet_async_with_flags()
> doesn't mean the packet is sent, if the sent_cb is not provided and
> the other peer is not able to receive, the packet will be dropped. 

It depends on how do you define 'actually been sent' and whether or not
we should have such accuracy. Packet could be dropped by various layers.
Reaching receive() or receive_iov() does not mean it can be sent for
sure. For example, lots of nics drop packet in their receive()
implementation.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 06:14 PM, Jason Wang wrote:
>
>
> On 07/30/2015 05:49 PM, Yang Hongyang wrote:
>> On 07/30/2015 05:33 PM, Jason Wang wrote:
>>> On 07/30/2015 05:04 PM, Yang Hongyang wrote:
>>>>
>>>>
>>>> On 07/30/2015 04:40 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 07/30/2015 02:47 PM, Yang Hongyang wrote:
>>>>>> On 07/30/2015 01:13 PM, Jason Wang wrote:
>>>>>> [...]
>>>>>>>> +
>>>>>>>> +#include "net/filter.h"
>>>>>>>> +#include "net/queue.h"
>>>>>>>> +#include "filters.h"
>>>>>>>> +#include "qemu-common.h"
>>>>>>>> +#include "qemu/error-report.h"
>>>>>>>> +
>>>>>>>> +typedef struct FILTERBUFFERState {
>>>>>>>> +    NetFilterState nf;
>>>>>>>> +    NetClientState dummy; /* used to send buffered packets */
>>>>>>>
>>>>>>> Why need this? Couldn't we just infer this from NetFilterState?
>>>>>>
>>>>>> Because we use existing API qemu_send_packet_async/raw to send
>>>>>> packet, it takes an NetClientState as the first argument sender,
>>>>>> and use sender->peer->incoming_queue as the dest queue, so in
>>>>>> order to
>>>>>> make this API work, we need to use this dummy NC and init it's
>>>>>> peer to our dest(which is the network backend)
>>>>>> Another way is to call
>>>>>> qemu_net_queue_send(netdev->incoming_queue,...)
>>>>>> directly, we still need a NetClientState *sender param, can not
>>>>>> use NetFilterState.
>>>>>
>>>>> I think this is my meaning. Use NetFilterState->netdev.
>>>>
>>>> Problem is NetFilterState->netdev is our destination, we need a
>>>> sender...
>>>> if we use this, packet will be sent back to NIC...
>>>>
>>>
>>> I see, then NetFilterState->netdev->peer is sender. But I think it's
>>> better to track sender instead of destination in this case. Something
>>> like dummy NC is not elegant.
>>>
>>>>>
>>>>>> This dummy NC also been checked in filter_buffer_receive to avoid
>>>>>> buffering
>>>>>> packet been sent by ourself.
>>>>>>
>>>>>
>>>>> I don't get why this is needed. Who is going to queue a packet in
>>>>> dummy
>>>>> NC, consider it was not peered by any others?
>>>>
>>>> There's nothing in the dummy NC except the dummy->peer =
>>>> NetFilterState->netdev
>>>> This dummy NC only used to as a sender param of the existing APIs
>>>> which send
>>>> packets. When a buffered packet been sent, we shouldn't buffer it
>>>> again, we
>>>> cann't use any existing NC (packet->sender or NetFilterState->netdev)
>>>> as the sender because otherwise we can't distinguish if the packet is
>>>> a buffered
>>>> packet sent by ourself.
>>>
>>> I see, so the reason is you are using qemu_deliver_packet() for both
>>> enqueuing packet to filter and delivering packet to destination. How
>>> about something like:
>>>
>>> E.g for qemu_send_packet_async(), move the hook before
>>> qemu_send_packet_async_with_flags(). Then flush method can call
>>> qemu_send_packet_async_with_flags() without any issue?
>>
>> I think we can't move the hook earlier, because filters only deal
>> with the packets will actually been sent. for example, a dump filter.
>> dump packet that probably won't been sent is wrong. calling
>> qemu_send_packet_async() or qemu_send_packet_async_with_flags()
>> doesn't mean the packet is sent, if the sent_cb is not provided and
>> the other peer is not able to receive, the packet will be dropped.
>
> It depends on how do you define 'actually been sent' and whether or not
> we should have such accuracy. Packet could be dropped by various layers.
> Reaching receive() or receive_iov() does not mean it can be sent for
> sure. For example, lots of nics drop packet in their receive()
> implementation.

This is true, ok, I'm convinced that we might not need to be this accurate.
but Thomas might have different opinion, I saw this description in his
dump series:

+    /*
+     * Log network traffic into a dump file. Note: This should ideally
+     * be done after calling the ->receive() function below to make sure
+     * that we only log the packets that have really been sent. However,
+     * this does not work right with slirp networking since it immediately
+     * sends reply packets during the receive() function already, so we
+     * would get a wrong order of the packets in the dump file in that case.
+     */

So Thomas, what do you think of this?

> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Thomas Huth]

On 30/07/15 12:28, Yang Hongyang wrote:
> On 07/30/2015 06:14 PM, Jason Wang wrote:
>>
>>
>> On 07/30/2015 05:49 PM, Yang Hongyang wrote:
>>> On 07/30/2015 05:33 PM, Jason Wang wrote:
[...]
>>>> I see, so the reason is you are using qemu_deliver_packet() for both
>>>> enqueuing packet to filter and delivering packet to destination. How
>>>> about something like:
>>>>
>>>> E.g for qemu_send_packet_async(), move the hook before
>>>> qemu_send_packet_async_with_flags(). Then flush method can call
>>>> qemu_send_packet_async_with_flags() without any issue?
>>>
>>> I think we can't move the hook earlier, because filters only deal
>>> with the packets will actually been sent. for example, a dump filter.
>>> dump packet that probably won't been sent is wrong. calling
>>> qemu_send_packet_async() or qemu_send_packet_async_with_flags()
>>> doesn't mean the packet is sent, if the sent_cb is not provided and
>>> the other peer is not able to receive, the packet will be dropped.
>>
>> It depends on how do you define 'actually been sent' and whether or not
>> we should have such accuracy. Packet could be dropped by various layers.
>> Reaching receive() or receive_iov() does not mean it can be sent for
>> sure. For example, lots of nics drop packet in their receive()
>> implementation.
> 
> This is true, ok, I'm convinced that we might not need to be this accurate.
> but Thomas might have different opinion, I saw this description in his
> dump series:
> 
> +    /*
> +     * Log network traffic into a dump file. Note: This should ideally
> +     * be done after calling the ->receive() function below to make sure
> +     * that we only log the packets that have really been sent. However,
> +     * this does not work right with slirp networking since it immediately
> +     * sends reply packets during the receive() function already, so we
> +     * would get a wrong order of the packets in the dump file in that
> case.
> +     */
> 
> So Thomas, what do you think of this?

IMHO it should be ok if a dump captures a packet multiple times - it's
not nice, but it could theoretically also happen on a physical line when
a packet has to be retransmitted.

 Thomas

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 10:16 PM, Thomas Huth wrote:
> On 30/07/15 12:28, Yang Hongyang wrote:
>> On 07/30/2015 06:14 PM, Jason Wang wrote:
>>>
>>>
>>> On 07/30/2015 05:49 PM, Yang Hongyang wrote:
>>>> On 07/30/2015 05:33 PM, Jason Wang wrote:
> [...]
>>>>> I see, so the reason is you are using qemu_deliver_packet() for both
>>>>> enqueuing packet to filter and delivering packet to destination. How
>>>>> about something like:
>>>>>
>>>>> E.g for qemu_send_packet_async(), move the hook before
>>>>> qemu_send_packet_async_with_flags(). Then flush method can call
>>>>> qemu_send_packet_async_with_flags() without any issue?
>>>>
>>>> I think we can't move the hook earlier, because filters only deal
>>>> with the packets will actually been sent. for example, a dump filter.
>>>> dump packet that probably won't been sent is wrong. calling
>>>> qemu_send_packet_async() or qemu_send_packet_async_with_flags()
>>>> doesn't mean the packet is sent, if the sent_cb is not provided and
>>>> the other peer is not able to receive, the packet will be dropped.
>>>
>>> It depends on how do you define 'actually been sent' and whether or not
>>> we should have such accuracy. Packet could be dropped by various layers.
>>> Reaching receive() or receive_iov() does not mean it can be sent for
>>> sure. For example, lots of nics drop packet in their receive()
>>> implementation.
>>
>> This is true, ok, I'm convinced that we might not need to be this accurate.
>> but Thomas might have different opinion, I saw this description in his
>> dump series:
>>
>> +    /*
>> +     * Log network traffic into a dump file. Note: This should ideally
>> +     * be done after calling the ->receive() function below to make sure
>> +     * that we only log the packets that have really been sent. However,
>> +     * this does not work right with slirp networking since it immediately
>> +     * sends reply packets during the receive() function already, so we
>> +     * would get a wrong order of the packets in the dump file in that
>> case.
>> +     */
>>
>> So Thomas, what do you think of this?
>
> IMHO it should be ok if a dump captures a packet multiple times - it's
> not nice, but it could theoretically also happen on a physical line when
> a packet has to be retransmitted.

Ok, thanks! then I'll move the filter hook earlier.

>
>   Thomas
>
>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 06:14 PM, Jason Wang wrote:
>
>
> On 07/30/2015 05:49 PM, Yang Hongyang wrote:
>> On 07/30/2015 05:33 PM, Jason Wang wrote:
>>> On 07/30/2015 05:04 PM, Yang Hongyang wrote:
>>>>
>>>>
>>>> On 07/30/2015 04:40 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 07/30/2015 02:47 PM, Yang Hongyang wrote:
>>>>>> On 07/30/2015 01:13 PM, Jason Wang wrote:
>>>>>> [...]
>>>>>>>> +
>>>>>>>> +#include "net/filter.h"
>>>>>>>> +#include "net/queue.h"
>>>>>>>> +#include "filters.h"
>>>>>>>> +#include "qemu-common.h"
>>>>>>>> +#include "qemu/error-report.h"
>>>>>>>> +
>>>>>>>> +typedef struct FILTERBUFFERState {
>>>>>>>> +    NetFilterState nf;
>>>>>>>> +    NetClientState dummy; /* used to send buffered packets */
>>>>>>>
>>>>>>> Why need this? Couldn't we just infer this from NetFilterState?
>>>>>>
>>>>>> Because we use existing API qemu_send_packet_async/raw to send
>>>>>> packet, it takes an NetClientState as the first argument sender,
>>>>>> and use sender->peer->incoming_queue as the dest queue, so in
>>>>>> order to
>>>>>> make this API work, we need to use this dummy NC and init it's
>>>>>> peer to our dest(which is the network backend)
>>>>>> Another way is to call
>>>>>> qemu_net_queue_send(netdev->incoming_queue,...)
>>>>>> directly, we still need a NetClientState *sender param, can not
>>>>>> use NetFilterState.
>>>>>
>>>>> I think this is my meaning. Use NetFilterState->netdev.
>>>>
>>>> Problem is NetFilterState->netdev is our destination, we need a
>>>> sender...
>>>> if we use this, packet will be sent back to NIC...
>>>>
>>>
>>> I see, then NetFilterState->netdev->peer is sender. But I think it's
>>> better to track sender instead of destination in this case. Something
>>> like dummy NC is not elegant.
>>>
>>>>>
>>>>>> This dummy NC also been checked in filter_buffer_receive to avoid
>>>>>> buffering
>>>>>> packet been sent by ourself.
>>>>>>
>>>>>
>>>>> I don't get why this is needed. Who is going to queue a packet in
>>>>> dummy
>>>>> NC, consider it was not peered by any others?
>>>>
>>>> There's nothing in the dummy NC except the dummy->peer =
>>>> NetFilterState->netdev
>>>> This dummy NC only used to as a sender param of the existing APIs
>>>> which send
>>>> packets. When a buffered packet been sent, we shouldn't buffer it
>>>> again, we
>>>> cann't use any existing NC (packet->sender or NetFilterState->netdev)
>>>> as the sender because otherwise we can't distinguish if the packet is
>>>> a buffered
>>>> packet sent by ourself.
>>>
>>> I see, so the reason is you are using qemu_deliver_packet() for both
>>> enqueuing packet to filter and delivering packet to destination. How
>>> about something like:
>>>
>>> E.g for qemu_send_packet_async(), move the hook before
>>> qemu_send_packet_async_with_flags(). Then flush method can call
>>> qemu_send_packet_async_with_flags() without any issue?
>>
>> I think we can't move the hook earlier, because filters only deal
>> with the packets will actually been sent. for example, a dump filter.
>> dump packet that probably won't been sent is wrong. calling
>> qemu_send_packet_async() or qemu_send_packet_async_with_flags()
>> doesn't mean the packet is sent, if the sent_cb is not provided and
>> the other peer is not able to receive, the packet will be dropped.
>
> It depends on how do you define 'actually been sent' and whether or not
> we should have such accuracy. Packet could be dropped by various layers.
> Reaching receive() or receive_iov() does not mean it can be sent for
> sure. For example, lots of nics drop packet in their receive()
> implementation.

Ok, I think we can move the filter hook before calling
qemu_net_queue_send/qemu_net_queue_send_iov, then in the
flush method, we can call qemu_net_queue_send directly
to avoid the packet go to the filter hook again.

> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Yang Hongyang]

On 07/30/2015 01:13 PM, Jason Wang wrote:
[...]
>> +    if (sender->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
>> +        /* we only buffer guest output packets */
>> +        qemu_net_queue_append(queue, sender, flags, data, size,
>> +                              packet_send_completed);
>
> This may brings some confusion for user. Since the name is 'netbuffer'.
> Maybe we can change the filter to be ingress or out or both? Like:
>
> -device virtio-net-pci,id=virtio0
> -netfilter buffer,id=filter0,dev=virtio0,interval=1000,type=out
>
> Then we can just try to enqueue the packet when virtio-net-pci is sender?

A good idea, I also thought about this, but a question, should we make
this type option a mandatory to netfilter object or optional? if it's
optional, the default will be "all"

>
>> +        /* Now that we have buffered the packet, return sucess */
>> +        return size;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void filter_buffer_cleanup(NetFilterState *nf)
>> +{
>> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>> +
>> +    /* flush inflight packets */
>> +    filter_buffer_flush(nf);
>> +    /* flush incoming packets */
>> +    s->inflight_queue = s->incoming_queue;
>> +    s->incoming_queue = NULL;
>> +    filter_buffer_flush(nf);
>> +
>> +    return;
>> +}
>> +
>> +
>> +static NetFilterInfo net_filter_buffer_info = {
>> +    .type = NET_FILTER_OPTIONS_KIND_BUFFER,
>> +    .size = sizeof(FILTERBUFFERState),
>> +    .receive = filter_buffer_receive,
>> +    .cleanup = filter_buffer_cleanup,
>> +};
>> +
>> +int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
>> +                           NetClientState *netdev, Error **errp)
>> +{
>> +    NetFilterState *nf;
>> +    FILTERBUFFERState *s;
>> +
>> +    assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
>> +
>> +    nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, "buffer", name);
>> +    s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>> +    /*
>> +     * we need the dummy NetClientState to send packets in order to avoid
>> +     * receive packets again.
>> +     * we are buffering guest output packets, our buffered packets should be
>> +     * sent to real network backend, so dummy's peer should be that backend.
>> +     */
>> +    s->dummy.peer = netdev;
>> +    s->incoming_queue = qemu_new_net_queue(nf);
>> +
>> +    return 0;
>> +}
>> diff --git a/net/filter.c b/net/filter.c
>> index 50fb837..e741e2a 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -18,6 +18,7 @@
>>
>>   #include "net/filter.h"
>>   #include "net/net.h"
>> +#include "filters.h"
>>
>>   static QTAILQ_HEAD(, NetFilterState) net_filters;
>>
>> @@ -152,6 +153,7 @@ typedef int (NetFilterInit)(const NetFilterOptions *opts,
>>
>>   static
>>   NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
>> +    [NET_FILTER_OPTIONS_KIND_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
>> new file mode 100644
>> index 0000000..6c249b8
>> --- /dev/null
>> +++ b/net/filters.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * 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_buffer(const NetFilterOptions *opts, const char *name,
>> +                           NetClientState *netdev, Error **errp);
>> +
>> +#endif /* QEMU_NET_FILTERS_H */
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 1fc6390..67e00a0 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2577,6 +2577,16 @@
>>   { 'command': 'netfilter_del', 'data': {'id': 'str'} }
>>
>>   ##
>> +# @NetFilterBufferOptions
>> +#
>> +# a netbuffer filter for network backend.
>> +#
>> +# Since 2.5
>> +##
>> +{ 'struct': 'NetFilterBufferOptions',
>> +  'data': { } }
>> +
>> +##
>>   # @NetFilterOptions
>>   #
>>   # A discriminated record of network filters.
>> @@ -2585,7 +2595,8 @@
>>   #
>>   ##
>>   { 'union': 'NetFilterOptions',
>> -  'data': { } }
>> +  'data': {
>> +    'buffer':     'NetFilterBufferOptions'} }
>>
>>   ##
>>   # @NetFilter
>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter

[Jason Wang]

On 07/30/2015 03:00 PM, Yang Hongyang wrote:
> On 07/30/2015 01:13 PM, Jason Wang wrote:
> [...]
>>> +    if (sender->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
>>> +        /* we only buffer guest output packets */
>>> +        qemu_net_queue_append(queue, sender, flags, data, size,
>>> +                              packet_send_completed);
>>
>> This may brings some confusion for user. Since the name is 'netbuffer'.
>> Maybe we can change the filter to be ingress or out or both? Like:
>>
>> -device virtio-net-pci,id=virtio0
>> -netfilter buffer,id=filter0,dev=virtio0,interval=1000,type=out
>>
>> Then we can just try to enqueue the packet when virtio-net-pci is
>> sender?
>
> A good idea, I also thought about this, but a question, should we make
> this type option a mandatory to netfilter object or optional? if it's
> optional, the default will be "all"

Yes, this should work.

[Qemu-devel] [PATCH 10/12] netbuffer: add a public api filter_buffer_release_all

[Yang Hongyang]

add a public api filter_buffer_release_all to release all
buffered packets.
also introduce qemu_find_netfilters_by_model to find all buffer
filters.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 include/net/filter.h |  5 +++++
 net/filter-buffer.c  | 41 +++++++++++++++++++++++++++++++++++++++++
 net/filter.c         | 18 ++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 5292563..798b5b2 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -50,5 +50,10 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
                                     const char *name);
 void netfilter_add(QemuOpts *opts, Error **errp);
 void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
+int qemu_find_netfilters_by_model(const char *model, NetFilterState **nfs,
+                                  int max);
+
+/* netbuffer filter */
+void filter_buffer_release_all(void);
 
 #endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index 628e66f..8bac73b 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -11,12 +11,14 @@
 #include "filters.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
+#include "qemu/main-loop.h"
 
 typedef struct FILTERBUFFERState {
     NetFilterState nf;
     NetClientState dummy; /* used to send buffered packets */
     NetQueue *incoming_queue;
     NetQueue *inflight_queue;
+    QEMUBH *flush_bh;
 } FILTERBUFFERState;
 
 static void packet_send_completed(NetClientState *nc, ssize_t len)
@@ -56,6 +58,27 @@ static void filter_buffer_flush(NetFilterState *nf)
     }
 }
 
+static void filter_buffer_flush_bh(void *opaque)
+{
+    FILTERBUFFERState *s = opaque;
+    NetFilterState *nf = &s->nf;
+    filter_buffer_flush(nf);
+}
+
+static void filter_buffer_release_one(NetFilterState *nf)
+{
+    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
+
+    /* flush inflight packets */
+    if (s->inflight_queue) {
+        filter_buffer_flush(nf);
+    }
+
+    s->inflight_queue = s->incoming_queue;
+    s->incoming_queue = qemu_new_net_queue(nf);
+    qemu_bh_schedule(s->flush_bh);
+}
+
 /* filter APIs */
 static ssize_t filter_buffer_receive(NetFilterState *nf,
                                      NetClientState *sender,
@@ -93,6 +116,10 @@ static void filter_buffer_cleanup(NetFilterState *nf)
     s->incoming_queue = NULL;
     filter_buffer_flush(nf);
 
+    if (s->flush_bh) {
+        qemu_bh_delete(s->flush_bh);
+        s->flush_bh = NULL;
+    }
     return;
 }
 
@@ -122,6 +149,20 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
      */
     s->dummy.peer = netdev;
     s->incoming_queue = qemu_new_net_queue(nf);
+    s->flush_bh = qemu_bh_new(filter_buffer_flush_bh, s);
 
     return 0;
 }
+
+/* public APIs */
+void filter_buffer_release_all(void)
+{
+    NetFilterState *nfs[MAX_QUEUE_NUM];
+    int queues, i;
+
+    queues = qemu_find_netfilters_by_model("buffer", nfs, MAX_QUEUE_NUM);
+
+    for (i = 0; i < queues; i++) {
+        filter_buffer_release_one(nfs[i]);
+    }
+}
diff --git a/net/filter.c b/net/filter.c
index e741e2a..b9a6216 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -93,6 +93,24 @@ static NetFilterState *qemu_find_netfilter(const char *id)
     return NULL;
 }
 
+int qemu_find_netfilters_by_model(const char *model, NetFilterState **nfs,
+                                  int max)
+{
+    NetFilterState *nf;
+    int ret = 0;
+
+    QTAILQ_FOREACH(nf, &net_filters, next) {
+        if (!strcmp(nf->model, model)) {
+            if (ret < max) {
+                nfs[ret] = nf;
+            }
+            ret++;
+        }
+    }
+
+    return ret;
+}
+
 static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp);
 void netfilter_add(QemuOpts *opts, Error **errp)
 {
-- 
1.9.1

Re: [Qemu-devel] [PATCH 10/12] netbuffer: add a public api filter_buffer_release_all

[Jason Wang]

On 07/29/2015 06:51 PM, Yang Hongyang wrote:
> add a public api filter_buffer_release_all to release all
> buffered packets.
> also introduce qemu_find_netfilters_by_model to find all buffer
> filters.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  include/net/filter.h |  5 +++++
>  net/filter-buffer.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>  net/filter.c         | 18 ++++++++++++++++++
>  3 files changed, 64 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 5292563..798b5b2 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -50,5 +50,10 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
>                                      const char *name);
>  void netfilter_add(QemuOpts *opts, Error **errp);
>  void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
> +int qemu_find_netfilters_by_model(const char *model, NetFilterState **nfs,
> +                                  int max);
> +
> +/* netbuffer filter */
> +void filter_buffer_release_all(void);
>  
>  #endif /* QEMU_NET_FILTER_H */
> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
> index 628e66f..8bac73b 100644
> --- a/net/filter-buffer.c
> +++ b/net/filter-buffer.c
> @@ -11,12 +11,14 @@
>  #include "filters.h"
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
>  
>  typedef struct FILTERBUFFERState {
>      NetFilterState nf;
>      NetClientState dummy; /* used to send buffered packets */
>      NetQueue *incoming_queue;
>      NetQueue *inflight_queue;
> +    QEMUBH *flush_bh;

bh should be stopped and restarted during vm stop and continue.

>  } FILTERBUFFERState;
>  
>  static void packet_send_completed(NetClientState *nc, ssize_t len)
> @@ -56,6 +58,27 @@ static void filter_buffer_flush(NetFilterState *nf)
>      }
>  }
>  
> +static void filter_buffer_flush_bh(void *opaque)
> +{
> +    FILTERBUFFERState *s = opaque;
> +    NetFilterState *nf = &s->nf;
> +    filter_buffer_flush(nf);
> +}
> +
> +static void filter_buffer_release_one(NetFilterState *nf)
> +{
> +    FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
> +
> +    /* flush inflight packets */
> +    if (s->inflight_queue) {
> +        filter_buffer_flush(nf);
> +    }
> +
> +    s->inflight_queue = s->incoming_queue;
> +    s->incoming_queue = qemu_new_net_queue(nf);
> +    qemu_bh_schedule(s->flush_bh);
> +}
> +
>  /* filter APIs */
>  static ssize_t filter_buffer_receive(NetFilterState *nf,
>                                       NetClientState *sender,
> @@ -93,6 +116,10 @@ static void filter_buffer_cleanup(NetFilterState *nf)
>      s->incoming_queue = NULL;
>      filter_buffer_flush(nf);
>  
> +    if (s->flush_bh) {
> +        qemu_bh_delete(s->flush_bh);
> +        s->flush_bh = NULL;
> +    }
>      return;
>  }
>  
> @@ -122,6 +149,20 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
>       */
>      s->dummy.peer = netdev;
>      s->incoming_queue = qemu_new_net_queue(nf);
> +    s->flush_bh = qemu_bh_new(filter_buffer_flush_bh, s);
>  
>      return 0;
>  }
> +
> +/* public APIs */
> +void filter_buffer_release_all(void)
> +{
> +    NetFilterState *nfs[MAX_QUEUE_NUM];
> +    int queues, i;
> +
> +    queues = qemu_find_netfilters_by_model("buffer", nfs, MAX_QUEUE_NUM);
> +
> +    for (i = 0; i < queues; i++) {
> +        filter_buffer_release_one(nfs[i]);
> +    }
> +}

Looks like the function was never used by following patches?

> diff --git a/net/filter.c b/net/filter.c
> index e741e2a..b9a6216 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -93,6 +93,24 @@ static NetFilterState *qemu_find_netfilter(const char *id)
>      return NULL;
>  }
>  
> +int qemu_find_netfilters_by_model(const char *model, NetFilterState **nfs,
> +                                  int max)
> +{
> +    NetFilterState *nf;
> +    int ret = 0;
> +
> +    QTAILQ_FOREACH(nf, &net_filters, next) {
> +        if (!strcmp(nf->model, model)) {
> +            if (ret < max) {
> +                nfs[ret] = nf;
> +            }
> +            ret++;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>  static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp);
>  void netfilter_add(QemuOpts *opts, Error **errp)
>  {

Re: [Qemu-devel] [PATCH 10/12] netbuffer: add a public api filter_buffer_release_all

[Yang Hongyang]

On 07/30/2015 01:25 PM, Jason Wang wrote:
>
>
> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>> add a public api filter_buffer_release_all to release all
>> buffered packets.
>> also introduce qemu_find_netfilters_by_model to find all buffer
>> filters.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> ---
>>   include/net/filter.h |  5 +++++
>>   net/filter-buffer.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>>   net/filter.c         | 18 ++++++++++++++++++
>>   3 files changed, 64 insertions(+)
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index 5292563..798b5b2 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -50,5 +50,10 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
>>                                       const char *name);
>>   void netfilter_add(QemuOpts *opts, Error **errp);
>>   void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
>> +int qemu_find_netfilters_by_model(const char *model, NetFilterState **nfs,
>> +                                  int max);
>> +
>> +/* netbuffer filter */
>> +void filter_buffer_release_all(void);
>>
>>   #endif /* QEMU_NET_FILTER_H */
>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>> index 628e66f..8bac73b 100644
>> --- a/net/filter-buffer.c
>> +++ b/net/filter-buffer.c
>> @@ -11,12 +11,14 @@
>>   #include "filters.h"
>>   #include "qemu-common.h"
>>   #include "qemu/error-report.h"
>> +#include "qemu/main-loop.h"
>>
>>   typedef struct FILTERBUFFERState {
>>       NetFilterState nf;
>>       NetClientState dummy; /* used to send buffered packets */
>>       NetQueue *incoming_queue;
>>       NetQueue *inflight_queue;
>> +    QEMUBH *flush_bh;
>
> bh should be stopped and restarted during vm stop and continue.

Sorry, could you explain more about this? do you mean to check
the vm state before bh_schedule?
how to stop&restart a bh? bh_delete bh_new bh_schedule?

>
>>   } FILTERBUFFERState;
>>
>>   static void packet_send_completed(NetClientState *nc, ssize_t len)
>> @@ -56,6 +58,27 @@ static void filter_buffer_flush(NetFilterState *nf)
>>       }
>>   }
>>
>> +static void filter_buffer_flush_bh(void *opaque)
>> +{
>> +    FILTERBUFFERState *s = opaque;
[...]
>> +
>> +/* public APIs */
>> +void filter_buffer_release_all(void)
>> +{
>> +    NetFilterState *nfs[MAX_QUEUE_NUM];
>> +    int queues, i;
>> +
>> +    queues = qemu_find_netfilters_by_model("buffer", nfs, MAX_QUEUE_NUM);
>> +
>> +    for (i = 0; i < queues; i++) {
>> +        filter_buffer_release_one(nfs[i]);
>> +    }
>> +}
>
> Looks like the function was never used by following patches?

Right, it's not used in this series. But it can be used by MC to release packets
at checkpoint, Should I mark this unused, or drop this API?

>
>> diff --git a/net/filter.c b/net/filter.c
>> index e741e2a..b9a6216 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -93,6 +93,24 @@ static NetFilterState *qemu_find_netfilter(const char *id)
>>       return NULL;
>>   }
>>
>> +int qemu_find_netfilters_by_model(const char *model, NetFilterState **nfs,
>> +                                  int max)
>> +{
>> +    NetFilterState *nf;
>> +    int ret = 0;
>> +
>> +    QTAILQ_FOREACH(nf, &net_filters, next) {
>> +        if (!strcmp(nf->model, model)) {
>> +            if (ret < max) {
>> +                nfs[ret] = nf;
>> +            }
>> +            ret++;
>> +        }
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>>   static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp);
>>   void netfilter_add(QemuOpts *opts, Error **errp)
>>   {
>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 10/12] netbuffer: add a public api filter_buffer_release_all

[Jason Wang]

On 07/30/2015 01:50 PM, Yang Hongyang wrote:
> On 07/30/2015 01:25 PM, Jason Wang wrote:
>>
>>
>> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>>> add a public api filter_buffer_release_all to release all
>>> buffered packets.
>>> also introduce qemu_find_netfilters_by_model to find all buffer
>>> filters.
>>>
>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>> ---
>>>   include/net/filter.h |  5 +++++
>>>   net/filter-buffer.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>>>   net/filter.c         | 18 ++++++++++++++++++
>>>   3 files changed, 64 insertions(+)
>>>
>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>> index 5292563..798b5b2 100644
>>> --- a/include/net/filter.h
>>> +++ b/include/net/filter.h
>>> @@ -50,5 +50,10 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo
>>> *info,
>>>                                       const char *name);
>>>   void netfilter_add(QemuOpts *opts, Error **errp);
>>>   void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
>>> +int qemu_find_netfilters_by_model(const char *model, NetFilterState
>>> **nfs,
>>> +                                  int max);
>>> +
>>> +/* netbuffer filter */
>>> +void filter_buffer_release_all(void);
>>>
>>>   #endif /* QEMU_NET_FILTER_H */
>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>>> index 628e66f..8bac73b 100644
>>> --- a/net/filter-buffer.c
>>> +++ b/net/filter-buffer.c
>>> @@ -11,12 +11,14 @@
>>>   #include "filters.h"
>>>   #include "qemu-common.h"
>>>   #include "qemu/error-report.h"
>>> +#include "qemu/main-loop.h"
>>>
>>>   typedef struct FILTERBUFFERState {
>>>       NetFilterState nf;
>>>       NetClientState dummy; /* used to send buffered packets */
>>>       NetQueue *incoming_queue;
>>>       NetQueue *inflight_queue;
>>> +    QEMUBH *flush_bh;
>>
>> bh should be stopped and restarted during vm stop and continue.
>
> Sorry, could you explain more about this? do you mean to check
> the vm state before bh_schedule?
> how to stop&restart a bh? bh_delete bh_new bh_schedule?
>
>>
>>>   } FILTERBUFFERState;
>>>
>>>   static void packet_send_completed(NetClientState *nc, ssize_t len)
>>> @@ -56,6 +58,27 @@ static void filter_buffer_flush(NetFilterState *nf)
>>>       }
>>>   }
>>>
>>> +static void filter_buffer_flush_bh(void *opaque)
>>> +{
>>> +    FILTERBUFFERState *s = opaque;
> [...]
>>> +
>>> +/* public APIs */
>>> +void filter_buffer_release_all(void)
>>> +{
>>> +    NetFilterState *nfs[MAX_QUEUE_NUM];
>>> +    int queues, i;
>>> +
>>> +    queues = qemu_find_netfilters_by_model("buffer", nfs,
>>> MAX_QUEUE_NUM);
>>> +
>>> +    for (i = 0; i < queues; i++) {
>>> +        filter_buffer_release_one(nfs[i]);
>>> +    }
>>> +}
>>
>> Looks like the function was never used by following patches?
>
> Right, it's not used in this series. But it can be used by MC to
> release packets
> at checkpoint, Should I mark this unused, or drop this API?

Please drop this and re-introduce them which it has users.

Re: [Qemu-devel] [PATCH 10/12] netbuffer: add a public api filter_buffer_release_all

[Yang Hongyang]

On 07/30/2015 04:42 PM, Jason Wang wrote:
>
>
> On 07/30/2015 01:50 PM, Yang Hongyang wrote:
>> On 07/30/2015 01:25 PM, Jason Wang wrote:
>>>
>>>
>>> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>>>> add a public api filter_buffer_release_all to release all
>>>> buffered packets.
>>>> also introduce qemu_find_netfilters_by_model to find all buffer
>>>> filters.
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>> ---
>>>>    include/net/filter.h |  5 +++++
>>>>    net/filter-buffer.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>>>>    net/filter.c         | 18 ++++++++++++++++++
>>>>    3 files changed, 64 insertions(+)
>>>>
>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>> index 5292563..798b5b2 100644
>>>> --- a/include/net/filter.h
>>>> +++ b/include/net/filter.h
>>>> @@ -50,5 +50,10 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo
>>>> *info,
>>>>                                        const char *name);
>>>>    void netfilter_add(QemuOpts *opts, Error **errp);
>>>>    void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
>>>> +int qemu_find_netfilters_by_model(const char *model, NetFilterState
>>>> **nfs,
>>>> +                                  int max);
>>>> +
>>>> +/* netbuffer filter */
>>>> +void filter_buffer_release_all(void);
>>>>
>>>>    #endif /* QEMU_NET_FILTER_H */
>>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>>>> index 628e66f..8bac73b 100644
>>>> --- a/net/filter-buffer.c
>>>> +++ b/net/filter-buffer.c
>>>> @@ -11,12 +11,14 @@
>>>>    #include "filters.h"
>>>>    #include "qemu-common.h"
>>>>    #include "qemu/error-report.h"
>>>> +#include "qemu/main-loop.h"
>>>>
>>>>    typedef struct FILTERBUFFERState {
>>>>        NetFilterState nf;
>>>>        NetClientState dummy; /* used to send buffered packets */
>>>>        NetQueue *incoming_queue;
>>>>        NetQueue *inflight_queue;
>>>> +    QEMUBH *flush_bh;
>>>
>>> bh should be stopped and restarted during vm stop and continue.
>>
>> Sorry, could you explain more about this? do you mean to check
>> the vm state before bh_schedule?
>> how to stop&restart a bh? bh_delete bh_new bh_schedule?
>>
>>>
>>>>    } FILTERBUFFERState;
>>>>
>>>>    static void packet_send_completed(NetClientState *nc, ssize_t len)
>>>> @@ -56,6 +58,27 @@ static void filter_buffer_flush(NetFilterState *nf)
>>>>        }
>>>>    }
>>>>
>>>> +static void filter_buffer_flush_bh(void *opaque)
>>>> +{
>>>> +    FILTERBUFFERState *s = opaque;
>> [...]
>>>> +
>>>> +/* public APIs */
>>>> +void filter_buffer_release_all(void)
>>>> +{
>>>> +    NetFilterState *nfs[MAX_QUEUE_NUM];
>>>> +    int queues, i;
>>>> +
>>>> +    queues = qemu_find_netfilters_by_model("buffer", nfs,
>>>> MAX_QUEUE_NUM);
>>>> +
>>>> +    for (i = 0; i < queues; i++) {
>>>> +        filter_buffer_release_one(nfs[i]);
>>>> +    }
>>>> +}
>>>
>>> Looks like the function was never used by following patches?
>>
>> Right, it's not used in this series. But it can be used by MC to
>> release packets
>> at checkpoint, Should I mark this unused, or drop this API?
>
> Please drop this and re-introduce them which it has users.

Ok.

>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 10/12] netbuffer: add a public api filter_buffer_release_all

[Jason Wang]

On 07/30/2015 01:50 PM, Yang Hongyang wrote:
> On 07/30/2015 01:25 PM, Jason Wang wrote:
>>
>>
>> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>>> add a public api filter_buffer_release_all to release all
>>> buffered packets.
>>> also introduce qemu_find_netfilters_by_model to find all buffer
>>> filters.
>>>
>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>> ---
>>>   include/net/filter.h |  5 +++++
>>>   net/filter-buffer.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>>>   net/filter.c         | 18 ++++++++++++++++++
>>>   3 files changed, 64 insertions(+)
>>>
>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>> index 5292563..798b5b2 100644
>>> --- a/include/net/filter.h
>>> +++ b/include/net/filter.h
>>> @@ -50,5 +50,10 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo
>>> *info,
>>>                                       const char *name);
>>>   void netfilter_add(QemuOpts *opts, Error **errp);
>>>   void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
>>> +int qemu_find_netfilters_by_model(const char *model, NetFilterState
>>> **nfs,
>>> +                                  int max);
>>> +
>>> +/* netbuffer filter */
>>> +void filter_buffer_release_all(void);
>>>
>>>   #endif /* QEMU_NET_FILTER_H */
>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>>> index 628e66f..8bac73b 100644
>>> --- a/net/filter-buffer.c
>>> +++ b/net/filter-buffer.c
>>> @@ -11,12 +11,14 @@
>>>   #include "filters.h"
>>>   #include "qemu-common.h"
>>>   #include "qemu/error-report.h"
>>> +#include "qemu/main-loop.h"
>>>
>>>   typedef struct FILTERBUFFERState {
>>>       NetFilterState nf;
>>>       NetClientState dummy; /* used to send buffered packets */
>>>       NetQueue *incoming_queue;
>>>       NetQueue *inflight_queue;
>>> +    QEMUBH *flush_bh;
>>
>> bh should be stopped and restarted during vm stop and continue.
>
> Sorry, could you explain more about this? do you mean to check
> the vm state before bh_schedule?

Rethink about this. Looks safe since qemu_can_send_packet() check vmstate.

> how to stop&restart a bh? bh_delete bh_new bh_schedule?

qemu_bh_cancel() and qemu_bh_schedule().

Re: [Qemu-devel] [PATCH 10/12] netbuffer: add a public api filter_buffer_release_all

[Yang Hongyang]

On 07/30/2015 04:50 PM, Jason Wang wrote:
>
>
> On 07/30/2015 01:50 PM, Yang Hongyang wrote:
>> On 07/30/2015 01:25 PM, Jason Wang wrote:
>>>
>>>
>>> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>>>> add a public api filter_buffer_release_all to release all
>>>> buffered packets.
>>>> also introduce qemu_find_netfilters_by_model to find all buffer
>>>> filters.
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>> ---
>>>>    include/net/filter.h |  5 +++++
>>>>    net/filter-buffer.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>>>>    net/filter.c         | 18 ++++++++++++++++++
>>>>    3 files changed, 64 insertions(+)
>>>>
>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>> index 5292563..798b5b2 100644
>>>> --- a/include/net/filter.h
>>>> +++ b/include/net/filter.h
>>>> @@ -50,5 +50,10 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo
>>>> *info,
>>>>                                        const char *name);
>>>>    void netfilter_add(QemuOpts *opts, Error **errp);
>>>>    void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
>>>> +int qemu_find_netfilters_by_model(const char *model, NetFilterState
>>>> **nfs,
>>>> +                                  int max);
>>>> +
>>>> +/* netbuffer filter */
>>>> +void filter_buffer_release_all(void);
>>>>
>>>>    #endif /* QEMU_NET_FILTER_H */
>>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>>>> index 628e66f..8bac73b 100644
>>>> --- a/net/filter-buffer.c
>>>> +++ b/net/filter-buffer.c
>>>> @@ -11,12 +11,14 @@
>>>>    #include "filters.h"
>>>>    #include "qemu-common.h"
>>>>    #include "qemu/error-report.h"
>>>> +#include "qemu/main-loop.h"
>>>>
>>>>    typedef struct FILTERBUFFERState {
>>>>        NetFilterState nf;
>>>>        NetClientState dummy; /* used to send buffered packets */
>>>>        NetQueue *incoming_queue;
>>>>        NetQueue *inflight_queue;
>>>> +    QEMUBH *flush_bh;
>>>
>>> bh should be stopped and restarted during vm stop and continue.
>>
>> Sorry, could you explain more about this? do you mean to check
>> the vm state before bh_schedule?
>
> Rethink about this. Looks safe since qemu_can_send_packet() check vmstate.
>
>> how to stop&restart a bh? bh_delete bh_new bh_schedule?
>
> qemu_bh_cancel() and qemu_bh_schedule().

Got it, Thank you!

>
> .
>

-- 
Thanks,
Yang.

[Qemu-devel] [PATCH 11/12] filter/buffer: add an interval option to buffer filter

[Yang Hongyang]

the buffer filter will release packets by interval.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 net/filter-buffer.c | 24 ++++++++++++++++++++++++
 qapi-schema.json    |  7 ++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index 8bac73b..902d9f7 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -12,6 +12,7 @@
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
+#include "qemu/timer.h"
 
 typedef struct FILTERBUFFERState {
     NetFilterState nf;
@@ -19,6 +20,8 @@ typedef struct FILTERBUFFERState {
     NetQueue *incoming_queue;
     NetQueue *inflight_queue;
     QEMUBH *flush_bh;
+    int64_t interval;
+    QEMUTimer release_timer;
 } FILTERBUFFERState;
 
 static void packet_send_completed(NetClientState *nc, ssize_t len)
@@ -79,6 +82,14 @@ static void filter_buffer_release_one(NetFilterState *nf)
     qemu_bh_schedule(s->flush_bh);
 }
 
+static void filter_buffer_release_timer(void *opaque)
+{
+    FILTERBUFFERState *s = opaque;
+    filter_buffer_release_one(&s->nf);
+    timer_mod(&s->release_timer,
+              qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
+}
+
 /* filter APIs */
 static ssize_t filter_buffer_receive(NetFilterState *nf,
                                      NetClientState *sender,
@@ -109,6 +120,10 @@ static void filter_buffer_cleanup(NetFilterState *nf)
 {
     FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
 
+    if (s->interval) {
+        timer_del(&s->release_timer);
+    }
+
     /* flush inflight packets */
     filter_buffer_flush(nf);
     /* flush incoming packets */
@@ -136,8 +151,10 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
 {
     NetFilterState *nf;
     FILTERBUFFERState *s;
+    const NetFilterBufferOptions *bufferopt;
 
     assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
+    bufferopt = opts->buffer;
 
     nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, "buffer", name);
     s = DO_UPCAST(FILTERBUFFERState, nf, nf);
@@ -150,6 +167,13 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
     s->dummy.peer = netdev;
     s->incoming_queue = qemu_new_net_queue(nf);
     s->flush_bh = qemu_bh_new(filter_buffer_flush_bh, s);
+    s->interval = bufferopt->has_interval ? bufferopt->interval : 0;
+    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/qapi-schema.json b/qapi-schema.json
index 67e00a0..45b357d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2581,10 +2581,15 @@
 #
 # a netbuffer filter for network backend.
 #
+# @interval: #optional release packets by interval, if no interval supplied,
+#            will release packets when filter_buffer_release_all been called.
+#            scale: microsecond
+#
 # Since 2.5
 ##
 { 'struct': 'NetFilterBufferOptions',
-  'data': { } }
+  'data': {
+    '*interval':     'int64' } }
 
 ##
 # @NetFilterOptions
-- 
1.9.1

Re: [Qemu-devel] [PATCH 11/12] filter/buffer: add an interval option to buffer filter

[Jason Wang]

On 07/29/2015 06:51 PM, Yang Hongyang wrote:
> the buffer filter will release packets by interval.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

Looks liked it's better to squash this patch into the buffer? And should
we stop the timer during vm stop?

> ---
>  net/filter-buffer.c | 24 ++++++++++++++++++++++++
>  qapi-schema.json    |  7 ++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
> index 8bac73b..902d9f7 100644
> --- a/net/filter-buffer.c
> +++ b/net/filter-buffer.c
> @@ -12,6 +12,7 @@
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
> +#include "qemu/timer.h"
>  
>  typedef struct FILTERBUFFERState {
>      NetFilterState nf;
> @@ -19,6 +20,8 @@ typedef struct FILTERBUFFERState {
>      NetQueue *incoming_queue;
>      NetQueue *inflight_queue;
>      QEMUBH *flush_bh;
> +    int64_t interval;
> +    QEMUTimer release_timer;
>  } FILTERBUFFERState;
>  
>  static void packet_send_completed(NetClientState *nc, ssize_t len)
> @@ -79,6 +82,14 @@ static void filter_buffer_release_one(NetFilterState *nf)
>      qemu_bh_schedule(s->flush_bh);
>  }
>  
> +static void filter_buffer_release_timer(void *opaque)
> +{
> +    FILTERBUFFERState *s = opaque;
> +    filter_buffer_release_one(&s->nf);
> +    timer_mod(&s->release_timer,
> +              qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
> +}
> +
>  /* filter APIs */
>  static ssize_t filter_buffer_receive(NetFilterState *nf,
>                                       NetClientState *sender,
> @@ -109,6 +120,10 @@ static void filter_buffer_cleanup(NetFilterState *nf)
>  {
>      FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>  
> +    if (s->interval) {
> +        timer_del(&s->release_timer);
> +    }
> +
>      /* flush inflight packets */
>      filter_buffer_flush(nf);
>      /* flush incoming packets */
> @@ -136,8 +151,10 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
>  {
>      NetFilterState *nf;
>      FILTERBUFFERState *s;
> +    const NetFilterBufferOptions *bufferopt;
>  
>      assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
> +    bufferopt = opts->buffer;
>  
>      nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, "buffer", name);
>      s = DO_UPCAST(FILTERBUFFERState, nf, nf);
> @@ -150,6 +167,13 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
>      s->dummy.peer = netdev;
>      s->incoming_queue = qemu_new_net_queue(nf);
>      s->flush_bh = qemu_bh_new(filter_buffer_flush_bh, s);
> +    s->interval = bufferopt->has_interval ? bufferopt->interval : 0;
> +    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/qapi-schema.json b/qapi-schema.json
> index 67e00a0..45b357d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2581,10 +2581,15 @@
>  #
>  # a netbuffer filter for network backend.
>  #
> +# @interval: #optional release packets by interval, if no interval supplied,
> +#            will release packets when filter_buffer_release_all been called.
> +#            scale: microsecond
> +#
>  # Since 2.5
>  ##
>  { 'struct': 'NetFilterBufferOptions',
> -  'data': { } }
> +  'data': {
> +    '*interval':     'int64' } }
>  
>  ##
>  # @NetFilterOptions

Re: [Qemu-devel] [PATCH 11/12] filter/buffer: add an interval option to buffer filter

[Yang Hongyang]

Hi Jason,

   Thank you for review!

On 07/30/2015 01:27 PM, Jason Wang wrote:
>
>
> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>> the buffer filter will release packets by interval.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>
> Looks liked it's better to squash this patch into the buffer?

I don't know if it's better...but if it brings inconvenience to the
reviewers, I will squash it in the next version.

> And should
> we stop the timer during vm stop?

The timer group is QEMU_CLOCK_VIRTUAL, so the timer should be automatically
stopped during vm stop.

  * @QEMU_CLOCK_VIRTUAL: virtual clock
  *
  * The virtual clock is only run during the emulation. It is stopped
  * when the virtual machine is stopped. Virtual timers use a high
  * precision clock, usually cpu cycles (use ticks_per_sec).

>
>> ---
>>   net/filter-buffer.c | 24 ++++++++++++++++++++++++
>>   qapi-schema.json    |  7 ++++++-
>>   2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>> index 8bac73b..902d9f7 100644
>> --- a/net/filter-buffer.c
>> +++ b/net/filter-buffer.c
>> @@ -12,6 +12,7 @@
>>   #include "qemu-common.h"
>>   #include "qemu/error-report.h"
>>   #include "qemu/main-loop.h"
>> +#include "qemu/timer.h"
>>
>>   typedef struct FILTERBUFFERState {
>>       NetFilterState nf;
>> @@ -19,6 +20,8 @@ typedef struct FILTERBUFFERState {
>>       NetQueue *incoming_queue;
>>       NetQueue *inflight_queue;
>>       QEMUBH *flush_bh;
>> +    int64_t interval;
>> +    QEMUTimer release_timer;
>>   } FILTERBUFFERState;
>>
>>   static void packet_send_completed(NetClientState *nc, ssize_t len)
>> @@ -79,6 +82,14 @@ static void filter_buffer_release_one(NetFilterState *nf)
>>       qemu_bh_schedule(s->flush_bh);
>>   }
>>
>> +static void filter_buffer_release_timer(void *opaque)
>> +{
>> +    FILTERBUFFERState *s = opaque;
>> +    filter_buffer_release_one(&s->nf);
>> +    timer_mod(&s->release_timer,
>> +              qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
>> +}
>> +
>>   /* filter APIs */
>>   static ssize_t filter_buffer_receive(NetFilterState *nf,
>>                                        NetClientState *sender,
>> @@ -109,6 +120,10 @@ static void filter_buffer_cleanup(NetFilterState *nf)
>>   {
>>       FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>>
>> +    if (s->interval) {
>> +        timer_del(&s->release_timer);
>> +    }
>> +
>>       /* flush inflight packets */
>>       filter_buffer_flush(nf);
>>       /* flush incoming packets */
>> @@ -136,8 +151,10 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
>>   {
>>       NetFilterState *nf;
>>       FILTERBUFFERState *s;
>> +    const NetFilterBufferOptions *bufferopt;
>>
>>       assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
>> +    bufferopt = opts->buffer;
>>
>>       nf = qemu_new_net_filter(&net_filter_buffer_info, netdev, "buffer", name);
>>       s = DO_UPCAST(FILTERBUFFERState, nf, nf);
>> @@ -150,6 +167,13 @@ int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
>>       s->dummy.peer = netdev;
>>       s->incoming_queue = qemu_new_net_queue(nf);
>>       s->flush_bh = qemu_bh_new(filter_buffer_flush_bh, s);
>> +    s->interval = bufferopt->has_interval ? bufferopt->interval : 0;
>> +    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/qapi-schema.json b/qapi-schema.json
>> index 67e00a0..45b357d 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2581,10 +2581,15 @@
>>   #
>>   # a netbuffer filter for network backend.
>>   #
>> +# @interval: #optional release packets by interval, if no interval supplied,
>> +#            will release packets when filter_buffer_release_all been called.
>> +#            scale: microsecond
>> +#
>>   # Since 2.5
>>   ##
>>   { 'struct': 'NetFilterBufferOptions',
>> -  'data': { } }
>> +  'data': {
>> +    '*interval':     'int64' } }
>>
>>   ##
>>   # @NetFilterOptions
>
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 11/12] filter/buffer: add an interval option to buffer filter

[Jason Wang]

On 07/30/2015 01:37 PM, Yang Hongyang wrote:
> Hi Jason,
>
>   Thank you for review!
>
> On 07/30/2015 01:27 PM, Jason Wang wrote:
>>
>>
>> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>>> the buffer filter will release packets by interval.
>>>
>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>
>> Looks liked it's better to squash this patch into the buffer?
>
> I don't know if it's better...but if it brings inconvenience to the
> reviewers, I will squash it in the next version.
>

Please do this.

>> And should
>> we stop the timer during vm stop?
>
> The timer group is QEMU_CLOCK_VIRTUAL, so the timer should be
> automatically
> stopped during vm stop.
>
>  * @QEMU_CLOCK_VIRTUAL: virtual clock
>  *
>  * The virtual clock is only run during the emulation. It is stopped
>  * when the virtual machine is stopped. Virtual timers use a high
>  * precision clock, usually cpu cycles (use ticks_per_sec).
>

I see, but I don't get why it should be a virtual clock? It has nothing
to do with guest.

Re: [Qemu-devel] [PATCH 11/12] filter/buffer: add an interval option to buffer filter

[Yang Hongyang]

On 07/30/2015 04:53 PM, Jason Wang wrote:
>
>
> On 07/30/2015 01:37 PM, Yang Hongyang wrote:
>> Hi Jason,
>>
>>    Thank you for review!
>>
>> On 07/30/2015 01:27 PM, Jason Wang wrote:
>>>
>>>
>>> On 07/29/2015 06:51 PM, Yang Hongyang wrote:
>>>> the buffer filter will release packets by interval.
>>>>
>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>
>>> Looks liked it's better to squash this patch into the buffer?
>>
>> I don't know if it's better...but if it brings inconvenience to the
>> reviewers, I will squash it in the next version.
>>
>
> Please do this.

Ok.

>
>>> And should
>>> we stop the timer during vm stop?
>>
>> The timer group is QEMU_CLOCK_VIRTUAL, so the timer should be
>> automatically
>> stopped during vm stop.
>>
>>   * @QEMU_CLOCK_VIRTUAL: virtual clock
>>   *
>>   * The virtual clock is only run during the emulation. It is stopped
>>   * when the virtual machine is stopped. Virtual timers use a high
>>   * precision clock, usually cpu cycles (use ticks_per_sec).
>>
>
> I see, but I don't get why it should be a virtual clock? It has nothing
> to do with guest.

It has, it buffers the guest output packets, if the guest is stopped, there
should be no packets sent from guest. So make the timer a virtual clock
ought to be reasonable...

>
> .
>

-- 
Thanks,
Yang.

[Qemu-devel] [PATCH 12/12] filter/buffer: update command description and help

[Yang Hongyang]

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
---
 hmp-commands.hx | 2 +-
 qemu-options.hx | 5 ++++-
 qmp-commands.hx | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 5326a82..d678ebd 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1255,7 +1255,7 @@ ETEXI
     {
         .name       = "netfilter_add",
         .args_type  = "netfilter:O",
-        .params     = "[type],id=str,netdev=str[,prop=value][,...]",
+        .params     = "[buffer],id=str,netdev=str[,prop=value][,...]",
         .help       = "add netfilter",
         .mhandler.cmd = hmp_netfilter_add,
         .command_completion = netfilter_add_completion,
diff --git a/qemu-options.hx b/qemu-options.hx
index 8c1eb30..6a90521 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,7 +1575,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "socket][,vlan=n][,option][,option][,...]\n"
     "                old way to initialize a host network interface\n"
     "                (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
-DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
+DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter,
+    "-netfilter buffer,id=str,netdev=str[,interval=n]\n"
+    "                buffer guest output packets. if interval provided, will release\n"
+    "                packets by interval. interval scale: microsecond\n", QEMU_ARCH_ALL)
 STEXI
 @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
 @findex -net
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9e62e0b..1135320 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -947,7 +947,7 @@ Arguments:
 Example:
 
 -> { "execute": "netfilter_add",
-                "arguments": { "type": "type", "id": "nf0", "netdev": "bn" } }
+                "arguments": { "type": "buffer", "id": "nf0", "netdev": "bn" } }
 <- { "return": {} }
 
 Note: The supported filter options are the same ones supported by the
-- 
1.9.1

Re: [Qemu-devel] [PATCH 00/12] For QEMU 2.5: Add a netfilter object and netbuffer filter

[Thomas Huth]

On Wednesday, July 29, 2015 12:51:44 PM,
"Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> 
> I do not take this as v2 because the design of this has been changed,
> and code is rewritten. according to thread:
> http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg05445.html
> 
> This patch add a new object netfilter, capture all network packets.
> Also implement a netbuffer based on this object.
> the "buffer" netfilter could be used by VM FT solutions like
> Macrocheckpointing, to buffer/release packets. Or to simulate
> packet delay.
> 
> Usage:
>  -netdev tap,id=bn0
>  -netfilter buffer,id=f0,netdev=bn0,interval=1000
>  -device e1000,netdev=bn0

What if I want to use multiple filters at once? Is there a way to
determine the order in which the filters are run through?

> dynamically add/remove netfilters:
>  netfilter_add buffer,id=f0,netdev=bn0,interval=1000
>  netfilter_del f0

Same here ... let's assume there are already two filters - can I
somehow add a third filter inbetween the two?

 Thomas

Re: [Qemu-devel] [PATCH 00/12] For QEMU 2.5: Add a netfilter object and netbuffer filter

[Yang Hongyang]

Hi Thomas,

On 07/29/2015 08:56 PM, Thomas Huth wrote:
> On Wednesday, July 29, 2015 12:51:44 PM,
> "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
>>
>> I do not take this as v2 because the design of this has been changed,
>> and code is rewritten. according to thread:
>> http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg05445.html
>>
>> This patch add a new object netfilter, capture all network packets.
>> Also implement a netbuffer based on this object.
>> the "buffer" netfilter could be used by VM FT solutions like
>> Macrocheckpointing, to buffer/release packets. Or to simulate
>> packet delay.
>>
>> Usage:
>>   -netdev tap,id=bn0
>>   -netfilter buffer,id=f0,netdev=bn0,interval=1000
>>   -device e1000,netdev=bn0
>
> What if I want to use multiple filters at once? Is there a way to
> determine the order in which the filters are run through?

The filters are in an QTAILQ list, so it's order is what appears
first will be first. for example:
   -netfilter dump,...
   -netfilter buffer,...

dump will be the first filter.

>
>> dynamically add/remove netfilters:
>>   netfilter_add buffer,id=f0,netdev=bn0,interval=1000
>>   netfilter_del f0
>
> Same here ... let's assume there are already two filters - can I
> somehow add a third filter inbetween the two?

It's not possiable in this series, but it can be supported later,
we can add an order option to the netfilter, so that it can be
the order whatever you want.

>
>   Thomas
> .
>

-- 
Thanks,
Yang.

Re: [Qemu-devel] [PATCH 00/12] For QEMU 2.5: Add a netfilter object and netbuffer filter

[Thomas Huth]

On Wednesday, July 29, 2015 3:39:45 PM,
"Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> 
> Hi Thomas,
> 
> On 07/29/2015 08:56 PM, Thomas Huth wrote:
> > On Wednesday, July 29, 2015 12:51:44 PM,
> > "Yang Hongyang" <yanghy@cn.fujitsu.com> wrote:
> >>
> >> I do not take this as v2 because the design of this has been changed,
> >> and code is rewritten. according to thread:
> >> http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg05445.html
> >>
> >> This patch add a new object netfilter, capture all network packets.
> >> Also implement a netbuffer based on this object.
> >> the "buffer" netfilter could be used by VM FT solutions like
> >> Macrocheckpointing, to buffer/release packets. Or to simulate
> >> packet delay.
> >>
> >> Usage:
> >>   -netdev tap,id=bn0
> >>   -netfilter buffer,id=f0,netdev=bn0,interval=1000
> >>   -device e1000,netdev=bn0
> >
> > What if I want to use multiple filters at once? Is there a way to
> > determine the order in which the filters are run through?
> 
> The filters are in an QTAILQ list, so it's order is what appears
> first will be first. for example:
>    -netfilter dump,...
>    -netfilter buffer,...
> 
> dump will be the first filter.
> 
> >
> >> dynamically add/remove netfilters:
> >>   netfilter_add buffer,id=f0,netdev=bn0,interval=1000
> >>   netfilter_del f0
> >
> > Same here ... let's assume there are already two filters - can I
> > somehow add a third filter inbetween the two?
> 
> It's not possiable in this series, but it can be supported later,
> we can add an order option to the netfilter, so that it can be
> the order whatever you want.

Ok, thanks for the clarification. You're right, that's something
that also can be added later if needed.

 Thomas