Re: [Qemu-devel] [PATCH V2 1/3] net/filter-traffic: add filter-traffic.h

[Jason Wang]

On 02/29/2016 08:23 PM, Zhang Chen wrote:
> We can reuse filter-traffic by filter-mirror,
> filter-redirector and so on.

I think we could share more than this. E.g just use filter-mirror.c to
implement both mirror and redirector.

>
> Signed-off-by: Zhang Chen <address@hidden>
> Signed-off-by: Wen Congyang <address@hidden>
> ---
>  net/filter-mirror.c  | 70 +++++++--------------------------------------------
>  net/filter-traffic.h | 71 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 80 insertions(+), 61 deletions(-)
>  create mode 100644 net/filter-traffic.h
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index ee13d94..d9698f2 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -9,65 +9,13 @@
>   * later.  See the COPYING file in the top-level directory.
>   */
>  
> -#include "net/filter.h"
> -#include "net/net.h"
> -#include "qemu-common.h"
> -#include "qapi/qmp/qerror.h"
> -#include "qapi-visit.h"
> -#include "qom/object.h"
> -#include "qemu/main-loop.h"
> -#include "qemu/error-report.h"
> -#include "trace.h"
> -#include "sysemu/char.h"
> -#include "qemu/iov.h"
> -#include "qemu/sockets.h"
> +#include "net/filter-traffic.h"
>  
>  #define FILTER_MIRROR(obj) \
> -    OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR)
> +    OBJECT_CHECK(TrafficState, (obj), TYPE_FILTER_MIRROR)
>  
>  #define TYPE_FILTER_MIRROR "filter-mirror"
>  
> -typedef struct MirrorState {
> -    NetFilterState parent_obj;
> -    char *outdev;
> -    CharDriverState *chr_out;
> -} MirrorState;
> -
> -static int filter_mirror_send(NetFilterState *nf,
> -                                   const struct iovec *iov,
> -                                   int iovcnt)
> -{
> -    MirrorState *s = FILTER_MIRROR(nf);
> -    int ret = 0;
> -    ssize_t size = 0;
> -    uint32_t len =  0;
> -    char *buf;
> -
> -    size = iov_size(iov, iovcnt);
> -    if (!size) {
> -        return 0;
> -    }
> -
> -    len = htonl(size);
> -    ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)&len, sizeof(len));
> -    if (ret != sizeof(len)) {
> -        goto err;
> -    }
> -
> -    buf = g_malloc(size);
> -    iov_to_buf(iov, iovcnt, 0, buf, size);
> -    ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)buf, size);
> -    g_free(buf);
> -    if (ret != size) {
> -        goto err;
> -    }
> -
> -    return 0;
> -
> -err:
> -    return ret < 0 ? ret : -EIO;
> -}
> -
>  static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>                                           NetClientState *sender,
>                                           unsigned flags,
> @@ -77,7 +25,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>  {
>      int ret;
>  
> -    ret = filter_mirror_send(nf, iov, iovcnt);
> +    ret = filter_traffic_send(nf, iov, iovcnt);
>      if (ret) {
>          error_report("filter_mirror_send failed(%s)", strerror(-ret));
>      }
> @@ -91,7 +39,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>  
>  static void filter_mirror_cleanup(NetFilterState *nf)
>  {
> -    MirrorState *s = FILTER_MIRROR(nf);
> +    TrafficState *s = FILTER_MIRROR(nf);
>  
>      if (s->chr_out) {
>          qemu_chr_fe_release(s->chr_out);
> @@ -100,7 +48,7 @@ static void filter_mirror_cleanup(NetFilterState *nf)
>  
>  static void filter_mirror_setup(NetFilterState *nf, Error **errp)
>  {
> -    MirrorState *s = FILTER_MIRROR(nf);
> +    TrafficState *s = FILTER_MIRROR(nf);
>  
>      if (!s->outdev) {
>          error_setg(errp, "filter filter mirror needs 'outdev' "
> @@ -132,7 +80,7 @@ static void filter_mirror_class_init(ObjectClass *oc, void 
> *data)
>  
>  static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>  {
> -    MirrorState *s = FILTER_MIRROR(obj);
> +    TrafficState *s = FILTER_MIRROR(obj);
>  
>      return g_strdup(s->outdev);
>  }
> @@ -140,7 +88,7 @@ static char *filter_mirror_get_outdev(Object *obj, Error 
> **errp)
>  static void
>  filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>  {
> -    MirrorState *s = FILTER_MIRROR(obj);
> +    TrafficState *s = FILTER_MIRROR(obj);
>  
>      g_free(s->outdev);
>      s->outdev = g_strdup(value);
> @@ -159,7 +107,7 @@ static void filter_mirror_init(Object *obj)
>  
>  static void filter_mirror_fini(Object *obj)
>  {
> -    MirrorState *s = FILTER_MIRROR(obj);
> +    TrafficState *s = FILTER_MIRROR(obj);
>  
>      g_free(s->outdev);
>  }
> @@ -170,7 +118,7 @@ static const TypeInfo filter_mirror_info = {
>      .class_init = filter_mirror_class_init,
>      .instance_init = filter_mirror_init,
>      .instance_finalize = filter_mirror_fini,
> -    .instance_size = sizeof(MirrorState),
> +    .instance_size = sizeof(TrafficState),
>  };
>  
>  static void register_types(void)
> diff --git a/net/filter-traffic.h b/net/filter-traffic.h
> new file mode 100644
> index 0000000..24cb5c3
> --- /dev/null
> +++ b/net/filter-traffic.h
> @@ -0,0 +1,71 @@
> +/*
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + * Copyright (c) 2016 FUJITSU LIMITED
> + * Copyright (c) 2016 Intel Corporation
> + *
> + * Author: Zhang Chen <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include "net/filter.h"
> +#include "net/net.h"
> +#include "qemu-common.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qapi-visit.h"
> +#include "qom/object.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/error-report.h"
> +#include "trace.h"
> +#include "sysemu/char.h"
> +#include "qemu/iov.h"
> +#include "qemu/sockets.h"
> +
> +#ifndef QEMU_FILTER_MIRROR_H
> +#define QEMU_FILTER_MIRROR_H
> +
> +typedef struct TrafficState {
> +    NetFilterState parent_obj;
> +    NetQueue *incoming_queue;/* guest normal net queue */

The comment is wrong here, guest nic net queue should be incoming_queue
of NetClientState.

> +    char *indev;
> +    char *outdev;
> +    CharDriverState *chr_in;
> +    CharDriverState *chr_out;

We could just extend current MirrorState.

> +} TrafficState;
> +
> +static int filter_traffic_send(NetFilterState *nf,
> +                                   const struct iovec *iov,
> +                                   int iovcnt)
> +{
> +    TrafficState *s = (TrafficState *)nf;
> +    int ret = 0;
> +    ssize_t size = 0;
> +    uint32_t len =  0;
> +    char *buf;
> +    size = iov_size(iov, iovcnt);
> +    if (!size) {
> +        return 0;
> +    }
> +
> +    len = htonl(size);
> +    ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)&len, sizeof(len));
> +    if (ret != sizeof(len)) {
> +        goto err;
> +    }
> +
> +    buf = g_malloc(size);
> +    iov_to_buf(iov, iovcnt, 0, buf, size);
> +    ret = qemu_chr_fe_write_all(s->chr_out, (uint8_t *)buf, size);
> +    g_free(buf);
> +    if (ret != size) {
> +        goto err;
> +    }
> +
> +    return 0;
> +
> +err:
> +    return ret < 0 ? ret : -EIO;
> +}
> +
> +#endif /* QEMU_FILTER_MIRROR_H */