There is a race condition between the revalidator threads and
the handler/pmd threads.

revalidator                          PMD threads
push_dp_ops deletes a key and tries
to del the dp magaflow.
                                     does the upcall, generates a new ukey,
                                     and replaces the old ukey, now the old
                                     ukey state is UKEY_DELETED

dp_ops succeeds, tries to change
the old ukey's state into
UKEY_EVICTED, however, the old
ukey's state is already UKEY_DELETED,
so OVS aborts.

I did not observe this in the real environment, as it takes time for
PMDs to finish the upcall and replace the old ukeys. Normally, the
revalidator will change ukey state into UKEY_EVICTED first.
But it's better to cover this case.

Signed-off-by: Peng He <[email protected]>
---
 ofproto/ofproto-dpif-upcall.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 4f68f14f2..9c55e43c1 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2411,7 +2411,9 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, 
size_t n_ops)
         if (op->dop.error) {
             if (op->ukey) {
                 ovs_mutex_lock(&op->ukey->mutex);
-                transition_ukey(op->ukey, UKEY_EVICTED);
+                if (op->ukey->state < UKEY_EVICTED) {
+                    transition_ukey(op->ukey, UKEY_EVICTED);
+                }
                 ovs_mutex_unlock(&op->ukey->mutex);
             }
             /* if it's a flow_del error, 'stats' is unusable, it's ok
@@ -2432,7 +2434,9 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, 
size_t n_ops)
 
         if (op->ukey) {
             ovs_mutex_lock(&op->ukey->mutex);
-            transition_ukey(op->ukey, UKEY_EVICTED);
+            if (op->ukey->state < UKEY_EVICTED) {
+                transition_ukey(op->ukey, UKEY_EVICTED);
+            }
             push->used = MAX(stats->used, op->ukey->stats.used);
             push->tcp_flags = stats->tcp_flags | op->ukey->stats.tcp_flags;
             push->n_packets = stats->n_packets - op->ukey->stats.n_packets;
-- 
2.25.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to