Re: [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Fri, 22 Sep 2017 10:17:10 -0500
> From: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> To: jerin.jacob@caviumnetworks.com
> CC: dev@dpdk.org, bruce.richardson@intel.com, harry.van.haaren@intel.com,
>  hemant.agrawal@nxp.com, gage.ads@intel.com, nipun.gupta@nxp.com,
>  narender.vangati@intel.com, nikhil.rao@intel.com,
>  pbhagavatula@caviumnetworks.com, rsanford@akamai.com
> Subject: [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter 
> X-Mailer: git-send-email 1.7.10
> 
> Hi Jerin, et al.,
> 
> The following RFC patch contains enough of a skeleton implementation
> for an event timer adapter to show how the API could call through an ops
> structure to a plugin backend to provide different implementations.
> 
> In terms of differences from the original RFC, the most obvious one is
> that I've replaced occurrences of "wheel" in the API with "adapter",
> partly to reflect the similarity with the event_eth_rx_adapter, and
> partly because "wheel" suggests an implementation that may not be
> the one used (as in the case of the SW impl).

OK. Makes sense. We thought timer wheel is generic concept. Anyway
the name change is fine.

> 
> The second big change is to replace API parameters specifying pointer
> to rte_event_timer_adapter with ids, which seems more common throughout
> DPDK.
> 
> Other changes include:
>  - removed rte_event_timer_adapter_lookup() function, since APIs no longer
>    accept pointer to rte_event_timer_adapter

There is one difference between ethdev rx adapter, where we have
rte_event_timer_arm_burst(), rte_event_timer_arm_tmo_tick_burst(),
rte_event_timer_cancel_burst(),
APIs in fastpath. So any multi process scheme where resolving the
fastpath API functions in _one_ or zero redirection is fine.

I guess in we may need 2 or 3 indirection to resolve the fastpath functions
with id scheme. Please choose scheme with one 1 or no redirection.
I think,
- By allocating adapter memory from the heap and
- adapter hold the function pointers for multi process and
- mempool kind of pointer object scheme without id and lookup()
Can resolve function pointers without any indirection.

So please analyze on those lines as well.


>  - Replaced RTE_EVENT_TIMER_SUCCESS_{ARM,CANCEL} states with
>    RTE_EVENT_TIMER_ARMED, since the purpose of the SUCCESS_CANCEL state was
>    unclear

ARM- to denote it has been armed
CANCEL to denote it has been canceled, ie. on rte_event_timer_cancel_burst()
function it can update the state as CANCELED. So that application can
know the exact status of timer event.

> 
> Please have a look and let me know what you think.

Looks like you have started with implementation without commenting on
initial RFC, so I think, you can continue with implementation of common code.
We will contribute on review and adding HW drivers.

Jerin

> 
> Thanks,
> Gabriel
> 
> 
> Erik Gabriel Carrillo (1):
>   eventtimer: introduce event timer adapter