From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: mbuff rearm_data aligmenet issue on non x86 Date: Thu, 12 May 2016 17:47:21 +0530 Message-ID: <20160512121719.GA1806@localhost.localdomain> References: <20160512091349.GA10395@localhost.localdomain> <2601191342CEEE43887BDE71AB97725836B4FFE2@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "Richardson, Bruce" , "thomas.monjalon@6wind.com" To: "Ananyev, Konstantin" Return-path: Received: from na01-by2-obe.outbound.protection.outlook.com (mail-by2on0076.outbound.protection.outlook.com [207.46.100.76]) by dpdk.org (Postfix) with ESMTP id C46316CA2 for ; Thu, 12 May 2016 14:17:49 +0200 (CEST) Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB97725836B4FFE2@irsmsx105.ger.corp.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, May 12, 2016 at 10:07:09AM +0000, Ananyev, Konstantin wrote: > Hi Jerrin, >=20 > >=20 > > Hi All, > >=20 > > I would like align mbuff rearm_data field to 8 byte aligned so that > > write to mbuf->rearm_data with uint64_t* will be naturally aligned. > > I am not sure about IA but some other architecture/implementation has= overhead > > in non-naturally aligned stores. > >=20 > > Proposed patch is something like this below, But open for any change = to > > make fit for all other architectures/platform. > >=20 > > Any thoughts ? > >=20 > > =E2=9E=9C [master] [dpdk-master] $ git diff > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > index 529debb..5a917d0 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -733,10 +733,8 @@ struct rte_mbuf { > > void *buf_addr; /**< Virtual address of segment > > buffer. */ > > phys_addr_t buf_physaddr; /**< Physical address of segment > > buffer. */ > >=20 > > - uint16_t buf_len; /**< Length of segment buffer. */ > > - >=20 >=20 > There is no need to move buf_len itself, I think. > Just move rearm_data marker prior to buf_len is enough. > Though how do you suggest to deal with the fact, that right now we blin= dly > update the whole 64bits pointed by rearm_data: >=20 > drivers/net/ixgbe/ixgbe_rxtx_vec.c: > /* > * Flush mbuf with pkt template. > * Data to be rearmed is 6 bytes long. > * Though, RX will overwrite ol_flags that are coming n= ext > * anyway. So overwrite whole 8 bytes with one load: > * 6 bytes of rearm_data plus first 2 bytes of ol_flags= . > */ > p0 =3D (uintptr_t)&mb0->rearm_data; > *(uint64_t *)p0 =3D rxq->mbuf_initializer; >=20 > ? >=20 > If buf_len will be inside these 64bits, we can't do it anymore. >=20 > Are you suggesting something like: >=20 > uint64_t *p0, v0;=20 >=20 > p0 =3D &mb0->rearm_data; > v0 =3D *p0 & REARM_MASK; > *p0 =3D v0 | rxq->mbuf_initializer; > ?=20 Due to unaligned rearm_data issue, In ThunderX platform, we need to write multiple half word of aligned stores(so masking was better us). But I think, if we can put 16bit hole between port and ol_flags then we may not need the masking stuff in ixgbe. Right? OR Even better, if we can fill in a uint16_t variable which will replaced later in the flow like "data_len"? and move buf_len at end the first cache line? or any other thoughts to fix unaligned rearm_data issue? Jerin >=20 > If so I wonder what would be the performance impact of that change. > Konstantin >=20 >=20 > > /* next 6 bytes are initialised on RX descriptor rearm */ > > - MARKER8 rearm_data; > > + MARKER64 rearm_data; > > uint16_t data_off; > >=20 > > /** > > @@ -754,6 +752,7 @@ struct rte_mbuf { > > uint8_t nb_segs; /**< Number of segments. */ > > uint8_t port; /**< Input port. */ > >=20 > > + uint16_t buf_len; /**< Length of segment buffer. */ > > uint64_t ol_flags; /**< Offload features. */ > >=20 > > /* remaining bytes are set on RX when pulling packet from > > * descriptor > >=20 > > /Jerin