Re: [Qemu-devel] [PATCH v3 4/4] net/filter: Add a default filter to each netdev

[Hailiang Zhang]

On 2016/2/2 11:16, Yang Hongyang wrote:
> On 02/01/2016 08:01 PM, zhanghailiang wrote:
> > We add each netdev a default buffer filter, and
> > the default buffer filter is disabled, so it has
> > no side effect for packets delivering in qemu net layer.
> >
> > The default buffer filter can be used by COLO or Micro-checkpoint,
> > The reason we add the default filter is we hope to support
> > hot add network during COLO state in future.
> >
> > Signed-off-by: zhanghailiang <address@hidden>
> > ---
> > v2:
> > - Add codes that generate id automatically for default filter
> >    (Jason's suggestion)
> > - Some other minor fixes.
> > ---
> >   include/net/filter.h |  4 ++++
> >   net/net.c            | 23 +++++++++++++++++++++++
> >   2 files changed, 27 insertions(+)
> >
> > diff --git a/include/net/filter.h b/include/net/filter.h
> > index 661abef..8f5fa31 100644
> > --- a/include/net/filter.h
> > +++ b/include/net/filter.h
> > @@ -22,6 +22,10 @@
> >   #define NETFILTER_CLASS(klass) \
> >       OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER)
> >
> > +#define DEFAULT_FILTER_TYPE "nop"
> > +
> > +#define TYPE_FILTER_BUFFER "filter-buffer"
> > +
> >   typedef void (FilterSetup) (NetFilterState *nf, Error **errp);
> >   typedef void (FilterCleanup) (NetFilterState *nf);
> >   /*
> > diff --git a/net/net.c b/net/net.c
> > index a49af48..998e90a 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -77,6 +77,12 @@ const char *host_net_devices[] = {
> >
> >   int default_net = 1;
> >
> > +/*
> > + * TODO: Export this with an option for users to control
> > + * this with comand line ?
> > + */
> > +char default_netfilter_type[16] = TYPE_FILTER_BUFFER;
> > +
> >   /***********************************************************/
> >   /* network device redirectors */
> >
> > @@ -1029,6 +1035,23 @@ static int net_client_init1(const void *object, int 
> > is_netdev, Error **errp)
> >           }
> >           return -1;
> >       }
> > +
> > +    if (is_netdev) {
>
> If as I said earlier add a 'defult-filter' property to netdev, we
> can use that property here to init default filter.
>
> > +        const Netdev *netdev = object;
> > +        char default_name[128];
> > +
> > +        snprintf(default_name, sizeof(default_name),
> > +                "%s%s", netdev->id, DEFAULT_FILTER_TYPE);
>
> name is "nop"

No, we shouldn't use the same name for all default filters,
we need to generate the name automatically,
Or there will be an error: " attempt to add duplicate property 'nop' to object".

Here, i rename DEFAULT_FILTER_TYPE to DEFAULT_FILTER_ID,
it is only an identification for default filter.

Thanks,
Hailiang

> > +        /*
> > +        * Here we add each netdev a default filter,
> > +        * it will disabled by default, Users can enable it when necessary.
> > +        */
> > +        netdev_add_filter(netdev->id,
> > +                          default_netfilter_type,
> > +                          default_name,
>
> type is filter-buffer, but name is "nop"?
>
> > +                          true,
> > +                          errp);
> > +    }
> >       return 0;
> >   }