From patchwork Thu May 4 11:28:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peng He X-Patchwork-Id: 1776848 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=P8jE0PoP; dkim-atps=neutral Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QBs5z17pTz213v for ; Thu, 4 May 2023 21:29:11 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 0AF1F41EAB; Thu, 4 May 2023 11:29:09 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 0AF1F41EAB Authentication-Results: smtp4.osuosl.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=P8jE0PoP X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id kxICf02BNPRd; Thu, 4 May 2023 11:29:07 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id 98D8C41EA8; Thu, 4 May 2023 11:29:06 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 98D8C41EA8 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7ACEDC0037; Thu, 4 May 2023 11:29:06 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id BA87FC002A for ; Thu, 4 May 2023 11:29:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id A270C81A6A for ; Thu, 4 May 2023 11:29:04 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org A270C81A6A Authentication-Results: smtp1.osuosl.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=P8jE0PoP X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LJSDUzy6ynFK for ; Thu, 4 May 2023 11:29:04 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org E8ABF817EB Received: from mail-pg1-x535.google.com (mail-pg1-x535.google.com [IPv6:2607:f8b0:4864:20::535]) by smtp1.osuosl.org (Postfix) with ESMTPS id E8ABF817EB for ; Thu, 4 May 2023 11:29:03 +0000 (UTC) Received: by mail-pg1-x535.google.com with SMTP id 41be03b00d2f7-52c759b7d45so229574a12.3 for ; Thu, 04 May 2023 04:29:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683199743; x=1685791743; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=Z7tPX+52Ah4W4ibf/5baZJJJ0XQ15YLH5X3GKdIbcJc=; b=P8jE0PoPmYE6AzrwKPFOaUkwm4EIqhB6RYXiH5YFE5D4aHQVLulOiN5Z+w7thgEDcu GrBnrMggKU2gAuspk+pxzLdwilDmCUy/24b1KmZTWZ3t+ui2jkF586nrERLAW7MyyhIe T/yVg1EqhrS4pcAm+rPViUEXjgZX2c4xFXvUr0kbPx2WVM9op35iUutjyCu0kiqN7dDW 1hPykd3g4t70CAD/5zmD8Ww7Uh8YJYROpJVsojjAlKKD3nQQjCNKJUsnQn9boaXMDx9q YwVHmVDOgo9fSRpdyyouoQ+c4rNRb5Oo/23s86Z5tY3DB6WrWHs4uJaFvwF8F47Q1JOp VMlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683199743; x=1685791743; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Z7tPX+52Ah4W4ibf/5baZJJJ0XQ15YLH5X3GKdIbcJc=; b=M6J53Njk81pmeOAs8ikDsBHlWvf5dVqsxPmfbdmLQv6XoDjj0LVhCoa2E/ToGbCpg4 u7Kvrjhni3EhekX7nHZ2F5FLKAbLC4MmdOoINTBAyi2yo4lJrK6u8lCZ2VRB3scCd2AI CjrJzbxPoJQwyZ03nYahmFy9BKMtvrfRMaV3NC639rN7uYhjq4M0/aMFAKHE6I3h/MP0 TidBUbNlR6fjV8F1ZIyg+CYddxGX2n7nx/OekgO0waKEWqJuGZhxrDCog7hE5aUF9Da+ GuHGmyJp3kp8Uob3YCDkLBDyN17tNwcQ5renUm932yG1P1YFTEdQ7VkE538ymNzQ2THF 0sQQ== X-Gm-Message-State: AC+VfDxHx43bZizoiZrGM3ASn1feL6AOQ+P0rIroiyVMClOGQf88bl9y OstvQFBZgzx2k2zJjz0tf9eX6lOhq9m72yXz X-Google-Smtp-Source: ACHHUZ5qkTR6Cbt872fzPHbsgkTJtvk5SSenUjKV7gv8Ash1194uURALQh1yavMMXbEqF9k6M2qzNg== X-Received: by 2002:a17:902:e541:b0:1a6:6f3f:bc3 with SMTP id n1-20020a170902e54100b001a66f3f0bc3mr4015277plf.57.1683199742984; Thu, 04 May 2023 04:29:02 -0700 (PDT) Received: from localhost ([61.213.176.6]) by smtp.gmail.com with ESMTPSA id i9-20020a170902c94900b001ab23d663ccsm2523909pla.86.2023.05.04.04.29.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 May 2023 04:29:02 -0700 (PDT) From: Peng He X-Google-Original-From: Peng He To: ovs-dev@openvswitch.org Date: Thu, 4 May 2023 11:28:59 +0000 Message-Id: <20230504112900.638529-1-hepeng.0320@bytedance.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Subject: [ovs-dev] [ovs-dev v8 1/2] ofproto-dpif-upcall: fix push_dp_ops X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" push_dp_ops only handles delete ops errors but ignores the modify ops results. It's better to handle all the dp operation errors in a consistent way. We observe in the production environment that sometimes a megaflow with wrong actions keep staying in datapath. The coverage command shows revalidators have dumped several times, however the correct actions are not set. This implies that the ukey's action does not equal to the meagaflow's, i.e. revalidators think the underlying megaflow's actions are correct however they are not. We also check the megaflow using the ofproto/trace command, and the actions are not matched with the ones in the actual magaflow. By performing a revalidator/purge command, the right actions are set. This patch prevents the inconsistency by considering modify failure in revalidators. To note, we cannot perform two state transitions and change ukey_state into UKEY_EVICTED directly here, because, if we do so, the sweep will remove the ukey alone and leave dp flow alive. Later, the dump will retrieve the dp flow and might even recover it. This will contribute the stats of this dp flow twice. Signed-off-by: Peng He --- ofproto/ofproto-dpif-upcall.c | 45 +++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index cd57fdbd9..c5b01de18 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -258,6 +258,7 @@ enum ukey_state { UKEY_CREATED = 0, UKEY_VISIBLE, /* Ukey is in umap, datapath flow install is queued. */ UKEY_OPERATIONAL, /* Ukey is in umap, datapath flow is installed. */ + UKEY_INCONSISTENT, /* Ukey is in umap, datapath flow is inconsistent. */ UKEY_EVICTING, /* Ukey is in umap, datapath flow delete is queued. */ UKEY_EVICTED, /* Ukey is in umap, datapath flow is deleted. */ UKEY_DELETED, /* Ukey removed from umap, ukey free is deferred. */ @@ -1999,6 +2000,10 @@ transition_ukey_at(struct udpif_key *ukey, enum ukey_state dst, * UKEY_VISIBLE -> UKEY_EVICTED * A handler attempts to install the flow, but the datapath rejects it. * Consider that the datapath has already destroyed it. + * UKEY_OPERATIONAL -> UKEY_INCONSISTENT + * A revalidator modifies the flow with error returns. + * UKEY_INCONSISTENT -> UKEY_EVICTING + * A revalidator decides to evict the datapath flow. * UKEY_OPERATIONAL -> UKEY_EVICTING * A revalidator decides to evict the datapath flow. * UKEY_EVICTING -> UKEY_EVICTED @@ -2007,7 +2012,9 @@ transition_ukey_at(struct udpif_key *ukey, enum ukey_state dst, * A revalidator has removed the ukey from the umap and is deleting it. */ if (ukey->state == dst - 1 || (ukey->state == UKEY_VISIBLE && - dst < UKEY_DELETED)) { + dst < UKEY_DELETED) || + (ukey->state == UKEY_OPERATIONAL && + dst == UKEY_EVICTING)) { ukey->state = dst; } else { struct ds ds = DS_EMPTY_INITIALIZER; @@ -2472,26 +2479,31 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, size_t n_ops) for (i = 0; i < n_ops; i++) { struct ukey_op *op = &ops[i]; - struct dpif_flow_stats *push, *stats, push_buf; - - stats = op->dop.flow_del.stats; - push = &push_buf; - - if (op->dop.type != DPIF_OP_FLOW_DEL) { - /* Only deleted flows need their stats pushed. */ - continue; - } if (op->dop.error) { - /* flow_del error, 'stats' is unusable. */ if (op->ukey) { ovs_mutex_lock(&op->ukey->mutex); - transition_ukey(op->ukey, UKEY_EVICTED); + if (op->ukey->state == UKEY_EVICTING) { + transition_ukey(op->ukey, UKEY_EVICTED); + } else if (op->ukey->state == UKEY_OPERATIONAL) { + /* Modify failed, ukey's state was UKEY_OPERATIONAL */ + transition_ukey(op->ukey, UKEY_INCONSISTENT); + } ovs_mutex_unlock(&op->ukey->mutex); } continue; } + if (op->dop.type != DPIF_OP_FLOW_DEL) { + /* Only deleted flows need their stats pushed. */ + continue; + } + + struct dpif_flow_stats *push, *stats, push_buf; + + stats = op->dop.flow_del.stats; + push = &push_buf; + if (op->ukey) { ovs_mutex_lock(&op->ukey->mutex); transition_ukey(op->ukey, UKEY_EVICTED); @@ -2839,6 +2851,15 @@ revalidate(struct revalidator *revalidator) continue; } + if (ukey->state == UKEY_INCONSISTENT) { + ukey->dump_seq = dump_seq; + reval_op_init(&ops[n_ops++], UKEY_DELETE, udpif, ukey, + &recircs, &odp_actions); + ovs_mutex_unlock(&ukey->mutex); + continue; + } + + if (ukey->state <= UKEY_OPERATIONAL) { /* The flow is now confirmed to be in the datapath. */ transition_ukey(ukey, UKEY_OPERATIONAL);