27/03/2020 15:36, Ray Kinsella:
> On 27/03/2020 14:32, Van Haaren, Harry wrote:
> > From: Thomas Monjalon <tho...@monjalon.net>
> >> 27/03/2020 14:44, Neil Horman:
> >>> On Fri, Mar 27, 2020 at 01:24:12PM +0100, David Marchand wrote:
> >>>> On Wed, Mar 25, 2020 at 12:11 PM Kevin Laatz <kevin.la...@intel.com>
> >> wrote:
> >>>>> --- a/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h
> >>>>> +++ b/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h
> >>>>> @@ -113,6 +113,24 @@ enum rte_cpu_flag_t {
> >>>>> /* (EAX 80000007h) EDX features */
> >>>>> RTE_CPUFLAG_INVTSC, /**< INVTSC */
> >>>>>
> >>>>> + RTE_CPUFLAG_AVX512DQ, /**< AVX512 Doubleword and
> >> Quadword */
> >>>>> + RTE_CPUFLAG_AVX512IFMA, /**< AVX512 Integer Fused
> >> Multiply-Add */
> >>>>> + RTE_CPUFLAG_AVX512CD, /**< AVX512 Conflict
> >> Detection*/
> >>>>> + RTE_CPUFLAG_AVX512BW, /**< AVX512 Byte and Word */
> >>>>> + RTE_CPUFLAG_AVX512VL, /**< AVX512 Vector Length */
> >>>>> + RTE_CPUFLAG_AVX512VBMI, /**< AVX512 Vector Bit
> >> Manipulation */
> >>>>> + RTE_CPUFLAG_AVX512VBMI2, /**< AVX512 Vector Bit
> >> Manipulation 2 */
> >>>>> + RTE_CPUFLAG_GFNI, /**< Galois Field New
> >> Instructions */
> >>>>> + RTE_CPUFLAG_VAES, /**< Vector AES */
> >>>>> + RTE_CPUFLAG_VPCLMULQDQ, /**< Vector Carry-less
> >> Multiply */
> >>>>> + RTE_CPUFLAG_AVX512VNNI, /**< AVX512 Vector Neural
> >> Network Instructions */
> >>>>> + RTE_CPUFLAG_AVX512BITALG, /**< AVX512 Bit Algorithms
> >> */
> >>>>> + RTE_CPUFLAG_AVX512VPOPCNTDQ, /**< AVX512 Vector Popcount
> >> */
> >>>>> + RTE_CPUFLAG_CLDEMOTE, /**< Cache Line Demote */
> >>>>> + RTE_CPUFLAG_MOVDIRI, /**< Direct Store
> >> Instructions */
> >>>>> + RTE_CPUFLAG_MOVDIR64B, /**< Direct Store
> >> Instructions 64B */
> >>>>> + RTE_CPUFLAG_AVX512VP2INTERSECT, /**< AVX512 Two Register
> >> Intersection */
> >>>>> +
> >>>>> /* The last item */
> >>>>> RTE_CPUFLAG_NUMFLAGS, /**< This should always be
> >> the last! */
> >>>>
> >>>> This is seen as an ABI break because of the change on _NUMFLAGS:
> >>>> https://travis-ci.com/github/ovsrobot/dpdk/jobs/302524264#L2351
> >>>>
> >>> It shouldn't be, as the only API calls we expose that use rte_cpu_flag_t
> >> accept
> >>> it as an integer parameter to see if the flag is enabled. Theres no use
> >>> of
> >> the
> >>> enum in a public array or any struct that is sized based on the number of
> >> flags,
> >>> so you should be good to go
> >>
> >> Indeed I cannot imagine an ABI incompatibility in this case.
> >> The only behaviour change is to accept new (higher) RTE_CPUFLAG values
> >> in functions rte_cpu_get_flag_enabled() and rte_cpu_get_flag_name().
> >> Is changing the range of valid values an ABI break?
> >> Why is it flagged by libabigail?
> >
> > If this enum _MAX value was used by the application to allocate an array,
> > that later our DPDK code would write to it could cause out-of-bounds array
> > accesses of the application supplied array. Abigail doesn't know what
> > applications could use the value for, so it flags it.
> >
> > IMO Abigail is right to flag it to us - a manual review to understand what
> > that _MAX enum value is used for, and then decide on a case by case basis
> > seems the best way forward to me.
> >
> > Thanks Neil/Thomas for reviewing, as reply in this thread, I also believe
> > this is not going to break ABI.
> >
>
> +1 to Harrry's comments.
> I can't immediately see how this might break the ABI either.
So we all agree that increasing the range of valid rte_cpu_flag_t values
is OK for ABI compatibility.
This conclusion must be written as a libabigail exception in this patch.
