From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38623) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWFGO-0007Ug-Kt for qemu-devel@nongnu.org; Wed, 17 Feb 2016 22:28:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWFGK-0006ec-QL for qemu-devel@nongnu.org; Wed, 17 Feb 2016 22:28:20 -0500 Received: from szxga01-in.huawei.com ([58.251.152.64]:11312) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWFGK-0006da-3k for qemu-devel@nongnu.org; Wed, 17 Feb 2016 22:28:16 -0500 References: <1454750932-7556-1-git-send-email-zhang.zhanghailiang@huawei.com> <1454750932-7556-32-git-send-email-zhang.zhanghailiang@huawei.com> <56C533D3.5020307@redhat.com> From: Hailiang Zhang Message-ID: <56C53A22.2010706@huawei.com> Date: Thu, 18 Feb 2016 11:27:30 +0800 MIME-Version: 1.0 In-Reply-To: <56C533D3.5020307@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed 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: Jason Wang , 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 Hi Jason, Happy spring festival! ;) On 2016/2/18 11:00, Jason Wang wrote: > > > 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. > OK, i will fix it. >> + >> 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. > OK, i will remove it. >> 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. > I don't think we need to do that here, we drain the filter's queue explicitly when we try to change the filter's status . Besides, it seems that, we didn't have a callback in filter layer to process the queued packets for different types of filters. >> + } 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. > OK, i will fix it in next version. Thanks, Hailiang >> + >> 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) > > > . >