From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49315) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJLkU-0003Oc-VY for Qemu-devel@nongnu.org; Fri, 28 Feb 2014 06:37:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WJLkO-0005MH-EP for Qemu-devel@nongnu.org; Fri, 28 Feb 2014 06:37:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:4561) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJLkN-0005Kl-VF for Qemu-devel@nongnu.org; Fri, 28 Feb 2014 06:36:56 -0500 Message-ID: <531074CC.2020204@redhat.com> Date: Fri, 28 Feb 2014 12:36:44 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <5310489A.4060501@cisco.com> <53105EB0.3060702@redhat.com> <5310703C.1080303@cisco.com> In-Reply-To: <5310703C.1080303@cisco.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] Contribution - L2TPv3 transport List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Anton Ivanov (antivano)" Cc: "Qemu-devel@nongnu.org" , Stefan Hajnoczi Il 28/02/2014 12:17, Anton Ivanov (antivano) ha scritto: >> > As mentioned below, I suggest storing the cookies and session ids in >> > host order in NetL2TPV3State, and doing the conversion in >> > l2tpv3_form_header and friends. > I can fix it. I prefer to keep all params in "ready to use" form so that > no cycles are wasted on conversion in the portions which may affect > performance. > This is just one instruction (bswap) or zero on some hardware (PPC with has lwbrz, Haswell which has movbe), no reason to worry about it. It makes the code simpler by making all accesses use stX_be_p. >> Space before the opening brace, and parentheses around !(a & b) are >> unnecessary. More instances in the rest of the file. > > Bad habits die hard. After being burned by a couple of buggy borland > compilers 20 years ago I brace everything to the hilt. You have a point > though. We also brace everything, but we do not parenthesize everything. :) >> Why do you need separate mallocs for these? > > You do not really need to use a separate malloc for TX and RX. You can > reuse the first element of the RX vector for TX. > > Fair point. No, I mean: why not just use arrays in NetL2TPV3State? All of them are sized statically. Avoiding pointer chasing also improves performance. :) It also avoids memory leaks; I just noticed that you're not freeing the memory you allocate in net_l2tpv3_init (I checked s->header-buf). >> Is the local address mandatory? > > In L2TPv3 - yes. Ok. > In fact so is the remote - our "listen mode" is a hack. The listen mode is not implemented in this patch, is it? Thanks for the prompt reply. Note that I posted two small fixes to qemu-sockets.c. You may want to include them. Paolo