| Message ID | 1438679277-28178-1-git-send-email-balakrishna.garapati@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 04, 2015 at 11:07:57AM +0200, Balakrishna.Garapati wrote: > Signed-off-by: Balakrishna.Garapati <balakrishna.garapati@linaro.org> > --- > Added packet rate stats > example/generator/odp_generator.c | 67 ++++++++++++++++++++++++++++++++------- > 1 file changed, 56 insertions(+), 11 deletions(-) > > diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c > index bdee222..aee6e7e 100644 > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -101,6 +101,7 @@ static void usage(char *progname); > static int scan_ip(char *buf, unsigned int *paddr); > static int scan_mac(char *in, odph_ethaddr_t *des); > static void tv_sub(struct timeval *recvtime, struct timeval *sendtime); > +static void print_global_stats(void); > > /** > * Sleep for the specified amount of milliseconds > @@ -371,7 +372,6 @@ static odp_pktio_t create_pktio(const char *dev, odp_pool_t pool) > static void *gen_send_thread(void *arg) > { > int thr; > - uint64_t start, now, diff; > odp_pktio_t pktio; > thread_args_t *thr_args; > odp_queue_t outq_def; > @@ -393,7 +393,6 @@ static void *gen_send_thread(void *arg) > return NULL; > } > > - start = odp_time_cycles(); > printf(" [%02i] created mode: SEND\n", thr); > for (;;) { > int err; > @@ -434,15 +433,6 @@ static void *gen_send_thread(void *arg) > >= (unsigned int)args->appl.number) { > break; > } > - > - now = odp_time_cycles(); > - diff = odp_time_diff_cycles(start, now); > - if (odp_time_cycles_to_ns(diff) > 20 * ODP_TIME_SEC) { > - start = odp_time_cycles(); > - printf(" [%02i] total send: %ju\n", > - thr, odp_atomic_load_u64(&counters.seq)); > - fflush(stdout); > - } > } > > /* receive number of reply pks until timeout */ > @@ -589,6 +579,59 @@ static void *gen_recv_thread(void *arg) > > return arg; > } > + > +/** > + * printing verbose statistics > + * > + */ > +static void print_global_stats(void) > +{ > + uint64_t start, now, diff; > + uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0; > + int loop_forever = 1, verbose_interval = 20; > + > + if (args->appl.timeout != -1 || args->appl.number > 0) > + loop_forever = 0; > + > + start = odp_time_cycles(); > + do { > + pkts = 0; > + now = odp_time_cycles(); > + diff = odp_time_diff_cycles(start, now); > + if (odp_time_cycles_to_ns(diff) > > + verbose_interval * ODP_TIME_SEC) { If you check < instead here and "continue;" the indentation level of the reset of the loop would be reduced. > + start = odp_time_cycles(); > + > + if (odp_atomic_load_u64(&counters.seq) > 0) { > + pkts = odp_atomic_load_u64(&counters.seq); Move this assignment outside of the condition to avoid loading the atomic twice. Actually I'm not sure about the condition at all, if you're in _UDP mode don't you want to print the number of sent packets even if it's 0? > + printf(" total send: %ju\n", pkts); PRIu64 as is used elsewhere > + } > + > + pps = (pkts - pkts_prev) / verbose_interval; > + if (pps > maximum_pps) > + maximum_pps = pps; > + > + printf(" %" PRIu64 " pps, %" PRIu64 " max pps \n", > + pps, maximum_pps); checkpatch -> WARNING: unnecessary whitespace before a quoted newline Also this print is only valid for _UDP mode but it will be printed (and always 0) in _RCV mode too. > + > + if (args->appl.mode == APPL_MODE_RCV || > + odp_atomic_load_u64(&counters.icmp) != 0 || > + odp_atomic_load_u64(&counters.udp) != 0) { > + printf(" total receive(icmp:%ju, udp:%ju)\n", PRIu64 > + odp_atomic_load_u64(&counters.icmp), > + odp_atomic_load_u64(&counters.udp)); At the minute this output is flooded by the "receive Packet proto:IP .." lines that get printed unconditionally for every packet, I think they need go except for in ICMP mode. Also it would be better to print *either* UDP or ICMP counters rather than both, only one will be non-zero. > + } > + > + if (args->appl.number > 0 && > + odp_atomic_load_u64(&counters.icmp) >= > + (unsigned int)args->appl.number) > + break; This logic and the loop condition logic in general seems fragile. I think you'd get stuck in this loop if one of the worker threads aborted early (e.g. due to a odp_queue_enq() failure). It would be better to have a simpler condition for this loop that indicated when all of the workers had completed and leave the timeout/number/counters logic in the gen/send threads. This could be done using odp_thrmask_worker() and odp_thrmask_count(). > + > + pkts_prev = pkts; > + } > + } while (loop_forever || args->appl.timeout >= 0); > +} > + > /** > * ODP packet example main function > */ > @@ -796,6 +839,8 @@ int main(int argc, char *argv[]) > } > } > > + print_global_stats(); > + > /* Master thread waits for other threads to exit */ > odph_linux_pthread_join(thread_tbl, num_workers); > > -- > 1.9.1 >
made changes and pushed new version v3. On 5 August 2015 at 18:39, Stuart Haslam <stuart.haslam@linaro.org> wrote: > On Tue, Aug 04, 2015 at 11:07:57AM +0200, Balakrishna.Garapati wrote: > > Signed-off-by: Balakrishna.Garapati <balakrishna.garapati@linaro.org> > > --- > > Added packet rate stats > > example/generator/odp_generator.c | 67 > ++++++++++++++++++++++++++++++++------- > > 1 file changed, 56 insertions(+), 11 deletions(-) > > > > diff --git a/example/generator/odp_generator.c > b/example/generator/odp_generator.c > > index bdee222..aee6e7e 100644 > > --- a/example/generator/odp_generator.c > > +++ b/example/generator/odp_generator.c > > @@ -101,6 +101,7 @@ static void usage(char *progname); > > static int scan_ip(char *buf, unsigned int *paddr); > > static int scan_mac(char *in, odph_ethaddr_t *des); > > static void tv_sub(struct timeval *recvtime, struct timeval *sendtime); > > +static void print_global_stats(void); > > > > /** > > * Sleep for the specified amount of milliseconds > > @@ -371,7 +372,6 @@ static odp_pktio_t create_pktio(const char *dev, > odp_pool_t pool) > > static void *gen_send_thread(void *arg) > > { > > int thr; > > - uint64_t start, now, diff; > > odp_pktio_t pktio; > > thread_args_t *thr_args; > > odp_queue_t outq_def; > > @@ -393,7 +393,6 @@ static void *gen_send_thread(void *arg) > > return NULL; > > } > > > > - start = odp_time_cycles(); > > printf(" [%02i] created mode: SEND\n", thr); > > for (;;) { > > int err; > > @@ -434,15 +433,6 @@ static void *gen_send_thread(void *arg) > > >= (unsigned int)args->appl.number) { > > break; > > } > > - > > - now = odp_time_cycles(); > > - diff = odp_time_diff_cycles(start, now); > > - if (odp_time_cycles_to_ns(diff) > 20 * ODP_TIME_SEC) { > > - start = odp_time_cycles(); > > - printf(" [%02i] total send: %ju\n", > > - thr, odp_atomic_load_u64(&counters.seq)); > > - fflush(stdout); > > - } > > } > > > > /* receive number of reply pks until timeout */ > > @@ -589,6 +579,59 @@ static void *gen_recv_thread(void *arg) > > > > return arg; > > } > > + > > +/** > > + * printing verbose statistics > > + * > > + */ > > +static void print_global_stats(void) > > +{ > > + uint64_t start, now, diff; > > + uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0; > > + int loop_forever = 1, verbose_interval = 20; > > + > > + if (args->appl.timeout != -1 || args->appl.number > 0) > > + loop_forever = 0; > > + > > + start = odp_time_cycles(); > > + do { > > + pkts = 0; > > + now = odp_time_cycles(); > > + diff = odp_time_diff_cycles(start, now); > > + if (odp_time_cycles_to_ns(diff) > > > + verbose_interval * ODP_TIME_SEC) { > > If you check < instead here and "continue;" the indentation level of the > reset of the loop would be reduced. > > > + start = odp_time_cycles(); > > + > > + if (odp_atomic_load_u64(&counters.seq) > 0) { > > + pkts = odp_atomic_load_u64(&counters.seq); > > Move this assignment outside of the condition to avoid loading the > atomic twice. Actually I'm not sure about the condition at all, if you're > in _UDP mode don't you want to print the number of sent packets even if > it's 0? > > > + printf(" total send: %ju\n", pkts); > > PRIu64 as is used elsewhere > > > + } > > + > > + pps = (pkts - pkts_prev) / verbose_interval; > > + if (pps > maximum_pps) > > + maximum_pps = pps; > > + > > + printf(" %" PRIu64 " pps, %" PRIu64 " max pps \n", > > + pps, maximum_pps); > > checkpatch -> > > WARNING: unnecessary whitespace before a quoted newline > > Also this print is only valid for _UDP mode but it will be printed (and > always 0) in _RCV mode too. > > > + > > + if (args->appl.mode == APPL_MODE_RCV || > > + odp_atomic_load_u64(&counters.icmp) != 0 || > > + odp_atomic_load_u64(&counters.udp) != 0) { > > + printf(" total receive(icmp:%ju, > udp:%ju)\n", > > PRIu64 > > > + odp_atomic_load_u64(&counters.icmp), > > + odp_atomic_load_u64(&counters.udp)); > > At the minute this output is flooded by the "receive Packet proto:IP .." > lines that get printed unconditionally for every packet, I think they > need go except for in ICMP mode. > > Also it would be better to print *either* UDP or ICMP counters rather > than both, only one will be non-zero. > > > + } > > + > > + if (args->appl.number > 0 && > > + odp_atomic_load_u64(&counters.icmp) >= > > + (unsigned int)args->appl.number) > > + break; > > This logic and the loop condition logic in general seems fragile. I > think you'd get stuck in this loop if one of the worker threads aborted > early (e.g. due to a odp_queue_enq() failure). It would be better to > have a simpler condition for this loop that indicated when all of the > workers had completed and leave the timeout/number/counters logic in the > gen/send threads. This could be done using odp_thrmask_worker() and > odp_thrmask_count(). > > > + > > + pkts_prev = pkts; > > + } > > + } while (loop_forever || args->appl.timeout >= 0); > > +} > > + > > /** > > * ODP packet example main function > > */ > > @@ -796,6 +839,8 @@ int main(int argc, char *argv[]) > > } > > } > > > > + print_global_stats(); > > + > > /* Master thread waits for other threads to exit */ > > odph_linux_pthread_join(thread_tbl, num_workers); > > > > -- > > 1.9.1 > > > > -- > Stuart. >
diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c index bdee222..aee6e7e 100644 --- a/example/generator/odp_generator.c +++ b/example/generator/odp_generator.c @@ -101,6 +101,7 @@ static void usage(char *progname); static int scan_ip(char *buf, unsigned int *paddr); static int scan_mac(char *in, odph_ethaddr_t *des); static void tv_sub(struct timeval *recvtime, struct timeval *sendtime); +static void print_global_stats(void); /** * Sleep for the specified amount of milliseconds @@ -371,7 +372,6 @@ static odp_pktio_t create_pktio(const char *dev, odp_pool_t pool) static void *gen_send_thread(void *arg) { int thr; - uint64_t start, now, diff; odp_pktio_t pktio; thread_args_t *thr_args; odp_queue_t outq_def; @@ -393,7 +393,6 @@ static void *gen_send_thread(void *arg) return NULL; } - start = odp_time_cycles(); printf(" [%02i] created mode: SEND\n", thr); for (;;) { int err; @@ -434,15 +433,6 @@ static void *gen_send_thread(void *arg) >= (unsigned int)args->appl.number) { break; } - - now = odp_time_cycles(); - diff = odp_time_diff_cycles(start, now); - if (odp_time_cycles_to_ns(diff) > 20 * ODP_TIME_SEC) { - start = odp_time_cycles(); - printf(" [%02i] total send: %ju\n", - thr, odp_atomic_load_u64(&counters.seq)); - fflush(stdout); - } } /* receive number of reply pks until timeout */ @@ -589,6 +579,59 @@ static void *gen_recv_thread(void *arg) return arg; } + +/** + * printing verbose statistics + * + */ +static void print_global_stats(void) +{ + uint64_t start, now, diff; + uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0; + int loop_forever = 1, verbose_interval = 20; + + if (args->appl.timeout != -1 || args->appl.number > 0) + loop_forever = 0; + + start = odp_time_cycles(); + do { + pkts = 0; + now = odp_time_cycles(); + diff = odp_time_diff_cycles(start, now); + if (odp_time_cycles_to_ns(diff) > + verbose_interval * ODP_TIME_SEC) { + start = odp_time_cycles(); + + if (odp_atomic_load_u64(&counters.seq) > 0) { + pkts = odp_atomic_load_u64(&counters.seq); + printf(" total send: %ju\n", pkts); + } + + pps = (pkts - pkts_prev) / verbose_interval; + if (pps > maximum_pps) + maximum_pps = pps; + + printf(" %" PRIu64 " pps, %" PRIu64 " max pps \n", + pps, maximum_pps); + + if (args->appl.mode == APPL_MODE_RCV || + odp_atomic_load_u64(&counters.icmp) != 0 || + odp_atomic_load_u64(&counters.udp) != 0) { + printf(" total receive(icmp:%ju, udp:%ju)\n", + odp_atomic_load_u64(&counters.icmp), + odp_atomic_load_u64(&counters.udp)); + } + + if (args->appl.number > 0 && + odp_atomic_load_u64(&counters.icmp) >= + (unsigned int)args->appl.number) + break; + + pkts_prev = pkts; + } + } while (loop_forever || args->appl.timeout >= 0); +} + /** * ODP packet example main function */ @@ -796,6 +839,8 @@ int main(int argc, char *argv[]) } } + print_global_stats(); + /* Master thread waits for other threads to exit */ odph_linux_pthread_join(thread_tbl, num_workers);
Signed-off-by: Balakrishna.Garapati <balakrishna.garapati@linaro.org> --- Added packet rate stats example/generator/odp_generator.c | 67 ++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 11 deletions(-)