From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNG56-0006pH-PW for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:59:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNG52-0006nb-GY for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:59:16 -0400 Received: from [59.151.112.132] (port=14379 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNG4z-0006lG-2h for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:59:12 -0400 Message-ID: <55C313BA.80307@cn.fujitsu.com> Date: Thu, 6 Aug 2015 15:58:50 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <1438677044-13030-1-git-send-email-yanghy@cn.fujitsu.com> <1438677044-13030-6-git-send-email-yanghy@cn.fujitsu.com> <55C3092A.8020104@redhat.com> <55C30CA5.8040808@cn.fujitsu.com> <55C30EC6.1040704@redhat.com> In-Reply-To: <55C30EC6.1040704@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 05/11] netfilter: hook packets before net queue send List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: thuth@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, eddie.dong@intel.com, mrhines@linux.vnet.ibm.com, stefanha@redhat.com On 08/06/2015 03:37 PM, Jason Wang wrote: > > > On 08/06/2015 03:28 PM, Yang Hongyang wrote: >> >> >> On 08/06/2015 03:13 PM, Jason Wang wrote: >>> >>> >>> On 08/04/2015 04:30 PM, Yang Hongyang wrote: >>>> Capture packets that will be sent. >>>> >>>> Signed-off-by: Yang Hongyang >>>> --- >>>> net/net.c | 66 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 65 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/net/net.c b/net/net.c >>>> index 03b2296..efccd56 100644 >>>> --- a/net/net.c >>>> +++ b/net/net.c >>>> @@ -569,6 +569,42 @@ int qemu_can_send_packet(NetClientState *sender) >>>> return 1; >>>> } >>>> >>>> +static ssize_t filter_receive_iov(NetClientState *nc, int chain, >>>> + NetClientState *sender, >>>> + unsigned flags, >>>> + const struct iovec *iov, >>>> + int iovcnt) >>>> +{ >>>> + ssize_t ret = 0; >>>> + NetFilterState *nf = NULL; >>>> + ssize_t size = iov_size(iov, iovcnt); >>>> + >>>> + QTAILQ_FOREACH(nf, &nc->filters, next) { >>>> + if (nf->chain == chain || nf->chain == NET_FILTER_ALL) { >>>> + ret = nf->info->receive_iov(nf, sender, flags, iov, >>>> iovcnt); >>>> + if (ret == size) { >>>> + return ret; >>>> + } >>>> + } >>>> + } >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static ssize_t filter_receive(NetClientState *nc, int chain, >>>> + NetClientState *sender, >>>> + unsigned flags, >>>> + const uint8_t *data, >>>> + size_t size) >>>> +{ >>>> + struct iovec iov = { >>>> + .iov_base = (void *)data, >>>> + .iov_len = size >>>> + }; >>>> + >>>> + return filter_receive_iov(nc, chain, sender, flags, &iov, 1); >>>> +} >>>> + >>>> ssize_t qemu_deliver_packet(NetClientState *sender, >>>> unsigned flags, >>>> const uint8_t *data, >>>> @@ -640,6 +676,7 @@ static ssize_t >>>> qemu_send_packet_async_with_flags(NetClientState *sender, >>>> NetPacketSent >>>> *sent_cb) >>>> { >>>> NetQueue *queue; >>>> + int ret; >>>> >>>> #ifdef DEBUG_NET >>>> printf("qemu_send_packet_async:\n"); >>>> @@ -650,6 +687,18 @@ static ssize_t >>>> qemu_send_packet_async_with_flags(NetClientState *sender, >>>> return size; >>>> } >>>> >>>> + /* Let filters handle the packet first */ >>>> + ret = filter_receive(sender, NET_FILTER_OUT, sender, flags, >>>> buf, size); >>>> + if (ret == size) { >>>> + return size; >>>> + } >>>> + >>>> + ret = filter_receive(sender->peer, NET_FILTER_IN, >>>> + sender, flags, buf, size); >>>> + if (ret == size) { >>>> + return size; >>>> + } >>> >>> I think we can just trust the return value of filter_receive(), no need >>> to check it against iov_size. >> >> You mean: >> if (ret) >> return ret; >> ? We need to check ret however. > > Yes, but the problem is you check it against size. What if > filter_receive() returns a value which is greater than 0 but smaller > than size. If this could not happen, probably no need to check. Not a > must but better to change this. Ok, will fix, thank you. > >> >>> >>> Also sent_cb was lost track if the packet was held by filter which seems >>> wrong. >> >> So we need to extend the params of filter's receive_iov API, add a >> sent_cb >> param. > > Yes. > >> BTW, it seems that sent_cb will only been called when packet is >> queueed and then sent, if the packet being sent directly sent_cb won't >> be called? am I right? or I missed somthing from the code... > > Yes, but if for some reason it couldn't. Sent cb is usually used to let > main loop to re-poll network backend. Yes, this matches what I saw. It is kind of strange though, a provided sent_cb won't be called when a packet been sent directly... > >> >>> >>> [...] >>> . >>> >> > > . > -- Thanks, Yang.