From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2000A42C3E; Tue, 6 Jun 2023 11:20:51 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0E2E040A84; Tue, 6 Jun 2023 11:20:51 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 4E69640697 for ; Tue, 6 Jun 2023 11:20:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686043248; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QprZ+mGauByp7PbuKXkf/psQi0gdPzLVWiUFGblUI3g=; b=Q6bo9hgSJejkju5LY3VEx9uIDHEWKx2Or12uHd2lttcCd8eY1p3KPkFNBO6Yc1fDfK6Yin jZjnsLUv1+5YvNn9EwdvnOPbA4TGc9nWCtMvRt5Gw0OQyN2vzI5ujmM+/lKbSpEZuOwIdU UTQYS8TgsBW26TUSakB+lJTCBHJBtGQ= Received: from mail-oi1-f199.google.com (mail-oi1-f199.google.com [209.85.167.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-611-dtgSfZjKN62qIEtiZFYhKw-1; Tue, 06 Jun 2023 05:20:45 -0400 X-MC-Unique: dtgSfZjKN62qIEtiZFYhKw-1 Received: by mail-oi1-f199.google.com with SMTP id 5614622812f47-39aa9617c6bso1938503b6e.1 for ; Tue, 06 Jun 2023 02:20:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686043245; x=1688635245; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=QprZ+mGauByp7PbuKXkf/psQi0gdPzLVWiUFGblUI3g=; b=D+RwMw69HY+1BURfcTFYFAL7yE8sIzDBG95YkCqohDn3TUIPIVo4oQePkQkBiJiTLO AKhazMjvoRgHfZ/GBYJbQs7t6cJdEaYr6UvU7buGuIxweZyWuSYnyJwcip3ER96i30Hl gbZ+D/5TbmTKjNW0tcCVn/nJ5XQeI3QSOTviQsV9LZkA+ualmTygHTjyNLfQ2gT5text YQDypbxzhEvrzwOY+LVpeysA7Hyh//vjU43vEjMwlHYW1PoeGylcyhoGq3HI/cEniFBy OJhRWi3S9oVebwrM+PBr8U+WKjiIaWfpbDRB+3s+OkpMqWJ+zXX29zqrUTkJVX4+x5Ni ikkw== X-Gm-Message-State: AC+VfDy66YafXcxh0RwofVumQOqsJ8EcuVpSSkduGaSzXeDLMpJzhnui px0TWnJlpRYUbuAfN2td4NiQ8PGZ2GHyqieD7M8w0f6+QlZVT+Fb3IKpmlsznWhC8r23YMjm3nv 0pcGOzpOLACsA5Awahw4= X-Received: by 2002:a05:6808:250:b0:394:41e7:5719 with SMTP id m16-20020a056808025000b0039441e75719mr1562265oie.40.1686043245065; Tue, 06 Jun 2023 02:20:45 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ72jnQVCEcgU6OaBRfB0bQEowP7gzvX5EdmE7U1oc47AhIaNK88GLXLGK669FNCPB+x2HIqOz0Mz4egrNsOARM= X-Received: by 2002:a05:6808:250:b0:394:41e7:5719 with SMTP id m16-20020a056808025000b0039441e75719mr1562251oie.40.1686043244820; Tue, 06 Jun 2023 02:20:44 -0700 (PDT) MIME-Version: 1.0 References: <20230526021132.41413-1-nicolas.chautru@intel.com> <20230526021132.41413-2-nicolas.chautru@intel.com> <20230525204722.73635324@hermes.local> <874c2179-6dfd-caba-0a8a-75137cb1a418@redhat.com> In-Reply-To: From: David Marchand Date: Tue, 6 Jun 2023 11:20:33 +0200 Message-ID: Subject: Re: [PATCH v1 1/1] doc: announce change in bbdev api related to operation extension To: "Chautru, Nicolas" Cc: Maxime Coquelin , Stephen Hemminger , "dev@dpdk.org" , "Rix, Tom" , "hemant.agrawal@nxp.com" , "Vargas, Hernan" X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, Jun 5, 2023 at 10:08=E2=80=AFPM Chautru, Nicolas 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 ca= using 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 > +`. 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', a= t offset 640 (in bits) at rte_bbdev.h:520:1 > 'rte_bbdev_dequeue_mldts_ops_t rte_bbdev::dequeue_mldts_ops', a= t 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_bbd= ev_queue_data*, rte_bbdev_fft_op**, typedef uint16_t)': > parameter 2 of type 'rte_bbdev_fft_op**' has sub-type cha= nges: > in pointed to type 'rte_bbdev_fft_op*': > in pointed to type 'struct rte_bbdev_fft_op' at rte_b= bdev_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::dewindow= ing_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_s= ize', 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]', a= t offset 1376 (in bits) at rte_bbdev_op.h:815:1 > 17 data member changes: > 'rte_bbdev_op_data rte_bbdev_op_fft::power_me= as_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_si= ze' offset changed from 416 to 544 (in bits) (by +128 bits) > 'uint16_t rte_bbdev_op_fft::input_leading_pad= ding' offset changed from 432 to 560 (in bits) (by +128 bits) > 'uint16_t rte_bbdev_op_fft::output_sequence_s= ize' offset changed from 448 to 576 (in bits) (by +128 bits) > 'uint16_t rte_bbdev_op_fft::output_leading_de= padding' offset changed from 464 to 592 (in bits) (by +128 bits) > 'uint8_t rte_bbdev_op_fft::window_index[6]' o= ffset 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 c= hanged 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 c= hanged from 584 to 712 (in bits) (by +128 bits) > 'uint16_t rte_bbdev_op_fft::ncs_reciprocal' o= ffset changed from 592 to 720 (in bits) (by +128 bits) > 'uint16_t rte_bbdev_op_fft::power_shift' offs= et 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 6= 40 to 768 (in bits) (by +128 bits) > 'rte_bbdev_data* rte_bbdev::data' offset changed from 704 to 83= 2 (in bits) (by +128 bits) > 'rte_bbdev_state rte_bbdev::state' offset changed from 768 to 8= 96 (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 1= 024 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 =3D enum changed_enumerators =3D RTE_CRYPTO_ASYM_XFORM_ECPM, RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END +[suppress_type] + name =3D rte_bbdev_fft_op ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ; Temporary exceptions till next major ABI version ; --=20 David Marchand