From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35230) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJN23-0005Es-6z for Qemu-devel@nongnu.org; Fri, 28 Feb 2014 07:59:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WJN1w-0007BQ-84 for Qemu-devel@nongnu.org; Fri, 28 Feb 2014 07:59:15 -0500 Received: from rcdn-iport-9.cisco.com ([173.37.86.80]:27263) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJN1v-0007BM-Vd for Qemu-devel@nongnu.org; Fri, 28 Feb 2014 07:59:08 -0500 From: "Anton Ivanov (antivano)" Date: Fri, 28 Feb 2014 12:59:06 +0000 Message-ID: <531087ED.10202@cisco.com> References: <5310489A.4060501@cisco.com> <53105EB0.3060702@redhat.com> <5310703C.1080303@cisco.com> <531074CC.2020204@redhat.com> In-Reply-To: <531074CC.2020204@redhat.com> Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [Qemu-devel] Contribution - L2TPv3 transport List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: "Qemu-devel@nongnu.org" , Stefan Hajnoczi On 28/02/14 11:36, Paolo Bonzini wrote: > 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. OK. > >>> 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). Correct, I have not updated the cleanup procedures to deal with freeing all the vector bits, it will be in the updated version. > >>> 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? Not yet. I had to remove it to add recvmmsg, I was going to add it. However it is not mandatory by any means. I am happy for this to be without it. > > Thanks for the prompt reply. > > Note that I posted two small fixes to qemu-sockets.c. You may want to > include them. OK. > > Paolo