On Wed, Oct 18, 2017 at 09:48:04PM +0000, Carrillo, Erik G wrote: > > > > -----Original Message----- > > From: Pavan Nikhilesh Bhagavatula > > [mailto:pbhagavat...@caviumnetworks.com] > > Sent: Monday, October 16, 2017 7:37 AM > > To: Carrillo, Erik G <erik.g.carri...@intel.com> > > Cc: dev@dpdk.org > > Subject: Re: [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter > > > > On Mon, Oct 16, 2017 at 05:34:36PM +0530, Pavan Nikhilesh Bhagavatula > > wrote: > > > On Mon, Oct 09, 2017 at 08:30:35PM +0000, Carrillo, Erik G wrote: > > > > > > > > > > > > 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 see, makes sense. > > > > > > > > > > > > > > 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 > > > > > > > > Just to check, are you talking about the heap (e.g., rte_malloc) in > > hugepage memory? > > > > > > Yes, If we use rte_memzone_reserve and create the adapter in the > > > primary process, the name can be used to do a lookup in the secondary. > > > > > > > > > - 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. > > > > > > > > > > > > > Can you elaborate here? In the mempool implementation, it looks like > > there's a per-process array of ops objects, and each mempool instance has > > an index into the array that it uses to select the ops object to call > > through. > > Since the ops structures are initialized per-process, they get function > > pointers valid in that process - which relates to the second point above. > > If the > > adapter holds function pointers for a primary process (for example), they'll > > be invalid in the secondary. Am I missing something? > > > > > > > > > > Once adapter is created using memzone in the Primary process, the > > > secondary process can use the memzone name to get the pointer to the > > > adapter and use data such as ops_id stored in the memzone to fixup the > > fastpath function pointers. > > > The slowpath could still use ops_id based function pointers(mempool). > > > > > > > Thanks, > > > > Gabriel > > > > > > > > > So please analyze on those lines as well. > > > > > > > > > > > > > > > > > > > > Pavan > > > > Adding to this > > > > Have a two level scheme such as: > > > > --Allocate this structure from a global array based on the adapter ID: > > > > struct rte_event_timer_wheel { > > event_timer_arm_burst arm_burst; > > /**< Pointer to driver arm_burst function. */ > > event_timer_arm_tmo_tick_burst arm_tmo_tick_burst; > > /**< Pointer to driver arm_tmo_tick_burst function. */ > > event_timer_cancel_burst cancel_burst; > > /**< Pointer to driver cancel function. */ > > > > --Allocate the data section using memzone reserve. > > struct rte_event_timer_wheel_data *data; > > > > > > /**< Pointer to timer wheel data*/ > > struct rte_event_timer_wheel_ops *ops; > > /**< Driver specific ops */ > > } __rte_cache_aligned; > > > > /** > > * Timer wheel data structure. > > */ > > struct rte_event_timer_wheel_data { > > void *wheel_priv; > > /**< Timer wheel private data*/ > > uint8_t ops_index; > > /**< Timer wheel ops index id. */ > > uint8_t wheel_id; > > /**< Timer wheel id. */ > > uint8_t event_port_id; > > /**< Optional Event port used when the inbuilt port is absent.*/ > > const struct rte_memzone *mz; > > /**< Timer wheel memzone pointer. */ > > struct rte_event_timer_wheel_config conf; > > /**< Configuration used to configure the wheel. */ > > > > RTE_STD_C11 > > uint8_t started : 1; > > /**< Flag to indicate wheel started. */ } __rte_cache_aligned; > > > > /* Primary process */ > > *_create(wheel_id, ...) > > { > > struct rte_event_timer_wheel *wl; > > > > /* do checks */ > > ... > > > > wl = &rte_event_timer_wheels[wheel_id]; > > > > ... > > /* Get ops ID based on eventdev */ > > ... > > ret = rte_event_dev_get_timer_wheel_capability(event_dev_id, > > &flags, > > &ops_id); > > > > mz = rte_memzone_reserve(name, ...); > > ... > > /* fill in data */ > > data = mz->addr; > > ... > > > > data = ops_id; > > ... > > /* Call the driver specific functions. */ } > > > > /*Secondaty process*/ > > struct rte_event_timer_wheel * > > rte_event_timer_wheel_lookup(uint16_t wheel_id) { > > ... > > struct rte_event_timer_wheel *wl; > > struct rte_event_timer_wheel_data *data; > > ... > > > > /* Do checks*/ > > ... > > /* Look up memzone created in primary. */ > > mz = rte_memzone_lookup(name); > > > > data = mz->addr; > > > > /* Setup up global array in secondary */ > > wl = &rte_event_timer_wheels[data->wheel_id]; > > wl->data = data; > > ... > > /* Use the ops_id set in primary to get the ops in secondary*/ > > wl->ops = rte_event_timer_wheel_get_ops(data->ops_index); > > > > /* Reset the ops*/ > > (*wl->ops->set_ops)(wl); > > > > return wl; > > } > > > > Hope this clears few things up. > > Hi Pavan, > > OK, I think I see what you mean. We could have a global table of ops structs > (one per plugin), where each entry contained function pointers valid in that > process. We could save the ops_index in the shared data portion of the timer > wheel, and in create() or lookup(), we could use that index to retrieve the > ops struct, which could contain an operation to set the fastpath pointers for > the timer wheel instance. I assume the *lookup() function would rely on the > memzone having a predictable name (e.g. "timer_wheel_memzone_<id>") so that > it could locate it knowing only the wheel_id.
Ack > > Thanks for the info, > Gabriel Cheers, Pavan >
