From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40529) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2YUK-0002RN-2h for qemu-devel@nongnu.org; Sat, 28 Nov 2015 00:56:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a2YUF-0000yw-Rp for qemu-devel@nongnu.org; Sat, 28 Nov 2015 00:56:00 -0500 Received: from szxga02-in.huawei.com ([119.145.14.65]:29632) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2YUE-0000y4-VG for qemu-devel@nongnu.org; Sat, 28 Nov 2015 00:55:55 -0500 References: <1448357149-17572-1-git-send-email-zhang.zhanghailiang@huawei.com> <1448357149-17572-35-git-send-email-zhang.zhanghailiang@huawei.com> <56584107.4060709@easystack.cn> From: Hailiang Zhang Message-ID: <565941DC.1080405@huawei.com> Date: Sat, 28 Nov 2015 13:55:40 +0800 MIME-Version: 1.0 In-Reply-To: <56584107.4060709@easystack.cn> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v11 34/39] net/filter-buffer: Add default filter-buffer for each netdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yang Hongyang , qemu-devel@nongnu.org Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, Jason Wang , 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 On 2015/11/27 19:39, Yang Hongyang wrote: > On 2015年11月24日 17:25, zhanghailiang wrote: >> We add each netdev a default filter-buffer, which will be used for COLO >> or Micro-checkpoint to buffer VM's packets. The name of default filter-buffer >> is 'nop'. >> For the default filter-buffer, it will not buffer any packets in default. >> So it has no side effect for the netdev. >> >> Signed-off-by: zhanghailiang >> Cc: Jason Wang >> Cc: Yang Hongyang >> --- >> v11: >> - New patch >> --- > [...] >> static void filter_buffer_flush(NetFilterState *nf) >> @@ -65,6 +73,10 @@ static ssize_t filter_buffer_receive_iov(NetFilterState *nf, >> { >> FilterBufferState *s = FILTER_BUFFER(nf); >> >> + /* Don't buffer any packets if the filter is not enabled */ >> + if (!s->enable_buffer) { >> + return 0; >> + } >> /* >> * We return size when buffer a packet, the sender will take it as >> * a already sent packet, so sent_cb should not be called later. >> @@ -102,6 +114,7 @@ static void filter_buffer_cleanup(NetFilterState *nf) >> static void filter_buffer_setup(NetFilterState *nf, Error **errp) >> { >> FilterBufferState *s = FILTER_BUFFER(nf); >> + char *path = object_get_canonical_path_component(OBJECT(nf)); > > path should be freed after use. > >> >> /* >> * We may want to accept zero interval when VM FT solutions like MC >> @@ -114,6 +127,7 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) >> } >> >> s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); >> + s->is_default = !strcmp(path, "nop"); > > free(path); Good catch, :) > >> if (s->interval) { >> timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, >> filter_buffer_release_timer, nf); >> @@ -163,6 +177,66 @@ out: >> error_propagate(errp, local_err); >> } >> >> +/* >> +* This will be used by COLO or MC FT, for which they will need >> +* to buffer the packets of VM's net devices, Here we add a default >> +* buffer filter for each netdev. The name of default buffer filter is >> +* 'nop' >> +*/ >> +void netdev_add_default_filter_buffer(const char *netdev_id, >> + NetFilterDirection direction, >> + Error **errp) >> +{ >> + QmpOutputVisitor *qov; >> + QmpInputVisitor *qiv; >> + Visitor *ov, *iv; >> + QObject *obj = NULL; >> + QDict *qdict; >> + void *dummy = NULL; >> + const char *id = "nop"; >> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >> + NetClientState *nc = qemu_find_netdev(netdev_id); >> + Error *err = NULL; >> + >> + /* FIXME: Not support multiple queues */ >> + if (!nc || nc->queue_index > 1) { >> + return; >> + } >> + 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; >> + } >> + visit_type_str(ov, &queue, "queue", &err); >> + if (err) { >> + goto out; >> + } >> + visit_end_struct(ov, &err); >> + if (err) { >> + goto out; >> + } >> + obj = qmp_output_get_qobject(qov); >> + g_assert(obj != NULL); >> + qdict = qobject_to_qdict(obj); >> + qmp_output_visitor_cleanup(qov); >> + >> + qiv = qmp_input_visitor_new(obj); >> + iv = qmp_input_get_visitor(qiv); >> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >> + qmp_input_visitor_cleanup(qiv); >> + qobject_decref(obj); >> +out: >> + g_free(queue); >> + if (err) { >> + error_propagate(errp, err); >> + } >> +} >> + >> static void filter_buffer_init(Object *obj) >> { >> object_property_add(obj, "interval", "int", >> diff --git a/net/net.c b/net/net.c >> index ade6051..b36d49f 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -1028,6 +1028,14 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp) >> } >> return -1; >> } >> + >> + if (is_netdev) { >> + const Netdev *netdev = object; >> + >> + netdev_add_default_filter_buffer(netdev->id, >> + NET_FILTER_DIRECTION_RX, >> + errp); >> + } > > I'm not sure if add this default to all netdev is a good idea, because > in most cases, they do not need the default filter, although it is a > nop, it still go though the filter. Maybe we can add a argument to > netdev to easily turn if on, for example, > -netdev default-filter=true > Hmm, Adding default filter is suggested by Jason, and yes, it still go through the default filter though it does nothing, which is not graceful. > if not supplied, default to off, that is, by default no default filter > will be attached to the netdev. > That's a good idea, and we should also show the off/on info for 'info network' command. Thanks, zhanghailiang >> return 0; >> } >> >> >