From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46105) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNHGI-0005Ly-0s for qemu-devel@nongnu.org; Thu, 06 Aug 2015 05:14:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNHGG-00016I-SE for qemu-devel@nongnu.org; Thu, 06 Aug 2015 05:14:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33851) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNHGG-00016C-KJ for qemu-devel@nongnu.org; Thu, 06 Aug 2015 05:14:52 -0400 Message-ID: <55C32585.7010603@redhat.com> Date: Thu, 06 Aug 2015 17:14:45 +0800 From: Jason Wang MIME-Version: 1.0 References: <1438677044-13030-1-git-send-email-yanghy@cn.fujitsu.com> <1438677044-13030-5-git-send-email-yanghy@cn.fujitsu.com> <55C3083F.70904@redhat.com> <55C31E89.9030809@cn.fujitsu.com> In-Reply-To: <55C31E89.9030809@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 04/11] net: delete netfilter object when delete netdev 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 04:44 PM, Yang Hongyang wrote: > > > On 08/06/2015 03:09 PM, Jason Wang wrote: >> >> >> On 08/04/2015 04:30 PM, Yang Hongyang wrote: >>> When we delete the netdev, we also delete the netfilter object >>> attached to it, because if the netdev is removed, the filters >>> which attached to it is useless. >>> >>> Signed-off-by: Yang Hongyang >> >> Please squash this into patch 3. (otherwise you're fixing a bug >> introduced by patch 3). >> >>> --- >>> include/net/filter.h | 1 + >>> net/filter.c | 2 +- >>> net/net.c | 14 ++++++++++++++ >>> 3 files changed, 16 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/net/filter.h b/include/net/filter.h >>> index 8eff85a..c146be2 100644 >>> --- a/include/net/filter.h >>> +++ b/include/net/filter.h >>> @@ -50,6 +50,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo >>> *info, >>> const char *model, >>> const char *name, >>> int chain); >>> +void qemu_del_net_filter(NetFilterState *nf); >>> void netfilter_add(QemuOpts *opts, Error **errp); >>> void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp); >>> >>> diff --git a/net/filter.c b/net/filter.c >>> index 54a5c06..24ec4e1 100644 >>> --- a/net/filter.c >>> +++ b/net/filter.c >>> @@ -57,7 +57,7 @@ static void qemu_cleanup_net_filter(NetFilterState >>> *nf) >>> g_free(nf); >>> } >>> >>> -static void qemu_del_net_filter(NetFilterState *nf) >>> +void qemu_del_net_filter(NetFilterState *nf) >>> { >>> /* handle multi queue? */ >>> qemu_cleanup_net_filter(nf); >>> diff --git a/net/net.c b/net/net.c >>> index d9b70cd..03b2296 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -28,6 +28,7 @@ >>> #include "hub.h" >>> #include "net/slirp.h" >>> #include "net/eth.h" >>> +#include "net/filter.h" >>> #include "util.h" >>> >>> #include "monitor/monitor.h" >>> @@ -385,6 +386,8 @@ void qemu_del_net_client(NetClientState *nc) >>> { >>> NetClientState *ncs[MAX_QUEUE_NUM]; >>> int queues, i; >>> + NetFilterState *nf, *next; >>> + QemuOpts *opts; >>> >>> assert(nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC); >>> >>> @@ -396,6 +399,17 @@ void qemu_del_net_client(NetClientState *nc) >>> MAX_QUEUE_NUM); >>> assert(queues != 0); >>> >>> + /* >>> + * we delete/free the netfilter object attached to this netdev >>> + * multiqueue netfilter is not supported now, so only delete >>> + * nc->filters is enough. >>> + */ >> >> The comment is not correct since for multiqueue, each NetClientState is >> a queue. I think you can just remove above. > > But in this function qemu_del_net_client(), it will find all ncs with the > same name (which I thought is multiqueue), and then delete those > netclients. > Right. > is it ok to move this chunk to qemu_cleanup_net_client()? because in > qemu_cleanup_net_client(), we don't need to deal with multiqueue. Yes, looks good.