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 9AF9DA00C4; Fri, 30 Sep 2022 09:54:40 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4227340E5A; Fri, 30 Sep 2022 09:54:40 +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 828AD40684 for ; Fri, 30 Sep 2022 09:54:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1664524477; 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=CU+1F7tGK3K7PkeajlLWZMUHns0+1EvgBHKJtLSand8=; b=WHXww7LfROfqeH/9y6PCWnHXifdYr246vCHhsLeVdaZCofJh/1BgECZjnB+dD6xZbgOcQe QQLMH6B7Zzl5te/cNIOEFzK3gji+V4H+lR8Hq1+ap3HY5ZQrGlLH34syF5M2GosEkDrNa0 3UltDZ2DOjecP+nRZIn13nAJ0YCZLF4= 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-271-eY96203mNV6DB_bY4Mw4NQ-1; Fri, 30 Sep 2022 03:54:35 -0400 X-MC-Unique: eY96203mNV6DB_bY4Mw4NQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1DE9F1019C89; Fri, 30 Sep 2022 07:54:34 +0000 (UTC) Received: from [10.39.208.19] (unknown [10.39.208.19]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7FF49492CA2; Fri, 30 Sep 2022 07:54:31 +0000 (UTC) Message-ID: <6b03d267-b292-7ed3-20cf-be330ac89595@redhat.com> Date: Fri, 30 Sep 2022 09:54:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 Subject: Re: [EXT] [PATCH v7 6/7] bbdev: add queue related warning and status information To: "Chautru, Nicolas" , Akhil Goyal , Ferruh Yigit , "dev@dpdk.org" , "ferruh.yigit@xilinx.com" , Ray Kinsella , "thomas@monjalon.net" Cc: "trix@redhat.com" , "Richardson, Bruce" , "david.marchand@redhat.com" , "stephen@networkplumber.org" , "Zhang, Mingshan" , "hemant.agrawal@nxp.com" References: <1655491040-183649-6-git-send-email-nicolas.chautru@intel.com> <1661796438-204861-1-git-send-email-nicolas.chautru@intel.com> <1661796438-204861-7-git-send-email-nicolas.chautru@intel.com> <88a06267-0cf3-a6cd-0785-b5b2a419fc02@amd.com> <198c3707-abad-40df-a0ee-0e1b2aa330ee@amd.com> From: Maxime Coquelin In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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 Hi Nic, On 9/29/22 21:48, Chautru, Nicolas wrote: > Hi Thomas, > In absence of Ray (I did not see email from him for a some time) can you please advise on best option so that as to move on. > I can either keep as is based on initial review with Ray, or replace _PADDED_MAX to _SIZE_MAX macro as suggested by Ferruh. > I am happy either way as long as we are able to move forward. There is no full consensus but not strong opinion either from anyone. I would go with Ferruh's suggestion. Regards, Maxime > Thanks, > Nic > >> -----Original Message----- >> From: Akhil Goyal >> Sent: Thursday, September 29, 2022 11:33 AM >> To: Ferruh Yigit ; Chautru, Nicolas >> ; dev@dpdk.org; Maxime Coquelin >> ; ferruh.yigit@xilinx.com; Ray Kinsella >> >> Cc: thomas@monjalon.net; trix@redhat.com; Richardson, Bruce >> ; david.marchand@redhat.com; >> stephen@networkplumber.org; Zhang, Mingshan >> ; hemant.agrawal@nxp.com >> Subject: RE: [EXT] [PATCH v7 6/7] bbdev: add queue related warning and status >> information >> >>>> Thanks for your comment. >>>> To be totally honest I don't yet see how your suggestion would be >>>> better, but I >>> quite possibly miss something. I did not reply in line with your >>> comments so that to try to be clearer and avoid spreading the argument >>> to much. Ray and Bruce feel free to chime in as well. >>>> >>>> First to state the obvious: Nothing will change the fact that in >>>> case new enums >>> are being added in DPDK, and if the application doesn't change, then >>> user would not be able to interpret any such additional >>> status/capability (backward compatible only feature parity and still >>> ABI compliant) which is totally accepted as fine and up to the user, >>> but the intention is at least not to have adverse effect even when >>> they don’t update their code for such new features (notably in case >>> they just use an older PMD not supporting such new features as a basic >> typical example in the ecosystem). I think we agree on that problematic. >>>> >>>> In term of history of not using MAX value for enum, I believe there >>>> is already >>> well documented and you agree with the reasoning of why we had to move >>> away from this [1]. Not just cosmetically where the max value is >>> called an enum or a #define but to have application making hardcoded >>> assumption on the possible maximum range for such enum notably when >>> sizing array. The only caveat being that at the time, the community >>> did spot the weakness but did not come to an agreement with regards to >>> the best way to manage this moving forward. >>>> >>>> In case your point is purely cosmetic to rename the PADDED_MAX value >>>> from >>> the enum to a #define (both public) I don't see how this would make >>> thing clearer by obfuscating the fact it is genuinely a padded value >>> and to have that value directly related to the enum structure. Also >>> note that there is already an actual max value defined for these enums >>> (but kept private on purpose) which is used by the lib/bbdev functions >>> to restrict usage to what is actually supported in the given implementation >> (distinct from padded max value). >>>> >>>> Arguably the only concern I could understand in your message would >>>> be this >>> one " my concern was if user assumes all values valid until PADDED_MAX >>> and tries to iterate array until that value". >>>> But really the fact that it is indeed a padded value implies fairly >>>> explicitly that >>> we have padded the supported enums with placeholders enums not yet >> defined. >>> That is fairly tautological! I cannot see how it could confuse anyone. >>> That is indeed to avoid such confusion that we went on that direction >>> to expose a public future-proof padded maximum value. >>>> >>>> Then looking at usage in practice: when integrating the bbdev api >>>> with higher >>> level SW stacks (such as FlexRAN reference sw or 3rd party stacks) I >>> don’t see how any of this theoretical concerns you raised would be >>> relevant for any of these very cases (enqueue status, new capability >>> etc...). The only genuine concern was sizing array based on MAX value being >> not ABI compliant. >>>> I cannot think of any code in the application presently deployed or >>>> future that >>> would then do what you are concerned about and cause an issue, and we >>> definitely don’t do such things in any example for bbdev-test or in >>> FlexRAN reference code provided to the ecosystem. The application >>> would already have a default case when an enum being provided has no >>> matching application, or more accurately in practice they would purely >>> not look for these and hence these would be ignored seamlessly. >>>> >>>> Thanks again for the discussion. I wish this had happened earlier >>>> (we only >>> discussed this with Ray and Bruce while you were still at Intel), let >>> me know what you think. >>>> It may be more generally good moving forward to come to a general >>> agreement at your technical forum level to avoid confusion. When we >>> discussed earlier we came to the conclusion that the DPDK community >>> had well documented what not to do to avoid ABI breakage but not >>> necessarily what are the best alternatives. >>>> Hopefully such future discussion should not delay this serie to be >>>> applied but >>> still let me know. >>>> >>> >>> Hi Nic, >>> >>> I believe it is more clear/safe to convert to SIZE_MAX macros, >>> although it is not a blocker. >>> >>> Anyway, I am not sure about the value of continuing this discussion, >>> perhaps it is better to clarify the guidance for similar case with ABI >>> maintainer and techboard, so it can proceed according to the decision. >>> >> I agree with Ferruh's comment for converting to SIZE_MAX macros. >> However, it is not a strong comment from my side. >> Moving to techboard would mean this patchset would skip the RC1 window. >> I believe as Ray is the maintainer and go to person for ABI related issues. >> I believe if he can take a look at the suggestion and provide ack/nack to >> whichever Approach would be fine and we can go ahead in that direction. >> I would like to close this as soon as possible. There are a lot of patches to be >> blocked on this series. >> >> Regards, >> Akhil >