From patchwork Tue Jul 30 12:22:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konstantin Ananyev X-Patchwork-Id: 142760 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B169E456F7; Tue, 30 Jul 2024 15:58:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 401734065B; Tue, 30 Jul 2024 15:58:02 +0200 (CEST) Received: from forward501d.mail.yandex.net (forward501d.mail.yandex.net [178.154.239.209]) by mails.dpdk.org (Postfix) with ESMTP id 373B3400D7 for ; Tue, 30 Jul 2024 14:22:55 +0200 (CEST) Received: from mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net (mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net [IPv6:2a02:6b8:c12:3c91:0:640:38da:0]) by forward501d.mail.yandex.net (Yandex) with ESMTPS id 531B660CC7; Tue, 30 Jul 2024 15:22:54 +0300 (MSK) Received: by mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net (smtp/Yandex) with ESMTPSA id cMUdT3ZoOSw0-FUlyJKAB; Tue, 30 Jul 2024 15:22:53 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1722342173; bh=+OJH8UwOW85R8+9lth7oImxW3gZW8isGONTRa+tL5Ro=; h=Message-Id:Date:In-Reply-To:Cc:Subject:References:To:From; b=Ve7wMWpIpY9/EJtoPGnJmgSUvqqeVwjRwZhdTLXXded2CkZ759/IZl0HZSbe8Ovjr XEJn39ZeDM+RDxDHGARzeqz18uSoJWyODGdXu5LXR7l6a6erBdXjCZvnmt0+V4vEec 9rJaZGJmKgruB1MZoLhogv9odfBilN/lyP8bxFns= Authentication-Results: mail-nwsmtp-smtp-production-main-42.myt.yp-c.yandex.net; dkim=pass header.i=@yandex.ru From: Konstantin Ananyev To: dev@dpdk.org Cc: songx.jiale@intel.com, Konstantin Ananyev Subject: [PATCH v2 2/2] examples/l3fwd: fix read beyond array boundaries in ACL mode Date: Tue, 30 Jul 2024 13:22:35 +0100 Message-Id: <20240730122235.1084-3-konstantin.v.ananyev@yandex.ru> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru> References: <20240726194302.920-1-konstantin.v.ananyev@yandex.ru> <20240730122235.1084-1-konstantin.v.ananyev@yandex.ru> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org From: Konstantin Ananyev 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 Signed-off-by: Konstantin Ananyev Acked-by: Stephen Hemminger --- 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); } } }