Re: [dpdk-dev] [EXT] Re: [RFC 11/15] eventdev: reserve fields in timer object
From: Pavan Nikhilesh Bhagavatula <hidden>
Date: 2021-09-01 06:48:34
On Tue, 24 Aug 2021 01:10:15 +0530 [off-list ref] wrote:quoted
From: Pavan Nikhilesh <redacted> 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 <redacted>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.