[dpdk-dev] [PATCH] event/sw: fix credit tracking in port dequeue

[dpdk-dev] [PATCH] event/sw: fix credit tracking in port dequeue

[Harry van Haaren]

This patch targets the next-eventdev tree.

Single-link optimized ports previously did not correctly track
credits when dequeued, and re-enqueued as a FORWARD type. This
could "inflate" the number of credits in the system.

A unit test is added to reproduce and verify the issue. The
fixed implementation counts FORWARD packets and reduces the
number of credits the port has if it is of single-link type.

Fixes: 656af9180014 ("event/sw: add worker core functions")

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 drivers/event/sw/sw_evdev_worker.c |  5 ++++
 test/test/test_eventdev_sw.c       | 58 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/event/sw/sw_evdev_worker.c b/drivers/event/sw/sw_evdev_worker.c
index 9cb6bef..b738506 100644
--- a/drivers/event/sw/sw_evdev_worker.c
+++ b/drivers/event/sw/sw_evdev_worker.c
@@ -87,6 +87,7 @@ sw_event_enqueue_burst(void *port, const struct rte_event ev[], uint16_t num)
 			return 0;
 	}
 
+	uint32_t forwards = 0;
 	for (i = 0; i < num; i++) {
 		int op = ev[i].op;
 		int outstanding = p->outstanding_releases > 0;
@@ -95,6 +96,7 @@ sw_event_enqueue_burst(void *port, const struct rte_event ev[], uint16_t num)
 		p->inflight_credits -= (op == RTE_EVENT_OP_NEW);
 		p->inflight_credits += (op == RTE_EVENT_OP_RELEASE) *
 					outstanding;
+		forwards += (op == RTE_EVENT_OP_FORWARD);
 
 		new_ops[i] = sw_qe_flag_map[op];
 		new_ops[i] &= ~(invalid_qid << QE_FLAG_VALID_SHIFT);
@@ -113,6 +115,9 @@ sw_event_enqueue_burst(void *port, const struct rte_event ev[], uint16_t num)
 		}
 	}
 
+	/* handle directed port forward credits */
+	p->inflight_credits -= forwards * p->is_directed;
+
 	/* returns number of events actually enqueued */
 	uint32_t enq = qe_ring_enqueue_burst_with_ops(p->rx_worker_ring, ev, i,
 					     new_ops);
diff --git a/test/test/test_eventdev_sw.c b/test/test/test_eventdev_sw.c
index b187d02..aaa9729 100644
--- a/test/test/test_eventdev_sw.c
+++ b/test/test/test_eventdev_sw.c
@@ -548,6 +548,57 @@ test_single_directed_packet(struct test *t)
 	return 0;
 }
 
+static int
+test_directed_forward_credits(struct test *t)
+{
+	uint32_t i;
+	int32_t err;
+
+	if (init(t, 1, 1) < 0 ||
+			create_ports(t, 1) < 0 ||
+			create_directed_qids(t, 1, t->port) < 0)
+		return -1;
+
+	if (rte_event_dev_start(evdev) < 0) {
+		printf("%d: Error with start call\n", __LINE__);
+		return -1;
+	}
+
+	struct rte_event ev = {
+			.op = RTE_EVENT_OP_NEW,
+			.queue_id = 0,
+	};
+
+	for (i = 0; i < 1000; i++) {
+		err = rte_event_enqueue_burst(evdev, 0, &ev, 1);
+		if (err < 0) {
+			printf("%d: error failed to enqueue\n", __LINE__);
+			return -1;
+		}
+		rte_event_schedule(evdev);
+		struct test_event_dev_stats stats;
+		err = test_event_dev_stats_get(evdev, &stats);
+		if (err) {
+			printf("%d: error failed to get stats\n", __LINE__);
+			return -1;
+		}
+
+		uint32_t deq_pkts;
+		deq_pkts = rte_event_dequeue_burst(evdev, 0, &ev, 1, 0);
+		if (deq_pkts != 1) {
+			printf("%d: error failed to deq\n", __LINE__);
+			return -1;
+		}
+
+		/* re-write event to be a forward, and continue looping it */
+		ev.op = RTE_EVENT_OP_FORWARD;
+	}
+
+	//rte_event_dev_dump(0, stdout);
+	cleanup(t);
+	return 0;
+}
+
 
 static int
 test_priority_directed(struct test *t)
@@ -3025,13 +3076,18 @@ test_sw_eventdev(void)
 		}
 	}
 	t->mbuf_pool = eventdev_func_mempool;
-
 	printf("*** Running Single Directed Packet test...\n");
 	ret = test_single_directed_packet(t);
 	if (ret != 0) {
 		printf("ERROR - Single Directed Packet test FAILED.\n");
 		return ret;
 	}
+	printf("*** Running Directed Forward Credit test...\n");
+	ret = test_directed_forward_credits(t);
+	if (ret != 0) {
+		printf("ERROR - Directed Forward Credit test FAILED.\n");
+		return ret;
+	}
 	printf("*** Running Single Load Balanced Packet test...\n");
 	ret = single_packet(t);
 	if (ret != 0) {
-- 
2.7.4

Re: [dpdk-dev] [PATCH] event/sw: fix credit tracking in port dequeue

[Eads, Gage]

A couple nit-picks on the test, but once resolved:
Acked-by: Gage Eads <gage.eads@intel.com>

>  -----Original Message-----
>  From: Van Haaren, Harry
>  Sent: Monday, May 29, 2017 10:17 AM
>  To: dev@dpdk.org
>  Cc: jerin.jacob@caviumnetworks.com; Eads, Gage <gage.eads@intel.com>;
>  Van Haaren, Harry <harry.van.haaren@intel.com>
>  Subject: [PATCH] event/sw: fix credit tracking in port dequeue
>  
>  This patch targets the next-eventdev tree.
>  
>  Single-link optimized ports previously did not correctly track credits when
>  dequeued, and re-enqueued as a FORWARD type. This could "inflate" the
>  number of credits in the system.
>  
>  A unit test is added to reproduce and verify the issue. The fixed implementation
>  counts FORWARD packets and reduces the number of credits the port has if it is
>  of single-link type.
>  
>  Fixes: 656af9180014 ("event/sw: add worker core functions")
>  
>  Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>  ---
>   drivers/event/sw/sw_evdev_worker.c |  5 ++++
>   test/test/test_eventdev_sw.c       | 58
>  +++++++++++++++++++++++++++++++++++++-
>   2 files changed, 62 insertions(+), 1 deletion(-)
>  
>  diff --git a/drivers/event/sw/sw_evdev_worker.c
>  b/drivers/event/sw/sw_evdev_worker.c
>  index 9cb6bef..b738506 100644
>  --- a/drivers/event/sw/sw_evdev_worker.c
>  +++ b/drivers/event/sw/sw_evdev_worker.c
>  @@ -87,6 +87,7 @@ sw_event_enqueue_burst(void *port, const struct
>  rte_event ev[], uint16_t num)
>   			return 0;
>   	}
>  
>  +	uint32_t forwards = 0;
>   	for (i = 0; i < num; i++) {
>   		int op = ev[i].op;
>   		int outstanding = p->outstanding_releases > 0; @@ -95,6
>  +96,7 @@ sw_event_enqueue_burst(void *port, const struct rte_event ev[],
>  uint16_t num)
>   		p->inflight_credits -= (op == RTE_EVENT_OP_NEW);
>   		p->inflight_credits += (op == RTE_EVENT_OP_RELEASE) *
>   					outstanding;
>  +		forwards += (op == RTE_EVENT_OP_FORWARD);
>  
>   		new_ops[i] = sw_qe_flag_map[op];
>   		new_ops[i] &= ~(invalid_qid << QE_FLAG_VALID_SHIFT); @@ -
>  113,6 +115,9 @@ sw_event_enqueue_burst(void *port, const struct rte_event
>  ev[], uint16_t num)
>   		}
>   	}
>  
>  +	/* handle directed port forward credits */
>  +	p->inflight_credits -= forwards * p->is_directed;
>  +
>   	/* returns number of events actually enqueued */
>   	uint32_t enq = qe_ring_enqueue_burst_with_ops(p->rx_worker_ring,
>  ev, i,
>   					     new_ops);
>  diff --git a/test/test/test_eventdev_sw.c b/test/test/test_eventdev_sw.c index
>  b187d02..aaa9729 100644
>  --- a/test/test/test_eventdev_sw.c
>  +++ b/test/test/test_eventdev_sw.c
>  @@ -548,6 +548,57 @@ test_single_directed_packet(struct test *t)
>   	return 0;
>   }
>  
>  +static int
>  +test_directed_forward_credits(struct test *t) {
>  +	uint32_t i;
>  +	int32_t err;
>  +
>  +	if (init(t, 1, 1) < 0 ||
>  +			create_ports(t, 1) < 0 ||
>  +			create_directed_qids(t, 1, t->port) < 0)
>  +		return -1;
>  +
>  +	if (rte_event_dev_start(evdev) < 0) {
>  +		printf("%d: Error with start call\n", __LINE__);
>  +		return -1;
>  +	}
>  +
>  +	struct rte_event ev = {
>  +			.op = RTE_EVENT_OP_NEW,
>  +			.queue_id = 0,
>  +	};
>  +
>  +	for (i = 0; i < 1000; i++) {
>  +		err = rte_event_enqueue_burst(evdev, 0, &ev, 1);
>  +		if (err < 0) {
>  +			printf("%d: error failed to enqueue\n", __LINE__);
>  +			return -1;
>  +		}
>  +		rte_event_schedule(evdev);
>  +		struct test_event_dev_stats stats;
>  +		err = test_event_dev_stats_get(evdev, &stats);

Does this stats get call serve a purpose?

>  +		if (err) {
>  +			printf("%d: error failed to get stats\n", __LINE__);
>  +			return -1;
>  +		}
>  +
>  +		uint32_t deq_pkts;
>  +		deq_pkts = rte_event_dequeue_burst(evdev, 0, &ev, 1, 0);
>  +		if (deq_pkts != 1) {
>  +			printf("%d: error failed to deq\n", __LINE__);
>  +			return -1;
>  +		}
>  +
>  +		/* re-write event to be a forward, and continue looping it */
>  +		ev.op = RTE_EVENT_OP_FORWARD;
>  +	}
>  +
>  +	//rte_event_dev_dump(0, stdout);

Comment can be deleted

[dpdk-dev] [PATCH v2] event/sw: fix credit tracking in port dequeue

[Harry van Haaren]

Single-link optimized ports previously did not correctly track
credits when dequeued, and re-enqueued as a FORWARD type. This
could "inflate" the number of credits in the system.

A unit test is added to reproduce and verify the issue, and the
fixed implementation counts FORWARD packets, and reduces the
number of credits the port has if it is of single-link type.

Fixes: 656af9180014 ("event/sw: add worker core functions")

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Acked-by: Gage Eads <gage.eads@intel.com>

---

v2:
- Remove useless stats retrieving code
- Remove commented code
- Added Ack

---
 drivers/event/sw/sw_evdev_worker.c |  5 ++++
 test/test/test_eventdev_sw.c       | 51 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/event/sw/sw_evdev_worker.c b/drivers/event/sw/sw_evdev_worker.c
index 9cb6bef..b738506 100644
--- a/drivers/event/sw/sw_evdev_worker.c
+++ b/drivers/event/sw/sw_evdev_worker.c
@@ -87,6 +87,7 @@ sw_event_enqueue_burst(void *port, const struct rte_event ev[], uint16_t num)
 			return 0;
 	}
 
+	uint32_t forwards = 0;
 	for (i = 0; i < num; i++) {
 		int op = ev[i].op;
 		int outstanding = p->outstanding_releases > 0;
@@ -95,6 +96,7 @@ sw_event_enqueue_burst(void *port, const struct rte_event ev[], uint16_t num)
 		p->inflight_credits -= (op == RTE_EVENT_OP_NEW);
 		p->inflight_credits += (op == RTE_EVENT_OP_RELEASE) *
 					outstanding;
+		forwards += (op == RTE_EVENT_OP_FORWARD);
 
 		new_ops[i] = sw_qe_flag_map[op];
 		new_ops[i] &= ~(invalid_qid << QE_FLAG_VALID_SHIFT);
@@ -113,6 +115,9 @@ sw_event_enqueue_burst(void *port, const struct rte_event ev[], uint16_t num)
 		}
 	}
 
+	/* handle directed port forward credits */
+	p->inflight_credits -= forwards * p->is_directed;
+
 	/* returns number of events actually enqueued */
 	uint32_t enq = qe_ring_enqueue_burst_with_ops(p->rx_worker_ring, ev, i,
 					     new_ops);
diff --git a/test/test/test_eventdev_sw.c b/test/test/test_eventdev_sw.c
index b187d02..2cb9f59 100644
--- a/test/test/test_eventdev_sw.c
+++ b/test/test/test_eventdev_sw.c
@@ -548,6 +548,50 @@ test_single_directed_packet(struct test *t)
 	return 0;
 }
 
+static int
+test_directed_forward_credits(struct test *t)
+{
+	uint32_t i;
+	int32_t err;
+
+	if (init(t, 1, 1) < 0 ||
+			create_ports(t, 1) < 0 ||
+			create_directed_qids(t, 1, t->port) < 0)
+		return -1;
+
+	if (rte_event_dev_start(evdev) < 0) {
+		printf("%d: Error with start call\n", __LINE__);
+		return -1;
+	}
+
+	struct rte_event ev = {
+			.op = RTE_EVENT_OP_NEW,
+			.queue_id = 0,
+	};
+
+	for (i = 0; i < 1000; i++) {
+		err = rte_event_enqueue_burst(evdev, 0, &ev, 1);
+		if (err < 0) {
+			printf("%d: error failed to enqueue\n", __LINE__);
+			return -1;
+		}
+		rte_event_schedule(evdev);
+
+		uint32_t deq_pkts;
+		deq_pkts = rte_event_dequeue_burst(evdev, 0, &ev, 1, 0);
+		if (deq_pkts != 1) {
+			printf("%d: error failed to deq\n", __LINE__);
+			return -1;
+		}
+
+		/* re-write event to be a forward, and continue looping it */
+		ev.op = RTE_EVENT_OP_FORWARD;
+	}
+
+	cleanup(t);
+	return 0;
+}
+
 
 static int
 test_priority_directed(struct test *t)
@@ -3025,13 +3069,18 @@ test_sw_eventdev(void)
 		}
 	}
 	t->mbuf_pool = eventdev_func_mempool;
-
 	printf("*** Running Single Directed Packet test...\n");
 	ret = test_single_directed_packet(t);
 	if (ret != 0) {
 		printf("ERROR - Single Directed Packet test FAILED.\n");
 		return ret;
 	}
+	printf("*** Running Directed Forward Credit test...\n");
+	ret = test_directed_forward_credits(t);
+	if (ret != 0) {
+		printf("ERROR - Directed Forward Credit test FAILED.\n");
+		return ret;
+	}
 	printf("*** Running Single Load Balanced Packet test...\n");
 	ret = single_packet(t);
 	if (ret != 0) {
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] event/sw: fix credit tracking in port dequeue

[Jerin Jacob]

-----Original Message-----
> Date: Thu, 1 Jun 2017 16:45:54 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, gage.eads@intel.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH v2] event/sw: fix credit tracking in port dequeue
> X-Mailer: git-send-email 2.7.4
> 
> Single-link optimized ports previously did not correctly track
> credits when dequeued, and re-enqueued as a FORWARD type. This
> could "inflate" the number of credits in the system.
> 
> A unit test is added to reproduce and verify the issue, and the
> fixed implementation counts FORWARD packets, and reduces the
> number of credits the port has if it is of single-link type.
> 
> Fixes: 656af9180014 ("event/sw: add worker core functions")
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Acked-by: Gage Eads <gage.eads@intel.com>

Applied to dpdk-next-eventdev/master. Thanks.