diff mbox

[V2] net/traffic-mirror:Add traffic-mirror

Message ID 1453862428-25570-1-git-send-email-zhangchen.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

Zhang Chen Jan. 27, 2016, 2:40 a.m. UTC
From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>

Traffic-mirror is a netfilter plugin.
It gives qemu the ability to copy and mirror guest's
net packet. we output packet to chardev.

usage:

-netdev tap,id=hn0
-chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
-traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0

Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
---
 net/Makefile.objs    |   1 +
 net/traffic-mirror.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx      |   5 ++
 vl.c                 |   3 +-
 4 files changed, 181 insertions(+), 1 deletion(-)
 create mode 100644 net/traffic-mirror.c

Comments

Zhanghailiang Jan. 27, 2016, 9:23 a.m. UTC | #1
On 2016/1/27 10:40, Zhang Chen wrote:
> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>
> Traffic-mirror is a netfilter plugin.
> It gives qemu the ability to copy and mirror guest's
> net packet. we output packet to chardev.
>
> usage:
>
> -netdev tap,id=hn0
> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>
> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
> ---
>   net/Makefile.objs    |   1 +
>   net/traffic-mirror.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   qemu-options.hx      |   5 ++
>   vl.c                 |   3 +-
>   4 files changed, 181 insertions(+), 1 deletion(-)
>   create mode 100644 net/traffic-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
> diff --git a/net/traffic-mirror.c b/net/traffic-mirror.c
> new file mode 100644
> index 0000000..bed915c
> --- /dev/null
> +++ b/net/traffic-mirror.c
> @@ -0,0 +1,173 @@
> +/*
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + * Copyright (c) 2016 FUJITSU LIMITED
> + * Copyright (c) 2016 Intel Corporation
> + *
> + * Author: Zhang Chen <zhangchen.fnst@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/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"
> +
> +#define FILTER_TRAFFIC_MIRROR(obj) \
> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_TRAFFIC_MIRROR)
> +
> +#define TYPE_FILTER_TRAFFIC_MIRROR "traffic-mirror"
> +
> +typedef struct MirrorState {
> +    NetFilterState parent_obj;
> +    char *outdev;
> +    CharDriverState *chr_out;
> +

Redundant space here. (Maybe it is no need
for another version if there is no other problem ;) ).
Other seems to be OK.

Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

> +} MirrorState;
> +




> +static ssize_t traffic_mirror_send(NetFilterState *nf,
> +                                   const struct iovec *iov,
> +                                   int iovcnt)
> +{
> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
> +    ssize_t ret = 0;
> +    ssize_t size = 0;
> +    char *buf;
> +
> +    size = iov_size(iov, iovcnt);
> +    if (!size) {
> +        return 0;
> +    }
> +
> +    buf = g_malloc0(size);
> +    iov_to_buf(iov, iovcnt, 0, buf, size);
> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)&size, sizeof(size));
> +    if (ret < 0) {
> +        g_free(buf);
> +        return ret;
> +    }
> +
> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)buf, size);
> +    g_free(buf);
> +    return ret;
> +}
> +
> +static ssize_t traffic_mirror_receive_iov(NetFilterState *nf,
> +                                         NetClientState *sender,
> +                                         unsigned flags,
> +                                         const struct iovec *iov,
> +                                         int iovcnt,
> +                                         NetPacketSent *sent_cb)
> +{
> +    /*
> +     * We copy and mirror packet to outdev,
> +     * then put back the packet.
> +     */
> +    ssize_t ret = 0;
> +
> +    ret = traffic_mirror_send(nf, iov, iovcnt);
> +    if (ret < 0) {
> +        error_report("traffic_mirror_send failed");
> +    }
> +
> +    return 0;
> +}
> +
> +static void traffic_mirror_cleanup(NetFilterState *nf)
> +{
> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
> +
> +    if (s->chr_out) {
> +        qemu_chr_fe_release(s->chr_out);
> +    }
> +}
> +
> +static void traffic_mirror_setup(NetFilterState *nf, Error **errp)
> +{
> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
> +
> +    if (!s->outdev) {
> +        error_setg(errp, "filter traffic mirror needs 'outdev' "
> +                "property set!");
> +        return;
> +    }
> +
> +    s->chr_out = qemu_chr_find(s->outdev);
> +    if (s->chr_out == NULL) {
> +        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +                  "Device '%s' not found", s->outdev);
> +        return;
> +    }
> +
> +    if (qemu_chr_fe_claim(s->chr_out) != 0) {
> +        error_setg(errp, QERR_DEVICE_IN_USE, s->outdev);
> +        return;
> +    }
> +}
> +
> +static void traffic_mirror_class_init(ObjectClass *oc, void *data)
> +{
> +    NetFilterClass *nfc = NETFILTER_CLASS(oc);
> +
> +    nfc->setup = traffic_mirror_setup;
> +    nfc->cleanup = traffic_mirror_cleanup;
> +    nfc->receive_iov = traffic_mirror_receive_iov;
> +}
> +
> +static char *traffic_mirror_get_outdev(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(obj);
> +
> +    return g_strdup(s->outdev);
> +}
> +
> +static void
> +traffic_mirror_set_outdev(Object *obj, const char *value, Error **errp)
> +{
> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(obj);
> +
> +    g_free(s->outdev);
> +    s->outdev = g_strdup(value);
> +    if (!s->outdev) {
> +        error_setg(errp, "filter traffic mirror needs 'outdev' "
> +                "property set!");
> +        return;
> +    }
> +}
> +
> +static void traffic_mirror_init(Object *obj)
> +{
> +    object_property_add_str(obj, "outdev", traffic_mirror_get_outdev,
> +                            traffic_mirror_set_outdev, NULL);
> +}
> +
> +static void traffic_mirror_fini(Object *obj)
> +{
> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(obj);
> +
> +    g_free(s->outdev);
> +}
> +
> +static const TypeInfo traffic_mirror_info = {
> +    .name = TYPE_FILTER_TRAFFIC_MIRROR,
> +    .parent = TYPE_NETFILTER,
> +    .class_init = traffic_mirror_class_init,
> +    .instance_init = traffic_mirror_init,
> +    .instance_finalize = traffic_mirror_fini,
> +    .instance_size = sizeof(MirrorState),
> +};
> +
> +static void register_types(void)
> +{
> +    type_register_static(&traffic_mirror_info);
> +}
> +
> +type_init(register_types);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0eea4ee..6fd2d46 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3670,6 +3670,11 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>   @option{tx}: the filter is attached to the transmit queue of the netdev,
>                where it will receive packets sent by the netdev.
>
> +@item -object traffic-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
> +
> +traffic-mirror on netdev @var{netdevid},mirror net packet to outdev.
> +queue @var{all|rx|tx} is an option that can be applied to traffic-mirror.
> +
>   @item -object filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
>
>   Dump the network traffic on netdev @var{dev} to the file specified by
> diff --git a/vl.c b/vl.c
> index 8dc34ce..413d73a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2838,7 +2838,8 @@ static bool object_create_initial(const char *type)
>        * they depend on netdevs already existing
>        */
>       if (g_str_equal(type, "filter-buffer") ||
> -        g_str_equal(type, "filter-dump")) {
> +        g_str_equal(type, "filter-dump") ||
> +        g_str_equal(type, "traffic-mirror")) {
>           return false;
>       }
>
>
Jason Wang Jan. 28, 2016, 5:44 a.m. UTC | #2
On 01/27/2016 10:40 AM, Zhang Chen wrote:
> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>
> Traffic-mirror is a netfilter plugin.
> It gives qemu the ability to copy and mirror guest's
> net packet. we output packet to chardev.
>
> usage:
>
> -netdev tap,id=hn0
> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>
> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>

Thanks for the patch. Several questions:

- I'm curious about how the patch was tested? Simple setup e.g:

-netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
socket,id=c0,host=localhost,port=4444,server,nowait -object
traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0

does not works for me.

- Is a reliable mirroring (e.g no packet drops during mirroring) is
needed for COLO? If yes, this patch seems could not guarantee this.
- Please consider to write a unit test for this patch.

And see comments below.

Thanks


> ---
>  net/Makefile.objs    |   1 +
>  net/traffic-mirror.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx      |   5 ++
>  vl.c                 |   3 +-
>  4 files changed, 181 insertions(+), 1 deletion(-)
>  create mode 100644 net/traffic-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

Let's s/traffic-mirror/filter-mirror/g to be consistent with other filters.

> diff --git a/net/traffic-mirror.c b/net/traffic-mirror.c
> new file mode 100644
> index 0000000..bed915c
> --- /dev/null
> +++ b/net/traffic-mirror.c
> @@ -0,0 +1,173 @@
> +/*
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + * Copyright (c) 2016 FUJITSU LIMITED
> + * Copyright (c) 2016 Intel Corporation
> + *
> + * Author: Zhang Chen <zhangchen.fnst@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/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"
> +
> +#define FILTER_TRAFFIC_MIRROR(obj) \
> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_TRAFFIC_MIRROR)
> +
> +#define TYPE_FILTER_TRAFFIC_MIRROR "traffic-mirror"
> +
> +typedef struct MirrorState {
> +    NetFilterState parent_obj;
> +    char *outdev;
> +    CharDriverState *chr_out;
> +
> +} MirrorState;
> +
> +static ssize_t traffic_mirror_send(NetFilterState *nf,
> +                                   const struct iovec *iov,
> +                                   int iovcnt)
> +{
> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
> +    ssize_t ret = 0;
> +    ssize_t size = 0;
> +    char *buf;
> +
> +    size = iov_size(iov, iovcnt);
> +    if (!size) {
> +        return 0;
> +    }
> +
> +    buf = g_malloc0(size);
> +    iov_to_buf(iov, iovcnt, 0, buf, size);
> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)&size, sizeof(size));

htonl(size)?

> +    if (ret < 0) {

This check is not sufficient, for some reason, only part of the packets
maybe sent by the socket. Need to handle this properly, otherwise it may
confuse receiver.

> +        g_free(buf);
> +        return ret;
> +    }
> +
> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)buf, size);
> +    g_free(buf);
> +    return ret;

Ditto.

> +}
> +
> +static ssize_t traffic_mirror_receive_iov(NetFilterState *nf,
> +                                         NetClientState *sender,
> +                                         unsigned flags,
> +                                         const struct iovec *iov,
> +                                         int iovcnt,
> +                                         NetPacketSent *sent_cb)
> +{
> +    /*
> +     * We copy and mirror packet to outdev,
> +     * then put back the packet.
> +     */

The code could explain itself, so the comment is unnecessary.

> +    ssize_t ret = 0;
> +
> +    ret = traffic_mirror_send(nf, iov, iovcnt);
> +    if (ret < 0) {
> +        error_report("traffic_mirror_send failed");

Monitor could be flooded by this.

> +    }
> +
> +    return 0;
> +}
> +

Other looks good.
Zhang Chen Jan. 28, 2016, 7:44 a.m. UTC | #3
On 01/28/2016 01:44 PM, Jason Wang wrote:
>
> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>
>> Traffic-mirror is a netfilter plugin.
>> It gives qemu the ability to copy and mirror guest's
>> net packet. we output packet to chardev.
>>
>> usage:
>>
>> -netdev tap,id=hn0
>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>
>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
> Thanks for the patch. Several questions:
>
> - I'm curious about how the patch was tested? Simple setup e.g:
>
> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
> socket,id=c0,host=localhost,port=4444,server,nowait -object
> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>
> does not works for me.

I test it in this way.
primary:
-netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev 
socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
  -object traffic-mirror,id=f0,netdev=hn0,queue=tx,outdev=mirror0

secondary:
-netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev 
socket,id=mirror0,host=3.3.3.3,port=9003 -object 
traffic-reader,id=f1,netdev=hn0,queue=rx,indev=mirror0

I write a traffic-reader demo to read chardev socket and print it in 
monitor.


>
> - Is a reliable mirroring (e.g no packet drops during mirroring) is
> needed for COLO? If yes, this patch seems could not guarantee this.

I will fix it in V3

> - Please consider to write a unit test for this patch.

write a unit test like tests/test-netfilter.c ?

> And see comments below.
>
> Thanks
>
>
>> ---
>>   net/Makefile.objs    |   1 +
>>   net/traffic-mirror.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-options.hx      |   5 ++
>>   vl.c                 |   3 +-
>>   4 files changed, 181 insertions(+), 1 deletion(-)
>>   create mode 100644 net/traffic-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
> Let's s/traffic-mirror/filter-mirror/g to be consistent with other filters.
>

OK~ I will fix it in V3

>> diff --git a/net/traffic-mirror.c b/net/traffic-mirror.c
>> new file mode 100644
>> index 0000000..bed915c
>> --- /dev/null
>> +++ b/net/traffic-mirror.c
>> @@ -0,0 +1,173 @@
>> +/*
>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
>> + * Copyright (c) 2016 FUJITSU LIMITED
>> + * Copyright (c) 2016 Intel Corporation
>> + *
>> + * Author: Zhang Chen <zhangchen.fnst@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/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"
>> +
>> +#define FILTER_TRAFFIC_MIRROR(obj) \
>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_TRAFFIC_MIRROR)
>> +
>> +#define TYPE_FILTER_TRAFFIC_MIRROR "traffic-mirror"
>> +
>> +typedef struct MirrorState {
>> +    NetFilterState parent_obj;
>> +    char *outdev;
>> +    CharDriverState *chr_out;
>> +
>> +} MirrorState;
>> +
>> +static ssize_t traffic_mirror_send(NetFilterState *nf,
>> +                                   const struct iovec *iov,
>> +                                   int iovcnt)
>> +{
>> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
>> +    ssize_t ret = 0;
>> +    ssize_t size = 0;
>> +    char *buf;
>> +
>> +    size = iov_size(iov, iovcnt);
>> +    if (!size) {
>> +        return 0;
>> +    }
>> +
>> +    buf = g_malloc0(size);
>> +    iov_to_buf(iov, iovcnt, 0, buf, size);
>> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)&size, sizeof(size));
> htonl(size)?

We do not need this.

>> +    if (ret < 0) {
> This check is not sufficient, for some reason, only part of the packets
> maybe sent by the socket. Need to handle this properly, otherwise it may
> confuse receiver.

I will fix it in next version.

>> +        g_free(buf);
>> +        return ret;
>> +    }
>> +
>> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)buf, size);
>> +    g_free(buf);
>> +    return ret;
> Ditto.

I will fix it in next version.

>> +}
>> +
>> +static ssize_t traffic_mirror_receive_iov(NetFilterState *nf,
>> +                                         NetClientState *sender,
>> +                                         unsigned flags,
>> +                                         const struct iovec *iov,
>> +                                         int iovcnt,
>> +                                         NetPacketSent *sent_cb)
>> +{
>> +    /*
>> +     * We copy and mirror packet to outdev,
>> +     * then put back the packet.
>> +     */
> The code could explain itself, so the comment is unnecessary.

OK,I will remove it.

Thanks
zhangchen

>> +    ssize_t ret = 0;
>> +
>> +    ret = traffic_mirror_send(nf, iov, iovcnt);
>> +    if (ret < 0) {
>> +        error_report("traffic_mirror_send failed");
> Monitor could be flooded by this.
>
>> +    }
>> +
>> +    return 0;
>> +}
>> +
> Other looks good.
>
>
>
> .
>
Jason Wang Jan. 28, 2016, 8:37 a.m. UTC | #4
On 01/28/2016 03:44 PM, Zhang Chen wrote:
>
>
> On 01/28/2016 01:44 PM, Jason Wang wrote:
>>
>> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>
>>> Traffic-mirror is a netfilter plugin.
>>> It gives qemu the ability to copy and mirror guest's
>>> net packet. we output packet to chardev.
>>>
>>> usage:
>>>
>>> -netdev tap,id=hn0
>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>>
>>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>> Thanks for the patch. Several questions:
>>
>> - I'm curious about how the patch was tested? Simple setup e.g:
>>
>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
>> socket,id=c0,host=localhost,port=4444,server,nowait -object
>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>>
>> does not works for me.
>
> I test it in this way.
> primary:
> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
> socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
>  -object traffic-mirror,id=f0,netdev=hn0,queue=tx,outdev=mirror0
>
> secondary:
> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
> socket,id=mirror0,host=3.3.3.3,port=9003 -object
> traffic-reader,id=f1,netdev=hn0,queue=rx,indev=mirror0
>
> I write a traffic-reader demo to read chardev socket and print it in
> monitor.

Ok, but maybe you can try socket backend. I think the protocol should be
at least compatible with it.

>
>
>>
>> - Is a reliable mirroring (e.g no packet drops during mirroring) is
>> needed for COLO? If yes, this patch seems could not guarantee this.
>
> I will fix it in V3
>
>> - Please consider to write a unit test for this patch.
>
> write a unit test like tests/test-netfilter.c ?

Even more for its basic function to work. E.g, start qemu with:

-netdev socket,id=s0,listen=localhost:X -chardev
socket,id=c0,host=localhost,pory=Y,server,nowait -object
filter-mirror,netdev=hn0,outdev=c0

Then you can inject packet from the socket connected to s0 and see if
you can read it from socket that connected from c0 (or your traffic reader).

>
>> And see comments below.
>>
>> Thanks
>>
>>
>>> ---
>>>   net/Makefile.objs    |   1 +
>>>   net/traffic-mirror.c | 173
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   qemu-options.hx      |   5 ++
>>>   vl.c                 |   3 +-
>>>   4 files changed, 181 insertions(+), 1 deletion(-)
>>>   create mode 100644 net/traffic-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
>> Let's s/traffic-mirror/filter-mirror/g to be consistent with other
>> filters.
>>
>
> OK~ I will fix it in V3
>
>>> diff --git a/net/traffic-mirror.c b/net/traffic-mirror.c
>>> new file mode 100644
>>> index 0000000..bed915c
>>> --- /dev/null
>>> +++ b/net/traffic-mirror.c
>>> @@ -0,0 +1,173 @@
>>> +/*
>>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
>>> + * Copyright (c) 2016 FUJITSU LIMITED
>>> + * Copyright (c) 2016 Intel Corporation
>>> + *
>>> + * Author: Zhang Chen <zhangchen.fnst@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/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"
>>> +
>>> +#define FILTER_TRAFFIC_MIRROR(obj) \
>>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_TRAFFIC_MIRROR)
>>> +
>>> +#define TYPE_FILTER_TRAFFIC_MIRROR "traffic-mirror"
>>> +
>>> +typedef struct MirrorState {
>>> +    NetFilterState parent_obj;
>>> +    char *outdev;
>>> +    CharDriverState *chr_out;
>>> +
>>> +} MirrorState;
>>> +
>>> +static ssize_t traffic_mirror_send(NetFilterState *nf,
>>> +                                   const struct iovec *iov,
>>> +                                   int iovcnt)
>>> +{
>>> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
>>> +    ssize_t ret = 0;
>>> +    ssize_t size = 0;
>>> +    char *buf;
>>> +
>>> +    size = iov_size(iov, iovcnt);
>>> +    if (!size) {
>>> +        return 0;
>>> +    }
>>> +
>>> +    buf = g_malloc0(size);
>>> +    iov_to_buf(iov, iovcnt, 0, buf, size);
>>> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)&size,
>>> sizeof(size));
>> htonl(size)?
>
> We do not need this.
>

Why? Did you test your mirroring on the wire?

Thanks
Zhang Chen Jan. 28, 2016, 9:52 a.m. UTC | #5
On 01/28/2016 04:37 PM, Jason Wang wrote:
>
> On 01/28/2016 03:44 PM, Zhang Chen wrote:
>>
>> On 01/28/2016 01:44 PM, Jason Wang wrote:
>>> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>>>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>
>>>> Traffic-mirror is a netfilter plugin.
>>>> It gives qemu the ability to copy and mirror guest's
>>>> net packet. we output packet to chardev.
>>>>
>>>> usage:
>>>>
>>>> -netdev tap,id=hn0
>>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>>>
>>>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>>> Thanks for the patch. Several questions:
>>>
>>> - I'm curious about how the patch was tested? Simple setup e.g:
>>>
>>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
>>> socket,id=c0,host=localhost,port=4444,server,nowait -object
>>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
>>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>>>
>>> does not works for me.
>> I test it in this way.
>> primary:
>> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
>> socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
>>   -object traffic-mirror,id=f0,netdev=hn0,queue=tx,outdev=mirror0
>>
>> secondary:
>> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
>> socket,id=mirror0,host=3.3.3.3,port=9003 -object
>> traffic-reader,id=f1,netdev=hn0,queue=rx,indev=mirror0
>>
>> I write a traffic-reader demo to read chardev socket and print it in
>> monitor.
> Ok, but maybe you can try socket backend. I think the protocol should be
> at least compatible with it.

Yes,I will try it.

>>
>>> - Is a reliable mirroring (e.g no packet drops during mirroring) is
>>> needed for COLO? If yes, this patch seems could not guarantee this.
>> I will fix it in V3
>>
>>> - Please consider to write a unit test for this patch.
>> write a unit test like tests/test-netfilter.c ?
> Even more for its basic function to work. E.g, start qemu with:
>
> -netdev socket,id=s0,listen=localhost:X -chardev
> socket,id=c0,host=localhost,pory=Y,server,nowait -object
> filter-mirror,netdev=hn0,outdev=c0
>
> Then you can inject packet from the socket connected to s0 and see if
> you can read it from socket that connected from c0 (or your traffic reader).

I got it~ will add test in next version.

>>> And see comments below.
>>>
>>> Thanks
>>>
>>>
>>>> ---
>>>>    net/Makefile.objs    |   1 +
>>>>    net/traffic-mirror.c | 173
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    qemu-options.hx      |   5 ++
>>>>    vl.c                 |   3 +-
>>>>    4 files changed, 181 insertions(+), 1 deletion(-)
>>>>    create mode 100644 net/traffic-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
>>> Let's s/traffic-mirror/filter-mirror/g to be consistent with other
>>> filters.
>>>
>> OK~ I will fix it in V3
>>
>>>> diff --git a/net/traffic-mirror.c b/net/traffic-mirror.c
>>>> new file mode 100644
>>>> index 0000000..bed915c
>>>> --- /dev/null
>>>> +++ b/net/traffic-mirror.c
>>>> @@ -0,0 +1,173 @@
>>>> +/*
>>>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
>>>> + * Copyright (c) 2016 FUJITSU LIMITED
>>>> + * Copyright (c) 2016 Intel Corporation
>>>> + *
>>>> + * Author: Zhang Chen <zhangchen.fnst@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/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"
>>>> +
>>>> +#define FILTER_TRAFFIC_MIRROR(obj) \
>>>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_TRAFFIC_MIRROR)
>>>> +
>>>> +#define TYPE_FILTER_TRAFFIC_MIRROR "traffic-mirror"
>>>> +
>>>> +typedef struct MirrorState {
>>>> +    NetFilterState parent_obj;
>>>> +    char *outdev;
>>>> +    CharDriverState *chr_out;
>>>> +
>>>> +} MirrorState;
>>>> +
>>>> +static ssize_t traffic_mirror_send(NetFilterState *nf,
>>>> +                                   const struct iovec *iov,
>>>> +                                   int iovcnt)
>>>> +{
>>>> +    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
>>>> +    ssize_t ret = 0;
>>>> +    ssize_t size = 0;
>>>> +    char *buf;
>>>> +
>>>> +    size = iov_size(iov, iovcnt);
>>>> +    if (!size) {
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    buf = g_malloc0(size);
>>>> +    iov_to_buf(iov, iovcnt, 0, buf, size);
>>>> +    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)&size,
>>>> sizeof(size));
>>> htonl(size)?
>> We do not need this.
>>
> Why? Did you test your mirroring on the wire?

Oh, you are right. I will fix it.
I have test it in same endian computer.so it work.
but difference endian not.

Thanks
zhangchen

> Thanks
>
>
> .
>
Jason Wang Jan. 28, 2016, 10:06 a.m. UTC | #6
On 01/28/2016 05:52 PM, Zhang Chen wrote:
>
>
> On 01/28/2016 04:37 PM, Jason Wang wrote:
>>
>> On 01/28/2016 03:44 PM, Zhang Chen wrote:
>>>
>>> On 01/28/2016 01:44 PM, Jason Wang wrote:
>>>> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>>>>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>>
>>>>> Traffic-mirror is a netfilter plugin.
>>>>> It gives qemu the ability to copy and mirror guest's
>>>>> net packet. we output packet to chardev.
>>>>>
>>>>> usage:
>>>>>
>>>>> -netdev tap,id=hn0
>>>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>>>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>>>>
>>>>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>>>> Thanks for the patch. Several questions:
>>>>
>>>> - I'm curious about how the patch was tested? Simple setup e.g:
>>>>
>>>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
>>>> socket,id=c0,host=localhost,port=4444,server,nowait -object
>>>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
>>>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>>>>
>>>> does not works for me.
>>> I test it in this way.
>>> primary:
>>> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
>>> socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
>>>   -object traffic-mirror,id=f0,netdev=hn0,queue=tx,outdev=mirror0
>>>
>>> secondary:
>>> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
>>> socket,id=mirror0,host=3.3.3.3,port=9003 -object
>>> traffic-reader,id=f1,netdev=hn0,queue=rx,indev=mirror0
>>>
>>> I write a traffic-reader demo to read chardev socket and print it in
>>> monitor.
>> Ok, but maybe you can try socket backend. I think the protocol should be
>> at least compatible with it.
>
> Yes,I will try it.
>
>>>
>>>> - Is a reliable mirroring (e.g no packet drops during mirroring) is
>>>> needed for COLO? If yes, this patch seems could not guarantee this.
>>> I will fix it in V3
>>>
>>>> - Please consider to write a unit test for this patch.
>>> write a unit test like tests/test-netfilter.c ?
>> Even more for its basic function to work. E.g, start qemu with:
>>
>> -netdev socket,id=s0,listen=localhost:X -chardev
>> socket,id=c0,host=localhost,pory=Y,server,nowait -object
>> filter-mirror,netdev=hn0,outdev=c0
>>
>> Then you can inject packet from the socket connected to s0 and see if
>> you can read it from socket that connected from c0 (or your traffic
>> reader).
>
> I got it~ will add test in next version. 

Thanks and make it a separate patch on top.
Li Zhijian Jan. 29, 2016, 1:38 a.m. UTC | #7
On 01/28/2016 01:44 PM, Jason Wang wrote:
>
>
> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>
>> Traffic-mirror is a netfilter plugin.
>> It gives qemu the ability to copy and mirror guest's
>> net packet. we output packet to chardev.
>>
>> usage:
>>
>> -netdev tap,id=hn0
>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>
>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>
> Thanks for the patch. Several questions:
>
> - I'm curious about how the patch was tested? Simple setup e.g:
>
> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
> socket,id=c0,host=localhost,port=4444,server,nowait -object
> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>
> does not works for me.
Hi, Jason

I just test the mirror using the command line above, it don't work too.
I am looking to it, and find that seems because the -net socket problem that
I have ever post a patch  try to fix(refer to ↓)
[Qemu-devel] [PATCH] report a error message if -net socket can not connect to server
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00758.html

after applying this patch, the qemu monitor tell me following message:
(qemu) qemu-system-x86_64: net socket is not connected Connection refused


Thanks
Li Zhijian
Jason Wang Feb. 1, 2016, 2:57 a.m. UTC | #8
On 01/29/2016 09:38 AM, Li Zhijian wrote:
>
>
> On 01/28/2016 01:44 PM, Jason Wang wrote:
>>
>>
>> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>
>>> Traffic-mirror is a netfilter plugin.
>>> It gives qemu the ability to copy and mirror guest's
>>> net packet. we output packet to chardev.
>>>
>>> usage:
>>>
>>> -netdev tap,id=hn0
>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>>
>>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>>
>> Thanks for the patch. Several questions:
>>
>> - I'm curious about how the patch was tested? Simple setup e.g:
>>
>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
>> socket,id=c0,host=localhost,port=4444,server,nowait -object
>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>>
>> does not works for me.
> Hi, Jason
>
> I just test the mirror using the command line above, it don't work too.
> I am looking to it, and find that seems because the -net socket
> problem that
> I have ever post a patch  try to fix(refer to ↓)
> [Qemu-devel] [PATCH] report a error message if -net socket can not
> connect to server
> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00758.html

Will have a look at this.

>
> after applying this patch, the qemu monitor tell me following message:
> (qemu) qemu-system-x86_64: net socket is not connected Connection refused

Maybe two issues. Have you tired to start the mirror on one VM and then
using socket backend to connect it from another VM?

>
>
> Thanks
> Li Zhijian
>
>
>
Li Zhijian Feb. 1, 2016, 7:50 a.m. UTC | #9
On 02/01/2016 10:57 AM, Jason Wang wrote:
>
>
> On 01/29/2016 09:38 AM, Li Zhijian wrote:
>>
>>
>> On 01/28/2016 01:44 PM, Jason Wang wrote:
>>>
>>>
>>> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>>>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>
>>>> Traffic-mirror is a netfilter plugin.
>>>> It gives qemu the ability to copy and mirror guest's
>>>> net packet. we output packet to chardev.
>>>>
>>>> usage:
>>>>
>>>> -netdev tap,id=hn0
>>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>>>
>>>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>>>
>>> Thanks for the patch. Several questions:
>>>
>>> - I'm curious about how the patch was tested? Simple setup e.g:
>>>
>>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
>>> socket,id=c0,host=localhost,port=4444,server,nowait -object
>>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
>>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>>>

a strange thing is about "host=localhost", connection is refused at SUSE 11.3 but
connection is connected successfully at Ubuntu 15.10 if i launch qemu with the
command line above.
I try to launch qemu at three physical machines installed with SUSE 11.3, they all
connect failed. But when I specified "host=127.0.0.1", the connection is OK.

I have comfirmed that:
- "localhost have pointed to 127.0.0.1 if I "ping localhost" at SUSE
- "telnet localhost 4444" works at SUSE


>>> does not works for me.
>> Hi, Jason
>>
>> I just test the mirror using the command line above, it don't work too.
>> I am looking to it, and find that seems because the -net socket
>> problem that
>> I have ever post a patch  try to fix(refer to ↓)
>> [Qemu-devel] [PATCH] report a error message if -net socket can not
>> connect to server
>> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00758.html
>
> Will have a look at this.
>
>>
>> after applying this patch, the qemu monitor tell me following message:
>> (qemu) qemu-system-x86_64: net socket is not connected Connection refused
>
> Maybe two issues. Have you tired to start the mirror on one VM and then
> using socket backend to connect it from another VM?

Yes, if i connect the mirror on VM1 using socket backend from another VM2, the connection
is established successfully. But on VM2 guest, I can't dump any packet using 'tcpdump'
That's because in current version code, mirror is not compatible with socket backend and
we will fix it in next version.


Best regards.
Li Zhijian

>
>>
>>
>> Thanks
>> Li Zhijian
>>
>>
>>
>
>
>
> .
>
Dr. David Alan Gilbert Feb. 1, 2016, 9:11 a.m. UTC | #10
* Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
> 
> 
> On 02/01/2016 10:57 AM, Jason Wang wrote:
> >
> >
> >On 01/29/2016 09:38 AM, Li Zhijian wrote:
> >>
> >>
> >>On 01/28/2016 01:44 PM, Jason Wang wrote:
> >>>
> >>>
> >>>On 01/27/2016 10:40 AM, Zhang Chen wrote:
> >>>>From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
> >>>>
> >>>>Traffic-mirror is a netfilter plugin.
> >>>>It gives qemu the ability to copy and mirror guest's
> >>>>net packet. we output packet to chardev.
> >>>>
> >>>>usage:
> >>>>
> >>>>-netdev tap,id=hn0
> >>>>-chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
> >>>>-traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
> >>>>
> >>>>Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
> >>>>Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> >>>>Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
> >>>
> >>>Thanks for the patch. Several questions:
> >>>
> >>>- I'm curious about how the patch was tested? Simple setup e.g:
> >>>
> >>>-netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
> >>>socket,id=c0,host=localhost,port=4444,server,nowait -object
> >>>traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
> >>>socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
> >>>
> 
> a strange thing is about "host=localhost", connection is refused at SUSE 11.3 but
> connection is connected successfully at Ubuntu 15.10 if i launch qemu with the
> command line above.
> I try to launch qemu at three physical machines installed with SUSE 11.3, they all
> connect failed. But when I specified "host=127.0.0.1", the connection is OK.
> 
> I have comfirmed that:
> - "localhost have pointed to 127.0.0.1 if I "ping localhost" at SUSE
> - "telnet localhost 4444" works at SUSE

My guess is that it's IPv6 related; check the /etc/hosts so see if there's
a ::1 entry for localhost; I've seen some weird behaviour on rhel in the
same way but in other uses.

Dave

> 
> >>>does not works for me.
> >>Hi, Jason
> >>
> >>I just test the mirror using the command line above, it don't work too.
> >>I am looking to it, and find that seems because the -net socket
> >>problem that
> >>I have ever post a patch  try to fix(refer to ↓)
> >>[Qemu-devel] [PATCH] report a error message if -net socket can not
> >>connect to server
> >>https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00758.html
> >
> >Will have a look at this.
> >
> >>
> >>after applying this patch, the qemu monitor tell me following message:
> >>(qemu) qemu-system-x86_64: net socket is not connected Connection refused
> >
> >Maybe two issues. Have you tired to start the mirror on one VM and then
> >using socket backend to connect it from another VM?
> 
> Yes, if i connect the mirror on VM1 using socket backend from another VM2, the connection
> is established successfully. But on VM2 guest, I can't dump any packet using 'tcpdump'
> That's because in current version code, mirror is not compatible with socket backend and
> we will fix it in next version.
> 
> 
> Best regards.
> Li Zhijian
> 
> >
> >>
> >>
> >>Thanks
> >>Li Zhijian
> >>
> >>
> >>
> >
> >
> >
> >.
> >
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Li Zhijian Feb. 1, 2016, 9:42 a.m. UTC | #11
On 02/01/2016 05:11 PM, Dr. David Alan Gilbert wrote:
> * Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
>>
>>
>> On 02/01/2016 10:57 AM, Jason Wang wrote:
>>>
>>>
>>> On 01/29/2016 09:38 AM, Li Zhijian wrote:
>>>>
>>>>
>>>> On 01/28/2016 01:44 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>>>>>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>>>
>>>>>> Traffic-mirror is a netfilter plugin.
>>>>>> It gives qemu the ability to copy and mirror guest's
>>>>>> net packet. we output packet to chardev.
>>>>>>
>>>>>> usage:
>>>>>>
>>>>>> -netdev tap,id=hn0
>>>>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>>>>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>>>>>
>>>>>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>>>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>>>>>
>>>>> Thanks for the patch. Several questions:
>>>>>
>>>>> - I'm curious about how the patch was tested? Simple setup e.g:
>>>>>
>>>>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
>>>>> socket,id=c0,host=localhost,port=4444,server,nowait -object
>>>>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
>>>>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>>>>>
>>
>> a strange thing is about "host=localhost", connection is refused at SUSE 11.3 but
>> connection is connected successfully at Ubuntu 15.10 if i launch qemu with the
>> command line above.
>> I try to launch qemu at three physical machines installed with SUSE 11.3, they all
>> connect failed. But when I specified "host=127.0.0.1", the connection is OK.
>>
>> I have comfirmed that:
>> - "localhost have pointed to 127.0.0.1 if I "ping localhost" at SUSE
>> - "telnet localhost 4444" works at SUSE
>
> My guess is that it's IPv6 related; check the /etc/hosts so see if there's
> a ::1 entry for localhost; I've seen some weird behaviour on rhel in the
> same way but in other uses.

Thank you Dave,
As you said, there are 2 entry record (ipv4 and ipv6) for "localhost" at my /etc/hosts
after removing the ipv6 entry, the whole world become fine ^_^

Thanks
Li Zhijian

>
> Dave
>
>>
>>>>> does not works for me.
>>>> Hi, Jason
>>>>
>>>> I just test the mirror using the command line above, it don't work too.
>>>> I am looking to it, and find that seems because the -net socket
>>>> problem that
>>>> I have ever post a patch  try to fix(refer to ↓)
>>>> [Qemu-devel] [PATCH] report a error message if -net socket can not
>>>> connect to server
>>>> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00758.html
>>>
>>> Will have a look at this.
>>>
>>>>
>>>> after applying this patch, the qemu monitor tell me following message:
>>>> (qemu) qemu-system-x86_64: net socket is not connected Connection refused
>>>
>>> Maybe two issues. Have you tired to start the mirror on one VM and then
>>> using socket backend to connect it from another VM?
>>
>> Yes, if i connect the mirror on VM1 using socket backend from another VM2, the connection
>> is established successfully. But on VM2 guest, I can't dump any packet using 'tcpdump'
>> That's because in current version code, mirror is not compatible with socket backend and
>> we will fix it in next version.
>>
>>
>> Best regards.
>> Li Zhijian
>>
>>>
>>>>
>>>>
>>>> Thanks
>>>> Li Zhijian
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> .
>>>
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
> .
>
diff mbox

Patch

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
diff --git a/net/traffic-mirror.c b/net/traffic-mirror.c
new file mode 100644
index 0000000..bed915c
--- /dev/null
+++ b/net/traffic-mirror.c
@@ -0,0 +1,173 @@ 
+/*
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ * Copyright (c) 2016 FUJITSU LIMITED
+ * Copyright (c) 2016 Intel Corporation
+ *
+ * Author: Zhang Chen <zhangchen.fnst@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/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"
+
+#define FILTER_TRAFFIC_MIRROR(obj) \
+    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_TRAFFIC_MIRROR)
+
+#define TYPE_FILTER_TRAFFIC_MIRROR "traffic-mirror"
+
+typedef struct MirrorState {
+    NetFilterState parent_obj;
+    char *outdev;
+    CharDriverState *chr_out;
+
+} MirrorState;
+
+static ssize_t traffic_mirror_send(NetFilterState *nf,
+                                   const struct iovec *iov,
+                                   int iovcnt)
+{
+    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
+    ssize_t ret = 0;
+    ssize_t size = 0;
+    char *buf;
+
+    size = iov_size(iov, iovcnt);
+    if (!size) {
+        return 0;
+    }
+
+    buf = g_malloc0(size);
+    iov_to_buf(iov, iovcnt, 0, buf, size);
+    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)&size, sizeof(size));
+    if (ret < 0) {
+        g_free(buf);
+        return ret;
+    }
+
+    ret = qemu_chr_fe_write(s->chr_out, (uint8_t *)buf, size);
+    g_free(buf);
+    return ret;
+}
+
+static ssize_t traffic_mirror_receive_iov(NetFilterState *nf,
+                                         NetClientState *sender,
+                                         unsigned flags,
+                                         const struct iovec *iov,
+                                         int iovcnt,
+                                         NetPacketSent *sent_cb)
+{
+    /*
+     * We copy and mirror packet to outdev,
+     * then put back the packet.
+     */
+    ssize_t ret = 0;
+
+    ret = traffic_mirror_send(nf, iov, iovcnt);
+    if (ret < 0) {
+        error_report("traffic_mirror_send failed");
+    }
+
+    return 0;
+}
+
+static void traffic_mirror_cleanup(NetFilterState *nf)
+{
+    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
+
+    if (s->chr_out) {
+        qemu_chr_fe_release(s->chr_out);
+    }
+}
+
+static void traffic_mirror_setup(NetFilterState *nf, Error **errp)
+{
+    MirrorState *s = FILTER_TRAFFIC_MIRROR(nf);
+
+    if (!s->outdev) {
+        error_setg(errp, "filter traffic mirror needs 'outdev' "
+                "property set!");
+        return;
+    }
+
+    s->chr_out = qemu_chr_find(s->outdev);
+    if (s->chr_out == NULL) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", s->outdev);
+        return;
+    }
+
+    if (qemu_chr_fe_claim(s->chr_out) != 0) {
+        error_setg(errp, QERR_DEVICE_IN_USE, s->outdev);
+        return;
+    }
+}
+
+static void traffic_mirror_class_init(ObjectClass *oc, void *data)
+{
+    NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+    nfc->setup = traffic_mirror_setup;
+    nfc->cleanup = traffic_mirror_cleanup;
+    nfc->receive_iov = traffic_mirror_receive_iov;
+}
+
+static char *traffic_mirror_get_outdev(Object *obj, Error **errp)
+{
+    MirrorState *s = FILTER_TRAFFIC_MIRROR(obj);
+
+    return g_strdup(s->outdev);
+}
+
+static void
+traffic_mirror_set_outdev(Object *obj, const char *value, Error **errp)
+{
+    MirrorState *s = FILTER_TRAFFIC_MIRROR(obj);
+
+    g_free(s->outdev);
+    s->outdev = g_strdup(value);
+    if (!s->outdev) {
+        error_setg(errp, "filter traffic mirror needs 'outdev' "
+                "property set!");
+        return;
+    }
+}
+
+static void traffic_mirror_init(Object *obj)
+{
+    object_property_add_str(obj, "outdev", traffic_mirror_get_outdev,
+                            traffic_mirror_set_outdev, NULL);
+}
+
+static void traffic_mirror_fini(Object *obj)
+{
+    MirrorState *s = FILTER_TRAFFIC_MIRROR(obj);
+
+    g_free(s->outdev);
+}
+
+static const TypeInfo traffic_mirror_info = {
+    .name = TYPE_FILTER_TRAFFIC_MIRROR,
+    .parent = TYPE_NETFILTER,
+    .class_init = traffic_mirror_class_init,
+    .instance_init = traffic_mirror_init,
+    .instance_finalize = traffic_mirror_fini,
+    .instance_size = sizeof(MirrorState),
+};
+
+static void register_types(void)
+{
+    type_register_static(&traffic_mirror_info);
+}
+
+type_init(register_types);
diff --git a/qemu-options.hx b/qemu-options.hx
index 0eea4ee..6fd2d46 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3670,6 +3670,11 @@  queue @var{all|rx|tx} is an option that can be applied to any netfilter.
 @option{tx}: the filter is attached to the transmit queue of the netdev,
              where it will receive packets sent by the netdev.
 
+@item -object traffic-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
+
+traffic-mirror on netdev @var{netdevid},mirror net packet to outdev.
+queue @var{all|rx|tx} is an option that can be applied to traffic-mirror.
+
 @item -object filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
 
 Dump the network traffic on netdev @var{dev} to the file specified by
diff --git a/vl.c b/vl.c
index 8dc34ce..413d73a 100644
--- a/vl.c
+++ b/vl.c
@@ -2838,7 +2838,8 @@  static bool object_create_initial(const char *type)
      * they depend on netdevs already existing
      */
     if (g_str_equal(type, "filter-buffer") ||
-        g_str_equal(type, "filter-dump")) {
+        g_str_equal(type, "filter-dump") ||
+        g_str_equal(type, "traffic-mirror")) {
         return false;
     }