From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34253) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWEqI-00080k-Ph for qemu-devel@nongnu.org; Wed, 17 Feb 2016 22:01:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWEqE-00027B-L9 for qemu-devel@nongnu.org; Wed, 17 Feb 2016 22:01:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51146) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWEqE-00026s-DP for qemu-devel@nongnu.org; Wed, 17 Feb 2016 22:01:18 -0500 References: <1454750932-7556-1-git-send-email-zhang.zhanghailiang@huawei.com> <1454750932-7556-32-git-send-email-zhang.zhanghailiang@huawei.com> From: Jason Wang Message-ID: <56C533D3.5020307@redhat.com> Date: Thu, 18 Feb 2016 11:00:35 +0800 MIME-Version: 1.0 In-Reply-To: <1454750932-7556-32-git-send-email-zhang.zhanghailiang@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v14 31/40] net/filter: Add a 'status' property for filter object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: zhanghailiang , qemu-devel@nongnu.org Cc: xiecl.fnst@cn.fujitsu.com, lizhijian@cn.fujitsu.com, quintela@redhat.com, armbru@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, dgilbert@redhat.com, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com, zhangchen.fnst@cn.fujitsu.com, hongyang.yang@easystack.cn On 02/06/2016 05:28 PM, zhanghailiang wrote: > With this property, users can control if this filter is 'enable' > or 'disable'. The default behavior for filter is enabled. > > We will skip the disabled filter when delivering packets in net layer. > > Signed-off-by: zhanghailiang > Cc: Jason Wang > Cc: Yang Hongyang > --- > include/net/filter.h | 1 + > net/filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/include/net/filter.h b/include/net/filter.h > index 5639976..af3c53c 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -55,6 +55,7 @@ struct NetFilterState { > char *netdev_id; > NetClientState *netdev; > NetFilterDirection direction; > + bool enabled; > QTAILQ_ENTRY(NetFilterState) next; > }; > > diff --git a/net/filter.c b/net/filter.c > index d2a514e..5551cf1 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -17,6 +17,11 @@ > #include "qom/object_interfaces.h" > #include "qemu/iov.h" > > +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) > +{ > + return nf->enabled ? false : true; > +} Suggest to rename this to qemu_netfilter_can_skip. > + > ssize_t qemu_netfilter_receive(NetFilterState *nf, > NetFilterDirection direction, > NetClientState *sender, > @@ -25,6 +30,10 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, > int iovcnt, > NetPacketSent *sent_cb) > { > + /* Don't go through the filter if it is disabled */ > + if (qemu_need_skip_netfilter(nf)) { > + return 0; > + } The code is self explained, so the commnet is useless. > if (nf->direction == direction || > nf->direction == NET_FILTER_DIRECTION_ALL) { > return NETFILTER_GET_CLASS(OBJECT(nf))->receive_iov( > @@ -134,8 +143,41 @@ static void netfilter_set_direction(Object *obj, int direction, Error **errp) > nf->direction = direction; > } > > +static char *netfilter_get_status(Object *obj, Error **errp) > +{ > + NetFilterState *nf = NETFILTER(obj); > + > + if (nf->enabled) { > + return g_strdup("enable"); > + } else { > + return g_strdup("disable"); > + } > +} > + > +static void netfilter_set_status(Object *obj, const char *str, Error **errp) > +{ > + NetFilterState *nf = NETFILTER(obj); > + > + if (!strcmp(str, "enable")) { > + nf->enabled = true; > + } else if (!strcmp(str, "disable")) { > + nf->enabled = false; Do we need a filter specific callback here to drain filter's queue? E.g for filter-buffer ,need to release all the packets that has been buffered. > + } else { > + error_setg(errp, "Invalid value for netfilter status, " > + "should be 'enable' or 'disable'"); > + } > +} > + > static void netfilter_init(Object *obj) > { > + NetFilterState *nf = NETFILTER(obj); > + > + /* > + * If not configured with 'status' property, the default status > + * for netfilter will be enabled. > + */ > + nf->enabled = true; The code is clear too, so the comment need to be moved to qemu-options.hx. > + > object_property_add_str(obj, "netdev", > netfilter_get_netdev_id, netfilter_set_netdev_id, > NULL); > @@ -143,6 +185,9 @@ static void netfilter_init(Object *obj) > NetFilterDirection_lookup, > netfilter_get_direction, netfilter_set_direction, > NULL); > + object_property_add_str(obj, "status", > + netfilter_get_status, netfilter_set_status, > + NULL); > } > > static void netfilter_complete(UserCreatable *uc, Error **errp)