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.uva...@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
> >>
>
--
Stuart.
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
