From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2AD8042214; Thu, 31 Aug 2023 18:47:51 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ACB5940285; Thu, 31 Aug 2023 18:47:50 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id 2D2FA4027B for ; Thu, 31 Aug 2023 18:47:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693500469; x=1725036469; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=e1gYnforJyllAPJWL5wHAxr6FexckH70KJnb0FPNKRU=; b=LlZTxhZqdI6b3iWTp7paaWRmoa8Jp0PkCcAqc9Ylb1x8IJm/S27tM/0S K4mWUxzL6NpCmfToqmJ2mDJf1LNe8rUGHAzr4vbR9E7IA7hs6HLUAS7NV NmIf0WtMOfELe2zhhvruSIywPQ5yOToAygoeHHzvhDUAZcxLm84FNtnLH qJe4uGNPjLWQ6AWqoIXndp5e59F388mS/khPJafjByvQxWiIfL7y9Zq8M 1Q9hU/zqZtFvRk13+3nsMUGeFd29kBhrmUvQMQEAj3m4mx/XJu5Hwhr/U 9/YBmz+9isOW28iHOXUjXRapKE+YhYDjPg3xaXRIRHpO7MxZGUnYFWGaO Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="355499851" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="355499851" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Aug 2023 09:47:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10819"; a="863163395" X-IronPort-AV: E=Sophos;i="6.02,217,1688454000"; d="scan'208";a="863163395" Received: from silpixa00401454.ir.intel.com ([10.55.128.147]) by orsmga004.jf.intel.com with ESMTP; 31 Aug 2023 09:47:46 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Subject: [PATCH 2/2] event/sw: fix ordering corruption with op release Date: Thu, 31 Aug 2023 17:47:36 +0100 Message-Id: <20230831164736.2472671-2-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230831164736.2472671-1-harry.van.haaren@intel.com> References: <20230831164736.2472671-1-harry.van.haaren@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 This commit changes the logic in the scheduler to always reset reorder-buffer entries in the QE_FLAG_COMPLETE path, and not just the QE_FLAG_VALID path. A release event is a COMPLETE but not VALID (no new event). As a result, release events previously left the history-list in an inconsistent state, and future events with op type of forward could be incorrectly reordered. Signed-off-by: Harry van Haaren --- drivers/event/sw/sw_evdev_scheduler.c | 45 ++++++++++++++++----------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/drivers/event/sw/sw_evdev_scheduler.c b/drivers/event/sw/sw_evdev_scheduler.c index 8bc21944f5..9ee6698525 100644 --- a/drivers/event/sw/sw_evdev_scheduler.c +++ b/drivers/event/sw/sw_evdev_scheduler.c @@ -360,10 +360,15 @@ __pull_port_lb(struct sw_evdev *sw, uint32_t port_id, int allow_reorder) while (port->pp_buf_count) { const struct rte_event *qe = &port->pp_buf[port->pp_buf_start]; - struct sw_hist_list_entry *hist_entry = NULL; uint8_t flags = qe->op; const uint16_t eop = !(flags & QE_FLAG_NOT_EOP); - int needs_reorder = 0; + + /* rob_entry being NULL or a value is used as the distinction + * between reordering being required (mark ROB as ready) or + * just an Atomic completion. + */ + struct reorder_buffer_entry *rob_ptr = NULL; + /* if no-reordering, having PARTIAL == NEW */ if (!allow_reorder && !eop) flags = QE_FLAG_VALID; @@ -386,6 +391,7 @@ __pull_port_lb(struct sw_evdev *sw, uint32_t port_id, int allow_reorder) const uint32_t hist_tail = port->hist_tail & (SW_PORT_HIST_LIST - 1); + struct sw_hist_list_entry *hist_entry; hist_entry = &port->hist_list[hist_tail]; const uint32_t hist_qid = hist_entry->qid; const uint32_t hist_fid = hist_entry->fid; @@ -396,17 +402,24 @@ __pull_port_lb(struct sw_evdev *sw, uint32_t port_id, int allow_reorder) if (fid->pcount == 0) fid->cq = -1; + /* Assign current hist-list entry to the rob_entry, to + * allow VALID code below re-use it for checks. + */ + rob_ptr = hist_entry->rob_entry; + + /* Clear the rob entry in this COMPLETE flag phase, as + * RELEASE events must clear hist-list, but MIGHT NOT + * contain a VALID flag too. + */ + hist_entry->rob_entry = NULL; + if (allow_reorder) { - /* set reorder ready if an ordered QID */ - uintptr_t rob_ptr = - (uintptr_t)hist_entry->rob_entry; const uintptr_t valid = (rob_ptr != 0); - needs_reorder = valid; - rob_ptr |= - ((valid - 1) & (uintptr_t)&dummy_rob); + uintptr_t tmp = (uintptr_t)rob_ptr; + tmp |= ((valid - 1) & (uintptr_t)&dummy_rob); struct reorder_buffer_entry *tmp_rob_ptr = - (struct reorder_buffer_entry *)rob_ptr; - tmp_rob_ptr->ready = eop * needs_reorder; + (struct reorder_buffer_entry *)tmp; + tmp_rob_ptr->ready = eop * valid; } port->inflights -= eop; @@ -415,22 +428,18 @@ __pull_port_lb(struct sw_evdev *sw, uint32_t port_id, int allow_reorder) if (flags & QE_FLAG_VALID) { port->stats.rx_pkts++; - if (allow_reorder && needs_reorder) { - struct reorder_buffer_entry *rob_entry = - hist_entry->rob_entry; - - hist_entry->rob_entry = NULL; + if (allow_reorder && rob_ptr) { /* Although fragmentation not currently * supported by eventdev API, we support it * here. Open: How do we alert the user that * they've exceeded max frags? */ - int num_frag = rob_entry->num_fragments; + int num_frag = rob_ptr->num_fragments; if (num_frag == SW_FRAGMENTS_MAX) sw->stats.rx_dropped++; else { - int idx = rob_entry->num_fragments++; - rob_entry->fragments[idx] = *qe; + int idx = rob_ptr->num_fragments++; + rob_ptr->fragments[idx] = *qe; } goto end_qe; } -- 2.34.1