Eelco Chaudron <echau...@redhat.com> 于2022年9月30日周五 22:37写道：
>
>
> On 23 Sep 2022, at 18:29, Peng He 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.
>
> Can you give a call trace example, as try_ukey_replace() should handle
> this?
>
> If it is a valid state change, it should be handled transition_ukey() not
> by bypassing the call to it.
>
It means if the ukey->state >= UKEY_EVICTED, it's not needed to do the
state transition.
It's not a state change. since we have actually two threads moving forward
the state machine.
>
> > 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 <hepeng.0...@bytedance.com>
> > ---
> > 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 7ea2a30f5..e8bbcfeaf 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -2420,7 +2420,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
> > @@ -2441,7 +2443,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
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
--
hepeng
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
