From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:45851)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1aaHzP-00060a-VQ
	for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:11:32 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1aaHzK-0007gf-Sm
	for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:11:31 -0500
Received: from mx1.redhat.com ([209.132.183.28]:44203)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1aaHzK-0007gb-Mg
	for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:11:26 -0500
References: <1454655023-29701-1-git-send-email-zhangchen.fnst@cn.fujitsu.com>
	<56C52F61.1070702@redhat.com> <56C577BC.4060901@cn.fujitsu.com>
	<56CD2606.7030102@redhat.com> <56CD71DD.6040900@cn.fujitsu.com>
From: Jason Wang <jasowang@redhat.com>
Message-ID: <56D3EF0F.6040306@redhat.com>
Date: Mon, 29 Feb 2016 15:11:11 +0800
MIME-Version: 1.0
In-Reply-To: <56CD71DD.6040900@cn.fujitsu.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] net/filter-redirector:Add filter-redirector
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>, qemu devel <qemu-devel@nongnu.org>
Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>, Li Zhijian <lizhijian@cn.fujitsu.com>, Gui jianfeng <guijianfeng@cn.fujitsu.com>, "eddie.dong" <eddie.dong@intel.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Yang Hongyang <hongyang.yang@easystack.cn>



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.