From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53779) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZfHZW-0002U1-Mp for qemu-devel@nongnu.org; Thu, 24 Sep 2015 21:13:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZfHZV-0000Pe-3z for qemu-devel@nongnu.org; Thu, 24 Sep 2015 21:13:10 -0400 Received: from [59.151.112.132] (port=38383 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZfHZU-0000PF-4g for qemu-devel@nongnu.org; Thu, 24 Sep 2015 21:13:09 -0400 Message-ID: <56049F9C.5090900@cn.fujitsu.com> Date: Fri, 25 Sep 2015 09:13:00 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <1442405768-23019-1-git-send-email-yanghy@cn.fujitsu.com> <1442405768-23019-3-git-send-email-yanghy@cn.fujitsu.com> <87oagsryyg.fsf@blackfin.pond.sub.org> <5603B888.7060107@cn.fujitsu.com> <8737y4nizg.fsf@blackfin.pond.sub.org> In-Reply-To: <8737y4nizg.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v11 02/12] init/cleanup of netfilter object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: thuth@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, jasowang@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On 09/24/2015 07:40 PM, Markus Armbruster wrote: > Yang Hongyang writes: > >> On 09/24/2015 04:41 PM, Markus Armbruster wrote: >>> Yang Hongyang writes: >>> >>>> Add a netfilter object based on QOM. >>>> >>>> A netfilter is attached to a netdev, captures all network packets >>>> that pass through the netdev. When we delete the netdev, we also >>>> delete the netfilter object attached to it, because if the netdev is >>>> removed, the filter which attached to it is useless. >>>> >>>> QTAILQ_ENTRY next used by netdev, filter belongs to the specific netdev is >>>> in this queue. >>> >>> I don't get this paragraph. Not sure it's needed. >>> >>>> Also init delayed object after net_init_clients, because netfilters need >>>> to be initialized after net clients initialized. >>> >>> A paragraph starting with "Also" in a commit message is a pretty good >>> sign the patch should be split :) >>> >>>> >>>> Signed-off-by: Yang Hongyang >>>> --- >>>> v11: no need to free nf->netdev_id, it will be auto freeed while object deleted >>>> remove global_list net_filters, will add back when needed >>>> v10: use QOM for netfilter >>>> v9: use flat union instead of simple union in QAPI schema >>>> v8: include vhost_net header >>>> v7: add check for vhost >>>> fix error propagate bug >>>> v6: add multiqueue support (net_filter_init1) >>>> v5: remove model from NetFilterState >>>> add a sent_cb param to receive_iov API >>>> --- >>>> include/net/filter.h | 60 +++++++++++++++++++++ >>>> include/net/net.h | 1 + >>>> include/qemu/typedefs.h | 1 + >>>> net/Makefile.objs | 1 + >>>> net/filter.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++ >>>> net/net.c | 7 +++ >>>> qapi-schema.json | 18 +++++++ >>>> vl.c | 13 ++--- >>>> 8 files changed, 233 insertions(+), 6 deletions(-) >>>> 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..226f2f7 >>>> --- /dev/null >>>> +++ b/include/net/filter.h >>>> @@ -0,0 +1,60 @@ >>>> +/* >>>> + * Copyright (c) 2015 FUJITSU LIMITED >>>> + * Author: Yang Hongyang >>>> + * >>>> + * 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 "qom/object.h" >>>> +#include "qemu-common.h" >>>> +#include "qemu/typedefs.h" >>>> +#include "net/queue.h" >>>> + >>>> +#define TYPE_NETFILTER "netfilter" >>>> +#define NETFILTER(obj) \ >>>> + OBJECT_CHECK(NetFilterState, (obj), TYPE_NETFILTER) >>>> +#define NETFILTER_GET_CLASS(obj) \ >>>> + OBJECT_GET_CLASS(NetFilterClass, (obj), TYPE_NETFILTER) >>>> +#define NETFILTER_CLASS(klass) \ >>>> + OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER) >>>> + >>>> +typedef void (FilterSetup) (NetFilterState *nf, Error **errp); >>>> +typedef void (FilterCleanup) (NetFilterState *nf); >>>> +/* >>>> + * Return: >>>> + * 0: finished handling the packet, we should continue >>>> + * size: filter stolen this packet, we stop pass this packet further >>>> + */ >>>> +typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, >>>> + NetClientState *sender, >>>> + unsigned flags, >>>> + const struct iovec *iov, >>>> + int iovcnt, >>>> + NetPacketSent *sent_cb); >>>> + >>>> +struct NetFilterClass { >>>> + ObjectClass parent_class; >>>> + >>>> + FilterSetup *setup; >>>> + FilterCleanup *cleanup; >>>> + FilterReceiveIOV *receive_iov; >>>> +}; >>>> +typedef struct NetFilterClass NetFilterClass; >>> >>> Not splitting the declaration is more concise: >>> >>> typedef struct { >>> ObjectClass parent_class; >>> FilterSetup *setup; >>> FilterCleanup *cleanup; >>> FilterReceiveIOV *receive_iov; >>> } NetFilterClass; >>> >>> Are any of the methods optional? If yes, please add suitable comments. >> >> Hi Markus, I split it because the checkpatch.pl told me to do so... > > Understand. However, it's a recent change to checkpatch.pl that's going > to be reverted: > Message-ID: <55FAEB33.50809@redhat.com> > http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg04644.html Thanks for the information. > > [...] > . > -- Thanks, Yang.