DPDK patches and discussions
 help / color / mirror / Atom feed
From: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
To: dev@dpdk.org
Cc: songx.jiale@intel.com,
	Konstantin Ananyev <konstantin.ananyev@huawei.com>
Subject: [PATCH v2 2/2] examples/l3fwd: fix read beyond array boundaries in ACL mode
Date: Tue, 30 Jul 2024 13:22:35 +0100	[thread overview]
Message-ID: <20240730122235.1084-3-konstantin.v.ananyev@yandex.ru> (raw)
In-Reply-To: <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru>

From: Konstantin Ananyev <konstantin.ananyev@huawei.com>

With commit: ACL mode now can use send_packets_multi().
What I missed with that changes: send_packets_multi() can't deal
properly with input dst_port[i] == BAD_PORT (though it can set
it itself), as it uses dst_port[i] values to read L2 addresses for the port
and assumes dst_port[] to contain valid only values.
To fix that just add a check that all dst_port[] entries are valid before
calling : send_packets_multi(). Otherwhise  use  send_packets_single().
An alternative, and probably more logical approach would be to
re-arrange send_packets_multi() so that it updates L2 packet headers
at the very last state - when dst_port[] are finialized.
But that would affect all other modes, but that would affect all other
modes and will require much more code changes and testing.

Bugzilla ID: 1502
Fixes: aa7c6077c19b ("examples/l3fwd: avoid packets reorder in ACL mode")

Reported-by: Song Jiale <songx.jiale@intel.com>
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
 examples/l3fwd/l3fwd_acl.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/examples/l3fwd/l3fwd_acl.c b/examples/l3fwd/l3fwd_acl.c
index baa01e6dde..45013ac06e 100644
--- a/examples/l3fwd/l3fwd_acl.c
+++ b/examples/l3fwd/l3fwd_acl.c
@@ -993,11 +993,15 @@ dump_denied_pkt(const struct rte_mbuf *pkt, uint32_t res)
 #endif
 }
 
-static inline void
+/*
+ * run packets through ACL classify.
+ * returns number of packets to be dropped (hops[i] == BAD_PORT)
+ */
+static inline uint32_t
 acl_process_pkts(struct rte_mbuf *pkts[MAX_PKT_BURST],
 	uint16_t hops[MAX_PKT_BURST], uint32_t num, int32_t socketid)
 {
-	uint32_t i, n4, n6, res;
+	uint32_t i, k, n4, n6, res;
 	struct acl_search_t acl_search;
 
 	/* split packets burst depending on packet type (IPv4/IPv6) */
@@ -1020,6 +1024,7 @@ acl_process_pkts(struct rte_mbuf *pkts[MAX_PKT_BURST],
 	/* combine lookup results back, into one array of next hops */
 	n4 = 0;
 	n6 = 0;
+	k = 0;
 	for (i = 0; i != num; i++) {
 		switch (acl_search.types[i]) {
 		case TYPE_IPV4:
@@ -1034,21 +1039,33 @@ acl_process_pkts(struct rte_mbuf *pkts[MAX_PKT_BURST],
 		if (likely((res & ACL_DENY_SIGNATURE) == 0 && res != 0))
 			hops[i] = res - FWD_PORT_SHIFT;
 		else {
+			/* bad or deined by ACL rule packets */
 			hops[i] = BAD_PORT;
 			dump_denied_pkt(pkts[i], res);
+			k++;
 		}
 	}
+
+	return k;
 }
 
+/*
+ * send_packets_multi() can't deal properly with hops[i] == BAD_PORT
+ * (it assumes input hops[] contain only valid port numbers),
+ * so it is ok to use it only when there are no denied packets.
+ */
 static inline void
 acl_send_packets(struct lcore_conf *qconf, struct rte_mbuf *pkts[],
-	uint16_t hops[], uint32_t num)
+	uint16_t hops[], uint32_t num, uint32_t nb_drop)
 {
 #if defined ACL_SEND_MULTI
-	send_packets_multi(qconf, pkts, hops, num);
+	if (nb_drop == 0)
+		send_packets_multi(qconf, pkts, hops, num);
+	else
 #else
-	send_packets_single(qconf, pkts, hops, num);
+		RTE_SET_USED(nb_drop);
 #endif
+		send_packets_single(qconf, pkts, hops, num);
 }
 
 /* main processing loop */
@@ -1059,7 +1076,7 @@ acl_main_loop(__rte_unused void *dummy)
 	uint16_t hops[SENDM_PORT_OVERHEAD(MAX_PKT_BURST)];
 	unsigned int lcore_id;
 	uint64_t prev_tsc, diff_tsc, cur_tsc;
-	int i, nb_rx;
+	int i, nb_drop, nb_rx;
 	uint16_t portid;
 	uint16_t queueid;
 	struct lcore_conf *qconf;
@@ -1122,10 +1139,10 @@ acl_main_loop(__rte_unused void *dummy)
 				pkts_burst, MAX_PKT_BURST);
 
 			if (nb_rx > 0) {
-				acl_process_pkts(pkts_burst, hops, nb_rx,
-					socketid);
+				nb_drop = acl_process_pkts(pkts_burst, hops,
+					nb_rx, socketid);
 				acl_send_packets(qconf, pkts_burst, hops,
-					nb_rx);
+					nb_rx, nb_drop);
 			}
 		}
 	}
-- 
2.35.3


  parent reply	other threads:[~2024-07-30 13:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-26 19:43 [PATCH] examples/l3fwd: fix read beyond array bondaries Konstantin Ananyev
2024-07-30 12:22 ` [PATCH v2 0/2] examples/l3fwd fixes for ACL mode Konstantin Ananyev
2024-07-30 12:22   ` [PATCH v2 1/2] examples/l3fwd: fix read beyond array bondaries Konstantin Ananyev
2024-10-10  0:30     ` Stephen Hemminger
2024-07-30 12:22   ` Konstantin Ananyev [this message]
2024-10-10  0:30     ` [PATCH v2 2/2] examples/l3fwd: fix read beyond array boundaries in ACL mode Stephen Hemminger
2024-10-12  2:43     ` Stephen Hemminger
2024-11-07 18:50   ` [PATCH v3 0/2] examples/l3fwd fixes for " Konstantin Ananyev
2024-11-07 18:50     ` [PATCH v3 1/2] examples/l3fwd: fix read beyond array bondaries Konstantin Ananyev
2024-11-07 18:50     ` [PATCH v3 2/2] examples/l3fwd: fix read beyond array boundaries in ACL mode Konstantin Ananyev
2024-11-11 14:06     ` [PATCH v3 0/2] examples/l3fwd fixes for " Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240730122235.1084-3-konstantin.v.ananyev@yandex.ru \
    --to=konstantin.v.ananyev@yandex.ru \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@huawei.com \
    --cc=songx.jiale@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).