Re: [EXT] Re: [PATCH v4 07/10] ethdev: add IPsec SA expiry event subtypes

[Ray Kinsella <mdr@ashroe.eu>]

Akhil Goyal <gakhil@marvell.com> writes:

>> > Time expiry means after x seconds SA will expire.
>> > Packet expiry means after x packets processing SA will expire.
>> > Byte expiry means after x bytes of packet processing SA will expire.
>> 
>> I think you should use the syntax @ref packets_soft_limit
>> so it is clear where the event come from.
>
> OK will update the comments.
>
>> 
>> 
>> > > > +	RTE_ETH_EVENT_IPSEC_SA_PKT_EXPIRY,
>> > > > +	/** Hard byte expiry of SA */
>> > > > +	RTE_ETH_EVENT_IPSEC_SA_BYTE_HARD_EXPIRY,
>> > > > +	/** Hard packet expiry of SA */
>> > > > +	RTE_ETH_EVENT_IPSEC_SA_PKT_HARD_EXPIRY,
>> > >
>> > > Same comment for the 3 events.
>> > >
>> > > >  	/** Max value of this enum */
>> > > >  	RTE_ETH_EVENT_IPSEC_MAX
>> > > >  };
>> > >
>> > > What is the impact of this "MAX" value on ABI compatibility?
>> >
>> > I see no issues reported while running ABI check.
>> > There is no array being used inside library based on MAX.
>> 
>> No need of array inside the library, the events are exposed to the app.
>> I'm surprised libabigail is OK with changing an enum value.
>> 
> @Ray Can you please check if it is an issue to add more values in this enum?

Well look there is two seperate things going on here.

Why isn't libabigail complaining about the _MAX value changing. I'll
need to look at libabigail to see what the issue is, so lets put this
one aside for a moment. 

This second issue is it safe for the _MAX value to change?
We have a lot of back and forth argument on these, and previously
concluded that we should probably look to remove _MAX values in the
22.11 release.

The core issue is that we need look at how a user is likely to use
rte_eth_event_ipsec_subtype. Take a look at the example below:-

/root/src/dpdk/examples/ipsec-secgw/ipsec-secgw.c:2592:0

For instance, can we guarantee that an application built against DPDK
21.11, but running against 22.xx will never recieve one of the new
values in event_desc->subtype (or by any other means)?

-- 
Regards, Ray K