Re: [Qemu-devel] [PATCH v4 04/11] net: delete netfilter object when delete netdev

[Yang Hongyang]

On 08/06/2015 03:09 PM, Jason Wang wrote:
> On 08/04/2015 04:30 PM, Yang Hongyang wrote:
> > When we delete the netdev, we also delete the netfilter object
> > attached to it, because if the netdev is removed, the filters
> > which attached to it is useless.
> >
> > Signed-off-by: Yang Hongyang <address@hidden>
>
> Please squash this into patch 3. (otherwise you're fixing a bug
> introduced by patch 3).

Will do, thanks.

> > ---
> >   include/net/filter.h |  1 +
> >   net/filter.c         |  2 +-
> >   net/net.c            | 14 ++++++++++++++
> >   3 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/filter.h b/include/net/filter.h
> > index 8eff85a..c146be2 100644
> > --- a/include/net/filter.h
> > +++ b/include/net/filter.h
> > @@ -50,6 +50,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
> >                                       const char *model,
> >                                       const char *name,
> >                                       int chain);
> > +void qemu_del_net_filter(NetFilterState *nf);
> >   void netfilter_add(QemuOpts *opts, Error **errp);
> >   void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
> >
> > diff --git a/net/filter.c b/net/filter.c
> > index 54a5c06..24ec4e1 100644
> > --- a/net/filter.c
> > +++ b/net/filter.c
> > @@ -57,7 +57,7 @@ static void qemu_cleanup_net_filter(NetFilterState *nf)
> >       g_free(nf);
> >   }
> >
> > -static void qemu_del_net_filter(NetFilterState *nf)
> > +void qemu_del_net_filter(NetFilterState *nf)
> >   {
> >       /* handle multi queue? */
> >       qemu_cleanup_net_filter(nf);
> > diff --git a/net/net.c b/net/net.c
> > index d9b70cd..03b2296 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -28,6 +28,7 @@
> >   #include "hub.h"
> >   #include "net/slirp.h"
> >   #include "net/eth.h"
> > +#include "net/filter.h"
> >   #include "util.h"
> >
> >   #include "monitor/monitor.h"
> > @@ -385,6 +386,8 @@ void qemu_del_net_client(NetClientState *nc)
> >   {
> >       NetClientState *ncs[MAX_QUEUE_NUM];
> >       int queues, i;
> > +    NetFilterState *nf, *next;
> > +    QemuOpts *opts;
> >
> >       assert(nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC);
> >
> > @@ -396,6 +399,17 @@ void qemu_del_net_client(NetClientState *nc)
> >                                             MAX_QUEUE_NUM);
> >       assert(queues != 0);
> >
> > +    /*
> > +     * we delete/free the netfilter object attached to this netdev
> > +     * multiqueue netfilter is not supported now, so only delete
> > +     * nc->filters is enough.
> > +     */
>
> The comment is not correct since for multiqueue, each NetClientState is
> a queue. I think you can just remove above.

Will remove it in the next version, thanks.

> > +    QTAILQ_FOREACH_SAFE(nf, &nc->filters, next, next) {
> > +        opts = qemu_opts_find(qemu_find_opts_err("netfilter", NULL), nf->name);
> > +        qemu_del_net_filter(nf);
> > +        qemu_opts_del(opts);
> > +    }
> > +
> >       /* If there is a peer NIC, delete and cleanup client, but do not free. */
> >       if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> >           NICState *nic = qemu_get_nic(nc->peer);
>
> .

--
Thanks,
Yang.