[API-NEXT,PATCHv2,6/7] validation: test odp_pktio_start and odp_pktio_stop

Message ID 1439550184-28383-7-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Aug. 14, 2015, 11:03 a.m.
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 test/validation/pktio/pktio.c | 114 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

Comments

Stuart Haslam Aug. 19, 2015, 11:03 a.m. | #1
On Fri, Aug 14, 2015 at 02:03:03PM +0300, Maxim Uvarov wrote:
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  test/validation/pktio/pktio.c | 114 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 114 insertions(+)
> 
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index ebe34fa..4222463 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -626,6 +626,119 @@ void pktio_test_inq(void)
>  	CU_ASSERT(odp_pktio_close(pktio) == 0);
>  }
>  
> +static uint32_t pktio_pkt_validate(odp_packet_t pkt)
> +{
> +	size_t off;
> +	pkt_head_t head;
> +	pkt_tail_t tail;
> +
> +	if (pkt == ODP_PACKET_INVALID)
> +		return -1;
> +
> +	off = odp_packet_l4_offset(pkt);
> +	if (off ==  ODP_PACKET_OFFSET_INVALID)
> +		return -1;
> +
> +	off += ODPH_UDPHDR_LEN;
> +	if (odp_packet_copydata_out(pkt, off, sizeof(head), &head) != 0)
> +		return -1;
> +
> +	if (head.magic != TEST_SEQ_MAGIC)
> +		return -1;
> +
> +	if (odp_packet_len(pkt) == packet_len) {
> +		off = packet_len - sizeof(tail);
> +		if (odp_packet_copydata_out(pkt, off, sizeof(tail), &tail) != 0)
> +			return -1;
> +	}
> +
> +	return 0;
> +}

This is pretty similar to pktio_pkt_seq() except that this isn't checking
packets arrive in sequence or that their tail magic is valid, what's the
reason for that?

If the two functions are needed they could do with being refactored a bit.

> +
> +static void pktio_test_start_stop(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;
> +
> +	for (i = 0; i < num_ifaces; i++) {
> +		pktio[i] = create_pktio(iface_name[i], ODP_QUEUE_TYPE_SCHED, 0);
> +		CU_ASSERT(pktio[i] != ODP_PKTIO_INVALID);
> +		create_inq(pktio[i],  ODP_QUEUE_TYPE_SCHED);
> +	}
> +
> +	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);
> +	}
> +
> +	outq = odp_pktio_outq_getdef(pktio[0]);
> +
> +	/* stop and check that we can not transmit*/
> +	odp_pktio_stop(pktio[0]);

Check return value

> +	odp_errno_zero();
> +	ret = odp_queue_enq_multi(outq, tx_ev, 10);
> +	CU_ASSERT(ret == -1);
> +	CU_ASSERT(odp_errno() == EPERM);

Shouldn't be trying to enq to a stopped pktio.

Putting this specific return value and errno in the validation test
would mean that it needs to be defined in the API so that all pktios
behave the same. I don't think we should do that though as attempting
to enq to a pktio that's in the wrong state is a programming error and
we shouldn't force all implementations to guard against it.

> +
> +	/* start first and queue packets */
> +	odp_pktio_start(pktio[0]);

Check return value

> +	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;
> +	}
> +	/* stop second and check that packets did not arrive */
> +	if (num_ifaces == 1)
> +		odp_pktio_stop(pktio[0]);
> +	else
> +		odp_pktio_stop(pktio[1]);

I imagine some platforms will have a problem with this as it assumes the
enqueued packets haven't already been received and pre-scheduled before
the pktio was stopped.

It works in linux-generic only because the scheduler runs within the
context of this (single) test thread so everything is serialised.

> +	for (i = 0, pkts = 0; i < 1000; i++) {
> +		ev = odp_schedule(NULL, ODP_TIME_MSEC);
> +		if (ev != ODP_EVENT_INVALID) {
> +			if (odp_event_type(ev) == ODP_EVENT_PACKET) {
> +				if (!pktio_pkt_validate(pkt))
> +					pkts++;
> +			}
> +			odp_event_free(ev);
> +		}
> +	}
> +	if (pkts)
> +		CU_FAIL("pktio stopped, received unexpected events");
> +
> +	/* start second and get packets */
> +	if (num_ifaces == 1)
> +		odp_pktio_start(pktio[0]);
> +	else
> +		odp_pktio_start(pktio[1]);

This assumes that packets enqueued before the pktio was previously
stopped are retained somewhere until it's started again, which doesn't
make sense as expected behaviour given that in the meantime while the
pktio was stopped all received packets were dropped. IMO we should only
expect to receive packets that have arrived at the pktio since
odp_pktio_start() was last called.

> +	for (i = 0, pkts = 0; i < 1000; i++) {
> +		ev = odp_schedule(NULL, ODP_TIME_MSEC);
> +		if (ev != ODP_EVENT_INVALID) {
> +			if (odp_event_type(ev) == ODP_EVENT_PACKET) {
> +				pkt = odp_packet_from_event(ev);
> +				if (!pktio_pkt_validate(pkt))
> +					pkts++;
> +			}
> +			odp_event_free(ev);
> +		}
> +	}
> +	CU_ASSERT(pkts == alloc);
> +
> +	for (i = 0; i < num_ifaces; i++) {
> +		destroy_inq(pktio[i]);
> +		CU_ASSERT(odp_pktio_close(pktio[i]) == 0);
> +	}
> +}
> +
>  static int create_pool(const char *iface, int num)
>  {
>  	char pool_name[ODP_POOL_NAME_LEN];
> @@ -722,6 +835,7 @@ CU_TestInfo pktio_suite[] = {
>  	{"pktio promisc mode",	pktio_test_promisc},
>  	{"pktio mac",		pktio_test_mac},
>  	{"pktio inq_remdef",	pktio_test_inq_remdef},
> +	{"pktio start stop",	pktio_test_start_stop},
>  	CU_TEST_INFO_NULL
>  };
>

Patch

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index ebe34fa..4222463 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -626,6 +626,119 @@  void pktio_test_inq(void)
 	CU_ASSERT(odp_pktio_close(pktio) == 0);
 }
 
+static uint32_t pktio_pkt_validate(odp_packet_t pkt)
+{
+	size_t off;
+	pkt_head_t head;
+	pkt_tail_t tail;
+
+	if (pkt == ODP_PACKET_INVALID)
+		return -1;
+
+	off = odp_packet_l4_offset(pkt);
+	if (off ==  ODP_PACKET_OFFSET_INVALID)
+		return -1;
+
+	off += ODPH_UDPHDR_LEN;
+	if (odp_packet_copydata_out(pkt, off, sizeof(head), &head) != 0)
+		return -1;
+
+	if (head.magic != TEST_SEQ_MAGIC)
+		return -1;
+
+	if (odp_packet_len(pkt) == packet_len) {
+		off = packet_len - sizeof(tail);
+		if (odp_packet_copydata_out(pkt, off, sizeof(tail), &tail) != 0)
+			return -1;
+	}
+
+	return 0;
+}
+
+static void pktio_test_start_stop(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;
+
+	for (i = 0; i < num_ifaces; i++) {
+		pktio[i] = create_pktio(iface_name[i], ODP_QUEUE_TYPE_SCHED, 0);
+		CU_ASSERT(pktio[i] != ODP_PKTIO_INVALID);
+		create_inq(pktio[i],  ODP_QUEUE_TYPE_SCHED);
+	}
+
+	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);
+	}
+
+	outq = odp_pktio_outq_getdef(pktio[0]);
+
+	/* stop and check that we can not transmit*/
+	odp_pktio_stop(pktio[0]);
+	odp_errno_zero();
+	ret = odp_queue_enq_multi(outq, tx_ev, 10);
+	CU_ASSERT(ret == -1);
+	CU_ASSERT(odp_errno() == EPERM);
+
+	/* start first and queue packets */
+	odp_pktio_start(pktio[0]);
+	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;
+	}
+	/* stop second and check that packets did not arrive */
+	if (num_ifaces == 1)
+		odp_pktio_stop(pktio[0]);
+	else
+		odp_pktio_stop(pktio[1]);
+	for (i = 0, pkts = 0; i < 1000; i++) {
+		ev = odp_schedule(NULL, ODP_TIME_MSEC);
+		if (ev != ODP_EVENT_INVALID) {
+			if (odp_event_type(ev) == ODP_EVENT_PACKET) {
+				if (!pktio_pkt_validate(pkt))
+					pkts++;
+			}
+			odp_event_free(ev);
+		}
+	}
+	if (pkts)
+		CU_FAIL("pktio stopped, received unexpected events");
+
+	/* start second and get packets */
+	if (num_ifaces == 1)
+		odp_pktio_start(pktio[0]);
+	else
+		odp_pktio_start(pktio[1]);
+	for (i = 0, pkts = 0; i < 1000; i++) {
+		ev = odp_schedule(NULL, ODP_TIME_MSEC);
+		if (ev != ODP_EVENT_INVALID) {
+			if (odp_event_type(ev) == ODP_EVENT_PACKET) {
+				pkt = odp_packet_from_event(ev);
+				if (!pktio_pkt_validate(pkt))
+					pkts++;
+			}
+			odp_event_free(ev);
+		}
+	}
+	CU_ASSERT(pkts == alloc);
+
+	for (i = 0; i < num_ifaces; i++) {
+		destroy_inq(pktio[i]);
+		CU_ASSERT(odp_pktio_close(pktio[i]) == 0);
+	}
+}
+
 static int create_pool(const char *iface, int num)
 {
 	char pool_name[ODP_POOL_NAME_LEN];
@@ -722,6 +835,7 @@  CU_TestInfo pktio_suite[] = {
 	{"pktio promisc mode",	pktio_test_promisc},
 	{"pktio mac",		pktio_test_mac},
 	{"pktio inq_remdef",	pktio_test_inq_remdef},
+	{"pktio start stop",	pktio_test_start_stop},
 	CU_TEST_INFO_NULL
 };