From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNG0B-0004ZK-Q0 for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:54:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNG06-0004Fi-6p for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:54:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35688) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNG05-0004Fb-Vy for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:54:06 -0400 Message-ID: <55C31296.4030905@redhat.com> Date: Thu, 06 Aug 2015 15:53:58 +0800 From: Jason Wang MIME-Version: 1.0 References: <1438677044-13030-1-git-send-email-yanghy@cn.fujitsu.com> <1438677044-13030-3-git-send-email-yanghy@cn.fujitsu.com> <55C307B5.8060201@redhat.com> <55C30B1D.1070304@cn.fujitsu.com> <55C30CBD.9080705@redhat.com> <55C30E3C.3010106@cn.fujitsu.com> <55C3103A.9030605@redhat.com> <55C311D3.8010305@cn.fujitsu.com> In-Reply-To: <55C311D3.8010305@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 02/11] init/cleanup of netfilter object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yang Hongyang , qemu-devel@nongnu.org Cc: thuth@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, eddie.dong@intel.com, mrhines@linux.vnet.ibm.com, stefanha@redhat.com On 08/06/2015 03:50 PM, Yang Hongyang wrote: > On 08/06/2015 03:43 PM, Jason Wang wrote: >> On 08/06/2015 03:35 PM, Yang Hongyang wrote: >>> On 08/06/2015 03:29 PM, Jason Wang wrote: >>>> >>>> >>>> On 08/06/2015 03:22 PM, Yang Hongyang wrote: >>>>> On 08/06/2015 03:07 PM, Jason Wang wrote: >>>>>> >>>>>> >>>>>> On 08/04/2015 04:30 PM, Yang Hongyang wrote: >>>>>>> QTAILQ_ENTRY global_list but used by filter layer, so that we can >>>>>>> manage all filters together. >>>>>>> QTAILQ_ENTRY next used by netdev, filter belongs to the specific >>>>>>> netdev is >>>>>>> in this queue. >>>>>>> This is mostly the same with init/cleanup of netdev object. >>>>>>> >>>>>>> Signed-off-by: Yang Hongyang >>>>>>> --- >>>>>>> include/net/filter.h | 39 +++++++++++++ >>>>>>> include/net/net.h | 1 + >>>>>>> include/qemu/typedefs.h | 1 + >>>>>>> net/filter.c | 147 >>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>> net/net.c | 1 + >>>>>>> qapi-schema.json | 37 ++++++++++++ >>>>>>> 6 files changed, 226 insertions(+) >>>>>>> >>>>>>> diff --git a/include/net/filter.h b/include/net/filter.h >>>>>>> index 4242ded..9aafe08 100644 >>>>>>> --- a/include/net/filter.h >>>>>>> +++ b/include/net/filter.h >>>>>>> @@ -9,7 +9,46 @@ >>>>>>> #define QEMU_NET_FILTER_H >>>>>>> >>>>>>> #include "qemu-common.h" >>>>>>> +#include "qemu/typedefs.h" >>>>>>> + >>>>>>> +/* the netfilter chain */ >>>>>>> +enum { >>>>>>> + NET_FILTER_IN, >>>>>>> + NET_FILTER_OUT, >>>>>>> + NET_FILTER_ALL, >>>>>>> +}; >>>>>>> + >>>>>>> +typedef void (FilterCleanup) (NetFilterState *); >>>>>>> +/* >>>>>>> + * Return: >>>>>>> + * 0: finished handling the packet, we should continue >>>>>>> + * size: filter stolen this packet, we stop pass this packet >>>>>>> further >>>>>>> + */ >>>>>>> +typedef ssize_t (FilterReceiveIOV)(NetFilterState *, >>>>>>> NetClientState >>>>>>> *sender, >>>>>>> + unsigned flags, const struct >>>>>>> iovec *, int); >>>>>> >>>>>> Please name all parameters. >>>>> >>>>> Ok, thanks. >>>>> >>>>>> >>>>>>> + >>>>>>> +typedef struct NetFilterInfo { >>>>>>> + NetFilterOptionsKind type; >>>>>>> + size_t size; >>>>>>> + FilterCleanup *cleanup; >>>>>>> + FilterReceiveIOV *receive_iov; >>>>>>> +} NetFilterInfo; >>>>>>> + >>>>>>> +struct NetFilterState { >>>>>>> + NetFilterInfo *info; >>>>>>> + char *model; >>>>>> >>>>>> Looks like model is never used? >>>>> >>>>> It can be used when we want to find filters by model. For example, >>>>> when we need to find all "buffer" filter, and release all buffered >>>>> packets. >>>> >>>> But this is not implemented in this series. >>> >>> true. >>> >>>> And I don't get why you need >>>> a such command. Management should keep track of this. >>> >>> There's a patch in v1 which introduce an API use this, and I dropped >>> that >>> because there's no user currently, but that API I suppose to be used >>> by MC or COLO which can do periodic checkpoint, to release packets. >>> the API is filter_buffer_release_all() IIRC... >> >> I see, let's start with simple case to make review and merging easier. >> You can re-introduce them when it has real users. > > So do I need to remove model also? Yes, please.