From patchwork Wed Dec 13 11:07:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Qian Hao X-Patchwork-Id: 135133 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 030AE436DE; Wed, 13 Dec 2023 12:07:47 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BFBAA42E8D; Wed, 13 Dec 2023 12:07:46 +0100 (CET) Received: from m126.mail.126.com (m126.mail.126.com [220.181.12.36]) by mails.dpdk.org (Postfix) with ESMTP id BB56C4021D for ; Wed, 13 Dec 2023 12:07:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=126.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=BbDOf 2QztonpdnHe9XgT04FW6YbTFH9hNd6WXR6leLI=; b=LFardM+1h85ZTS1DUhMC/ aWDhHW1ZenmB10opFkx/EyOVBqlPr9M8n6M5ueZfuLObZ+J8FWsoVj08qwQl2Ewx PGYsWA5aeKLzRjkNsTocB+zS7QB89V/wl6tjwtLJo+8FdpJ1jajvAY5Bvb7YYF4S +lqSaB6dCSmTBwAF61qCIM= Received: from R750-427Server16.. (unknown [58.213.8.49]) by zwqz-smtp-mta-g3-0 (Coremail) with SMTP id _____wD3_0JvkHllR9yGDw--.11835S2; Wed, 13 Dec 2023 19:07:34 +0800 (CST) From: Qian Hao To: dev@dpdk.org Cc: vfialko@marvell.com Subject: [PATCH v3] examples/packet_ordering: fix segfault in disable_reorder mode Date: Wed, 13 Dec 2023 19:07:18 +0800 Message-Id: <20231213110718.18859-1-qi_an_hao@126.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231213104945.16634-1-qi_an_hao@126.com> References: <20231213104945.16634-1-qi_an_hao@126.com> MIME-Version: 1.0 X-CM-TRANSID: _____wD3_0JvkHllR9yGDw--.11835S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxWFy8CF1fArWktFyfZr47urg_yoW5ZF43pF Z8K34xtr48ZF1Fgrs3Ja47Xry5XrWrXF1xurZ5Wwn0kw45Ca45ZrW0qFy5uFy8CFWkGr4U Ar45XF9I9FWqkF7anT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07U-6pQUUUUU= X-Originating-IP: [58.213.8.49] X-CM-SenderInfo: ptlbt0pbkd0qqrswhudrp/1tbi4xVFGVpEFE8w+wABsl 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 The packet_ordering example works in two modes (opt via --disable-reorder): - When reorder is enabled: rx_thread - N*worker_thread - send_thread - When reorder is disabled: rx_thread - N*worker_thread - tx_thread N parallel worker_thread(s) generate out-of-order packets. When reorder is enabled, send_thread uses sequence number generated in rx_thread (L459) to enforce packet ordering. Otherwise rx_thread just sends any packet it receives. rx_thread writes sequence number into a dynamic field, which is only registered by calling rte_reorder_create() (Line 741) when reorder is enabled. However, rx_thread marks sequence number onto each packet no matter whether reorder is enabled, overwriting the leading bytes in packet mbufs when reorder is disabled, resulting in segfaults when PMD tries to DMA packets. `if (!disable_reorder_flag) {...}` is added in rx_thread to fix the bug. The test is inlined by the compiler to prevent any performance loss. Signed-off-by: Qian Hao Acked-by: Volodymyr Fialko --- examples/packet_ordering/main.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/examples/packet_ordering/main.c b/examples/packet_ordering/main.c index d2fd6f77e4..f839db9102 100644 --- a/examples/packet_ordering/main.c +++ b/examples/packet_ordering/main.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -427,8 +428,8 @@ int_handler(int sig_num) * The mbufs are then passed to the worker threads via the rx_to_workers * ring. */ -static int -rx_thread(struct rte_ring *ring_out) +static __rte_always_inline int +rx_thread(struct rte_ring *ring_out, bool disable_reorder_flag) { uint32_t seqn = 0; uint16_t i, ret = 0; @@ -454,9 +455,11 @@ rx_thread(struct rte_ring *ring_out) } app_stats.rx.rx_pkts += nb_rx_pkts; - /* mark sequence number */ - for (i = 0; i < nb_rx_pkts; ) - *rte_reorder_seqn(pkts[i++]) = seqn++; + /* mark sequence number if reorder is enabled */ + if (!disable_reorder_flag) { + for (i = 0; i < nb_rx_pkts;) + *rte_reorder_seqn(pkts[i++]) = seqn++; + } /* enqueue to rx_to_workers ring */ ret = rte_ring_enqueue_burst(ring_out, @@ -473,6 +476,18 @@ rx_thread(struct rte_ring *ring_out) return 0; } +static __rte_noinline int +rx_thread_reorder(struct rte_ring *ring_out) +{ + return rx_thread(ring_out, false); +} + +static __rte_noinline int +rx_thread_reorder_disabled(struct rte_ring *ring_out) +{ + return rx_thread(ring_out, true); +} + /** * This thread takes bursts of packets from the rx_to_workers ring and * Changes the input port value to output port value. And feds it to @@ -772,8 +787,11 @@ main(int argc, char **argv) (void *)&send_args, last_lcore_id); } - /* Start rx_thread() on the main core */ - rx_thread(rx_to_workers); + /* Start rx_thread_xxx() on the main core */ + if (disable_reorder) + rx_thread_reorder_disabled(rx_to_workers); + else + rx_thread_reorder(rx_to_workers); RTE_LCORE_FOREACH_WORKER(lcore_id) { if (rte_eal_wait_lcore(lcore_id) < 0)