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 E798F42CA4; Tue, 13 Jun 2023 10:14:31 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D6EB040C35; Tue, 13 Jun 2023 10:14:31 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id EF75540698 for ; Tue, 13 Jun 2023 10:14:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686644069; 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=AoLhW8EySj19xXt9U1nCiQsTojhfjVPvDI2O12Er/gI=; b=XEekzNG/c3sSIl2fdUA4OR3m/Y+v1m2nPldptNexNrixXFibxgNC+6Z6+9kpjPLbcORQdB U4lMP7Gi6yNakeZKSYQnPiSxpQA1VYZc7dYpkYhrJxZ+/cyeU3QWMeyheNTnH/Van3pah7 usBYS0F/MF9BTIILE/H6qucbl/5dC7Q= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-400-LhVL_HoNPu6qhNoaYC8gZA-1; Tue, 13 Jun 2023 04:14:28 -0400 X-MC-Unique: LhVL_HoNPu6qhNoaYC8gZA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D97B985A5AA; Tue, 13 Jun 2023 08:14:27 +0000 (UTC) Received: from [10.39.208.37] (unknown [10.39.208.37]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5DF3140C20F5; Tue, 13 Jun 2023 08:14:26 +0000 (UTC) Message-ID: Date: Tue, 13 Jun 2023 10:14:24 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 To: "Chautru, Nicolas" , David Marchand Cc: Stephen Hemminger , "dev@dpdk.org" , "Rix, Tom" , "hemant.agrawal@nxp.com" , "Vargas, Hernan" 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> <30fbed4d-9f04-9a44-d77d-156e7a6257f5@redhat.com> From: Maxime Coquelin Subject: Re: [PATCH v1 1/1] doc: announce change in bbdev api related to operation extension In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 6/12/23 22:53, Chautru, Nicolas wrote: > Hi Maxime, David, > >> -----Original Message----- >> From: Maxime Coquelin >> >> On 6/6/23 23:01, Chautru, Nicolas wrote: >>> Hi David, >>> >>>> -----Original Message----- >>>> From: David Marchand > >> >>>> On Mon, Jun 5, 2023 at 10:08 PM 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 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 >>>>> +`. 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. >>> >>> It would probably be best to move all these ops at the end of the structure >> (ie. keep them together). >>> In that case the deprecation notice would call out that the rte_bbdev >> structure content is more generally modified. Probably best for the longer >> run. >>> David, Maxime, ok with that option? >>> >>> struct __rte_cache_aligned rte_bbdev { >>> rte_bbdev_enqueue_enc_ops_t enqueue_enc_ops; >>> rte_bbdev_enqueue_dec_ops_t enqueue_dec_ops; >>> rte_bbdev_dequeue_enc_ops_t dequeue_enc_ops; >>> rte_bbdev_dequeue_dec_ops_t dequeue_dec_ops; >>> rte_bbdev_enqueue_enc_ops_t enqueue_ldpc_enc_ops; >>> rte_bbdev_enqueue_dec_ops_t enqueue_ldpc_dec_ops; >>> rte_bbdev_dequeue_enc_ops_t dequeue_ldpc_enc_ops; >>> rte_bbdev_dequeue_dec_ops_t dequeue_ldpc_dec_ops; >>> rte_bbdev_enqueue_fft_ops_t enqueue_fft_ops; >>> rte_bbdev_dequeue_fft_ops_t dequeue_fft_ops; >>> const struct rte_bbdev_ops *dev_ops; >>> struct rte_bbdev_data *data; >>> enum rte_bbdev_state state; >>> struct rte_device *device; >>> struct rte_bbdev_cb_list list_cbs; >>> struct rte_intr_handle *intr_handle; >>> }; >> >> The best thing, as suggested by David, would be to move all the ops out of >> struct rte_bbdev, as these should not be visible to the application. > > That would be quite disruptive across all PMDs and possible perf impact to validate. I don’t think this is anywhere realistic to consider such a change in 23.11. > I believe moving these function at the end of the structure is a good compromise to avoid future breakage of rte_bbdev structure with almost seamless impact (purely a ABI break when moving into 23.11 which is not avoidable. Retrospectively we should have done that in 22.11 really. If we are going to break the ABI, better to do the right rework directly. Otherwise we'll end-up breaking it again next year. IMHO, moving these ops should be quite trivial and not much work. Otherwise, if we just placed the rte_bbdev_dequeue_mldts_ops and rte_bbdev_enqueue_mldts_ops at the bottom of struct rte_bbdev, it may not break the ABI, but that's a bit fragile: - rte_bbdev_devices[] is not static, but is placed in the BSS section so should be OK - struct rte_bbdev is cache-aligned, so it may work if adding these two ops do not overlap a cacheline which depends on the CPU architecture. Maxime > What do you think Maxime, David? Based on this I can adjust the change for 23.11 and update slightly the deprecation notice accordingly. > > Thanks > Nic >