{"id":2223127,"url":"http://patchwork.ozlabs.org/api/patches/2223127/?format=json","web_url":"http://patchwork.ozlabs.org/project/ovn/patch/20260414135102.333175-1-jtanenba@redhat.com/","project":{"id":68,"url":"http://patchwork.ozlabs.org/api/projects/68/?format=json","name":"Open Virtual Network development","link_name":"ovn","list_id":"ovs-dev.openvswitch.org","list_email":"ovs-dev@openvswitch.org","web_url":"http://openvswitch.org/","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20260414135102.333175-1-jtanenba@redhat.com>","list_archive_url":null,"date":"2026-04-14T13:51:02","name":"[ovs-dev,v6] ovn-controller: Port up/ovn-installed reported too early.","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"e3755922c6d4441ff7c9cfa06fc7476e4538ded0","submitter":{"id":88238,"url":"http://patchwork.ozlabs.org/api/people/88238/?format=json","name":"Jacob Tanenbaum","email":"jtanenba@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/ovn/patch/20260414135102.333175-1-jtanenba@redhat.com/mbox/","series":[{"id":499850,"url":"http://patchwork.ozlabs.org/api/series/499850/?format=json","web_url":"http://patchwork.ozlabs.org/project/ovn/list/?series=499850","date":"2026-04-14T13:51:02","name":"[ovs-dev,v6] ovn-controller: Port up/ovn-installed reported too early.","version":6,"mbox":"http://patchwork.ozlabs.org/series/499850/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/2223127/comments/","check":"success","checks":"http://patchwork.ozlabs.org/api/patches/2223127/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","ovs-dev@lists.linuxfoundation.org"],"Authentication-Results":["legolas.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=DPAz1afU;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=140.211.166.138; helo=smtp1.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org)","smtp1.osuosl.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key)\n header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=DPAz1afU","smtp3.osuosl.org; dmarc=pass (p=quarantine dis=none)\n header.from=redhat.com","smtp3.osuosl.org; dkim=pass (1024-bit key,\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=DPAz1afU"],"Received":["from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fw5KJ1LgRz1y2d\n\tfor ; Tue, 14 Apr 2026 23:51:16 +1000 (AEST)","from localhost (localhost [127.0.0.1])\n\tby smtp1.osuosl.org (Postfix) with ESMTP id D5E6D84BEE;\n\tTue, 14 Apr 2026 13:51:13 +0000 (UTC)","from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id zdybnizGZFbI; Tue, 14 Apr 2026 13:51:12 +0000 (UTC)","from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])\n\tby smtp1.osuosl.org (Postfix) with ESMTPS id B629084BE2;\n\tTue, 14 Apr 2026 13:51:12 +0000 (UTC)","from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id 90A59C054A;\n\tTue, 14 Apr 2026 13:51:12 +0000 (UTC)","from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 639F7C0549\n for ; Tue, 14 Apr 2026 13:51:11 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp3.osuosl.org (Postfix) with ESMTP id 556A261C96\n for ; Tue, 14 Apr 2026 13:51:11 +0000 (UTC)","from smtp3.osuosl.org ([127.0.0.1])\n by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id xzGOv24VudaH for ;\n Tue, 14 Apr 2026 13:51:10 +0000 (UTC)","from us-smtp-delivery-124.mimecast.com\n (us-smtp-delivery-124.mimecast.com [170.10.133.124])\n by smtp3.osuosl.org (Postfix) with ESMTPS id 14A3961C95\n for ; Tue, 14 Apr 2026 13:51:09 +0000 (UTC)","from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com\n (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by\n relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n cipher=TLS_AES_256_GCM_SHA384) id us-mta-114-eaiPq0bpOe2-dKEDE8FWOA-1; Tue,\n 14 Apr 2026 09:51:06 -0400","from mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com\n (mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.111])\n (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest\n SHA256)\n (No client certificate requested)\n by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS\n id C710A1800454\n for ; Tue, 14 Apr 2026 13:51:05 +0000 (UTC)","from jtanenba-thinkpadp16vgen1.boston.csb (unknown [10.22.81.40])\n by mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP\n id 8F041180049F; Tue, 14 Apr 2026 13:51:04 +0000 (UTC)"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections - client-ip=140.211.9.56;\n helo=lists.linuxfoundation.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver= ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp1.osuosl.org B629084BE2","OpenDKIM Filter v2.11.0 smtp3.osuosl.org 14A3961C95"],"Received-SPF":"Pass (mailfrom) identity=mailfrom; client-ip=170.10.133.124;\n helo=us-smtp-delivery-124.mimecast.com; envelope-from=jtanenba@redhat.com;\n receiver=","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp3.osuosl.org 14A3961C95","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1776174668;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n content-transfer-encoding:content-transfer-encoding;\n bh=RenZSPniM1CC8SHVhHU9Qgj+jHoPjgKSJflXldTXbQo=;\n b=DPAz1afU6GJcxBI0oAcE/gNvQqOepCzPsr2RnNpL2zrZK5WtZyyKuddrsVO+6iQhIMMksG\n ZIrEWButY3blYcKJSgeqFEIelF5vIlq1QxE9Y3I/N+xxkcmpY0hBt4LSy/8tkLTwOfM6HX\n rCXwy6TFJbR0vG0oomVYbL9MoLDEJ4o=","X-MC-Unique":"eaiPq0bpOe2-dKEDE8FWOA-1","X-Mimecast-MFC-AGG-ID":"eaiPq0bpOe2-dKEDE8FWOA_1776174665","To":"dev@openvswitch.org","Date":"Tue, 14 Apr 2026 09:51:02 -0400","Message-ID":"<20260414135102.333175-1-jtanenba@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.4.1 on 10.30.177.111","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"s5Vt7pYP08AwRtOYJCGqr9h08T2U2HUhWvECA7pA7cg_1776174665","X-Mimecast-Originator":"redhat.com","Subject":"[ovs-dev] [Patch ovn v6] ovn-controller: Port up/ovn-installed\n reported too early.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","From":"Jacob Tanenbaum via dev ","Reply-To":"Jacob Tanenbaum ","Cc":"dceara@redhat.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"ovs-dev-bounces@openvswitch.org","Sender":"\"dev\" "},"content":"When ovn-monitor-all is set to false ovn-controller sets ovn-installed\non OVS interfaces too early. ovn-controller needs to wait for the\nresponse from the southbound database with the updates to the newly\nmonitored fields only then can it install flows and label the OVS\ninterface as installed.\n\nReported-at: https://redhat.atlassian.net/browse/FDP-2887\nSigned-off-by: Jacob Tanenbaum \n\n---\nv5->v6\n* simplified the logic as requested, Ales saw that we did not need to\n save the seqno if we checked before the engine run.\n* removing the extra state from the state machine\n* removing some leftover logic that was seen.\n\nv4->v5\n* corrected a sanitizer error: used bool update_seqno without\n initializing\n\nv3->v4\n* Added state OIF_WAITING_SB_COND to the state machine that manages\n the adding of interfaces. This state waits until the Southbound has\n updated the ovn-controller of relevent information about ports related\n to it\n* Addressed several nits\n\nv2->v3\n* adding the ld->monitor_updated required the additiona of checking for\n monitor_all in update_sb_monitors. I didn't account for being able to\n toggle on monitor_all\n\nv1->v2\n* if_status_mgr_run() will run everytime the conditional seqno is\n changed so it should be safe to only skip when the expected_seqno and\n seqno returned from ovn are strictly not equal, that way we do not\n have to deal with overflow in the seqno. Additionally add a boolean to\n the local_datapath in the event that the seqno wraps around at the\n same time the datapath would go back into the state OIF_INSTALL_FLOWS.\n* remove setting the state to itself for OIF_INSTALL_FLOWS in\n if_status_mgr_update()\n* added assert(pb) in if_status_mgr_run()\n* removed a manual loop looking for the local_datapath and replaced with\n get_local_datapath() in if_status_mgr_run\n* remove a few nit spelling errors in the test case","diff":"diff --git a/controller/if-status.c b/controller/if-status.c\nindex ee9337e63..df5580422 100644\n--- a/controller/if-status.c\n+++ b/controller/if-status.c\n@@ -18,6 +18,7 @@\n #include \"binding.h\"\n #include \"if-status.h\"\n #include \"lib/ofctrl-seqno.h\"\n+#include \"local_data.h\"\n #include \"ovsport.h\"\n #include \"simap.h\"\n \n@@ -115,6 +116,7 @@ static const char *if_state_names[] = {\n * | | | | | |\n * | | | mgr_update(when sb is rw i.e. pb->chassis) | | |\n * | | | has been updated | | |\n+ * | | | and the sb has the required info | | |\n * | | release_iface | - request seqno | | |\n * | | | | | |\n * | | V | | |\n@@ -500,6 +502,7 @@ if_status_mgr_update(struct if_status_mgr *mgr,\n const struct sbrec_chassis *chassis_rec,\n const struct ovsrec_interface_table *iface_table,\n const struct sbrec_port_binding_table *pb_table,\n+ const struct hmap *local_datapaths,\n bool ovs_readonly,\n bool sb_readonly)\n {\n@@ -622,9 +625,23 @@ if_status_mgr_update(struct if_status_mgr *mgr,\n * in if_status_handle_claims or if_status_mgr_claim_iface\n */\n if (iface->is_vif) {\n- ovs_iface_set_state(mgr, iface, OIF_INSTALL_FLOWS);\n- iface->install_seqno = mgr->iface_seqno + 1;\n- new_ifaces = true;\n+ if (local_datapaths) {\n+ const struct sbrec_port_binding *pb =\n+ sbrec_port_binding_table_get_for_uuid(pb_table,\n+ &iface->pb_uuid);\n+ ovs_assert(pb);\n+ struct local_datapath *ld =\n+ get_local_datapath(local_datapaths,\n+ pb->datapath->tunnel_key);\n+ if (!ld) {\n+ continue;\n+ }\n+ if (ld->is_sb_updated) {\n+ ovs_iface_set_state(mgr, iface, OIF_INSTALL_FLOWS);\n+ iface->install_seqno = mgr->iface_seqno + 1;\n+ new_ifaces = true;\n+ }\n+}\n } else {\n ovs_iface_set_state(mgr, iface, OIF_MARK_UP);\n }\ndiff --git a/controller/if-status.h b/controller/if-status.h\nindex d15ca3008..b8f1d6a2e 100644\n--- a/controller/if-status.h\n+++ b/controller/if-status.h\n@@ -43,6 +43,7 @@ void if_status_mgr_update(struct if_status_mgr *, struct local_binding_data *,\n const struct sbrec_chassis *chassis,\n const struct ovsrec_interface_table *iface_table,\n const struct sbrec_port_binding_table *pb_table,\n+ const struct hmap *local_datapaths,\n bool ovs_readonly,\n bool sb_readonly);\n void if_status_mgr_run(struct if_status_mgr *mgr, struct local_binding_data *,\ndiff --git a/controller/local_data.h b/controller/local_data.h\nindex 948c1a935..2ca00c1fb 100644\n--- a/controller/local_data.h\n+++ b/controller/local_data.h\n@@ -53,6 +53,7 @@ struct local_datapath {\n const struct sbrec_datapath_binding *datapath;\n bool is_switch;\n bool is_transit_switch;\n+ bool is_sb_updated;\n \n /* The localnet port in this datapath, if any (at most one is allowed). */\n const struct sbrec_port_binding *localnet_port;\ndiff --git a/controller/ovn-controller.c b/controller/ovn-controller.c\nindex da43051ed..2964c54d2 100644\n--- a/controller/ovn-controller.c\n+++ b/controller/ovn-controller.c\n@@ -7842,6 +7842,21 @@ main(int argc, char *argv[])\n \n bool recompute_allowed = (ovnsb_idl_txn &&\n !ofctrl_has_backlog());\n+\n+ if (runtime_data &&\n+ ((ovnsb_cond_seqno == ovnsb_expected_cond_seqno &&\n+ ovnsb_expected_cond_seqno != UINT_MAX) ||\n+ sb_monitor_all)) {\n+\n+ struct local_datapath *ld;\n+ HMAP_FOR_EACH_SAFE (ld,\n+ hmap_node,\n+ &runtime_data->local_datapaths) {\n+\n+ ld->is_sb_updated = true;\n+ }\n+ }\n+\n engine_run(recompute_allowed);\n tracked_acl_ids = engine_get_data(&en_acl_id);\n \n@@ -7973,6 +7988,21 @@ main(int argc, char *argv[])\n * a continuous reason for monitor updates. */\n daemon_started_recently_countdown();\n }\n+\n+ /* If sb_monitor_all is true we are monitoring all\n+ * the southbound database tables already and the\n+ * needed data will be avalible already */\n+ if (sb_monitor_all) {\n+ struct local_datapath *ld;\n+ struct hmap *local_datapaths =\n+ &runtime_data->local_datapaths;\n+\n+ HMAP_FOR_EACH (ld,\n+ hmap_node,\n+ local_datapaths) {\n+ ld->is_sb_updated = true;\n+ }\n+ }\n }\n /* If there is no new expected seqno we have finished\n * loading all needed data from southbound. We then\n@@ -8017,6 +8047,9 @@ main(int argc, char *argv[])\n ovs_idl_loop.idl),\n sbrec_port_binding_table_get(\n ovnsb_idl_loop.idl),\n+ runtime_data ?\n+ &runtime_data->local_datapaths\n+ : NULL,\n !ovs_idl_txn,\n !ovnsb_idl_txn);\n stopwatch_stop(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME,\ndiff --git a/tests/ovn-controller.at b/tests/ovn-controller.at\nindex c98de9bc4..9dc7555ba 100644\n--- a/tests/ovn-controller.at\n+++ b/tests/ovn-controller.at\n@@ -3944,3 +3944,69 @@ OVN_CLEANUP([hv1], [hv2\n /already has encap ip.*cannot duplicate on/d])\n AT_CLEANUP\n ])\n+\n+OVN_FOR_EACH_NORTHD([\n+AT_SETUP([ovn-installed])\n+ovn_start\n+\n+net_add n1\n+sim_add hv1\n+\n+as hv1\n+ovs-vsctl add-br br-phys\n+ovn_attach n1 br-phys 192.168.0.1\n+ovn-appctl vlog/set dbg\n+ovs-vsctl add-port br-int vif1 -- \\\n+ set Interface vif1 external-ids:iface-id=lsp1\n+\n+check ovn-nbctl ls-add ls1\n+sleep_controller hv1\n+check ovn-nbctl --wait=sb lsp-add ls1 lsp1 -- \\\n+ lsp-set-addresses lsp1 \"f0:00:00:00:00:01 10.0.0.1\"\n+\n+sleep_sb\n+wake_up_controller hv1\n+\n+# Wait for pflow for lsp1\n+OVS_WAIT_UNTIL([\n+ ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=vif1)\n+ echo \"vif1 port=$ofport\"\n+ test -n \"$ofport\" && test 1 -le $(as hv1 ovs-ofctl dump-flows br-int | grep -c in_port=$ofport)\n+])\n+\n+# If ovn-installed in ovs, all flows should be installed.\n+# In that case, there should be at least one flow with lsp1 address.\n+OVS_WAIT_UNTIL([\n+ ovn_installed=$(as hv1 ovs-vsctl get Interface vif1 external_ids:ovn-installed)\n+ echo $ovn_installed\n+ flow_count=$(as hv1 ovs-ofctl dump-flows br-int | grep -Fc \"10.0.0.1\")\n+ # for the monitor-all=true case the flow gets installed because ovn-controller is monitoring all\n+ # tables in OVN_SOUTHBOUND.\n+ if test -n \"$ovn_installed\"; then\n+ as hv1 ovs-ofctl dump-flows br-int > output\n+ test $flow_count -ge 1\n+ else\n+ true\n+ fi\n+])\n+\n+wake_up_sb\n+# After the southbound db has woken up and can send the update to the\n+# ovn-controller not monitoring all tables in the southbound db it\n+# should be able to install the interface.\n+OVS_WAIT_UNTIL([\n+ ovn_installed=$(as hv1 ovs-vsctl get Interface vif1 external_ids:ovn-installed)\n+ flow_count=$(as hv1 ovs-ofctl dump-flows br-int | grep -Fc \"10.0.0.1\")\n+ echo \"installed=$ovn_installed, count=$flow_count\"\n+ if test -n \"$ovn_installed\"; then\n+ as hv1 ovs-ofctl dump-flows br-int > output\n+ test $flow_count -ge 1\n+ else\n+ false\n+ fi\n+])\n+wait_for_ports_up\n+\n+OVN_CLEANUP([hv1])\n+AT_CLEANUP\n+])\n","prefixes":["ovs-dev","v6"]}