[v3,04/14] baseband/turbo_sw: support large size code block
Checks
Commit Message
From: Nic Chautru <nicolas.chautru@intel.com>
This is to support cases when the input data for
decoding a code block is larger than 64kB and would
not fit as a contiguous block of data into one
mbuf. In that case the length from the opearation
supersedes the mbug default structure.
Signed-off-by: Nic Chautru <nicolas.chautru@intel.com>
---
app/test-bbdev/test_bbdev_perf.c | 40 +++++++++++++++++++-----
doc/guides/rel_notes/release_20_05.rst | 6 ++++
drivers/baseband/turbo_sw/bbdev_turbo_software.c | 11 ++++---
3 files changed, 45 insertions(+), 12 deletions(-)
Comments
Acked-by: Dave Burley <dave.burley@accelercomm.com>
On 04/03/2020 18:54, Nicolas Chautru wrote:
> From: Nic Chautru <nicolas.chautru@intel.com>
>
> This is to support cases when the input data for
> decoding a code block is larger than 64kB and would
> not fit as a contiguous block of data into one
> mbuf. In that case the length from the opearation
> supersedes the mbug default structure.
>
> Signed-off-by: Nic Chautru <nicolas.chautru@intel.com>
> ---
> app/test-bbdev/test_bbdev_perf.c | 40 +++++++++++++++++++-----
> doc/guides/rel_notes/release_20_05.rst | 6 ++++
> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 11 ++++---
> 3 files changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index d8db58e..d46966d 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -764,6 +764,7 @@ typedef int (test_case_function)(struct active_device *ad,
> {
> int ret;
> unsigned int i, j;
> + bool large_input = false;
>
> for (i = 0; i < n; ++i) {
> char *data;
> @@ -774,24 +775,47 @@ typedef int (test_case_function)(struct active_device *ad,
> op_type, n * ref_entries->nb_segments,
> mbuf_pool->size);
>
> - TEST_ASSERT_SUCCESS(((seg->length + RTE_PKTMBUF_HEADROOM) >
> - (uint32_t)UINT16_MAX),
> - "Given data is bigger than allowed mbuf segment size");
> -
> + if (seg->length > 64000) {
> + /*
> + * Special case when DPDK mbuf cannot handle
> + * the required input size
> + */
> + printf("Warning: Larger input size than DPDK mbuf %d\n",
> + seg->length);
> + large_input = true;
> + } else {
> + TEST_ASSERT_SUCCESS(
> + ((seg->length + RTE_PKTMBUF_HEADROOM)
> + > (uint32_t)UINT16_MAX),
> + "Given data is bigger than allowed mbuf segment size"
> + );
> + }
> bufs[i].data = m_head;
> bufs[i].offset = 0;
> bufs[i].length = 0;
>
> if ((op_type == DATA_INPUT) || (op_type == DATA_HARQ_INPUT)) {
> - data = rte_pktmbuf_append(m_head, seg->length);
> - TEST_ASSERT_NOT_NULL(data,
> + if ((op_type == DATA_INPUT) && large_input) {
> + /* Allocate a fake overused mbuf */
> + data = rte_malloc(NULL, 128 * 1024, 0);
> + memcpy(data, seg->addr, seg->length);
> + m_head->buf_addr = data;
> + m_head->buf_iova = rte_mem_virt2phy(data);
> + m_head->data_off = 0;
> + m_head->data_len = seg->length;
> + } else {
> + data = rte_pktmbuf_append(m_head, seg->length);
> + TEST_ASSERT_NOT_NULL(data,
> "Couldn't append %u bytes to mbuf from %d data type mbuf pool",
> seg->length, op_type);
>
> - TEST_ASSERT(data == RTE_PTR_ALIGN(data, min_alignment),
> + TEST_ASSERT(data == RTE_PTR_ALIGN(
> + data, min_alignment),
> "Data addr in mbuf (%p) is not aligned to device min alignment (%u)",
> data, min_alignment);
> - rte_memcpy(data, seg->addr, seg->length);
> + rte_memcpy(data, seg->addr, seg->length);
> + }
> +
> bufs[i].length += seg->length;
>
> for (j = 1; j < ref_entries->nb_segments; ++j) {
> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
> index 2190eaf..d6c3dfb 100644
> --- a/doc/guides/rel_notes/release_20_05.rst
> +++ b/doc/guides/rel_notes/release_20_05.rst
> @@ -56,6 +56,12 @@ New Features
> Also, make sure to start the actual text at the margin.
> =========================================================
>
> +* **Updated the TURBO_SW bbdev PMD.**
> +
> + Updated the ``turbo_sw`` bbdev driver with changes including:
> +
> + * Support for large size code block not fitting in one mbuf segment.
> + * Exposes the accurate LLR decimal assumption.
>
> Removed Items
> -------------
> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> index 5ca8ca1..ea3fecb 100644
> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> @@ -1335,7 +1335,7 @@ struct turbo_sw_queue {
>
> static inline void
> process_ldpc_dec_cb(struct turbo_sw_queue *q, struct rte_bbdev_dec_op *op,
> - uint8_t c, uint16_t out_length, uint16_t e,
> + uint8_t c, uint16_t out_length, uint32_t e,
> struct rte_mbuf *m_in,
> struct rte_mbuf *m_out_head, struct rte_mbuf *m_out,
> struct rte_mbuf *m_harq_in,
> @@ -1617,8 +1617,8 @@ struct turbo_sw_queue {
> struct rte_bbdev_stats *queue_stats)
> {
> uint8_t c, r = 0;
> - uint16_t e, out_length;
> - uint16_t crc24_overlap = 0;
> + uint32_t e;
> + uint16_t out_length, crc24_overlap = 0;
> struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
> struct rte_mbuf *m_in = dec->input.data;
> struct rte_mbuf *m_harq_in = dec->harq_combined_input.data;
> @@ -1661,7 +1661,10 @@ struct turbo_sw_queue {
> e = (r < dec->tb_params.cab) ?
> dec->tb_params.ea : dec->tb_params.eb;
>
> - seg_total_left = rte_pktmbuf_data_len(m_in) - in_offset;
> + if (e < 64000) /* Special case handling when overusing mbuf */
> + seg_total_left = rte_pktmbuf_data_len(m_in) - in_offset;
> + else
> + seg_total_left = e;
>
> process_ldpc_dec_cb(q, op, c, out_length, e,
> m_in, m_out_head, m_out,
>
> From: Nic Chautru <nicolas.chautru@intel.com>
>
> This is to support cases when the input data for
> decoding a code block is larger than 64kB and would
> not fit as a contiguous block of data into one
> mbuf. In that case the length from the opearation
> supersedes the mbug default structure.
Spell check
Operation
Mbuf
>
> Signed-off-by: Nic Chautru <nicolas.chautru@intel.com>
> ---
> app/test-bbdev/test_bbdev_perf.c | 40 +++++++++++++++++++-----
> doc/guides/rel_notes/release_20_05.rst | 6 ++++
> drivers/baseband/turbo_sw/bbdev_turbo_software.c | 11 ++++---
> 3 files changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-
> bbdev/test_bbdev_perf.c
> index d8db58e..d46966d 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -764,6 +764,7 @@ typedef int (test_case_function)(struct active_device
> *ad,
> {
> int ret;
> unsigned int i, j;
> + bool large_input = false;
>
> for (i = 0; i < n; ++i) {
> char *data;
> @@ -774,24 +775,47 @@ typedef int (test_case_function)(struct active_device
> *ad,
> op_type, n * ref_entries->nb_segments,
> mbuf_pool->size);
>
> - TEST_ASSERT_SUCCESS(((seg->length +
> RTE_PKTMBUF_HEADROOM) >
> - (uint32_t)UINT16_MAX),
> - "Given data is bigger than allowed mbuf
> segment size");
> -
> + if (seg->length > 64000) {
Shouldn't this be seg->length + RTE_PKTMBUF_HEADROOM and you don't need
A check again in assert.
64000 should be a MACRO
> + /*
> + * Special case when DPDK mbuf cannot handle
> + * the required input size
> + */
> + printf("Warning: Larger input size than DPDK
> mbuf %d\n",
> + seg->length);
> + large_input = true;
> + } else {
> + TEST_ASSERT_SUCCESS(
> + ((seg->length +
> RTE_PKTMBUF_HEADROOM)
> + > (uint32_t)UINT16_MAX),
> + "Given data is bigger than allowed
> mbuf segment size"
> + );
> + }
> bufs[i].data = m_head;
> bufs[i].offset = 0;
> bufs[i].length = 0;
>
> if ((op_type == DATA_INPUT) || (op_type ==
> DATA_HARQ_INPUT)) {
> - data = rte_pktmbuf_append(m_head, seg->length);
> - TEST_ASSERT_NOT_NULL(data,
> + if ((op_type == DATA_INPUT) && large_input) {
> + /* Allocate a fake overused mbuf */
> + data = rte_malloc(NULL, 128 * 1024, 0);
Again hard coding. Define some macros to know the basis of this value.
> + memcpy(data, seg->addr, seg->length);
> + m_head->buf_addr = data;
> + m_head->buf_iova = rte_mem_virt2phy(data);
> + m_head->data_off = 0;
> + m_head->data_len = seg->length;
> + } else {
> + data = rte_pktmbuf_append(m_head, seg-
> >length);
> + TEST_ASSERT_NOT_NULL(data,
> "Couldn't append %u bytes to mbuf
> from %d data type mbuf pool",
> seg->length, op_type);
>
> - TEST_ASSERT(data == RTE_PTR_ALIGN(data,
> min_alignment),
> + TEST_ASSERT(data == RTE_PTR_ALIGN(
> + data, min_alignment),
> "Data addr in mbuf (%p) is not aligned
> to device min alignment (%u)",
> data, min_alignment);
> - rte_memcpy(data, seg->addr, seg->length);
> + rte_memcpy(data, seg->addr, seg->length);
> + }
> +
> bufs[i].length += seg->length;
>
> for (j = 1; j < ref_entries->nb_segments; ++j) {
> diff --git a/doc/guides/rel_notes/release_20_05.rst
> b/doc/guides/rel_notes/release_20_05.rst
> index 2190eaf..d6c3dfb 100644
> --- a/doc/guides/rel_notes/release_20_05.rst
> +++ b/doc/guides/rel_notes/release_20_05.rst
> @@ -56,6 +56,12 @@ New Features
> Also, make sure to start the actual text at the margin.
> =========================================================
>
> +* **Updated the TURBO_SW bbdev PMD.**
> +
> + Updated the ``turbo_sw`` bbdev driver with changes including:
> +
> + * Support for large size code block not fitting in one mbuf segment.
> + * Exposes the accurate LLR decimal assumption.
Is it really worth to highlight this in release note? You even suggested not to
Backport it to stable. I am not sure if it is really that important to be highlighted
In release notes.
If at all, this patch is not changing the LLR decimal. This should be moved to
Appropriate patch.
>
> Removed Items
> -------------
> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> index 5ca8ca1..ea3fecb 100644
> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> @@ -1335,7 +1335,7 @@ struct turbo_sw_queue {
>
> static inline void
> process_ldpc_dec_cb(struct turbo_sw_queue *q, struct rte_bbdev_dec_op *op,
> - uint8_t c, uint16_t out_length, uint16_t e,
> + uint8_t c, uint16_t out_length, uint32_t e,
> struct rte_mbuf *m_in,
> struct rte_mbuf *m_out_head, struct rte_mbuf *m_out,
> struct rte_mbuf *m_harq_in,
> @@ -1617,8 +1617,8 @@ struct turbo_sw_queue {
> struct rte_bbdev_stats *queue_stats)
> {
> uint8_t c, r = 0;
> - uint16_t e, out_length;
> - uint16_t crc24_overlap = 0;
> + uint32_t e;
> + uint16_t out_length, crc24_overlap = 0;
> struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
> struct rte_mbuf *m_in = dec->input.data;
> struct rte_mbuf *m_harq_in = dec->harq_combined_input.data;
> @@ -1661,7 +1661,10 @@ struct turbo_sw_queue {
> e = (r < dec->tb_params.cab) ?
> dec->tb_params.ea : dec->tb_params.eb;
>
> - seg_total_left = rte_pktmbuf_data_len(m_in) - in_offset;
> + if (e < 64000) /* Special case handling when overusing mbuf */
Macro for 64000
> + seg_total_left = rte_pktmbuf_data_len(m_in) -
> in_offset;
> + else
> + seg_total_left = e;
>
> process_ldpc_dec_cb(q, op, c, out_length, e,
> m_in, m_out_head, m_out,
> --
> 1.8.3.1
From: Akhil Goyal <akhil.goyal@nxp.com>
>> + if (seg->length > 64000) {
>Shouldn't this be seg->length + RTE_PKTMBUF_HEADROOM and you don't need A check again in assert.
>64000 should be a MACRO
Adding macro for this at rte_bbdev level in new version uploaded today.
>> + data = rte_malloc(NULL, 128 * 1024, 0);
>Again hard coding. Define some macros to know the basis of this value.
>
>> + Updated the ``turbo_sw`` bbdev driver with changes including:
>> +
>> + * Support for large size code block not fitting in one mbuf segment.
>> + * Exposes the accurate LLR decimal assumption.
>
>Is it really worth to highlight this in release note? You even suggested not to Backport it to stable. I am not sure if it is really that important to be highlighted In release notes.
>If at all, this patch is not changing the LLR decimal. This should be moved to Appropriate patch.
Removed.
Thanks
@@ -764,6 +764,7 @@ typedef int (test_case_function)(struct active_device *ad,
{
int ret;
unsigned int i, j;
+ bool large_input = false;
for (i = 0; i < n; ++i) {
char *data;
@@ -774,24 +775,47 @@ typedef int (test_case_function)(struct active_device *ad,
op_type, n * ref_entries->nb_segments,
mbuf_pool->size);
- TEST_ASSERT_SUCCESS(((seg->length + RTE_PKTMBUF_HEADROOM) >
- (uint32_t)UINT16_MAX),
- "Given data is bigger than allowed mbuf segment size");
-
+ if (seg->length > 64000) {
+ /*
+ * Special case when DPDK mbuf cannot handle
+ * the required input size
+ */
+ printf("Warning: Larger input size than DPDK mbuf %d\n",
+ seg->length);
+ large_input = true;
+ } else {
+ TEST_ASSERT_SUCCESS(
+ ((seg->length + RTE_PKTMBUF_HEADROOM)
+ > (uint32_t)UINT16_MAX),
+ "Given data is bigger than allowed mbuf segment size"
+ );
+ }
bufs[i].data = m_head;
bufs[i].offset = 0;
bufs[i].length = 0;
if ((op_type == DATA_INPUT) || (op_type == DATA_HARQ_INPUT)) {
- data = rte_pktmbuf_append(m_head, seg->length);
- TEST_ASSERT_NOT_NULL(data,
+ if ((op_type == DATA_INPUT) && large_input) {
+ /* Allocate a fake overused mbuf */
+ data = rte_malloc(NULL, 128 * 1024, 0);
+ memcpy(data, seg->addr, seg->length);
+ m_head->buf_addr = data;
+ m_head->buf_iova = rte_mem_virt2phy(data);
+ m_head->data_off = 0;
+ m_head->data_len = seg->length;
+ } else {
+ data = rte_pktmbuf_append(m_head, seg->length);
+ TEST_ASSERT_NOT_NULL(data,
"Couldn't append %u bytes to mbuf from %d data type mbuf pool",
seg->length, op_type);
- TEST_ASSERT(data == RTE_PTR_ALIGN(data, min_alignment),
+ TEST_ASSERT(data == RTE_PTR_ALIGN(
+ data, min_alignment),
"Data addr in mbuf (%p) is not aligned to device min alignment (%u)",
data, min_alignment);
- rte_memcpy(data, seg->addr, seg->length);
+ rte_memcpy(data, seg->addr, seg->length);
+ }
+
bufs[i].length += seg->length;
for (j = 1; j < ref_entries->nb_segments; ++j) {
@@ -56,6 +56,12 @@ New Features
Also, make sure to start the actual text at the margin.
=========================================================
+* **Updated the TURBO_SW bbdev PMD.**
+
+ Updated the ``turbo_sw`` bbdev driver with changes including:
+
+ * Support for large size code block not fitting in one mbuf segment.
+ * Exposes the accurate LLR decimal assumption.
Removed Items
-------------
@@ -1335,7 +1335,7 @@ struct turbo_sw_queue {
static inline void
process_ldpc_dec_cb(struct turbo_sw_queue *q, struct rte_bbdev_dec_op *op,
- uint8_t c, uint16_t out_length, uint16_t e,
+ uint8_t c, uint16_t out_length, uint32_t e,
struct rte_mbuf *m_in,
struct rte_mbuf *m_out_head, struct rte_mbuf *m_out,
struct rte_mbuf *m_harq_in,
@@ -1617,8 +1617,8 @@ struct turbo_sw_queue {
struct rte_bbdev_stats *queue_stats)
{
uint8_t c, r = 0;
- uint16_t e, out_length;
- uint16_t crc24_overlap = 0;
+ uint32_t e;
+ uint16_t out_length, crc24_overlap = 0;
struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
struct rte_mbuf *m_in = dec->input.data;
struct rte_mbuf *m_harq_in = dec->harq_combined_input.data;
@@ -1661,7 +1661,10 @@ struct turbo_sw_queue {
e = (r < dec->tb_params.cab) ?
dec->tb_params.ea : dec->tb_params.eb;
- seg_total_left = rte_pktmbuf_data_len(m_in) - in_offset;
+ if (e < 64000) /* Special case handling when overusing mbuf */
+ seg_total_left = rte_pktmbuf_data_len(m_in) - in_offset;
+ else
+ seg_total_left = e;
process_ldpc_dec_cb(q, op, c, out_length, e,
m_in, m_out_head, m_out,