Hi Pavan,
> -----Original Message-----
> From: Bhagavatula, Pavan [mailto:pavan.bhagavat...@cavium.com]
> Sent: Friday, March 30, 2018 10:49 AM
> To: Carrillo, Erik G <erik.g.carri...@intel.com>; Jacob, Jerin
> <jerin.jacobkollanukka...@cavium.com>; nipun.gu...@nxp.com;
> hemant.agra...@nxp.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v8 7/9] test: add event timer adapter auto-test
>
> Hi Erik,
>
> Few comments below,
>
<...snipped...>
> > +/* Test that adapter stops correctly. */ static int
> > +adapter_stop(void)
> > +{
> > + uint32_t evdev_service_id, adapter_service_id;
> > + struct rte_event_timer_adapter *l_adapter = NULL;
> > +
> Please use INTERNAL_PORT capability to determine if service core is
> required.
>
Good catch. I meant to remove the references to service ids here, so I've gone
ahead and done that.
<...snipped...>
> > +stat_inc_reset_ev_enq(void)
> > +{
> > + int ret, i, n;
> > + int num_evtims = MAX_TIMERS;
> > + struct rte_event_timer *evtims[num_evtims];
> > + struct rte_event evs[BATCH_SIZE];
> > + struct rte_event_timer_adapter_stats stats;
> > + const struct rte_event_timer init_tim = {
> > + .ev.op = RTE_EVENT_OP_NEW,
> > + .ev.queue_id = TEST_QUEUE_ID,
> > + .ev.sched_type = RTE_SCHED_TYPE_ATOMIC,
> > + .ev.priority = RTE_EVENT_DEV_PRIORITY_NORMAL,
> > + .ev.event_type = RTE_EVENT_TYPE_TIMER,
> > + .state = RTE_EVENT_TIMER_NOT_ARMED,
> > + .timeout_ticks = 5, // expire in .5 sec
> > + };
> > +
> > + ret = rte_mempool_get_bulk(eventdev_test_mempool, (void
> **)evtims,
> > + num_evtims);
> > + TEST_ASSERT_EQUAL(ret, 0, "Failed to get array of timer objs: ret =
> %d",
> > + ret);
> > +
> > + for (i = 0; i < num_evtims; i++) {
> > + *evtims[i] = init_tim;
> > + evtims[i]->ev.event_ptr = evtims[i];
> > + }
> > +
> > + ret = rte_event_timer_adapter_stats_get(timdev, &stats);
> > + TEST_ASSERT_EQUAL(ret, 0, "Failed to get stats");
> > + TEST_ASSERT_EQUAL((int)stats.ev_enq_count, 0, "Stats not clear at "
> > + "startup");
> > +
> > + /* Test with the max value for the adapter */
> > + ret = rte_event_timer_arm_burst(timdev, evtims, num_evtims);
> > + TEST_ASSERT_EQUAL(ret, num_evtims,
> > + "Failed to arm all event timers: attempted = %d, "
> > + "succeeded = %d, rte_errno = %s",
> > + num_evtims, ret, rte_strerror(rte_errno));
> > +
> > + rte_delay_ms(1000);
> > +
> > +#define MAX_TRIES 1000
>
> Please make MAX_TRIES equivalent to num_evtims, here we are trying to
> deq 10 events in burst and assume it to succeed but in case event dev
> doesn't support burst mode event will be stuck as it will have only 1000 tries
> instead of 4096.
Good catch again - I'll make these changes.
<...snipped...>
> > +static int
> > +adapter_create_max(void)
> > +{
> > + int i;
> > + uint32_t svc_start_count, svc_end_count;
> > + struct rte_event_timer_adapter *adapters[
> > +
> RTE_EVENT_TIMER_ADAPTER_NUM_MAX + 1];
> > +
> > + struct rte_event_timer_adapter_conf conf = {
> > + .event_dev_id = evdev,
> > + // timer_adapter_id set in loop
> > + .clk_src = RTE_EVENT_TIMER_ADAPTER_CPU_CLK,
> > + .timer_tick_ns = NSECPERSEC / 10,
> > + .max_tmo_ns = 180 * NSECPERSEC,
> > + .nb_timers = MAX_TIMERS,
> > + .flags = 0,
> > + };
> > +
> > + svc_start_count = rte_service_get_count();
> > +
> > + /* This test expects that there are sufficient service IDs available
> > + * to be allocated. I.e., RTE_EVENT_TIMER_ADAPTER_NUM_MAX
> may need to
> > + * be less than RTE_SERVICE_NUM_MAX if anything else uses a
> service
> > + * (the SW event device, for example).
> > + */
> Same as above service use need to be dependent on INTERNAL_PORT
> capability.
>
> Also, in software event dev case this can be a valid test but in case of a hw
> event dev RTE_EVENT_TIMER_ADAPTER_NUM_MAX number of event
> devicesmight not be binded to dpdk. Either we need to provide a API to
> check how many event devices are supported or use
> eventdev_timer_adapter_caps_get_t to get the number.
>
> Thoughts?
>
For now, I've modified the test case so that it returns ENOTSUP for the hw case.
But I like the idea of adding an API to query the count of devices.
Thanks,
Gabriel
