[Qemu-devel] [PATCH V3 0/3] Introduce filter-redirector

[Qemu-devel] [PATCH V3 0/3] Introduce filter-redirector

[Zhang Chen]

Filter-redirector is a netfilter plugin.
It gives qemu the ability to redirect net packet.
redirector can redirect filter's net packet to outdev.
and redirect indev's packet to filter.

                      filter
                        +
                        |
                        |
            redirector  |
               +--------------+
               |        |     |
               |        |     |
               |        |     |
  indev +-----------+   +---------->  outdev
               |    |         |
               |    |         |
               |    |         |
               +--------------+
                    |
                    |
                    v
                  filter


v3:
 -Address Jason's comments.

v2:
 - Address Jason's comments.
 - Add filter-traffic.h to reuse parts of the codes
 - Add unit test case

v1:
 initial patch.


Zhang Chen (3):
  net/filter-mirror: Change filter_mirror_send interface
  net/filter-mirror:Add filter-redirector func
  tests/test-filter-redirector: Add unit test for filter-redirector

 net/filter-mirror.c            | 221 ++++++++++++++++++++++++++++++++++++++++-
 qemu-options.hx                |   8 ++
 tests/.gitignore               |   1 +
 tests/Makefile                 |   2 +
 tests/test-filter-redirector.c | 214 +++++++++++++++++++++++++++++++++++++++
 vl.c                           |   3 +-
 6 files changed, 443 insertions(+), 6 deletions(-)
 create mode 100644 tests/test-filter-redirector.c

-- 
1.9.1

[Qemu-devel] [PATCH V3 1/3] net/filter-mirror: Change filter_mirror_send interface

[Zhang Chen]

Change filter_mirror_send interface to make it easier
to used by other filter

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 net/filter-mirror.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index ee13d94..4ff7619 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -33,11 +33,10 @@ typedef struct MirrorState {
     CharDriverState *chr_out;
 } MirrorState;
 
-static int filter_mirror_send(NetFilterState *nf,
+static int filter_mirror_send(CharDriverState *chr_out,
                                    const struct iovec *iov,
                                    int iovcnt)
 {
-    MirrorState *s = FILTER_MIRROR(nf);
     int ret = 0;
     ssize_t size = 0;
     uint32_t len =  0;
@@ -49,14 +48,14 @@ static int filter_mirror_send(NetFilterState *nf,
     }
 
     len = htonl(size);
-    ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)&len, sizeof(len));
+    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, sizeof(len));
     if (ret != sizeof(len)) {
         goto err;
     }
 
     buf = g_malloc(size);
     iov_to_buf(iov, iovcnt, 0, buf, size);
-    ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)buf, size);
+    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
     g_free(buf);
     if (ret != size) {
         goto err;
@@ -75,9 +74,10 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
                                          int iovcnt,
                                          NetPacketSent *sent_cb)
 {
+    MirrorState *s = FILTER_MIRROR(nf);
     int ret;
 
-    ret = filter_mirror_send(nf, iov, iovcnt);
+    ret = filter_mirror_send(s->chr_out, iov, iovcnt);
     if (ret) {
         error_report("filter_mirror_send failed(%s)", strerror(-ret));
     }
-- 
1.9.1

Re: [Qemu-devel] [PATCH V3 1/3] net/filter-mirror: Change filter_mirror_send interface

[Jason Wang]

On 03/04/2016 08:01 PM, Zhang Chen wrote:
> Change filter_mirror_send interface to make it easier
> to used by other filter
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  net/filter-mirror.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index ee13d94..4ff7619 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -33,11 +33,10 @@ typedef struct MirrorState {
>      CharDriverState *chr_out;
>  } MirrorState;
>  
> -static int filter_mirror_send(NetFilterState *nf,
> +static int filter_mirror_send(CharDriverState *chr_out,
>                                     const struct iovec *iov,
>                                     int iovcnt)

The indent looks odds here, please send an independent patch to fix this
by the way.

Thanks

Re: [Qemu-devel] [PATCH V3 1/3] net/filter-mirror: Change filter_mirror_send interface

[Zhang Chen]

On 03/07/2016 03:29 PM, Jason Wang wrote:
>
> On 03/04/2016 08:01 PM, Zhang Chen wrote:
>> Change filter_mirror_send interface to make it easier
>> to used by other filter
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index ee13d94..4ff7619 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -33,11 +33,10 @@ typedef struct MirrorState {
>>       CharDriverState *chr_out;
>>   } MirrorState;
>>   
>> -static int filter_mirror_send(NetFilterState *nf,
>> +static int filter_mirror_send(CharDriverState *chr_out,
>>                                      const struct iovec *iov,
>>                                      int iovcnt)
> The indent looks odds here, please send an independent patch to fix this
> by the way.

OK, I will fix it and send an independent patch latter~~

Thanks
zhangchen

> Thanks
>
>
> .
>

-- 
Thanks
zhangchen

[Qemu-devel] [PATCH V3 2/3] net/filter-mirror:Add filter-redirector func

[Zhang Chen]

Filter-redirector is a netfilter plugin.
It gives qemu the ability to redirect net packet.
redirector can redirect filter's net packet to outdev.
and redirect indev's packet to filter.

                      filter
                        +
                        |
                        |
            redirector  |
               +--------------+
               |        |     |
               |        |     |
               |        |     |
  indev +-----------+   +---------->  outdev
               |    |         |
               |    |         |
               |    |         |
               +--------------+
                    |
                    |
                    v
                  filter

usage:

-netdev user,id=hn0
-chardev socket,id=s0,host=ip_primary,port=X,server,nowait
-chardev socket,id=s1,host=ip_primary,port=Y,server,nowait
-filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 net/filter-mirror.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx     |   8 ++
 vl.c                |   3 +-
 3 files changed, 221 insertions(+), 1 deletion(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 4ff7619..d137168 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -25,11 +25,19 @@
 #define FILTER_MIRROR(obj) \
     OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
 
+#define FILTER_REDIRECTOR(obj) \
+    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR)
+
 #define TYPE_FILTER_MIRROR "filter-mirror"
+#define TYPE_FILTER_REDIRECTOR "filter-redirector"
+#define REDIRECT_HEADER_LEN sizeof(uint32_t)
 
 typedef struct MirrorState {
     NetFilterState parent_obj;
+    NetQueue *incoming_queue;
+    char *indev;
     char *outdev;
+    CharDriverState *chr_in;
     CharDriverState *chr_out;
 } MirrorState;
 
@@ -67,6 +75,68 @@ err:
     return ret < 0 ? ret : -EIO;
 }
 
+static int redirector_chr_can_read(void *opaque)
+{
+    return REDIRECT_HEADER_LEN;
+}
+
+static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
+{
+    NetFilterState *nf = opaque;
+    MirrorState *s = FILTER_REDIRECTOR(nf);
+    uint32_t len;
+    int ret = 0;
+    uint8_t *recv_buf;
+
+    memcpy(&len, buf, size);
+    if (size < REDIRECT_HEADER_LEN) {
+        ret = qemu_chr_fe_read_all(s->chr_in, ((uint8_t *)&len) + size,
+                REDIRECT_HEADER_LEN - size);
+        if (ret != (REDIRECT_HEADER_LEN - size)) {
+            error_report("filter-redirector recv size failed");
+            return;
+        }
+    }
+
+    len = ntohl(len);
+
+    if (len > 0 && len < NET_BUFSIZE) {
+        recv_buf = g_malloc(len);
+        ret = qemu_chr_fe_read_all(s->chr_in, recv_buf, len);
+        if (ret != len) {
+            error_report("filter-redirector recv buf failed");
+            g_free(recv_buf);
+            return;
+        }
+
+        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
+                nf->direction == NET_FILTER_DIRECTION_TX) {
+            ret = qemu_net_queue_send(s->incoming_queue, nf->netdev,
+                            0, (const uint8_t *)recv_buf, len, NULL);
+
+            if (ret < 0) {
+                /*
+                  *FIXME: just report an error, let NET_FILTER_DIRECTION_RX have chance to pass
+                  * to next filter ?
+                  */
+                error_report("filter-redirector out to guest failed");
+            }
+        }
+
+        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
+                nf->direction == NET_FILTER_DIRECTION_RX) {
+            struct iovec iov = {
+                .iov_base = recv_buf,
+                .iov_len = sizeof(recv_buf)
+            };
+            qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
+        }
+        g_free(recv_buf);
+    } else {
+        error_report("filter-redirector recv len failed");
+    }
+}
+
 static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
                                          NetClientState *sender,
                                          unsigned flags,
@@ -89,6 +159,27 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
     return 0;
 }
 
+static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
+                                         NetClientState *sender,
+                                         unsigned flags,
+                                         const struct iovec *iov,
+                                         int iovcnt,
+                                         NetPacketSent *sent_cb)
+{
+    MirrorState *s = FILTER_REDIRECTOR(nf);
+    int ret;
+
+    if (s->chr_out) {
+        ret = filter_mirror_send(s->chr_out, iov, iovcnt);
+        if (ret) {
+            error_report("filter_mirror_send failed(%s)", strerror(-ret));
+        }
+        return iov_size(iov, iovcnt);
+    } else {
+        return 0;
+    }
+}
+
 static void filter_mirror_cleanup(NetFilterState *nf)
 {
     MirrorState *s = FILTER_MIRROR(nf);
@@ -98,6 +189,18 @@ static void filter_mirror_cleanup(NetFilterState *nf)
     }
 }
 
+static void filter_redirector_cleanup(NetFilterState *nf)
+{
+    MirrorState *s = FILTER_REDIRECTOR(nf);
+
+    if (s->chr_in) {
+        qemu_chr_fe_release(s->chr_in);
+    }
+    if (s->chr_out) {
+        qemu_chr_fe_release(s->chr_out);
+    }
+}
+
 static void filter_mirror_setup(NetFilterState *nf, Error **errp)
 {
     MirrorState *s = FILTER_MIRROR(nf);
@@ -121,6 +224,47 @@ static void filter_mirror_setup(NetFilterState *nf, Error **errp)
     }
 }
 
+static void filter_redirector_setup(NetFilterState *nf, Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(nf);
+
+    if (!s->indev && !s->outdev) {
+        error_setg(errp, "filter redirector needs 'indev' or "
+                "'outdev' at least one property set");
+        return;
+    } else if (s->indev && s->outdev) {
+        if (!strcmp(s->indev, s->outdev)) {
+            error_setg(errp, "filter redirector needs 'indev' and "
+                    "'outdev'  are not same");
+            return;
+        }
+    }
+
+    if (s->indev) {
+        s->chr_in = qemu_chr_find(s->indev);
+        if (s->chr_in == NULL) {
+            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                      "IN Device '%s' not found", s->indev);
+            return;
+        }
+
+        qemu_chr_fe_claim_no_fail(s->chr_in);
+        qemu_chr_add_handlers(s->chr_in, redirector_chr_can_read,
+                                redirector_chr_read, NULL, nf);
+        s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
+    }
+
+    if (s->outdev) {
+        s->chr_out = qemu_chr_find(s->outdev);
+        if (s->chr_out == NULL) {
+            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                      "OUT Device '%s' not found", s->outdev);
+            return;
+        }
+        qemu_chr_fe_claim_no_fail(s->chr_out);
+    }
+}
+
 static void filter_mirror_class_init(ObjectClass *oc, void *data)
 {
     NetFilterClass *nfc = NETFILTER_CLASS(oc);
@@ -130,6 +274,31 @@ static void filter_mirror_class_init(ObjectClass *oc, void *data)
     nfc->receive_iov = filter_mirror_receive_iov;
 }
 
+static void filter_redirector_class_init(ObjectClass *oc, void *data)
+{
+    NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+    nfc->setup = filter_redirector_setup;
+    nfc->cleanup = filter_redirector_cleanup;
+    nfc->receive_iov = filter_redirector_receive_iov;
+}
+
+static char *filter_redirector_get_indev(Object *obj, Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    return g_strdup(s->indev);
+}
+
+static void
+filter_redirector_set_indev(Object *obj, const char *value, Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    g_free(s->indev);
+    s->indev = g_strdup(value);
+}
+
 static char *filter_mirror_get_outdev(Object *obj, Error **errp)
 {
     MirrorState *s = FILTER_MIRROR(obj);
@@ -151,12 +320,36 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
     }
 }
 
+static char *filter_redirector_get_outdev(Object *obj, Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    return g_strdup(s->outdev);
+}
+
+static void
+filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    g_free(s->outdev);
+    s->outdev = g_strdup(value);
+}
+
 static void filter_mirror_init(Object *obj)
 {
     object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
                             filter_mirror_set_outdev, NULL);
 }
 
+static void filter_redirector_init(Object *obj)
+{
+    object_property_add_str(obj, "indev", filter_redirector_get_indev,
+                            filter_redirector_set_indev, NULL);
+    object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
+                            filter_redirector_set_outdev, NULL);
+}
+
 static void filter_mirror_fini(Object *obj)
 {
     MirrorState *s = FILTER_MIRROR(obj);
@@ -164,6 +357,23 @@ static void filter_mirror_fini(Object *obj)
     g_free(s->outdev);
 }
 
+static void filter_redirector_fini(Object *obj)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    g_free(s->indev);
+    g_free(s->outdev);
+}
+
+static const TypeInfo filter_redirector_info = {
+    .name = TYPE_FILTER_REDIRECTOR,
+    .parent = TYPE_NETFILTER,
+    .class_init = filter_redirector_class_init,
+    .instance_init = filter_redirector_init,
+    .instance_finalize = filter_redirector_fini,
+    .instance_size = sizeof(MirrorState),
+};
+
 static const TypeInfo filter_mirror_info = {
     .name = TYPE_FILTER_MIRROR,
     .parent = TYPE_NETFILTER,
@@ -176,6 +386,7 @@ static const TypeInfo filter_mirror_info = {
 static void register_types(void)
 {
     type_register_static(&filter_mirror_info);
+    type_register_static(&filter_redirector_info);
 }
 
 type_init(register_types);
diff --git a/qemu-options.hx b/qemu-options.hx
index ca27863..84584e3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3764,6 +3764,14 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
 filter-mirror on netdev @var{netdevid},mirror net packet to chardev
 @var{chardevid}
 
+@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
+outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
+
+filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
+@var{chardevid},and redirect indev's packet to filter.
+Create a filter-redirector we need to differ outdev id from indev id, id can not
+be the same. we can just use indev or outdev, but at least one to be set.
+
 @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 d68533a..c389a3f 100644
--- a/vl.c
+++ b/vl.c
@@ -2799,7 +2799,8 @@ static bool object_create_initial(const char *type)
      */
     if (g_str_equal(type, "filter-buffer") ||
         g_str_equal(type, "filter-dump") ||
-        g_str_equal(type, "filter-mirror")) {
+        g_str_equal(type, "filter-mirror") ||
+        g_str_equal(type, "filter-redirector")) {
         return false;
     }
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH V3 2/3] net/filter-mirror:Add filter-redirector func

[Jason Wang]

On 03/04/2016 08:01 PM, Zhang Chen wrote:
> Filter-redirector is a netfilter plugin.
> It gives qemu the ability to redirect net packet.
> redirector can redirect filter's net packet to outdev.
> and redirect indev's packet to filter.
>
>                       filter
>                         +
>                         |
>                         |
>             redirector  |
>                +--------------+
>                |        |     |
>                |        |     |
>                |        |     |
>   indev +-----------+   +---------->  outdev
>                |    |         |
>                |    |         |
>                |    |         |
>                +--------------+
>                     |
>                     |
>                     v
>                   filter
>
> usage:
>
> -netdev user,id=hn0
> -chardev socket,id=s0,host=ip_primary,port=X,server,nowait
> -chardev socket,id=s1,host=ip_primary,port=Y,server,nowait
> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  net/filter-mirror.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx     |   8 ++
>  vl.c                |   3 +-
>  3 files changed, 221 insertions(+), 1 deletion(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 4ff7619..d137168 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -25,11 +25,19 @@
>  #define FILTER_MIRROR(obj) \
>      OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
>  
> +#define FILTER_REDIRECTOR(obj) \
> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR)
> +
>  #define TYPE_FILTER_MIRROR "filter-mirror"
> +#define TYPE_FILTER_REDIRECTOR "filter-redirector"
> +#define REDIRECT_HEADER_LEN sizeof(uint32_t)
>  
>  typedef struct MirrorState {
>      NetFilterState parent_obj;
> +    NetQueue *incoming_queue;
> +    char *indev;
>      char *outdev;
> +    CharDriverState *chr_in;
>      CharDriverState *chr_out;
>  } MirrorState;
>  
> @@ -67,6 +75,68 @@ err:
>      return ret < 0 ? ret : -EIO;
>  }
>  
> +static int redirector_chr_can_read(void *opaque)
> +{
> +    return REDIRECT_HEADER_LEN;
> +}
> +
> +static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
> +{
> +    NetFilterState *nf = opaque;
> +    MirrorState *s = FILTER_REDIRECTOR(nf);
> +    uint32_t len;
> +    int ret = 0;
> +    uint8_t *recv_buf;
> +
> +    memcpy(&len, buf, size);

stack overflow if size > sizeof(len)?

> +    if (size < REDIRECT_HEADER_LEN) {
> +        ret = qemu_chr_fe_read_all(s->chr_in, ((uint8_t *)&len) + size,
> +                REDIRECT_HEADER_LEN - size);

There maybe some misunderstanding for my previous reply. You can have a
look at net_socket_send() for reference. You could

- use a buffer for storing len
- each time when you receive partial len, store them in the buffer and
advance the pointer until you receive at least sizeof(len) bytes.

Then you can proceed and do something similar when you're receiving the
packet itself.

> +        if (ret != (REDIRECT_HEADER_LEN - size)) {
> +            error_report("filter-redirector recv size failed");
> +            return;
> +        }
> +    }
> +
> +    len = ntohl(len);
> +
> +    if (len > 0 && len < NET_BUFSIZE) {
> +        recv_buf = g_malloc(len);
> +        ret = qemu_chr_fe_read_all(s->chr_in, recv_buf, len);
> +        if (ret != len) {
> +            error_report("filter-redirector recv buf failed");
> +            g_free(recv_buf);
> +            return;
> +        }
> +
> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
> +                nf->direction == NET_FILTER_DIRECTION_TX) {
> +            ret = qemu_net_queue_send(s->incoming_queue, nf->netdev,
> +                            0, (const uint8_t *)recv_buf, len, NULL);

Rethink about this, we don't queue any packet in incoming_queue, so
probably there's no need to have a incoming_queue for redirector. We can
just use qemu_netfilter_pass_to_next() here.

> +
> +            if (ret < 0) {
> +                /*
> +                  *FIXME: just report an error, let NET_FILTER_DIRECTION_RX have chance to pass
> +                  * to next filter ?
> +                  */
> +                error_report("filter-redirector out to guest failed");
> +            }
> +        }
> +
> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
> +                nf->direction == NET_FILTER_DIRECTION_RX) {
> +            struct iovec iov = {
> +                .iov_base = recv_buf,
> +                .iov_len = sizeof(recv_buf)
> +            };
> +            qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);

Another issue not relate to this patch. Looks like we can rename this to
qemu_netfilter_iterate(), which seems better. Care to send a patch of this?

> +        }
> +        g_free(recv_buf);
> +    } else {
> +        error_report("filter-redirector recv len failed");

What will happen if the socket was closed at the other end? Can we get
size == 0 here? Btw, the grammar looks not correct :)

> +    }
> +}
> +
>  static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>                                           NetClientState *sender,
>                                           unsigned flags,
> @@ -89,6 +159,27 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>      return 0;
>  }
>  
> +static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
> +                                         NetClientState *sender,
> +                                         unsigned flags,
> +                                         const struct iovec *iov,
> +                                         int iovcnt,
> +                                         NetPacketSent *sent_cb)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(nf);
> +    int ret;
> +
> +    if (s->chr_out) {
> +        ret = filter_mirror_send(s->chr_out, iov, iovcnt);
> +        if (ret) {
> +            error_report("filter_mirror_send failed(%s)", strerror(-ret));
> +        }
> +        return iov_size(iov, iovcnt);
> +    } else {
> +        return 0;
> +    }
> +}
> +
>  static void filter_mirror_cleanup(NetFilterState *nf)
>  {
>      MirrorState *s = FILTER_MIRROR(nf);
> @@ -98,6 +189,18 @@ static void filter_mirror_cleanup(NetFilterState *nf)
>      }
>  }
>  
> +static void filter_redirector_cleanup(NetFilterState *nf)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(nf);
> +
> +    if (s->chr_in) {
> +        qemu_chr_fe_release(s->chr_in);
> +    }
> +    if (s->chr_out) {
> +        qemu_chr_fe_release(s->chr_out);
> +    }
> +}
> +
>  static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>  {
>      MirrorState *s = FILTER_MIRROR(nf);
> @@ -121,6 +224,47 @@ static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>      }
>  }
>  
> +static void filter_redirector_setup(NetFilterState *nf, Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(nf);
> +
> +    if (!s->indev && !s->outdev) {
> +        error_setg(errp, "filter redirector needs 'indev' or "
> +                "'outdev' at least one property set");
> +        return;
> +    } else if (s->indev && s->outdev) {
> +        if (!strcmp(s->indev, s->outdev)) {
> +            error_setg(errp, "filter redirector needs 'indev' and "
> +                    "'outdev'  are not same");
> +            return;
> +        }
> +    }
> +
> +    if (s->indev) {
> +        s->chr_in = qemu_chr_find(s->indev);
> +        if (s->chr_in == NULL) {
> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +                      "IN Device '%s' not found", s->indev);
> +            return;
> +        }
> +
> +        qemu_chr_fe_claim_no_fail(s->chr_in);
> +        qemu_chr_add_handlers(s->chr_in, redirector_chr_can_read,
> +                                redirector_chr_read, NULL, nf);
> +        s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
> +    }
> +
> +    if (s->outdev) {
> +        s->chr_out = qemu_chr_find(s->outdev);
> +        if (s->chr_out == NULL) {
> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +                      "OUT Device '%s' not found", s->outdev);
> +            return;
> +        }
> +        qemu_chr_fe_claim_no_fail(s->chr_out);
> +    }
> +}
> +
>  static void filter_mirror_class_init(ObjectClass *oc, void *data)
>  {
>      NetFilterClass *nfc = NETFILTER_CLASS(oc);
> @@ -130,6 +274,31 @@ static void filter_mirror_class_init(ObjectClass *oc, void *data)
>      nfc->receive_iov = filter_mirror_receive_iov;
>  }
>  
> +static void filter_redirector_class_init(ObjectClass *oc, void *data)
> +{
> +    NetFilterClass *nfc = NETFILTER_CLASS(oc);
> +
> +    nfc->setup = filter_redirector_setup;
> +    nfc->cleanup = filter_redirector_cleanup;
> +    nfc->receive_iov = filter_redirector_receive_iov;
> +}
> +
> +static char *filter_redirector_get_indev(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    return g_strdup(s->indev);
> +}
> +
> +static void
> +filter_redirector_set_indev(Object *obj, const char *value, Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    g_free(s->indev);
> +    s->indev = g_strdup(value);
> +}
> +
>  static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>  {
>      MirrorState *s = FILTER_MIRROR(obj);
> @@ -151,12 +320,36 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>      }
>  }
>  
> +static char *filter_redirector_get_outdev(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    return g_strdup(s->outdev);
> +}
> +
> +static void
> +filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    g_free(s->outdev);
> +    s->outdev = g_strdup(value);
> +}
> +
>  static void filter_mirror_init(Object *obj)
>  {
>      object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>                              filter_mirror_set_outdev, NULL);
>  }
>  
> +static void filter_redirector_init(Object *obj)
> +{
> +    object_property_add_str(obj, "indev", filter_redirector_get_indev,
> +                            filter_redirector_set_indev, NULL);
> +    object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
> +                            filter_redirector_set_outdev, NULL);
> +}
> +
>  static void filter_mirror_fini(Object *obj)
>  {
>      MirrorState *s = FILTER_MIRROR(obj);
> @@ -164,6 +357,23 @@ static void filter_mirror_fini(Object *obj)
>      g_free(s->outdev);
>  }
>  
> +static void filter_redirector_fini(Object *obj)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    g_free(s->indev);
> +    g_free(s->outdev);
> +}
> +
> +static const TypeInfo filter_redirector_info = {
> +    .name = TYPE_FILTER_REDIRECTOR,
> +    .parent = TYPE_NETFILTER,
> +    .class_init = filter_redirector_class_init,
> +    .instance_init = filter_redirector_init,
> +    .instance_finalize = filter_redirector_fini,
> +    .instance_size = sizeof(MirrorState),
> +};
> +
>  static const TypeInfo filter_mirror_info = {
>      .name = TYPE_FILTER_MIRROR,
>      .parent = TYPE_NETFILTER,
> @@ -176,6 +386,7 @@ static const TypeInfo filter_mirror_info = {
>  static void register_types(void)
>  {
>      type_register_static(&filter_mirror_info);
> +    type_register_static(&filter_redirector_info);
>  }
>  
>  type_init(register_types);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ca27863..84584e3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3764,6 +3764,14 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>  filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>  @var{chardevid}
>  
> +@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
> +outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
> +
> +filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
> +@var{chardevid},and redirect indev's packet to filter.
> +Create a filter-redirector we need to differ outdev id from indev id, id can not
> +be the same. we can just use indev or outdev, but at least one to be set.

Not a native English speaker, but this looks better:

At least one of indev or outdev need to be specified.

> +
>  @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 d68533a..c389a3f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2799,7 +2799,8 @@ static bool object_create_initial(const char *type)
>       */
>      if (g_str_equal(type, "filter-buffer") ||
>          g_str_equal(type, "filter-dump") ||
> -        g_str_equal(type, "filter-mirror")) {
> +        g_str_equal(type, "filter-mirror") ||
> +        g_str_equal(type, "filter-redirector")) {
>          return false;
>      }
>  

Re: [Qemu-devel] [PATCH V3 2/3] net/filter-mirror:Add filter-redirector func

[Li Zhijian]

On 03/07/2016 03:56 PM, Jason Wang wrote:
>
>
> On 03/04/2016 08:01 PM, Zhang Chen wrote:
>> Filter-redirector is a netfilter plugin.
>> It gives qemu the ability to redirect net packet.
>> redirector can redirect filter's net packet to outdev.
>> and redirect indev's packet to filter.
>>
>>                        filter
>>                          +
>>                          |
>>                          |
>>              redirector  |
>>                 +--------------+
>>                 |        |     |
>>                 |        |     |
>>                 |        |     |
>>    indev +-----------+   +---------->  outdev
>>                 |    |         |
>>                 |    |         |
>>                 |    |         |
>>                 +--------------+
>>                      |
>>                      |
>>                      v
>>                    filter
>>
>> usage:
>>
>> -netdev user,id=hn0
>> -chardev socket,id=s0,host=ip_primary,port=X,server,nowait
>> -chardev socket,id=s1,host=ip_primary,port=Y,server,nowait
>> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-options.hx     |   8 ++
>>   vl.c                |   3 +-
>>   3 files changed, 221 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 4ff7619..d137168 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -25,11 +25,19 @@
>>   #define FILTER_MIRROR(obj) \
>>       OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
>>
>> +#define FILTER_REDIRECTOR(obj) \
>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR)
>> +
>>   #define TYPE_FILTER_MIRROR "filter-mirror"
>> +#define TYPE_FILTER_REDIRECTOR "filter-redirector"
>> +#define REDIRECT_HEADER_LEN sizeof(uint32_t)
>>
>>   typedef struct MirrorState {
>>       NetFilterState parent_obj;
>> +    NetQueue *incoming_queue;
>> +    char *indev;
>>       char *outdev;
>> +    CharDriverState *chr_in;
>>       CharDriverState *chr_out;
>>   } MirrorState;
>>
>> @@ -67,6 +75,68 @@ err:
>>       return ret < 0 ? ret : -EIO;
>>   }
>>
>> +static int redirector_chr_can_read(void *opaque)
>> +{
>> +    return REDIRECT_HEADER_LEN;
>> +}
>> +
>> +static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
>> +{
>> +    NetFilterState *nf = opaque;
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +    uint32_t len;
>> +    int ret = 0;
>> +    uint8_t *recv_buf;
>> +
>> +    memcpy(&len, buf, size);
>
> stack overflow if size > sizeof(len)?
IIUC, it seems never happend because the 'size' will never greater than the return value of
redirector_chr_can_read() which will always return REDIRECT_HEADER_LEN ?


>
>> +    if (size < REDIRECT_HEADER_LEN) {
>> +        ret = qemu_chr_fe_read_all(s->chr_in, ((uint8_t *)&len) + size,
>> +                REDIRECT_HEADER_LEN - size);
>
> There maybe some misunderstanding for my previous reply. You can have a
> look at net_socket_send() for reference. You could
>
> - use a buffer for storing len
> - each time when you receive partial len, store them in the buffer and
> advance the pointer until you receive at least sizeof(len) bytes.
qemu_chr_fe_read_all() seem have done this work. Do you mean that
we implement a similar code to do that instead of qemu_chr_fe_read_all()

thanks
Li Zhijian

>
> Then you can proceed and do something similar when you're receiving the
> packet itself.
>
>> +        if (ret != (REDIRECT_HEADER_LEN - size)) {
>> +            error_report("filter-redirector recv size failed");
>> +            return;
>> +        }
>> +    }
>> +
>> +    len = ntohl(len);
>> +
>> +    if (len > 0 && len < NET_BUFSIZE) {
>> +        recv_buf = g_malloc(len);
>> +        ret = qemu_chr_fe_read_all(s->chr_in, recv_buf, len);
>> +        if (ret != len) {
>> +            error_report("filter-redirector recv buf failed");
>> +            g_free(recv_buf);
>> +            return;
>> +        }
>> +
>> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +                nf->direction == NET_FILTER_DIRECTION_TX) {
>> +            ret = qemu_net_queue_send(s->incoming_queue, nf->netdev,
>> +                            0, (const uint8_t *)recv_buf, len, NULL);
>
> Rethink about this, we don't queue any packet in incoming_queue, so
> probably there's no need to have a incoming_queue for redirector. We can
> just use qemu_netfilter_pass_to_next() here.
>
>> +
>> +            if (ret < 0) {
>> +                /*
>> +                  *FIXME: just report an error, let NET_FILTER_DIRECTION_RX have chance to pass
>> +                  * to next filter ?
>> +                  */
>> +                error_report("filter-redirector out to guest failed");
>> +            }
>> +        }
>> +
>> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +                nf->direction == NET_FILTER_DIRECTION_RX) {
>> +            struct iovec iov = {
>> +                .iov_base = recv_buf,
>> +                .iov_len = sizeof(recv_buf)
>> +            };
>> +            qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
>
> Another issue not relate to this patch. Looks like we can rename this to
> qemu_netfilter_iterate(), which seems better. Care to send a patch of this?
>
>> +        }
>> +        g_free(recv_buf);
>> +    } else {
>> +        error_report("filter-redirector recv len failed");
>
> What will happen if the socket was closed at the other end? Can we get
> size == 0 here? Btw, the grammar looks not correct :)
>
>> +    }
>> +}
>> +
>>   static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>                                            NetClientState *sender,
>>                                            unsigned flags,
>> @@ -89,6 +159,27 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>       return 0;
>>   }
>>
>> +static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
>> +                                         NetClientState *sender,
>> +                                         unsigned flags,
>> +                                         const struct iovec *iov,
>> +                                         int iovcnt,
>> +                                         NetPacketSent *sent_cb)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +    int ret;
>> +
>> +    if (s->chr_out) {
>> +        ret = filter_mirror_send(s->chr_out, iov, iovcnt);
>> +        if (ret) {
>> +            error_report("filter_mirror_send failed(%s)", strerror(-ret));
>> +        }
>> +        return iov_size(iov, iovcnt);
>> +    } else {
>> +        return 0;
>> +    }
>> +}
>> +
>>   static void filter_mirror_cleanup(NetFilterState *nf)
>>   {
>>       MirrorState *s = FILTER_MIRROR(nf);
>> @@ -98,6 +189,18 @@ static void filter_mirror_cleanup(NetFilterState *nf)
>>       }
>>   }
>>
>> +static void filter_redirector_cleanup(NetFilterState *nf)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +
>> +    if (s->chr_in) {
>> +        qemu_chr_fe_release(s->chr_in);
>> +    }
>> +    if (s->chr_out) {
>> +        qemu_chr_fe_release(s->chr_out);
>> +    }
>> +}
>> +
>>   static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>>   {
>>       MirrorState *s = FILTER_MIRROR(nf);
>> @@ -121,6 +224,47 @@ static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>>       }
>>   }
>>
>> +static void filter_redirector_setup(NetFilterState *nf, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +
>> +    if (!s->indev && !s->outdev) {
>> +        error_setg(errp, "filter redirector needs 'indev' or "
>> +                "'outdev' at least one property set");
>> +        return;
>> +    } else if (s->indev && s->outdev) {
>> +        if (!strcmp(s->indev, s->outdev)) {
>> +            error_setg(errp, "filter redirector needs 'indev' and "
>> +                    "'outdev'  are not same");
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (s->indev) {
>> +        s->chr_in = qemu_chr_find(s->indev);
>> +        if (s->chr_in == NULL) {
>> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> +                      "IN Device '%s' not found", s->indev);
>> +            return;
>> +        }
>> +
>> +        qemu_chr_fe_claim_no_fail(s->chr_in);
>> +        qemu_chr_add_handlers(s->chr_in, redirector_chr_can_read,
>> +                                redirector_chr_read, NULL, nf);
>> +        s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
>> +    }
>> +
>> +    if (s->outdev) {
>> +        s->chr_out = qemu_chr_find(s->outdev);
>> +        if (s->chr_out == NULL) {
>> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> +                      "OUT Device '%s' not found", s->outdev);
>> +            return;
>> +        }
>> +        qemu_chr_fe_claim_no_fail(s->chr_out);
>> +    }
>> +}
>> +
>>   static void filter_mirror_class_init(ObjectClass *oc, void *data)
>>   {
>>       NetFilterClass *nfc = NETFILTER_CLASS(oc);
>> @@ -130,6 +274,31 @@ static void filter_mirror_class_init(ObjectClass *oc, void *data)
>>       nfc->receive_iov = filter_mirror_receive_iov;
>>   }
>>
>> +static void filter_redirector_class_init(ObjectClass *oc, void *data)
>> +{
>> +    NetFilterClass *nfc = NETFILTER_CLASS(oc);
>> +
>> +    nfc->setup = filter_redirector_setup;
>> +    nfc->cleanup = filter_redirector_cleanup;
>> +    nfc->receive_iov = filter_redirector_receive_iov;
>> +}
>> +
>> +static char *filter_redirector_get_indev(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    return g_strdup(s->indev);
>> +}
>> +
>> +static void
>> +filter_redirector_set_indev(Object *obj, const char *value, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->indev);
>> +    s->indev = g_strdup(value);
>> +}
>> +
>>   static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>>   {
>>       MirrorState *s = FILTER_MIRROR(obj);
>> @@ -151,12 +320,36 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>>       }
>>   }
>>
>> +static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    return g_strdup(s->outdev);
>> +}
>> +
>> +static void
>> +filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->outdev);
>> +    s->outdev = g_strdup(value);
>> +}
>> +
>>   static void filter_mirror_init(Object *obj)
>>   {
>>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>>                               filter_mirror_set_outdev, NULL);
>>   }
>>
>> +static void filter_redirector_init(Object *obj)
>> +{
>> +    object_property_add_str(obj, "indev", filter_redirector_get_indev,
>> +                            filter_redirector_set_indev, NULL);
>> +    object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
>> +                            filter_redirector_set_outdev, NULL);
>> +}
>> +
>>   static void filter_mirror_fini(Object *obj)
>>   {
>>       MirrorState *s = FILTER_MIRROR(obj);
>> @@ -164,6 +357,23 @@ static void filter_mirror_fini(Object *obj)
>>       g_free(s->outdev);
>>   }
>>
>> +static void filter_redirector_fini(Object *obj)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->indev);
>> +    g_free(s->outdev);
>> +}
>> +
>> +static const TypeInfo filter_redirector_info = {
>> +    .name = TYPE_FILTER_REDIRECTOR,
>> +    .parent = TYPE_NETFILTER,
>> +    .class_init = filter_redirector_class_init,
>> +    .instance_init = filter_redirector_init,
>> +    .instance_finalize = filter_redirector_fini,
>> +    .instance_size = sizeof(MirrorState),
>> +};
>> +
>>   static const TypeInfo filter_mirror_info = {
>>       .name = TYPE_FILTER_MIRROR,
>>       .parent = TYPE_NETFILTER,
>> @@ -176,6 +386,7 @@ static const TypeInfo filter_mirror_info = {
>>   static void register_types(void)
>>   {
>>       type_register_static(&filter_mirror_info);
>> +    type_register_static(&filter_redirector_info);
>>   }
>>
>>   type_init(register_types);
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ca27863..84584e3 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3764,6 +3764,14 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>>   filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>>   @var{chardevid}
>>
>> +@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>> +outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>> +
>> +filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
>> +@var{chardevid},and redirect indev's packet to filter.
>> +Create a filter-redirector we need to differ outdev id from indev id, id can not
>> +be the same. we can just use indev or outdev, but at least one to be set.
>
> Not a native English speaker, but this looks better:
>
> At least one of indev or outdev need to be specified.
>
>> +
>>   @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 d68533a..c389a3f 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2799,7 +2799,8 @@ static bool object_create_initial(const char *type)
>>        */
>>       if (g_str_equal(type, "filter-buffer") ||
>>           g_str_equal(type, "filter-dump") ||
>> -        g_str_equal(type, "filter-mirror")) {
>> +        g_str_equal(type, "filter-mirror") ||
>> +        g_str_equal(type, "filter-redirector")) {
>>           return false;
>>       }
>>
>
>
>
> .
>

-- 
Best regards.
Li Zhijian (8555)

Re: [Qemu-devel] [PATCH V3 2/3] net/filter-mirror:Add filter-redirector func

[Jason Wang]

On 03/07/2016 04:26 PM, Li Zhijian wrote:
>
>
> On 03/07/2016 03:56 PM, Jason Wang wrote:
>>
>>
>> On 03/04/2016 08:01 PM, Zhang Chen wrote:
>>> Filter-redirector is a netfilter plugin.
>>> It gives qemu the ability to redirect net packet.
>>> redirector can redirect filter's net packet to outdev.
>>> and redirect indev's packet to filter.
>>>
>>>                        filter
>>>                          +
>>>                          |
>>>                          |
>>>              redirector  |
>>>                 +--------------+
>>>                 |        |     |
>>>                 |        |     |
>>>                 |        |     |
>>>    indev +-----------+   +---------->  outdev
>>>                 |    |         |
>>>                 |    |         |
>>>                 |    |         |
>>>                 +--------------+
>>>                      |
>>>                      |
>>>                      v
>>>                    filter
>>>
>>> usage:
>>>
>>> -netdev user,id=hn0
>>> -chardev socket,id=s0,host=ip_primary,port=X,server,nowait
>>> -chardev socket,id=s1,host=ip_primary,port=Y,server,nowait
>>> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>> ---
>>>   net/filter-mirror.c | 211
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   qemu-options.hx     |   8 ++
>>>   vl.c                |   3 +-
>>>   3 files changed, 221 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>> index 4ff7619..d137168 100644
>>> --- a/net/filter-mirror.c
>>> +++ b/net/filter-mirror.c
>>> @@ -25,11 +25,19 @@
>>>   #define FILTER_MIRROR(obj) \
>>>       OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
>>>
>>> +#define FILTER_REDIRECTOR(obj) \
>>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR)
>>> +
>>>   #define TYPE_FILTER_MIRROR "filter-mirror"
>>> +#define TYPE_FILTER_REDIRECTOR "filter-redirector"
>>> +#define REDIRECT_HEADER_LEN sizeof(uint32_t)
>>>
>>>   typedef struct MirrorState {
>>>       NetFilterState parent_obj;
>>> +    NetQueue *incoming_queue;
>>> +    char *indev;
>>>       char *outdev;
>>> +    CharDriverState *chr_in;
>>>       CharDriverState *chr_out;
>>>   } MirrorState;
>>>
>>> @@ -67,6 +75,68 @@ err:
>>>       return ret < 0 ? ret : -EIO;
>>>   }
>>>
>>> +static int redirector_chr_can_read(void *opaque)
>>> +{
>>> +    return REDIRECT_HEADER_LEN;
>>> +}
>>> +
>>> +static void redirector_chr_read(void *opaque, const uint8_t *buf,
>>> int size)
>>> +{
>>> +    NetFilterState *nf = opaque;
>>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>>> +    uint32_t len;
>>> +    int ret = 0;
>>> +    uint8_t *recv_buf;
>>> +
>>> +    memcpy(&len, buf, size);
>>
>> stack overflow if size > sizeof(len)?
> IIUC, it seems never happend because the 'size' will never greater
> than the return value of
> redirector_chr_can_read() which will always return REDIRECT_HEADER_LEN ?

Right, so it's safe.

>
>
>>
>>> +    if (size < REDIRECT_HEADER_LEN) {
>>> +        ret = qemu_chr_fe_read_all(s->chr_in, ((uint8_t *)&len) +
>>> size,
>>> +                REDIRECT_HEADER_LEN - size);
>>
>> There maybe some misunderstanding for my previous reply. You can have a
>> look at net_socket_send() for reference. You could
>>
>> - use a buffer for storing len
>> - each time when you receive partial len, store them in the buffer and
>> advance the pointer until you receive at least sizeof(len) bytes.
> qemu_chr_fe_read_all() seem have done this work.

Not the same. qemu_chr_fe_read_all() will do loop reading and usleep
which is suboptimal. My proposal does not have this issue. It will make
redirector_chr_read() can handle arbitrary length of data and won't do
any busy reading.

> Do you mean that
> we implement a similar code to do that instead of qemu_chr_fe_read_all()

Nope, if you have a look at net_socket_send() it won't do any usleep and
loop reading, it will return immediately when it does not get sufficient
data. But it's really your call, not a must but worth to be optimized on
top in the future.

>
> thanks
> Li Zhijian

Re: [Qemu-devel] [PATCH V3 2/3] net/filter-mirror:Add filter-redirector func

[Li Zhijian]

On 03/07/2016 05:09 PM, Jason Wang wrote:
>
>
> On 03/07/2016 04:26 PM, Li Zhijian wrote:
>>
>>
>> On 03/07/2016 03:56 PM, Jason Wang wrote:
>>>
>>>
>>> On 03/04/2016 08:01 PM, Zhang Chen wrote:
>>>> Filter-redirector is a netfilter plugin.
>>>> It gives qemu the ability to redirect net packet.
>>>> redirector can redirect filter's net packet to outdev.
>>>> and redirect indev's packet to filter.
>>>>
>>>>                         filter
>>>>                           +
>>>>                           |
>>>>                           |
>>>>               redirector  |
>>>>                  +--------------+
>>>>                  |        |     |
>>>>                  |        |     |
>>>>                  |        |     |
>>>>     indev +-----------+   +---------->  outdev
>>>>                  |    |         |
>>>>                  |    |         |
>>>>                  |    |         |
>>>>                  +--------------+
>>>>                       |
>>>>                       |
>>>>                       v
>>>>                     filter
>>>>
>>>> usage:
>>>>
>>>> -netdev user,id=hn0
>>>> -chardev socket,id=s0,host=ip_primary,port=X,server,nowait
>>>> -chardev socket,id=s1,host=ip_primary,port=Y,server,nowait
>>>> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>> ---
>>>>    net/filter-mirror.c | 211
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    qemu-options.hx     |   8 ++
>>>>    vl.c                |   3 +-
>>>>    3 files changed, 221 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>> index 4ff7619..d137168 100644
>>>> --- a/net/filter-mirror.c
>>>> +++ b/net/filter-mirror.c
>>>> @@ -25,11 +25,19 @@
>>>>    #define FILTER_MIRROR(obj) \
>>>>        OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
>>>>
>>>> +#define FILTER_REDIRECTOR(obj) \
>>>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR)
>>>> +
>>>>    #define TYPE_FILTER_MIRROR "filter-mirror"
>>>> +#define TYPE_FILTER_REDIRECTOR "filter-redirector"
>>>> +#define REDIRECT_HEADER_LEN sizeof(uint32_t)
>>>>
>>>>    typedef struct MirrorState {
>>>>        NetFilterState parent_obj;
>>>> +    NetQueue *incoming_queue;
>>>> +    char *indev;
>>>>        char *outdev;
>>>> +    CharDriverState *chr_in;
>>>>        CharDriverState *chr_out;
>>>>    } MirrorState;
>>>>
>>>> @@ -67,6 +75,68 @@ err:
>>>>        return ret < 0 ? ret : -EIO;
>>>>    }
>>>>
>>>> +static int redirector_chr_can_read(void *opaque)
>>>> +{
>>>> +    return REDIRECT_HEADER_LEN;
>>>> +}
>>>> +
>>>> +static void redirector_chr_read(void *opaque, const uint8_t *buf,
>>>> int size)
>>>> +{
>>>> +    NetFilterState *nf = opaque;
>>>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>>>> +    uint32_t len;
>>>> +    int ret = 0;
>>>> +    uint8_t *recv_buf;
>>>> +
>>>> +    memcpy(&len, buf, size);
>>>
>>> stack overflow if size > sizeof(len)?
>> IIUC, it seems never happend because the 'size' will never greater
>> than the return value of
>> redirector_chr_can_read() which will always return REDIRECT_HEADER_LEN ?
>
> Right, so it's safe.
>
>>
>>
>>>
>>>> +    if (size < REDIRECT_HEADER_LEN) {
>>>> +        ret = qemu_chr_fe_read_all(s->chr_in, ((uint8_t *)&len) +
>>>> size,
>>>> +                REDIRECT_HEADER_LEN - size);
>>>
>>> There maybe some misunderstanding for my previous reply. You can have a
>>> look at net_socket_send() for reference. You could
>>>
>>> - use a buffer for storing len
>>> - each time when you receive partial len, store them in the buffer and
>>> advance the pointer until you receive at least sizeof(len) bytes.
>> qemu_chr_fe_read_all() seem have done this work.
>
> Not the same. qemu_chr_fe_read_all() will do loop reading and usleep
> which is suboptimal. My proposal does not have this issue. It will make
> redirector_chr_read() can handle arbitrary length of data and won't do
> any busy reading.
>
>> Do you mean that
>> we implement a similar code to do that instead of qemu_chr_fe_read_all()
>
> Nope, if you have a look at net_socket_send() it won't do any usleep and
> loop reading, it will return immediately when it does not get sufficient
> data. But it's really your call, not a must but worth to be optimized on
> top in the future.
>
Thanks for you explain.
Is it just like bellow code(not completed) ?

#define REDIRECTOR_MAX_LEN NET_BUFSIZE

typedef struct MirrorState {
      NetFilterState parent_obj;
      char *indev;
      char *outdev;
      CharDriverState *chr_in;
      CharDriverState *chr_out;
+    int state; /* 0 = getting length, 1 = getting data */
+    unsigned int index;
+    unsigned int packet_len;
+    uint8_t buf[REDIRECTOR_MAX_LEN];
  } MirrorState;


static int redirector_chr_can_read(void *opaque)
{
     return REDIRECTOR_MAX_LEN;
}

static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
{
     NetFilterState *nf = opaque;
     MirrorState *s = FILTER_REDIRECTOR(nf);
     unsigned int l;

     while (size > 0) {
         /* reassemble a packet from the network */
         switch (s->state) {
         case 0:
             l = 4 - s->index;
             if (l > size) {
                 l = size;
             }
             memcpy(s->buf + s->index, buf, l);
             buf += l;
             size -= l;
             s->index += l;
             if (s->index == 4) {
                 /* got length */
                 s->packet_len = ntohl(*(uint32_t *)s->buf);
                 s->index = 0;
                 s->state = 1;
             }
             break;
         case 1:
             l = s->packet_len - s->index;
             if (l > size) {
                 l = size;
             }
             if (s->index + l <= sizeof(s->buf)) {
                 memcpy(s->buf + s->index, buf, l);
             } else {
                 fprintf(stderr, "serious error: oversized packet received,"
                     "connection terminated.\n");
                 s->state = 0;
                 /* TODO: do something
                  */
             }

             s->index += l;
             buf += l;
             size -= l;
             if (s->index >= s->packet_len) {
                 s->index = 0;
                 s->state = 0;
                 /*
                  * TODO: pass packet to next filter
                   */
             }
             break;
         }
     }
}

Thanks
Li Zhijian

Re: [Qemu-devel] [PATCH V3 2/3] net/filter-mirror:Add filter-redirector func

[Zhang Chen]

On 03/07/2016 03:56 PM, Jason Wang wrote:
>
> On 03/04/2016 08:01 PM, Zhang Chen wrote:
>> Filter-redirector is a netfilter plugin.
>> It gives qemu the ability to redirect net packet.
>> redirector can redirect filter's net packet to outdev.
>> and redirect indev's packet to filter.
>>
>>                        filter
>>                          +
>>                          |
>>                          |
>>              redirector  |
>>                 +--------------+
>>                 |        |     |
>>                 |        |     |
>>                 |        |     |
>>    indev +-----------+   +---------->  outdev
>>                 |    |         |
>>                 |    |         |
>>                 |    |         |
>>                 +--------------+
>>                      |
>>                      |
>>                      v
>>                    filter
>>
>> usage:
>>
>> -netdev user,id=hn0
>> -chardev socket,id=s0,host=ip_primary,port=X,server,nowait
>> -chardev socket,id=s1,host=ip_primary,port=Y,server,nowait
>> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-options.hx     |   8 ++
>>   vl.c                |   3 +-
>>   3 files changed, 221 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 4ff7619..d137168 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -25,11 +25,19 @@
>>   #define FILTER_MIRROR(obj) \
>>       OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
>>   
>> +#define FILTER_REDIRECTOR(obj) \
>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR)
>> +
>>   #define TYPE_FILTER_MIRROR "filter-mirror"
>> +#define TYPE_FILTER_REDIRECTOR "filter-redirector"
>> +#define REDIRECT_HEADER_LEN sizeof(uint32_t)
>>   
>>   typedef struct MirrorState {
>>       NetFilterState parent_obj;
>> +    NetQueue *incoming_queue;
>> +    char *indev;
>>       char *outdev;
>> +    CharDriverState *chr_in;
>>       CharDriverState *chr_out;
>>   } MirrorState;
>>   
>> @@ -67,6 +75,68 @@ err:
>>       return ret < 0 ? ret : -EIO;
>>   }
>>   
>> +static int redirector_chr_can_read(void *opaque)
>> +{
>> +    return REDIRECT_HEADER_LEN;
>> +}
>> +
>> +static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
>> +{
>> +    NetFilterState *nf = opaque;
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +    uint32_t len;
>> +    int ret = 0;
>> +    uint8_t *recv_buf;
>> +
>> +    memcpy(&len, buf, size);
> stack overflow if size > sizeof(len)?
>> +    if (size < REDIRECT_HEADER_LEN) {
>> +        ret = qemu_chr_fe_read_all(s->chr_in, ((uint8_t *)&len) + size,
>> +                REDIRECT_HEADER_LEN - size);
> There maybe some misunderstanding for my previous reply. You can have a
> look at net_socket_send() for reference. You could
>
> - use a buffer for storing len
> - each time when you receive partial len, store them in the buffer and
> advance the pointer until you receive at least sizeof(len) bytes.
>
> Then you can proceed and do something similar when you're receiving the
> packet itself.
>> +        if (ret != (REDIRECT_HEADER_LEN - size)) {
>> +            error_report("filter-redirector recv size failed");
>> +            return;
>> +        }
>> +    }
>> +
>> +    len = ntohl(len);
>> +
>> +    if (len > 0 && len < NET_BUFSIZE) {
>> +        recv_buf = g_malloc(len);
>> +        ret = qemu_chr_fe_read_all(s->chr_in, recv_buf, len);
>> +        if (ret != len) {
>> +            error_report("filter-redirector recv buf failed");
>> +            g_free(recv_buf);
>> +            return;
>> +        }
>> +
>> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +                nf->direction == NET_FILTER_DIRECTION_TX) {
>> +            ret = qemu_net_queue_send(s->incoming_queue, nf->netdev,
>> +                            0, (const uint8_t *)recv_buf, len, NULL);
> Rethink about this, we don't queue any packet in incoming_queue, so
> probably there's no need to have a incoming_queue for redirector. We can
> just use qemu_netfilter_pass_to_next() here.

OK~~ I will fix it.

>
>> +
>> +            if (ret < 0) {
>> +                /*
>> +                  *FIXME: just report an error, let NET_FILTER_DIRECTION_RX have chance to pass
>> +                  * to next filter ?
>> +                  */
>> +                error_report("filter-redirector out to guest failed");
>> +            }
>> +        }
>> +
>> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +                nf->direction == NET_FILTER_DIRECTION_RX) {
>> +            struct iovec iov = {
>> +                .iov_base = recv_buf,
>> +                .iov_len = sizeof(recv_buf)
>> +            };
>> +            qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
> Another issue not relate to this patch. Looks like we can rename this to
> qemu_netfilter_iterate(), which seems better. Care to send a patch of this?

OK, I will send a patch for this~~

>
>> +        }
>> +        g_free(recv_buf);
>> +    } else {
>> +        error_report("filter-redirector recv len failed");
> What will happen if the socket was closed at the other end? Can we get
> size == 0 here? Btw, the grammar looks not correct :)

I will fix it in next version.

>> +    }
>> +}
>> +
>>   static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>                                            NetClientState *sender,
>>                                            unsigned flags,
>> @@ -89,6 +159,27 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>       return 0;
>>   }
>>   
>> +static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
>> +                                         NetClientState *sender,
>> +                                         unsigned flags,
>> +                                         const struct iovec *iov,
>> +                                         int iovcnt,
>> +                                         NetPacketSent *sent_cb)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +    int ret;
>> +
>> +    if (s->chr_out) {
>> +        ret = filter_mirror_send(s->chr_out, iov, iovcnt);
>> +        if (ret) {
>> +            error_report("filter_mirror_send failed(%s)", strerror(-ret));
>> +        }
>> +        return iov_size(iov, iovcnt);
>> +    } else {
>> +        return 0;
>> +    }
>> +}
>> +
>>   static void filter_mirror_cleanup(NetFilterState *nf)
>>   {
>>       MirrorState *s = FILTER_MIRROR(nf);
>> @@ -98,6 +189,18 @@ static void filter_mirror_cleanup(NetFilterState *nf)
>>       }
>>   }
>>   
>> +static void filter_redirector_cleanup(NetFilterState *nf)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +
>> +    if (s->chr_in) {
>> +        qemu_chr_fe_release(s->chr_in);
>> +    }
>> +    if (s->chr_out) {
>> +        qemu_chr_fe_release(s->chr_out);
>> +    }
>> +}
>> +
>>   static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>>   {
>>       MirrorState *s = FILTER_MIRROR(nf);
>> @@ -121,6 +224,47 @@ static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>>       }
>>   }
>>   
>> +static void filter_redirector_setup(NetFilterState *nf, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +
>> +    if (!s->indev && !s->outdev) {
>> +        error_setg(errp, "filter redirector needs 'indev' or "
>> +                "'outdev' at least one property set");
>> +        return;
>> +    } else if (s->indev && s->outdev) {
>> +        if (!strcmp(s->indev, s->outdev)) {
>> +            error_setg(errp, "filter redirector needs 'indev' and "
>> +                    "'outdev'  are not same");
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (s->indev) {
>> +        s->chr_in = qemu_chr_find(s->indev);
>> +        if (s->chr_in == NULL) {
>> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> +                      "IN Device '%s' not found", s->indev);
>> +            return;
>> +        }
>> +
>> +        qemu_chr_fe_claim_no_fail(s->chr_in);
>> +        qemu_chr_add_handlers(s->chr_in, redirector_chr_can_read,
>> +                                redirector_chr_read, NULL, nf);
>> +        s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
>> +    }
>> +
>> +    if (s->outdev) {
>> +        s->chr_out = qemu_chr_find(s->outdev);
>> +        if (s->chr_out == NULL) {
>> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> +                      "OUT Device '%s' not found", s->outdev);
>> +            return;
>> +        }
>> +        qemu_chr_fe_claim_no_fail(s->chr_out);
>> +    }
>> +}
>> +
>>   static void filter_mirror_class_init(ObjectClass *oc, void *data)
>>   {
>>       NetFilterClass *nfc = NETFILTER_CLASS(oc);
>> @@ -130,6 +274,31 @@ static void filter_mirror_class_init(ObjectClass *oc, void *data)
>>       nfc->receive_iov = filter_mirror_receive_iov;
>>   }
>>   
>> +static void filter_redirector_class_init(ObjectClass *oc, void *data)
>> +{
>> +    NetFilterClass *nfc = NETFILTER_CLASS(oc);
>> +
>> +    nfc->setup = filter_redirector_setup;
>> +    nfc->cleanup = filter_redirector_cleanup;
>> +    nfc->receive_iov = filter_redirector_receive_iov;
>> +}
>> +
>> +static char *filter_redirector_get_indev(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    return g_strdup(s->indev);
>> +}
>> +
>> +static void
>> +filter_redirector_set_indev(Object *obj, const char *value, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->indev);
>> +    s->indev = g_strdup(value);
>> +}
>> +
>>   static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>>   {
>>       MirrorState *s = FILTER_MIRROR(obj);
>> @@ -151,12 +320,36 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>>       }
>>   }
>>   
>> +static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    return g_strdup(s->outdev);
>> +}
>> +
>> +static void
>> +filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->outdev);
>> +    s->outdev = g_strdup(value);
>> +}
>> +
>>   static void filter_mirror_init(Object *obj)
>>   {
>>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>>                               filter_mirror_set_outdev, NULL);
>>   }
>>   
>> +static void filter_redirector_init(Object *obj)
>> +{
>> +    object_property_add_str(obj, "indev", filter_redirector_get_indev,
>> +                            filter_redirector_set_indev, NULL);
>> +    object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
>> +                            filter_redirector_set_outdev, NULL);
>> +}
>> +
>>   static void filter_mirror_fini(Object *obj)
>>   {
>>       MirrorState *s = FILTER_MIRROR(obj);
>> @@ -164,6 +357,23 @@ static void filter_mirror_fini(Object *obj)
>>       g_free(s->outdev);
>>   }
>>   
>> +static void filter_redirector_fini(Object *obj)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->indev);
>> +    g_free(s->outdev);
>> +}
>> +
>> +static const TypeInfo filter_redirector_info = {
>> +    .name = TYPE_FILTER_REDIRECTOR,
>> +    .parent = TYPE_NETFILTER,
>> +    .class_init = filter_redirector_class_init,
>> +    .instance_init = filter_redirector_init,
>> +    .instance_finalize = filter_redirector_fini,
>> +    .instance_size = sizeof(MirrorState),
>> +};
>> +
>>   static const TypeInfo filter_mirror_info = {
>>       .name = TYPE_FILTER_MIRROR,
>>       .parent = TYPE_NETFILTER,
>> @@ -176,6 +386,7 @@ static const TypeInfo filter_mirror_info = {
>>   static void register_types(void)
>>   {
>>       type_register_static(&filter_mirror_info);
>> +    type_register_static(&filter_redirector_info);
>>   }
>>   
>>   type_init(register_types);
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ca27863..84584e3 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3764,6 +3764,14 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>>   filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>>   @var{chardevid}
>>   
>> +@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>> +outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>> +
>> +filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
>> +@var{chardevid},and redirect indev's packet to filter.
>> +Create a filter-redirector we need to differ outdev id from indev id, id can not
>> +be the same. we can just use indev or outdev, but at least one to be set.
> Not a native English speaker, but this looks better:
>
> At least one of indev or outdev need to be specified.

I will fix it in next version.
Thanks
zhangchen

>
>> +
>>   @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 d68533a..c389a3f 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2799,7 +2799,8 @@ static bool object_create_initial(const char *type)
>>        */
>>       if (g_str_equal(type, "filter-buffer") ||
>>           g_str_equal(type, "filter-dump") ||
>> -        g_str_equal(type, "filter-mirror")) {
>> +        g_str_equal(type, "filter-mirror") ||
>> +        g_str_equal(type, "filter-redirector")) {
>>           return false;
>>       }
>>   
>
>
> .
>

-- 
Thanks
zhangchen

[Qemu-devel] [PATCH V3 3/3] tests/test-filter-redirector: Add unit test for filter-redirector

[Zhang Chen]

In this unit test,we will test the filter redirector function.

Case 1, tx traffic flow:

qemu side              | test side
                       |
+---------+            |  +-------+
| backend <---------------+ sock0 |
+----+----+            |  +-------+
     |                 |
+----v----+  +-------+ |
|  rd0    +->+chardev| |
+---------+  +---+---+ |
                 |     |
+---------+      |     |
|  rd1    <------+     |
+----+----+            |
     |                 |
+----v----+            |  +-------+
|  rd2    +--------------->sock1  |
+---------+            |  +-------+
                       +

Start qemu with:

"-netdev socket,id=qtest-bn0,fd=%d "
"-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
"-chardev socket,id=redirector0,path=%s,server,nowait "
"-chardev socket,id=redirector1,path=%s,server,nowait "
"-chardev socket,id=redirector2,path=%s,nowait "
"-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
"queue=tx,outdev=redirector0 "
"-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
"queue=tx,indev=redirector2 "
"-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
"queue=tx,outdev=redirector1 "

--------------------------------------
Case 2, rx traffic flow
qemu side              | test side
                       |
+---------+            |  +-------+
| backend +---------------> sock1 |
+----^----+            |  +-------+
     |                 |
+----+----+  +-------+ |
|  rd0    +<-+chardev| |
+---------+  +---+---+ |
                 ^     |
+---------+      |     |
|  rd1    +------+     |
+----^----+            |
     |                 |
+----+----+            |  +-------+
|  rd2    <---------------+sock0  |
+---------+            |  +-------+

Start qemu with:

"-netdev socket,id=qtest-bn0,fd=%d "
"-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
"-chardev socket,id=redirector0,path=%s,server,nowait "
"-chardev socket,id=redirector1,path=%s,server,nowait "
"-chardev socket,id=redirector2,path=%s,nowait "
"-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
"queue=rx,outdev=redirector0 "
"-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
"queue=rx,indev=redirector2 "
"-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
"queue=rx,outdev=redirector1 "

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 tests/.gitignore               |   1 +
 tests/Makefile                 |   2 +
 tests/test-filter-redirector.c | 214 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+)
 create mode 100644 tests/test-filter-redirector.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 10df017..5069d5d 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -64,5 +64,6 @@ test-x86-cpuid
 test-xbzrle
 test-netfilter
 test-filter-mirror
+test-filter-redirector
 *-test
 qapi-schema/*.test.*
diff --git a/tests/Makefile b/tests/Makefile
index e56c514..275c4cc 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -213,6 +213,7 @@ check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += tests/vhost-user-test$(EXE
 endif
 check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
+check-qtest-i386-y += tests/test-filter-redirector$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -565,6 +566,7 @@ tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(test-block-obj-y)
 tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
 tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
+tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
 tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
 
diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c
new file mode 100644
index 0000000..6074cd7
--- /dev/null
+++ b/tests/test-filter-redirector.c
@@ -0,0 +1,214 @@
+/*
+ * QTest testcase for filter-redirector
+ *
+ * Copyright (c) 2016 FUJITSU LIMITED
+ * 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.
+ *
+ * Case 1, tx traffic flow:
+ *
+ * qemu side              | test side
+ *                        |
+ * +---------+            |  +-------+
+ * | backend <---------------+ sock0 |
+ * +----+----+            |  +-------+
+ *      |                 |
+ * +----v----+  +-------+ |
+ * |  rd0    +->+chardev| |
+ * +---------+  +---+---+ |
+ *                  |     |
+ * +---------+      |     |
+ * |  rd1    <------+     |
+ * +----+----+            |
+ *      |                 |
+ * +----v----+            |  +-------+
+ * |  rd2    +--------------->sock1  |
+ * +---------+            |  +-------+
+ *                        +
+ *
+ * --------------------------------------
+ * Case 2, rx traffic flow
+ * qemu side              | test side
+ *                        |
+ * +---------+            |  +-------+
+ * | backend +---------------> sock1 |
+ * +----^----+            |  +-------+
+ *      |                 |
+ * +----+----+  +-------+ |
+ * |  rd0    +<-+chardev| |
+ * +---------+  +---+---+ |
+ *                  ^     |
+ * +---------+      |     |
+ * |  rd1    +------+     |
+ * +----^----+            |
+ *      |                 |
+ * +----+----+            |  +-------+
+ * |  rd2    <---------------+sock0  |
+ * +---------+            |  +-------+
+ *                        +
+ */
+
+#include <glib.h>
+#include "libqtest.h"
+#include "qemu/iov.h"
+#include "qemu/sockets.h"
+#include "qemu/error-report.h"
+#include "qemu/main-loop.h"
+
+static void test_redirector_tx(void)
+{
+#ifndef _WIN32
+/* socketpair(PF_UNIX) which does not exist on windows */
+
+    int backend_sock[2], recv_sock;
+    char *cmdline;
+    uint32_t ret = 0, len = 0;
+    char send_buf[] = "Hello!!";
+    char sock_path0[] = "filter-redirector0.XXXXXX";
+    char sock_path1[] = "filter-redirector1.XXXXXX";
+    char *recv_buf;
+    uint32_t size = sizeof(send_buf);
+    size = htonl(size);
+
+    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
+    g_assert_cmpint(ret, !=, -1);
+
+    ret = mkstemp(sock_path0);
+    g_assert_cmpint(ret, !=, -1);
+    ret = mkstemp(sock_path1);
+    g_assert_cmpint(ret, !=, -1);
+
+    cmdline = g_strdup_printf("-netdev socket,id=qtest-bn0,fd=%d "
+                "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
+                "-chardev socket,id=redirector0,path=%s,server,nowait "
+                "-chardev socket,id=redirector1,path=%s,server,nowait "
+                "-chardev socket,id=redirector2,path=%s,nowait "
+                "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
+                "queue=tx,outdev=redirector0 "
+                "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
+                "queue=tx,indev=redirector2 "
+                "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
+                "queue=tx,outdev=redirector1 "
+                , backend_sock[1], sock_path0, sock_path1, sock_path0);
+    qtest_start(cmdline);
+    g_free(cmdline);
+
+    recv_sock = unix_connect(sock_path1, NULL);
+    g_assert_cmpint(recv_sock, !=, -1);
+
+    struct iovec iov[] = {
+        {
+            .iov_base = &size,
+            .iov_len = sizeof(size),
+        }, {
+            .iov_base = send_buf,
+            .iov_len = sizeof(send_buf),
+        },
+    };
+
+    ret = iov_send(backend_sock[0], iov, 2, 0, sizeof(size) + sizeof(send_buf));
+    g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
+    close(backend_sock[0]);
+
+    ret = qemu_recv(recv_sock, &len, sizeof(len), 0);
+    g_assert_cmpint(ret, ==, sizeof(len));
+    len = ntohl(len);
+
+    g_assert_cmpint(len, ==, sizeof(send_buf));
+    recv_buf = g_malloc(len);
+    ret = qemu_recv(recv_sock, recv_buf, len, 0);
+    g_assert_cmpstr(recv_buf, ==, send_buf);
+
+    g_free(recv_buf);
+    close(recv_sock);
+    unlink(sock_path0);
+    unlink(sock_path1);
+    qtest_end();
+
+#endif
+}
+
+static void test_redirector_rx(void)
+{
+#ifndef _WIN32
+/* socketpair(PF_UNIX) which does not exist on windows */
+
+    int backend_sock[2],send_sock, recv_sock;
+    char *cmdline;
+    uint32_t ret = 0, len = 0;
+    char send_buf[] = "Hello!!";
+    char sock_path0[] = "filter-redirector0.XXXXXX";
+    char sock_path1[] = "filter-redirector1.XXXXXX";
+    char *recv_buf;
+    uint32_t size = sizeof(send_buf);
+    size = htonl(size);
+
+    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
+    g_assert_cmpint(ret, !=, -1);
+
+    ret = mkstemp(sock_path0);
+    g_assert_cmpint(ret, !=, -1);
+    ret = mkstemp(sock_path1);
+    g_assert_cmpint(ret, !=, -1);
+
+    cmdline = g_strdup_printf("-netdev socket,id=qtest-bn0,fd=%d "
+                "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
+                "-chardev socket,id=redirector0,path=%s,server,nowait "
+                "-chardev socket,id=redirector1,path=%s,server,nowait "
+                "-chardev socket,id=redirector2,path=%s,nowait "
+                "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
+                "queue=rx,indev=redirector0 "
+                "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
+                "queue=rx,outdev=redirector2 "
+                "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
+                "queue=rx,indev=redirector1 "
+                , backend_sock[1], sock_path0, sock_path1, sock_path0);
+    qtest_start(cmdline);
+    g_free(cmdline);
+
+    struct iovec iov[] = {
+        {
+            .iov_base = &size,
+            .iov_len = sizeof(size),
+        }, {
+            .iov_base = send_buf,
+            .iov_len = sizeof(send_buf),
+        },
+    };
+
+    send_sock = unix_connect(sock_path1, NULL);
+    ret = iov_send(send_sock, iov, 2, 0, sizeof(size) + sizeof(send_buf));
+    g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
+    close(send_sock);
+
+    ret = qemu_recv(backend_sock[0], &len, sizeof(len), 0);
+    g_assert_cmpint(ret, ==, sizeof(len));
+    len = ntohl(len);
+
+    g_assert_cmpint(len, ==, sizeof(send_buf));
+    recv_buf = g_malloc(len);
+    ret = qemu_recv(backend_sock[0], recv_buf, len, 0);
+    g_assert_cmpstr(recv_buf, ==, send_buf);
+
+    g_free(recv_buf);
+    close(recv_sock);
+    unlink(sock_path0);
+    unlink(sock_path1);
+    qtest_end();
+
+#endif
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/netfilter/redirector_tx", test_redirector_tx);
+    qtest_add_func("/netfilter/redirector_rx", test_redirector_rx);
+    ret = g_test_run();
+
+    return ret;
+}
-- 
1.9.1

Re: [Qemu-devel] [PATCH V3 3/3] tests/test-filter-redirector: Add unit test for filter-redirector

[Jason Wang]

On 03/04/2016 08:01 PM, Zhang Chen wrote:
> In this unit test,we will test the filter redirector function.
>
> Case 1, tx traffic flow:
>
> qemu side              | test side
>                        |
> +---------+            |  +-------+
> | backend <---------------+ sock0 |
> +----+----+            |  +-------+
>      |                 |
> +----v----+  +-------+ |
> |  rd0    +->+chardev| |
> +---------+  +---+---+ |
>                  |     |
> +---------+      |     |
> |  rd1    <------+     |
> +----+----+            |
>      |                 |
> +----v----+            |  +-------+
> |  rd2    +--------------->sock1  |
> +---------+            |  +-------+
>                        +
>
> Start qemu with:
>
> "-netdev socket,id=qtest-bn0,fd=%d "
> "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
> "-chardev socket,id=redirector0,path=%s,server,nowait "
> "-chardev socket,id=redirector1,path=%s,server,nowait "
> "-chardev socket,id=redirector2,path=%s,nowait "
> "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
> "queue=tx,outdev=redirector0 "
> "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
> "queue=tx,indev=redirector2 "
> "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
> "queue=tx,outdev=redirector1 "

Both the patch and the above figure looks good. But it would be still
better to use several sentences to explain the test.

Thanks

>
> --------------------------------------
> Case 2, rx traffic flow
> qemu side              | test side
>                        |
> +---------+            |  +-------+
> | backend +---------------> sock1 |
> +----^----+            |  +-------+
>      |                 |
> +----+----+  +-------+ |
> |  rd0    +<-+chardev| |
> +---------+  +---+---+ |
>                  ^     |
> +---------+      |     |
> |  rd1    +------+     |
> +----^----+            |
>      |                 |
> +----+----+            |  +-------+
> |  rd2    <---------------+sock0  |
> +---------+            |  +-------+
>
> Start qemu with:
>
> "-netdev socket,id=qtest-bn0,fd=%d "
> "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
> "-chardev socket,id=redirector0,path=%s,server,nowait "
> "-chardev socket,id=redirector1,path=%s,server,nowait "
> "-chardev socket,id=redirector2,path=%s,nowait "
> "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
> "queue=rx,outdev=redirector0 "
> "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
> "queue=rx,indev=redirector2 "
> "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
> "queue=rx,outdev=redirector1 "
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  tests/.gitignore               |   1 +
>  tests/Makefile                 |   2 +
>  tests/test-filter-redirector.c | 214 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 217 insertions(+)
>  create mode 100644 tests/test-filter-redirector.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 10df017..5069d5d 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -64,5 +64,6 @@ test-x86-cpuid
>  test-xbzrle
>  test-netfilter
>  test-filter-mirror
> +test-filter-redirector
>  *-test
>  qapi-schema/*.test.*
> diff --git a/tests/Makefile b/tests/Makefile
> index e56c514..275c4cc 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -213,6 +213,7 @@ check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += tests/vhost-user-test$(EXE
>  endif
>  check-qtest-i386-y += tests/test-netfilter$(EXESUF)
>  check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
> +check-qtest-i386-y += tests/test-filter-redirector$(EXESUF)
>  check-qtest-x86_64-y = $(check-qtest-i386-y)
>  gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
>  gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
> @@ -565,6 +566,7 @@ tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
>  tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(test-block-obj-y)
>  tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
>  tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
> +tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
>  tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
>  tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
>  
> diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c
> new file mode 100644
> index 0000000..6074cd7
> --- /dev/null
> +++ b/tests/test-filter-redirector.c
> @@ -0,0 +1,214 @@
> +/*
> + * QTest testcase for filter-redirector
> + *
> + * Copyright (c) 2016 FUJITSU LIMITED
> + * 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.
> + *
> + * Case 1, tx traffic flow:
> + *
> + * qemu side              | test side
> + *                        |
> + * +---------+            |  +-------+
> + * | backend <---------------+ sock0 |
> + * +----+----+            |  +-------+
> + *      |                 |
> + * +----v----+  +-------+ |
> + * |  rd0    +->+chardev| |
> + * +---------+  +---+---+ |
> + *                  |     |
> + * +---------+      |     |
> + * |  rd1    <------+     |
> + * +----+----+            |
> + *      |                 |
> + * +----v----+            |  +-------+
> + * |  rd2    +--------------->sock1  |
> + * +---------+            |  +-------+
> + *                        +
> + *
> + * --------------------------------------
> + * Case 2, rx traffic flow
> + * qemu side              | test side
> + *                        |
> + * +---------+            |  +-------+
> + * | backend +---------------> sock1 |
> + * +----^----+            |  +-------+
> + *      |                 |
> + * +----+----+  +-------+ |
> + * |  rd0    +<-+chardev| |
> + * +---------+  +---+---+ |
> + *                  ^     |
> + * +---------+      |     |
> + * |  rd1    +------+     |
> + * +----^----+            |
> + *      |                 |
> + * +----+----+            |  +-------+
> + * |  rd2    <---------------+sock0  |
> + * +---------+            |  +-------+
> + *                        +
> + */
> +
> +#include <glib.h>
> +#include "libqtest.h"
> +#include "qemu/iov.h"
> +#include "qemu/sockets.h"
> +#include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
> +
> +static void test_redirector_tx(void)
> +{
> +#ifndef _WIN32
> +/* socketpair(PF_UNIX) which does not exist on windows */
> +
> +    int backend_sock[2], recv_sock;
> +    char *cmdline;
> +    uint32_t ret = 0, len = 0;
> +    char send_buf[] = "Hello!!";
> +    char sock_path0[] = "filter-redirector0.XXXXXX";
> +    char sock_path1[] = "filter-redirector1.XXXXXX";
> +    char *recv_buf;
> +    uint32_t size = sizeof(send_buf);
> +    size = htonl(size);
> +
> +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
> +    g_assert_cmpint(ret, !=, -1);
> +
> +    ret = mkstemp(sock_path0);
> +    g_assert_cmpint(ret, !=, -1);
> +    ret = mkstemp(sock_path1);
> +    g_assert_cmpint(ret, !=, -1);
> +
> +    cmdline = g_strdup_printf("-netdev socket,id=qtest-bn0,fd=%d "
> +                "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
> +                "-chardev socket,id=redirector0,path=%s,server,nowait "
> +                "-chardev socket,id=redirector1,path=%s,server,nowait "
> +                "-chardev socket,id=redirector2,path=%s,nowait "
> +                "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
> +                "queue=tx,outdev=redirector0 "
> +                "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
> +                "queue=tx,indev=redirector2 "
> +                "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
> +                "queue=tx,outdev=redirector1 "
> +                , backend_sock[1], sock_path0, sock_path1, sock_path0);
> +    qtest_start(cmdline);
> +    g_free(cmdline);
> +
> +    recv_sock = unix_connect(sock_path1, NULL);
> +    g_assert_cmpint(recv_sock, !=, -1);
> +
> +    struct iovec iov[] = {
> +        {
> +            .iov_base = &size,
> +            .iov_len = sizeof(size),
> +        }, {
> +            .iov_base = send_buf,
> +            .iov_len = sizeof(send_buf),
> +        },
> +    };
> +
> +    ret = iov_send(backend_sock[0], iov, 2, 0, sizeof(size) + sizeof(send_buf));
> +    g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
> +    close(backend_sock[0]);
> +
> +    ret = qemu_recv(recv_sock, &len, sizeof(len), 0);
> +    g_assert_cmpint(ret, ==, sizeof(len));
> +    len = ntohl(len);
> +
> +    g_assert_cmpint(len, ==, sizeof(send_buf));
> +    recv_buf = g_malloc(len);
> +    ret = qemu_recv(recv_sock, recv_buf, len, 0);
> +    g_assert_cmpstr(recv_buf, ==, send_buf);
> +
> +    g_free(recv_buf);
> +    close(recv_sock);
> +    unlink(sock_path0);
> +    unlink(sock_path1);
> +    qtest_end();
> +
> +#endif
> +}
> +
> +static void test_redirector_rx(void)
> +{
> +#ifndef _WIN32
> +/* socketpair(PF_UNIX) which does not exist on windows */
> +
> +    int backend_sock[2],send_sock, recv_sock;
> +    char *cmdline;
> +    uint32_t ret = 0, len = 0;
> +    char send_buf[] = "Hello!!";
> +    char sock_path0[] = "filter-redirector0.XXXXXX";
> +    char sock_path1[] = "filter-redirector1.XXXXXX";
> +    char *recv_buf;
> +    uint32_t size = sizeof(send_buf);
> +    size = htonl(size);
> +
> +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
> +    g_assert_cmpint(ret, !=, -1);
> +
> +    ret = mkstemp(sock_path0);
> +    g_assert_cmpint(ret, !=, -1);
> +    ret = mkstemp(sock_path1);
> +    g_assert_cmpint(ret, !=, -1);
> +
> +    cmdline = g_strdup_printf("-netdev socket,id=qtest-bn0,fd=%d "
> +                "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
> +                "-chardev socket,id=redirector0,path=%s,server,nowait "
> +                "-chardev socket,id=redirector1,path=%s,server,nowait "
> +                "-chardev socket,id=redirector2,path=%s,nowait "
> +                "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
> +                "queue=rx,indev=redirector0 "
> +                "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
> +                "queue=rx,outdev=redirector2 "
> +                "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
> +                "queue=rx,indev=redirector1 "
> +                , backend_sock[1], sock_path0, sock_path1, sock_path0);
> +    qtest_start(cmdline);
> +    g_free(cmdline);
> +
> +    struct iovec iov[] = {
> +        {
> +            .iov_base = &size,
> +            .iov_len = sizeof(size),
> +        }, {
> +            .iov_base = send_buf,
> +            .iov_len = sizeof(send_buf),
> +        },
> +    };
> +
> +    send_sock = unix_connect(sock_path1, NULL);
> +    ret = iov_send(send_sock, iov, 2, 0, sizeof(size) + sizeof(send_buf));
> +    g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
> +    close(send_sock);
> +
> +    ret = qemu_recv(backend_sock[0], &len, sizeof(len), 0);
> +    g_assert_cmpint(ret, ==, sizeof(len));
> +    len = ntohl(len);
> +
> +    g_assert_cmpint(len, ==, sizeof(send_buf));
> +    recv_buf = g_malloc(len);
> +    ret = qemu_recv(backend_sock[0], recv_buf, len, 0);
> +    g_assert_cmpstr(recv_buf, ==, send_buf);
> +
> +    g_free(recv_buf);
> +    close(recv_sock);
> +    unlink(sock_path0);
> +    unlink(sock_path1);
> +    qtest_end();
> +
> +#endif
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    int ret;
> +
> +    g_test_init(&argc, &argv, NULL);
> +    qtest_add_func("/netfilter/redirector_tx", test_redirector_tx);
> +    qtest_add_func("/netfilter/redirector_rx", test_redirector_rx);
> +    ret = g_test_run();
> +
> +    return ret;
> +}

Re: [Qemu-devel] [PATCH V3 3/3] tests/test-filter-redirector: Add unit test for filter-redirector

[Zhang Chen]

On 03/07/2016 04:10 PM, Jason Wang wrote:
>
> On 03/04/2016 08:01 PM, Zhang Chen wrote:
>> In this unit test,we will test the filter redirector function.
>>
>> Case 1, tx traffic flow:
>>
>> qemu side              | test side
>>                         |
>> +---------+            |  +-------+
>> | backend <---------------+ sock0 |
>> +----+----+            |  +-------+
>>       |                 |
>> +----v----+  +-------+ |
>> |  rd0    +->+chardev| |
>> +---------+  +---+---+ |
>>                   |     |
>> +---------+      |     |
>> |  rd1    <------+     |
>> +----+----+            |
>>       |                 |
>> +----v----+            |  +-------+
>> |  rd2    +--------------->sock1  |
>> +---------+            |  +-------+
>>                         +
>>
>> Start qemu with:
>>
>> "-netdev socket,id=qtest-bn0,fd=%d"
>> "-device rtl8139,netdev=qtest-bn0,id=qtest-e0"
>> "-chardev socket,id=redirector0,path=%s,server,nowait"
>> "-chardev socket,id=redirector1,path=%s,server,nowait"
>> "-chardev socket,id=redirector2,path=%s,nowait"
>> "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
>> "queue=tx,outdev=redirector0"
>> "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
>> "queue=tx,indev=redirector2"
>> "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
>> "queue=tx,outdev=redirector1"
> Both the patch and the above figure looks good. But it would be still
> better to use several sentences to explain the test.

OK, I will fix it in next version.

Thanks
zhangchen

> Thanks
>
>> --------------------------------------
>> Case 2, rx traffic flow
>> qemu side              | test side
>>                         |
>> +---------+            |  +-------+
>> | backend +---------------> sock1 |
>> +----^----+            |  +-------+
>>       |                 |
>> +----+----+  +-------+ |
>> |  rd0    +<-+chardev| |
>> +---------+  +---+---+ |
>>                   ^     |
>> +---------+      |     |
>> |  rd1    +------+     |
>> +----^----+            |
>>       |                 |
>> +----+----+            |  +-------+
>> |  rd2    <---------------+sock0  |
>> +---------+            |  +-------+
>>
>> Start qemu with:
>>
>> "-netdev socket,id=qtest-bn0,fd=%d"
>> "-device rtl8139,netdev=qtest-bn0,id=qtest-e0"
>> "-chardev socket,id=redirector0,path=%s,server,nowait"
>> "-chardev socket,id=redirector1,path=%s,server,nowait"
>> "-chardev socket,id=redirector2,path=%s,nowait"
>> "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
>> "queue=rx,outdev=redirector0"
>> "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
>> "queue=rx,indev=redirector2"
>> "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
>> "queue=rx,outdev=redirector1"
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>   tests/.gitignore               |   1 +
>>   tests/Makefile                 |   2 +
>>   tests/test-filter-redirector.c | 214 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 217 insertions(+)
>>   create mode 100644 tests/test-filter-redirector.c
>>
>> diff --git a/tests/.gitignore b/tests/.gitignore
>> index 10df017..5069d5d 100644
>> --- a/tests/.gitignore
>> +++ b/tests/.gitignore
>> @@ -64,5 +64,6 @@ test-x86-cpuid
>>   test-xbzrle
>>   test-netfilter
>>   test-filter-mirror
>> +test-filter-redirector
>>   *-test
>>   qapi-schema/*.test.*
>> diff --git a/tests/Makefile b/tests/Makefile
>> index e56c514..275c4cc 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -213,6 +213,7 @@ check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += tests/vhost-user-test$(EXE
>>   endif
>>   check-qtest-i386-y += tests/test-netfilter$(EXESUF)
>>   check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
>> +check-qtest-i386-y += tests/test-filter-redirector$(EXESUF)
>>   check-qtest-x86_64-y = $(check-qtest-i386-y)
>>   gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
>>   gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
>> @@ -565,6 +566,7 @@ tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
>>   tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(test-block-obj-y)
>>   tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
>>   tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
>> +tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
>>   tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
>>   tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
>>   
>> diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c
>> new file mode 100644
>> index 0000000..6074cd7
>> --- /dev/null
>> +++ b/tests/test-filter-redirector.c
>> @@ -0,0 +1,214 @@
>> +/*
>> + * QTest testcase for filter-redirector
>> + *
>> + * Copyright (c) 2016 FUJITSU LIMITED
>> + * 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.
>> + *
>> + * Case 1, tx traffic flow:
>> + *
>> + * qemu side              | test side
>> + *                        |
>> + * +---------+            |  +-------+
>> + * | backend <---------------+ sock0 |
>> + * +----+----+            |  +-------+
>> + *      |                 |
>> + * +----v----+  +-------+ |
>> + * |  rd0    +->+chardev| |
>> + * +---------+  +---+---+ |
>> + *                  |     |
>> + * +---------+      |     |
>> + * |  rd1    <------+     |
>> + * +----+----+            |
>> + *      |                 |
>> + * +----v----+            |  +-------+
>> + * |  rd2    +--------------->sock1  |
>> + * +---------+            |  +-------+
>> + *                        +
>> + *
>> + * --------------------------------------
>> + * Case 2, rx traffic flow
>> + * qemu side              | test side
>> + *                        |
>> + * +---------+            |  +-------+
>> + * | backend +---------------> sock1 |
>> + * +----^----+            |  +-------+
>> + *      |                 |
>> + * +----+----+  +-------+ |
>> + * |  rd0    +<-+chardev| |
>> + * +---------+  +---+---+ |
>> + *                  ^     |
>> + * +---------+      |     |
>> + * |  rd1    +------+     |
>> + * +----^----+            |
>> + *      |                 |
>> + * +----+----+            |  +-------+
>> + * |  rd2    <---------------+sock0  |
>> + * +---------+            |  +-------+
>> + *                        +
>> + */
>> +
>> +#include <glib.h>
>> +#include "libqtest.h"
>> +#include "qemu/iov.h"
>> +#include "qemu/sockets.h"
>> +#include "qemu/error-report.h"
>> +#include "qemu/main-loop.h"
>> +
>> +static void test_redirector_tx(void)
>> +{
>> +#ifndef _WIN32
>> +/* socketpair(PF_UNIX) which does not exist on windows */
>> +
>> +    int backend_sock[2], recv_sock;
>> +    char *cmdline;
>> +    uint32_t ret = 0, len = 0;
>> +    char send_buf[] = "Hello!!";
>> +    char sock_path0[] = "filter-redirector0.XXXXXX";
>> +    char sock_path1[] = "filter-redirector1.XXXXXX";
>> +    char *recv_buf;
>> +    uint32_t size = sizeof(send_buf);
>> +    size = htonl(size);
>> +
>> +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
>> +    g_assert_cmpint(ret, !=, -1);
>> +
>> +    ret = mkstemp(sock_path0);
>> +    g_assert_cmpint(ret, !=, -1);
>> +    ret = mkstemp(sock_path1);
>> +    g_assert_cmpint(ret, !=, -1);
>> +
>> +    cmdline = g_strdup_printf("-netdev socket,id=qtest-bn0,fd=%d "
>> +                "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
>> +                "-chardev socket,id=redirector0,path=%s,server,nowait "
>> +                "-chardev socket,id=redirector1,path=%s,server,nowait "
>> +                "-chardev socket,id=redirector2,path=%s,nowait "
>> +                "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
>> +                "queue=tx,outdev=redirector0 "
>> +                "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
>> +                "queue=tx,indev=redirector2 "
>> +                "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
>> +                "queue=tx,outdev=redirector1 "
>> +                , backend_sock[1], sock_path0, sock_path1, sock_path0);
>> +    qtest_start(cmdline);
>> +    g_free(cmdline);
>> +
>> +    recv_sock = unix_connect(sock_path1, NULL);
>> +    g_assert_cmpint(recv_sock, !=, -1);
>> +
>> +    struct iovec iov[] = {
>> +        {
>> +            .iov_base = &size,
>> +            .iov_len = sizeof(size),
>> +        }, {
>> +            .iov_base = send_buf,
>> +            .iov_len = sizeof(send_buf),
>> +        },
>> +    };
>> +
>> +    ret = iov_send(backend_sock[0], iov, 2, 0, sizeof(size) + sizeof(send_buf));
>> +    g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
>> +    close(backend_sock[0]);
>> +
>> +    ret = qemu_recv(recv_sock, &len, sizeof(len), 0);
>> +    g_assert_cmpint(ret, ==, sizeof(len));
>> +    len = ntohl(len);
>> +
>> +    g_assert_cmpint(len, ==, sizeof(send_buf));
>> +    recv_buf = g_malloc(len);
>> +    ret = qemu_recv(recv_sock, recv_buf, len, 0);
>> +    g_assert_cmpstr(recv_buf, ==, send_buf);
>> +
>> +    g_free(recv_buf);
>> +    close(recv_sock);
>> +    unlink(sock_path0);
>> +    unlink(sock_path1);
>> +    qtest_end();
>> +
>> +#endif
>> +}
>> +
>> +static void test_redirector_rx(void)
>> +{
>> +#ifndef _WIN32
>> +/* socketpair(PF_UNIX) which does not exist on windows */
>> +
>> +    int backend_sock[2],send_sock, recv_sock;
>> +    char *cmdline;
>> +    uint32_t ret = 0, len = 0;
>> +    char send_buf[] = "Hello!!";
>> +    char sock_path0[] = "filter-redirector0.XXXXXX";
>> +    char sock_path1[] = "filter-redirector1.XXXXXX";
>> +    char *recv_buf;
>> +    uint32_t size = sizeof(send_buf);
>> +    size = htonl(size);
>> +
>> +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
>> +    g_assert_cmpint(ret, !=, -1);
>> +
>> +    ret = mkstemp(sock_path0);
>> +    g_assert_cmpint(ret, !=, -1);
>> +    ret = mkstemp(sock_path1);
>> +    g_assert_cmpint(ret, !=, -1);
>> +
>> +    cmdline = g_strdup_printf("-netdev socket,id=qtest-bn0,fd=%d "
>> +                "-device rtl8139,netdev=qtest-bn0,id=qtest-e0 "
>> +                "-chardev socket,id=redirector0,path=%s,server,nowait "
>> +                "-chardev socket,id=redirector1,path=%s,server,nowait "
>> +                "-chardev socket,id=redirector2,path=%s,nowait "
>> +                "-object filter-redirector,id=qtest-f0,netdev=qtest-bn0,"
>> +                "queue=rx,indev=redirector0 "
>> +                "-object filter-redirector,id=qtest-f1,netdev=qtest-bn0,"
>> +                "queue=rx,outdev=redirector2 "
>> +                "-object filter-redirector,id=qtest-f2,netdev=qtest-bn0,"
>> +                "queue=rx,indev=redirector1 "
>> +                , backend_sock[1], sock_path0, sock_path1, sock_path0);
>> +    qtest_start(cmdline);
>> +    g_free(cmdline);
>> +
>> +    struct iovec iov[] = {
>> +        {
>> +            .iov_base = &size,
>> +            .iov_len = sizeof(size),
>> +        }, {
>> +            .iov_base = send_buf,
>> +            .iov_len = sizeof(send_buf),
>> +        },
>> +    };
>> +
>> +    send_sock = unix_connect(sock_path1, NULL);
>> +    ret = iov_send(send_sock, iov, 2, 0, sizeof(size) + sizeof(send_buf));
>> +    g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
>> +    close(send_sock);
>> +
>> +    ret = qemu_recv(backend_sock[0], &len, sizeof(len), 0);
>> +    g_assert_cmpint(ret, ==, sizeof(len));
>> +    len = ntohl(len);
>> +
>> +    g_assert_cmpint(len, ==, sizeof(send_buf));
>> +    recv_buf = g_malloc(len);
>> +    ret = qemu_recv(backend_sock[0], recv_buf, len, 0);
>> +    g_assert_cmpstr(recv_buf, ==, send_buf);
>> +
>> +    g_free(recv_buf);
>> +    close(recv_sock);
>> +    unlink(sock_path0);
>> +    unlink(sock_path1);
>> +    qtest_end();
>> +
>> +#endif
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +    int ret;
>> +
>> +    g_test_init(&argc, &argv, NULL);
>> +    qtest_add_func("/netfilter/redirector_tx", test_redirector_tx);
>> +    qtest_add_func("/netfilter/redirector_rx", test_redirector_rx);
>> +    ret = g_test_run();
>> +
>> +    return ret;
>> +}
>
>
> .
>

-- 
Thanks
zhangchen