Re: [dpdk-dev] [PATCH 04/13] baseband/turbo_sw: memcpy changed or removed from driver

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Thursday, April 26, 2018 2:30 PM
> To: dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Chalupnik, KamilX
> <kamilx.chalupnik@intel.com>
> Subject: [PATCH 04/13] baseband/turbo_sw: memcpy changed or removed from
> driver
> 

It is not clear what this patch is trying to do, based on the title?
Minimizing memory copying? Please, modify the title to be more clear.

> From: KamilX Chalupnik <kamilx.chalupnik@intel.com>
> 
> Optimization of Turbo Software driver:
> - usage of memcpy changed or removed
> - minor changes in defines definitions

Could you split this patch in two patches? One with the macro changes
and the other one making changes on the memory copying?

Also, do those macros need to be in bbdev layer?
Will they be used in other PMDs?

> 
> Signed-off-by: Kamil Chalupnik <kamilx.chalupnik@intel.com>
> ---
>  drivers/baseband/turbo_sw/bbdev_turbo_software.c | 143 +++++++++++++-----
> -----
>  lib/librte_bbdev/rte_bbdev_op.h                  |  18 ++-
>  2 files changed, 100 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> index 26b8560..2728b30 100644
> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c

...

> -		/* copy the input to the temporary buffer to be able to extend
> -		 * it by 3 CRC bytes
> -		 */
> -		rte_memcpy(q->enc_in, in, (k - 24) >> 3);
>  		crc_req.data = in;
>  		crc_req.len = (k - 24) >> 3;
> -		crc_resp.data = q->enc_in;
> -		bblib_lte_crc24a_gen(&crc_req, &crc_resp);
> +		/* Check if there is a room for CRC bits if not use
> +		 * the temporary buffer.
> +		 */

Check if there is room for CRC bits. If not....

> +		if (rte_pktmbuf_append(m_in, 3) == NULL) {
> +			rte_memcpy(q->enc_in, in, (k - 24) >> 3);
> +			in = q->enc_in;
> +		} else {
> +			/* Store 3 first bytes of next CB as they will be
> +			 * overwritten by CRC bytes. If it is the last CB then
> +			 * there is no point to store 3 next bytes and this
> +			 * if..else branch will be omitted.
> +			 */
> +			first_3_bytes = *((uint64_t *)&in[(k - 32) >> 3]);
> 

...

> -		rte_memcpy(q->enc_in, in, (k - 24) >> 3);
>  		crc_req.data = in;
>  		crc_req.len = (k - 24) >> 3;
> -		crc_resp.data = q->enc_in;
> -		bblib_lte_crc24b_gen(&crc_req, &crc_resp);
> +		/* Check if there is a room for CRC bits if this is the last
> +		 * CB in TB. If not use temporary buffer.

Same as above.

> +		 */