Re: [PATCH v1 1/1] doc: announce change in bbdev api related to operation extension

[David Marchand <david.marchand@redhat.com>]

On Mon, Jun 5, 2023 at 10:08 PM Chautru, Nicolas
<nicolas.chautru@intel.com> wrote:
> Wrt the MLD functions: these are new into the related serie but still the break the ABI since the struct rte_bbdev includes these functions hence causing offset changes.
>
> Should I then just rephrase as:
>
> +* bbdev: Will extend the API to support the new operation type
>  +``RTE_BBDEV_OP_MLDTS`` as per
>  +  this `v1
>  +<https://patches.dpdk.org/project/dpdk/list/?series=28192>`. This
>  +  will notably introduce
>  +  new symbols for ``rte_bbdev_dequeue_mldts_ops``,
>  +``rte_bbdev_enqueue_mldts_ops`` into the stuct rte_bbdev.

I don't think we need this deprecation notice.


Do you need to expose those new mldts ops in rte_bbdev?
Can't they go to dev_ops?
If you can't, at least moving those new ops at the end of the
structure would avoid the breakage on rte_bbdev.


>
> Pasting below the ABI results for reference
>
>   [C] 'function rte_bbdev* rte_bbdev_allocate(const char*)' at rte_bbdev.c:174:1 has some indirect sub-type changes:
>     return type changed:
>       in pointed to type 'struct rte_bbdev' at rte_bbdev.h:498:1:
>         type size hasn't changed
>         2 data member insertions:
>           'rte_bbdev_enqueue_mldts_ops_t rte_bbdev::enqueue_mldts_ops', at offset 640 (in bits) at rte_bbdev.h:520:1
>           'rte_bbdev_dequeue_mldts_ops_t rte_bbdev::dequeue_mldts_ops', at offset 704 (in bits) at rte_bbdev.h:522:1
>         7 data member changes (9 filtered):
>           type of 'rte_bbdev_dequeue_fft_ops_t rte_bbdev::dequeue_fft_ops' changed:
>             underlying type 'typedef uint16_t (rte_bbdev_queue_data*, rte_bbdev_fft_op**, typedef uint16_t)*' changed:
>               in pointed to type 'function type typedef uint16_t (rte_bbdev_queue_data*, rte_bbdev_fft_op**, typedef uint16_t)':
>                 parameter 2 of type 'rte_bbdev_fft_op**' has sub-type changes:
>                   in pointed to type 'rte_bbdev_fft_op*':
>                     in pointed to type 'struct rte_bbdev_fft_op' at rte_bbdev_op.h:978:1:
>                       type size changed from 832 to 1664 (in bits)
>                       1 data member change:
>                         type of 'rte_bbdev_op_fft rte_bbdev_fft_op::fft' changed:
>                           type size changed from 640 to 1472 (in bits)
>                           6 data member insertions:
>                             'rte_bbdev_op_data rte_bbdev_op_fft::dewindowing_input', at offset 256 (in bits) at rte_bbdev_op.h:771:1
>                             'int8_t rte_bbdev_op_fft::freq_resample_mode', at offset 768 (in bits) at rte_bbdev_op.h:807:1
>                             'uint16_t rte_bbdev_op_fft::output_depadded_size', at offset 784 (in bits) at rte_bbdev_op.h:809:1
>                             'uint16_t rte_bbdev_op_fft::cs_theta_0[12]', at offset 800 (in bits) at rte_bbdev_op.h:811:1
>                             'uint32_t rte_bbdev_op_fft::cs_theta_d[12]', at offset 992 (in bits) at rte_bbdev_op.h:813:1
>                             'int8_t rte_bbdev_op_fft::time_offset[12]', at offset 1376 (in bits) at rte_bbdev_op.h:815:1
>                           17 data member changes:
>                             'rte_bbdev_op_data rte_bbdev_op_fft::power_meas_output' offset changed from 256 to 384 (in bits) (by +128 bits)
>                             'uint32_t rte_bbdev_op_fft::op_flags' offset changed from 384 to 512 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::input_sequence_size' offset changed from 416 to 544 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::input_leading_padding' offset changed from 432 to 560 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::output_sequence_size' offset changed from 448 to 576 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::output_leading_depadding' offset changed from 464 to 592 (in bits) (by +128 bits)
>                             'uint8_t rte_bbdev_op_fft::window_index[6]' offset changed from 480 to 608 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::cs_bitmap' offset changed from 528 to 656 (in bits) (by +128 bits)
>                             'uint8_t rte_bbdev_op_fft::num_antennas_log2' offset changed from 544 to 672 (in bits) (by +128 bits)
>                             'uint8_t rte_bbdev_op_fft::idft_log2' offset changed from 552 to 680 (in bits) (by +128 bits)
>                             'uint8_t rte_bbdev_op_fft::dft_log2' offset changed from 560 to 688 (in bits) (by +128 bits)
>                             'int8_t rte_bbdev_op_fft::cs_time_adjustment' offset changed from 568 to 696 (in bits) (by +128 bits)
>                             'int8_t rte_bbdev_op_fft::idft_shift' offset changed from 576 to 704 (in bits) (by +128 bits)
>                             'int8_t rte_bbdev_op_fft::dft_shift' offset changed from 584 to 712 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::ncs_reciprocal' offset changed from 592 to 720 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::power_shift' offset changed from 608 to 736 (in bits) (by +128 bits)
>                             'uint16_t rte_bbdev_op_fft::fp16_exp_adjust' offset changed from 624 to 752 (in bits) (by +128 bits)
>           'const rte_bbdev_ops* rte_bbdev::dev_ops' offset changed from 640 to 768 (in bits) (by +128 bits)
>           'rte_bbdev_data* rte_bbdev::data' offset changed from 704 to 832 (in bits) (by +128 bits)
>           'rte_bbdev_state rte_bbdev::state' offset changed from 768 to 896 (in bits) (by +128 bits)
>           'rte_device* rte_bbdev::device' offset changed from 832 to 960 (in bits) (by +128 bits)
>           'rte_bbdev_cb_list rte_bbdev::list_cbs' offset changed from 896 to 1024 (in bits) (by +128 bits)
>           'rte_intr_handle* rte_bbdev::intr_handle' offset changed from 1024 to 1152 (in bits) (by +128 bits)

As for the report on the rte_bbdev_op_fft structure changes:
- wrt to its size, I think it is okay to waive it, rte_bbdev_fft_op
objects are coming from a bbdev mempool which is created by the bbdev
library itself (with the right element size if the application asked
for RTE_BBDEV_OP_FFT type),
- wrt to the fields locations, an application may have been touching
those fields, so moving all the added fields at the end of the
structure would be better.
  But on the other hand, an application will have to call an fft_ops
experimental API at some point, and the application developer is
already warned that ABI is not preserved on this part of the API,

So I would waive the changes on rte_bbdev_fft_op with something like:

diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index 3ff51509de..3cdce69418 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -36,6 +36,8 @@
 [suppress_type]
         type_kind = enum
         changed_enumerators = RTE_CRYPTO_ASYM_XFORM_ECPM,
RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
+[suppress_type]
+       name = rte_bbdev_fft_op

 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ; Temporary exceptions till next major ABI version ;


-- 
David Marchand