From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34626) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZMX76-0007iX-QL for qemu-devel@nongnu.org; Tue, 04 Aug 2015 03:58:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZMX70-0002cb-KQ for qemu-devel@nongnu.org; Tue, 04 Aug 2015 03:58:20 -0400 Received: from [59.151.112.132] (port=46184 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZMX6y-0002Yu-8f for qemu-devel@nongnu.org; Tue, 04 Aug 2015 03:58:14 -0400 Message-ID: <55C07080.2040203@cn.fujitsu.com> Date: Tue, 4 Aug 2015 15:57:52 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <1438590616-21142-1-git-send-email-yanghy@cn.fujitsu.com> <1438590616-21142-11-git-send-email-yanghy@cn.fujitsu.com> <55C0479C.7000105@redhat.com> <55C05644.20905@cn.fujitsu.com> <55C05BE8.1000907@redhat.com> In-Reply-To: <55C05BE8.1000907@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 10/12] netfilter: add a netbuffer filter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: thuth@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, dgilbert@redhat.com, mrhines@linux.vnet.ibm.com, stefanha@redhat.com On 08/04/2015 02:30 PM, Jason Wang wrote: > > > On 08/04/2015 02:05 PM, Yang Hongyang wrote: >> On 08/04/2015 01:03 PM, Jason Wang wrote: >>> >>> >>> On 08/03/2015 04:30 PM, Yang Hongyang wrote: >>>> This filter is to buffer/release packets, this feature can be used >>>> when using MicroCheckpointing, or other Remus like VM FT solutions, you >>>> can also use it to simulate the network delay. >>>> It has an interval option, if supplied, this filter will release >>>> packets by interval. >>>> >>>> Usage: >>>> -netdev tap,id=bn0 >>>> -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000 >>>> >>>> NOTE: >>>> the scale of interval is microsecond. >>>> >>>> Signed-off-by: Yang Hongyang >>>> --- >>>> v3: check packet's sender and sender->peer when flush it >>>> --- >>>> net/Makefile.objs | 1 + >>>> net/filter-buffer.c | 162 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> net/filter.c | 2 + >>>> net/filters.h | 17 ++++++ >>>> qapi-schema.json | 18 +++++- >>>> 5 files changed, 199 insertions(+), 1 deletion(-) >>>> create mode 100644 net/filter-buffer.c >>>> create mode 100644 net/filters.h >>>> >>>> diff --git a/net/Makefile.objs b/net/Makefile.objs >>>> index 914aec0..5fa2f97 100644 >>>> --- a/net/Makefile.objs >>>> +++ b/net/Makefile.objs >>>> @@ -14,3 +14,4 @@ common-obj-$(CONFIG_SLIRP) += slirp.o >>>> common-obj-$(CONFIG_VDE) += vde.o >>>> common-obj-$(CONFIG_NETMAP) += netmap.o >>>> common-obj-y += filter.o >>>> +common-obj-y += filter-buffer.o >>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c >>>> new file mode 100644 >>>> index 0000000..1547765 >>>> --- /dev/null >>>> +++ b/net/filter-buffer.c >>>> @@ -0,0 +1,162 @@ >>>> +/* >>>> + * Copyright (c) 2015 FUJITSU LIMITED >>>> + * Author: Yang Hongyang >>>> + * >>>> + * This work is licensed under the terms of the GNU GPL, version 2 or >>>> + * later. See the COPYING file in the top-level directory. >>>> + */ >>>> + >>>> +#include "net/filter.h" >>>> +#include "net/queue.h" >>>> +#include "filters.h" >>>> +#include "qemu-common.h" >>>> +#include "qemu/error-report.h" >>>> +#include "qemu/main-loop.h" >>>> +#include "qemu/timer.h" >>>> +#include "qemu/iov.h" >>>> + >>>> +typedef struct FILTERBUFFERState { >>>> + NetFilterState nf; >>>> + NetQueue *incoming_queue; >>>> + NetQueue *inflight_queue; >>>> + QEMUBH *flush_bh; >>>> + int64_t interval; >>>> + QEMUTimer release_timer; >>>> +} FILTERBUFFERState; >>>> + >>>> +static void packet_send_completed(NetClientState *nc, ssize_t len) >>>> +{ >>>> + return; >>>> +} >>>> + >>>> +static void filter_buffer_flush(NetFilterState *nf) >>>> +{ >>>> + FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf); >>>> + NetQueue *queue = s->inflight_queue; >>>> + NetPacket *packet; >>>> + >>>> + while (queue && !QTAILQ_EMPTY(&queue->packets)) { >>>> + packet = QTAILQ_FIRST(&queue->packets); >>>> + QTAILQ_REMOVE(&queue->packets, packet, entry); >>>> + queue->nq_count--; >>>> + >>>> + if (packet->sender && packet->sender->peer) { >>>> + qemu_net_queue_send(packet->sender->peer->incoming_queue, >>>> + packet->sender, >>>> + packet->flags, >>>> + packet->data, >>>> + packet->size, >>>> + packet->sent_cb); >>>> + } >>>> + >>>> + /* >>>> + * now that we pass the packet to >>>> sender->peer->incoming_queue, we >>>> + * don't care the reture value here, because the peer's >>>> queue will >>>> + * take care of this packet >>>> + */ >>>> + g_free(packet); >>> >>> So looks like the packet was still not passed to next filter? >> >> I didn't get your suggestion last time, sorry, will add it in next >> version. >> Just to confirm, do you mean we need to pass the packet to next filter >> instead of pass to the receiver's incoming_queue? > > Yes, consider you may have two filters. First is dump and second is > buffer, I believe you still want to buffer the packet even if it has > been dumped. > >> and even if we pass to next >> filter, the check of sender and it's peer is still needed, because if >> there's no receiver, it's nonsense to pass it further? > > Yes. thanks. > >> >>> >>>> + } >>>> + >>>> + g_free(queue); >>>> + s->inflight_queue = NULL; >>>> +} >>>> + >>>> +static void filter_buffer_flush_bh(void *opaque) >>>> +{ >>>> + FILTERBUFFERState *s = opaque; >>>> + NetFilterState *nf = &s->nf; >>>> + filter_buffer_flush(nf); >>>> +} >>>> + >>>> +static void filter_buffer_release_one(NetFilterState *nf) >>>> +{ >>>> + FILTERBUFFERState *s = DO_UPCAST(FILTERBUFFERState, nf, nf); >>>> + >>>> + /* flush inflight packets */ >>>> + if (s->inflight_queue) { >>>> + filter_buffer_flush(nf); >>>> + } >>>> + >>>> + s->inflight_queue = s->incoming_queue; >>>> + s->incoming_queue = qemu_new_net_queue(nf); >>> >>> So this in fact flush a brunch of packets. If yes, the name of function >>> is confusing >> >> maybe filter_buffer_release ? >> > > Right. By removing bh, this function can be dropped, just call filter_buffer_flush. thanks. > >>> >>>> + qemu_bh_schedule(s->flush_bh); >>> >>> Don't get why a bh is needed. If we could get rid of it, there's >>> probably no need for inflight_queue, and we can just drain >>> incoming_queue here. >> >> Seems we can get rid of bh, will do, thanks. > > . > -- Thanks, Yang.