On 01/25/2016 02:27 PM, Li Zhijian wrote:
>
>
> 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.
After the changes, it become 'always' not 'sometimes' :)
> 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.
>
Other, looks ok tough I may tweak the commit more or less.
Thanks
>
>>
>>>
>>> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
>>> Signed-off-by: Li Zhijian <lizhij...@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;
>>> + }
>>> }
>>> }
>>>
>>
>>
>>
>> .
>>
>
>
>
