[dpdk-dev] [PATCH v1 2/3] app/crypto-perf: honour cryptodev's min headroom/tailroom

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Tuesday, July 10, 2018 12:17 PM
> To: 'Anoob Joseph' <anoob.joseph at caviumnetworks.com>; Doherty, Declan
> <declan.doherty at intel.com>
> Cc: 'Akhil Goyal' <akhil.goyal at nxp.com>; 'Ankur Dwivedi'
> <ankur.dwivedi at caviumnetworks.com>; 'Jerin Jacob'
> <jerin.jacob at caviumnetworks.com>; 'Narayana Prasad'
> <narayanaprasad.athreya at caviumnetworks.com>; 'dev at dpdk.org'
> <dev at dpdk.org>
> Subject: RE: [PATCH v1 2/3] app/crypto-perf: honour cryptodev's min
> headroom/tailroom
> 
> 
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Tuesday, July 10, 2018 12:08 PM
> > To: 'Anoob Joseph' <anoob.joseph at caviumnetworks.com>; Doherty, Declan
> > <declan.doherty at intel.com>
> > Cc: Akhil Goyal <akhil.goyal at nxp.com>; Ankur Dwivedi
> > <ankur.dwivedi at caviumnetworks.com>; Jerin Jacob
> > <jerin.jacob at caviumnetworks.com>; Narayana Prasad
> > <narayanaprasad.athreya at caviumnetworks.com>; dev at dpdk.org
> > Subject: RE: [PATCH v1 2/3] app/crypto-perf: honour cryptodev's min
> > headroom/tailroom
> >
> >
> >
> > > -----Original Message-----
> > > From: Anoob Joseph [mailto:anoob.joseph at caviumnetworks.com]
> > > Sent: Wednesday, July 4, 2018 2:56 PM
> > > To: Doherty, Declan <declan.doherty at intel.com>; De Lara Guarch,
> > > Pablo <pablo.de.lara.guarch at intel.com>
> > > Cc: Anoob Joseph <anoob.joseph at caviumnetworks.com>; Akhil Goyal
> > > <akhil.goyal at nxp.com>; Ankur Dwivedi
> > > <ankur.dwivedi at caviumnetworks.com>; Jerin Jacob
> > > <jerin.jacob at caviumnetworks.com>; Narayana Prasad
> > > <narayanaprasad.athreya at caviumnetworks.com>; dev at dpdk.org
> > > Subject: [PATCH v1 2/3] app/crypto-perf: honour cryptodev's min
> > > headroom/tailroom
> > >
> > > Crypto dev would specify its headroom and tailroom requirement and
> > > the application is expected to honour this while creating buffers.
> > >
> > > Signed-off-by: Anoob Joseph <anoob.joseph at caviumnetworks.com>
> >
> > ...
> >
> > > --- a/app/test-crypto-perf/cperf_test_common.c
> > > +++ b/app/test-crypto-perf/cperf_test_common.c
> >
> > ...
> >
> > > fill_multi_seg_mbuf(struct rte_mbuf *m, struct rte_mempool *mp,
> > >  		m->buf_iova = next_seg_phys_addr;
> > >  		next_seg_phys_addr += mbuf_hdr_size + segment_sz;
> > >  		m->buf_len = segment_sz;
> > > -		m->data_len = segment_sz;
> > > +		m->data_len = data_len;
> > >
> > > -		/* No headroom needed for the buffer */
> > > -		m->data_off = 0;
> > > +		/* Use headroom specified for the buffer */
> > > +		m->data_off = headroom;
> >
> > Headroom is only applicable for the first segment/s.
> > This is adding headroom in all the segments, which looks wrong.
> >
> 
> I think "max_size" needs to be recalculated in "cperf_alloc_common_memory",
> adding headroom and tailroom size, which will potentially increase the number
> of segments required.
> Then, headroom size needs to be checked in case it is bigger than segment size,
> so data might need to start in the next segment.
> Similar thing for tailroom.

Actually, forget about this. I have been thinking about it, and it looks artificial to do this.
Generally, in a mbuf pool, headroom is the same for all mbufs/segments.

In any case, I have a concern though about this. Headroom size is got from a compile time option:
CONFIG_RTE_PKTMBUF_HEADROOM=128. PMDs generally use this value to set "data_off",
but they could use another different value.
So what happens if min_mbuf_headroom is more than this value?
since this is not configurable, this won't work.

Also, generally, headroom and tailroom are used for encapsulation, so I am not sure if this is the best place.
What about using the private size of the mbuf? That is actually configurable, even though that data is not necessarily contiguous
to the mbuf data.

Sorry for the confusion and this last minute concern.

Thanks,
Pablo


> 
> Thanks,
> Pablo
> 
>