> > 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
