From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48682) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNFkU-0004zH-56 for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:37:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNFkP-0005hf-Bu for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:37:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55625) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNFkP-0005hZ-4S for qemu-devel@nongnu.org; Thu, 06 Aug 2015 03:37:53 -0400 Message-ID: <55C30EC6.1040704@redhat.com> Date: Thu, 06 Aug 2015 15:37:42 +0800 From: Jason Wang 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> In-Reply-To: <55C30CA5.8040808@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 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: Yang Hongyang , 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: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. > >> >> 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. > >> >> [...] >> . >> >