All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector
@ 2016-03-15 10:03 Zhang Chen
  2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector Zhang Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Zhang Chen @ 2016-03-15 10:03 UTC (permalink / raw)
  To: qemu devel, Jason Wang
  Cc: Li Zhijian, Gui jianfeng, eddie.dong, zhanghailiang,
	Dr. David Alan Gilbert, Zhang Chen, Yang Hongyang

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


v4:
 Address Jason's comments.
 - remove redirector's incoming queue
 - just pass packet come from in_dev to filter's next
 - rework redirector_chr_read, most code is stolen from net_socket_send  
 - fix comments error
 - add some comments 

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 (2):
  net/filter-mirror: implement filter-redirector
  tests/test-filter-redirector: Add unit test for filter-redirector

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

-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-15 10:03 [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector Zhang Chen
@ 2016-03-15 10:03 ` Zhang Chen
  2016-03-16  8:18   ` Jason Wang
  2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 2/2] tests/test-filter-redirector: Add unit test for filter-redirector Zhang Chen
  2016-03-16  8:18 ` [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector Jason Wang
  2 siblings, 1 reply; 11+ messages in thread
From: Zhang Chen @ 2016-03-15 10:03 UTC (permalink / raw)
  To: qemu devel, Jason Wang
  Cc: Li Zhijian, Gui jianfeng, eddie.dong, zhanghailiang,
	Dr. David Alan Gilbert, Zhang Chen, Yang Hongyang

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>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx     |   9 ++
 vl.c                |   3 +-
 3 files changed, 247 insertions(+), 1 deletion(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 1b1ec16..77ece41 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -26,12 +26,23 @@
 #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 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 filter_mirror_send(CharDriverState *chr_out,
@@ -68,6 +79,89 @@ err:
     return ret < 0 ? ret : -EIO;
 }
 
+static void
+redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)
+{
+    struct iovec iov = {
+        .iov_base = (void *)buf,
+        .iov_len = len,
+    };
+
+    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
+        nf->direction == NET_FILTER_DIRECTION_TX) {
+        qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf);
+    }
+
+    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
+        nf->direction == NET_FILTER_DIRECTION_RX) {
+        qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
+     }
+}
+
+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;
+
+    if (size == 0) {
+        /* the peer is closed ? */
+        return ;
+    }
+
+    /* most of code is stolen from net_socket_send */
+    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;
+                /* FIXME: do something ? */
+                return;
+            }
+
+            s->index += l;
+            buf += l;
+            size -= l;
+            if (s->index >= s->packet_len) {
+                s->index = 0;
+                s->state = 0;
+                redirector_to_filter(nf, s->buf, s->packet_len);
+            }
+            break;
+        }
+    }
+}
+
 static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
                                          NetClientState *sender,
                                          unsigned flags,
@@ -90,6 +184,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);
@@ -99,6 +214,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);
@@ -122,6 +249,48 @@ 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;
+        }
+    }
+
+    s->state = s->index = 0;
+
+    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);
+    }
+
+    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);
@@ -131,6 +300,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);
@@ -152,12 +346,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);
@@ -165,6 +383,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,
@@ -177,6 +412,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 5379cb5..796d053 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3821,6 +3821,15 @@ 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 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 79bb11a..dc6e63a 100644
--- a/vl.c
+++ b/vl.c
@@ -2841,7 +2841,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH V4 2/2] tests/test-filter-redirector: Add unit test for filter-redirector
  2016-03-15 10:03 [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector Zhang Chen
  2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector Zhang Chen
@ 2016-03-15 10:03 ` Zhang Chen
  2016-03-16  8:18 ` [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector Jason Wang
  2 siblings, 0 replies; 11+ messages in thread
From: Zhang Chen @ 2016-03-15 10:03 UTC (permalink / raw)
  To: qemu devel, Jason Wang
  Cc: Li Zhijian, Gui jianfeng, eddie.dong, zhanghailiang,
	Dr. David Alan Gilbert, Zhang Chen, Yang Hongyang

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  |
+---------+            |  +-------+
                       +

a. we(sock0) inject packet to qemu socket backend
b. backend pass packet to filter redirector0(rd0)
c. rd0 redirect packet to out_dev(chardev) which is connected with
filter redirector1's(rd1) in_dev
d. rd1 read this packet from in_dev, and pass to next filter redirector2(rd2)
e. rd2 redirect packet to rd2's out_dev which is connected with an opened socketed(sock1)
f. we read packet from sock1 and compare to what we inject

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  |
+---------+            |  +-------+

a. we(sock0) insert packet to filter redirector2(rd2)
b. rd2 pass packet to filter redirector1(rd1)
c. rd1 redirect packet to out_dev(chardev) which is connected with
   filter redirector0's(rd0) in_dev
d. rd0 read this packet from in_dev, and pass ti to qemu backend which is
   connected with an opened socketed(sock1)
e. we read packet from sock1 and compare to what we inject

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>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 tests/.gitignore               |   1 +
 tests/Makefile                 |   2 +
 tests/test-filter-redirector.c | 221 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 224 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 5a8f590..ff212b6 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -214,6 +214,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))
@@ -568,6 +569,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..b93012c
--- /dev/null
+++ b/tests/test-filter-redirector.c
@@ -0,0 +1,221 @@
+/*
+ * 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 "qemu/osdep.h"
+#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);
+
+    /* send a qmp command to guarantee that 'connected' is setting to true. */
+    qmp("{ 'execute' : 'query-status'}");
+
+    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;
+    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);
+    g_assert_cmpint(send_sock, !=, -1);
+    /* send a qmp command to guarantee that 'connected' is setting to true. */
+    qmp("{ 'execute' : 'query-status'}");
+
+    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);
+    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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector
  2016-03-15 10:03 [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector Zhang Chen
  2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector Zhang Chen
  2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 2/2] tests/test-filter-redirector: Add unit test for filter-redirector Zhang Chen
@ 2016-03-16  8:18 ` Jason Wang
  2 siblings, 0 replies; 11+ messages in thread
From: Jason Wang @ 2016-03-16  8:18 UTC (permalink / raw)
  To: Zhang Chen, qemu devel
  Cc: Li Zhijian, Gui jianfeng, eddie.dong, zhanghailiang,
	Dr. David Alan Gilbert, Yang Hongyang



On 03/15/2016 06:03 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
>
>
> v4:
>  Address Jason's comments.
>  - remove redirector's incoming queue
>  - just pass packet come from in_dev to filter's next
>  - rework redirector_chr_read, most code is stolen from net_socket_send  
>  - fix comments error
>  - add some comments 
>
> 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 (2):
>   net/filter-mirror: implement filter-redirector
>   tests/test-filter-redirector: Add unit test for filter-redirector
>
>  net/filter-mirror.c            | 236 +++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx                |   9 ++
>  tests/.gitignore               |   1 +
>  tests/Makefile                 |   2 +
>  tests/test-filter-redirector.c | 221 ++++++++++++++++++++++++++++++++++++++
>  vl.c                           |   3 +-
>  6 files changed, 471 insertions(+), 1 deletion(-)
>  create mode 100644 tests/test-filter-redirector.c
>

Looks good, just few comments, please see individual patches.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector Zhang Chen
@ 2016-03-16  8:18   ` Jason Wang
  2016-03-16  9:11     ` Li Zhijian
  2016-03-16  9:34     ` Wen Congyang
  0 siblings, 2 replies; 11+ messages in thread
From: Jason Wang @ 2016-03-16  8:18 UTC (permalink / raw)
  To: Zhang Chen, qemu devel
  Cc: zhanghailiang, Li Zhijian, Gui jianfeng, eddie.dong,
	Dr. David Alan Gilbert, Yang Hongyang



On 03/15/2016 06:03 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>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>  net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx     |   9 ++
>  vl.c                |   3 +-
>  3 files changed, 247 insertions(+), 1 deletion(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 1b1ec16..77ece41 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -26,12 +26,23 @@
>  #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 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 filter_mirror_send(CharDriverState *chr_out,
> @@ -68,6 +79,89 @@ err:
>      return ret < 0 ? ret : -EIO;
>  }
>  
> +static void
> +redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)
> +{
> +    struct iovec iov = {
> +        .iov_base = (void *)buf,
> +        .iov_len = len,
> +    };
> +
> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
> +        nf->direction == NET_FILTER_DIRECTION_TX) {
> +        qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf);
> +    }
> +
> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
> +        nf->direction == NET_FILTER_DIRECTION_RX) {
> +        qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
> +     }
> +}
> +
> +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;
> +
> +    if (size == 0) {
> +        /* the peer is closed ? */
> +        return ;
> +    }

Looks like if you want to handle connection close, you need use event
handler when calling qemu_chr_add_handlers().

> +
> +    /* most of code is stolen from net_socket_send */

This comment seems redundant.

> +    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");

error_report() looks better.

> +                s->state = 0;
> +                /* FIXME: do something ? */

This needs some thought, but at least reset the fd handler and state is
needed.

> +                return;
> +            }
> +
> +            s->index += l;
> +            buf += l;
> +            size -= l;
> +            if (s->index >= s->packet_len) {
> +                s->index = 0;
> +                s->state = 0;
> +                redirector_to_filter(nf, s->buf, s->packet_len);
> +            }
> +            break;
> +        }
> +    }
> +}
> +
>  static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>                                           NetClientState *sender,
>                                           unsigned flags,
> @@ -90,6 +184,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);
> @@ -99,6 +214,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);
> @@ -122,6 +249,48 @@ 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;
> +        }
> +    }
> +
> +    s->state = s->index = 0;
> +
> +    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);
> +    }
> +
> +    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);
> @@ -131,6 +300,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);
> @@ -152,12 +346,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);
> @@ -165,6 +383,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);

We probably want to reset chr handlers there, otherwise there may be
use-after-free.

> +}
> +
> +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,
> @@ -177,6 +412,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 5379cb5..796d053 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3821,6 +3821,15 @@ 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 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 79bb11a..dc6e63a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2841,7 +2841,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;
>      }
>  

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-16  8:18   ` Jason Wang
@ 2016-03-16  9:11     ` Li Zhijian
  2016-03-16  9:34     ` Wen Congyang
  1 sibling, 0 replies; 11+ messages in thread
From: Li Zhijian @ 2016-03-16  9:11 UTC (permalink / raw)
  To: Jason Wang, Zhang Chen, qemu devel
  Cc: Gui jianfeng, Yang Hongyang, eddie.dong, zhanghailiang,
	Dr. David Alan Gilbert



On 03/16/2016 04:18 PM, Jason Wang wrote:
>
>
> On 03/15/2016 06:03 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>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-options.hx     |   9 ++
>>   vl.c                |   3 +-
>>   3 files changed, 247 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 1b1ec16..77ece41 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -26,12 +26,23 @@
>>   #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 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 filter_mirror_send(CharDriverState *chr_out,
>> @@ -68,6 +79,89 @@ err:
>>       return ret < 0 ? ret : -EIO;
>>   }
>>
>> +static void
>> +redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)
>> +{
>> +    struct iovec iov = {
>> +        .iov_base = (void *)buf,
>> +        .iov_len = len,
>> +    };
>> +
>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +        nf->direction == NET_FILTER_DIRECTION_TX) {
>> +        qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf);
>> +    }
>> +
>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +        nf->direction == NET_FILTER_DIRECTION_RX) {
>> +        qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
>> +     }
>> +}
>> +
>> +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;
>> +
>> +    if (size == 0) {
>> +        /* the peer is closed ? */
>> +        return ;
>> +    }
>
> Looks like if you want to handle connection close, you need use event
> handler when calling qemu_chr_add_handlers().

That sounds good. we will remove this check and register a event handler to
deal with a close event.

>
>> +
>> +    /* most of code is stolen from net_socket_send */
>
> This comment seems redundant.
>
>> +    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");
>
> error_report() looks better.

OK

>
>> +                s->state = 0;
>> +                /* FIXME: do something ? */
>
> This needs some thought, but at least reset the fd handler and state is
> needed.

OK

>
>> +                return;
>> +            }
>> +
>> +            s->index += l;
>> +            buf += l;
>> +            size -= l;
>> +            if (s->index >= s->packet_len) {
>> +                s->index = 0;
>> +                s->state = 0;
>> +                redirector_to_filter(nf, s->buf, s->packet_len);
>> +            }
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>>   static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>                                            NetClientState *sender,
>>                                            unsigned flags,
>> @@ -90,6 +184,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);
>> @@ -99,6 +214,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);
>> @@ -122,6 +249,48 @@ 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;
>> +        }
>> +    }
>> +
>> +    s->state = s->index = 0;
>> +
>> +    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);
>> +    }
>> +
>> +    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);
>> @@ -131,6 +300,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);
>> @@ -152,12 +346,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);
>> @@ -165,6 +383,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);
>
> We probably want to reset chr handlers there, otherwise there may be
> use-after-free.
>
OK

-- 
Best regards.
Li Zhijian
>> +}
>> +
>> +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,
>> @@ -177,6 +412,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 5379cb5..796d053 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3821,6 +3821,15 @@ 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 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 79bb11a..dc6e63a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2841,7 +2841,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;
>>       }
>>
>
>
>
> .
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-16  8:18   ` Jason Wang
  2016-03-16  9:11     ` Li Zhijian
@ 2016-03-16  9:34     ` Wen Congyang
  2016-03-16 10:07       ` Li Zhijian
  2016-03-17  6:10       ` Jason Wang
  1 sibling, 2 replies; 11+ messages in thread
From: Wen Congyang @ 2016-03-16  9:34 UTC (permalink / raw)
  To: Jason Wang, Zhang Chen, qemu devel
  Cc: zhanghailiang, Li Zhijian, Gui jianfeng, eddie.dong,
	Dr. David Alan Gilbert, Yang Hongyang

On 03/16/2016 04:18 PM, Jason Wang wrote:
> 
> 
> On 03/15/2016 06:03 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>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> ---
>>  net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  qemu-options.hx     |   9 ++
>>  vl.c                |   3 +-
>>  3 files changed, 247 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 1b1ec16..77ece41 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -26,12 +26,23 @@
>>  #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 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 filter_mirror_send(CharDriverState *chr_out,
>> @@ -68,6 +79,89 @@ err:
>>      return ret < 0 ? ret : -EIO;
>>  }
>>  
>> +static void
>> +redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)
>> +{
>> +    struct iovec iov = {
>> +        .iov_base = (void *)buf,
>> +        .iov_len = len,
>> +    };
>> +
>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +        nf->direction == NET_FILTER_DIRECTION_TX) {
>> +        qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf);
>> +    }
>> +
>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +        nf->direction == NET_FILTER_DIRECTION_RX) {
>> +        qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
>> +     }
>> +}
>> +
>> +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;
>> +
>> +    if (size == 0) {
>> +        /* the peer is closed ? */
>> +        return ;
>> +    }
> 
> Looks like if you want to handle connection close, you need use event
> handler when calling qemu_chr_add_handlers().

In which case, we will see size is 0 if we don't have a event handler?

For redirector filter, I think we don't care about if the char device
is disconnected. If the char device is ready again, we will continue
to read from the char device.

So I think we just add more comments here.

> 
>> +
>> +    /* most of code is stolen from net_socket_send */
> 
> This comment seems redundant.
> 
>> +    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");
> 
> error_report() looks better.
> 
>> +                s->state = 0;
>> +                /* FIXME: do something ? */
> 
> This needs some thought, but at least reset the fd handler and state is
> needed.

Maybe we can read the data from the chardev, and drop it?

Thanks
Wen Congyang

> 
>> +                return;
>> +            }
>> +
>> +            s->index += l;
>> +            buf += l;
>> +            size -= l;
>> +            if (s->index >= s->packet_len) {
>> +                s->index = 0;
>> +                s->state = 0;
>> +                redirector_to_filter(nf, s->buf, s->packet_len);
>> +            }
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>>  static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>                                           NetClientState *sender,
>>                                           unsigned flags,
>> @@ -90,6 +184,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);
>> @@ -99,6 +214,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);
>> @@ -122,6 +249,48 @@ 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;
>> +        }
>> +    }
>> +
>> +    s->state = s->index = 0;
>> +
>> +    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);
>> +    }
>> +
>> +    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);
>> @@ -131,6 +300,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);
>> @@ -152,12 +346,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);
>> @@ -165,6 +383,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);
> 
> We probably want to reset chr handlers there, otherwise there may be
> use-after-free.
> 
>> +}
>> +
>> +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,
>> @@ -177,6 +412,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 5379cb5..796d053 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3821,6 +3821,15 @@ 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 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 79bb11a..dc6e63a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2841,7 +2841,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;
>>      }
>>  
> 
> 
> 
> 
> .
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-16  9:34     ` Wen Congyang
@ 2016-03-16 10:07       ` Li Zhijian
  2016-03-17  6:10       ` Jason Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Li Zhijian @ 2016-03-16 10:07 UTC (permalink / raw)
  To: Wen Congyang, Jason Wang, Zhang Chen, qemu devel
  Cc: Gui jianfeng, Yang Hongyang, eddie.dong, zhanghailiang,
	Dr. David Alan Gilbert



On 03/16/2016 05:34 PM, Wen Congyang wrote:
> On 03/16/2016 04:18 PM, Jason Wang wrote:
>>
>>
>> On 03/15/2016 06:03 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>
>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>> ---
>>>   net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   qemu-options.hx     |   9 ++
>>>   vl.c                |   3 +-
>>>   3 files changed, 247 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>> index 1b1ec16..77ece41 100644
>>> --- a/net/filter-mirror.c
>>> +++ b/net/filter-mirror.c
>>> @@ -26,12 +26,23 @@
>>>   #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 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 filter_mirror_send(CharDriverState *chr_out,
>>> @@ -68,6 +79,89 @@ err:
>>>       return ret < 0 ? ret : -EIO;
>>>   }
>>>
>>> +static void
>>> +redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)
>>> +{
>>> +    struct iovec iov = {
>>> +        .iov_base = (void *)buf,
>>> +        .iov_len = len,
>>> +    };
>>> +
>>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>>> +        nf->direction == NET_FILTER_DIRECTION_TX) {
>>> +        qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf);
>>> +    }
>>> +
>>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>>> +        nf->direction == NET_FILTER_DIRECTION_RX) {
>>> +        qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
>>> +     }
>>> +}
>>> +
>>> +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;
>>> +
>>> +    if (size == 0) {
>>> +        /* the peer is closed ? */
>>> +        return ;
>>> +    }
>>
>> Looks like if you want to handle connection close, you need use event
>> handler when calling qemu_chr_add_handlers().
>
> In which case, we will see size is 0 if we don't have a event handler?

It seems that the caller will never passes a '0' size to it.
So the size will never be 0.

But I perfer to have a event handler.
e.g.
- peer is closed after sending the length part only
- read handler always expect the data part
- (another) peer is connected again(assume peer can connect successfully)
- peer will send a length part first, which will confuse the read handler


>
> For redirector filter, I think we don't care about if the char device
> is disconnected. If the char device is ready again, we will continue
> to read from the char device.
>
> So I think we just add more comments here.
>
>>
>>> +
>>> +    /* most of code is stolen from net_socket_send */
>>
>> This comment seems redundant.
>>
>>> +    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");
>>
>> error_report() looks better.
>>
>>> +                s->state = 0;
>>> +                /* FIXME: do something ? */
>>
>> This needs some thought, but at least reset the fd handler and state is
>> needed.
>
> Maybe we can read the data from the chardev, and drop it?
>
> Thanks
> Wen Congyang
I think this is an unrecoverable error, we reset the fd handler would be better.

Thanks
Li

>
>>
>>> +                return;
>>> +            }
>>> +
>>> +            s->index += l;
>>> +            buf += l;
>>> +            size -= l;
>>> +            if (s->index >= s->packet_len) {
>>> +                s->index = 0;
>>> +                s->state = 0;
>>> +                redirector_to_filter(nf, s->buf, s->packet_len);
>>> +            }
>>> +            break;
>>> +        }
>>> +    }
>>> +}
>>> +
>>>   static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>>                                            NetClientState *sender,
>>>                                            unsigned flags,
>>> @@ -90,6 +184,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);
>>> @@ -99,6 +214,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);
>>> @@ -122,6 +249,48 @@ 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;
>>> +        }
>>> +    }
>>> +
>>> +    s->state = s->index = 0;
>>> +
>>> +    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);
>>> +    }
>>> +
>>> +    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);
>>> @@ -131,6 +300,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);
>>> @@ -152,12 +346,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);
>>> @@ -165,6 +383,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);
>>
>> We probably want to reset chr handlers there, otherwise there may be
>> use-after-free.
>>
>>> +}
>>> +
>>> +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,
>>> @@ -177,6 +412,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 5379cb5..796d053 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -3821,6 +3821,15 @@ 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 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 79bb11a..dc6e63a 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -2841,7 +2841,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)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-16  9:34     ` Wen Congyang
  2016-03-16 10:07       ` Li Zhijian
@ 2016-03-17  6:10       ` Jason Wang
  2016-03-17  6:18         ` Wen Congyang
  1 sibling, 1 reply; 11+ messages in thread
From: Jason Wang @ 2016-03-17  6:10 UTC (permalink / raw)
  To: Wen Congyang, Zhang Chen, qemu devel
  Cc: zhanghailiang, Li Zhijian, Gui jianfeng, eddie.dong,
	Dr. David Alan Gilbert, Yang Hongyang



On 03/16/2016 05:34 PM, Wen Congyang wrote:
> On 03/16/2016 04:18 PM, Jason Wang wrote:
>> > 
>> > 
>> > On 03/15/2016 06:03 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>
>>> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>> >> ---
>>> >>  net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> >>  qemu-options.hx     |   9 ++
>>> >>  vl.c                |   3 +-
>>> >>  3 files changed, 247 insertions(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>> >> index 1b1ec16..77ece41 100644
>>> >> --- a/net/filter-mirror.c
>>> >> +++ b/net/filter-mirror.c
>>> >> @@ -26,12 +26,23 @@
>>> >>  #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 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 filter_mirror_send(CharDriverState *chr_out,
>>> >> @@ -68,6 +79,89 @@ err:
>>> >>      return ret < 0 ? ret : -EIO;
>>> >>  }
>>> >>  
>>> >> +static void
>>> >> +redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)
>>> >> +{
>>> >> +    struct iovec iov = {
>>> >> +        .iov_base = (void *)buf,
>>> >> +        .iov_len = len,
>>> >> +    };
>>> >> +
>>> >> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>>> >> +        nf->direction == NET_FILTER_DIRECTION_TX) {
>>> >> +        qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf);
>>> >> +    }
>>> >> +
>>> >> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>>> >> +        nf->direction == NET_FILTER_DIRECTION_RX) {
>>> >> +        qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
>>> >> +     }
>>> >> +}
>>> >> +
>>> >> +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;
>>> >> +
>>> >> +    if (size == 0) {
>>> >> +        /* the peer is closed ? */
>>> >> +        return ;
>>> >> +    }
>> > 
>> > Looks like if you want to handle connection close, you need use event
>> > handler when calling qemu_chr_add_handlers().
> In which case, we will see size is 0 if we don't have a event handler?

Looking at tcp_chr_read(), it seems that we won't see zero size.

>
> For redirector filter, I think we don't care about if the char device
> is disconnected. If the char device is ready again, we will continue
> to read from the char device.
>
> So I think we just add more comments here.
>
>> > 
>>> >> +
>>> >> +    /* most of code is stolen from net_socket_send */
>> > 
>> > This comment seems redundant.
>> > 
>>> >> +    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");
>> > 
>> > error_report() looks better.
>> > 
>>> >> +                s->state = 0;
>>> >> +                /* FIXME: do something ? */
>> > 
>> > This needs some thought, but at least reset the fd handler and state is
>> > needed.
> Maybe we can read the data from the chardev, and drop it?
>
> Thanks
> Wen Congyang
>

No function difference, but reading and dropping will cost extra cpu
which seems not good.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-17  6:10       ` Jason Wang
@ 2016-03-17  6:18         ` Wen Congyang
  2016-03-17  6:28           ` Jason Wang
  0 siblings, 1 reply; 11+ messages in thread
From: Wen Congyang @ 2016-03-17  6:18 UTC (permalink / raw)
  To: Jason Wang, Zhang Chen, qemu devel
  Cc: zhanghailiang, Li Zhijian, Gui jianfeng, eddie.dong,
	Dr. David Alan Gilbert, Yang Hongyang

On 03/17/2016 02:10 PM, Jason Wang wrote:
> 
> 
> On 03/16/2016 05:34 PM, Wen Congyang wrote:
>> On 03/16/2016 04:18 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 03/15/2016 06:03 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>
>>>>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>>>>> ---
>>>>>>  net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  qemu-options.hx     |   9 ++
>>>>>>  vl.c                |   3 +-
>>>>>>  3 files changed, 247 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>>>> index 1b1ec16..77ece41 100644
>>>>>> --- a/net/filter-mirror.c
>>>>>> +++ b/net/filter-mirror.c
>>>>>> @@ -26,12 +26,23 @@
>>>>>>  #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 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 filter_mirror_send(CharDriverState *chr_out,
>>>>>> @@ -68,6 +79,89 @@ err:
>>>>>>      return ret < 0 ? ret : -EIO;
>>>>>>  }
>>>>>>  
>>>>>> +static void
>>>>>> +redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)
>>>>>> +{
>>>>>> +    struct iovec iov = {
>>>>>> +        .iov_base = (void *)buf,
>>>>>> +        .iov_len = len,
>>>>>> +    };
>>>>>> +
>>>>>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>>>>>> +        nf->direction == NET_FILTER_DIRECTION_TX) {
>>>>>> +        qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf);
>>>>>> +    }
>>>>>> +
>>>>>> +    if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>>>>>> +        nf->direction == NET_FILTER_DIRECTION_RX) {
>>>>>> +        qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
>>>>>> +     }
>>>>>> +}
>>>>>> +
>>>>>> +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;
>>>>>> +
>>>>>> +    if (size == 0) {
>>>>>> +        /* the peer is closed ? */
>>>>>> +        return ;
>>>>>> +    }
>>>>
>>>> Looks like if you want to handle connection close, you need use event
>>>> handler when calling qemu_chr_add_handlers().
>> In which case, we will see size is 0 if we don't have a event handler?
> 
> Looking at tcp_chr_read(), it seems that we won't see zero size.
> 
>>
>> For redirector filter, I think we don't care about if the char device
>> is disconnected. If the char device is ready again, we will continue
>> to read from the char device.
>>
>> So I think we just add more comments here.
>>
>>>>
>>>>>> +
>>>>>> +    /* most of code is stolen from net_socket_send */
>>>>
>>>> This comment seems redundant.
>>>>
>>>>>> +    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");
>>>>
>>>> error_report() looks better.
>>>>
>>>>>> +                s->state = 0;
>>>>>> +                /* FIXME: do something ? */
>>>>
>>>> This needs some thought, but at least reset the fd handler and state is
>>>> needed.
>> Maybe we can read the data from the chardev, and drop it?
>>
>> Thanks
>> Wen Congyang
>>
> 
> No function difference, but reading and dropping will cost extra cpu
> which seems not good.

If we reset and receive it again, the unread data will be auto dropped?
If so, I think reset the fd handler is better choice.

Thanks
Wen Congyang

> 
> 
> 
> .
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector
  2016-03-17  6:18         ` Wen Congyang
@ 2016-03-17  6:28           ` Jason Wang
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Wang @ 2016-03-17  6:28 UTC (permalink / raw)
  To: Wen Congyang, Zhang Chen, qemu devel
  Cc: zhanghailiang, Li Zhijian, Gui jianfeng, eddie.dong,
	Dr. David Alan Gilbert, Yang Hongyang



On 03/17/2016 02:18 PM, Wen Congyang wrote:
>>>>>>> >>>>>> +                s->state = 0;
>>>>>>> >>>>>> +                /* FIXME: do something ? */
>>>>> >>>>
>>>>> >>>> This needs some thought, but at least reset the fd handler and state is
>>>>> >>>> needed.
>>> >> Maybe we can read the data from the chardev, and drop it?
>>> >>
>>> >> Thanks
>>> >> Wen Congyang
>>> >>
>> > 
>> > No function difference, but reading and dropping will cost extra cpu
>> > which seems not good.
> If we reset and receive it again, the unread data will be auto dropped?

If socket receive queue is not full, the data will be buffered there
(but the queue will be purged after qemu process exits). Otherwise it
will be dropped.

> If so, I think reset the fd handler is better choice.
>
> Thanks
> Wen Congyang
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-03-17  6:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-15 10:03 [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector Zhang Chen
2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector Zhang Chen
2016-03-16  8:18   ` Jason Wang
2016-03-16  9:11     ` Li Zhijian
2016-03-16  9:34     ` Wen Congyang
2016-03-16 10:07       ` Li Zhijian
2016-03-17  6:10       ` Jason Wang
2016-03-17  6:18         ` Wen Congyang
2016-03-17  6:28           ` Jason Wang
2016-03-15 10:03 ` [Qemu-devel] [PATCH V4 2/2] tests/test-filter-redirector: Add unit test for filter-redirector Zhang Chen
2016-03-16  8:18 ` [Qemu-devel] [PATCH V4 0/2] Introduce filter-redirector Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.