On 02/05/2016 02:50 PM, Zhang Chen wrote:
> From: ZhangChen <zhangchen.f...@cn.fujitsu.com>
>
> 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 tap,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: ZhangChen <zhangchen.f...@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
> ---
Thanks a lot for the patch. Like mirror, let's design a unit-test for
this. And what's more, is there any chance to unify the codes? (At least
parts of the codes could be reused).
> net/Makefile.objs | 1 +
> net/filter-redirector.c | 245
> ++++++++++++++++++++++++++++++++++++++++++++++++
> qemu-options.hx | 6 ++
> vl.c | 3 +-
> 4 files changed, 254 insertions(+), 1 deletion(-)
> create mode 100644 net/filter-redirector.c
>
> diff --git a/net/Makefile.objs b/net/Makefile.objs
> index 5fa2f97..f4290a5 100644
> --- a/net/Makefile.objs
> +++ b/net/Makefile.objs
> @@ -15,3 +15,4 @@ common-obj-$(CONFIG_VDE) += vde.o
> common-obj-$(CONFIG_NETMAP) += netmap.o
> common-obj-y += filter.o
> common-obj-y += filter-buffer.o
> +common-obj-y += filter-redirector.o
> diff --git a/net/filter-redirector.c b/net/filter-redirector.c
> new file mode 100644
> index 0000000..364e463
> --- /dev/null
> +++ b/net/filter-redirector.c
> @@ -0,0 +1,245 @@
> +/*
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + * Copyright (c) 2016 FUJITSU LIMITED
> + * Copyright (c) 2016 Intel Corporation
> + *
> + * Author: Zhang Chen <zhangchen.f...@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later. See the COPYING file in the top-level directory.
> + */
> +
> +#include "net/filter.h"
> +#include "net/net.h"
> +#include "qemu-common.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qapi-visit.h"
> +#include "qom/object.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/error-report.h"
> +#include "trace.h"
> +#include "sysemu/char.h"
> +#include "qemu/iov.h"
> +#include "qemu/sockets.h"
> +
> +#define FILTER_REDIRECTOR(obj) \
> + OBJECT_CHECK(RedirectorState, (obj), TYPE_FILTER_REDIRECTOR)
> +
> +#define TYPE_FILTER_REDIRECTOR "filter-redirector"
> +#define REDIRECT_HEADER_LEN sizeof(uint32_t)
> +
> +typedef struct RedirectorState {
> + NetFilterState parent_obj;
> + NetQueue *incoming_queue;/* guest normal net queue */
The comment looks unless and maybe even wrong when queue=rx?
> + char *indev;
> + char *outdev;
> + CharDriverState *chr_in;
> + CharDriverState *chr_out;
> +} RedirectorState;
> +
> +static ssize_t filter_redirector_send(NetFilterState *nf,
> + const struct iovec *iov,
> + int iovcnt)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(nf);
> + ssize_t ret = 0;
> + ssize_t size = 0;
> + uint32_t len = 0;
> + char *buf;
> +
> + size = iov_size(iov, iovcnt);
> + len = htonl(size);
> + if (!size) {
> + return 0;
> + }
> +
> + buf = g_malloc0(size);
> + iov_to_buf(iov, iovcnt, 0, buf, size);
> + ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)&len, sizeof(len));
> + if (ret < 0) {
Similar to mirror, need check the return value against sizeof(len). And
the code could be simplified with something like:
goto out;
...
out:
g_free(buf);
return ret;
And you can see another advantages of unifying the codes here, avoid
duplicating bugx/fixes.
> + g_free(buf);
> + return ret;
> + }
> +
> + ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)buf, size);
> + g_free(buf);
> + return ret;
> +}
> +
> +static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
> + NetClientState *sender,
> + unsigned flags,
> + const struct iovec *iov,
> + int iovcnt,
> + NetPacketSent *sent_cb)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(nf);
> + ssize_t ret = 0;
> +
> + if (s->chr_out) {
> + ret = filter_redirector_send(nf, iov, iovcnt);
> + if (ret < 0) {
> + error_report("filter_redirector_send failed");
> + }
> + }
> + return iov_size(iov, iovcnt);
> +}
> +
> +static void filter_redirector_cleanup(NetFilterState *nf)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(nf);
> +
> + if (s->chr_in) {
> + qemu_chr_fe_release(s->chr_in);
release but no claim?
> + }
> + if (s->chr_out) {
> + qemu_chr_fe_release(s->chr_out);
> + }
> +}
> +
> +static int redirector_chr_can_read(void *opaque)
> +{
> + return REDIRECT_HEADER_LEN;
> +}
> +
> +static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
> +{
> + NetFilterState *nf = opaque;
> + RedirectorState *s = FILTER_REDIRECTOR(nf);
> + uint32_t len;
> + ssize_t ret = 0;
> + uint8_t *recv_buf;
> +
> + if (size != REDIRECT_HEADER_LEN) {
> + // FIXME: do something else
This is not packet corruption since I believe you want tcp socket here
which is doing byte stream. Need some logic to prepare to wait for all
the bytes for length is received.
Though sub-optimal (since it uses usleep() which may block iothread),
maybe you could just do a for simplicity:
qemu_chr_fe_read_all(s->chr_in, recv_buf, sizeof(len));
If you wish, you can have a look at socket backend (net_socket_send) who
has a better solution for this.
> + error_report("packet is corruption, drop it");
> + return;
> + }
> + memcpy(&len, buf, sizeof(len));
> + len = ntohl(len);
> +
> + if (len > 0 && len < NET_BUFSIZE) {
> + recv_buf = g_malloc0(len);
Looks like g_malloc() is sufficient here.
> + ret = qemu_chr_fe_read_all(s->chr_in, recv_buf, len);
> + if (ret != len) {
> + error_report("filter-redirector recv buf failed");
> + g_free(recv_buf);
> + return;
> + }
> +
> + ret = qemu_net_queue_send(s->incoming_queue, nf->netdev,
> + 0, (const uint8_t *)recv_buf, len, NULL);
> + g_free(recv_buf);
> + if (ret < 0) {
> + error_report("filter-redirector out to guest failed");
"guest" is inaccurate here, what if queue=rx?
> + }
> + } else {
> + error_report("filter-redirector recv len failed");
> + }
> +}
> +
> +static void filter_redirector_setup(NetFilterState *nf, Error **errp)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(nf);
> +
> + if (!s->indev && !s->outdev) {
> + error_setg(errp, "filter redirector needs 'indev' or "
> + "'outdev' at least one property set");
> + return;
> + }
> +
> + if (s->indev) {
> + s->chr_in = qemu_chr_find(s->indev);
> + if (s->chr_in == NULL) {
> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> + "IN Device '%s' not found", s->indev);
> + return;
> + }
> +
> + qemu_chr_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;
> + }
> + }
> + s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
> +}
> +
> +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)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(obj);
> +
> + return g_strdup(s->indev);
> +}
> +
> +static void
> +filter_redirector_set_indev(Object *obj, const char *value, Error **errp)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(obj);
> +
> + g_free(s->indev);
> + s->indev = g_strdup(value);
> +}
> +
> +static char *filter_redirector_get_outdev(Object *obj, Error **errp)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(obj);
> +
> + return g_strdup(s->outdev);
> +}
> +
> +static void
> +filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
> +{
> + RedirectorState *s = FILTER_REDIRECTOR(obj);
> +
> + g_free(s->outdev);
> + s->outdev = g_strdup(value);
> +}
> +
> +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_redirector_fini(Object *obj)
> +{
> + RedirectorState *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(RedirectorState),
> +};
> +
> +static void register_types(void)
> +{
> + type_register_static(&filter_redirector_info);
> +}
> +
> +type_init(register_types);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f31a240..1f796ed 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3745,6 +3745,12 @@ queue @var{all|rx|tx} is an option that can be applied
> to any netfilter.
> @option{tx}: the filter is attached to the transmit queue of the netdev,
> where it will receive packets sent by the netdev.
>
> +@item -object
> 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},redirector can redirect filter's
> net packet to outdev.
> +and redirect indev's packet to filter.
Exceeds 80 characters limit. And need more work. e.g:
- Need to differs outdev id from indev id.
- Need to clarify at least one of outdev and indev needs to be set
- We're in fact redirect packet between chardev and netdev's queue (not
filter).
> +queue @var{all|rx|tx} is an option that can be applied to filter-redirector.
This line is unnecessary, we've already had
"
queue @var{all|rx|tx} is an option that can be applied to any netfilter.
"
> +
> @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 f043009..4a9a9a0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2801,7 +2801,8 @@ static bool object_create_initial(const char *type)
> * they depend on netdevs already existing
> */
> if (g_str_equal(type, "filter-buffer") ||
> - g_str_equal(type, "filter-dump")) {
> + g_str_equal(type, "filter-dump") ||
> + g_str_equal(type, "filter-redirector")) {
> return false;
> }
>
