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

[zhanghailiang]

On 2015/11/4 10:56, Jason Wang wrote:
> On 11/03/2015 07:56 PM, zhanghailiang wrote:
> > Signed-off-by: zhanghailiang <address@hidden>
> > Cc: Jason Wang <address@hidden>
>
> 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?

Actually, we didn't support hotplug device when COLO is enabled, maybe we
could support it in future.
If we support hotplug, and yes, it is a problem.

> - 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) ?

That's really a good idea, and we can enable packets 'buffer' capability only 
when needed.
I will investigate ... :)

Thanks，
zhanghailiang

> >   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);
>
>
> .