From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40329) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKkDX-0007xM-EC for qemu-devel@nongnu.org; Thu, 30 Jul 2015 05:33:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKkDR-00067g-7p for qemu-devel@nongnu.org; Thu, 30 Jul 2015 05:33:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52630) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKkDR-00067O-2p for qemu-devel@nongnu.org; Thu, 30 Jul 2015 05:33:29 -0400 Message-ID: <55B9EF61.8060206@redhat.com> Date: Thu, 30 Jul 2015 17:33:21 +0800 From: Jason Wang 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> In-Reply-To: <55B9E89F.3010106@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 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: Yang Hongyang , 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: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?