diff mbox

[V3,2/3] net/filter-mirror:Add filter-redirector func

Message ID 1457092906-19828-3-git-send-email-zhangchen.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang Chen March 4, 2016, 12:01 p.m. UTC
Filter-redirector is a netfilter plugin.
It gives qemu the ability to redirect net packet.
redirector can redirect filter's net packet to outdev.
and redirect indev's packet to filter.

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

usage:

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

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

Comments

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

stack overflow if size > sizeof(len)?

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

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

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

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

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

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

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

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

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

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

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

Not a native English speaker, but this looks better:

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

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


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

thanks
Li Zhijian

>
> Then you can proceed and do something similar when you're receiving the
> packet itself.
>
>> +        if (ret != (REDIRECT_HEADER_LEN - size)) {
>> +            error_report("filter-redirector recv size failed");
>> +            return;
>> +        }
>> +    }
>> +
>> +    len = ntohl(len);
>> +
>> +    if (len > 0 && len < NET_BUFSIZE) {
>> +        recv_buf = g_malloc(len);
>> +        ret = qemu_chr_fe_read_all(s->chr_in, recv_buf, len);
>> +        if (ret != len) {
>> +            error_report("filter-redirector recv buf failed");
>> +            g_free(recv_buf);
>> +            return;
>> +        }
>> +
>> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +                nf->direction == NET_FILTER_DIRECTION_TX) {
>> +            ret = qemu_net_queue_send(s->incoming_queue, nf->netdev,
>> +                            0, (const uint8_t *)recv_buf, len, NULL);
>
> Rethink about this, we don't queue any packet in incoming_queue, so
> probably there's no need to have a incoming_queue for redirector. We can
> just use qemu_netfilter_pass_to_next() here.
>
>> +
>> +            if (ret < 0) {
>> +                /*
>> +                  *FIXME: just report an error, let NET_FILTER_DIRECTION_RX have chance to pass
>> +                  * to next filter ?
>> +                  */
>> +                error_report("filter-redirector out to guest failed");
>> +            }
>> +        }
>> +
>> +        if (nf->direction == NET_FILTER_DIRECTION_ALL ||
>> +                nf->direction == NET_FILTER_DIRECTION_RX) {
>> +            struct iovec iov = {
>> +                .iov_base = recv_buf,
>> +                .iov_len = sizeof(recv_buf)
>> +            };
>> +            qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf);
>
> Another issue not relate to this patch. Looks like we can rename this to
> qemu_netfilter_iterate(), which seems better. Care to send a patch of this?
>
>> +        }
>> +        g_free(recv_buf);
>> +    } else {
>> +        error_report("filter-redirector recv len failed");
>
> What will happen if the socket was closed at the other end? Can we get
> size == 0 here? Btw, the grammar looks not correct :)
>
>> +    }
>> +}
>> +
>>   static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>                                            NetClientState *sender,
>>                                            unsigned flags,
>> @@ -89,6 +159,27 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>>       return 0;
>>   }
>>
>> +static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
>> +                                         NetClientState *sender,
>> +                                         unsigned flags,
>> +                                         const struct iovec *iov,
>> +                                         int iovcnt,
>> +                                         NetPacketSent *sent_cb)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +    int ret;
>> +
>> +    if (s->chr_out) {
>> +        ret = filter_mirror_send(s->chr_out, iov, iovcnt);
>> +        if (ret) {
>> +            error_report("filter_mirror_send failed(%s)", strerror(-ret));
>> +        }
>> +        return iov_size(iov, iovcnt);
>> +    } else {
>> +        return 0;
>> +    }
>> +}
>> +
>>   static void filter_mirror_cleanup(NetFilterState *nf)
>>   {
>>       MirrorState *s = FILTER_MIRROR(nf);
>> @@ -98,6 +189,18 @@ static void filter_mirror_cleanup(NetFilterState *nf)
>>       }
>>   }
>>
>> +static void filter_redirector_cleanup(NetFilterState *nf)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +
>> +    if (s->chr_in) {
>> +        qemu_chr_fe_release(s->chr_in);
>> +    }
>> +    if (s->chr_out) {
>> +        qemu_chr_fe_release(s->chr_out);
>> +    }
>> +}
>> +
>>   static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>>   {
>>       MirrorState *s = FILTER_MIRROR(nf);
>> @@ -121,6 +224,47 @@ static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>>       }
>>   }
>>
>> +static void filter_redirector_setup(NetFilterState *nf, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>> +
>> +    if (!s->indev && !s->outdev) {
>> +        error_setg(errp, "filter redirector needs 'indev' or "
>> +                "'outdev' at least one property set");
>> +        return;
>> +    } else if (s->indev && s->outdev) {
>> +        if (!strcmp(s->indev, s->outdev)) {
>> +            error_setg(errp, "filter redirector needs 'indev' and "
>> +                    "'outdev'  are not same");
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (s->indev) {
>> +        s->chr_in = qemu_chr_find(s->indev);
>> +        if (s->chr_in == NULL) {
>> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> +                      "IN Device '%s' not found", s->indev);
>> +            return;
>> +        }
>> +
>> +        qemu_chr_fe_claim_no_fail(s->chr_in);
>> +        qemu_chr_add_handlers(s->chr_in, redirector_chr_can_read,
>> +                                redirector_chr_read, NULL, nf);
>> +        s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
>> +    }
>> +
>> +    if (s->outdev) {
>> +        s->chr_out = qemu_chr_find(s->outdev);
>> +        if (s->chr_out == NULL) {
>> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> +                      "OUT Device '%s' not found", s->outdev);
>> +            return;
>> +        }
>> +        qemu_chr_fe_claim_no_fail(s->chr_out);
>> +    }
>> +}
>> +
>>   static void filter_mirror_class_init(ObjectClass *oc, void *data)
>>   {
>>       NetFilterClass *nfc = NETFILTER_CLASS(oc);
>> @@ -130,6 +274,31 @@ static void filter_mirror_class_init(ObjectClass *oc, void *data)
>>       nfc->receive_iov = filter_mirror_receive_iov;
>>   }
>>
>> +static void filter_redirector_class_init(ObjectClass *oc, void *data)
>> +{
>> +    NetFilterClass *nfc = NETFILTER_CLASS(oc);
>> +
>> +    nfc->setup = filter_redirector_setup;
>> +    nfc->cleanup = filter_redirector_cleanup;
>> +    nfc->receive_iov = filter_redirector_receive_iov;
>> +}
>> +
>> +static char *filter_redirector_get_indev(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    return g_strdup(s->indev);
>> +}
>> +
>> +static void
>> +filter_redirector_set_indev(Object *obj, const char *value, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->indev);
>> +    s->indev = g_strdup(value);
>> +}
>> +
>>   static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>>   {
>>       MirrorState *s = FILTER_MIRROR(obj);
>> @@ -151,12 +320,36 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>>       }
>>   }
>>
>> +static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    return g_strdup(s->outdev);
>> +}
>> +
>> +static void
>> +filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->outdev);
>> +    s->outdev = g_strdup(value);
>> +}
>> +
>>   static void filter_mirror_init(Object *obj)
>>   {
>>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>>                               filter_mirror_set_outdev, NULL);
>>   }
>>
>> +static void filter_redirector_init(Object *obj)
>> +{
>> +    object_property_add_str(obj, "indev", filter_redirector_get_indev,
>> +                            filter_redirector_set_indev, NULL);
>> +    object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
>> +                            filter_redirector_set_outdev, NULL);
>> +}
>> +
>>   static void filter_mirror_fini(Object *obj)
>>   {
>>       MirrorState *s = FILTER_MIRROR(obj);
>> @@ -164,6 +357,23 @@ static void filter_mirror_fini(Object *obj)
>>       g_free(s->outdev);
>>   }
>>
>> +static void filter_redirector_fini(Object *obj)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    g_free(s->indev);
>> +    g_free(s->outdev);
>> +}
>> +
>> +static const TypeInfo filter_redirector_info = {
>> +    .name = TYPE_FILTER_REDIRECTOR,
>> +    .parent = TYPE_NETFILTER,
>> +    .class_init = filter_redirector_class_init,
>> +    .instance_init = filter_redirector_init,
>> +    .instance_finalize = filter_redirector_fini,
>> +    .instance_size = sizeof(MirrorState),
>> +};
>> +
>>   static const TypeInfo filter_mirror_info = {
>>       .name = TYPE_FILTER_MIRROR,
>>       .parent = TYPE_NETFILTER,
>> @@ -176,6 +386,7 @@ static const TypeInfo filter_mirror_info = {
>>   static void register_types(void)
>>   {
>>       type_register_static(&filter_mirror_info);
>> +    type_register_static(&filter_redirector_info);
>>   }
>>
>>   type_init(register_types);
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ca27863..84584e3 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3764,6 +3764,14 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>>   filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>>   @var{chardevid}
>>
>> +@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>> +outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>> +
>> +filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
>> +@var{chardevid},and redirect indev's packet to filter.
>> +Create a filter-redirector we need to differ outdev id from indev id, id can not
>> +be the same. we can just use indev or outdev, but at least one to be set.
>
> Not a native English speaker, but this looks better:
>
> At least one of indev or outdev need to be specified.
>
>> +
>>   @item -object filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
>>
>>   Dump the network traffic on netdev @var{dev} to the file specified by
>> diff --git a/vl.c b/vl.c
>> index d68533a..c389a3f 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2799,7 +2799,8 @@ static bool object_create_initial(const char *type)
>>        */
>>       if (g_str_equal(type, "filter-buffer") ||
>>           g_str_equal(type, "filter-dump") ||
>> -        g_str_equal(type, "filter-mirror")) {
>> +        g_str_equal(type, "filter-mirror") ||
>> +        g_str_equal(type, "filter-redirector")) {
>>           return false;
>>       }
>>
>
>
>
> .
>
Jason Wang March 7, 2016, 9:09 a.m. UTC | #3
On 03/07/2016 04:26 PM, Li Zhijian wrote:
>
>
> On 03/07/2016 03:56 PM, Jason Wang wrote:
>>
>>
>> On 03/04/2016 08:01 PM, Zhang Chen wrote:
>>> Filter-redirector is a netfilter plugin.
>>> It gives qemu the ability to redirect net packet.
>>> redirector can redirect filter's net packet to outdev.
>>> and redirect indev's packet to filter.
>>>
>>>                        filter
>>>                          +
>>>                          |
>>>                          |
>>>              redirector  |
>>>                 +--------------+
>>>                 |        |     |
>>>                 |        |     |
>>>                 |        |     |
>>>    indev +-----------+   +---------->  outdev
>>>                 |    |         |
>>>                 |    |         |
>>>                 |    |         |
>>>                 +--------------+
>>>                      |
>>>                      |
>>>                      v
>>>                    filter
>>>
>>> usage:
>>>
>>> -netdev user,id=hn0
>>> -chardev socket,id=s0,host=ip_primary,port=X,server,nowait
>>> -chardev socket,id=s1,host=ip_primary,port=Y,server,nowait
>>> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>> ---
>>>   net/filter-mirror.c | 211
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   qemu-options.hx     |   8 ++
>>>   vl.c                |   3 +-
>>>   3 files changed, 221 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>> index 4ff7619..d137168 100644
>>> --- a/net/filter-mirror.c
>>> +++ b/net/filter-mirror.c
>>> @@ -25,11 +25,19 @@
>>>   #define FILTER_MIRROR(obj) \
>>>       OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
>>>
>>> +#define FILTER_REDIRECTOR(obj) \
>>> +    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR)
>>> +
>>>   #define TYPE_FILTER_MIRROR "filter-mirror"
>>> +#define TYPE_FILTER_REDIRECTOR "filter-redirector"
>>> +#define REDIRECT_HEADER_LEN sizeof(uint32_t)
>>>
>>>   typedef struct MirrorState {
>>>       NetFilterState parent_obj;
>>> +    NetQueue *incoming_queue;
>>> +    char *indev;
>>>       char *outdev;
>>> +    CharDriverState *chr_in;
>>>       CharDriverState *chr_out;
>>>   } MirrorState;
>>>
>>> @@ -67,6 +75,68 @@ err:
>>>       return ret < 0 ? ret : -EIO;
>>>   }
>>>
>>> +static int redirector_chr_can_read(void *opaque)
>>> +{
>>> +    return REDIRECT_HEADER_LEN;
>>> +}
>>> +
>>> +static void redirector_chr_read(void *opaque, const uint8_t *buf,
>>> int size)
>>> +{
>>> +    NetFilterState *nf = opaque;
>>> +    MirrorState *s = FILTER_REDIRECTOR(nf);
>>> +    uint32_t len;
>>> +    int ret = 0;
>>> +    uint8_t *recv_buf;
>>> +
>>> +    memcpy(&len, buf, size);
>>
>> stack overflow if size > sizeof(len)?
> IIUC, it seems never happend because the 'size' will never greater
> than the return value of
> redirector_chr_can_read() which will always return REDIRECT_HEADER_LEN ?

Right, so it's safe.

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

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

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

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

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

OK~~ I will fix it.

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

OK, I will send a patch for this~~

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

I will fix it in next version.

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

I will fix it in next version.
Thanks
zhangchen

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

#define REDIRECTOR_MAX_LEN NET_BUFSIZE

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


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

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

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

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

Thanks
Li Zhijian
diff mbox

Patch

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