Re: [dpdk-dev] [PATCH v4 1/8] bbdev: add network order data capability

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

+ Thomas, Akhil

In case this is for release 21.05 isn't it a bit late for introducing a new API change?


> -----Original Message-----
> From: Hemant Agrawal <hemant.agrawal@nxp.com>
> Sent: Saturday, April 24, 2021 3:37 AM
> To: dev@dpdk.org; gakhil@marvell.com; Chautru, Nicolas
> <nicolas.chautru@intel.com>
> Cc: david.marchand@redhat.com; Hemant Agrawal
> <hemant.agrawal@nxp.com>
> Subject: [PATCH v4 1/8] bbdev: add network order data capability
> 
> This patch intoduces a new capability of the bbdev device to process the
> LDPC data in network byte order.
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  doc/guides/bbdevs/features/default.ini | 1 +
>  doc/guides/prog_guide/bbdev.rst        | 6 ++++++
>  lib/bbdev/rte_bbdev_op.h               | 8 ++++++--
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/bbdevs/features/default.ini
> b/doc/guides/bbdevs/features/default.ini
> index 5fe267a625..e5da644099 100644
> --- a/doc/guides/bbdevs/features/default.ini
> +++ b/doc/guides/bbdevs/features/default.ini
> @@ -14,3 +14,4 @@ LLR/HARQ Compression   =
>  External DDR Access    =
>  HW Accelerated         =
>  BBDEV API              =
> +Network Order Data     =
> diff --git a/doc/guides/prog_guide/bbdev.rst
> b/doc/guides/prog_guide/bbdev.rst index 6b2bd54e1a..89a86d10fb 100644
> --- a/doc/guides/prog_guide/bbdev.rst
> +++ b/doc/guides/prog_guide/bbdev.rst
> @@ -747,6 +747,9 @@ given below.
>  |RTE_BBDEV_LDPC_ENC_CONCATENATION                                    |
>  | Set if a device supports concatenation of non byte aligned output  |  +------
> --------------------------------------------------------------+
> +|RTE_BBDEV_LDPC_ENC_NETWORK_ORDER                                    |
> +| Set if a device supports network order data processing             |
> ++--------------------------------------------------------------------+

I don't believe this is an extra capability extension per se here, but actually a different assumption. Tell me if I misinterpret the intent of your serie. 
From looking at the PMD I assume that you may mean " Set when a device `requires` network order data processing"
Basically the distinction I am trying to highlight is that it depends whether we want to expose this as an incremental dynamic operation flag that you can set at run time (enqueue ...), or whether this is more of distinct assumption that may be different for each PMD (either one of the other). 
I assume this is the later, please confirm. Ie I assume that your PMD would not be able to process the data in case this is presented with other endianness (ie you don't check ever that flag in the op_flag in your driver), in that case your driver would fail many existing unit test if considered as an additional capability on top of default one. You probably see such failures if you have tried to run the regression which would confirm the issue. 
In that case we may want discuss whether this is not actually more something to be capture under `struct rte_bbdev_driver_info` as a bool endianness. Both arguably may work but later arguably closer to the suggested intent and less convoluted.  Worth discussing further/
But basically if as this under that structure that would be exposed the same way as different LLR numerical assumption and that can be handled gracefulyl in the application/bbdev-test. 
+ Tom Rix, Dave Burley 


> 
>  The structure passed for each LDPC encode operation is given below,  with
> the operation flags forming a bitmask in the ``op_flags`` field.
> @@ -942,6 +945,9 @@ given below.
>  |RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK                        |
>  | Set if a device supports loopback access to HARQ internal memory   |
>  +--------------------------------------------------------------------+
> +|RTE_BBDEV_LDPC_DEC_NETWORK_ORDER                                    |
> +| Set if a device supports network order data processing             |
> ++--------------------------------------------------------------------+
> 
>  The structure passed for each LDPC decode operation is given below,  with
> the operation flags forming a bitmask in the ``op_flags`` field.
> diff --git a/lib/bbdev/rte_bbdev_op.h b/lib/bbdev/rte_bbdev_op.h index
> f946842727..8fab617768 100644
> --- a/lib/bbdev/rte_bbdev_op.h
> +++ b/lib/bbdev/rte_bbdev_op.h
> @@ -186,7 +186,9 @@ enum rte_bbdev_op_ldpcdec_flag_bitmasks {
>  	 *  for HARQ memory. If not set, it is assumed the filler bits are not
>  	 *  in HARQ memory and handled directly by the LDPC decoder.
>  	 */
> -	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS = (1ULL <<
> 18)
> +	RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS = (1ULL <<
> 18),
> +	/** Set if a device supports network order data processing */
> +	RTE_BBDEV_LDPC_DEC_NETWORK_ORDER = (1ULL << 19)
>  };
> 
>  /** Flags for LDPC encoder operation and capability structure */ @@ -206,7
> +208,9 @@ enum rte_bbdev_op_ldpcenc_flag_bitmasks {
>  	/** Set if a device supports scatter-gather functionality. */
>  	RTE_BBDEV_LDPC_ENC_SCATTER_GATHER = (1ULL << 6),
>  	/** Set if a device supports concatenation of non byte aligned output
> */
> -	RTE_BBDEV_LDPC_ENC_CONCATENATION = (1ULL << 7)
> +	RTE_BBDEV_LDPC_ENC_CONCATENATION = (1ULL << 7),
> +	/** Set if a device supports network order data processing */
> +	RTE_BBDEV_LDPC_ENC_NETWORK_ORDER = (1ULL << 8)
>  };
> 
>  /** Flags for the Code Block/Transport block mode  */
> --
> 2.17.1