Re: [Qemu-devel] [PATCH] net/filter-redirector:Add filter-redirector

[Jason Wang]

On 02/29/2016 08:33 PM, Zhang Chen wrote:
>
>
> On 02/29/2016 03:11 PM, Jason Wang wrote:
>>
>> On 02/24/2016 05:03 PM, Zhang Chen wrote:
>>> If queue=rx, filter-redirector will get the packet that guest send,
>>> then redirect
>>> to outdev(if none, do nothing). but queue=rx/tx/all not related to
>>> indev. please
>>> look the flow chart below. queue=xxx just work for one
>>> way(filter->outdev).
>>>
>>>                                filter
>>>                                  +
>>>                                  |
>>>                                  |
>>>                 redirector       |
>>>                  +-------------------------+
>>>                  |               |         |
>>>                  |               |         |
>>>                  |               |         |
>>>     indev +----------------+     +---------------->  outdev
>>>                  |         |               |
>>>                  |         |               |
>>>                  |         |               |
>>>                  +-------------------------+
>>>                            |
>>>                            |
>>>                            v
>>>                         filter
>>>
>>>                            |
>>>
>>>                            |
>>>
>>>                            v
>>>                         filter ........ filter ...... guest
>>>
>> This looks a violation on the assumption of current filter behavior.
>> Each filter should only talk to the 'next' or 'prev' filter on the chain
>> (depends on the direction) or netdev when queue=rx or netdev's peer when
>> queue=tx.
>>
>> And in fact there's subtle differences with your patch:
>>
>> When queue='all' since you force nf->netdev as sender, direction is
>> NET_FILTER_DIRECTION_TX, the packet will be passed to 'next' filter on
>> the chain.
>> When queue='rx', direction is NET_FILTER_DIRECTION_RX, the packet will
>> be pass to 'prev' filter on the chain.
>>
>> So as you can see, 'all' is ambiguous here. I think we should keep
>> current behavior by redirecting traffic to netdev when queue='rx'. For
>> queue='all', maybe we need redirect the traffic to both netdev and
>> netdev's peer.
>>
>>
>
> OK, I will change usage to :
>
> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,outdev=s1,indev=s0,in_direction=tx/rx
>
> How about this?

Looks like in_direction complicates the issue and is unnecessary. In
fact, it could be achieved by having another redirector.

>
> I will fix it in V3.
>
> Thanks
> zhangchen