Re: [dpdk-dev] [EXT] Re: [RFC 11/15] eventdev: reserve fields in timer object

[Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>]

>On Tue, 24 Aug 2021 01:10:15 +0530
><pbhagavatula@marvell.com> wrote:
>
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>
>> Reserve fields in rte_event_timer data structure to address future
>> use cases.
>> Also, remove volatile from rte_event_timer.
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
>Reserve fields are not a good idea. They don't solve future API/ABI
>problems.
>
>The issue is that you need to zero them and check they are zero
>otherwise
>they can't safely be used later.  This happened with the Linux kernel
>system calls where in several cases a flag field was added for future.
>The problem is that old programs would work with any garbage in the
>flag
>field, and therefore the flag could not be extended.

The change is in rte_event_timer which is a fastpath object similar to 
rte_mbuf.
I think fast path objects don't have the above mentioned caveat.

>
>A better way to make structures internal opaque objects that
>can be resized.  Why is rte_event_timer_adapter exposed in API?

rte_event_timer_adapter has similar API semantics of  rte_mempool, 
the objects of the adapter are rte_event_timer objects which have
information of the timer state.

Erik,

I think we should move the fields in current rte_event_timer structure
around, currently we have

struct rte_event_timer {
	struct rte_event ev;
	volatile enum rte_event_timer_state state;
              x-------x 4 byte hole x---------x
	uint64_t timeout_ticks;
	uint64_t impl_opaque[2];
	uint8_t user_meta[0];
} __rte_cache_aligned;

Move to

struct rte_event_timer {
	struct rte_event ev;
	uint64_t timeout_ticks;
	uint64_t impl_opaque[2];
	uint64_t rsvd;
	enum rte_event_timer_state state;
	uint8_t user_meta[0];
} __rte_cache_aligned;

Since its cache aligned, the size doesn't change.

Thoughts?

Thanks,
Pavan.