Re: [Qemu-devel] [PATCH V3 1/2] net/filter-mirror:Add filter-mirror

[Zhang Chen]

On 02/15/2016 03:06 PM, Zhang Chen wrote:
> On 02/15/2016 01:23 PM, Jason Wang wrote:
> > On 02/04/2016 05:00 PM, Zhang Chen wrote:
> > > On 02/04/2016 03:43 PM, Zhang Chen wrote:
> > > > From: ZhangChen <address@hidden>
> > > >
> > > > Filter-mirror is a netfilter plugin.
> > > > It gives qemu the ability to copy and mirror guest's
> > > > net packet. we output packet to chardev.
> > To make it compact, how about "It gives qemu the ability to mirror
> > packets to a chardev."?
>
> OK, will fix it in next version.
>
> > > > usage:
> > > >
> > > > -netdev tap,id=hn0
> > > > -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
> > > > -filter-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
> > An issue with mirror (and dump) is that it can not work correctly with
> > the netdev that has a vnet header. Need to fix this, a possible solution
> > is to checksum the buffer and strip the header before passing it to a
> > chardev.
>
> Thanks, I don't consider about vnet, we will fix it in next version.

We have discussed for vnet in our team.  we think filter-mirror no need to
do some analysis packet job, just do mirror job. and other job put it on
other plugin like filter-writer and filter-compare. If we have two guest
that both have vnet header, mirror one guest's packet to anther one.
strip the header then mirror packet will result in errors. so let's strip
vnet header in other plugin. keep filter-mirror simple.the filter-redirector
is same as filter-mirror.

> > > > Signed-off-by: ZhangChen <address@hidden>
> > > > Signed-off-by: Wen Congyang <address@hidden>
> > > > Reviewed-by: Yang Hongyang <address@hidden>
> > > > Reviewed-by: zhanghailiang <address@hidden>
> > > > ---
> > > >    net/Makefile.objs   |   1 +
> > > >    net/filter-mirror.c | 171
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >    qemu-options.hx     |   5 ++
> > > >    vl.c                |   3 +-
> > > >    4 files changed, 179 insertions(+), 1 deletion(-)
> > > >    create mode 100644 net/filter-mirror.c
> > > >
> > > > diff --git a/net/Makefile.objs b/net/Makefile.objs
> > > > index 5fa2f97..de06ebe 100644
> > > > --- a/net/Makefile.objs
> > > > +++ b/net/Makefile.objs
> > > > @@ -15,3 +15,4 @@ common-obj-$(CONFIG_VDE) += vde.o
> > > >    common-obj-$(CONFIG_NETMAP) += netmap.o
> > > >    common-obj-y += filter.o
> > > >    common-obj-y += filter-buffer.o
> > > > +common-obj-y += traffic-mirror.o
> > > s/traffic-mirror.o/filter-mirror.o/     rebase error....
> > >
> > > > diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> > > > new file mode 100644
> > > > index 0000000..87ccaf5
> > > > --- /dev/null
> > > > +++ b/net/filter-mirror.c
> > > > @@ -0,0 +1,171 @@
> > > > +/*
> > > > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > > > + * Copyright (c) 2016 FUJITSU LIMITED
> > > > + * Copyright (c) 2016 Intel Corporation
> > > > + *
> > > > + * Author: Zhang Chen <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/net.h"
> > > > +#include "qemu-common.h"
> > > > +#include "qapi/qmp/qerror.h"
> > > > +#include "qapi-visit.h"
> > > > +#include "qom/object.h"
> > > > +#include "qemu/main-loop.h"
> > > > +#include "qemu/error-report.h"
> > > > +#include "trace.h"
> > > > +#include "sysemu/char.h"
> > > > +#include "qemu/iov.h"
> > > > +#include "qemu/sockets.h"
> > > > +
> > > > +#define FILTER_MIRROR(obj) \
> > > > +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
> > > > +
> > > > +#define TYPE_FILTER_MIRROR "filter-mirror"
> > > > +
> > > > +typedef struct MirrorState {
> > > > +    NetFilterState parent_obj;
> > > > +    char *outdev;
> > > > +    CharDriverState *chr_out;
> > > > +} MirrorState;
> > > > +
> > > > +static ssize_t filter_mirror_send(NetFilterState *nf,
> > > > +                                   const struct iovec *iov,
> > > > +                                   int iovcnt)
> > > > +{
> > > > +    MirrorState *s = FILTER_MIRROR(nf);
> > > > +    ssize_t ret = 0;
> > > > +    ssize_t size = 0;
> > > > +    uint32_t len =  0;
> > > > +    char *buf;
> > > > +
> > > > +    size = iov_size(iov, iovcnt);
> > > > +    len = htonl(size);
> > > > +    if (!size) {
> > > > +        return 0;
> > > > +    }
> > > > +
> > > > +    buf = g_malloc0(size);
> > > > +    iov_to_buf(iov, iovcnt, 0, buf, size);
> > > > +    ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)&len,
> > > > sizeof(len));
> > > > +    if (ret < 0) {
> > I believe we should also fail when ret < sizeof(len) and modify the
> > caller check in filter_mirror_iov(). To make this a little bit easier,
> > there's no need to return ssize_t here (otherwise, caller need to call
> > iov_size() before checking the return value), just return 0 for success
> > and -EFXXX for failure.
>
> OK, will fix it in next version
>
> Thanks
> zhangchen
>
> > Other looks good.
> >
> >
> >
> > .

--
Thanks
zhangchen