From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQ4xC-0002RF-PO for qemu-devel@nongnu.org; Sun, 31 Jan 2016 22:15:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQ4x8-0008Ny-Na for qemu-devel@nongnu.org; Sun, 31 Jan 2016 22:15:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37799) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQ4x8-0008Nu-Fc for qemu-devel@nongnu.org; Sun, 31 Jan 2016 22:14:58 -0500 References: <1453883380-10532-1-git-send-email-zhang.zhanghailiang@huawei.com> <1453883380-10532-4-git-send-email-zhang.zhanghailiang@huawei.com> From: Jason Wang Message-ID: <56AECDA8.6000402@redhat.com> Date: Mon, 1 Feb 2016 11:14:48 +0800 MIME-Version: 1.0 In-Reply-To: <1453883380-10532-4-git-send-email-zhang.zhanghailiang@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: zhanghailiang , qemu-devel@nongnu.org Cc: zhangchen.fnst@cn.fujitsu.com, hongyang.yang@easystack.cn On 01/27/2016 04:29 PM, zhanghailiang wrote: > We add a new helper function netdev_add_filter(), this function > can help adding a filter object to a netdev. > Besides, we add a is_default member for struct NetFilterState > to indicate whether the filter is default or not. > > Signed-off-by: zhanghailiang > --- > v2: > -Re-implement netdev_add_filter() by re-using object_create() > (Jason's suggestion) > --- > include/net/filter.h | 7 +++++ > net/filter.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 87 insertions(+) > > diff --git a/include/net/filter.h b/include/net/filter.h > index af3c53c..ee1c024 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 is_default; > bool enabled; > QTAILQ_ENTRY(NetFilterState) next; > }; > @@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > int iovcnt, > void *opaque); > > +void netdev_add_filter(const char *netdev_id, > + const char *filter_type, > + const char *id, > + bool is_default, > + Error **errp); > + > #endif /* QEMU_NET_FILTER_H */ > diff --git a/net/filter.c b/net/filter.c > index d08a2be..dc7aa9b 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) > QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); > } > > +QemuOptsList qemu_filter_opts = { > + .name = "default-filter", > + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), > + .desc = { > + { > + .name = "qom-type", > + .type = QEMU_OPT_STRING, > + },{ > + .name = "id", > + .type = QEMU_OPT_STRING, > + },{ > + .name = "netdev", > + .type = QEMU_OPT_STRING, > + },{ > + .name = "status", > + .type = QEMU_OPT_STRING, > + }, > + { /* end of list */ } > + }, > +}; > + > +static void filter_set_default_flag(const char *id, > + bool is_default, > + Error **errp) > +{ > + Object *obj, *container; > + NetFilterState *nf; > + > + container = object_get_objects_root(); > + obj = object_resolve_path_component(container, id); > + if (!obj) { > + error_setg(errp, "object id not found"); > + return; > + } > + nf = NETFILTER(obj); > + nf->is_default = is_default; > +} > + > +void netdev_add_filter(const char *netdev_id, > + const char *filter_type, > + const char *id, > + bool is_default, > + Error **errp) > +{ > + NetClientState *nc = qemu_find_netdev(netdev_id); > + char *optarg; > + QemuOpts *opts = NULL; > + Error *err = NULL; > + > + /* FIXME: Not support multiple queues */ > + if (!nc || nc->queue_index > 1) { > + return; > + } > + /* Not support vhost-net */ > + if (get_vhost_net(nc)) { > + return; > + } > + > + optarg = g_strdup_printf("qom-type=%s,id=%s,netdev=%s,status=%s", > + filter_type, id, netdev_id, is_default ? "disable" : "enable" Instead of this, I wonder maybe it's better to: - store the default filter property into a pointer to string - colo code may change the pointer to "filter-buffer,status=disable" Then, there's no need for lots of codes above: - no need a "is_default" parameter in netdev_add_filter which does not scale consider we may want to have more property in the future - no need to hacking like "qemu_filter_opts" - no need to have a special flag like "is_default" Thoughts? > + opts = qemu_opts_parse_noisily(&qemu_filter_opts, > + optarg, false); > + if (!opts) { > + error_report("Failed to parse param '%s'", optarg); > + exit(1); > + } > + g_free(optarg); > + if (object_create(NULL, opts, &err) < 0) { > + error_report("Failed to create object"); > + goto out_clean; > + } > + filter_set_default_flag(id, is_default, &err); > + > +out_clean: > + qemu_opts_del(opts); > + if (err) { > + error_propagate(errp, err); > + } > +} > + > static void netfilter_finalize(Object *obj) > { > NetFilterState *nf = NETFILTER(obj);