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

[zhanghailiang]

Hi Jason,

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

I have thought about this. I'd like to add this default buffer filter quietly,
not through qemu cli. In this way, we can still keep the buffer filter that 
configured by users,
and keep its delay release packets capability. Though the delay time is not what
users suppose. (This is only happened in COLO's periodic mode, in normal colo 
mode, the delay time
is almost same with user's configure.)

What about call netdev_add_filter_buffer() in each netdev's init() ?
I didn't found a common code path for every netdev in their init path.

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