Since [0], en_route_exchange has en_sb_cond_seqno as an input without change handler. en_sb_cond_seqno changes are common with conditional monitoring (e.g., a port being claimed), and with no handler such change forces a node recompute. en_route_exchange is SB_WRITE, so if this recompute occurs while OpenFlow has backlog, it gets canceled, which forces a full recompute later, including a lflow_run recompute. This was caught by sporadic failures of test [1], which was validating that no lflow_run occurs when claiming a port.
Replace route_exchange_sb_ro_handler with route_exchange_sb_handler, shared by both the 'sb_ro' and 'sb_cond_seqno' inputs. It performs the sync when possible, returning EN_HANDLED_UNCHANGED/EN_HANDLED_UPDATED instead of EN_UNHANDLED. It never forces a recompute: - when the gate did not move, there is nothing to do. - when it did move but SB is not writable, it is safe to wait quietly, since 'sb_ro' will fire again when SB becomes writable, and we could not have synced this iteration anyway. This is unrelated to oflow backlog, as it does not program openflows. [0] 'controller: Defer route updates until routes load.' [1] "ovn-controller incremental processing". Reported-at: https://redhat.atlassian.net/browse/FDP-4699 Assisted-by: Claude Sonnet 5, Claude Code Fixes: a3e697fb9798 ("controller: Defer route updates until routes load.") Signed-off-by: Xavier Simonart <[email protected]> --- controller/ovn-controller.c | 27 +++++++++++++++++++++------ tests/ovn-inc-proc-graph-dump.at | 4 ++-- tests/ovn-performance.at | 21 +++++++++++++++++++-- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 01b0119b7..c39d9b8d0 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -6168,15 +6168,29 @@ route_exchange_route_table_handler(struct engine_node *node, void *data) return EN_HANDLED_UPDATED; } +/* Change handler shared by the 'sb_ro' and 'sb_cond_seqno' inputs. */ static enum engine_input_handler_result -route_exchange_sb_ro_handler(struct engine_node *node OVS_UNUSED, void *data) +route_exchange_sb_handler(struct engine_node *node, void *data) { struct ed_type_route_exchange *re = data; - if (re->sb_changes_pending) { - return EN_UNHANDLED; + + if (!sbrec_server_has_learned_route_table(re->sb_idl)) { + return EN_HANDLED_UNCHANGED; } - return EN_HANDLED_UNCHANGED; + bool ready = route_exchange_ready(node, re); + if (re->initial_routes_loaded == ready && !re->sb_changes_pending) { + /* The readiness gate did not flip and there is no SB write left + * over from a previous attempt: this event is not relevant to us. */ + return EN_HANDLED_UNCHANGED; + } + + if (!engine_get_context()->ovnsb_idl_txn) { + return EN_HANDLED_UNCHANGED; + } + + return route_exchange_sync(node, re) ? EN_HANDLED_UPDATED + : EN_HANDLED_UNCHANGED; } @@ -7725,11 +7739,12 @@ inc_proc_ovn_controller_init( route_exchange_route_table_handler); engine_add_input(&en_route_exchange, &en_route_exchange_status, NULL); engine_add_input(&en_route_exchange, &en_sb_ro, - route_exchange_sb_ro_handler); + route_exchange_sb_handler); /* Re-evaluate the initial-dump readiness gate whenever the SB monitor * conditions change (including the final ack of the scoped * Advertised_Route dump). */ - engine_add_input(&en_route_exchange, &en_sb_cond_seqno, NULL); + engine_add_input(&en_route_exchange, &en_sb_cond_seqno, + route_exchange_sb_handler); engine_add_input(&en_addr_sets, &en_sb_address_set, addr_sets_sb_address_set_handler); diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index b3df37a01..7853e5001 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -480,8 +480,8 @@ digraph "Incremental-Processing-Engine" { SB_port_binding -> route_exchange [[label="engine_noop_handler"]]; route_table_notify -> route_exchange [[label="route_exchange_route_table_handler"]]; route_exchange_status -> route_exchange [[label=""]]; - sb_ro -> route_exchange [[label="route_exchange_sb_ro_handler"]]; - sb_cond_seqno -> route_exchange [[label=""]]; + sb_ro -> route_exchange [[label="route_exchange_sb_handler"]]; + sb_cond_seqno -> route_exchange [[label="route_exchange_sb_handler"]]; garp_rarp [[style=filled, shape=box, fillcolor=white, label="garp_rarp"]]; OVS_open_vswitch -> garp_rarp [[label=""]]; SB_chassis -> garp_rarp [[label=""]]; diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 114917832..de56ca5f8 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -359,11 +359,28 @@ for i in 1 2; do [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv acl-add $ls to-lport 1000 'outport == \"$lp\"' drop] ) - # Bind port $lp and wait for it to come up + # + # Create openflow backlog by pausing ovs/ovsdb. + # We can't simply pause ovs to delay flow handling, as this would also prevent ofport creation. + # So, follow this pattern: + # 1) Pause ovs to prevent oflow creation, but keep ovsdb running so we can add interface. + # 2) Create interface. + # 3) Pause ovs-db and restart ovs. ovs now creates ofport, but ovsdb is sleeping. + # 4) After one second (i.e. enough for ovs to have time to write ofport), pause ovs and restart ovsdb. + # 5) Now ovn-controller should get ofport, claim and program flows (while ovs is paused). + # Sleep 2 more seconds to let ovn-controller handle the diff, start programming openflows, hitting backlog and cancelling. + # If previous pauses are not long enough (which should be rare), we might miss some failures on buggy code, but this should not cause flaky tests. + # 6) Wake up ovs so flows get programmed. + # 7) Wait for port up + sync. + # 8) This used to cause recomputes. + sleep_ovs hv$i OVN_CONTROLLER_EXPECT_NO_HIT( [hv$i hv$j], [lflow_run], - [as hv$i ovs-vsctl add-port br-int $vif -- set Interface $vif external-ids:iface-id=$lp && + [as hv$i ovs-vsctl --no-wait add-port br-int $vif -- set Interface $vif external-ids:iface-id=$lp && + sleep_ovsdb hv$i && wake_up_ovs hv$i && + sleep 1 && sleep_ovs hv$i && wake_up_ovsdb hv$i && + sleep 2 && wake_up_ovs hv$i && check ovn-nbctl wait-until Logical_Switch_Port $lp 'up=true' && check ovn-nbctl --wait=hv sync] ) -- 2.47.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
