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

[Li Zhijian <lizhijian@cn.fujitsu.com>]

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 <wency@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> ---
>>   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;
>> +            }
>>           }
>>       }
>>
>
>
>
> .
>