Re: [Qemu-devel] [PATCH v2 8/9] netfilter: add a netbuffer filter

[Yang Hongyang <yanghy@cn.fujitsu.com>]

On 07/31/2015 02:08 PM, Jason Wang wrote:
>
>
> On 07/31/2015 12:13 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 <yanghy@cn.fujitsu.com>
>> ---
>>   net/Makefile.objs   |   1 +
>>   net/filter-buffer.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   net/filter.c        |   2 +
>>   net/filters.h       |  17 ++++++
>>   qapi-schema.json    |  18 +++++-
>>   5 files changed, 197 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..7f2b050
>> --- /dev/null
>> +++ b/net/filter-buffer.c
>> @@ -0,0 +1,160 @@
>> +/*
>> + * Copyright (c) 2015 FUJITSU LIMITED
>> + * Author: Yang Hongyang <yanghy@cn.fujitsu.com>
>> + *
>> + * 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--;
>> +
>> +        qemu_net_queue_send(packet->sender->peer->incoming_queue,
>> +                            packet->sender,
>> +                            packet->flags,
>> +                            packet->data,
>> +                            packet->size,
>> +                            packet->sent_cb);
>
> Need to check peer since it may be NULL e.g after it was deleted. And we
> probably need to queue the packet into next filter.

Yes, if the peer is NULL, we can just free the packet and continue.
we might need to check the sender too, because the sender might be deleted
too...

I was thinking when we remove the netdev, we also remove/free the filters
attached to it automatically, do you think it is reasonable? because if the
netdev is removed, the filters which attached to it is useless.

-- 
Thanks,
Yang.