diff mbox

[v2,1/3] linux-generic: classification: release the packet as soon as an error happens

Message ID 1462462509-13337-2-git-send-email-zoltan.kiss@linaro.org
State Accepted
Commit 9f4672bd502c44ea3242eb177d081be0b1855e2f
Headers show

Commit Message

Zoltan Kiss May 5, 2016, 3:35 p.m. UTC
Move release to _odp_packet_classifier(), because caller has no way to
know if new_pkt were allocated. In that case there would be a double free
on pkt, and new_pkt would be leaked.
The only exception is when cos == NULL or odp_packet_copy() fails, in that case
the packet is reenqued.

Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---

v2:
- add comment for this function
- use errno.h values
- let caller handle EINVAL and ENOMEM
- loopback reenques them
- fix in stats that CoS drop means actually discard

 platform/linux-generic/odp_classification.c | 41 ++++++++++++++++++++++-------
 platform/linux-generic/pktio/loop.c         | 16 ++++++++---
 2 files changed, 44 insertions(+), 13 deletions(-)
diff mbox

Patch

diff --git a/platform/linux-generic/odp_classification.c b/platform/linux-generic/odp_classification.c
index 25e1c2c..a3fe545 100644
--- a/platform/linux-generic/odp_classification.c
+++ b/platform/linux-generic/odp_classification.c
@@ -20,6 +20,7 @@ 
 #include <odp/api/shared_memory.h>
 #include <odp/helper/eth.h>
 #include <string.h>
+#include <errno.h>
 #include <odp/api/spinlock.h>
 
 #define LOCK(a)      odp_spinlock_lock(a)
@@ -746,6 +747,20 @@  int pktio_classifier_init(pktio_entry_t *entry)
 	return 0;
 }
 
+/**
+ * Classify packet and enqueue to the right queue
+ *
+ * entry		pktio where it arrived
+ * pkt		packet handle
+ *
+ * Return values:
+ * 0 on success, packet is consumed
+ * -ENOENT CoS dropped the packet
+ * -EFAULT Bug, packet is released
+ * -EINVAL Config error, packet is NOT released
+ * -ENOMEM Target CoS pool exhausted, packet is NOT released
+ */
+
 int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt)
 {
 	queue_entry_t *queue;
@@ -754,8 +769,10 @@  int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt)
 	odp_packet_t new_pkt;
 	uint8_t *pkt_addr;
 
-	if (entry == NULL)
-		return -1;
+	if (entry == NULL) {
+		odp_packet_free(pkt);
+		return -EFAULT;
+	}
 
 	pkt_hdr = odp_packet_hdr(pkt);
 	pkt_addr = odp_packet_data(pkt);
@@ -763,18 +780,17 @@  int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt)
 	/* Matching PMR and selecting the CoS for the packet*/
 	cos = pktio_select_cos(entry, pkt_addr, pkt_hdr);
 	if (cos == NULL)
-		return -1;
+		return -EINVAL;
 
-	if (cos->s.pool == NULL)
-		return -1;
-
-	if (cos->s.queue == NULL)
-		return -1;
+	if (cos->s.queue == NULL || cos->s.pool == NULL) {
+		odp_packet_free(pkt);
+		return -ENOENT;
+	}
 
 	if (odp_packet_pool(pkt) != cos->s.pool->s.pool_hdl) {
 		new_pkt = odp_packet_copy(pkt, cos->s.pool->s.pool_hdl);
 		if (new_pkt == ODP_PACKET_INVALID)
-			return -1;
+			return -ENOMEM;
 		odp_packet_free(pkt);
 	} else {
 		new_pkt = pkt;
@@ -782,7 +798,12 @@  int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt)
 
 	/* Enqueuing the Packet based on the CoS */
 	queue = cos->s.queue;
-	return queue_enq(queue, odp_buf_to_hdr((odp_buffer_t)new_pkt), 0);
+	if (queue_enq(queue, odp_buf_to_hdr((odp_buffer_t)new_pkt), 0)) {
+		odp_packet_free(new_pkt);
+		return -EFAULT;
+	} else {
+		return 0;
+	}
 }
 
 cos_t *pktio_select_cos(pktio_entry_t *entry, const uint8_t *pkt_addr,
diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/pktio/loop.c
index 5819e66..f7ccbd9 100644
--- a/platform/linux-generic/pktio/loop.c
+++ b/platform/linux-generic/pktio/loop.c
@@ -73,19 +73,29 @@  static int loopback_recv(pktio_entry_t *pktio_entry, odp_packet_t pkts[],
 
 	if (pktio_cls_enabled(pktio_entry)) {
 		for (i = 0, j = 0; i < nbr; i++) {
+			int ret;
 			pkt = _odp_packet_from_buffer(odp_hdr_to_buf
 						      (hdr_tbl[i]));
 			pkt_hdr = odp_packet_hdr(pkt);
 			packet_parse_reset(pkt_hdr);
 			packet_parse_l2(pkt_hdr);
-			if (!_odp_packet_classifier(pktio_entry, pkt)) {
+			ret = _odp_packet_classifier(pktio_entry, pkt);
+			switch (ret) {
+			case 0:
 				packet_set_ts(pkt_hdr, ts);
 				pktio_entry->s.stats.in_octets +=
 					odp_packet_len(pkt);
-			} else {
+				break;
+			case -ENOENT:
+				pktio_entry->s.stats.in_discards +=
+					odp_packet_len(pkt);
+				break;
+			case -EFAULT:
 				pktio_entry->s.stats.in_errors +=
 					odp_packet_len(pkt);
-				odp_packet_free(pkt);
+				break;
+			default:
+				ret = queue_enq(qentry, hdr_tbl[i], 0);
 			}
 		}
 		nbr = j;