| Message ID | 1447068113-3733-6-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 09, 2015 at 02:21:53PM +0300, Maxim Uvarov wrote: > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > test/validation/pktio/pktio.c | 130 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 130 insertions(+) > > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index 6320b77..c591c94 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -690,6 +690,135 @@ void pktio_test_inq(void) > CU_ASSERT(odp_pktio_close(pktio) == 0); > } > > +static void _print_pktio_stats(odp_pktio_stats_t *s, const char *name) > +{ > + printf("\n%s:\n" > + " in_octets %"PRIu64"\n" > + " in_ucast_pkts %"PRIu64"\n" > + " in_discards %"PRIu64"\n" > + " in_errors %"PRIu64"\n" > + " in_unknown_protos %"PRIu64"\n" > + " out_octets %"PRIu64"\n" > + " out_ucast_pkts %"PRIu64"\n" > + " out_discards %"PRIu64"\n" > + " out_errors %"PRIu64"\n", > + name, > + s->in_octets, > + s->in_ucast_pkts, > + s->in_discards, > + s->in_errors, > + s->in_unknown_protos, > + s->out_octets, > + s->out_ucast_pkts, > + s->out_discards, > + s->out_errors); > +} > + > +static void pktio_test_statistics_counters(void) > +{ > + odp_pktio_t pktio[MAX_NUM_IFACES]; > + odp_packet_t pkt; > + odp_event_t tx_ev[1000]; > + odp_event_t ev; > + int i, pkts, ret, alloc = 0; > + odp_queue_t outq; > + uint64_t wait = odp_schedule_wait_time(ODP_TIME_MSEC); > + odp_pktio_stats_t stats[2]; > + > + for (i = 0; i < num_ifaces; i++) { > + pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); > + CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); > + create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); > + } > + > + outq = odp_pktio_outq_getdef(pktio[0]); > + > + ret = odp_pktio_start(pktio[0]); > + CU_ASSERT(ret == 0); > + if (num_ifaces > 1) { > + ret = odp_pktio_start(pktio[1]); > + CU_ASSERT(ret == 0); > + } > + > + /* flush packets with magic number in pipes */ > + for (i = 0; i < 1000; i++) { > + ev = odp_schedule(NULL, wait); > + if (ev != ODP_EVENT_INVALID) > + odp_event_free(ev); > + } > + > + /* alloc */ > + for (alloc = 0; alloc < 1000; alloc++) { > + pkt = odp_packet_alloc(default_pkt_pool, packet_len); > + if (pkt == ODP_PACKET_INVALID) > + break; > + pktio_init_packet(pkt); > + tx_ev[alloc] = odp_packet_to_event(pkt); > + } > + > + ret = odp_pktio_stats_reset(pktio[0]); > + CU_ASSERT(ret == 0); > + if (num_ifaces > 1) { > + ret = odp_pktio_stats_reset(pktio[1]); > + CU_ASSERT(ret == 0); > + } > + > + /* send */ > + for (pkts = 0; pkts != alloc; ) { > + ret = odp_queue_enq_multi(outq, &tx_ev[pkts], alloc - pkts); > + if (ret < 0) { > + CU_FAIL("unable to enqueue packet\n"); > + break; > + } > + pkts += ret; > + } > + > + /* get */ > + for (i = 0, pkts = 0; i < 1000; i++) { > + ev = odp_schedule(NULL, wait); > + if (ev != ODP_EVENT_INVALID) { > + if (odp_event_type(ev) == ODP_EVENT_PACKET) { > + pkt = odp_packet_from_event(ev); > + if (pktio_pkt_seq(pkt) != TEST_SEQ_INVALID) > + pkts++; > + } > + odp_event_free(ev); > + } > + } > + > + ret = odp_pktio_stats(pktio[0], &stats[0]); > + CU_ASSERT(ret == 0); > + _print_pktio_stats(&stats[0], iface_name[0]); I don't like tests printing to stdout, it clutters the output and makes it harder to see what's passed/failed. If all of the asserts pass then it doesn't matter what the actual values are, that's a detail of the test, if something fails though it's reasonable to dump to stderr. > + > + if (num_ifaces > 1) { > + ret = odp_pktio_stats(pktio[1], &stats[1]); > + CU_ASSERT(ret == 0); > + _print_pktio_stats(&stats[1], iface_name[1]); > + > + CU_ASSERT(stats[1].in_ucast_pkts >= (uint64_t)pkts); > + CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts); > + CU_ASSERT(stats[0].out_octets == stats[1].in_octets); > + CU_ASSERT(stats[0].out_octets >= > + (stats[0].out_ucast_pkts * (uint64_t)pkts)); What's this check for?.. seems to assert that average pkt size >= 2 octets. > + } else { Why not check out_octets and out_ucast_pkts in this case? The logic would be simpler with something like: if (num_ifaces == 1) pktio[1] = pktio[0]; stats[0] = odp_pktio_stats(pktio[0], &stats[0]); stats[1] = odp_pktio_stats(pktio[1], &stats[1]); Then all the asserts would be the same regardless of whether you have 1 or 2 interfaces. > + CU_ASSERT(stats[0].in_ucast_pkts == (uint64_t)pkts); > + CU_ASSERT(stats[0].in_octets == > + (PKT_LEN_NORMAL * (uint64_t)pkts)); > + } > + > + CU_ASSERT(pkts == alloc); > + CU_ASSERT((stats[0].in_discards | > + stats[0].in_errors | > + stats[0].in_unknown_protos | > + stats[0].out_discards | > + stats[0].out_errors) == 0); It would be better to have separate asserts for each of these so you can tell from the logs which failed. > + > + for (i = 0; i < num_ifaces; i++) { > + destroy_inq(pktio[i]); > + CU_ASSERT(odp_pktio_close(pktio[i]) == 0); > + } > +} > + > static void pktio_test_start_stop(void) > { > odp_pktio_t pktio[MAX_NUM_IFACES]; > @@ -1057,6 +1186,7 @@ odp_testinfo_t pktio_suite_unsegmented[] = { > ODP_TEST_INFO(pktio_test_mac), > ODP_TEST_INFO(pktio_test_inq_remdef), > ODP_TEST_INFO(pktio_test_start_stop), > + ODP_TEST_INFO(pktio_test_statistics_counters), > ODP_TEST_INFO_NULL > }; > > -- > 1.9.1 >
On 11/10/2015 21:30, Stuart Haslam wrote: > On Mon, Nov 09, 2015 at 02:21:53PM +0300, Maxim Uvarov wrote: >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> test/validation/pktio/pktio.c | 130 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 130 insertions(+) >> >> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c >> index 6320b77..c591c94 100644 >> --- a/test/validation/pktio/pktio.c >> +++ b/test/validation/pktio/pktio.c >> @@ -690,6 +690,135 @@ void pktio_test_inq(void) >> CU_ASSERT(odp_pktio_close(pktio) == 0); >> } >> >> +static void _print_pktio_stats(odp_pktio_stats_t *s, const char *name) >> +{ >> + printf("\n%s:\n" >> + " in_octets %"PRIu64"\n" >> + " in_ucast_pkts %"PRIu64"\n" >> + " in_discards %"PRIu64"\n" >> + " in_errors %"PRIu64"\n" >> + " in_unknown_protos %"PRIu64"\n" >> + " out_octets %"PRIu64"\n" >> + " out_ucast_pkts %"PRIu64"\n" >> + " out_discards %"PRIu64"\n" >> + " out_errors %"PRIu64"\n", >> + name, >> + s->in_octets, >> + s->in_ucast_pkts, >> + s->in_discards, >> + s->in_errors, >> + s->in_unknown_protos, >> + s->out_octets, >> + s->out_ucast_pkts, >> + s->out_discards, >> + s->out_errors); >> +} >> + >> +static void pktio_test_statistics_counters(void) >> +{ >> + odp_pktio_t pktio[MAX_NUM_IFACES]; >> + odp_packet_t pkt; >> + odp_event_t tx_ev[1000]; >> + odp_event_t ev; >> + int i, pkts, ret, alloc = 0; >> + odp_queue_t outq; >> + uint64_t wait = odp_schedule_wait_time(ODP_TIME_MSEC); >> + odp_pktio_stats_t stats[2]; >> + >> + for (i = 0; i < num_ifaces; i++) { >> + pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); >> + CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); >> + create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); >> + } >> + >> + outq = odp_pktio_outq_getdef(pktio[0]); >> + >> + ret = odp_pktio_start(pktio[0]); >> + CU_ASSERT(ret == 0); >> + if (num_ifaces > 1) { >> + ret = odp_pktio_start(pktio[1]); >> + CU_ASSERT(ret == 0); >> + } >> + >> + /* flush packets with magic number in pipes */ >> + for (i = 0; i < 1000; i++) { >> + ev = odp_schedule(NULL, wait); >> + if (ev != ODP_EVENT_INVALID) >> + odp_event_free(ev); >> + } >> + >> + /* alloc */ >> + for (alloc = 0; alloc < 1000; alloc++) { >> + pkt = odp_packet_alloc(default_pkt_pool, packet_len); >> + if (pkt == ODP_PACKET_INVALID) >> + break; >> + pktio_init_packet(pkt); >> + tx_ev[alloc] = odp_packet_to_event(pkt); >> + } >> + >> + ret = odp_pktio_stats_reset(pktio[0]); >> + CU_ASSERT(ret == 0); >> + if (num_ifaces > 1) { >> + ret = odp_pktio_stats_reset(pktio[1]); >> + CU_ASSERT(ret == 0); >> + } >> + >> + /* send */ >> + for (pkts = 0; pkts != alloc; ) { >> + ret = odp_queue_enq_multi(outq, &tx_ev[pkts], alloc - pkts); >> + if (ret < 0) { >> + CU_FAIL("unable to enqueue packet\n"); >> + break; >> + } >> + pkts += ret; >> + } >> + >> + /* get */ >> + for (i = 0, pkts = 0; i < 1000; i++) { >> + ev = odp_schedule(NULL, wait); >> + if (ev != ODP_EVENT_INVALID) { >> + if (odp_event_type(ev) == ODP_EVENT_PACKET) { >> + pkt = odp_packet_from_event(ev); >> + if (pktio_pkt_seq(pkt) != TEST_SEQ_INVALID) >> + pkts++; >> + } >> + odp_event_free(ev); >> + } >> + } >> + >> + ret = odp_pktio_stats(pktio[0], &stats[0]); >> + CU_ASSERT(ret == 0); >> + _print_pktio_stats(&stats[0], iface_name[0]); > I don't like tests printing to stdout, it clutters the output and makes > it harder to see what's passed/failed. If all of the asserts pass then > it doesn't matter what the actual values are, that's a detail of the > test, if something fails though it's reasonable to dump to stderr. ok, I can hide to error case. It was easy for me to debug when I see numbers. > >> + >> + if (num_ifaces > 1) { >> + ret = odp_pktio_stats(pktio[1], &stats[1]); >> + CU_ASSERT(ret == 0); >> + _print_pktio_stats(&stats[1], iface_name[1]); >> + >> + CU_ASSERT(stats[1].in_ucast_pkts >= (uint64_t)pkts); >> + CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts); >> + CU_ASSERT(stats[0].out_octets == stats[1].in_octets); >> + CU_ASSERT(stats[0].out_octets >= >> + (stats[0].out_ucast_pkts * (uint64_t)pkts)); > What's this check for?.. seems to assert that average pkt size >= 2 > octets. Opps. I planned to write: + CU_ASSERT(stats[0].out_octets >= + (pkt_size * (uint64_t)pkts)); For loop it's ==. But for other virtual dev some packets may be on network (like dhcp), so it will be >=. >> + } else { > Why not check out_octets and out_ucast_pkts in this case? > > The logic would be simpler with something like: > > if (num_ifaces == 1) > pktio[1] = pktio[0]; > > stats[0] = odp_pktio_stats(pktio[0], &stats[0]); > stats[1] = odp_pktio_stats(pktio[1], &stats[1]); > > Then all the asserts would be the same regardless of whether you have 1 > or 2 interfaces. yes, it's the same think. Calling 2 times odp_pktio_stats() for the same device but different index in array a little bit confusing (complicates code understanding). At least for me. >> + CU_ASSERT(stats[0].in_ucast_pkts == (uint64_t)pkts); >> + CU_ASSERT(stats[0].in_octets == >> + (PKT_LEN_NORMAL * (uint64_t)pkts)); >> + } >> + >> + CU_ASSERT(pkts == alloc); >> + CU_ASSERT((stats[0].in_discards | >> + stats[0].in_errors | >> + stats[0].in_unknown_protos | >> + stats[0].out_discards | >> + stats[0].out_errors) == 0); > It would be better to have separate asserts for each of these so you > can tell from the logs which failed. Ok. My plan was that current counters are not well tested. When we will add tests for them then we can exclude from that group. But I can do test case per counter, it's not a problem. > >> + >> + for (i = 0; i < num_ifaces; i++) { >> + destroy_inq(pktio[i]); >> + CU_ASSERT(odp_pktio_close(pktio[i]) == 0); >> + } >> +} >> + >> static void pktio_test_start_stop(void) >> { >> odp_pktio_t pktio[MAX_NUM_IFACES]; >> @@ -1057,6 +1186,7 @@ odp_testinfo_t pktio_suite_unsegmented[] = { >> ODP_TEST_INFO(pktio_test_mac), >> ODP_TEST_INFO(pktio_test_inq_remdef), >> ODP_TEST_INFO(pktio_test_start_stop), >> + ODP_TEST_INFO(pktio_test_statistics_counters), >> ODP_TEST_INFO_NULL >> }; >> >> -- >> 1.9.1 >>
On Thu, Nov 12, 2015 at 05:30:47PM +0300, Maxim Uvarov wrote: > On 11/10/2015 21:30, Stuart Haslam wrote: > >On Mon, Nov 09, 2015 at 02:21:53PM +0300, Maxim Uvarov wrote: > >>Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > >>--- > >> test/validation/pktio/pktio.c | 130 ++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 130 insertions(+) > >> > >>diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > >>index 6320b77..c591c94 100644 > >>--- a/test/validation/pktio/pktio.c > >>+++ b/test/validation/pktio/pktio.c > >>@@ -690,6 +690,135 @@ void pktio_test_inq(void) > >> CU_ASSERT(odp_pktio_close(pktio) == 0); > >> } > >>+static void _print_pktio_stats(odp_pktio_stats_t *s, const char *name) > >>+{ > >>+ printf("\n%s:\n" > >>+ " in_octets %"PRIu64"\n" > >>+ " in_ucast_pkts %"PRIu64"\n" > >>+ " in_discards %"PRIu64"\n" > >>+ " in_errors %"PRIu64"\n" > >>+ " in_unknown_protos %"PRIu64"\n" > >>+ " out_octets %"PRIu64"\n" > >>+ " out_ucast_pkts %"PRIu64"\n" > >>+ " out_discards %"PRIu64"\n" > >>+ " out_errors %"PRIu64"\n", > >>+ name, > >>+ s->in_octets, > >>+ s->in_ucast_pkts, > >>+ s->in_discards, > >>+ s->in_errors, > >>+ s->in_unknown_protos, > >>+ s->out_octets, > >>+ s->out_ucast_pkts, > >>+ s->out_discards, > >>+ s->out_errors); > >>+} > >>+ > >>+static void pktio_test_statistics_counters(void) > >>+{ > >>+ odp_pktio_t pktio[MAX_NUM_IFACES]; > >>+ odp_packet_t pkt; > >>+ odp_event_t tx_ev[1000]; > >>+ odp_event_t ev; > >>+ int i, pkts, ret, alloc = 0; > >>+ odp_queue_t outq; > >>+ uint64_t wait = odp_schedule_wait_time(ODP_TIME_MSEC); > >>+ odp_pktio_stats_t stats[2]; > >>+ > >>+ for (i = 0; i < num_ifaces; i++) { > >>+ pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); > >>+ CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); > >>+ create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); > >>+ } > >>+ > >>+ outq = odp_pktio_outq_getdef(pktio[0]); > >>+ > >>+ ret = odp_pktio_start(pktio[0]); > >>+ CU_ASSERT(ret == 0); > >>+ if (num_ifaces > 1) { > >>+ ret = odp_pktio_start(pktio[1]); > >>+ CU_ASSERT(ret == 0); > >>+ } > >>+ > >>+ /* flush packets with magic number in pipes */ > >>+ for (i = 0; i < 1000; i++) { > >>+ ev = odp_schedule(NULL, wait); > >>+ if (ev != ODP_EVENT_INVALID) > >>+ odp_event_free(ev); > >>+ } > >>+ > >>+ /* alloc */ > >>+ for (alloc = 0; alloc < 1000; alloc++) { > >>+ pkt = odp_packet_alloc(default_pkt_pool, packet_len); > >>+ if (pkt == ODP_PACKET_INVALID) > >>+ break; > >>+ pktio_init_packet(pkt); > >>+ tx_ev[alloc] = odp_packet_to_event(pkt); > >>+ } > >>+ > >>+ ret = odp_pktio_stats_reset(pktio[0]); > >>+ CU_ASSERT(ret == 0); > >>+ if (num_ifaces > 1) { > >>+ ret = odp_pktio_stats_reset(pktio[1]); > >>+ CU_ASSERT(ret == 0); > >>+ } > >>+ > >>+ /* send */ > >>+ for (pkts = 0; pkts != alloc; ) { > >>+ ret = odp_queue_enq_multi(outq, &tx_ev[pkts], alloc - pkts); > >>+ if (ret < 0) { > >>+ CU_FAIL("unable to enqueue packet\n"); > >>+ break; > >>+ } > >>+ pkts += ret; > >>+ } > >>+ > >>+ /* get */ > >>+ for (i = 0, pkts = 0; i < 1000; i++) { > >>+ ev = odp_schedule(NULL, wait); > >>+ if (ev != ODP_EVENT_INVALID) { > >>+ if (odp_event_type(ev) == ODP_EVENT_PACKET) { > >>+ pkt = odp_packet_from_event(ev); > >>+ if (pktio_pkt_seq(pkt) != TEST_SEQ_INVALID) > >>+ pkts++; > >>+ } > >>+ odp_event_free(ev); > >>+ } > >>+ } > >>+ > >>+ ret = odp_pktio_stats(pktio[0], &stats[0]); > >>+ CU_ASSERT(ret == 0); > >>+ _print_pktio_stats(&stats[0], iface_name[0]); > >I don't like tests printing to stdout, it clutters the output and makes > >it harder to see what's passed/failed. If all of the asserts pass then > >it doesn't matter what the actual values are, that's a detail of the > >test, if something fails though it's reasonable to dump to stderr. > > ok, I can hide to error case. It was easy for me to debug when I see > numbers. > > > > >>+ > >>+ if (num_ifaces > 1) { > >>+ ret = odp_pktio_stats(pktio[1], &stats[1]); > >>+ CU_ASSERT(ret == 0); > >>+ _print_pktio_stats(&stats[1], iface_name[1]); > >>+ > >>+ CU_ASSERT(stats[1].in_ucast_pkts >= (uint64_t)pkts); > >>+ CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts); > >>+ CU_ASSERT(stats[0].out_octets == stats[1].in_octets); > >>+ CU_ASSERT(stats[0].out_octets >= > >>+ (stats[0].out_ucast_pkts * (uint64_t)pkts)); > >What's this check for?.. seems to assert that average pkt size >= 2 > >octets. > > Opps. I planned to write: > > + CU_ASSERT(stats[0].out_octets >= > + (pkt_size * (uint64_t)pkts)); > OK, that makes sense. > > For loop it's ==. But for other virtual dev some packets may be on > network (like dhcp), > so it will be >=. > > > >>+ } else { > >Why not check out_octets and out_ucast_pkts in this case? > > > >The logic would be simpler with something like: > > > >if (num_ifaces == 1) > > pktio[1] = pktio[0]; > > > >stats[0] = odp_pktio_stats(pktio[0], &stats[0]); > >stats[1] = odp_pktio_stats(pktio[1], &stats[1]); > > > >Then all the asserts would be the same regardless of whether you have 1 > >or 2 interfaces. > yes, it's the same think. Calling 2 times odp_pktio_stats() for the > same device > but different index in array a little bit confusing (complicates > code understanding). > At least for me. The main point was to avoid having two different sets of CU_ASSERT()s depending on whether you're using one or two interfaces, the test should be exactly the same in both cases. > >>+ CU_ASSERT(stats[0].in_ucast_pkts == (uint64_t)pkts); > >>+ CU_ASSERT(stats[0].in_octets == > >>+ (PKT_LEN_NORMAL * (uint64_t)pkts)); > >>+ } > >>+ > >>+ CU_ASSERT(pkts == alloc); > >>+ CU_ASSERT((stats[0].in_discards | > >>+ stats[0].in_errors | > >>+ stats[0].in_unknown_protos | > >>+ stats[0].out_discards | > >>+ stats[0].out_errors) == 0); > >It would be better to have separate asserts for each of these so you > >can tell from the logs which failed. > Ok. My plan was that current counters are not well tested. When we > will add tests > for them then we can exclude from that group. But I can do test case > per counter, > it's not a problem. > > > > >>+ > >>+ for (i = 0; i < num_ifaces; i++) { > >>+ destroy_inq(pktio[i]); > >>+ CU_ASSERT(odp_pktio_close(pktio[i]) == 0); > >>+ } > >>+} > >>+ > >> static void pktio_test_start_stop(void) > >> { > >> odp_pktio_t pktio[MAX_NUM_IFACES]; > >>@@ -1057,6 +1186,7 @@ odp_testinfo_t pktio_suite_unsegmented[] = { > >> ODP_TEST_INFO(pktio_test_mac), > >> ODP_TEST_INFO(pktio_test_inq_remdef), > >> ODP_TEST_INFO(pktio_test_start_stop), > >>+ ODP_TEST_INFO(pktio_test_statistics_counters), > >> ODP_TEST_INFO_NULL > >> }; > >>-- > >>1.9.1 > >> >
On 11/10/2015 21:30, Stuart Haslam wrote: > On Mon, Nov 09, 2015 at 02:21:53PM +0300, Maxim Uvarov wrote: >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> test/validation/pktio/pktio.c | 130 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 130 insertions(+) >> >> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c >> index 6320b77..c591c94 100644 >> --- a/test/validation/pktio/pktio.c >> +++ b/test/validation/pktio/pktio.c >> @@ -690,6 +690,135 @@ void pktio_test_inq(void) >> CU_ASSERT(odp_pktio_close(pktio) == 0); >> } >> >> +static void _print_pktio_stats(odp_pktio_stats_t *s, const char *name) >> +{ >> + printf("\n%s:\n" >> + " in_octets %"PRIu64"\n" >> + " in_ucast_pkts %"PRIu64"\n" >> + " in_discards %"PRIu64"\n" >> + " in_errors %"PRIu64"\n" >> + " in_unknown_protos %"PRIu64"\n" >> + " out_octets %"PRIu64"\n" >> + " out_ucast_pkts %"PRIu64"\n" >> + " out_discards %"PRIu64"\n" >> + " out_errors %"PRIu64"\n", >> + name, >> + s->in_octets, >> + s->in_ucast_pkts, >> + s->in_discards, >> + s->in_errors, >> + s->in_unknown_protos, >> + s->out_octets, >> + s->out_ucast_pkts, >> + s->out_discards, >> + s->out_errors); >> +} >> + >> +static void pktio_test_statistics_counters(void) >> +{ >> + odp_pktio_t pktio[MAX_NUM_IFACES]; >> + odp_packet_t pkt; >> + odp_event_t tx_ev[1000]; >> + odp_event_t ev; >> + int i, pkts, ret, alloc = 0; >> + odp_queue_t outq; >> + uint64_t wait = odp_schedule_wait_time(ODP_TIME_MSEC); >> + odp_pktio_stats_t stats[2]; >> + >> + for (i = 0; i < num_ifaces; i++) { >> + pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); >> + CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); >> + create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); >> + } >> + >> + outq = odp_pktio_outq_getdef(pktio[0]); >> + >> + ret = odp_pktio_start(pktio[0]); >> + CU_ASSERT(ret == 0); >> + if (num_ifaces > 1) { >> + ret = odp_pktio_start(pktio[1]); >> + CU_ASSERT(ret == 0); >> + } >> + >> + /* flush packets with magic number in pipes */ >> + for (i = 0; i < 1000; i++) { >> + ev = odp_schedule(NULL, wait); >> + if (ev != ODP_EVENT_INVALID) >> + odp_event_free(ev); >> + } >> + >> + /* alloc */ >> + for (alloc = 0; alloc < 1000; alloc++) { >> + pkt = odp_packet_alloc(default_pkt_pool, packet_len); >> + if (pkt == ODP_PACKET_INVALID) >> + break; >> + pktio_init_packet(pkt); >> + tx_ev[alloc] = odp_packet_to_event(pkt); >> + } >> + >> + ret = odp_pktio_stats_reset(pktio[0]); >> + CU_ASSERT(ret == 0); >> + if (num_ifaces > 1) { >> + ret = odp_pktio_stats_reset(pktio[1]); >> + CU_ASSERT(ret == 0); >> + } >> + >> + /* send */ >> + for (pkts = 0; pkts != alloc; ) { >> + ret = odp_queue_enq_multi(outq, &tx_ev[pkts], alloc - pkts); >> + if (ret < 0) { >> + CU_FAIL("unable to enqueue packet\n"); >> + break; >> + } >> + pkts += ret; >> + } >> + >> + /* get */ >> + for (i = 0, pkts = 0; i < 1000; i++) { >> + ev = odp_schedule(NULL, wait); >> + if (ev != ODP_EVENT_INVALID) { >> + if (odp_event_type(ev) == ODP_EVENT_PACKET) { >> + pkt = odp_packet_from_event(ev); >> + if (pktio_pkt_seq(pkt) != TEST_SEQ_INVALID) >> + pkts++; >> + } >> + odp_event_free(ev); >> + } >> + } >> + >> + ret = odp_pktio_stats(pktio[0], &stats[0]); >> + CU_ASSERT(ret == 0); >> + _print_pktio_stats(&stats[0], iface_name[0]); > I don't like tests printing to stdout, it clutters the output and makes > it harder to see what's passed/failed. If all of the asserts pass then > it doesn't matter what the actual values are, that's a detail of the > test, if something fails though it's reasonable to dump to stderr. > >> + >> + if (num_ifaces > 1) { >> + ret = odp_pktio_stats(pktio[1], &stats[1]); >> + CU_ASSERT(ret == 0); >> + _print_pktio_stats(&stats[1], iface_name[1]); >> + >> + CU_ASSERT(stats[1].in_ucast_pkts >= (uint64_t)pkts); >> + CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts); >> + CU_ASSERT(stats[0].out_octets == stats[1].in_octets); >> + CU_ASSERT(stats[0].out_octets >= >> + (stats[0].out_ucast_pkts * (uint64_t)pkts)); > What's this check for?.. seems to assert that average pkt size >= 2 > octets. > >> + } else { > Why not check out_octets and out_ucast_pkts in this case? > > The logic would be simpler with something like: > > if (num_ifaces == 1) > pktio[1] = pktio[0]; > > stats[0] = odp_pktio_stats(pktio[0], &stats[0]); > stats[1] = odp_pktio_stats(pktio[1], &stats[1]); > > Then all the asserts would be the same regardless of whether you have 1 > or 2 interfaces. that does not go because pktio_open happens only for num devices: for (i = 0; i < num_ifaces; i++) { pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); } Other comments look good. Maxim. >> + CU_ASSERT(stats[0].in_ucast_pkts == (uint64_t)pkts); >> + CU_ASSERT(stats[0].in_octets == >> + (PKT_LEN_NORMAL * (uint64_t)pkts)); >> + } >> + >> + CU_ASSERT(pkts == alloc); >> + CU_ASSERT((stats[0].in_discards | >> + stats[0].in_errors | >> + stats[0].in_unknown_protos | >> + stats[0].out_discards | >> + stats[0].out_errors) == 0); > It would be better to have separate asserts for each of these so you > can tell from the logs which failed. > >> + >> + for (i = 0; i < num_ifaces; i++) { >> + destroy_inq(pktio[i]); >> + CU_ASSERT(odp_pktio_close(pktio[i]) == 0); >> + } >> +} >> + >> static void pktio_test_start_stop(void) >> { >> odp_pktio_t pktio[MAX_NUM_IFACES]; >> @@ -1057,6 +1186,7 @@ odp_testinfo_t pktio_suite_unsegmented[] = { >> ODP_TEST_INFO(pktio_test_mac), >> ODP_TEST_INFO(pktio_test_inq_remdef), >> ODP_TEST_INFO(pktio_test_start_stop), >> + ODP_TEST_INFO(pktio_test_statistics_counters), >> ODP_TEST_INFO_NULL >> }; >> >> -- >> 1.9.1 >>
diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 6320b77..c591c94 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -690,6 +690,135 @@ void pktio_test_inq(void) CU_ASSERT(odp_pktio_close(pktio) == 0); } +static void _print_pktio_stats(odp_pktio_stats_t *s, const char *name) +{ + printf("\n%s:\n" + " in_octets %"PRIu64"\n" + " in_ucast_pkts %"PRIu64"\n" + " in_discards %"PRIu64"\n" + " in_errors %"PRIu64"\n" + " in_unknown_protos %"PRIu64"\n" + " out_octets %"PRIu64"\n" + " out_ucast_pkts %"PRIu64"\n" + " out_discards %"PRIu64"\n" + " out_errors %"PRIu64"\n", + name, + s->in_octets, + s->in_ucast_pkts, + s->in_discards, + s->in_errors, + s->in_unknown_protos, + s->out_octets, + s->out_ucast_pkts, + s->out_discards, + s->out_errors); +} + +static void pktio_test_statistics_counters(void) +{ + odp_pktio_t pktio[MAX_NUM_IFACES]; + odp_packet_t pkt; + odp_event_t tx_ev[1000]; + odp_event_t ev; + int i, pkts, ret, alloc = 0; + odp_queue_t outq; + uint64_t wait = odp_schedule_wait_time(ODP_TIME_MSEC); + odp_pktio_stats_t stats[2]; + + for (i = 0; i < num_ifaces; i++) { + pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); + CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); + create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); + } + + outq = odp_pktio_outq_getdef(pktio[0]); + + ret = odp_pktio_start(pktio[0]); + CU_ASSERT(ret == 0); + if (num_ifaces > 1) { + ret = odp_pktio_start(pktio[1]); + CU_ASSERT(ret == 0); + } + + /* flush packets with magic number in pipes */ + for (i = 0; i < 1000; i++) { + ev = odp_schedule(NULL, wait); + if (ev != ODP_EVENT_INVALID) + odp_event_free(ev); + } + + /* alloc */ + for (alloc = 0; alloc < 1000; alloc++) { + pkt = odp_packet_alloc(default_pkt_pool, packet_len); + if (pkt == ODP_PACKET_INVALID) + break; + pktio_init_packet(pkt); + tx_ev[alloc] = odp_packet_to_event(pkt); + } + + ret = odp_pktio_stats_reset(pktio[0]); + CU_ASSERT(ret == 0); + if (num_ifaces > 1) { + ret = odp_pktio_stats_reset(pktio[1]); + CU_ASSERT(ret == 0); + } + + /* send */ + for (pkts = 0; pkts != alloc; ) { + ret = odp_queue_enq_multi(outq, &tx_ev[pkts], alloc - pkts); + if (ret < 0) { + CU_FAIL("unable to enqueue packet\n"); + break; + } + pkts += ret; + } + + /* get */ + for (i = 0, pkts = 0; i < 1000; i++) { + ev = odp_schedule(NULL, wait); + if (ev != ODP_EVENT_INVALID) { + if (odp_event_type(ev) == ODP_EVENT_PACKET) { + pkt = odp_packet_from_event(ev); + if (pktio_pkt_seq(pkt) != TEST_SEQ_INVALID) + pkts++; + } + odp_event_free(ev); + } + } + + ret = odp_pktio_stats(pktio[0], &stats[0]); + CU_ASSERT(ret == 0); + _print_pktio_stats(&stats[0], iface_name[0]); + + if (num_ifaces > 1) { + ret = odp_pktio_stats(pktio[1], &stats[1]); + CU_ASSERT(ret == 0); + _print_pktio_stats(&stats[1], iface_name[1]); + + CU_ASSERT(stats[1].in_ucast_pkts >= (uint64_t)pkts); + CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts); + CU_ASSERT(stats[0].out_octets == stats[1].in_octets); + CU_ASSERT(stats[0].out_octets >= + (stats[0].out_ucast_pkts * (uint64_t)pkts)); + } else { + CU_ASSERT(stats[0].in_ucast_pkts == (uint64_t)pkts); + CU_ASSERT(stats[0].in_octets == + (PKT_LEN_NORMAL * (uint64_t)pkts)); + } + + CU_ASSERT(pkts == alloc); + CU_ASSERT((stats[0].in_discards | + stats[0].in_errors | + stats[0].in_unknown_protos | + stats[0].out_discards | + stats[0].out_errors) == 0); + + for (i = 0; i < num_ifaces; i++) { + destroy_inq(pktio[i]); + CU_ASSERT(odp_pktio_close(pktio[i]) == 0); + } +} + static void pktio_test_start_stop(void) { odp_pktio_t pktio[MAX_NUM_IFACES]; @@ -1057,6 +1186,7 @@ odp_testinfo_t pktio_suite_unsegmented[] = { ODP_TEST_INFO(pktio_test_mac), ODP_TEST_INFO(pktio_test_inq_remdef), ODP_TEST_INFO(pktio_test_start_stop), + ODP_TEST_INFO(pktio_test_statistics_counters), ODP_TEST_INFO_NULL };
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- test/validation/pktio/pktio.c | 130 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+)