Hi, Mike by changing it into the function, you mean relax the checking if the state is either EVICTED or DELETED?
Mike Pattrick <[email protected]> 于2022年8月15日周一 20:16写道： > 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 > > > > -- hepeng _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
