Re: [dpdk-dev] /validate-abi.sh complains [PATCH v1 3/8] ring: introduce RTS ring mode

[Dodji Seketeli <dseketel@redhat.com>]

Hello,

"Ananyev, Konstantin" <konstantin.ananyev@intel.com> 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