From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 5541758C4 for ; Mon, 12 May 2014 18:07:14 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 12 May 2014 09:02:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,1036,1389772800"; d="scan'208";a="539005076" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by orsmga002.jf.intel.com with ESMTP; 12 May 2014 09:06:25 -0700 Received: from orsmsx158.amr.corp.intel.com (10.22.240.20) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.123.3; Mon, 12 May 2014 09:06:24 -0700 Received: from orsmsx102.amr.corp.intel.com ([169.254.1.253]) by ORSMSX158.amr.corp.intel.com ([169.254.10.148]) with mapi id 14.03.0123.003; Mon, 12 May 2014 09:06:24 -0700 From: "Venkatesan, Venky" To: Olivier MATZ , Neil Horman Thread-Topic: [dpdk-dev] [PATCH RFC 06/11] mbuf: replace data pointer by an offset Thread-Index: AQHPbfRbff0d49EMAUK11UZsS7YOkZs9E/SQ Date: Mon, 12 May 2014 16:06:23 +0000 Message-ID: <1FD9B82B8BF2CF418D9A1000154491D9740A9631@ORSMSX102.amr.corp.intel.com> References: <1399647038-15095-1-git-send-email-olivier.matz@6wind.com> <1399647038-15095-7-git-send-email-olivier.matz@6wind.com> <3144526.CGFdr4BbI8@xps13> <1FD9B82B8BF2CF418D9A1000154491D9740A92B8@ORSMSX102.amr.corp.intel.com> <20140512144108.GB21298@hmsreliant.think-freely.org> <5370E397.7000706@6wind.com> In-Reply-To: <5370E397.7000706@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.22.254.140] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH RFC 06/11] mbuf: replace data pointer by an offset X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 May 2014 16:07:14 -0000 Olivier,=20 The impact isn't going to be felt on the driver quite as much (and can be m= itigated) - the driver runs a pretty low IPC (~1.7) compared to some of the= more optimized code above it that actually accesses the data. The problem = with the dependent compute is like this - in effect you are changing=20 struct eth_hdr * eth =3D (struct eth_hdr *) m->data; to=20 struct eth_hdr * eth =3D (struct eth_hdr *) ( (char *)m->buf _addr + m->dat= a_offset); We have some code that actually processes 4-8 packets in parallel (parse + = hash), with a pretty high IPC. What we've done here is essentially replaced= is a simple load, with a load, load, add sequence in front of it. There i= s no real way to do these computations in parallel for multiple packets - i= t has to be done one or two at a time. What suffers is the IPC of the overa= ll function that does the parse/hash quite significantly. It's those functi= ons that I worry about more than the driver. I haven't yet been able to co= me up with a mitigation for this yet.=20 Neil,=20 The last time we looked at this change - and it's been a while ago, the neg= ative effect on the upper level functions built on this was on the order of= about 15-20%. It's probably will get worse once we tune the code even more= . Hope the above explanation gives you a flavour of the problem this will = introduce.=20 Regards,=20 -Venky -----Original Message----- From: Olivier MATZ [mailto:olivier.matz@6wind.com]=20 Sent: Monday, May 12, 2014 8:07 AM To: Neil Horman; Venkatesan, Venky Cc: Thomas Monjalon; dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH RFC 06/11] mbuf: replace data pointer by an = offset Hi Venky, On 05/12/2014 04:41 PM, Neil Horman wrote: >> This is a hugely problematic change, and has a pretty large=20 >> performance impact (because the dependency to compute and access). We=20 >> debated this for a long time during the early days of DPDK and=20 >> decided against it. This is also a repeated sequence - the driver=20 >> will do it twice (Rx + Tx) and the next level stack will do it twice=20 >> (Rx + Tx) ... >> >> My vote is to reject this change particular change to the mbuf. >> >> Regards, >> -Venky >> > Do you have perforamance numbers to compare throughput with and=20 > without this change? I always feel suspcious when I see the spectre=20 > of performane used to support or deny a change without supporting reasoni= ng or metrics. I agree with Neil. My feeling is that it won't impact performance, and it i= s correlated with the forwarding tests I've done with this patch. I don't really understand what would cost more by storing the offset instea= d of the virtual address. I agree that each time the stack will access to t= he begining of the mbuf, there will be an arithmetic operation, but it is c= ompensated by other operations that will be accelerated: - When receiving a packet, the driver will do: m->data_off =3D RTE_PKTMBUF_HEADROOM; instead of: m->data =3D (char*) rxm->buf_addr + RTE_PKTMBUF_HEADROOM; - Each time the stack will prepend data, it has to check if the headroom is large enough to do the operation. This will be faster as data_off is the headroom. - When transmitting a packet, the driver will get the physical address: phys_addr =3D m->buf_physaddr + m->data_off instead of: phys_addr =3D (m->buf_physaddr + \ ((char *)m->data - (char *)m->buf_addr))) Moreover, these operations look negligible to me (few cycles) compared to t= he large amount of arithmetic operations and tests done in the driver. Regards, Olivier