From: Harry van Haaren <harry.van.haaren@intel.com>
To: dev@dpdk.org
Cc: Harry van Haaren <harry.van.haaren@intel.com>
Subject: [PATCH 2/2] event/sw: fix ordering corruption with op release
Date: Thu, 31 Aug 2023 17:47:36 +0100 [thread overview]
Message-ID: <20230831164736.2472671-2-harry.van.haaren@intel.com> (raw)
In-Reply-To: <20230831164736.2472671-1-harry.van.haaren@intel.com>
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 <harry.van.haaren@intel.com>
---
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
next prev parent reply other threads:[~2023-08-31 16:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-31 16:47 [PATCH 1/2] event/sw: add selftest for ordered history list Harry van Haaren
2023-08-31 16:47 ` Harry van Haaren [this message]
2023-09-04 16:37 ` [PATCH 2/2] event/sw: fix ordering corruption with op release Bruce Richardson
2023-09-08 16:22 ` Van Haaren, Harry
2023-09-14 10:58 ` [PATCH v2 1/2] " Harry van Haaren
2023-09-14 10:58 ` [PATCH v2 2/2] event/sw: add selftest for ordered history list Harry van Haaren
2023-09-14 11:13 ` Bruce Richardson
2023-10-02 10:58 ` [PATCH v3 1/2] event/sw: fix ordering corruption with op release Harry van Haaren
2023-10-02 10:58 ` [PATCH v3 2/2] event/sw: add selftest for ordered history list Harry van Haaren
2023-10-03 6:38 ` [PATCH v3 1/2] event/sw: fix ordering corruption with op release Jerin Jacob
2023-09-14 11:12 ` [PATCH v2 " Bruce Richardson
2023-08-31 17:10 ` [PATCH 1/2] event/sw: add selftest for ordered history list Bruce Richardson
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=20230831164736.2472671-2-harry.van.haaren@intel.com \
--to=harry.van.haaren@intel.com \
--cc=dev@dpdk.org \
/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).