Re: DPDK Release Status Meeting 2022-06-30
David Marchand writes: > On Thu, Jun 30, 2022 at 1:43 PM Ferruh Yigit wrote: >> Last minute opens were: >> >> * I am getting ABI check crash, when again v21.11 with abigail 2.0.0, >> for the file 'librte_common_qat'. Not sure if this is specific to my >> environment, would be good if someone double checks. > > I ran into a crash too with 2.0.0. > This seems fixed in (unreleased) latest libabigail. Sorry about that. We are working towards releasing 2.1 soon, which should hopefully fix that. Cheers, -- Dodji
Re: [dpdk-dev] [PATCH v1] devtools: update abi ignore for cryptodev
Hello Thomas and others,
Thomas Monjalon writes:
> Question to an expert, Dodji,
Thanks for the kind words, but I am not an expert in anything, sadly. I
am just trying to keep learning about these things ;-)
> We have this structure:
>
> struct rte_cryptodev {
> lot of fields...
> uint8_t attached : 1;
> } __rte_cache_aligned;
>
> Because of the cache alignment, there is enough padding in the struct
> (no matter the size of the cache line) for adding two more pointers:
>
> struct rte_cryptodev {
> lot of fields...
> uint8_t attached : 1;
> struct rte_cryptodev_cb_rcu *enq_cbs;
> struct rte_cryptodev_cb_rcu *deq_cbs;
> } __rte_cache_aligned;
>
> We checked manually that the ABI is still compatible.
Right.
I am curious, but normally, libabigail should raise the addition of
structures, but then it'll tell you that there was no size or offset
change between the two structures. If it doesn't, then that's a bug. I
hope it does :-)
> Then I've added (quickly) a libabigail exception rule:
>
> [suppress_type]
> name = rte_cryptodev
> has_data_member_inserted_between = {0, 1023}
>
> Now we want to improve this rule to restrict the offsets
> to the padding at the end of the struct only,
> so we keep forbidding changes in existing fields,
> and forbidding additions further the current struct size.
> Is this new rule good?
>
> has_data_member_inserted_between = {offset_after(attached), end}
Yes, this rule should do what you think it says.
> Do you confirm that the keyword "end" means the old reference size?
Yes I do.
> What else do we need to check for adding a new field in a padding?
Actually, that rule will work independantly of it there is enough
padding or not. It'll shut down the change report, even if the added
data exceeds the padding.
You just made me think of an idea of a new feature there.
Maybe we'd need a new property for the [suppress_type] directive that
would suppress changes only if said changes don't modify the size of the
type or any offset of any member of the type?
Maybe something like:
[suppress_type]
; lots of properties can go here.
; ...
; If the type has any size or offset change
; then this suppression directive will fail
; and the change report will be emitted
has_no_size_or_offset_change
Would that be useful to you in this case,
Cheers,
--
Dodji
Re: [dpdk-dev] [PATCH v1] devtools: update abi ignore for cryptodev
Thomas Monjalon writes:
[...]
>> > Then I've added (quickly) a libabigail exception rule:
>> >
>> > [suppress_type]
>> >name = rte_cryptodev
>> >has_data_member_inserted_between = {0, 1023}
>> >
>> > Now we want to improve this rule to restrict the offsets
>> > to the padding at the end of the struct only,
>> > so we keep forbidding changes in existing fields,
>> > and forbidding additions further the current struct size.
>> > Is this new rule good?
>> >
>> >has_data_member_inserted_between = {offset_after(attached), end}
>>
>>
>> Yes, this rule should do what you think it says.
>>
>> > Do you confirm that the keyword "end" means the old reference size?
>>
>> Yes I do.
>>
>>
>> > What else do we need to check for adding a new field in a padding?
>>
>> Actually, that rule will work independantly of it there is enough
>> padding or not. It'll shut down the change report, even if the added
>> data exceeds the padding.
>
> I don't understand why.
> If "end" means the old reference size, then addition after the old size
> should be reported, isn't it?
Yes, you are right.
What I meant is that even if (in an hypothetical case, not yours) the
padding was so "small" that it wasn't going up to the 'end' of the
struct, that rule would have still shut down the change report.
[...]
Cheers,
--
Dodji
Re: [dpdk-dev] [PATCH v1] devtools: update abi ignore for cryptodev
"Kinsella, Ray" writes:
> On 22/01/2021 13:09, Dodji Seketeli wrote:
>> Thomas Monjalon writes:
>>
>> [...]
>>
>>>>> Then I've added (quickly) a libabigail exception rule:
>>>>>
>>>>> [suppress_type]
>>>>> name = rte_cryptodev
>>>>> has_data_member_inserted_between = {0, 1023}
>>>>>
>>>>> Now we want to improve this rule to restrict the offsets
>>>>> to the padding at the end of the struct only,
>>>>> so we keep forbidding changes in existing fields,
>>>>> and forbidding additions further the current struct size.
>>>>> Is this new rule good?
>>>>>
>>>>> has_data_member_inserted_between = {offset_after(attached), end}
>>>>
>>>>
>>>> Yes, this rule should do what you think it says.
>>>>
>>>>> Do you confirm that the keyword "end" means the old reference size?
>>>>
>>>> Yes I do.
>>>>
>>>>
>>>>> What else do we need to check for adding a new field in a padding?
>>>>
>>>> Actually, that rule will work independantly of it there is enough
>>>> padding or not. It'll shut down the change report, even if the added
>>>> data exceeds the padding.
>>>
>>> I don't understand why.
>>> If "end" means the old reference size, then addition after the old size
>>> should be reported, isn't it?
>>
>> Yes, you are right.
>>
>> What I meant is that even if (in an hypothetical case, not yours) the
>> padding was so "small" that it wasn't going up to the 'end' of the
>> struct, that rule would have still shut down the change report.
>
> Understood - you are talking about padding between members.
Exactly.
Cheers,
--
Dodji
Re: [PATCH v2 0/4] more replacement of zero length array
Hello, David Marchand writes: > Dodji confirmed the issue in libabigail and prepared a fix. Yes, that is correct. Sorry for the inconvenience. The patch fixing this issue in libabigail has been applied to the mainline and is https://sourceware.org/git/?p=libabigail.git;a=commit;h=3cc1c3423c89c2cfd9d451ab99b71f3a74b35127. > DPDK still needs a suppression rule (like the one proposed above) if > we want to merge this change before the libabigail fix makes it to all > distribs. We can discuss further if you need a "quick" 2.4.1 release as a vehicle to consume the fix or if you can wait for a 2.5 one that might come later. I agree however that in the mean time, a temporary suppression specification might be warranted. [...] Cheers, -- Dodji
Re: [PATCH v6 20/23] mbuf: remove and stop using rte marker fields
Hello,
David Marchand writes:
> Hello Dodji,
o/
[...]
> This change is reported as a potential ABI change.
>
> For the context, this patch
> https://patchwork.dpdk.org/project/dpdk/patch/1709012499-12813-21-git-send-email-roret...@linux.microsoft.com/
> removes null-sized markers (those fields were using RTE_MARKER, see
> https://git.dpdk.org/dpdk/tree/lib/eal/include/rte_common.h#n583) from
> the rte_mbuf struct.
Thank you for the context.
[...]
> As reported by the CI:
[...]
> [C] 'function const rte_eth_rxtx_callback*
> rte_eth_add_first_rx_callback(uint16_t, uint16_t, rte_rx_callback_fn,
> void*)' at rte_ethdev.c:5768:1 has some indirect sub-type changes:
> parameter 3 of type 'typedef rte_rx_callback_fn' has sub-type changes:
[...]
> in pointed to type 'struct rte_mbuf' at rte_mbuf_core.h:470:1:
> type size hasn't changed
> 4 data member deletions:
> 'RTE_MARKER cacheline0', at offset 0 (in bits) at
> rte_mbuf_core.h:467:1
> 'RTE_MARKER64 rearm_data', at offset 128 (in bits)
> at rte_mbuf_core.h:490:1
> 'RTE_MARKER rx_descriptor_fields1', at offset 256
> (in bits) at rte_mbuf_core.h:517:1
> 'RTE_MARKER cacheline1', at offset 512 (in bits) at
> rte_mbuf_core.h:598:1
> no data member change (1 filtered);
[...]
> I would argue this change do not impact ABI as the layout of the mbuf
> object is not impacted.
I agree that on the /particular platform/ that the checker runs on,
there is no incompatible ABI change because no data member offset from
the 'struct rte_mbuf' type got modified and the size of the type hasn't
changed either.
>
> Error: ABI issue reported for abidiff --suppr
> /home/runner/work/dpdk/dpdk/devtools/libabigail.abignore
> --no-added-syms --headers-dir1 reference/usr/local/include
> --headers-dir2 install/usr/local/include
> reference/usr/local/lib/librte_ethdev.so.24.0
> install/usr/local/lib/librte_ethdev.so.24.1
> ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged
> this as a potential issue).
>
> Opinions?
>
> Btw, I see no way to suppress this (except a global [suppress_type]
> name = rte_mbuf)...
Right.
To avoid having subsequent changes to that type from being "overly"
suppressed, maybe do something like:
[suppress_type]
name = rte_mbuf
has_size_change = no
has_data_member = {cacheline0, rearm_data, rx_descriptor_fields1,
cacheline1}
That way, only size-impacting changes to struct rte_mbuf in its form
that predates this patch would be suppressed, hopefully.
[...]
Cheers,
--
Dodji
Re: [PATCH v6 20/23] mbuf: remove and stop using rte marker fields
David Marchand writes:
> On Wed, Feb 28, 2024 at 3:04 PM Dodji Seketeli wrote:
>> > Btw, I see no way to suppress this (except a global [suppress_type]
>> > name = rte_mbuf)...
>>
>> Right.
>>
>> To avoid having subsequent changes to that type from being "overly"
>> suppressed, maybe do something like:
>>
>> [suppress_type]
>> name = rte_mbuf
>> has_size_change = no
>> has_data_member = {cacheline0, rearm_data, rx_descriptor_fields1,
>> cacheline1}
>>
>> That way, only size-impacting changes to struct rte_mbuf in its form
>> that predates this patch would be suppressed, hopefully.
>
> Do you mean, only changes *not* size-impacting would be suppressed?
Yes, of course. Sorry for the typo. You are right.
> This is slightly better than the suppression on the whole rte_mbuf
> object, but it won't catch field reordering iiuc.
Indeed.
>
> On the other hand, now that I try reordering fields (to test this
> suggestion of yours), I get build failures all over the DPDK tree
> because we have many build checks to ensure those fields are at known
> locations...
> So maybe we can relax and just go with the full suppression.
Yes, that would make sense.
Thanks!
--
Dodji
Re: [PATCH] lib: add get/set link settings interface
Hello, > On Fri, Apr 5, 2024 at 2:55 AM Tyler Retzlaff [...] >> i'm jealous we don't have libabigail on windows, so helpful. Heh, thank you for the kind words. David Marchand writes: [...] > libabigail is written in C++ and relies on the elfutils and libxml2 > libraries. That is correct. Thank you for chiming in, David. > I am unclear about what binary format is used in Windows... so I am > not sure how much work would be required to have it on Windows. So for a little bit of context, libabigail constructs its own internal representation (IR) of the artifacts relevant to perform the analysis. The middle-end of the system then performs the needed analysis on that IR. The binary format itself it handled by a front-end. Today, we have front-ends that reads the ELF format and constructs the IR and hands it over to the middle end. One could very well imagine a new front-end that knows how to read the Portable Executable (PE) format that is used on Windows, along with its accompanying debug information. Luckily, I've written articles[1][2] that explains how libabigail recently switched to having a multi-front-end architecture that we have used to support alternative debug information formats like CTF[3] and BTF. In light of that, adding a new PE front-end would be "just work to do", I would think. And I would obviously not be opposed to helping such a project and merging the result in the end. As for any feature request, I would encourage potentially interested parties to file an "enhancement request" on the bug tracker of the project[4]. > That's more something to discuss with Dodji :-). Thanks for the plug ;-) I hope this helps. [1]: https://developers.redhat.com/articles/2023/01/05/libabigail-multiple-debugging-formats#the_resulting_multi_front_end_architecture [2]: https://www.redhat.com/en/blog/bpf-type-format-support-libabigail-23 [3]: https://lwn.net/Articles/795384/ [4]: https://sourceware.org/bugzilla/enter_bug.cgi?product=libabigail Cheers, -- Dodji
Re: [dpdk-dev] [PATCH] devtools: give some hints for ABI errors
Hello, David Marchand writes: > abidiff can provide some more information about the ABI difference it > detected. > In all cases, a discussion on the mailing must happen but we can give > some hints to know if this is a problem with the script calling abidiff, > a potential ABI breakage or an unambiguous ABI breakage. > > Signed-off-by: David Marchand For what it's worth, the change looks good to me, at least from an abidiff perspective. Thanks. Cheers. -- Dodji
Re: [dpdk-dev] [PATCH v3] eal: use c11 atomic built-ins for interrupt status
David Marchand writes: [...] >> --- a/devtools/libabigail.abignore >> +++ b/devtools/libabigail.abignore >> @@ -48,6 +48,10 @@ >> changed_enumerators = RTE_CRYPTO_AEAD_LIST_END >> [suppress_variable] >> name = rte_crypto_aead_algorithm_strings >> +; Ignore updates of epoll event >> +[suppress_type] >> +type_kind = struct >> +name = rte_epoll_event > > In general, ignoring all changes on a structure is risky. > But the risk is acceptable as long as we remember this for the rest of > the 20.08 release (and we will start from scratch for 20.11). Right, I thought about this too when I saw that change. If that struct is inherently *not* part of the logically exposed ABI, the risk is really minimal as well. In that case, maybe a comment saying so in the .abignore file could be useful for future reference. [...] Cheers, -- Dodji
Re: [dpdk-dev] [PATCH v2] abi: change references to abi 20.0.1 to abi v21
Hello, Ray Kinsella writes: > ah ok, the particular system I made the change on was Ubuntu 18.04.2. > which is libabigail 1.2.0. Whoah, 1.2 is super old. In my opinion, one of the hallmarks of static analysis tools (and libabigail is just a static analysis framework) is to be able to recognize patterns used by developers, as much as we can. Because we can't really do that at once, we try to add recognition of new patterns (of ABI changes) at every single release. Furthermore, there are some change patterns that ought to be recognized and categorized as harmless, whereas some others out to be categorized as harmful. That categorization is also the result of input coming from users as you, fine fellows. All this to say that with every new version, the number of new supported features and bug fixes is potentially big. To alleviate that, some distributors update libabigail even in their old stable distros, because the value of having an up to date version there outweighs the potential drawbacks. > Given we still support v19.11 on Ubuntu 18.04.2. So maybe that's a discussion worth having with the maintainer of the Ubuntu package of Libabigail? > I think it's worthwhile keeping the suppression until v20.11? [...] David Marchand writes: > In Travis, we currently use libabigail 1.6 (mainly because I did not > update to 1.7 when it was released). Right, that's probably another way to stay up to date independently from the underlying distribution. I hope this helps, Cheers, -- Dodji
Re: [dpdk-dev] /validate-abi.sh complains [PATCH v1 3/8] ring: introduce RTS ring mode
Hello,
"Ananyev, Konstantin" writes:
> Hi David,
>
>> > Have a question regarding validate-abi.sh.
>> > It complains on the following changes with that patch:
>> >
>> > @@ -111,11 +129,21 @@ struct rte_ring {
>> > char pad0 __rte_cache_aligned; /**< empty cache line */
>> >
>> > /** Ring producer status. */
>> > - struct rte_ring_headtail prod __rte_cache_aligned;
>> > + RTE_STD_C11
>> > + union {
>> > + struct rte_ring_headtail prod;
>> > + struct rte_ring_rts_headtail rts_prod;
>> > + } __rte_cache_aligned;
>> > +
>> > char pad1 __rte_cache_aligned; /**< empty cache line */
>> >
>> > /** Ring consumer status. */
>> > - struct rte_ring_headtail cons __rte_cache_aligned;
>> > + RTE_STD_C11
>> > + union {
>> > + struct rte_ring_headtail cons;
>> > + struct rte_ring_rts_headtail rts_cons;
>> > + } __rte_cache_aligned;
>> > +
[...]
>> It reported a warning on those fields:
>> https://travis-ci.com/github/ovsrobot/dpdk/jobs/310689008#L2380
>> I understand this as a false positive too.
>>
>> It seems similar to the bz I opened about fields moved to anonymous
>> constructs: https://sourceware.org/bugzilla/show_bug.cgi?id=25661
>
> Yes, looks the same.
>
>
>> Cc: Dodji.
>>
>>
>> For the time being, you can waive this by adding a rule in
>> devtools/libabigail.abignore.
>
> Ok, so what is the procedure here?
> Should I submit changes to devtools/libabigail.abignore together with the
> patch that introduced the problem?
> BTW, I used the following changes in libabigail.abignore to supress errors:
> $ git diff devtools/libabigail.abignore
> diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> index a59df8f13..032479b9f 100644
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -11,3 +11,9 @@
> type_kind = enum
> name = rte_crypto_asym_xform_type
> changed_enumerators = RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> +; Ignore updates of ring prod/cons
> +[suppress_type]
> +type_kind = struct
> +name = rte_ring
> +has_data_members_inserted_at = offsetof(prod)
> +has_data_members_inserted_at = offsetof(cons)
>
> Do you know, is there a better (more fine-grained) approach?
I think what you did is correct.
I am teaching the tool to recognize this kind of change construct and
avoid emitting a false positive. So that you can drop these kind of
suppression specification.
Sorry for the inconvenience.
Cheers,
--
Dodji
Re: [dpdk-dev] [PATCH 00/16] NXP DPAAx fixes and enhancements
Hello Hemant,
Hemant Agrawal writes:
[...]
>> >> > > [Hemant]
>> >> > > As per the logs:
>> >> > >
>> >> > > Variables changes summary: 1 Removed, 2 Changed, 0 Added
>> >> > > variables
>> >> > > 1 Removed variable:
>> >> > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs'
>> >> > {per_lcore_dpaa2_held_bufs@@DPDK_20.0}
>> >> > > 2 Changed variables:
>> >> > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at
>> >> > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048
>> >> > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at
>> >> > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16
>> >> > >
>> >> > > Error: ABI issue reported for 'abidiff --suppr
>> >> > > devtools/libabigail.abignore --
>> >> > no-added-syms --headers-dir1 reference/usr/local/include
>> >> > --headers-dir2 install/usr/local/include
>> >> > reference/dump/librte_bus_fslmc.dump
>> >> > install/dump/librte_bus_fslmc.dump'
[...]
>> In the mean time, the tooling can be tought to ignore changes to these ELF
>> symbols, as you you guys all know already.
>>
> [Hemant] will you please help me about adding entry to libagigail.abignore
> I tried doing following, but it is not helping
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -2,10 +2,15 @@
> symbol_version = EXPERIMENTAL
> [suppress_variable]
> symbol_version = EXPERIMENTAL
> + name = per_lcore__dpaa2_io
> + name = dpaa2_io_portal
>
> ; Explicit ignore for driver-only ABI
> [suppress_type]
> name = rte_cryptodev_ops
> + name = dpaa2_io_portal_t
So, I understand you want the tooling to ignore changes to the global
arrays dpaa2_io_portal and per_lcore__dpaa2_io, right?
If that is correct, then here are the entries you should add to the
libabigail.abignore file (please make sure the comments I have added
there is accurate):
[suppress_variable]
# This global variable is exported by the binary but is not part of
# the logical ABI. In a perfect world, that variable should not be
# global, and we should access it via an accessor function. We do
# that right now because of performance concerns.
name = dpaa2_io_portal
[suppress_variable]
# This global variable is exported by the binary but is not part of
# the logical ABI. In a perfect world, that variable should not be
# global, and we should access it via an accessor function. We do
# that right now because of performance concerns.
name = per_lcore__dpaa2_io
I hope this helps.
Cheers,
--
Dodji
Re: [dpdk-dev] [PATCH 00/16] NXP DPAAx fixes and enhancements
Hello Thomas, Hemant,
Thomas Monjalon writes:
> 07/04/2020 12:25, Hemant Agrawal:
[...]
>> [Hemant] I have commented on Neil's series.
>> It needs more changes in existing code.
>> An approach like __rte_experimental will work better.
>
> I guess you mean __rte_internal?
>
> Please Hemant don't wait for someone else filling the gap.
> If __rte_internal is the right approach, please complete and use it.
Just so that I understand, is __rte_internal an ELF version that the
symbols per_lcore_dpaa2_held_bufs, dpaa2_io_portal and
per_lcore__dpaa2_io should have in the binary?
If that is the case, then it seems to me that the __rte_internal
approach that you are suggesting would be a much better approach that
the one I replied to Hemant about below.
I didn't mean to tell Hemant what approach he should take :-) I was just
trying to help him get the syntax of a libabigail suppression
specification right.
Sorry for the confusion I might have induced.
Dodji Seketeli writes:
> Hello Hemant,
>
> Hemant Agrawal writes:
>
> [...]
>
>>> >> > > [Hemant]
>>> >> > > As per the logs:
>>> >> > >
>>> >> > > Variables changes summary: 1 Removed, 2 Changed, 0 Added
>>> >> > > variables
>>> >> > > 1 Removed variable:
>>> >> > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs'
>>> >> > {per_lcore_dpaa2_held_bufs@@DPDK_20.0}
>>> >> > > 2 Changed variables:
>>> >> > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at
>>> >> > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048
>>> >> > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at
>>> >> > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16
>>> >> > >
>>> >> > > Error: ABI issue reported for 'abidiff --suppr
>>> >> > > devtools/libabigail.abignore --
>>> >> > no-added-syms --headers-dir1 reference/usr/local/include
>>> >> > --headers-dir2 install/usr/local/include
>>> >> > reference/dump/librte_bus_fslmc.dump
>>> >> > install/dump/librte_bus_fslmc.dump'
>
> [...]
>
>>> In the mean time, the tooling can be tought to ignore changes to these ELF
>>> symbols, as you you guys all know already.
>>>
>> [Hemant] will you please help me about adding entry to libagigail.abignore
>> I tried doing following, but it is not helping
>> --- a/devtools/libabigail.abignore
>> +++ b/devtools/libabigail.abignore
>> @@ -2,10 +2,15 @@
>> symbol_version = EXPERIMENTAL
>> [suppress_variable]
>> symbol_version = EXPERIMENTAL
>> + name = per_lcore__dpaa2_io
>> + name = dpaa2_io_portal
>>
>> ; Explicit ignore for driver-only ABI
>> [suppress_type]
>> name = rte_cryptodev_ops
>> + name = dpaa2_io_portal_t
>
> So, I understand you want the tooling to ignore changes to the global
> arrays dpaa2_io_portal and per_lcore__dpaa2_io, right?
>
> If that is correct, then here are the entries you should add to the
> libabigail.abignore file (please make sure the comments I have added
> there is accurate):
>
> [suppress_variable]
> # This global variable is exported by the binary but is not part of
> # the logical ABI. In a perfect world, that variable should not be
> # global, and we should access it via an accessor function. We do
> # that right now because of performance concerns.
> name = dpaa2_io_portal
>
> [suppress_variable]
> # This global variable is exported by the binary but is not part of
> # the logical ABI. In a perfect world, that variable should not be
> # global, and we should access it via an accessor function. We do
> # that right now because of performance concerns.
> name = per_lcore__dpaa2_io
>
> I hope this helps.
>
> Cheers,
--
Dodji
Re: [dpdk-dev] [PATCH v2 4/4] add ABI checks
Hello,
Ferruh Yigit a écrit:
[...]
> +1 the change/shuffle of the existing values are problematic, but we don't
> have
> it in this case.
Right.
[...]
> The concern is when there are cases we can waive, we can't directly rely on
> the
> tool and automate it. These indicators good for improving the code, but not
> good
> to use it as build time checker.
Well, it depends. The tooling as it is today have the capability to
automatically "waive" some classes of A{B,P}I change reports that you
guys (the developers) deem harmless, in the context of your project.
For instance, in the precise case of interest here, one could define a
"suppression specification" to teach the ABI verifier that, for the enum
rte_crypto_asym_xform_type, the only enumerator which numerical value is
allowed to change is the one named RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END.
The content of the suppression specification file would look like:
[suppress_type]
# So, in practise, this rule is to allow adding enumerators
# only to the of the the rte_crypto_asym_xform_type enum,
# right before the RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
# enumerator which is meant to always be the last enumerator.
type_kind = enum
name = rte_crypto_asym_xform_type
changed_enumerators = RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
This way, you can hopefully reduce the surface of the changes you want
to see reported, tailored in a way that is specific to your project.
This should hopefully bring the system closer to a state that would
allow you guys to having something that is automated enough to have it
be triggered at build time.
And if there is some sensibly needed tweaking that the libabigail
tooling doesn't allow you guys to do right away, I'd be happy to hear
about it and try to add the functionality to the framework for you guys.
> Is there any way to reduce the failure only to definite ABI breakages?
I hope my comment above somewhat answers this question of yours. If it
does not, please tell me.
Cheers,
--
Dodji
Re: [dpdk-dev] [PATCH v2] ci: build and use libabigail 1.6
Hello,
David Marchand writes:
>> > +
>> > LIBABIGAIL_REPO=${LIBABIGAIL_REPO:-https://sourceware.org/git/libabigail.git}
>> > +LIBABIGAIL_VERSION=${LIBABIGAIL_VERSION:-libabigail-1.6}
>> > +
>> > +if [ "$(cat libabigail/VERSION 2>/dev/null)" != "$LIBABIGAIL_VERSION"
>> > ]; then
>> > +rm -rf libabigail
>> > +# if we change libabigail, invalidate existing abi cache
>> > +rm -rf reference
>> > +fi
>> > +
>> > +if [ ! -d libabigail ]; then
>> > +git clone --single-branch -b $LIBABIGAIL_VERSION $LIBABIGAIL_REPO
>> > libabigail/src
>>
>> Why not using the tarball?
>> http://mirrors.kernel.org/sourceware/libabigail/libabigail-1.6.tar.gz
>
> No good reason "now".
>
> I was first bitten by a reference to redhat-hardened-ld in some
> libtool script in the tarball (/me looks in Dodji direction).
> I then considered switching to different versions of libabigail by
> just setting the LIBABIGAIL_VERSION env variable from .travis.yml.
> I ended up with the current latest version which is also what is in
> Ubuntu latest releases.
I think either way (distro package, tarball and git) has some benefits
and drawbacks.
I think one benefit of /being able/ to use the code from git is for
cases where you guys need some new features (and we tend to continuously
add fixes/functionalities to the git repository) that is available in
git only for now. Hopefully, with time, you'll only need to use the
distro package.
FWIW, I like the fact that your setup lets you choose between the distro
package, the tarball or the git code. That's powerful.
Cheers,
--
Dodji
Re: [dpdk-dev] [PATCH 00/16] NXP DPAAx fixes and enhancements
Hello,
David Marchand writes:
> On Thu, Mar 5, 2020 at 10:19 AM Hemant Agrawal (OSS)
> wrote:
>> > On Thu, Mar 5, 2020 at 10:06 AM Hemant Agrawal (OSS)
>> > wrote:
>> > >
>> > > Hi David,
>> > > > On Mon, Mar 2, 2020 at 10:26 AM Hemant Agrawal
>> > > > wrote:
>> > > > >
>> > > > > This patch series add various patches for enhancing and fixing NXP
>> > > > > fslmc bus, dpaa bus, and dpaax.
>> > > > >
>> > > > > - the main change is support to allow thread migration across
>> > > > > lcores
>> > > > > - improving the multi-process support
>> > > >
>> > > > This series triggers an ABI warning that must be investigated.
>> > > >https://travis-ci.com/ovsrobot/dpdk/jobs/292904119#L2233
>> > >
>> > > [Hemant]
>> > > As per the logs:
>> > >
>> > > Variables changes summary: 1 Removed, 2 Changed, 0 Added variables
>> > > 1 Removed variable:
>> > > 'dpaa2_portal_dqrr per_lcore_dpaa2_held_bufs'
>> > {per_lcore_dpaa2_held_bufs@@DPDK_20.0}
>> > > 2 Changed variables:
>> > > [C]'dpaa2_io_portal_t dpaa2_io_portal[128]' was changed at
>> > dpaa2_hw_dpio.h:40:1: size of symbol changed from 5120 to 2048
>> > > [C]'dpaa2_io_portal_t per_lcore__dpaa2_io' was changed at
>> > > dpaa2_hw_dpio.h:20:1: size of symbol changed from 40 to 16
>> > >
>> > > Error: ABI issue reported for 'abidiff --suppr
>> > > devtools/libabigail.abignore --
>> > no-added-syms --headers-dir1 reference/usr/local/include --headers-dir2
>> > install/usr/local/include reference/dump/librte_bus_fslmc.dump
>> > install/dump/librte_bus_fslmc.dump'
>> > >
>> > > ---
>> > >
>> > > These changes are w.r.t modifications in internal structures and
>> > > variables.
>> > They may be ignored.
>> >
>> > The ABI check considers symbol exposed in headers available to final users.
>> > If those are internal, why are the headers public?
>> >
>>
>> [Hemant] These symbols are not part of any public header files, but
>> they are part of *.map files to share them between different driver
>> libs i.e bus_fslmc and net_dpaa2
>
> I would expect libabigail to skip those symbols, so there is something
> I have missed in how --headers-dirX work.
In libabigail speak, we make a difference between *ELF symbols* and
types.
--header-dirX is about telling the tool what the public *types* are. As
you rightfully implied, types that are defined in files that are not
found in the directories specified by --header-dirX are considered to be
private types and are thus not shown in the ABI change report.
ELF symbols however are a different matter. Header files don't usually
define ELF symbols, be they variable of function symbols. Header files
can at most declare variables or functions that would be actually
defined elsewhere in source code, leading to the definition of ELF
variable or function symbols in the final binary. At this point, we
aren't talking about types anymore, as the ELF format doesn't know what
types (in C or any other language) are. So --header-dirX don't deal with
ELF symbols.
And from what I understand from the message quoted above, the changes we
are talking about have to do with EFL variable symbols which size have
changed. So in practise, these are global arrays (exposed by at the
binary level as an ELF variable symbol of a given size) with public
visibility which size have changed.
So my guess would be that if you guys don't want these arrays to be part
the binary interface of this library, they should probably be declared
static at the C level and accessed through some accessor function or
something like that. At least, that's my humble uninformed opinion.
In the mean time, the tooling can be tought to ignore changes to these
ELF symbols, as you you guys all know already.
> Anyway, all of those symbols in dpaa are part of the driver ABI.
> We are still missing a way to mark internal symbols.
> Neil had posted a framework for this
> http://patchwork.dpdk.org/project/dpdk/list/?series=5004.
>
> In order to get this series passing the checks, I recommend NXP
> rebasing Neil scripts (I will help reviewing this part), then mark all
> those symbols as internal in its drivers.
> Other vendor will convert their drivers later, as there is no need at
> the moment.
>
> Thanks.
Cheers,
--
Dodji
Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri
David Marchand writes: > On Thu, Jan 7, 2021 at 2:33 PM Thomas Monjalon wrote: >> > Yes that may break the ABI but fortunately the checking-abi-compatibility >> > tool shows negative :) , thanks Ferruh' s guide. >> > https://github.com/ferruhy/dpdk/actions/runs/468859673 >> >> That's very strange. An enum value is changed. >> Why it is not flagged by libabigail? > > I suspect this is because the enum is not referenced in any object... > all I see is an integer with a comment that it should be filled with > values from the enum. I am not sure about the full context but David is right in theory. If the enum is not reachable from a publically exported interface (function or global variable) then it won't we considered as being part of the ABI and changes to that enum won't be reported. I am not sure if that is what is happening in this particular case, though. Cheers, -- Dodji
Re: [dpdk-dev] [PATCH v4 1/2] mbuf: use C11 atomic built-ins for refcnt operations
Hello,
David Marchand writes:
[...]
On Thu, Jul 16, 2020 at 6:16 AM Phil Yang wrote:
>> >
>> > v4 does not pass the checks (in both my env, and Travis).
>> > https://travis-ci.com/github/ovsrobot/dpdk/jobs/359393389#L2405
>>
>> I think we need an exception in 'libabigail.abignore' for this change.
>> Is that OK with you?
David Marchand writes:
[...]
> Testing the series with libabigail 1.7.0:
>
> Functions changes summary: 0 Removed, 1 Changed (6 filtered out), 0
> Added functions
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
[...]
> in pointed to type 'struct rte_mbuf_ext_shared_info' at
> rte_mbuf_core.h:679:1:
>type size hasn't changed
>1 data member change:
> data member rte_atomic16_t
> rte_mbuf_ext_shared_info::refcnt_atomic at offset 128 (in bits) became
> anonymous data member 'union {rte_atomic16_t refcnt_atomic; uint16_t refcnt;}'
[...]
> We will have no other update on mbuf for 20.08, so the following rule
> can do the job for 20.08 and we will remove it in 20.11.
>
> diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> index daa4631bf..b35f91257 100644
> --- a/devtools/libabigail.abignore
> +++ b/devtools/libabigail.abignore
> @@ -52,6 +52,10 @@
> [suppress_type]
> type_kind = struct
> name = rte_epoll_event
> +; Ignore updates of rte_mbuf_ext_shared_info
> +[suppress_type]
> +type_kind = struct
> +name = rte_mbuf_ext_shared_info
[...]
> Olivier, Dodji, Ray?
Yes, that should work.
Just for the sake of precision, I'd like to say that in the coming 1.8
version of libabigail, this change won't be reported by the tooling as a
problem anymore. This is thanks to David filing the feature request
https://sourceware.org/bugzilla/show_bug.cgi?id=25661 a while ago.
Until then, I understand that the current tooling needs to work with
libabigail 1.6.
So maybe a more specific suppression rule (that you could still remove
for the 20.11 stable branch) could look like:
[suppress_type]
name = rte_mbuf_ext_shared_info
has_data_member_inserted_between = {offset_of(refcnt_atomic),
offset_of(refcnt_atomic)}
It's a "hack" that will only suppress change reports on the
rte_mbuf_ext_shared_info type if:
1/ it it has a data member inserted at the
offset of its data member 'refcnt_atomic',
AND
2/ the size of rte_mbuf_ext_shared_info doesn't change.
There are cases where this won't work, though. But it might work for
this case. If it does, then great. I think it'd be a better solution
than a blanket suppression of all the changes on the type.
Cheers,
--
Dodji
Re: [PATCH] [RFC] cryptodev: replace LIST_END enumerators with APIs
Hello,
Ferruh Yigit writes:
> On 9/5/2024 11:14 AM, Akhil Goyal wrote:
>> Replace *_LIST_END enumerators from asymmetric crypto
>> lib to avoid ABI breakage for every new addition in
>> enums with inline APIs.
>>
>> Signed-off-by: Akhil Goyal
>> ---
>> This patch was discussed in ML long time back.
>> https://patches.dpdk.org/project/dpdk/patch/20211008204516.3497060-1-gak...@marvell.com/
>> Now added inline APIs for getting the list end which need to be updated
>> for each new entry to the enum. This shall help in avoiding ABI break
>> for adding new algo.
>>
>
> Hi Akhil,
>
> *I think* this hides the problem instead of fixing it, and this may be
> partially because of the tooling (libabigail) limitation.
So, I am not sure to understand how this would be a libabigail
limitation in general. Let me explain.
The abidiff tool that compares two binaries and emits reports about
their ABI changes can be given a suppression specification file which
instructs the tool about what type of change to avoid emitting reports
about.
It turns out there is a construct specifically designed to handle the
case where an enumerator is added right before the last enumerator of a
enum. Such a change causes the value of the last enumerator to
increase.
For instance, consider this enum:
enum foo_enum
{
FOO_FIRST,
FOO_SECOND,
FOO_END
};
If I add a new enumerator right before the last enumerator, I would get
this:
enum foo_enum
{
FOO_FIRST,
FOO_SECOND,
FOO_THIRD,
FOO_END
};
This change cause the value of the the FOOD_END enumerator to increase.
And that increase might be problematic. At the moment, for it being
problematic or not has to be the result of a careful review.
So, by default, abidiff will complain by saying that the value of
FOO_END was changed.
But you, as a community of practice, can decide that this kind of change
to the value of the last enumerator is not a problematic change, after
careful review of your code and practice. You thus can specify that
the tool using a suppression specification which has the following
syntax:
[suppress_type]
type_kind = enum
changed_enumerators = FOO_END, ANOTHER_ENUM_END, AND_ANOTHER_ENUM_END
or, alternatively, you can specify the enumerators you want to suppress
the changes for as a list of regular expressions:
[suppress_type]
type_kind = enum
changed_enumerators_regexp = .*_END$, .*_LAST$, .*_LIST_END$
Wouldn't that be enough to address your use case here (honest question)?
> This patch prevents the libabigail warning, true, but it doesn't improve
> anything from the application's perspective.
> Before or after this patch, application knows a fixed value as END value.
>
> Not all changes in the END (MAX) enum values cause ABI break, but tool
> warns on all, that is why I think this may be tooling limitation [1].
> (Just to stress, I am NOT talking about regular enum values change, I am
> talking about only END (MAX) value changes caused by appending new enum
> items.)
>
> As far as I can see (please Dodji, David correct me if I am wrong) ABI
> break only happens if application and library exchange enum values in
> the API (directly or within a struct).
Sorry, I am not sure to understand what you mean by "exchange enum values".
I would say is that if a change to an enumerator value causes a change
to the layout of a type which is part of the ABI, then that enumerator
value change might change the ABI. That ABI change might be problematic
(i.e an ABI break) or not.
>
> Exchanging enum values via API cause ABI issues because:
> 1. enum size is not fixed, it uses min storage size that can hold all
> values, adding new enums may cause its size change and of course this
> breaks ABI.
> 2. Library can send enum values that application doesn't know, this may
> cause application behave undefined.
> 3. Application sending MAX value (or more) to API expects error, but
> that may be a valid value in the new library and applications gets
> unexpected result.
>
>
> Let's assume above 1. happens, with this patch warning is prevented, but
> ABI break still can occur.
>
> One option can be not exchanging enums in APIs at all, this way changes
> in the END (MAX) enum values are harmless. But I can see cryptodev does
> this a lot.
>
> When enum is exchanged in APIs, and if we want to be strict about the
> ABI, safest option can be not appending to enums at all, and keeping END
> (MAX) items can be a way to enable warnings for this.
>
>
> More relaxed option is protect against only 1. since that is the real
> ABI issue, 2 & 3 are more defensive programming, so as long as enum size
> is not changed we may ignore the END (MAX) value change warnings.
>
>
>
> [1] It would be better if tool gives END (MAX) enum value warnings only
> if it is exchanged in an API, but not sure if this can be possible to
> detect.
I believe that if you want to know if an enumerator value is *USED* by a
type (w
Re: [PATCH] [RFC] cryptodev: replace LIST_END enumerators with APIs
Hello,
Ferruh Yigit writes:
[...]
>> This change cause the value of the the FOOD_END enumerator to increase.
>> And that increase might be problematic. At the moment, for it being
>> problematic or not has to be the result of a careful review.
>>
>
> As you said, FOOD_END value change can be sometimes problematic, but
> sometimes it is not.
> This what I referred as limitation that tool is not reporting only
> problematic case, but require human review.
Oh, I see. Thank you for clarifying.
> (btw, this is a very useful tool, I don't want to sound like negative
> about it, only want to address this recurring subject in dpdk.)
No problem, I never assume you mean anything negative :-)
[...]
>> So, by default, abidiff will complain by saying that the value of
>> FOO_END was changed.
>>
>> But you, as a community of practice, can decide that this kind of change
>> to the value of the last enumerator is not a problematic change, after
>> careful review of your code and practice. You thus can specify that
>> the tool using a suppression specification which has the following
>> syntax:
>>
>> [suppress_type]
>> type_kind = enum
>> changed_enumerators = FOO_END, ANOTHER_ENUM_END, AND_ANOTHER_ENUM_END
>>
>> or, alternatively, you can specify the enumerators you want to suppress
>> the changes for as a list of regular expressions:
>>
>> [suppress_type]
>> type_kind = enum
>> changed_enumerators_regexp = .*_END$, .*_LAST$, .*_LIST_END$
>>
>> Wouldn't that be enough to address your use case here (honest question)?
>>
>
> We are already using suppress feature in dpdk.
>
> But difficulty is to decide if END (MAX) enum value change warning is an
> actual ABI break or not.
>
> When tool gives warning, tendency is to just make warning go away,
> mostly by removing END (MAX) enum without really analyzing if it is a
> real ABI break.
I see.
[...]
>>> [1] It would be better if tool gives END (MAX) enum value warnings only
>>> if it is exchanged in an API, but not sure if this can be possible to
>>> detect.
>>
>> I believe that if you want to know if an enumerator value is *USED* by a
>> type (which I believe is at the root of what you are alluding to), then
>> you would need a static analysis tool that works at the source level.
>> Or, you need a human review of the code once the binary analysis tool
>> told you that that value of the enumerator changed.
>>
>> Why ? please let me give you an example:
>>
>> enum foo_enum
>> {
>> FOO_FIRST,
>> FOO_SECOND,
>> FOO_END
>> };
>>
>> int array[FOO_END];
>>
>> Once this is compiled into binary, what libabigail is going to see by
>> analyzing the binary is that 'array' is an array of 2 integers. The
>> information about the size of the array being initially an enumerator
>> value is lost. To detect that, you need source level analysis.
>>
>
> I see the problem.
>
> Is this the main reason that changing FOO_END value reported as warning?
Yes, it is because of this class of issues.
Actually if ANY enumerator value is changed, that is actually an ABI
change. And that ABI change is either compatible or not.
> If we forbid this kind of usage of the FOO_END, can we ignore this
> warning safely?
I would think so.
But then, you'd have to also forbid the use of all enumerators,
basically. I am not sure that would be practical.
Rather I would tend to lean toward reviewing the use of enumerators, on
a case by case basis, using tools like 'grep' and whatnot.
What I would advise to forbid is the use of complicated macros or
constructs that makes the review of the use of enumerators
non-practical. You should be able to grep "FOO_END" and see where it's
used in the source code. Reviewing that shouldn't take more than a few
minutes whenever a tool warns about the change of its value.
>
>> But then, by reviewing the code, this is a construct that you can spot
>> and allow or forbid, depending on your goals as a project.
>>
>> [...]
>>
>> Cheers,
>>
>
--
Dodji
Re: [EXTERNAL] Re: [PATCH] [RFC] cryptodev: replace LIST_END enumerators with APIs
Hello,
Akhil Goyal writes:
[...]
>> I believe that if you want to know if an enumerator value is *USED* by a
>> type (which I believe is at the root of what you are alluding to), then
>> you would need a static analysis tool that works at the source level.
>> Or, you need a human review of the code once the binary analysis tool
>> told you that that value of the enumerator changed.
>>
>> Why ? please let me give you an example:
>>
>> enum foo_enum
>> {
>> FOO_FIRST,
>> FOO_SECOND,
>> FOO_END
>> };
>>
>> int array[FOO_END];
>>
>> Once this is compiled into binary, what libabigail is going to see by
>> analyzing the binary is that 'array' is an array of 2 integers. The
>> information about the size of the array being initially an enumerator
>> value is lost. To detect that, you need source level analysis.
>>
>> But then, by reviewing the code, this is a construct that you can spot
>> and allow or forbid, depending on your goals as a project.
>>
> In the above example if in newer library a FOO_THIRD is added.
> FOO_END value will change and will cause ABI break for change in existing
> value.
> But if we replace it with inline function to get the list_end and use it in
> array like below.
> So if FOO_THIRD is added, we will also update foo_enum_list_end() function to
> return (FOO_THIRD+1)
>
> enum foo_enum
> {
> FOO_FIRST,
> FOO_SECOND,
> };
> static inline int foo_enum_list_end()
> {
> return FOO_SECOND + 1;
> }
> int array[foo_enum_list_end()];
>
> Will this cause an ABI break if we add this array in application or in
> library?
I think this (inline function) construct is essentially the same as just
adding a FOO_END enumerator after FOO_SECOND. Using either
foo_enum_list_end() or FOO_END result in having the value '2' in the
application using the library to get FOO_END or foo_enum_list_end().
Newer versions of the library being linked to the application won't
change that value '2', regardless of the newer values of FOO_END or
foo_enum_list_end().
So, adding a FOO_THIRD right after FOO_END, induces and ABI change.
This change being "breaking" (incompatible) or not, really depends on
what the application expects, I would say. Sorry if that looks "vague",
but this whole concept is quite blurry.
For instance, if you add FOO_THIRD after FOO_SECOND in the newer version
of the library and the application still gets the value '2' rather than
getting the value '3', and that value is actually multiplied by "two
trillions" in the application to get the value of the dividend to be
payed to investors, then, then that's a very big error induced by that
change. That might be considered by the application as a "breaking" ABI
change and you might get a call or two from the CEO of an S&P500 company
that uses the library.
Other applications might consider that "off-by-one" error not being
problematic at all and thus might consider it not "breaking".
Note that REMOVING an enumerator however is always considered an
incompatible (breaking) ABI change.
Adding an enumerator however has this annoying "grey topic" (not black or
white) property that I am not sure how to address at this point.
Cheers,
--
Dodji
Re: [EXTERNAL] Re: [PATCH] [RFC] cryptodev: replace LIST_END enumerators with APIs
Akhil Goyal writes:
>> >>> Now added inline APIs for getting the list end which need to be updated
>> >>> for each new entry to the enum. This shall help in avoiding ABI break
>> >>> for adding new algo.
>> >>>
>> >>
>> >> Hi Akhil,
>> >>
>> >> *I think* this hides the problem instead of fixing it, and this may be
>> >> partially because of the tooling (libabigail) limitation.
>> >>
>> >> This patch prevents the libabigail warning, true, but it doesn't improve
>> >> anything from the application's perspective.
>> >> Before or after this patch, application knows a fixed value as END value.
>> >>
>> >> Not all changes in the END (MAX) enum values cause ABI break, but tool
>> >> warns on all, that is why I think this may be tooling limitation [1].
>> >> (Just to stress, I am NOT talking about regular enum values change, I am
>> >> talking about only END (MAX) value changes caused by appending new enum
>> >> items.)
>> >>
>> >> As far as I can see (please Dodji, David correct me if I am wrong) ABI
>> >> break only happens if application and library exchange enum values in
>> >> the API (directly or within a struct).
>> >
>> > - There is also the following issue:
>> > A library publicly exports an array sized against a END (MAX) enum in the
>> > API.
>> > https://developers.redhat.com/blog/2019/05/06/how-c-array-sizes-become-part-of-the-binary-interface-of-a-library
>> >
>>
>> I see. And Dodji explained this requires source code to detect.
>>
>> I don't remember seeing a public array whose size is defined by an enum,
>> are you aware of any instance of this usage?
>
> https://patches.dpdk.org/project/dpdk/patch/20241009071151.1106-1-gmuthukri...@marvell.com/
> This was merged yesterday.
I guess the problematic piece of the code is this:
diff --git a/lib/cryptodev/rte_cryptodev.h
b/lib/cryptodev/rte_cryptodev.h
index bec947f6d5..aa6ef3a94d 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -185,6 +185,9 @@ struct rte_cryptodev_asymmetric_xform_capability {
* Value 0 means unavailable, and application should pass the
required
* random value. Otherwise, PMD would internally compute
the random number.
*/
+
+ uint32_t op_capa[RTE_CRYPTO_ASYM_OP_LIST_END];
+/**< Operation specific capabilities. */
};
Is it possible for the struct rte_cryptodev_asymmetric_xform_capability
to be made an opaque struct which definition would be present only in a
.c file of the library?
Its data members would then be retrieved by getter functions that take a
pointer to that struct in parameter.
That way, the uint32_t op_capa[RTE_CRYPTO_ASYM_OP_LIST_END] data member
would be "private" to the .c file and thus would not be part of the
ABI. Any change to the RTE_CRYPTO_ASYM_OP enum would then become
harmless to that struct.
I hope this helps.
--
Dodji
