From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKkTH-0007kr-Uh for qemu-devel@nongnu.org; Thu, 30 Jul 2015 05:49:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKkTB-0004qM-UN for qemu-devel@nongnu.org; Thu, 30 Jul 2015 05:49:51 -0400 Received: from [59.151.112.132] (port=48152 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKkT3-0004i1-OA for qemu-devel@nongnu.org; Thu, 30 Jul 2015 05:49:45 -0400 Message-ID: <55B9F327.7020108@cn.fujitsu.com> Date: Thu, 30 Jul 2015 17:49:27 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <1438167116-29270-1-git-send-email-yanghy@cn.fujitsu.com> <1438167116-29270-10-git-send-email-yanghy@cn.fujitsu.com> <55B9B269.3030709@redhat.com> <55B9C898.8090406@cn.fujitsu.com> <55B9E30E.8070804@redhat.com> <55B9E89F.3010106@cn.fujitsu.com> <55B9EF61.8060206@redhat.com> In-Reply-To: <55B9EF61.8060206@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: thuth@redhat.com, mrhines@linux.vnet.ibm.com, zhang.zhanghailiang@huawei.com, stefanha@redhat.com On 07/30/2015 05:33 PM, Jason Wang wrote: > On 07/30/2015 05:04 PM, Yang Hongyang wrote: >> >> >> On 07/30/2015 04:40 PM, Jason Wang wrote: >>> >>> >>> On 07/30/2015 02:47 PM, Yang Hongyang wrote: >>>> On 07/30/2015 01:13 PM, Jason Wang wrote: >>>> [...] >>>>>> + >>>>>> +#include "net/filter.h" >>>>>> +#include "net/queue.h" >>>>>> +#include "filters.h" >>>>>> +#include "qemu-common.h" >>>>>> +#include "qemu/error-report.h" >>>>>> + >>>>>> +typedef struct FILTERBUFFERState { >>>>>> + NetFilterState nf; >>>>>> + NetClientState dummy; /* used to send buffered packets */ >>>>> >>>>> Why need this? Couldn't we just infer this from NetFilterState? >>>> >>>> Because we use existing API qemu_send_packet_async/raw to send >>>> packet, it takes an NetClientState as the first argument sender, >>>> and use sender->peer->incoming_queue as the dest queue, so in order to >>>> make this API work, we need to use this dummy NC and init it's >>>> peer to our dest(which is the network backend) >>>> Another way is to call qemu_net_queue_send(netdev->incoming_queue,...) >>>> directly, we still need a NetClientState *sender param, can not >>>> use NetFilterState. >>> >>> I think this is my meaning. Use NetFilterState->netdev. >> >> Problem is NetFilterState->netdev is our destination, we need a sender... >> if we use this, packet will be sent back to NIC... >> > > I see, then NetFilterState->netdev->peer is sender. But I think it's > better to track sender instead of destination in this case. Something > like dummy NC is not elegant. > >>> >>>> This dummy NC also been checked in filter_buffer_receive to avoid >>>> buffering >>>> packet been sent by ourself. >>>> >>> >>> I don't get why this is needed. Who is going to queue a packet in dummy >>> NC, consider it was not peered by any others? >> >> There's nothing in the dummy NC except the dummy->peer = >> NetFilterState->netdev >> This dummy NC only used to as a sender param of the existing APIs >> which send >> packets. When a buffered packet been sent, we shouldn't buffer it >> again, we >> cann't use any existing NC (packet->sender or NetFilterState->netdev) >> as the sender because otherwise we can't distinguish if the packet is >> a buffered >> packet sent by ourself. > > I see, so the reason is you are using qemu_deliver_packet() for both > enqueuing packet to filter and delivering packet to destination. How > about something like: > > E.g for qemu_send_packet_async(), move the hook before > qemu_send_packet_async_with_flags(). Then flush method can call > qemu_send_packet_async_with_flags() without any issue? I think we can't move the hook earlier, because filters only deal with the packets will actually been sent. for example, a dump filter. dump packet that probably won't been sent is wrong. calling qemu_send_packet_async() or qemu_send_packet_async_with_flags() doesn't mean the packet is sent, if the sent_cb is not provided and the other peer is not able to receive, the packet will be dropped. > > > > . > -- Thanks, Yang.