[v1,1/6] bbdev: add capability for CRC16 check
Checks
Commit Message
Adding a missing operation when CRC16
is being used for TB CRC check.
Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
app/test-bbdev/test_bbdev_vector.c | 2 ++
doc/guides/prog_guide/bbdev.rst | 3 +++
doc/guides/rel_notes/release_21_11.rst | 1 +
lib/bbdev/rte_bbdev_op.h | 34 ++++++++++++++++++----------------
4 files changed, 24 insertions(+), 16 deletions(-)
Comments
13/08/2021 18:51, Nicolas Chautru:
> Adding a missing operation when CRC16
> is being used for TB CRC check.
>
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -84,6 +84,7 @@ API Changes
> Also, make sure to start the actual text at the margin.
> =======================================================
>
> +* bbdev: Added capability related to more comprehensive CRC options.
That's not an API change, the enum symbols are the same.
Only enum values are changed so it impacts only ABI.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, October 11, 2021 1:17 PM
> To: gakhil@marvell.com; Chautru, Nicolas <nicolas.chautru@intel.com>
> Cc: dev@dpdk.org; trix@redhat.com; hemant.agrawal@nxp.com; Zhang,
> Mingshan <mingshan.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16
> check
>
> 13/08/2021 18:51, Nicolas Chautru:
> > Adding a missing operation when CRC16
> > is being used for TB CRC check.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> > --- a/doc/guides/rel_notes/release_21_11.rst
> > +++ b/doc/guides/rel_notes/release_21_11.rst
> > @@ -84,6 +84,7 @@ API Changes
> > Also, make sure to start the actual text at the margin.
> > =======================================================
> >
> > +* bbdev: Added capability related to more comprehensive CRC options.
>
> That's not an API change, the enum symbols are the same.
> Only enum values are changed so it impacts only ABI.
Hi Thomas,
How is that not a API change when new additional capability are exposed? Ie. new enums defined for new capabilities.
I think I see other similar cases in the same release notes " * cryptodev: ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algo ...".
You know best, just checking the intent, maybe worth clarifying the guideline except in case this is just me.
>
>
11/10/2021 22:38, Chautru, Nicolas:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 13/08/2021 18:51, Nicolas Chautru:
> > > Adding a missing operation when CRC16
> > > is being used for TB CRC check.
> > >
> > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > ---
> > > --- a/doc/guides/rel_notes/release_21_11.rst
> > > +++ b/doc/guides/rel_notes/release_21_11.rst
> > > @@ -84,6 +84,7 @@ API Changes
> > > Also, make sure to start the actual text at the margin.
> > > =======================================================
> > >
> > > +* bbdev: Added capability related to more comprehensive CRC options.
> >
> > That's not an API change, the enum symbols are the same.
> > Only enum values are changed so it impacts only ABI.
>
> Hi Thomas,
> How is that not a API change when new additional capability are exposed? Ie. new enums defined for new capabilities.
API change is when the app source code has to be updated.
ABI change is when the app binary has to be rebuilt.
> I think I see other similar cases in the same release notes " * cryptodev: ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algo ...".
I don't see this one.
> You know best, just checking the intent, maybe worth clarifying the guideline except in case this is just me.
Given my explanation above, how would you classify your change?
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, October 11, 2021 11:53 PM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>
> Cc: gakhil@marvell.com; dev@dpdk.org; trix@redhat.com;
> hemant.agrawal@nxp.com; Zhang, Mingshan <mingshan.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1 1/6] bbdev: add capability for CRC16
> check
>
> 11/10/2021 22:38, Chautru, Nicolas:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 13/08/2021 18:51, Nicolas Chautru:
> > > > Adding a missing operation when CRC16 is being used for TB CRC
> > > > check.
> > > >
> > > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > > ---
> > > > --- a/doc/guides/rel_notes/release_21_11.rst
> > > > +++ b/doc/guides/rel_notes/release_21_11.rst
> > > > @@ -84,6 +84,7 @@ API Changes
> > > > Also, make sure to start the actual text at the margin.
> > > >
> =======================================================
> > > >
> > > > +* bbdev: Added capability related to more comprehensive CRC
> options.
> > >
> > > That's not an API change, the enum symbols are the same.
> > > Only enum values are changed so it impacts only ABI.
> >
> > Hi Thomas,
> > How is that not a API change when new additional capability are exposed?
> Ie. new enums defined for new capabilities.
>
> API change is when the app source code has to be updated.
Thanks. What you are referring to may be strictly API breakage as opposed to generic API change. I would expect an API change could be either backward compatible (extending API but application only has to change if it wants to use the new functionality) vs an actual API breakage (application needs to change regardless even to keep same functionality as before).
In case the intent is to use the 2 terms interchangeably (change vs breakage) then I agree that these 2 bbdev changes do not constitute an API breakage (only ABI).
It might be good to capture this more explicitly except if you believe this is obvious (doc describes ABI change, not API change). Regardless for next time I will use that distinction (change == breakage).
Thanks
> ABI change is when the app binary has to be rebuilt.
>
> > I think I see other similar cases in the same release notes " * cryptodev:
> ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algo ...".
>
> I don't see this one.
>
> > You know best, just checking the intent, maybe worth clarifying the
> guideline except in case this is just me.
>
> Given my explanation above, how would you classify your change?
>
>
12/10/2021 18:36, Chautru, Nicolas:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 11/10/2021 22:38, Chautru, Nicolas:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 13/08/2021 18:51, Nicolas Chautru:
> > > > > Adding a missing operation when CRC16 is being used for TB CRC
> > > > > check.
> > > > >
> > > > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > > > ---
> > > > > --- a/doc/guides/rel_notes/release_21_11.rst
> > > > > +++ b/doc/guides/rel_notes/release_21_11.rst
> > > > > @@ -84,6 +84,7 @@ API Changes
> > > > > Also, make sure to start the actual text at the margin.
> > > > >
> > =======================================================
> > > > >
> > > > > +* bbdev: Added capability related to more comprehensive CRC
> > options.
> > > >
> > > > That's not an API change, the enum symbols are the same.
> > > > Only enum values are changed so it impacts only ABI.
> > >
> > > Hi Thomas,
> > > How is that not a API change when new additional capability are exposed?
> > Ie. new enums defined for new capabilities.
> >
> > API change is when the app source code has to be updated.
>
> Thanks. What you are referring to may be strictly API breakage as opposed to generic API change. I would expect an API change could be either backward compatible (extending API but application only has to change if it wants to use the new functionality) vs an actual API breakage (application needs to change regardless even to keep same functionality as before).
Yes
An API change which does not break is a new feature, so it is referenced
at the beginning of the release notes in general.
> In case the intent is to use the 2 terms interchangeably (change vs breakage) then I agree that these 2 bbdev changes do not constitute an API breakage (only ABI).
> It might be good to capture this more explicitly except if you believe this is obvious (doc describes ABI change, not API change). Regardless for next time I will use that distinction (change == breakage).
Yes feel free to send a patch to rename "change" to 'breakage".
@@ -167,6 +167,8 @@
*op_flag_value = RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK;
else if (!strcmp(token, "RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP"))
*op_flag_value = RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP;
+ else if (!strcmp(token, "RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK"))
+ *op_flag_value = RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK;
else if (!strcmp(token, "RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS"))
*op_flag_value = RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS;
else if (!strcmp(token, "RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE"))
@@ -891,6 +891,9 @@ given below.
|RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP |
| Set to drop the last CRC bits decoding output |
+--------------------------------------------------------------------+
+|RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK |
+| Set for code block CRC-16 checking |
++--------------------------------------------------------------------+
|RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS |
| Set for bit-level de-interleaver bypass on input stream |
+--------------------------------------------------------------------+
@@ -84,6 +84,7 @@ API Changes
Also, make sure to start the actual text at the margin.
=======================================================
+* bbdev: Added capability related to more comprehensive CRC options.
ABI Changes
-----------
@@ -142,51 +142,53 @@ enum rte_bbdev_op_ldpcdec_flag_bitmasks {
RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK = (1ULL << 1),
/** Set to drop the last CRC bits decoding output */
RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP = (1ULL << 2),
+ /** Set for transport block CRC-16 checking */
+ RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK = (1ULL << 3),
/** Set for bit-level de-interleaver bypass on Rx stream. */
- RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS = (1ULL << 3),
+ RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS = (1ULL << 4),
/** Set for HARQ combined input stream enable. */
- RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE = (1ULL << 4),
+ RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE = (1ULL << 5),
/** Set for HARQ combined output stream enable. */
- RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE = (1ULL << 5),
+ RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE = (1ULL << 6),
/** Set for LDPC decoder bypass.
* RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE must be set.
*/
- RTE_BBDEV_LDPC_DECODE_BYPASS = (1ULL << 6),
+ RTE_BBDEV_LDPC_DECODE_BYPASS = (1ULL << 7),
/** Set for soft-output stream enable */
- RTE_BBDEV_LDPC_SOFT_OUT_ENABLE = (1ULL << 7),
+ RTE_BBDEV_LDPC_SOFT_OUT_ENABLE = (1ULL << 8),
/** Set for Rate-Matching bypass on soft-out stream. */
- RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS = (1ULL << 8),
+ RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS = (1ULL << 9),
/** Set for bit-level de-interleaver bypass on soft-output stream. */
- RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS = (1ULL << 9),
+ RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS = (1ULL << 10),
/** Set for iteration stopping on successful decode condition
* i.e. a successful syndrome check.
*/
- RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE = (1ULL << 10),
+ RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE = (1ULL << 11),
/** Set if a device supports decoder dequeue interrupts. */
- RTE_BBDEV_LDPC_DEC_INTERRUPTS = (1ULL << 11),
+ RTE_BBDEV_LDPC_DEC_INTERRUPTS = (1ULL << 12),
/** Set if a device supports scatter-gather functionality. */
- RTE_BBDEV_LDPC_DEC_SCATTER_GATHER = (1ULL << 12),
+ RTE_BBDEV_LDPC_DEC_SCATTER_GATHER = (1ULL << 13),
/** Set if a device supports input/output HARQ compression. */
- RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION = (1ULL << 13),
+ RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION = (1ULL << 14),
/** Set if a device supports input LLR compression. */
- RTE_BBDEV_LDPC_LLR_COMPRESSION = (1ULL << 14),
+ RTE_BBDEV_LDPC_LLR_COMPRESSION = (1ULL << 15),
/** Set if a device supports HARQ input from
* device's internal memory.
*/
- RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE = (1ULL << 15),
+ RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE = (1ULL << 16),
/** Set if a device supports HARQ output to
* device's internal memory.
*/
- RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE = (1ULL << 16),
+ RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE = (1ULL << 17),
/** Set if a device supports loop-back access to
* HARQ internal memory. Intended for troubleshooting.
*/
- RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK = (1ULL << 17),
+ RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK = (1ULL << 18),
/** Set if a device includes LLR filler bits in the circular buffer
* 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 << 19)
};
/** Flags for LDPC encoder operation and capability structure */