Re: [Qemu-devel] [PATCH v3 07/12] netfilter: add an API to pass the packet to next filter

[Yang Hongyang]

On 08/04/2015 01:00 PM, Jason Wang wrote:
> On 08/03/2015 04:30 PM, Yang Hongyang wrote:
> > add an API qemu_netfilter_pass_to_next_iov() to pass the packet
> > to next filter, and a wrapper qemu_netfilter_pass_to_next().
> >
> > Signed-off-by: Yang Hongyang <address@hidden>
> > ---
> >   include/net/filter.h | 12 ++++++++++++
> >   include/net/net.h    |  1 +
> >   net/filter.c         | 31 +++++++++++++++++++++++++++++++
> >   net/net.c            | 13 +++++++++++++
> >   4 files changed, 57 insertions(+)
> >
> > diff --git a/include/net/filter.h b/include/net/filter.h
> > index 7f0c949..c2be970 100644
> > --- a/include/net/filter.h
> > +++ b/include/net/filter.h
> > @@ -53,4 +53,16 @@ void qemu_del_net_filter(NetFilterState *nf);
> >   void netfilter_add(QemuOpts *opts, Error **errp);
> >   void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
> >
> > +/* pass the packet to the next filter */
> > +void qemu_netfilter_pass_to_next_iov(NetFilterState *nf,
> > +                                     NetClientState *sender,
> > +                                     unsigned flags,
> > +                                     const struct iovec *iov,
> > +                                     int iovcnt);
> > +void qemu_netfilter_pass_to_next(NetFilterState *nf,
> > +                                 NetClientState *sender,
> > +                                 unsigned flags,
> > +                                 const uint8_t *data,
> > +                                 size_t size);
> > +
> >   #endif /* QEMU_NET_FILTER_H */
> > diff --git a/include/net/net.h b/include/net/net.h
> > index 5c5c109..d3bfe12 100644
> > --- a/include/net/net.h
> > +++ b/include/net/net.h
> > @@ -117,6 +117,7 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
> >                                       const char *name);
> >   int qemu_netdev_add_filter(NetClientState *nc, NetFilterState *nf);
> >   void qemu_netdev_remove_filter(NetClientState *nc, NetFilterState *nf);
> > +NetFilterState *qemu_netdev_next_filter(NetClientState *nc, NetFilterState 
> > *nf);
> >   NICState *qemu_new_nic(NetClientInfo *info,
> >                          NICConf *conf,
> >                          const char *model,
> > diff --git a/net/filter.c b/net/filter.c
> > index bf113e9..84845b1 100644
> > --- a/net/filter.c
> > +++ b/net/filter.c
> > @@ -131,6 +131,37 @@ void qmp_netfilter_del(const char *id, Error **errp)
> >       qemu_opts_del(opts);
> >   }
> >
> > +void qemu_netfilter_pass_to_next_iov(NetFilterState *nf,
> > +                                     NetClientState *sender,
> > +                                     unsigned flags,
> > +                                     const struct iovec *iov,
> > +                                     int iovcnt)
> > +{
> > +    NetFilterState *next = qemu_netdev_next_filter(nf->netdev, nf);
> > +
> > +    while (next) {
> > +        if (next->chain == nf->chain || next->chain == NET_FILTER_ALL) {
> > +            next->info->receive_iov(next, sender, flags, iov, iovcnt);
>
> If a packet is not held by one filter, we need pass it to next filter?

Yes, will fix, thank you.

> > +            return;
> > +        }
> > +        next = qemu_netdev_next_filter(next->netdev, next);
> > +    }
>
> If a packet has gone through all filters, we probably need to pass it
> the receiver's incoming queue.

Ok, will do, thanks.

> > +}
>
> Looks like no real user for this helper?

No real user in this series, I see your suggestion on the buffer
filter, will use this api in that filter, thank you.

> > +
> > +void qemu_netfilter_pass_to_next(NetFilterState *nf,
> > +                                 NetClientState *sender,
> > +                                 unsigned flags,
> > +                                 const uint8_t *data,
> > +                                 size_t size)
> > +{
> > +    struct iovec iov = {
> > +        .iov_base = (void *)data,
> > +        .iov_len = size
> > +    };
> > +
> > +    return qemu_netfilter_pass_to_next_iov(nf, sender, flags, &iov, 1);
> > +}
> > +
> >   typedef int (NetFilterInit)(const NetFilterOptions *opts,
> >                               const char *name, int chain,
> >                               NetClientState *netdev, Error **errp);
> > diff --git a/net/net.c b/net/net.c
> > index f774c39..e087763 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -339,6 +339,19 @@ void qemu_netdev_remove_filter(NetClientState *nc, 
> > NetFilterState *nf)
> >       remove_filter(nc, filter);
> >   }
> >
> > +NetFilterState *qemu_netdev_next_filter(NetClientState *nc, NetFilterState *nf)
> > +{
> > +    Filter *filter = NULL;
> > +
> > +    QTAILQ_FOREACH(filter, &nc->filters, next) {
> > +        if (filter->nf == nf) {
> > +            break;
> > +        }
> > +    }
>
> A little bit confused, can we just use net->next?

it's because we use struct Filter to manage filters attached to a netdev,
not struct NetFilterState.

> > +
> > +    return QTAILQ_NEXT(filter, next)->nf;
> > +}
> > +
> >   NICState *qemu_new_nic(NetClientInfo *info,
> >                          NICConf *conf,
> >                          const char *model,
>
> .

--
Thanks,
Yang.