From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOJoB-0007Gn-P1 for qemu-devel@nongnu.org; Wed, 27 Jan 2016 01:42:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOJo6-00020N-UR for qemu-devel@nongnu.org; Wed, 27 Jan 2016 01:42:27 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:3541) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOJo5-0001z4-U0 for qemu-devel@nongnu.org; Wed, 27 Jan 2016 01:42:22 -0500 References: <1453451811-11860-1-git-send-email-zhang.zhanghailiang@huawei.com> <1453451811-11860-7-git-send-email-zhang.zhanghailiang@huawei.com> <56A5B031.3020707@redhat.com> <56A5CD1D.9090303@huawei.com> <56A6E57F.4050402@redhat.com> <56A81156.60102@huawei.com> <56A85CB7.1020400@redhat.com> From: Hailiang Zhang Message-ID: <56A866AB.1080205@huawei.com> Date: Wed, 27 Jan 2016 14:41:47 +0800 MIME-Version: 1.0 In-Reply-To: <56A85CB7.1020400@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC 6/7] net/filter: Add a default filter to each netdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com, zhangchen.fnst@cn.fujitsu.com, hongyang.yang@easystack.cn On 2016/1/27 13:59, Jason Wang wrote: > > > On 01/27/2016 08:37 AM, Hailiang Zhang wrote: >> On 2016/1/26 11:18, Jason Wang wrote: >>> >>> >>> On 01/25/2016 03:22 PM, Hailiang Zhang wrote: >>>> On 2016/1/25 13:18, Jason Wang wrote: >>>>> >>>>> >>>>> On 01/22/2016 04:36 PM, zhanghailiang wrote: >>>>>> We add each netdev a default buffer filter, which the name is >>>>>> 'nop', and the default buffer filter is disabled, so it has >>>>>> no side effect for packets delivering in qemu net layer. >>>>>> >>>>>> The default buffer filter can be used by COLO or Micro-checkpoint, >>>>>> The reason we add the default filter is we hope to support >>>>>> hot add network during COLO state in future. >>>>>> >>>>>> Signed-off-by: zhanghailiang >>>>>> --- >>>>>> include/net/filter.h | 11 +++++++++++ >>>>>> net/dump.c | 2 -- >>>>>> net/filter.c | 15 ++++++++++++++- >>>>>> net/net.c | 18 ++++++++++++++++++ >>>>>> 4 files changed, 43 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/include/net/filter.h b/include/net/filter.h >>>>>> index c7bd8f9..2043609 100644 >>>>>> --- a/include/net/filter.h >>>>>> +++ b/include/net/filter.h >>>>>> @@ -22,6 +22,16 @@ >>>>>> #define NETFILTER_CLASS(klass) \ >>>>>> OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER) >>>>>> >>> >>> [...] >>> >>>>>> >>>>>> nf->netdev = ncs[0]; >>>>>> + nf->is_default = !strcmp(path, DEFAULT_FILTER_NAME); >>>>>> + /* >>>>>> + * For the default buffer filter, it will be disabled by default, >>>>>> + * So it will not buffer any packets. >>>>>> + */ >>>>>> + if (nf->is_default) { >>>>>> + nf->enabled = false; >>>>>> + } >>>>> >>>>> This seems not very elegant. Besides DEFAULT_FILTER_NAME(TYPE), we may >>>>> also want a DEFAULT_FILTER_PROPERTIES? Then you can store the "status" >>>>> into properties. >>>>> >>>> >>>> A little confused, do you mean add a 'default' property for filter ? >>>> Just like the new 'status' property which is exported to users ? >>>> Is the type of 'default' property string or bool ? >>> >>> For example, is it possible to store the default property into a string >>> and just create the filter through qemu_opts_parse_noisily() by just >> >> We still need to use some *visit* helpers to realize the capability, >> because the object_add() helper need a 'Visitor *v' parameter, and the >> codes >> will be like: >> QemuOptsList qemu_filter_opts = { >> .name = "default-filter", >> .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), >> .desc = { >> { >> .name = "netdev", >> .type = QEMU_OPT_STRING, >> },{ >> .name = "status", >> .type = QEMU_OPT_STRING, >> }, >> { /* end of list */ } >> }, >> }; >> void netdev_add_filter(const char *netdev_id, >> const char *filter_type, >> const char *id, >> bool is_default, >> Error **errp) >> { >> sprintf(optarg, "netdev=%s,status=%s", netdev_id, >> is_default ? "disable" : "enable"); >> opts = qemu_opts_parse_noisily(&qemu_filter_opts, >> optarg, false); >> if (!opts) { >> error_report("Failed to parse param '%s'", optarg); >> exit(1); >> } >> >> qdict = qemu_opts_to_qdict(opts, NULL); >> ov = opts_visitor_new(opts); >> visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, >> &err); >> if (err) { >> goto out_clean; >> } >> object_add(filter_type, id, qdict, opts_get_visitor(ov), &err); >> if (err) { >> goto out_clean; >> } >> >> visit_end_struct(opts_get_visitor(ov), &err); >> if (err) { >> qmp_object_del(id, NULL); >> goto out_clean; >> } >> >> } >> >> Or, we can simplify patch 4 by using qmp_object_add(), codes will be >> like: >> >> void netdev_add_filter(const char *netdev_id, >> const char *filter_type, >> const char *id, >> bool is_default, >> Error **errp) >> { >> ... ... >> >> qov = qmp_output_visitor_new(); >> ov = qmp_output_get_visitor(qov); >> visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >> if (err) { >> goto out; >> } >> visit_type_str(ov, &nc->name, "netdev", &err); >> if (err) { >> goto out; >> } >> status = is_default ? g_strdup("disable") : g_strdup("enable"); >> visit_type_str(ov, &status, "status", &err); >> g_free(status); >> if (err) { >> goto out; >> } >> visit_end_struct(ov, &err); >> if (err) { >> goto out; >> } >> obj = qmp_output_get_qobject(qov); >> g_assert(obj != NULL); >> qmp_object_add(filter_type, id, true, obj, &err); >> qmp_output_visitor_cleanup(qov); >> qobject_decref(obj); >> >> } >> >> what's your suggestion ? :) >> > > Can we just reuse object_create()? here > Yes, the codes is more clean if we reuse it. I will fix it like that in v2, thanks. >> Thanks, >> Hailiang > > > . >