Honnappa Nagarahalli wrote:
> On 23 May 2017 at 09: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?
> >
> 
> We have been running TM related test cases and they are working fine.
> So, I would assume we do not need to do anything.

The packet ordering question above is not specific to TM but is
relevant also when TM is not used. And a test case may not reveal
ordering problems unless you are specifically looking for it and
somehow making sure that the right order does not happen by accident.

Here is a simple use case for packet ordering at output: Application
gets incoming packets from single pktin through single ordered and
scheduled queue. The packets get spread to multiple threads by the
scheduler. The threads output the packets to the same pktout queue
used in queued mode. The output packets in the wire should be in the
same order as they were received, even if the worker threads processed
the packets at very different speeds.

> 
> What is the use case from TM? Does TM use ODP queues?

I am not familiar with the TM, but it looks like the scheduler may want to
ensure correct packet order using the order_lock function and therefore
that function being no-op in the scalable scheduler looked alarming to me.

        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.
> >
> >> +
> >> +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
> >
> >

Reply via email to