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.
> 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 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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
