Re: [Qemu-devel] [PATCH v13 09/10] netfilter: add a netbuffer filter

[Markus Armbruster]

Yang Hongyang <address@hidden> writes:

> This filter is to buffer/release packets. Can be used when using
> MicroCheckpointing or other Remus like VM FT solutions.
> You can also use it to simulate network delay.

Would be nice to mention this doesn't actually delay individual packets.
Perhaps

    You can also use it to crudely simulate network delay.  Doesn't
    actually delay individual packets, but batches them together, which
    is a delay of sorts.

Could be touched up on commit.

>
> Usage:
>  -netdev tap,id=bn0
>  -object filter-buffer,id=f0,netdev=bn0,queue=rx,interval=1000
>
> NOTE:
>  Interval is in microseconds, it can't be omitted currently, and can't be 0.
>
> Signed-off-by: Yang Hongyang <address@hidden>
> Signed-off-by: Jason Wang <address@hidden>
> ---
>  net/Makefile.objs   |   1 +
>  net/filter-buffer.c | 186 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx     |  17 +++++
>  vl.c                |   6 +-
>  4 files changed, 209 insertions(+), 1 deletion(-)
>  create mode 100644 net/filter-buffer.c
>
> 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..336bdcc
> --- /dev/null
> +++ b/net/filter-buffer.c
> @@ -0,0 +1,186 @@
> +/*
> + * Copyright (c) 2015 FUJITSU LIMITED
> + * Author: Yang Hongyang <address@hidden>
> + *
> + * 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 "qemu-common.h"
> +#include "qemu/timer.h"
> +#include "qemu/iov.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qapi-visit.h"
> +#include "qom/object.h"
> +
> +#define TYPE_FILTER_BUFFER "filter-buffer"
> +
> +#define FILTER_BUFFER(obj) \
> +    OBJECT_CHECK(FilterBufferState, (obj), TYPE_FILTER_BUFFER)
> +
> +typedef struct FilterBufferState {
> +    NetFilterState parent_obj;
> +
> +    NetQueue *incoming_queue;
> +    uint32_t interval;
> +    QEMUTimer release_timer;
> +} FilterBufferState;
> +
> +static void filter_buffer_flush(NetFilterState *nf)
> +{
> +    FilterBufferState *s = FILTER_BUFFER(nf);
> +
> +    if (!qemu_net_queue_flush(s->incoming_queue)) {
> +        /* Unable to empty the queue, purge remaining packets */
> +        qemu_net_queue_purge(s->incoming_queue, nf->netdev);
> +    }
> +}
> +
> +static void filter_buffer_release_timer(void *opaque)
> +{
> +    NetFilterState *nf = opaque;
> +    FilterBufferState *s = FILTER_BUFFER(nf);
> +
> +    /*
> +     * Note: filter_buffer_flush() drops packets that can't be sent
> +     * TODO: We should leave them queued.  But currently there's no way
> +     * for the next filter or recivier to notify us that it can receive

You faithfully pasted my typo: it's receiver, not recivier.  Could be
touched up on commit.

> +     * more packets.
> +     */
> +    filter_buffer_flush(nf);
> +    /* Timer rearmed to fire again in s->interval microseconds. */
> +    timer_mod(&s->release_timer,
> +              qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
> +}
[...]

Preferably with these touch-ups:

Reviewed-by: Markus Armbruster <address@hidden>