Re: [Qemu-devel] [PATCH] net: walk through filters reversely if traffic is outgress

[Li Zhijian]

On 01/25/2016 12:58 PM, Jason Wang wrote:
> On 01/22/2016 04:11 PM, Li Zhijian wrote:
> > Previously, if the netdev has more than one filters, the ingress
> > or outgress traffic pass the filter in the same order. this patch
> > is to make the outgress pass the filter in a reverse order
>
> Need a description why we need this.
how about the following description:
Previously, if we attach more than filters for one netdev, IN/OUT traffic pass 
through
filters in the a same order.
ingress: netdev ->filter1 ->filter2->...filter[n] -> emulated device
outgress: emulated device ->filter1 ->filter2 ->...filter[n] ->netdev.

But in some scene, we hope filters handle the outgress traffic in a reverse 
order.
For example, in colo-proxy(will be implemented later), we have a redirector 
filter and
a colo-rewriter filter, we need the filter behavior like that:
ingress(->)/outgress(<-): <->redirector <-> colo-rewriter <->emulated device

So, the patch will make the outgress traffic pass the filter in a reverse order.


> > Signed-off-by: Wen Congyang <address@hidden>
> > Signed-off-by: Li Zhijian <address@hidden>
> > ---
> >   include/net/net.h |  4 +++-
> >   net/filter.c      | 21 +++++++++++++++++++--
> >   net/net.c         | 23 ++++++++++++++++++-----
> >   3 files changed, 40 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/net/net.h b/include/net/net.h
> > index 7af3e15..1d807cc 100644
> > --- a/include/net/net.h
> > +++ b/include/net/net.h
> > @@ -79,6 +79,8 @@ typedef struct NetClientInfo {
> >       SetVnetBE *set_vnet_be;
> >   } NetClientInfo;
> >
> > +QTAILQ_HEAD(NetFilterHead, NetFilterState);
> > +
> >   struct NetClientState {
> >       NetClientInfo *info;
> >       int link_down;
> > @@ -92,7 +94,7 @@ struct NetClientState {
> >       NetClientDestructor *destructor;
> >       unsigned int queue_index;
> >       unsigned rxfilter_notify_enabled:1;
> > -    QTAILQ_HEAD(, NetFilterState) filters;
> > +    struct NetFilterHead filters;
> >   };
> >
> >   typedef struct NICState {
> > diff --git a/net/filter.c b/net/filter.c
> > index 5d90f83..17a8398 100644
> > --- a/net/filter.c
> > +++ b/net/filter.c
> > @@ -34,6 +34,22 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf,
> >       return 0;
> >   }
> >
> > +static NetFilterState *netfilter_next(NetFilterState *nf,
> > +                                      NetFilterDirection dir)
> > +{
> > +    NetFilterState *next;
> > +
> > +    if (dir == NET_FILTER_DIRECTION_TX) {
> > +        /* forward walk through filters */
> > +        next = QTAILQ_NEXT(nf, next);
> > +    } else {
> > +        /* reverse order */
> > +        next = QTAILQ_PREV(nf, NetFilterHead, next);
> > +    }
> > +
> > +    return next;
> > +}
> > +
> >   ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
> >                                       unsigned flags,
> >                                       const struct iovec *iov,
> > @@ -43,7 +59,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
> >       int ret = 0;
> >       int direction;
> >       NetFilterState *nf = opaque;
> > -    NetFilterState *next = QTAILQ_NEXT(nf, next);
> > +    NetFilterState *next = NULL;
> >
> >       if (!sender || !sender->peer) {
> >           /* no receiver, or sender been deleted, no need to pass it further */
> > @@ -61,6 +77,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
> >           direction = nf->direction;
> >       }
> >
> > +    next = netfilter_next(nf, direction);
> >       while (next) {
> >           /*
> >            * if qemu_netfilter_pass_to_next been called, means that
> > @@ -73,7 +90,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
> >           if (ret) {
> >               return ret;
> >           }
> > -        next = QTAILQ_NEXT(next, next);
> > +        next = netfilter_next(next, direction);
> >       }
> >
> >       /*
> > diff --git a/net/net.c b/net/net.c
> > index 87dd356..05ec996 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -580,11 +580,24 @@ static ssize_t filter_receive_iov(NetClientState *nc,
> >       ssize_t ret = 0;
> >       NetFilterState *nf = NULL;
> >
> > -    QTAILQ_FOREACH(nf, &nc->filters, next) {
> > -        ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
> > -                                     iovcnt, sent_cb);
> > -        if (ret) {
> > -            return ret;
> > +    assert(direction == NET_FILTER_DIRECTION_TX ||
> > +           direction == NET_FILTER_DIRECTION_RX);
> > +
>
> Don't get why we need this assert.
i will remove this assertion.


Thanks
Li Zhijian

> Other looks good.
>
> > +    if (direction == NET_FILTER_DIRECTION_TX) {
> > +        QTAILQ_FOREACH(nf, &nc->filters, next) {
> > +            ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
> > +                                         iovcnt, sent_cb);
> > +            if (ret) {
> > +                return ret;
> > +            }
> > +        }
> > +    } else {
> > +        QTAILQ_FOREACH_REVERSE(nf, &nc->filters, NetFilterHead, next) {
> > +            ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
> > +                                         iovcnt, sent_cb);
> > +            if (ret) {
> > +                return ret;
> > +            }
> >           }
> >       }
>
>
>
> .