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

[Yang Hongyang <yanghy@cn.fujitsu.com>]

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.