On Mon, May 23, 2016 at 01:19:46PM +0200, Olivier Matz wrote: > Hi, > > On 05/19/2016 05:50 PM, Thomas Monjalon wrote: > > 2016-05-19 19:05, Jerin Jacob: > >> On Thu, May 19, 2016 at 12:18:57PM +0000, Ananyev, Konstantin wrote: > >>>> On Thu, May 19, 2016 at 12:20:16AM +0530, Jerin Jacob wrote: > >>>>> On Wed, May 18, 2016 at 05:43:00PM +0100, Bruce Richardson wrote: > >>>>>> On Wed, May 18, 2016 at 07:27:43PM +0530, Jerin Jacob wrote: > >>> I wonder does anyone really use mbuf port field? > >>> My though was - could we to drop it completely? > >>> Actually, after discussing it with Bruce offline, an interesting idea > >>> came out: > >>> if we'll drop port and make mbuf_prefree() to reset nb_segs=1, then > >>> we can reduce RX rearm_data to 4B. So with that layout: > >>> > >>> struct rte_mbuf { > >>> > >>> MARKER cacheline0; > >>> > >>> void *buf_addr; > >>> phys_addr_t buf_physaddr; > >>> uint16_t buf_len; > >>> uint8_t nb_segs; > >>> uint8_t reserved_1byte; /* former port */ > >>> > >>> MARKER32 rearm_data; > >>> uint16_t data_off; > >>> uint16_t refcnt; > >>> > >>> uint64_t ol_flags; > >>> ... > >>> > >>> We can keep buf_len at its place and avoid 2B gap, while making rearm_data > >>> 4B long and 4B aligned. > >> > >> Couple of comments, > >> - IMO, It is good if nb_segs can move under rearm_data, as some > >> drivers(not in ixgbe may be) can write nb_segs in one shot also > >> in segmented rx handler case > >> - I think, it makes sense to keep port in mbuf so that application > >> can make use of it(Not sure what real application developers think of > >> this) > > > > I agree we could try to remove the port id from mbuf. > > These mbuf data are a common base to pass infos between drivers and apps. > > If you need to store some data which are read and write from the app only, > > you can have use the private zone (see rte_pktmbuf_priv_size). > > At the first read, I was in favor of keeping the port_id in the > mbuf. But after checking the examples and applications, I'm not > opposed to remove it. Indeed, this information could go in an > application-specific part or it could be an additional function > parameter in the application processing function. > > The same question could be raised for nb_seg: it seems this info > is not used a lot, and having a 8 bits value here also prevents from > having long chains (ex: for socket buffer in a tcp stack). > > Just an idea thrown in the air: if these 2 fields are removed, it > brings some room for the m->next field to go in the first cache line. > This was mentioned several times (at least [1]). > > [1] http://dpdk.org/ml/archives/dev/2015-June/019182.html
Can we come to some consensus on this for this release. The original problem was mbuf->rearm_data not being natually aligned and the assosiated performacnce issues with ARM architecture for non naturally aligned access. I believe that is fixing in this patch without any performance degradation on IA. I believe that is a good progress. Can we make further mbuff improvements as a additional problem to solve. Thoughts ? Jerin