From patchwork Tue Sep 24 13:33:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1988972 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=REKaeuFB; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XCgm96pHZz1xsN for ; Tue, 24 Sep 2024 23:33:13 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id EB5AC60AC4; Tue, 24 Sep 2024 13:33:11 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id tX_aWNYjpaB0; Tue, 24 Sep 2024 13:33:10 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.9.56; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 89A6E60AC5 Authentication-Results: smtp3.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=REKaeuFB Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 89A6E60AC5; Tue, 24 Sep 2024 13:33:10 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 43188C0013; Tue, 24 Sep 2024 13:33:10 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id AA0E8C0011 for ; Tue, 24 Sep 2024 13:33:08 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id A370B60AC5 for ; Tue, 24 Sep 2024 13:33:08 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 3M07UgVM680e for ; Tue, 24 Sep 2024 13:33:07 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=amusil@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp3.osuosl.org 0BBFB60AC3 Authentication-Results: smtp3.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 0BBFB60AC3 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 0BBFB60AC3 for ; Tue, 24 Sep 2024 13:33:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1727184785; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=GROblU61wdla+xpe6n0AxswajCGmUWZXzxsAtJVCQ2g=; b=REKaeuFBCt4E1SEXPFZ0dImUKgwoT8F6KL1cOKxL34XhtMdx88zfn9OWs+bLbwKyNsb4oV g0LA3SBtWglYkUv59K5/8Q8gZXBONHv1HCrUjK4Tt2+xjjCZkPgBXx31XtkIN3heXB5Gkj byHkO6FzA7IXSHehVDXdwIKNo2MkE6M= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-7-4y9SeNp9Oc-w761c6R8Rdg-1; Tue, 24 Sep 2024 09:33:04 -0400 X-MC-Unique: 4y9SeNp9Oc-w761c6R8Rdg-1 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (unknown [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id B087518B63DB for ; Tue, 24 Sep 2024 13:33:03 +0000 (UTC) Received: from amusil.redhat.com (unknown [10.45.225.88]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 54864195608A; Tue, 24 Sep 2024 13:33:02 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Tue, 24 Sep 2024 15:33:00 +0200 Message-ID: <20240924133300.329879-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2] inc-engine: Adjust the force recompute API. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dceara@redhat.com Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" There are cases when we need to wake the thread immediately after setting force recompute. This is mainly in error handling that happens after engine_run() call. In order to achieve that make the API more ergonomic with parameters to force the wake if needed. Reported-at: https://issues.redhat.com/browse/FDP-753 Signed-off-by: Ales Musil --- controller/ovn-controller.c | 17 +++++++---------- lib/inc-proc-eng.c | 20 +++++++++++++++++--- lib/inc-proc-eng.h | 15 ++++++++++++--- northd/inc-proc-northd.c | 14 ++------------ northd/inc-proc-northd.h | 13 ++++++++++++- northd/ovn-northd.c | 15 +++++++-------- 6 files changed, 57 insertions(+), 37 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 168167b1a..f21d46fc3 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -641,7 +641,7 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, } if (reset_ovnsb_idl_min_index && *reset_ovnsb_idl_min_index) { VLOG_INFO("Resetting southbound database cluster state"); - engine_set_force_recompute(true); + engine_set_force_recompute(false); ovsdb_idl_reset_min_index(ovnsb_idl); *reset_ovnsb_idl_min_index = false; } @@ -4768,7 +4768,7 @@ check_northd_version(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, * full recompute. */ if (version_mismatch) { - engine_set_force_recompute(true); + engine_set_force_recompute(false); } version_mismatch = false; return true; @@ -5382,7 +5382,7 @@ main(int argc, char *argv[]) if (new_ovs_cond_seqno != ovs_cond_seqno) { if (!new_ovs_cond_seqno) { VLOG_INFO("OVS IDL reconnected, force recompute."); - engine_set_force_recompute(true); + engine_set_force_recompute(false); } ovs_cond_seqno = new_ovs_cond_seqno; } @@ -5399,7 +5399,7 @@ main(int argc, char *argv[]) if (new_ovnsb_cond_seqno != ovnsb_cond_seqno) { if (!new_ovnsb_cond_seqno) { VLOG_INFO("OVNSB IDL reconnected, force recompute."); - engine_set_force_recompute(true); + engine_set_force_recompute(false); } ovnsb_cond_seqno = new_ovnsb_cond_seqno; } @@ -5488,7 +5488,7 @@ main(int argc, char *argv[]) br_int_remote.target, br_int_remote.probe_interval)) { VLOG_INFO("OVS feature set changed, force recompute."); - engine_set_force_recompute(true); + engine_set_force_recompute(false); struct ed_type_lflow_output *lflow_out_data = engine_get_internal_data(&en_lflow_output); @@ -5510,7 +5510,7 @@ main(int argc, char *argv[]) VLOG_INFO_RL(&rl, "OVS OpenFlow connection reconnected," "force recompute."); - engine_set_force_recompute(true); + engine_set_force_recompute(false); } if (chassis && ovs_feature_set_discovered()) { @@ -5738,7 +5738,6 @@ main(int argc, char *argv[]) VLOG_DBG("engine did not run, force recompute next time: " "br_int %p, chassis %p", br_int, chassis); engine_set_force_recompute(true); - poll_immediate_wake(); } else { VLOG_DBG("engine did not run, and it was not needed" " either: br_int %p, chassis %p", @@ -5748,9 +5747,8 @@ main(int argc, char *argv[]) VLOG_DBG("engine was canceled, force recompute next time: " "br_int %p, chassis %p", br_int, chassis); engine_set_force_recompute(true); - poll_immediate_wake(); } else { - engine_set_force_recompute(false); + engine_clear_force_recompute(); } store_nb_cfg(ovnsb_idl_txn, ovs_idl_txn, chassis_private, @@ -6138,7 +6136,6 @@ lflow_cache_flush_cmd(struct unixctl_conn *conn OVS_UNUSED, struct lflow_output_persistent_data *fo_pd = arg_; lflow_cache_flush(fo_pd->lflow_cache); engine_set_force_recompute(true); - poll_immediate_wake(); unixctl_command_reply(conn, NULL); } diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c index a01440bb4..5748727b5 100644 --- a/lib/inc-proc-eng.c +++ b/lib/inc-proc-eng.c @@ -54,9 +54,24 @@ engine_recompute(struct engine_node *node, bool allowed, const char *reason_fmt, ...) OVS_PRINTF_FORMAT(3, 4); void -engine_set_force_recompute(bool val) +engine_set_force_recompute(bool immediate) { - engine_force_recompute = val; + engine_force_recompute = true; + if (immediate) { + poll_immediate_wake(); + } +} + +void +engine_clear_force_recompute(void) +{ + engine_force_recompute = false; +} + +bool +engine_get_force_recompute(void) +{ + return engine_force_recompute; } const struct engine_context * @@ -556,5 +571,4 @@ engine_trigger_recompute(void) { VLOG_INFO("User triggered force recompute."); engine_set_force_recompute(true); - poll_immediate_wake(); } diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h index 5bb3b8f3e..db80ae975 100644 --- a/lib/inc-proc-eng.h +++ b/lib/inc-proc-eng.h @@ -297,11 +297,20 @@ void *engine_get_input_data(const char *input_name, struct engine_node *); void engine_add_input(struct engine_node *node, struct engine_node *input, bool (*change_handler)(struct engine_node *, void *)); -/* Force the engine to recompute everything if set to true. It is used +/* Force the engine to recompute everything. It is used * in circumstances when we are not sure there is change or not, or * when there is change but the engine couldn't be executed in that - * iteration, and the change can't be tracked across iterations */ -void engine_set_force_recompute(bool val); + * iteration, and the change can't be tracked across iterations. + * The immediate makes sure that the loop is woken up immediately + * so the next engine run is not delayed. */ +void engine_set_force_recompute(bool immediate); + +/* Clear the force flag for the next run so the engine does the + * usual processing without forced full recompute. */ +void engine_clear_force_recompute(void); + +/* Returns whether next engine_run() is forced to rempute. */ +bool engine_get_force_recompute(void); /* Return the current engine_context. The values in the context can be NULL * if the engine is run with allow_recompute == false in the current diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 1f79916a5..667a13422 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -428,14 +428,6 @@ bool inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, int64_t start = time_msec(); engine_init_run(); - /* Force a full recompute if instructed to, for example, after a NB/SB - * reconnect event. However, make sure we don't overwrite an existing - * force-recompute request if 'recompute' is false. - */ - if (ctx->recompute) { - engine_set_force_recompute(ctx->recompute); - } - struct engine_context eng_ctx = { .ovnnb_idl_txn = ovnnb_txn, .ovnsb_idl_txn = ovnsb_txn, @@ -448,16 +440,14 @@ bool inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, if (engine_need_run()) { VLOG_DBG("engine did not run, force recompute next time."); engine_set_force_recompute(true); - poll_immediate_wake(); } else { VLOG_DBG("engine did not run, and it was not needed"); } } else if (engine_canceled()) { VLOG_DBG("engine was canceled, force recompute next time."); engine_set_force_recompute(true); - poll_immediate_wake(); } else { - engine_set_force_recompute(false); + engine_clear_force_recompute(); } int64_t now = time_msec(); @@ -477,7 +467,7 @@ void inc_proc_northd_cleanup(void) bool inc_proc_northd_can_run(struct northd_engine_context *ctx) { - if (ctx->recompute || time_msec() >= ctx->next_run_ms || + if (engine_get_force_recompute() || time_msec() >= ctx->next_run_ms || ctx->nb_idl_duration_ms >= IDL_LOOP_MAX_DURATION_MS || ctx->sb_idl_duration_ms >= IDL_LOOP_MAX_DURATION_MS) { return true; diff --git a/northd/inc-proc-northd.h b/northd/inc-proc-northd.h index a2b9b7fdb..7c2cb2e7a 100644 --- a/northd/inc-proc-northd.h +++ b/northd/inc-proc-northd.h @@ -13,7 +13,6 @@ struct northd_engine_context { uint64_t nb_idl_duration_ms; uint64_t sb_idl_duration_ms; uint32_t backoff_ms; - bool recompute; }; void inc_proc_northd_init(struct ovsdb_idl_loop *nb, @@ -24,4 +23,16 @@ bool inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, void inc_proc_northd_cleanup(void); bool inc_proc_northd_can_run(struct northd_engine_context *ctx); +static inline void +inc_proc_northd_force_recompute(bool immediate) +{ + engine_set_force_recompute(immediate); +} + +static inline bool +inc_proc_northd_get_force_recompute(void) +{ + return engine_get_force_recompute(); +} + #endif /* INC_PROC_NORTHD */ diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index d71114f35..ba3e1ad9d 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -932,7 +932,7 @@ main(int argc, char *argv[]) if (new_ovnnb_cond_seqno != ovnnb_cond_seqno) { if (!new_ovnnb_cond_seqno) { VLOG_INFO("OVN NB IDL reconnected, force recompute."); - eng_ctx.recompute = true; + inc_proc_northd_force_recompute(false); } ovnnb_cond_seqno = new_ovnnb_cond_seqno; } @@ -945,7 +945,7 @@ main(int argc, char *argv[]) if (new_ovnsb_cond_seqno != ovnsb_cond_seqno) { if (!new_ovnsb_cond_seqno) { VLOG_INFO("OVN SB IDL reconnected, force recompute."); - eng_ctx.recompute = true; + inc_proc_northd_force_recompute(false); } ovnsb_cond_seqno = new_ovnsb_cond_seqno; } @@ -969,7 +969,6 @@ main(int argc, char *argv[]) int64_t loop_start_time = time_wall_msec(); activity = inc_proc_northd_run(ovnnb_txn, ovnsb_txn, &eng_ctx); - eng_ctx.recompute = false; check_and_add_supported_dhcp_opts_to_sb_db( ovnsb_txn, ovnsb_idl_loop.idl); check_and_add_supported_dhcpv6_opts_to_sb_db( @@ -982,7 +981,7 @@ main(int argc, char *argv[]) ovnsb_idl_loop.idl, ovnnb_txn, ovnsb_txn, &ovnsb_idl_loop); - } else if (!eng_ctx.recompute) { + } else if (!inc_proc_northd_get_force_recompute()) { clear_idl_track = false; } @@ -991,13 +990,13 @@ main(int argc, char *argv[]) if (!ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop)) { VLOG_INFO("OVNNB commit failed, " "force recompute next time."); - eng_ctx.recompute = true; + inc_proc_northd_force_recompute(true); } if (!ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop)) { VLOG_INFO("OVNSB commit failed, " "force recompute next time."); - eng_ctx.recompute = true; + inc_proc_northd_force_recompute(true); } run_memory_trimmer(ovnnb_idl_loop.idl, activity); } else { @@ -1006,7 +1005,7 @@ main(int argc, char *argv[]) ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); /* Force a full recompute next time we become active. */ - eng_ctx.recompute = true; + inc_proc_northd_force_recompute(false); } } else { /* ovn-northd is paused @@ -1030,7 +1029,7 @@ main(int argc, char *argv[]) ovsdb_idl_wait(ovnsb_idl_loop.idl); /* Force a full recompute next time we become active. */ - eng_ctx.recompute = true; + inc_proc_northd_force_recompute(true); } if (clear_idl_track) {