On 31/05/2017, 10:38, "Peltonen, Janne (Nokia - FI/Espoo)" <[email protected]> wrote:
> > >Ola Liljedahl wrote: >> On 23/05/2017, 16:49, "Peltonen, Janne (Nokia - FI/Espoo)" >> <[email protected]> wrote: >> >> >> > >> >> +static int ord_enq_multi(uint32_t queue_index, void *p_buf_hdr[], >> >> + int num, int *ret) >> >> +{ >> >> + (void)queue_index; >> >> + (void)p_buf_hdr; >> >> + (void)num; >> >> + (void)ret; >> >> + return 0; >> >> +} >> > >> >How is packet order maintained when enqueuing packets read from an >>ordered >> >queue to a pktout queue? Matias' recent fix uses the ord_enq_multi >> >scheduler >> >function for that, but this version does not do any ordering. Or is the >> >ordering guaranteed by some other means? >> The scalable scheduler standard queue enqueue function also handles >> ordered queues. odp_queue_scalable.c can refer to the same >>thread-specific >> data as odp_schedule_scalable.c so we don¹t need this internal >>interface. >> We could perhaps adapt the code to use this interface but I think this >> interface is just an artefact of the implementation of the default >> queues/scheduler. > >The problem is that odp_pktout_queue_config() sets qentry->s.enqueue >to pktout_enqueue() and that does not have any of the scalable scheduler >specific magic that odp_queue_scalable.c:queue_enq{_multi}() has. So >ordering does not happen for pktout queues even if it works for other >queues, right? This must be a recent change, it doesn’t look like that in the working branch we are using. I see the code when changing to the master branch. The code in pktout_enqueue() does look like a hack: if (sched_fn->ord_enq_multi(qentry->s.index, (void **)buf_hdr, len, &nbr)) A cast to “void **”??? What’s the purpose of calling ord_enq_multi() here? To save (stash) packets if the thread is out-of-order? And when the thread is in-order, it is re-enqueueing the packets which again will invoke pktout_enqueue/pktout_enq_multi but this time ord_enq_multi() will not save the packets, instead they will actually be transmitted by odp_pktout_send()? > > Janne > >> >> > >> >> +static void order_lock(void) >> >> +{ >> >> +} >> >> + >> >> +static void order_unlock(void) >> >> +{ >> >> +} >> > >> >Is it ok that these are no-ops? tm_enqueue() seems to use these. >> No these ought to be implemented. We have fixed that now. Thanks. >> >> >> -- Ola >> >> Ola Liljedahl, Networking System Architect, ARM >> Phone: +46 706 866 373 Skype: ola.liljedahl >> >> >> >> >> > >> >> + >> >> +const schedule_fn_t schedule_scalable_fn = { >> >> + .pktio_start = pktio_start, >> >> + .thr_add = thr_add, >> >> + .thr_rem = thr_rem, >> >> + .num_grps = num_grps, >> >> + .init_queue = init_queue, >> >> + .destroy_queue = destroy_queue, >> >> + .sched_queue = sched_queue, >> >> + .ord_enq_multi = ord_enq_multi, >> >> + .init_global = schedule_init_global, >> >> + .term_global = schedule_term_global, >> >> + .init_local = schedule_init_local, >> >> + .term_local = schedule_term_local, >> >> + .order_lock = order_lock, >> >> + .order_unlock = order_unlock, >> >> +}; >> > >> > Janne >> > >> > >
