From patchwork Fri Aug 2 09:22:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1141048 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 460MCN3KnRz9sBF for ; Fri, 2 Aug 2019 19:23:10 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 4BA661017; Fri, 2 Aug 2019 09:23:07 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 7A6F31012 for ; Fri, 2 Aug 2019 09:23:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 211D0712 for ; Fri, 2 Aug 2019 09:23:06 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7731D5AFD9; Fri, 2 Aug 2019 09:23:05 +0000 (UTC) Received: from dceara.remote.csb (ovpn-117-201.ams2.redhat.com [10.36.117.201]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8E9A710002B3; Fri, 2 Aug 2019 09:23:04 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 2 Aug 2019 11:22:58 +0200 Message-Id: <1564737778-27292-1-git-send-email-dceara@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 02 Aug 2019 09:23:05 +0000 (UTC) X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH ovn] ovn-controller: Fix IP engine run with in-flight messages X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org When trying to incrementally process changes even if there are in-flight messages we were incorrectly setting the engine_aborted variable to the value returned by engine_run. However, engine_run returns true if the run was completed. This was causing discrepancies between logical flows and openflow flows due to the fact that the rerun wasn't happening after an aborted run. In order to avoid confusion the engine_aborted variable is now renamed to engine_run_done thus avoiding the negated logic. CC: Han Zhou Fixes: a6b7d9f4f047 ("ovn-controller: Fix flow installation latency caused by recompute.") Signed-off-by: Dumitru Ceara Tested-by: Numan Siddique Acked-by: Mark Michelson Acked-by: Han Zhou --- controller/ovn-controller.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index ad33d17..15b9f4e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1896,7 +1896,7 @@ main(int argc, char *argv[]) uint64_t engine_run_id = 0; uint64_t old_engine_run_id = 0; - bool engine_aborted = false; + bool engine_run_done = true; unsigned int ovs_cond_seqno = UINT_MAX; unsigned int ovnsb_cond_seqno = UINT_MAX; @@ -1997,14 +1997,14 @@ main(int argc, char *argv[]) * this round of engine_run and continue processing * acculated changes incrementally later when * ofctrl_can_put() returns true. */ - if (!engine_aborted) { + if (engine_run_done) { engine_set_abort_recompute(true); - engine_aborted = engine_run(&en_flow_output, - ++engine_run_id); + engine_run_done = engine_run(&en_flow_output, + ++engine_run_id); } } else { engine_set_abort_recompute(false); - engine_aborted = false; + engine_run_done = true; engine_run(&en_flow_output, ++engine_run_id); } } @@ -2048,8 +2048,8 @@ main(int argc, char *argv[]) } } - if (old_engine_run_id == engine_run_id || engine_aborted) { - if (engine_aborted || engine_need_run(&en_flow_output)) { + if (old_engine_run_id == engine_run_id || !engine_run_done) { + if (!engine_run_done || engine_need_run(&en_flow_output)) { VLOG_DBG("engine did not run, force recompute next time: " "br_int %p, chassis %p", br_int, chassis); engine_set_force_recompute(true);