diff mbox

[V4,1/2] net/filter-mirror: implement filter-redirector

Message ID 1458036214-867-2-git-send-email-zhangchen.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

Zhang Chen March 15, 2016, 10:03 a.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>
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(-)

Comments

Jason Wang March 16, 2016, 8:18 a.m. UTC | #1
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;
>      }
>
Li Zhijian March 16, 2016, 9:11 a.m. UTC | #2
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
Wen Congyang March 16, 2016, 9:34 a.m. UTC | #3
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;
>>      }
>>  
> 
> 
> 
> 
> .
>
Li Zhijian March 16, 2016, 10:07 a.m. UTC | #4
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;
>>>       }
>>>
>>
>>
>>
>>
>> .
>>
>
> .
>
Jason Wang March 17, 2016, 6:10 a.m. UTC | #5
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.
Wen Congyang March 17, 2016, 6:18 a.m. UTC | #6
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

> 
> 
> 
> .
>
Jason Wang March 17, 2016, 6:28 a.m. UTC | #7
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
>
diff mbox

Patch

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;
     }