| Message ID | 1425399570-908-1-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | Accepted |
| Commit | 844231b2dadb474b242c5b029e0620622c84e2ed |
| Headers | show |
On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > Use ODP timer facility instead of POSIX sleep/nanosleep. > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > (This document/code contribution attached is provided under the terms of > agreement LES-LTM-21309) > > example/generator/odp_generator.c | 105 > +++++++++++++++++++++++++++++++++++--- > 1 file changed, 98 insertions(+), 7 deletions(-) > > diff --git a/example/generator/odp_generator.c > b/example/generator/odp_generator.c > index 3870fd1..9147c15 100644 > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -79,6 +79,10 @@ static struct { > typedef struct { > char *pktio_dev; /**< Interface name to use */ > odp_pool_t pool; /**< Pool for packet IO */ > + odp_timer_pool_t tp; /**< Timer pool handle */ > + odp_queue_t tq; /**< Queue for timeouts */ > + odp_timer_t tim; /**< Timer handle */ > + odp_timeout_t tmo_ev; /**< Timeout event */ > int mode; /**< Thread mode */ > } thread_args_t; > > @@ -104,6 +108,26 @@ static int scan_mac(char *in, odph_ethaddr_t *des); > static void tv_sub(struct timeval *recvtime, struct timeval *sendtime); > > /** > + * Sleep for the specified amount of milliseconds > + * Use ODP timer, busy wait until timer expired and timeout event received > + */ > +static void millisleep(uint32_t ms, > + odp_timer_pool_t tp, > + odp_timer_t tim, > + odp_queue_t q, > + odp_timeout_t tmo) > +{ > + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); > + odp_event_t ev = odp_timeout_to_event(tmo); > + int rc = odp_timer_set_rel(tim, ticks, &ev); > + if (rc != ODP_TIMER_SUCCESS) > + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); > + /* Spin waiting for timeout event */ > + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) > + (void)0; > +} > + > +/** > * Scan ip > * Parse ip address. > * > @@ -400,11 +424,12 @@ static void *gen_send_thread(void *arg) > thr, > odp_atomic_load_u64(&counters.seq), > odp_atomic_load_u64(&counters.seq)%0xffff); > - /* TODO use odp timer */ > - struct timespec ts; > - ts.tv_sec = 0; > - ts.tv_nsec = args->appl.interval; > - nanosleep(&ts, NULL); > + millisleep(args->appl.interval, > + thr_args->tp, > + thr_args->tim, > + thr_args->tq, > + thr_args->tmo_ev); > + > } > if (args->appl.number != -1 && > odp_atomic_load_u64(&counters.seq) > @@ -419,8 +444,11 @@ static void *gen_send_thread(void *arg) > if (odp_atomic_load_u64(&counters.icmp) >= > (unsigned int)args->appl.number) > break; > - /* TODO use odp timer */ > - sleep(1); > + millisleep(1000, > + thr_args->tp, > + thr_args->tim, > + thr_args->tq, > + thr_args->tmo_ev); > args->appl.timeout--; > } > } > @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) > odp_cpumask_t cpumask; > char cpumaskstr[ODP_CPUMASK_STR_SIZE]; > odp_pool_param_t params; > + odp_timer_pool_param_t tparams; > + odp_timer_pool_t tp; > + odp_pool_t tmop; > > /* Init ODP before calling anything else */ > if (odp_init_global(NULL, NULL)) { > @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) > } > odp_pool_print(pool); > > + /* Create timer pool */ > + tparams.res_ns = 1 * ODP_TIME_MSEC; > + tparams.min_tmo = 0; > + tparams.max_tmo = 10000 * ODP_TIME_SEC; > + tparams.num_timers = num_workers; /* One timer per worker */ > + tparams.priv = 0; /* Shared */ > + tparams.clk_src = ODP_CLOCK_CPU; > + tp = odp_timer_pool_create("timer_pool", &tparams); > + if (tp == ODP_TIMER_POOL_INVALID) { > + EXAMPLE_ERR("Timer pool create failed.\n"); > + exit(EXIT_FAILURE); > + } > + odp_timer_pool_start(); > + > + /* Create timeout pool */ > + memset(¶ms, 0, sizeof(params)); > + params.tmo.num = tparams.num_timers; /* One timeout per timer > */ > + params.type = ODP_POOL_TIMEOUT; > + > + tmop = odp_pool_create("timeout_pool", ODP_SHM_NULL, ¶ms); > + > + if (pool == ODP_POOL_INVALID) { > + EXAMPLE_ERR("Error: packet pool create failed.\n"); > + exit(EXIT_FAILURE); > + } > for (i = 0; i < args->appl.if_count; ++i) > create_pktio(args->appl.if_names[i], pool); > > @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) > > if (args->appl.mode == APPL_MODE_PING) { > odp_cpumask_t cpu0_mask; > + odp_queue_t tq; > > /* Previous code forced both threads to CPU 0 */ > odp_cpumask_zero(&cpu0_mask); > odp_cpumask_set(&cpu0_mask, 0); > > + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); > + if (tq == ODP_QUEUE_INVALID) > + abort(); > args->thread[1].pktio_dev = args->appl.if_names[0]; > args->thread[1].pool = pool; > + args->thread[1].tp = tp; > + args->thread[1].tq = tq; > + args->thread[1].tim = odp_timer_alloc(tp, tq, NULL); > + if (args->thread[1].tim == ODP_TIMER_INVALID) > + abort(); > + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); > + if (args->thread[1].tmo_ev == ODP_TIMEOUT_INVALID) > + abort(); > args->thread[1].mode = args->appl.mode; > odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, > gen_recv_thread, > &args->thread[1]); > > + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); > + if (tq == ODP_QUEUE_INVALID) > + abort(); > args->thread[0].pktio_dev = args->appl.if_names[0]; > args->thread[0].pool = pool; > + args->thread[0].tp = tp; > + args->thread[0].tq = tq; > + args->thread[0].tim = odp_timer_alloc(tp, tq, NULL); > + if (args->thread[0].tim == ODP_TIMER_INVALID) > + abort(); > + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); > + if (args->thread[0].tmo_ev == ODP_TIMEOUT_INVALID) > + abort(); > args->thread[0].mode = args->appl.mode; > odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, > gen_send_thread, > &args->thread[0]); > @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) > odp_cpumask_t thd_mask; > void *(*thr_run_func) (void *); > int if_idx; > + odp_queue_t tq; > > if_idx = i % args->appl.if_count; > > args->thread[i].pktio_dev = > args->appl.if_names[if_idx]; > + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, > NULL); > + if (tq == ODP_QUEUE_INVALID) > + abort(); > args->thread[i].pool = pool; > + args->thread[i].tp = tp; > + args->thread[i].tq = tq; > + args->thread[i].tim = odp_timer_alloc(tp, tq, > NULL); > + if (args->thread[i].tim == ODP_TIMER_INVALID) > + abort(); > + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); > + if (args->thread[i].tmo_ev == ODP_TIMEOUT_INVALID) > + abort(); > args->thread[i].mode = args->appl.mode; > > if (args->appl.mode == APPL_MODE_UDP) { > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
It could be good to add the bug tracker ID or link to the commit message when merging. https://bugs.linaro.org/show_bug.cgi?id=1025 On 4 March 2015 at 04:54, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: >> >> Use ODP timer facility instead of POSIX sleep/nanosleep. >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > >> >> --- >> (This document/code contribution attached is provided under the terms of >> agreement LES-LTM-21309) >> >> example/generator/odp_generator.c | 105 >> +++++++++++++++++++++++++++++++++++--- >> 1 file changed, 98 insertions(+), 7 deletions(-) >> >> diff --git a/example/generator/odp_generator.c >> b/example/generator/odp_generator.c >> index 3870fd1..9147c15 100644 >> --- a/example/generator/odp_generator.c >> +++ b/example/generator/odp_generator.c >> @@ -79,6 +79,10 @@ static struct { >> typedef struct { >> char *pktio_dev; /**< Interface name to use */ >> odp_pool_t pool; /**< Pool for packet IO */ >> + odp_timer_pool_t tp; /**< Timer pool handle */ >> + odp_queue_t tq; /**< Queue for timeouts */ >> + odp_timer_t tim; /**< Timer handle */ >> + odp_timeout_t tmo_ev; /**< Timeout event */ >> int mode; /**< Thread mode */ >> } thread_args_t; >> >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, odph_ethaddr_t *des); >> static void tv_sub(struct timeval *recvtime, struct timeval *sendtime); >> >> /** >> + * Sleep for the specified amount of milliseconds >> + * Use ODP timer, busy wait until timer expired and timeout event >> received >> + */ >> +static void millisleep(uint32_t ms, >> + odp_timer_pool_t tp, >> + odp_timer_t tim, >> + odp_queue_t q, >> + odp_timeout_t tmo) >> +{ >> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); >> + odp_event_t ev = odp_timeout_to_event(tmo); >> + int rc = odp_timer_set_rel(tim, ticks, &ev); >> + if (rc != ODP_TIMER_SUCCESS) >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >> + /* Spin waiting for timeout event */ >> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) >> + (void)0; >> +} >> + >> +/** >> * Scan ip >> * Parse ip address. >> * >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void *arg) >> thr, >> odp_atomic_load_u64(&counters.seq), >> odp_atomic_load_u64(&counters.seq)%0xffff); >> - /* TODO use odp timer */ >> - struct timespec ts; >> - ts.tv_sec = 0; >> - ts.tv_nsec = args->appl.interval; >> - nanosleep(&ts, NULL); >> + millisleep(args->appl.interval, >> + thr_args->tp, >> + thr_args->tim, >> + thr_args->tq, >> + thr_args->tmo_ev); >> + >> } >> if (args->appl.number != -1 && >> odp_atomic_load_u64(&counters.seq) >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void *arg) >> if (odp_atomic_load_u64(&counters.icmp) >= >> (unsigned int)args->appl.number) >> break; >> - /* TODO use odp timer */ >> - sleep(1); >> + millisleep(1000, >> + thr_args->tp, >> + thr_args->tim, >> + thr_args->tq, >> + thr_args->tmo_ev); >> args->appl.timeout--; >> } >> } >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) >> odp_cpumask_t cpumask; >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; >> odp_pool_param_t params; >> + odp_timer_pool_param_t tparams; >> + odp_timer_pool_t tp; >> + odp_pool_t tmop; >> >> /* Init ODP before calling anything else */ >> if (odp_init_global(NULL, NULL)) { >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) >> } >> odp_pool_print(pool); >> >> + /* Create timer pool */ >> + tparams.res_ns = 1 * ODP_TIME_MSEC; >> + tparams.min_tmo = 0; >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; >> + tparams.num_timers = num_workers; /* One timer per worker */ >> + tparams.priv = 0; /* Shared */ >> + tparams.clk_src = ODP_CLOCK_CPU; >> + tp = odp_timer_pool_create("timer_pool", &tparams); >> + if (tp == ODP_TIMER_POOL_INVALID) { >> + EXAMPLE_ERR("Timer pool create failed.\n"); >> + exit(EXIT_FAILURE); >> + } >> + odp_timer_pool_start(); >> + >> + /* Create timeout pool */ >> + memset(¶ms, 0, sizeof(params)); >> + params.tmo.num = tparams.num_timers; /* One timeout per timer >> */ >> + params.type = ODP_POOL_TIMEOUT; >> + >> + tmop = odp_pool_create("timeout_pool", ODP_SHM_NULL, ¶ms); >> + >> + if (pool == ODP_POOL_INVALID) { >> + EXAMPLE_ERR("Error: packet pool create failed.\n"); >> + exit(EXIT_FAILURE); >> + } >> for (i = 0; i < args->appl.if_count; ++i) >> create_pktio(args->appl.if_names[i], pool); >> >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) >> >> if (args->appl.mode == APPL_MODE_PING) { >> odp_cpumask_t cpu0_mask; >> + odp_queue_t tq; >> >> /* Previous code forced both threads to CPU 0 */ >> odp_cpumask_zero(&cpu0_mask); >> odp_cpumask_set(&cpu0_mask, 0); >> >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); >> + if (tq == ODP_QUEUE_INVALID) >> + abort(); >> args->thread[1].pktio_dev = args->appl.if_names[0]; >> args->thread[1].pool = pool; >> + args->thread[1].tp = tp; >> + args->thread[1].tq = tq; >> + args->thread[1].tim = odp_timer_alloc(tp, tq, NULL); >> + if (args->thread[1].tim == ODP_TIMER_INVALID) >> + abort(); >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); >> + if (args->thread[1].tmo_ev == ODP_TIMEOUT_INVALID) >> + abort(); >> args->thread[1].mode = args->appl.mode; >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, >> gen_recv_thread, >> &args->thread[1]); >> >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); >> + if (tq == ODP_QUEUE_INVALID) >> + abort(); >> args->thread[0].pktio_dev = args->appl.if_names[0]; >> args->thread[0].pool = pool; >> + args->thread[0].tp = tp; >> + args->thread[0].tq = tq; >> + args->thread[0].tim = odp_timer_alloc(tp, tq, NULL); >> + if (args->thread[0].tim == ODP_TIMER_INVALID) >> + abort(); >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); >> + if (args->thread[0].tmo_ev == ODP_TIMEOUT_INVALID) >> + abort(); >> args->thread[0].mode = args->appl.mode; >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, >> gen_send_thread, >> &args->thread[0]); >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) >> odp_cpumask_t thd_mask; >> void *(*thr_run_func) (void *); >> int if_idx; >> + odp_queue_t tq; >> >> if_idx = i % args->appl.if_count; >> >> args->thread[i].pktio_dev = >> args->appl.if_names[if_idx]; >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, >> NULL); >> + if (tq == ODP_QUEUE_INVALID) >> + abort(); >> args->thread[i].pool = pool; >> + args->thread[i].tp = tp; >> + args->thread[i].tq = tq; >> + args->thread[i].tim = odp_timer_alloc(tp, tq, >> NULL); >> + if (args->thread[i].tim == ODP_TIMER_INVALID) >> + abort(); >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); >> + if (args->thread[i].tmo_ev == ODP_TIMEOUT_INVALID) >> + abort(); >> args->thread[i].mode = args->appl.mode; >> >> if (args->appl.mode == APPL_MODE_UDP) { >> -- >> 1.9.1 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 4 March 2015 at 15:17, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > It could be good to add the bug tracker ID or link to the commit > message when merging. > https://bugs.linaro.org/show_bug.cgi?id=1025 > > > On 4 March 2015 at 04:54, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > > > > > > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl <ola.liljedahl@linaro.org > > > > wrote: > >> > >> Use ODP timer facility instead of POSIX sleep/nanosleep. > >> > >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > > > > > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > Maxim this patch is reviewed but doesn't seem to be merged. I asked whether you could add the bug tracker id or link to the commit message. Or should I post a new version of the patch with this included? -- Ola > > > >> > >> --- > >> (This document/code contribution attached is provided under the terms of > >> agreement LES-LTM-21309) > >> > >> example/generator/odp_generator.c | 105 > >> +++++++++++++++++++++++++++++++++++--- > >> 1 file changed, 98 insertions(+), 7 deletions(-) > >> > >> diff --git a/example/generator/odp_generator.c > >> b/example/generator/odp_generator.c > >> index 3870fd1..9147c15 100644 > >> --- a/example/generator/odp_generator.c > >> +++ b/example/generator/odp_generator.c > >> @@ -79,6 +79,10 @@ static struct { > >> typedef struct { > >> char *pktio_dev; /**< Interface name to use */ > >> odp_pool_t pool; /**< Pool for packet IO */ > >> + odp_timer_pool_t tp; /**< Timer pool handle */ > >> + odp_queue_t tq; /**< Queue for timeouts */ > >> + odp_timer_t tim; /**< Timer handle */ > >> + odp_timeout_t tmo_ev; /**< Timeout event */ > >> int mode; /**< Thread mode */ > >> } thread_args_t; > >> > >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, odph_ethaddr_t *des); > >> static void tv_sub(struct timeval *recvtime, struct timeval *sendtime); > >> > >> /** > >> + * Sleep for the specified amount of milliseconds > >> + * Use ODP timer, busy wait until timer expired and timeout event > >> received > >> + */ > >> +static void millisleep(uint32_t ms, > >> + odp_timer_pool_t tp, > >> + odp_timer_t tim, > >> + odp_queue_t q, > >> + odp_timeout_t tmo) > >> +{ > >> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); > >> + odp_event_t ev = odp_timeout_to_event(tmo); > >> + int rc = odp_timer_set_rel(tim, ticks, &ev); > >> + if (rc != ODP_TIMER_SUCCESS) > >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); > >> + /* Spin waiting for timeout event */ > >> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) > >> + (void)0; > >> +} > >> + > >> +/** > >> * Scan ip > >> * Parse ip address. > >> * > >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void *arg) > >> thr, > >> odp_atomic_load_u64(&counters.seq), > >> > odp_atomic_load_u64(&counters.seq)%0xffff); > >> - /* TODO use odp timer */ > >> - struct timespec ts; > >> - ts.tv_sec = 0; > >> - ts.tv_nsec = args->appl.interval; > >> - nanosleep(&ts, NULL); > >> + millisleep(args->appl.interval, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> + > >> } > >> if (args->appl.number != -1 && > >> odp_atomic_load_u64(&counters.seq) > >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void *arg) > >> if (odp_atomic_load_u64(&counters.icmp) >= > >> (unsigned int)args->appl.number) > >> break; > >> - /* TODO use odp timer */ > >> - sleep(1); > >> + millisleep(1000, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> args->appl.timeout--; > >> } > >> } > >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t cpumask; > >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; > >> odp_pool_param_t params; > >> + odp_timer_pool_param_t tparams; > >> + odp_timer_pool_t tp; > >> + odp_pool_t tmop; > >> > >> /* Init ODP before calling anything else */ > >> if (odp_init_global(NULL, NULL)) { > >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) > >> } > >> odp_pool_print(pool); > >> > >> + /* Create timer pool */ > >> + tparams.res_ns = 1 * ODP_TIME_MSEC; > >> + tparams.min_tmo = 0; > >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; > >> + tparams.num_timers = num_workers; /* One timer per worker */ > >> + tparams.priv = 0; /* Shared */ > >> + tparams.clk_src = ODP_CLOCK_CPU; > >> + tp = odp_timer_pool_create("timer_pool", &tparams); > >> + if (tp == ODP_TIMER_POOL_INVALID) { > >> + EXAMPLE_ERR("Timer pool create failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> + odp_timer_pool_start(); > >> + > >> + /* Create timeout pool */ > >> + memset(¶ms, 0, sizeof(params)); > >> + params.tmo.num = tparams.num_timers; /* One timeout per > timer > >> */ > >> + params.type = ODP_POOL_TIMEOUT; > >> + > >> + tmop = odp_pool_create("timeout_pool", ODP_SHM_NULL, ¶ms); > >> + > >> + if (pool == ODP_POOL_INVALID) { > >> + EXAMPLE_ERR("Error: packet pool create failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> for (i = 0; i < args->appl.if_count; ++i) > >> create_pktio(args->appl.if_names[i], pool); > >> > >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) > >> > >> if (args->appl.mode == APPL_MODE_PING) { > >> odp_cpumask_t cpu0_mask; > >> + odp_queue_t tq; > >> > >> /* Previous code forced both threads to CPU 0 */ > >> odp_cpumask_zero(&cpu0_mask); > >> odp_cpumask_set(&cpu0_mask, 0); > >> > >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[1].pktio_dev = args->appl.if_names[0]; > >> args->thread[1].pool = pool; > >> + args->thread[1].tp = tp; > >> + args->thread[1].tq = tq; > >> + args->thread[1].tim = odp_timer_alloc(tp, tq, NULL); > >> + if (args->thread[1].tim == ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[1].tmo_ev == ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[1].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, > >> gen_recv_thread, > >> &args->thread[1]); > >> > >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[0].pktio_dev = args->appl.if_names[0]; > >> args->thread[0].pool = pool; > >> + args->thread[0].tp = tp; > >> + args->thread[0].tq = tq; > >> + args->thread[0].tim = odp_timer_alloc(tp, tq, NULL); > >> + if (args->thread[0].tim == ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[0].tmo_ev == ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[0].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, > >> gen_send_thread, > >> &args->thread[0]); > >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t thd_mask; > >> void *(*thr_run_func) (void *); > >> int if_idx; > >> + odp_queue_t tq; > >> > >> if_idx = i % args->appl.if_count; > >> > >> args->thread[i].pktio_dev = > >> args->appl.if_names[if_idx]; > >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, > >> NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[i].pool = pool; > >> + args->thread[i].tp = tp; > >> + args->thread[i].tq = tq; > >> + args->thread[i].tim = odp_timer_alloc(tp, tq, > >> NULL); > >> + if (args->thread[i].tim == ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[i].tmo_ev = > odp_timeout_alloc(tmop); > >> + if (args->thread[i].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[i].mode = args->appl.mode; > >> > >> if (args->appl.mode == APPL_MODE_UDP) { > >> -- > >> 1.9.1 > >> > >> > >> _______________________________________________ > >> lng-odp mailing list > >> lng-odp@lists.linaro.org > >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > >
On 03/24/15 12:46, Ola Liljedahl wrote: > On 4 March 2015 at 15:17, Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> wrote: > > It could be good to add the bug tracker ID or link to the commit > message when merging. > https://bugs.linaro.org/show_bug.cgi?id=1025 > > > On 4 March 2015 at 04:54, Bill Fischofer > <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote: > > > > > > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl > <ola.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org>> > > wrote: > >> > >> Use ODP timer facility instead of POSIX sleep/nanosleep. > >> > >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> > > > > > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > > Maxim this patch is reviewed but doesn't seem to be merged. > I asked whether you could add the bug tracker id or link to the commit > message. Or should I post a new version of the patch with this included? > > -- Ola That is good but I have 2 things to note: 1. coding style of millisleep() should be the same. (i.e. vars on top of the scope.) 2. It looks like we need to replace sleep() usleep() functions in other places also. It will be easy if we can create odp helper for timers with syntax as match as possible to posix. It might be: odph_posix_timers_init() odph_timer_sleep(); odph_timer_nanosleep() odph_posix_timers_destroy(). Then it will be very fast and clear update to examples and definitely will speed up apps development. What do you think about that? Thanks, Maxim. > > > >> > >> --- > >> (This document/code contribution attached is provided under the > terms of > >> agreement LES-LTM-21309) > >> > >> example/generator/odp_generator.c | 105 > >> +++++++++++++++++++++++++++++++++++--- > >> 1 file changed, 98 insertions(+), 7 deletions(-) > >> > >> diff --git a/example/generator/odp_generator.c > >> b/example/generator/odp_generator.c > >> index 3870fd1..9147c15 100644 > >> --- a/example/generator/odp_generator.c > >> +++ b/example/generator/odp_generator.c > >> @@ -79,6 +79,10 @@ static struct { > >> typedef struct { > >> char *pktio_dev; /**< Interface name to use */ > >> odp_pool_t pool; /**< Pool for packet IO */ > >> + odp_timer_pool_t tp; /**< Timer pool handle */ > >> + odp_queue_t tq; /**< Queue for timeouts */ > >> + odp_timer_t tim; /**< Timer handle */ > >> + odp_timeout_t tmo_ev; /**< Timeout event */ > >> int mode; /**< Thread mode */ > >> } thread_args_t; > >> > >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, > odph_ethaddr_t *des); > >> static void tv_sub(struct timeval *recvtime, struct timeval > *sendtime); > >> > >> /** > >> + * Sleep for the specified amount of milliseconds > >> + * Use ODP timer, busy wait until timer expired and timeout event > >> received > >> + */ > >> +static void millisleep(uint32_t ms, > >> + odp_timer_pool_t tp, > >> + odp_timer_t tim, > >> + odp_queue_t q, > >> + odp_timeout_t tmo) > >> +{ > >> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); > >> + odp_event_t ev = odp_timeout_to_event(tmo); > >> + int rc = odp_timer_set_rel(tim, ticks, &ev); > >> + if (rc != ODP_TIMER_SUCCESS) > >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); > >> + /* Spin waiting for timeout event */ > >> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) > >> + (void)0; > >> +} > >> + > >> +/** > >> * Scan ip > >> * Parse ip address. > >> * > >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void *arg) > >> thr, > >> odp_atomic_load_u64(&counters.seq), > >> odp_atomic_load_u64(&counters.seq)%0xffff); > >> - /* TODO use odp timer */ > >> - struct timespec ts; > >> - ts.tv_sec = 0; > >> - ts.tv_nsec = args->appl.interval; > >> - nanosleep(&ts, NULL); > >> + millisleep(args->appl.interval, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> + > >> } > >> if (args->appl.number != -1 && > >> odp_atomic_load_u64(&counters.seq) > >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void *arg) > >> if (odp_atomic_load_u64(&counters.icmp) >= > >> (unsigned int)args->appl.number) > >> break; > >> - /* TODO use odp timer */ > >> - sleep(1); > >> + millisleep(1000, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> args->appl.timeout--; > >> } > >> } > >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t cpumask; > >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; > >> odp_pool_param_t params; > >> + odp_timer_pool_param_t tparams; > >> + odp_timer_pool_t tp; > >> + odp_pool_t tmop; > >> > >> /* Init ODP before calling anything else */ > >> if (odp_init_global(NULL, NULL)) { > >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) > >> } > >> odp_pool_print(pool); > >> > >> + /* Create timer pool */ > >> + tparams.res_ns = 1 * ODP_TIME_MSEC; > >> + tparams.min_tmo = 0; > >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; > >> + tparams.num_timers = num_workers; /* One timer per > worker */ > >> + tparams.priv = 0; /* Shared */ > >> + tparams.clk_src = ODP_CLOCK_CPU; > >> + tp = odp_timer_pool_create("timer_pool", &tparams); > >> + if (tp == ODP_TIMER_POOL_INVALID) { > >> + EXAMPLE_ERR("Timer pool create failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> + odp_timer_pool_start(); > >> + > >> + /* Create timeout pool */ > >> + memset(¶ms, 0, sizeof(params)); > >> + params.tmo.num = tparams.num_timers; /* One timeout > per timer > >> */ > >> + params.type = ODP_POOL_TIMEOUT; > >> + > >> + tmop = odp_pool_create("timeout_pool", ODP_SHM_NULL, > ¶ms); > >> + > >> + if (pool == ODP_POOL_INVALID) { > >> + EXAMPLE_ERR("Error: packet pool create failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> for (i = 0; i < args->appl.if_count; ++i) > >> create_pktio(args->appl.if_names[i], pool); > >> > >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) > >> > >> if (args->appl.mode == APPL_MODE_PING) { > >> odp_cpumask_t cpu0_mask; > >> + odp_queue_t tq; > >> > >> /* Previous code forced both threads to CPU 0 */ > >> odp_cpumask_zero(&cpu0_mask); > >> odp_cpumask_set(&cpu0_mask, 0); > >> > >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, > NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[1].pktio_dev = args->appl.if_names[0]; > >> args->thread[1].pool = pool; > >> + args->thread[1].tp = tp; > >> + args->thread[1].tq = tq; > >> + args->thread[1].tim = odp_timer_alloc(tp, tq, > NULL); > >> + if (args->thread[1].tim == ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[1].tmo_ev == ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[1].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, > >> gen_recv_thread, > >> &args->thread[1]); > >> > >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, > NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[0].pktio_dev = args->appl.if_names[0]; > >> args->thread[0].pool = pool; > >> + args->thread[0].tp = tp; > >> + args->thread[0].tq = tq; > >> + args->thread[0].tim = odp_timer_alloc(tp, tq, > NULL); > >> + if (args->thread[0].tim == ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[0].tmo_ev == ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[0].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, > >> gen_send_thread, > >> &args->thread[0]); > >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t thd_mask; > >> void *(*thr_run_func) (void *); > >> int if_idx; > >> + odp_queue_t tq; > >> > >> if_idx = i % args->appl.if_count; > >> > >> args->thread[i].pktio_dev = > >> args->appl.if_names[if_idx]; > >> + tq = odp_queue_create("", > ODP_QUEUE_TYPE_POLL, > >> NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[i].pool = pool; > >> + args->thread[i].tp = tp; > >> + args->thread[i].tq = tq; > >> + args->thread[i].tim = odp_timer_alloc(tp, tq, > >> NULL); > >> + if (args->thread[i].tim == > ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[i].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[i].mode = args->appl.mode; > >> > >> if (args->appl.mode == APPL_MODE_UDP) { > >> -- > >> 1.9.1 > >> > >> > >> _______________________________________________ > >> lng-odp mailing list > >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > >
On 24 March 2015 at 11:51, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/24/15 12:46, Ola Liljedahl wrote: > >> On 4 March 2015 at 15:17, Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> wrote: >> >> It could be good to add the bug tracker ID or link to the commit >> message when merging. >> https://bugs.linaro.org/show_bug.cgi?id=1025 >> >> >> On 4 March 2015 at 04:54, Bill Fischofer >> <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote: >> > >> > >> > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl >> <ola.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org>> >> > wrote: >> >> >> >> Use ODP timer facility instead of POSIX sleep/nanosleep. >> >> >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> >> > >> > >> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> >> >> Maxim this patch is reviewed but doesn't seem to be merged. >> I asked whether you could add the bug tracker id or link to the commit >> message. Or should I post a new version of the patch with this included? >> >> -- Ola >> > > That is good but I have 2 things to note: > 1. coding style of millisleep() should be the same. (i.e. vars on top of > the scope.) > The variables definitions *are* at the beginning of the scope. The variables happen to be initialized as well. We don't have a rule against that? 2. It looks like we need to replace sleep() usleep() functions in other > places also. It will be easy if we can create odp helper for timers with > syntax as match as possible to posix. > > It might be: > odph_posix_timers_init() > > odph_timer_sleep(); > odph_timer_nanosleep() > > odph_posix_timers_destroy(). > > Then it will be very fast and clear update to examples and definitely will > speed up apps development. What do you think about that? > Preferably, ODP programs should not block in sleep-like functions at all. That is against the principle of worker threads running to completion. So this is not a behavior we want to recommend or support. Ideally, the ODP generator should also not need to sleep and should be redesigned. That was not in the scope of the bug report/change request though. > > Thanks, > Maxim. > > > > > >> >> >> >> --- >> >> (This document/code contribution attached is provided under the >> terms of >> >> agreement LES-LTM-21309) >> >> >> >> example/generator/odp_generator.c | 105 >> >> +++++++++++++++++++++++++++++++++++--- >> >> 1 file changed, 98 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/example/generator/odp_generator.c >> >> b/example/generator/odp_generator.c >> >> index 3870fd1..9147c15 100644 >> >> --- a/example/generator/odp_generator.c >> >> +++ b/example/generator/odp_generator.c >> >> @@ -79,6 +79,10 @@ static struct { >> >> typedef struct { >> >> char *pktio_dev; /**< Interface name to use */ >> >> odp_pool_t pool; /**< Pool for packet IO */ >> >> + odp_timer_pool_t tp; /**< Timer pool handle */ >> >> + odp_queue_t tq; /**< Queue for timeouts */ >> >> + odp_timer_t tim; /**< Timer handle */ >> >> + odp_timeout_t tmo_ev; /**< Timeout event */ >> >> int mode; /**< Thread mode */ >> >> } thread_args_t; >> >> >> >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, >> odph_ethaddr_t *des); >> >> static void tv_sub(struct timeval *recvtime, struct timeval >> *sendtime); >> >> >> >> /** >> >> + * Sleep for the specified amount of milliseconds >> >> + * Use ODP timer, busy wait until timer expired and timeout event >> >> received >> >> + */ >> >> +static void millisleep(uint32_t ms, >> >> + odp_timer_pool_t tp, >> >> + odp_timer_t tim, >> >> + odp_queue_t q, >> >> + odp_timeout_t tmo) >> >> +{ >> >> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); >> >> + odp_event_t ev = odp_timeout_to_event(tmo); >> >> + int rc = odp_timer_set_rel(tim, ticks, &ev); >> >> + if (rc != ODP_TIMER_SUCCESS) >> >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >> >> + /* Spin waiting for timeout event */ >> >> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) >> >> + (void)0; >> >> +} >> >> + >> >> +/** >> >> * Scan ip >> >> * Parse ip address. >> >> * >> >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void *arg) >> >> thr, >> >> odp_atomic_load_u64(&counters.seq), >> >> odp_atomic_load_u64(&counters.seq)%0xffff); >> >> - /* TODO use odp timer */ >> >> - struct timespec ts; >> >> - ts.tv_sec = 0; >> >> - ts.tv_nsec = args->appl.interval; >> >> - nanosleep(&ts, NULL); >> >> + millisleep(args->appl.interval, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> + >> >> } >> >> if (args->appl.number != -1 && >> >> odp_atomic_load_u64(&counters.seq) >> >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void *arg) >> >> if (odp_atomic_load_u64(&counters.icmp) >= >> >> (unsigned int)args->appl.number) >> >> break; >> >> - /* TODO use odp timer */ >> >> - sleep(1); >> >> + millisleep(1000, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> args->appl.timeout--; >> >> } >> >> } >> >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t cpumask; >> >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; >> >> odp_pool_param_t params; >> >> + odp_timer_pool_param_t tparams; >> >> + odp_timer_pool_t tp; >> >> + odp_pool_t tmop; >> >> >> >> /* Init ODP before calling anything else */ >> >> if (odp_init_global(NULL, NULL)) { >> >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) >> >> } >> >> odp_pool_print(pool); >> >> >> >> + /* Create timer pool */ >> >> + tparams.res_ns = 1 * ODP_TIME_MSEC; >> >> + tparams.min_tmo = 0; >> >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; >> >> + tparams.num_timers = num_workers; /* One timer per >> worker */ >> >> + tparams.priv = 0; /* Shared */ >> >> + tparams.clk_src = ODP_CLOCK_CPU; >> >> + tp = odp_timer_pool_create("timer_pool", &tparams); >> >> + if (tp == ODP_TIMER_POOL_INVALID) { >> >> + EXAMPLE_ERR("Timer pool create failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> + odp_timer_pool_start(); >> >> + >> >> + /* Create timeout pool */ >> >> + memset(¶ms, 0, sizeof(params)); >> >> + params.tmo.num = tparams.num_timers; /* One timeout >> per timer >> >> */ >> >> + params.type = ODP_POOL_TIMEOUT; >> >> + >> >> + tmop = odp_pool_create("timeout_pool", ODP_SHM_NULL, >> ¶ms); >> >> + >> >> + if (pool == ODP_POOL_INVALID) { >> >> + EXAMPLE_ERR("Error: packet pool create failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> for (i = 0; i < args->appl.if_count; ++i) >> >> create_pktio(args->appl.if_names[i], pool); >> >> >> >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) >> >> >> >> if (args->appl.mode == APPL_MODE_PING) { >> >> odp_cpumask_t cpu0_mask; >> >> + odp_queue_t tq; >> >> >> >> /* Previous code forced both threads to CPU 0 */ >> >> odp_cpumask_zero(&cpu0_mask); >> >> odp_cpumask_set(&cpu0_mask, 0); >> >> >> >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[1].pktio_dev = args->appl.if_names[0]; >> >> args->thread[1].pool = pool; >> >> + args->thread[1].tp = tp; >> >> + args->thread[1].tq = tq; >> >> + args->thread[1].tim = odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[1].tim == ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[1].tmo_ev == ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[1].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, >> >> gen_recv_thread, >> >> &args->thread[1]); >> >> >> >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[0].pktio_dev = args->appl.if_names[0]; >> >> args->thread[0].pool = pool; >> >> + args->thread[0].tp = tp; >> >> + args->thread[0].tq = tq; >> >> + args->thread[0].tim = odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[0].tim == ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[0].tmo_ev == ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[0].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, >> >> gen_send_thread, >> >> &args->thread[0]); >> >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t thd_mask; >> >> void *(*thr_run_func) (void *); >> >> int if_idx; >> >> + odp_queue_t tq; >> >> >> >> if_idx = i % args->appl.if_count; >> >> >> >> args->thread[i].pktio_dev = >> >> args->appl.if_names[if_idx]; >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[i].pool = pool; >> >> + args->thread[i].tp = tp; >> >> + args->thread[i].tq = tq; >> >> + args->thread[i].tim = odp_timer_alloc(tp, tq, >> >> NULL); >> >> + if (args->thread[i].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[i].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[i].mode = args->appl.mode; >> >> >> >> if (args->appl.mode == APPL_MODE_UDP) { >> >> -- >> >> 1.9.1 >> >> >> >> >> >> _______________________________________________ >> >> lng-odp mailing list >> >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >> > >> >> >> >
On 03/24/15 14:27, Ola Liljedahl wrote: > On 24 March 2015 at 11:51, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 03/24/15 12:46, Ola Liljedahl wrote: > > On 4 March 2015 at 15:17, Ola Liljedahl > <ola.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org> > <mailto:ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>>> wrote: > > It could be good to add the bug tracker ID or link to the > commit > message when merging. > https://bugs.linaro.org/show_bug.cgi?id=1025 > > > On 4 March 2015 at 04:54, Bill Fischofer > <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org> > <mailto:bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>>> wrote: > > > > > > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl > <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org> > <mailto:ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>>> > > wrote: > >> > >> Use ODP timer facility instead of POSIX sleep/nanosleep. > >> > >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org> > <mailto:ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>>> > > > > > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org> > <mailto:bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>>> > > Maxim this patch is reviewed but doesn't seem to be merged. > I asked whether you could add the bug tracker id or link to > the commit message. Or should I post a new version of the > patch with this included? > > -- Ola > > > That is good but I have 2 things to note: > 1. coding style of millisleep() should be the same. (i.e. vars on > top of the scope.) > > The variables definitions *are* at the beginning of the scope. The > variables happen to be initialized as well. We don't have a rule > against that? > > 2. It looks like we need to replace sleep() usleep() functions in > other places also. It will be easy if we can create odp helper for > timers with syntax as match as possible to posix. > > It might be: > odph_posix_timers_init() > > odph_timer_sleep(); > odph_timer_nanosleep() > > odph_posix_timers_destroy(). > > Then it will be very fast and clear update to examples and > definitely will speed up apps development. What do you think about > that? > > Preferably, ODP programs should not block in sleep-like functions at > all. That is against the principle of worker threads running to > completion. So this is not a behavior we want to recommend or support. > > Ideally, the ODP generator should also not need to sleep and should be > redesigned. That was not in the scope of the bug report/change request > though. > But at least in our examples main thread starts worker threads and then does nothing. That nothing time can be used to printing stats and cycles with sleeping. Is it common case where one starter thread does nothing? Maxim. > > Thanks, > Maxim. > > > > > > >> > >> --- > >> (This document/code contribution attached is provided > under the > terms of > >> agreement LES-LTM-21309) > >> > >> example/generator/odp_generator.c | 105 > >> +++++++++++++++++++++++++++++++++++--- > >> 1 file changed, 98 insertions(+), 7 deletions(-) > >> > >> diff --git a/example/generator/odp_generator.c > >> b/example/generator/odp_generator.c > >> index 3870fd1..9147c15 100644 > >> --- a/example/generator/odp_generator.c > >> +++ b/example/generator/odp_generator.c > >> @@ -79,6 +79,10 @@ static struct { > >> typedef struct { > >> char *pktio_dev; /**< Interface name to > use */ > >> odp_pool_t pool; /**< Pool for packet IO */ > >> + odp_timer_pool_t tp; /**< Timer pool handle */ > >> + odp_queue_t tq; /**< Queue for timeouts */ > >> + odp_timer_t tim; /**< Timer handle */ > >> + odp_timeout_t tmo_ev; /**< Timeout event */ > >> int mode; /**< Thread mode */ > >> } thread_args_t; > >> > >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, > odph_ethaddr_t *des); > >> static void tv_sub(struct timeval *recvtime, struct > timeval > *sendtime); > >> > >> /** > >> + * Sleep for the specified amount of milliseconds > >> + * Use ODP timer, busy wait until timer expired and > timeout event > >> received > >> + */ > >> +static void millisleep(uint32_t ms, > >> + odp_timer_pool_t tp, > >> + odp_timer_t tim, > >> + odp_queue_t q, > >> + odp_timeout_t tmo) > >> +{ > >> + uint64_t ticks = odp_timer_ns_to_tick(tp, > 1000000ULL * ms); > >> + odp_event_t ev = odp_timeout_to_event(tmo); > >> + int rc = odp_timer_set_rel(tim, ticks, &ev); > >> + if (rc != ODP_TIMER_SUCCESS) > >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); > >> + /* Spin waiting for timeout event */ > >> + while ((ev = odp_queue_deq(q)) == > ODP_EVENT_INVALID) > >> + (void)0; > >> +} > >> + > >> +/** > >> * Scan ip > >> * Parse ip address. > >> * > >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void > *arg) > >> thr, > >> odp_atomic_load_u64(&counters.seq), > >> odp_atomic_load_u64(&counters.seq)%0xffff); > >> - /* TODO use odp timer */ > >> - struct timespec ts; > >> - ts.tv_sec = 0; > >> - ts.tv_nsec = args->appl.interval; > >> - nanosleep(&ts, NULL); > >> + millisleep(args->appl.interval, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> + > >> } > >> if (args->appl.number != -1 && > >> odp_atomic_load_u64(&counters.seq) > >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void > *arg) > >> if > (odp_atomic_load_u64(&counters.icmp) >= > >> (unsigned > int)args->appl.number) > >> break; > >> - /* TODO use odp timer */ > >> - sleep(1); > >> + millisleep(1000, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> args->appl.timeout--; > >> } > >> } > >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t cpumask; > >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; > >> odp_pool_param_t params; > >> + odp_timer_pool_param_t tparams; > >> + odp_timer_pool_t tp; > >> + odp_pool_t tmop; > >> > >> /* Init ODP before calling anything else */ > >> if (odp_init_global(NULL, NULL)) { > >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) > >> } > >> odp_pool_print(pool); > >> > >> + /* Create timer pool */ > >> + tparams.res_ns = 1 * ODP_TIME_MSEC; > >> + tparams.min_tmo = 0; > >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; > >> + tparams.num_timers = num_workers; /* One timer per > worker */ > >> + tparams.priv = 0; /* Shared */ > >> + tparams.clk_src = ODP_CLOCK_CPU; > >> + tp = odp_timer_pool_create("timer_pool", &tparams); > >> + if (tp == ODP_TIMER_POOL_INVALID) { > >> + EXAMPLE_ERR("Timer pool create failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> + odp_timer_pool_start(); > >> + > >> + /* Create timeout pool */ > >> + memset(¶ms, 0, sizeof(params)); > >> + params.tmo.num = tparams.num_timers; /* One > timeout > per timer > >> */ > >> + params.type = ODP_POOL_TIMEOUT; > >> + > >> + tmop = odp_pool_create("timeout_pool", > ODP_SHM_NULL, > ¶ms); > >> + > >> + if (pool == ODP_POOL_INVALID) { > >> + EXAMPLE_ERR("Error: packet pool create > failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> for (i = 0; i < args->appl.if_count; ++i) > >> create_pktio(args->appl.if_names[i], pool); > >> > >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) > >> > >> if (args->appl.mode == APPL_MODE_PING) { > >> odp_cpumask_t cpu0_mask; > >> + odp_queue_t tq; > >> > >> /* Previous code forced both threads to > CPU 0 */ > >> odp_cpumask_zero(&cpu0_mask); > >> odp_cpumask_set(&cpu0_mask, 0); > >> > >> + tq = odp_queue_create("", > ODP_QUEUE_TYPE_POLL, > NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[1].pktio_dev = args->appl.if_names[0]; > >> args->thread[1].pool = pool; > >> + args->thread[1].tp = tp; > >> + args->thread[1].tq = tq; > >> + args->thread[1].tim = > odp_timer_alloc(tp, tq, > NULL); > >> + if (args->thread[1].tim == > ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[1].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[1].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, > >> gen_recv_thread, > >> &args->thread[1]); > >> > >> + tq = odp_queue_create("", > ODP_QUEUE_TYPE_POLL, > NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[0].pktio_dev = args->appl.if_names[0]; > >> args->thread[0].pool = pool; > >> + args->thread[0].tp = tp; > >> + args->thread[0].tq = tq; > >> + args->thread[0].tim = > odp_timer_alloc(tp, tq, > NULL); > >> + if (args->thread[0].tim == > ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[0].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[0].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, > >> gen_send_thread, > >> &args->thread[0]); > >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t thd_mask; > >> void *(*thr_run_func) (void *); > >> int if_idx; > >> + odp_queue_t tq; > >> > >> if_idx = i % args->appl.if_count; > >> > >> args->thread[i].pktio_dev = > >> args->appl.if_names[if_idx]; > >> + tq = odp_queue_create("", > ODP_QUEUE_TYPE_POLL, > >> NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[i].pool = pool; > >> + args->thread[i].tp = tp; > >> + args->thread[i].tq = tq; > >> + args->thread[i].tim = odp_timer_alloc(tp, tq, > >> NULL); > >> + if (args->thread[i].tim == > ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[i].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[i].mode = args->appl.mode; > >> > >> if (args->appl.mode == > APPL_MODE_UDP) { > >> -- > >> 1.9.1 > >> > >> > >> _______________________________________________ > >> lng-odp mailing list > >> lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org> > <mailto:lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org>> > >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > > >
On 03/03/15 19:19, Ola Liljedahl wrote: > +static void millisleep(uint32_t ms, > + odp_timer_pool_t tp, > + odp_timer_t tim, > + odp_queue_t q, > + odp_timeout_t tmo) > +{ > + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); > + odp_event_t ev = odp_timeout_to_event(tmo); > + int rc = odp_timer_set_rel(tim, ticks, &ev); > + if (rc != ODP_TIMER_SUCCESS) > + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); > + /* Spin waiting for timeout event */ > + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) > + (void)0; > +} In C++ (void)0 will be static_cast<void>. If we plan to support C++ compiler then it might be reason to avoid that. odp_spin() looks like more suitable for that case. Maxim.
On 24 March 2015 at 06:51, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/24/15 12:46, Ola Liljedahl wrote: > >> On 4 March 2015 at 15:17, Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> wrote: >> >> It could be good to add the bug tracker ID or link to the commit >> message when merging. >> https://bugs.linaro.org/show_bug.cgi?id=1025 >> >> >> On 4 March 2015 at 04:54, Bill Fischofer >> <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote: >> > >> > >> > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl >> <ola.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org>> >> > wrote: >> >> >> >> Use ODP timer facility instead of POSIX sleep/nanosleep. >> >> >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> >> > >> > >> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> >> >> Maxim this patch is reviewed but doesn't seem to be merged. >> I asked whether you could add the bug tracker id or link to the commit >> message. Or should I post a new version of the patch with this included? >> >> -- Ola >> > > That is good but I have 2 things to note: > 1. coding style of millisleep() should be the same. (i.e. vars on top of > the scope.) > 2. It looks like we need to replace sleep() usleep() functions in other > places also. It will be easy if we can create odp helper for timers with > syntax as match as possible to posix. > > It might be: > odph_posix_timers_init() > > odph_timer_sleep(); > odph_timer_nanosleep() > > odph_posix_timers_destroy(). > > Then it will be very fast and clear update to examples and definitely will > speed up apps development. What do you think about that? > I dont like adding OS portability to the ODP API or its helpers, ODP should be an abstraction for networking acceleration only. POSIX is supposed to be such an OS wrapper, do we want to wrap a wrapper ? > > Thanks, > Maxim. > > > > > >> >> >> >> --- >> >> (This document/code contribution attached is provided under the >> terms of >> >> agreement LES-LTM-21309) >> >> >> >> example/generator/odp_generator.c | 105 >> >> +++++++++++++++++++++++++++++++++++--- >> >> 1 file changed, 98 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/example/generator/odp_generator.c >> >> b/example/generator/odp_generator.c >> >> index 3870fd1..9147c15 100644 >> >> --- a/example/generator/odp_generator.c >> >> +++ b/example/generator/odp_generator.c >> >> @@ -79,6 +79,10 @@ static struct { >> >> typedef struct { >> >> char *pktio_dev; /**< Interface name to use */ >> >> odp_pool_t pool; /**< Pool for packet IO */ >> >> + odp_timer_pool_t tp; /**< Timer pool handle */ >> >> + odp_queue_t tq; /**< Queue for timeouts */ >> >> + odp_timer_t tim; /**< Timer handle */ >> >> + odp_timeout_t tmo_ev; /**< Timeout event */ >> >> int mode; /**< Thread mode */ >> >> } thread_args_t; >> >> >> >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, >> odph_ethaddr_t *des); >> >> static void tv_sub(struct timeval *recvtime, struct timeval >> *sendtime); >> >> >> >> /** >> >> + * Sleep for the specified amount of milliseconds >> >> + * Use ODP timer, busy wait until timer expired and timeout event >> >> received >> >> + */ >> >> +static void millisleep(uint32_t ms, >> >> + odp_timer_pool_t tp, >> >> + odp_timer_t tim, >> >> + odp_queue_t q, >> >> + odp_timeout_t tmo) >> >> +{ >> >> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); >> >> + odp_event_t ev = odp_timeout_to_event(tmo); >> >> + int rc = odp_timer_set_rel(tim, ticks, &ev); >> >> + if (rc != ODP_TIMER_SUCCESS) >> >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >> >> + /* Spin waiting for timeout event */ >> >> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) >> >> + (void)0; >> >> +} >> >> + >> >> +/** >> >> * Scan ip >> >> * Parse ip address. >> >> * >> >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void *arg) >> >> thr, >> >> odp_atomic_load_u64(&counters.seq), >> >> odp_atomic_load_u64(&counters.seq)%0xffff); >> >> - /* TODO use odp timer */ >> >> - struct timespec ts; >> >> - ts.tv_sec = 0; >> >> - ts.tv_nsec = args->appl.interval; >> >> - nanosleep(&ts, NULL); >> >> + millisleep(args->appl.interval, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> + >> >> } >> >> if (args->appl.number != -1 && >> >> odp_atomic_load_u64(&counters.seq) >> >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void *arg) >> >> if (odp_atomic_load_u64(&counters.icmp) >= >> >> (unsigned int)args->appl.number) >> >> break; >> >> - /* TODO use odp timer */ >> >> - sleep(1); >> >> + millisleep(1000, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> args->appl.timeout--; >> >> } >> >> } >> >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t cpumask; >> >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; >> >> odp_pool_param_t params; >> >> + odp_timer_pool_param_t tparams; >> >> + odp_timer_pool_t tp; >> >> + odp_pool_t tmop; >> >> >> >> /* Init ODP before calling anything else */ >> >> if (odp_init_global(NULL, NULL)) { >> >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) >> >> } >> >> odp_pool_print(pool); >> >> >> >> + /* Create timer pool */ >> >> + tparams.res_ns = 1 * ODP_TIME_MSEC; >> >> + tparams.min_tmo = 0; >> >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; >> >> + tparams.num_timers = num_workers; /* One timer per >> worker */ >> >> + tparams.priv = 0; /* Shared */ >> >> + tparams.clk_src = ODP_CLOCK_CPU; >> >> + tp = odp_timer_pool_create("timer_pool", &tparams); >> >> + if (tp == ODP_TIMER_POOL_INVALID) { >> >> + EXAMPLE_ERR("Timer pool create failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> + odp_timer_pool_start(); >> >> + >> >> + /* Create timeout pool */ >> >> + memset(¶ms, 0, sizeof(params)); >> >> + params.tmo.num = tparams.num_timers; /* One timeout >> per timer >> >> */ >> >> + params.type = ODP_POOL_TIMEOUT; >> >> + >> >> + tmop = odp_pool_create("timeout_pool", ODP_SHM_NULL, >> ¶ms); >> >> + >> >> + if (pool == ODP_POOL_INVALID) { >> >> + EXAMPLE_ERR("Error: packet pool create failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> for (i = 0; i < args->appl.if_count; ++i) >> >> create_pktio(args->appl.if_names[i], pool); >> >> >> >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) >> >> >> >> if (args->appl.mode == APPL_MODE_PING) { >> >> odp_cpumask_t cpu0_mask; >> >> + odp_queue_t tq; >> >> >> >> /* Previous code forced both threads to CPU 0 */ >> >> odp_cpumask_zero(&cpu0_mask); >> >> odp_cpumask_set(&cpu0_mask, 0); >> >> >> >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[1].pktio_dev = args->appl.if_names[0]; >> >> args->thread[1].pool = pool; >> >> + args->thread[1].tp = tp; >> >> + args->thread[1].tq = tq; >> >> + args->thread[1].tim = odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[1].tim == ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[1].tmo_ev == ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[1].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, >> >> gen_recv_thread, >> >> &args->thread[1]); >> >> >> >> + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[0].pktio_dev = args->appl.if_names[0]; >> >> args->thread[0].pool = pool; >> >> + args->thread[0].tp = tp; >> >> + args->thread[0].tq = tq; >> >> + args->thread[0].tim = odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[0].tim == ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[0].tmo_ev == ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[0].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, >> >> gen_send_thread, >> >> &args->thread[0]); >> >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t thd_mask; >> >> void *(*thr_run_func) (void *); >> >> int if_idx; >> >> + odp_queue_t tq; >> >> >> >> if_idx = i % args->appl.if_count; >> >> >> >> args->thread[i].pktio_dev = >> >> args->appl.if_names[if_idx]; >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[i].pool = pool; >> >> + args->thread[i].tp = tp; >> >> + args->thread[i].tq = tq; >> >> + args->thread[i].tim = odp_timer_alloc(tp, tq, >> >> NULL); >> >> + if (args->thread[i].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[i].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[i].mode = args->appl.mode; >> >> >> >> if (args->appl.mode == APPL_MODE_UDP) { >> >> -- >> >> 1.9.1 >> >> >> >> >> >> _______________________________________________ >> >> lng-odp mailing list >> >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >> > >> >> >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 24 March 2015 at 13:59, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/24/15 14:27, Ola Liljedahl wrote: > >> On 24 March 2015 at 11:51, Maxim Uvarov <maxim.uvarov@linaro.org <mailto: >> maxim.uvarov@linaro.org>> wrote: >> >> On 03/24/15 12:46, Ola Liljedahl wrote: >> >> On 4 March 2015 at 15:17, Ola Liljedahl >> <ola.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org> >> <mailto:ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>>> wrote: >> >> It could be good to add the bug tracker ID or link to the >> commit >> message when merging. >> https://bugs.linaro.org/show_bug.cgi?id=1025 >> >> >> On 4 March 2015 at 04:54, Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org> >> <mailto:bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>>> wrote: >> > >> > >> > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl >> <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org> >> <mailto:ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>>> >> > wrote: >> >> >> >> Use ODP timer facility instead of POSIX sleep/nanosleep. >> >> >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org> >> <mailto:ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>>> >> > >> > >> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org> >> <mailto:bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>>> >> >> Maxim this patch is reviewed but doesn't seem to be merged. >> I asked whether you could add the bug tracker id or link to >> the commit message. Or should I post a new version of the >> patch with this included? >> >> -- Ola >> >> >> That is good but I have 2 things to note: >> 1. coding style of millisleep() should be the same. (i.e. vars on >> top of the scope.) >> >> The variables definitions *are* at the beginning of the scope. The >> variables happen to be initialized as well. We don't have a rule against >> that? >> >> 2. It looks like we need to replace sleep() usleep() functions in >> other places also. It will be easy if we can create odp helper for >> timers with syntax as match as possible to posix. >> >> It might be: >> odph_posix_timers_init() >> >> odph_timer_sleep(); >> odph_timer_nanosleep() >> >> odph_posix_timers_destroy(). >> >> Then it will be very fast and clear update to examples and >> definitely will speed up apps development. What do you think about >> that? >> >> Preferably, ODP programs should not block in sleep-like functions at all. >> That is against the principle of worker threads running to completion. So >> this is not a behavior we want to recommend or support. >> >> Ideally, the ODP generator should also not need to sleep and should be >> redesigned. That was not in the scope of the bug report/change request >> though. >> >> > But at least in our examples main thread starts worker threads and then > does nothing. That nothing time can be used to printing stats and cycles > with sleeping. Is it common case where one starter thread does nothing? > I don't know. But that is a completely different discussion. I have fixed the bug per the report. I don't see why this bug fix should wait for some other design discussion to complete. > > Maxim. > > >> Thanks, >> Maxim. >> >> >> >> > >> >> >> >> --- >> >> (This document/code contribution attached is provided >> under the >> terms of >> >> agreement LES-LTM-21309) >> >> >> >> example/generator/odp_generator.c | 105 >> >> +++++++++++++++++++++++++++++++++++--- >> >> 1 file changed, 98 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/example/generator/odp_generator.c >> >> b/example/generator/odp_generator.c >> >> index 3870fd1..9147c15 100644 >> >> --- a/example/generator/odp_generator.c >> >> +++ b/example/generator/odp_generator.c >> >> @@ -79,6 +79,10 @@ static struct { >> >> typedef struct { >> >> char *pktio_dev; /**< Interface name to >> use */ >> >> odp_pool_t pool; /**< Pool for packet IO */ >> >> + odp_timer_pool_t tp; /**< Timer pool handle */ >> >> + odp_queue_t tq; /**< Queue for timeouts */ >> >> + odp_timer_t tim; /**< Timer handle */ >> >> + odp_timeout_t tmo_ev; /**< Timeout event */ >> >> int mode; /**< Thread mode */ >> >> } thread_args_t; >> >> >> >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, >> odph_ethaddr_t *des); >> >> static void tv_sub(struct timeval *recvtime, struct >> timeval >> *sendtime); >> >> >> >> /** >> >> + * Sleep for the specified amount of milliseconds >> >> + * Use ODP timer, busy wait until timer expired and >> timeout event >> >> received >> >> + */ >> >> +static void millisleep(uint32_t ms, >> >> + odp_timer_pool_t tp, >> >> + odp_timer_t tim, >> >> + odp_queue_t q, >> >> + odp_timeout_t tmo) >> >> +{ >> >> + uint64_t ticks = odp_timer_ns_to_tick(tp, >> 1000000ULL * ms); >> >> + odp_event_t ev = odp_timeout_to_event(tmo); >> >> + int rc = odp_timer_set_rel(tim, ticks, &ev); >> >> + if (rc != ODP_TIMER_SUCCESS) >> >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >> >> + /* Spin waiting for timeout event */ >> >> + while ((ev = odp_queue_deq(q)) == >> ODP_EVENT_INVALID) >> >> + (void)0; >> >> +} >> >> + >> >> +/** >> >> * Scan ip >> >> * Parse ip address. >> >> * >> >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void >> *arg) >> >> thr, >> >> odp_atomic_load_u64(&counters.seq), >> >> odp_atomic_load_u64(&counters.seq)%0xffff); >> >> - /* TODO use odp timer */ >> >> - struct timespec ts; >> >> - ts.tv_sec = 0; >> >> - ts.tv_nsec = args->appl.interval; >> >> - nanosleep(&ts, NULL); >> >> + millisleep(args->appl.interval, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> + >> >> } >> >> if (args->appl.number != -1 && >> >> odp_atomic_load_u64(&counters.seq) >> >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void >> *arg) >> >> if >> (odp_atomic_load_u64(&counters.icmp) >= >> >> (unsigned >> int)args->appl.number) >> >> break; >> >> - /* TODO use odp timer */ >> >> - sleep(1); >> >> + millisleep(1000, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> args->appl.timeout--; >> >> } >> >> } >> >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t cpumask; >> >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; >> >> odp_pool_param_t params; >> >> + odp_timer_pool_param_t tparams; >> >> + odp_timer_pool_t tp; >> >> + odp_pool_t tmop; >> >> >> >> /* Init ODP before calling anything else */ >> >> if (odp_init_global(NULL, NULL)) { >> >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) >> >> } >> >> odp_pool_print(pool); >> >> >> >> + /* Create timer pool */ >> >> + tparams.res_ns = 1 * ODP_TIME_MSEC; >> >> + tparams.min_tmo = 0; >> >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; >> >> + tparams.num_timers = num_workers; /* One timer per >> worker */ >> >> + tparams.priv = 0; /* Shared */ >> >> + tparams.clk_src = ODP_CLOCK_CPU; >> >> + tp = odp_timer_pool_create("timer_pool", >> &tparams); >> >> + if (tp == ODP_TIMER_POOL_INVALID) { >> >> + EXAMPLE_ERR("Timer pool create failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> + odp_timer_pool_start(); >> >> + >> >> + /* Create timeout pool */ >> >> + memset(¶ms, 0, sizeof(params)); >> >> + params.tmo.num = tparams.num_timers; /* One >> timeout >> per timer >> >> */ >> >> + params.type = ODP_POOL_TIMEOUT; >> >> + >> >> + tmop = odp_pool_create("timeout_pool", >> ODP_SHM_NULL, >> ¶ms); >> >> + >> >> + if (pool == ODP_POOL_INVALID) { >> >> + EXAMPLE_ERR("Error: packet pool create >> failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> for (i = 0; i < args->appl.if_count; ++i) >> >> create_pktio(args->appl.if_names[i], pool); >> >> >> >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) >> >> >> >> if (args->appl.mode == APPL_MODE_PING) { >> >> odp_cpumask_t cpu0_mask; >> >> + odp_queue_t tq; >> >> >> >> /* Previous code forced both threads to >> CPU 0 */ >> >> odp_cpumask_zero(&cpu0_mask); >> >> odp_cpumask_set(&cpu0_mask, 0); >> >> >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[1].pktio_dev = args->appl.if_names[0]; >> >> args->thread[1].pool = pool; >> >> + args->thread[1].tp = tp; >> >> + args->thread[1].tq = tq; >> >> + args->thread[1].tim = >> odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[1].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[1].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[1].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, >> >> gen_recv_thread, >> >> &args->thread[1]); >> >> >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[0].pktio_dev = args->appl.if_names[0]; >> >> args->thread[0].pool = pool; >> >> + args->thread[0].tp = tp; >> >> + args->thread[0].tq = tq; >> >> + args->thread[0].tim = >> odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[0].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[0].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[0].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, >> >> gen_send_thread, >> >> &args->thread[0]); >> >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t thd_mask; >> >> void *(*thr_run_func) (void *); >> >> int if_idx; >> >> + odp_queue_t tq; >> >> >> >> if_idx = i % args->appl.if_count; >> >> >> >> args->thread[i].pktio_dev = >> >> args->appl.if_names[if_idx]; >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[i].pool = pool; >> >> + args->thread[i].tp = tp; >> >> + args->thread[i].tq = tq; >> >> + args->thread[i].tim = odp_timer_alloc(tp, tq, >> >> NULL); >> >> + if (args->thread[i].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[i].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[i].mode = args->appl.mode; >> >> >> >> if (args->appl.mode == >> APPL_MODE_UDP) { >> >> -- >> >> 1.9.1 >> >> >> >> >> >> _______________________________________________ >> >> lng-odp mailing list >> >> lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org> >> <mailto:lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org>> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >> > >> >> >> >> >> >
On 03/24/15 16:26, Mike Holmes wrote: > > > On 24 March 2015 at 06:51, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 03/24/15 12:46, Ola Liljedahl wrote: > > On 4 March 2015 at 15:17, Ola Liljedahl > <ola.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org> > <mailto:ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>>> wrote: > > It could be good to add the bug tracker ID or link to the > commit > message when merging. > https://bugs.linaro.org/show_bug.cgi?id=1025 > > > On 4 March 2015 at 04:54, Bill Fischofer > <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org> > <mailto:bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>>> wrote: > > > > > > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl > <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org> > <mailto:ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>>> > > wrote: > >> > >> Use ODP timer facility instead of POSIX sleep/nanosleep. > >> > >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org> > <mailto:ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>>> > > > > > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org> > <mailto:bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>>> > > Maxim this patch is reviewed but doesn't seem to be merged. > I asked whether you could add the bug tracker id or link to > the commit message. Or should I post a new version of the > patch with this included? > > -- Ola > > > That is good but I have 2 things to note: > 1. coding style of millisleep() should be the same. (i.e. vars on > top of the scope.) > 2. It looks like we need to replace sleep() usleep() functions in > other places also. It will be easy if we can create odp helper for > timers with syntax as match as possible to posix. > > It might be: > odph_posix_timers_init() > > odph_timer_sleep(); > odph_timer_nanosleep() > > odph_posix_timers_destroy(). > > Then it will be very fast and clear update to examples and > definitely will speed up apps development. What do you think about > that? > > > I dont like adding OS portability to the ODP API or its helpers, ODP > should be an abstraction for networking acceleration only. > > POSIX is supposed to be such an OS wrapper, do we want to wrap a wrapper ? My point is that for new users of ODP it's easy to write application using more posix style: odph_sleep(unsigned int seconds); odph_nanosleep(const struct timespec *req, struct timespec *rem); Everybody knows that functions and it's good example how to port original application to odp. Also for example we have some big app, like Snort. And we want to make it use odp timers. We can just rename all sleep() to odph_sleep() and it will be very clear what this function does. (Compare it to create timer pool, queue and poll that queue and etc...). Maxim. > > Thanks, > Maxim. > > > > > > >> > >> --- > >> (This document/code contribution attached is provided > under the > terms of > >> agreement LES-LTM-21309) > >> > >> example/generator/odp_generator.c | 105 > >> +++++++++++++++++++++++++++++++++++--- > >> 1 file changed, 98 insertions(+), 7 deletions(-) > >> > >> diff --git a/example/generator/odp_generator.c > >> b/example/generator/odp_generator.c > >> index 3870fd1..9147c15 100644 > >> --- a/example/generator/odp_generator.c > >> +++ b/example/generator/odp_generator.c > >> @@ -79,6 +79,10 @@ static struct { > >> typedef struct { > >> char *pktio_dev; /**< Interface name to > use */ > >> odp_pool_t pool; /**< Pool for packet IO */ > >> + odp_timer_pool_t tp; /**< Timer pool handle */ > >> + odp_queue_t tq; /**< Queue for timeouts */ > >> + odp_timer_t tim; /**< Timer handle */ > >> + odp_timeout_t tmo_ev; /**< Timeout event */ > >> int mode; /**< Thread mode */ > >> } thread_args_t; > >> > >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, > odph_ethaddr_t *des); > >> static void tv_sub(struct timeval *recvtime, struct > timeval > *sendtime); > >> > >> /** > >> + * Sleep for the specified amount of milliseconds > >> + * Use ODP timer, busy wait until timer expired and > timeout event > >> received > >> + */ > >> +static void millisleep(uint32_t ms, > >> + odp_timer_pool_t tp, > >> + odp_timer_t tim, > >> + odp_queue_t q, > >> + odp_timeout_t tmo) > >> +{ > >> + uint64_t ticks = odp_timer_ns_to_tick(tp, > 1000000ULL * ms); > >> + odp_event_t ev = odp_timeout_to_event(tmo); > >> + int rc = odp_timer_set_rel(tim, ticks, &ev); > >> + if (rc != ODP_TIMER_SUCCESS) > >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); > >> + /* Spin waiting for timeout event */ > >> + while ((ev = odp_queue_deq(q)) == > ODP_EVENT_INVALID) > >> + (void)0; > >> +} > >> + > >> +/** > >> * Scan ip > >> * Parse ip address. > >> * > >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void > *arg) > >> thr, > >> odp_atomic_load_u64(&counters.seq), > >> odp_atomic_load_u64(&counters.seq)%0xffff); > >> - /* TODO use odp timer */ > >> - struct timespec ts; > >> - ts.tv_sec = 0; > >> - ts.tv_nsec = args->appl.interval; > >> - nanosleep(&ts, NULL); > >> + millisleep(args->appl.interval, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> + > >> } > >> if (args->appl.number != -1 && > >> odp_atomic_load_u64(&counters.seq) > >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void > *arg) > >> if > (odp_atomic_load_u64(&counters.icmp) >= > >> (unsigned > int)args->appl.number) > >> break; > >> - /* TODO use odp timer */ > >> - sleep(1); > >> + millisleep(1000, > >> + thr_args->tp, > >> + thr_args->tim, > >> + thr_args->tq, > >> + thr_args->tmo_ev); > >> args->appl.timeout--; > >> } > >> } > >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t cpumask; > >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; > >> odp_pool_param_t params; > >> + odp_timer_pool_param_t tparams; > >> + odp_timer_pool_t tp; > >> + odp_pool_t tmop; > >> > >> /* Init ODP before calling anything else */ > >> if (odp_init_global(NULL, NULL)) { > >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) > >> } > >> odp_pool_print(pool); > >> > >> + /* Create timer pool */ > >> + tparams.res_ns = 1 * ODP_TIME_MSEC; > >> + tparams.min_tmo = 0; > >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; > >> + tparams.num_timers = num_workers; /* One timer per > worker */ > >> + tparams.priv = 0; /* Shared */ > >> + tparams.clk_src = ODP_CLOCK_CPU; > >> + tp = odp_timer_pool_create("timer_pool", &tparams); > >> + if (tp == ODP_TIMER_POOL_INVALID) { > >> + EXAMPLE_ERR("Timer pool create failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> + odp_timer_pool_start(); > >> + > >> + /* Create timeout pool */ > >> + memset(¶ms, 0, sizeof(params)); > >> + params.tmo.num = tparams.num_timers; /* One > timeout > per timer > >> */ > >> + params.type = ODP_POOL_TIMEOUT; > >> + > >> + tmop = odp_pool_create("timeout_pool", > ODP_SHM_NULL, > ¶ms); > >> + > >> + if (pool == ODP_POOL_INVALID) { > >> + EXAMPLE_ERR("Error: packet pool create > failed.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> for (i = 0; i < args->appl.if_count; ++i) > >> create_pktio(args->appl.if_names[i], pool); > >> > >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) > >> > >> if (args->appl.mode == APPL_MODE_PING) { > >> odp_cpumask_t cpu0_mask; > >> + odp_queue_t tq; > >> > >> /* Previous code forced both threads to > CPU 0 */ > >> odp_cpumask_zero(&cpu0_mask); > >> odp_cpumask_set(&cpu0_mask, 0); > >> > >> + tq = odp_queue_create("", > ODP_QUEUE_TYPE_POLL, > NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[1].pktio_dev = args->appl.if_names[0]; > >> args->thread[1].pool = pool; > >> + args->thread[1].tp = tp; > >> + args->thread[1].tq = tq; > >> + args->thread[1].tim = > odp_timer_alloc(tp, tq, > NULL); > >> + if (args->thread[1].tim == > ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[1].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[1].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, > >> gen_recv_thread, > >> &args->thread[1]); > >> > >> + tq = odp_queue_create("", > ODP_QUEUE_TYPE_POLL, > NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[0].pktio_dev = args->appl.if_names[0]; > >> args->thread[0].pool = pool; > >> + args->thread[0].tp = tp; > >> + args->thread[0].tq = tq; > >> + args->thread[0].tim = > odp_timer_alloc(tp, tq, > NULL); > >> + if (args->thread[0].tim == > ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[0].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[0].mode = args->appl.mode; > >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, > >> gen_send_thread, > >> &args->thread[0]); > >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) > >> odp_cpumask_t thd_mask; > >> void *(*thr_run_func) (void *); > >> int if_idx; > >> + odp_queue_t tq; > >> > >> if_idx = i % args->appl.if_count; > >> > >> args->thread[i].pktio_dev = > >> args->appl.if_names[if_idx]; > >> + tq = odp_queue_create("", > ODP_QUEUE_TYPE_POLL, > >> NULL); > >> + if (tq == ODP_QUEUE_INVALID) > >> + abort(); > >> args->thread[i].pool = pool; > >> + args->thread[i].tp = tp; > >> + args->thread[i].tq = tq; > >> + args->thread[i].tim = odp_timer_alloc(tp, tq, > >> NULL); > >> + if (args->thread[i].tim == > ODP_TIMER_INVALID) > >> + abort(); > >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); > >> + if (args->thread[i].tmo_ev == > ODP_TIMEOUT_INVALID) > >> + abort(); > >> args->thread[i].mode = args->appl.mode; > >> > >> if (args->appl.mode == > APPL_MODE_UDP) { > >> -- > >> 1.9.1 > >> > >> > >> _______________________________________________ > >> lng-odp mailing list > >> lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org> > <mailto:lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org>> > >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs >
On 24 March 2015 at 14:50, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/24/15 16:26, Mike Holmes wrote: > >> >> >> On 24 March 2015 at 06:51, Maxim Uvarov <maxim.uvarov@linaro.org <mailto: >> maxim.uvarov@linaro.org>> wrote: >> >> On 03/24/15 12:46, Ola Liljedahl wrote: >> >> On 4 March 2015 at 15:17, Ola Liljedahl >> <ola.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org> >> <mailto:ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>>> wrote: >> >> It could be good to add the bug tracker ID or link to the >> commit >> message when merging. >> https://bugs.linaro.org/show_bug.cgi?id=1025 >> >> >> On 4 March 2015 at 04:54, Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org> >> <mailto:bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>>> wrote: >> > >> > >> > On Tue, Mar 3, 2015 at 10:19 AM, Ola Liljedahl >> <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org> >> <mailto:ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>>> >> > wrote: >> >> >> >> Use ODP timer facility instead of POSIX sleep/nanosleep. >> >> >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org> >> <mailto:ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>>> >> > >> > >> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org> >> <mailto:bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>>> >> >> Maxim this patch is reviewed but doesn't seem to be merged. >> I asked whether you could add the bug tracker id or link to >> the commit message. Or should I post a new version of the >> patch with this included? >> >> -- Ola >> >> >> That is good but I have 2 things to note: >> 1. coding style of millisleep() should be the same. (i.e. vars on >> top of the scope.) >> 2. It looks like we need to replace sleep() usleep() functions in >> other places also. It will be easy if we can create odp helper for >> timers with syntax as match as possible to posix. >> >> It might be: >> odph_posix_timers_init() >> >> odph_timer_sleep(); >> odph_timer_nanosleep() >> >> odph_posix_timers_destroy(). >> >> Then it will be very fast and clear update to examples and >> definitely will speed up apps development. What do you think about >> that? >> >> >> I dont like adding OS portability to the ODP API or its helpers, ODP >> should be an abstraction for networking acceleration only. >> >> POSIX is supposed to be such an OS wrapper, do we want to wrap a wrapper ? >> > > My point is that for new users of ODP it's easy to write application using > more posix style: > odph_sleep(unsigned int seconds); > odph_nanosleep(const struct timespec *req, struct timespec *rem); > Everybody knows that functions and it's good example how to port original > application to odp. > Yes but that's really poor style for ODP application which should only block in odp_schedule() (or equivalent calls). > Also for example we have some big app, like Snort. And we want to make it > use odp timers. We can just > rename all sleep() to odph_sleep() and it will be very clear what this > function does. (Compare it to create timer pool, > queue and poll that queue and etc...). > Better to make such legacy applications proper ODP event driven applications. > Maxim. > > > >> Thanks, >> Maxim. >> >> >> >> > >> >> >> >> --- >> >> (This document/code contribution attached is provided >> under the >> terms of >> >> agreement LES-LTM-21309) >> >> >> >> example/generator/odp_generator.c | 105 >> >> +++++++++++++++++++++++++++++++++++--- >> >> 1 file changed, 98 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/example/generator/odp_generator.c >> >> b/example/generator/odp_generator.c >> >> index 3870fd1..9147c15 100644 >> >> --- a/example/generator/odp_generator.c >> >> +++ b/example/generator/odp_generator.c >> >> @@ -79,6 +79,10 @@ static struct { >> >> typedef struct { >> >> char *pktio_dev; /**< Interface name to >> use */ >> >> odp_pool_t pool; /**< Pool for packet IO */ >> >> + odp_timer_pool_t tp; /**< Timer pool handle */ >> >> + odp_queue_t tq; /**< Queue for timeouts */ >> >> + odp_timer_t tim; /**< Timer handle */ >> >> + odp_timeout_t tmo_ev; /**< Timeout event */ >> >> int mode; /**< Thread mode */ >> >> } thread_args_t; >> >> >> >> @@ -104,6 +108,26 @@ static int scan_mac(char *in, >> odph_ethaddr_t *des); >> >> static void tv_sub(struct timeval *recvtime, struct >> timeval >> *sendtime); >> >> >> >> /** >> >> + * Sleep for the specified amount of milliseconds >> >> + * Use ODP timer, busy wait until timer expired and >> timeout event >> >> received >> >> + */ >> >> +static void millisleep(uint32_t ms, >> >> + odp_timer_pool_t tp, >> >> + odp_timer_t tim, >> >> + odp_queue_t q, >> >> + odp_timeout_t tmo) >> >> +{ >> >> + uint64_t ticks = odp_timer_ns_to_tick(tp, >> 1000000ULL * ms); >> >> + odp_event_t ev = odp_timeout_to_event(tmo); >> >> + int rc = odp_timer_set_rel(tim, ticks, &ev); >> >> + if (rc != ODP_TIMER_SUCCESS) >> >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >> >> + /* Spin waiting for timeout event */ >> >> + while ((ev = odp_queue_deq(q)) == >> ODP_EVENT_INVALID) >> >> + (void)0; >> >> +} >> >> + >> >> +/** >> >> * Scan ip >> >> * Parse ip address. >> >> * >> >> @@ -400,11 +424,12 @@ static void *gen_send_thread(void >> *arg) >> >> thr, >> >> odp_atomic_load_u64(&counters.seq), >> >> odp_atomic_load_u64(&counters.seq)%0xffff); >> >> - /* TODO use odp timer */ >> >> - struct timespec ts; >> >> - ts.tv_sec = 0; >> >> - ts.tv_nsec = args->appl.interval; >> >> - nanosleep(&ts, NULL); >> >> + millisleep(args->appl.interval, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> + >> >> } >> >> if (args->appl.number != -1 && >> >> odp_atomic_load_u64(&counters.seq) >> >> @@ -419,8 +444,11 @@ static void *gen_send_thread(void >> *arg) >> >> if >> (odp_atomic_load_u64(&counters.icmp) >= >> >> (unsigned >> int)args->appl.number) >> >> break; >> >> - /* TODO use odp timer */ >> >> - sleep(1); >> >> + millisleep(1000, >> >> + thr_args->tp, >> >> + thr_args->tim, >> >> + thr_args->tq, >> >> + thr_args->tmo_ev); >> >> args->appl.timeout--; >> >> } >> >> } >> >> @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t cpumask; >> >> char cpumaskstr[ODP_CPUMASK_STR_SIZE]; >> >> odp_pool_param_t params; >> >> + odp_timer_pool_param_t tparams; >> >> + odp_timer_pool_t tp; >> >> + odp_pool_t tmop; >> >> >> >> /* Init ODP before calling anything else */ >> >> if (odp_init_global(NULL, NULL)) { >> >> @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) >> >> } >> >> odp_pool_print(pool); >> >> >> >> + /* Create timer pool */ >> >> + tparams.res_ns = 1 * ODP_TIME_MSEC; >> >> + tparams.min_tmo = 0; >> >> + tparams.max_tmo = 10000 * ODP_TIME_SEC; >> >> + tparams.num_timers = num_workers; /* One timer per >> worker */ >> >> + tparams.priv = 0; /* Shared */ >> >> + tparams.clk_src = ODP_CLOCK_CPU; >> >> + tp = odp_timer_pool_create("timer_pool", >> &tparams); >> >> + if (tp == ODP_TIMER_POOL_INVALID) { >> >> + EXAMPLE_ERR("Timer pool create failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> + odp_timer_pool_start(); >> >> + >> >> + /* Create timeout pool */ >> >> + memset(¶ms, 0, sizeof(params)); >> >> + params.tmo.num = tparams.num_timers; /* One >> timeout >> per timer >> >> */ >> >> + params.type = ODP_POOL_TIMEOUT; >> >> + >> >> + tmop = odp_pool_create("timeout_pool", >> ODP_SHM_NULL, >> ¶ms); >> >> + >> >> + if (pool == ODP_POOL_INVALID) { >> >> + EXAMPLE_ERR("Error: packet pool create >> failed.\n"); >> >> + exit(EXIT_FAILURE); >> >> + } >> >> for (i = 0; i < args->appl.if_count; ++i) >> >> create_pktio(args->appl.if_names[i], pool); >> >> >> >> @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) >> >> >> >> if (args->appl.mode == APPL_MODE_PING) { >> >> odp_cpumask_t cpu0_mask; >> >> + odp_queue_t tq; >> >> >> >> /* Previous code forced both threads to >> CPU 0 */ >> >> odp_cpumask_zero(&cpu0_mask); >> >> odp_cpumask_set(&cpu0_mask, 0); >> >> >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[1].pktio_dev = args->appl.if_names[0]; >> >> args->thread[1].pool = pool; >> >> + args->thread[1].tp = tp; >> >> + args->thread[1].tq = tq; >> >> + args->thread[1].tim = >> odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[1].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[1].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[1].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, >> >> gen_recv_thread, >> >> &args->thread[1]); >> >> >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[0].pktio_dev = args->appl.if_names[0]; >> >> args->thread[0].pool = pool; >> >> + args->thread[0].tp = tp; >> >> + args->thread[0].tq = tq; >> >> + args->thread[0].tim = >> odp_timer_alloc(tp, tq, >> NULL); >> >> + if (args->thread[0].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[0].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[0].mode = args->appl.mode; >> >> odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, >> >> gen_send_thread, >> >> &args->thread[0]); >> >> @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) >> >> odp_cpumask_t thd_mask; >> >> void *(*thr_run_func) (void *); >> >> int if_idx; >> >> + odp_queue_t tq; >> >> >> >> if_idx = i % args->appl.if_count; >> >> >> >> args->thread[i].pktio_dev = >> >> args->appl.if_names[if_idx]; >> >> + tq = odp_queue_create("", >> ODP_QUEUE_TYPE_POLL, >> >> NULL); >> >> + if (tq == ODP_QUEUE_INVALID) >> >> + abort(); >> >> args->thread[i].pool = pool; >> >> + args->thread[i].tp = tp; >> >> + args->thread[i].tq = tq; >> >> + args->thread[i].tim = odp_timer_alloc(tp, tq, >> >> NULL); >> >> + if (args->thread[i].tim == >> ODP_TIMER_INVALID) >> >> + abort(); >> >> + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); >> >> + if (args->thread[i].tmo_ev == >> ODP_TIMEOUT_INVALID) >> >> + abort(); >> >> args->thread[i].mode = args->appl.mode; >> >> >> >> if (args->appl.mode == >> APPL_MODE_UDP) { >> >> -- >> >> 1.9.1 >> >> >> >> >> >> _______________________________________________ >> >> lng-odp mailing list >> >> lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org> >> <mailto:lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org>> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >> > >> >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM >> SoCs >> >> >
> > snip > > > My point is that for new users of ODP it's easy to write application using > more posix style: > odph_sleep(unsigned int seconds); > odph_nanosleep(const struct timespec *req, struct timespec *rem); > Everybody knows that functions and it's good example how to port original > application to odp. > Also for example we have some big app, like Snort. And we want to make it > use odp timers. We can just > rename all sleep() to odph_sleep() and it will be very clear what this > function does. (Compare it to create timer pool, > queue and poll that queue and etc...). > > Maxim. > > Snort etc is new topic along with odph_nanosleep, we can have that discussion but is this patch addressing the bug as described ? Should we start a new thread with fresh proposal for a new API Maxim - there is nothing to say we dont iterate again if that discussion is successful after debate.
On 03/24/15 17:21, Mike Holmes wrote: > > snip > > My point is that for new users of ODP it's easy to write > application using more posix style: > odph_sleep(unsigned int seconds); > odph_nanosleep(const struct timespec *req, struct timespec *rem); > Everybody knows that functions and it's good example how to port > original application to odp. > Also for example we have some big app, like Snort. And we want to > make it use odp timers. We can just > rename all sleep() to odph_sleep() and it will be very clear what > this function does. (Compare it to create timer pool, > queue and poll that queue and etc...). > > Maxim. > > > > Snort etc is new topic along with odph_nanosleep, we can have that > discussion but is this patch addressing the bug as described ? > > Should we start a new thread with fresh proposal for a new API Maxim - > there is nothing to say we dont iterate again if that discussion is > successful after debate. > yes, it is not blocker for current patch. Maxim. > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs >
Ola, you did not replay on comment bellow. also int rc, uint64_t ticks and odp_event_t ev here should be defined at the beginning of function. Thanks, Maxim. On 03/24/15 16:18, Maxim Uvarov wrote: > On 03/03/15 19:19, Ola Liljedahl wrote: >> +static void millisleep(uint32_t ms, >> + odp_timer_pool_t tp, >> + odp_timer_t tim, >> + odp_queue_t q, >> + odp_timeout_t tmo) >> +{ >> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); >> + odp_event_t ev = odp_timeout_to_event(tmo); >> + int rc = odp_timer_set_rel(tim, ticks, &ev); >> + if (rc != ODP_TIMER_SUCCESS) >> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >> + /* Spin waiting for timeout event */ >> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) >> + (void)0; >> +} > > In C++ (void)0 will be static_cast<void>. If we plan to support C++ > compiler then it might be reason to avoid that. > odp_spin() looks like more suitable for that case. > > Maxim. >
On 27 March 2015 at 16:45, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Ola, you did not replay on comment bellow. > I think I did. > > also int rc, uint64_t ticks and odp_event_t ev here should be defined at > the beginning of function. > These variables *are* defined at the beginning of the function. They are also initialized. Is there something wrong with that? I assume you mean the code has to be written as uint64_t ticks; odp_event_t ev; int rc; ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); ev = odp_timeout_to_event(tmo); rc = odp_timer_set_rel(tim, ticks, &ev); but is there any actual requirement for this style? I see lots of variable declarations with initializations that use function calls in the Linux kernel. Just a random sample: /usr/src/linux-source-3.13.0/linux-source-3.13.0/kernel/exit.c static int wait_consider_task(struct wait_opts *wo, int ptrace, struct task_struct *p) { int ret = eligible_child(wo, p); if (!ret) return ret; > Thanks, > Maxim. > > On 03/24/15 16:18, Maxim Uvarov wrote: > >> On 03/03/15 19:19, Ola Liljedahl wrote: >> >>> +static void millisleep(uint32_t ms, >>> + odp_timer_pool_t tp, >>> + odp_timer_t tim, >>> + odp_queue_t q, >>> + odp_timeout_t tmo) >>> +{ >>> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); >>> + odp_event_t ev = odp_timeout_to_event(tmo); >>> + int rc = odp_timer_set_rel(tim, ticks, &ev); >>> + if (rc != ODP_TIMER_SUCCESS) >>> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >>> + /* Spin waiting for timeout event */ >>> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) >>> + (void)0; >>> +} >>> >> >> In C++ (void)0 will be static_cast<void>. If we plan to support C++ >> compiler then it might be reason to avoid that. >> odp_spin() looks like more suitable for that case. >> > We won't compile this example with a C++ compiler. Users may chose to write *their* applications in C++. > >> Maxim. >> >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
Maxim - lets you and I look at this on Monday and un-block it. On 27 March 2015 at 15:05, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 27 March 2015 at 16:45, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > >> Ola, you did not replay on comment bellow. >> > I think I did. > >> >> also int rc, uint64_t ticks and odp_event_t ev here should be defined at >> the beginning of function. >> > These variables *are* defined at the beginning of the function. They are > also initialized. Is there something wrong with that? > > I assume you mean the code has to be written as > uint64_t ticks; > odp_event_t ev; > int rc; > > ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); > ev = odp_timeout_to_event(tmo); > rc = odp_timer_set_rel(tim, ticks, &ev); > > but is there any actual requirement for this style? I see lots of variable > declarations with initializations that use function calls in the Linux > kernel. > > Just a random sample: > /usr/src/linux-source-3.13.0/linux-source-3.13.0/kernel/exit.c > static int wait_consider_task(struct wait_opts *wo, int ptrace, > struct task_struct *p) > { > int ret = eligible_child(wo, p); > if (!ret) > return ret; > > > > >> Thanks, >> Maxim. >> >> On 03/24/15 16:18, Maxim Uvarov wrote: >> >>> On 03/03/15 19:19, Ola Liljedahl wrote: >>> >>>> +static void millisleep(uint32_t ms, >>>> + odp_timer_pool_t tp, >>>> + odp_timer_t tim, >>>> + odp_queue_t q, >>>> + odp_timeout_t tmo) >>>> +{ >>>> + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); >>>> + odp_event_t ev = odp_timeout_to_event(tmo); >>>> + int rc = odp_timer_set_rel(tim, ticks, &ev); >>>> + if (rc != ODP_TIMER_SUCCESS) >>>> + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); >>>> + /* Spin waiting for timeout event */ >>>> + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) >>>> + (void)0; >>>> +} >>>> >>> >>> In C++ (void)0 will be static_cast<void>. If we plan to support C++ >>> compiler then it might be reason to avoid that. >>> odp_spin() looks like more suitable for that case. >>> >> We won't compile this example with a C++ compiler. Users may chose to > write *their* applications in C++. > > > >> >>> Maxim. >>> >>> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c index 3870fd1..9147c15 100644 --- a/example/generator/odp_generator.c +++ b/example/generator/odp_generator.c @@ -79,6 +79,10 @@ static struct { typedef struct { char *pktio_dev; /**< Interface name to use */ odp_pool_t pool; /**< Pool for packet IO */ + odp_timer_pool_t tp; /**< Timer pool handle */ + odp_queue_t tq; /**< Queue for timeouts */ + odp_timer_t tim; /**< Timer handle */ + odp_timeout_t tmo_ev; /**< Timeout event */ int mode; /**< Thread mode */ } thread_args_t; @@ -104,6 +108,26 @@ static int scan_mac(char *in, odph_ethaddr_t *des); static void tv_sub(struct timeval *recvtime, struct timeval *sendtime); /** + * Sleep for the specified amount of milliseconds + * Use ODP timer, busy wait until timer expired and timeout event received + */ +static void millisleep(uint32_t ms, + odp_timer_pool_t tp, + odp_timer_t tim, + odp_queue_t q, + odp_timeout_t tmo) +{ + uint64_t ticks = odp_timer_ns_to_tick(tp, 1000000ULL * ms); + odp_event_t ev = odp_timeout_to_event(tmo); + int rc = odp_timer_set_rel(tim, ticks, &ev); + if (rc != ODP_TIMER_SUCCESS) + EXAMPLE_ABORT("odp_timer_set_rel() failed\n"); + /* Spin waiting for timeout event */ + while ((ev = odp_queue_deq(q)) == ODP_EVENT_INVALID) + (void)0; +} + +/** * Scan ip * Parse ip address. * @@ -400,11 +424,12 @@ static void *gen_send_thread(void *arg) thr, odp_atomic_load_u64(&counters.seq), odp_atomic_load_u64(&counters.seq)%0xffff); - /* TODO use odp timer */ - struct timespec ts; - ts.tv_sec = 0; - ts.tv_nsec = args->appl.interval; - nanosleep(&ts, NULL); + millisleep(args->appl.interval, + thr_args->tp, + thr_args->tim, + thr_args->tq, + thr_args->tmo_ev); + } if (args->appl.number != -1 && odp_atomic_load_u64(&counters.seq) @@ -419,8 +444,11 @@ static void *gen_send_thread(void *arg) if (odp_atomic_load_u64(&counters.icmp) >= (unsigned int)args->appl.number) break; - /* TODO use odp timer */ - sleep(1); + millisleep(1000, + thr_args->tp, + thr_args->tim, + thr_args->tq, + thr_args->tmo_ev); args->appl.timeout--; } } @@ -567,6 +595,9 @@ int main(int argc, char *argv[]) odp_cpumask_t cpumask; char cpumaskstr[ODP_CPUMASK_STR_SIZE]; odp_pool_param_t params; + odp_timer_pool_param_t tparams; + odp_timer_pool_t tp; + odp_pool_t tmop; /* Init ODP before calling anything else */ if (odp_init_global(NULL, NULL)) { @@ -637,6 +668,31 @@ int main(int argc, char *argv[]) } odp_pool_print(pool); + /* Create timer pool */ + tparams.res_ns = 1 * ODP_TIME_MSEC; + tparams.min_tmo = 0; + tparams.max_tmo = 10000 * ODP_TIME_SEC; + tparams.num_timers = num_workers; /* One timer per worker */ + tparams.priv = 0; /* Shared */ + tparams.clk_src = ODP_CLOCK_CPU; + tp = odp_timer_pool_create("timer_pool", &tparams); + if (tp == ODP_TIMER_POOL_INVALID) { + EXAMPLE_ERR("Timer pool create failed.\n"); + exit(EXIT_FAILURE); + } + odp_timer_pool_start(); + + /* Create timeout pool */ + memset(¶ms, 0, sizeof(params)); + params.tmo.num = tparams.num_timers; /* One timeout per timer */ + params.type = ODP_POOL_TIMEOUT; + + tmop = odp_pool_create("timeout_pool", ODP_SHM_NULL, ¶ms); + + if (pool == ODP_POOL_INVALID) { + EXAMPLE_ERR("Error: packet pool create failed.\n"); + exit(EXIT_FAILURE); + } for (i = 0; i < args->appl.if_count; ++i) create_pktio(args->appl.if_names[i], pool); @@ -645,19 +701,42 @@ int main(int argc, char *argv[]) if (args->appl.mode == APPL_MODE_PING) { odp_cpumask_t cpu0_mask; + odp_queue_t tq; /* Previous code forced both threads to CPU 0 */ odp_cpumask_zero(&cpu0_mask); odp_cpumask_set(&cpu0_mask, 0); + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); + if (tq == ODP_QUEUE_INVALID) + abort(); args->thread[1].pktio_dev = args->appl.if_names[0]; args->thread[1].pool = pool; + args->thread[1].tp = tp; + args->thread[1].tq = tq; + args->thread[1].tim = odp_timer_alloc(tp, tq, NULL); + if (args->thread[1].tim == ODP_TIMER_INVALID) + abort(); + args->thread[1].tmo_ev = odp_timeout_alloc(tmop); + if (args->thread[1].tmo_ev == ODP_TIMEOUT_INVALID) + abort(); args->thread[1].mode = args->appl.mode; odph_linux_pthread_create(&thread_tbl[1], &cpu0_mask, gen_recv_thread, &args->thread[1]); + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); + if (tq == ODP_QUEUE_INVALID) + abort(); args->thread[0].pktio_dev = args->appl.if_names[0]; args->thread[0].pool = pool; + args->thread[0].tp = tp; + args->thread[0].tq = tq; + args->thread[0].tim = odp_timer_alloc(tp, tq, NULL); + if (args->thread[0].tim == ODP_TIMER_INVALID) + abort(); + args->thread[0].tmo_ev = odp_timeout_alloc(tmop); + if (args->thread[0].tmo_ev == ODP_TIMEOUT_INVALID) + abort(); args->thread[0].mode = args->appl.mode; odph_linux_pthread_create(&thread_tbl[0], &cpu0_mask, gen_send_thread, &args->thread[0]); @@ -670,11 +749,23 @@ int main(int argc, char *argv[]) odp_cpumask_t thd_mask; void *(*thr_run_func) (void *); int if_idx; + odp_queue_t tq; if_idx = i % args->appl.if_count; args->thread[i].pktio_dev = args->appl.if_names[if_idx]; + tq = odp_queue_create("", ODP_QUEUE_TYPE_POLL, NULL); + if (tq == ODP_QUEUE_INVALID) + abort(); args->thread[i].pool = pool; + args->thread[i].tp = tp; + args->thread[i].tq = tq; + args->thread[i].tim = odp_timer_alloc(tp, tq, NULL); + if (args->thread[i].tim == ODP_TIMER_INVALID) + abort(); + args->thread[i].tmo_ev = odp_timeout_alloc(tmop); + if (args->thread[i].tmo_ev == ODP_TIMEOUT_INVALID) + abort(); args->thread[i].mode = args->appl.mode; if (args->appl.mode == APPL_MODE_UDP) {
Use ODP timer facility instead of POSIX sleep/nanosleep. Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- (This document/code contribution attached is provided under the terms of agreement LES-LTM-21309) example/generator/odp_generator.c | 105 +++++++++++++++++++++++++++++++++++--- 1 file changed, 98 insertions(+), 7 deletions(-)