From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58232) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKoB3-0005dz-Ax for qemu-devel@nongnu.org; Thu, 30 Jul 2015 09:47:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKoAz-0006EV-5s for qemu-devel@nongnu.org; Thu, 30 Jul 2015 09:47:17 -0400 Received: from [59.151.112.132] (port=40513 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKoAx-0005rP-L8 for qemu-devel@nongnu.org; Thu, 30 Jul 2015 09:47:13 -0400 Message-ID: <55BA2ACE.4060903@cn.fujitsu.com> Date: Thu, 30 Jul 2015 21:46:54 +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> <55B9F327.7020108@cn.fujitsu.com> <55B9F8EB.2040903@redhat.com> In-Reply-To: <55B9F8EB.2040903@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, stefanha@redhat.com, zhang.zhanghailiang@huawei.com, mrhines@linux.vnet.ibm.com On 07/30/2015 06:14 PM, Jason Wang wrote: > > > On 07/30/2015 05:49 PM, Yang Hongyang wrote: >> 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. > > It depends on how do you define 'actually been sent' and whether or not > we should have such accuracy. Packet could be dropped by various layers. > Reaching receive() or receive_iov() does not mean it can be sent for > sure. For example, lots of nics drop packet in their receive() > implementation. Ok, I think we can move the filter hook before calling qemu_net_queue_send/qemu_net_queue_send_iov, then in the flush method, we can call qemu_net_queue_send directly to avoid the packet go to the filter hook again. > . > -- Thanks, Yang.