On Sat, Jun 4, 2022 at 11:19 AM Peng He <[email protected]> wrote: > > 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.
Hello Peng, I've run into a similar issue by running OVS under Massif, but with a different caller. Do you think it makes sense to move this change into transition_ukey() ? Cheers, M > > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
