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

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Thursday, April 26, 2018 2:30 PM
> To: dev at dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar at intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>; Chalupnik, KamilX
> <kamilx.chalupnik at 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 at 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 at 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.

> +		 */