During code-review, it seems like it could be possible to publish a
flow into the cmap, and the hw offload thread assistance could miss
the appropriate flow. This situation probably didn't occur much in
practice, and the result might be just a dropped packet (although, I'm
not sure that it couldn't also result in a weird duplicate flow
getting installed due to the check in mark_to_flow_find that gets
called during dfc_processing).
This change reorders the assignment to before the cmap_insert. The
cmap_insert should act as a barrier in this case to ensure that the
write to flow->mark is ready before the read (after the
CMAP_FOR_EACH_WITH_HASH).
Fixes: 241bad15d99a ("dpif-netdev: associate flow with a mark id")
Signed-off-by: Aaron Conole <[email protected]>
---
lib/dpif-netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 224ce7086f..31b95f153c 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2600,10 +2600,10 @@ mark_to_flow_associate(const uint32_t mark, struct
dp_netdev_flow *flow)
unsigned int tid = netdev_offload_thread_id();
dp_netdev_flow_ref(flow);
+ flow->mark = mark;
cmap_insert(&dp_offload_threads[tid].mark_to_flow,
CONST_CAST(struct cmap_node *, &flow->mark_node),
hash_int(mark, 0));
- flow->mark = mark;
VLOG_DBG("Associated dp_netdev flow %p with mark %u mega_ufid "UUID_FMT,
flow, mark, UUID_ARGS((struct uuid *) &flow->mega_ufid));
--
2.51.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
