Re: [Qemu-devel] [PATCH COLO-Frame v10 35/38] netfilter: Introduce a API to automatically add filter-buffer for each netdev

[Jason Wang <jasowang@redhat.com>]

On 11/03/2015 07:56 PM, zhanghailiang wrote:
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Cc: Jason Wang <jasowang@redhat.com>

Commit log please.

> ---
> v10: new patch
> ---
>  include/net/filter.h |  1 +
>  include/net/net.h    |  3 ++
>  net/filter-buffer.c  | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/net.c            | 20 +++++++++++++
>  4 files changed, 108 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 4499d60..b0954ba 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -75,5 +75,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
>                                      void *opaque);
>  void filter_buffer_release_all(void);
>  void  filter_buffer_del_all_timers(void);
> +void qemu_auto_add_filter_buffer(NetFilterDirection direction, Error **errp);
>  
>  #endif /* QEMU_NET_FILTER_H */
> diff --git a/include/net/net.h b/include/net/net.h
> index 5c65c45..e32bd90 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -129,6 +129,9 @@ typedef void (*qemu_netfilter_foreach)(NetFilterState *nf, void *opaque,
>                                         Error **errp);
>  void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
>                              Error **errp);
> +typedef void (*qemu_netdev_foreach)(NetClientState *nc, void *opaque,
> +                                    Error **errp);
> +void qemu_foreach_netdev(qemu_netdev_foreach func, void *opaque, Error **errp);
>  int qemu_can_send_packet(NetClientState *nc);
>  ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
>                            int iovcnt);
> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
> index 05313de..0dc1efb 100644
> --- a/net/filter-buffer.c
> +++ b/net/filter-buffer.c
> @@ -15,6 +15,11 @@
>  #include "qapi-visit.h"
>  #include "qom/object.h"
>  #include "net/net.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp-output-visitor.h"
> +#include "qapi/qmp-input-visitor.h"
> +#include "monitor/monitor.h"
> +
>  
>  #define TYPE_FILTER_BUFFER "filter-buffer"
>  
> @@ -185,6 +190,85 @@ void filter_buffer_del_all_timers(void)
>      qemu_foreach_netfilter(filter_buffer_del_timer, NULL, NULL);
>  }
>  
> +static void netdev_add_filter_buffer(NetClientState *nc, void *opaque,
> +                                     Error **errp)
> +{
> +    NetFilterState *nf;
> +    bool found = false;
> +
> +    QTAILQ_FOREACH(nf, &nc->filters, next) {
> +        if (!strcmp(object_get_typename(OBJECT(nf)), TYPE_FILTER_BUFFER)) {
> +            found = true;
> +            break;
> +        }
> +    }
> +
> +    if (!found) {
> +        QmpOutputVisitor *qov;
> +        QmpInputVisitor *qiv;
> +        Visitor *ov, *iv;
> +        QObject *obj = NULL;
> +        QDict *qdict;
> +        void *dummy = NULL;
> +        char *id = g_strdup_printf("%s-%s.0", nc->name, TYPE_FILTER_BUFFER);
> +        char *queue = (char *) opaque;
> +        bool auto_add = true;
> +        Error *err = NULL;
> +
> +        qov = qmp_output_visitor_new();
> +        ov = qmp_output_get_visitor(qov);
> +        visit_start_struct(ov,  &dummy, NULL, NULL, 0, &err);
> +        if (err) {
> +            goto out;
> +        }
> +        visit_type_str(ov, &nc->name, "netdev", &err);
> +        if (err) {
> +            goto out;
> +        }
> +        visit_type_str(ov, &queue, "queue", &err);
> +        if (err) {
> +            goto out;
> +        }
> +        visit_type_bool(ov, &auto_add, "auto", &err);
> +        if (err) {
> +            goto out;
> +        }
> +        visit_end_struct(ov, &err);
> +        if (err) {
> +            goto out;
> +        }
> +        obj = qmp_output_get_qobject(qov);
> +        g_assert(obj != NULL);
> +        qdict = qobject_to_qdict(obj);
> +        qmp_output_visitor_cleanup(qov);
> +
> +        qiv = qmp_input_visitor_new(obj);
> +        iv = qmp_input_get_visitor(qiv);
> +        object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err);
> +        qmp_input_visitor_cleanup(qiv);
> +        qobject_decref(obj);
> +out:
> +        g_free(id);
> +        if (err) {
> +            error_propagate(errp, err);
> +        }
> +    }
> +}
> +/*
> +* This will be used by COLO or MC FT, for which they will need
> +* to buffer all the packets of all VM's net devices, Here we check
> +* and automatically add netfilter for netdev that doesn't attach any buffer
> +* netfilter.
> +*/
> +void qemu_auto_add_filter_buffer(NetFilterDirection direction, Error **errp)
> +{
> +    char *queue = g_strdup(NetFilterDirection_lookup[direction]);
> +
> +    qemu_foreach_netdev(netdev_add_filter_buffer, queue,
> +                                        errp);
> +    g_free(queue);
> +}
> +

This make me think for following questions:

- What if a nic is hot added after this "automatically" filter add?
- Maybe a better way is to have a default filter? It could be specified
through qemu cli or other (And default filter could be 'nop' which means
no filter) ?

>  static void filter_buffer_init(Object *obj)
>  {
>      object_property_add(obj, "interval", "int",
> diff --git a/net/net.c b/net/net.c
> index a333b01..4fbe0af 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -283,6 +283,26 @@ void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
>      }
>  }
>  
> +void qemu_foreach_netdev(qemu_netdev_foreach func, void *opaque, Error **errp)
> +{
> +    NetClientState *nc;
> +
> +    QTAILQ_FOREACH(nc, &net_clients, next) {
> +        if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> +            continue;
> +        }
> +        if (func) {
> +            Error *local_err = NULL;
> +
> +            func(nc, opaque, &local_err);
> +            if (local_err) {
> +                error_propagate(errp, local_err);
> +                return;
> +            }
> +        }
> +    }
> +}
> +
>  static void qemu_net_client_destructor(NetClientState *nc)
>  {
>      g_free(nc);