On Mon, May 4, 2026 at 7:23 AM Dumitru Ceara <[email protected]> wrote:
> On 4/30/26 10:44 PM, Jacob Tanenbaum wrote: > > When ovn-monitor-all is set to false ovn-controller sets ovn-installed > > on OVS interfaces too early. ovn-controller needs to wait for the > > response from the southbound database with the updates to the newly > > monitored fields only then can it install flows and label the OVS > > interface as installed. > > > > Reported-at: https://redhat.atlassian.net/browse/FDP-2887 > > Signed-off-by: Jacob Tanenbaum <[email protected]> > > > > --- > > Hi Jacob, > > Thanks for this new version! Overall it looks ok to me, but I have some > comments about some things that I think should be addressed in v10 > before we merge this. > > > v8->v9 > > * added the functionality of waiting for sb to update to the incremental > > processor > > > > v7->v8 > > * removed printing in the system testcases that shouldn't be there and > > caused failure. > > > > v6->v7 > > * added an sset that holds the uuid's of datapaths that are waiting for > > sb updates > > * added back the the state OIF_WAITING_SB_COND to the if_mgr state > > machine. > > * no longer hold if the datapath is updated in the local_datapaths > > struct as that is generated each transaction and cannot be relied > > upon. > > * removed some leftover logic from previous patch versions > > > > v5->v6 > > * simplified the logic as requested, Ales saw that we did not need to > > save the seqno if we checked before the engine run. > > * removing the extra state from the state machine > > * removing some leftover logic that was seen. > > > > v4->v5 > > * corrected a sanitizer error: used bool update_seqno without > > initializing > > > > v3->v4 > > * Added state OIF_WAITING_SB_COND to the state machine that manages > > the adding of interfaces. This state waits until the Southbound has > > updated the ovn-controller of relevent information about ports related > > to it > > * Addressed several nits > > > > v2->v3 > > * adding the ld->monitor_updated required the additiona of checking for > > monitor_all in update_sb_monitors. I didn't account for being able to > > toggle on monitor_all > > > > v1->v2 > > * if_status_mgr_run() will run everytime the conditional seqno is > > changed so it should be safe to only skip when the expected_seqno and > > seqno returned from ovn are strictly not equal, that way we do not > > have to deal with overflow in the seqno. Additionally add a boolean to > > the local_datapath in the event that the seqno wraps around at the > > same time the datapath would go back into the state OIF_INSTALL_FLOWS. > > * remove setting the state to itself for OIF_INSTALL_FLOWS in > > if_status_mgr_update() > > * added assert(pb) in if_status_mgr_run() > > * removed a manual loop looking for the local_datapath and replaced with > > get_local_datapath() in if_status_mgr_run > > * remove a few nit spelling errors in the test case > > > > diff --git a/controller/if-status.c b/controller/if-status.c > > index ee9337e63..aa789c63c 100644 > > --- a/controller/if-status.c > > +++ b/controller/if-status.c > > @@ -18,6 +18,7 @@ > > #include "binding.h" > > #include "if-status.h" > > #include "lib/ofctrl-seqno.h" > > +#include "local_data.h" > > #include "ovsport.h" > > #include "simap.h" > > > > @@ -58,6 +59,11 @@ VLOG_DEFINE_THIS_MODULE(if_status); > > enum if_state { > > OIF_CLAIMED, /* Newly claimed interface. pb->chassis > update not > > yet initiated. */ > > + OIF_WAITING_SB_COND, /* Waiting for the Southbound database to > update > > + * ovn-controller for a given datapath. We > should > > + * only be waiting in this state when > monitor_all > > + * is false AND it is the first time that we > see > > + * a specific datapath. */ > > OIF_INSTALL_FLOWS, /* Claimed interface with pb->chassis update > sent to > > * SB (but update notification not confirmed, > so the > > * update may be resent in any of the > following > > @@ -87,6 +93,7 @@ enum if_state { > > > > static const char *if_state_names[] = { > > [OIF_CLAIMED] = "CLAIMED", > > + [OIF_WAITING_SB_COND] = "WAITING_SB_COND", > > [OIF_INSTALL_FLOWS] = "INSTALL_FLOWS", > > [OIF_REM_OLD_OVN_INST] = "REM_OLD_OVN_INST", > > [OIF_MARK_UP] = "MARK_UP", > > @@ -114,7 +121,18 @@ static const char *if_state_names[] = { > > * | | | +--+ > | | | > > * | | | > | | | > > * | | | mgr_update(when sb is rw i.e. pb->chassis) > | | | > > - * | | | has been updated > | | | > > + * | | V has been updated > | | | > > + * | | +----------------------+ > | | | > > + * | | | | > | | | > > + * | | | WAITING_SB_COND | > | | | > > + * | | | | > | | | > > + * | | | | > | | | > > + * | | +----------------------+ > | | | > > + * | | | > | | | > > + * | | | > | | | > > + * | | | mgr_update(when sb_cond_seqno == expected) > | | | > > + * | | | - request seqno > | | | > > + * | | | > | | | > > * | | release_iface | - request seqno > | | | > > * | | | > | | | > > * | | V > | | | > > @@ -335,6 +353,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr, > > > > switch (iface->state) { > > case OIF_CLAIMED: > > + case OIF_WAITING_SB_COND: > > case OIF_INSTALL_FLOWS: > > case OIF_REM_OLD_OVN_INST: > > case OIF_MARK_UP: > > @@ -383,6 +402,7 @@ if_status_mgr_release_iface(struct if_status_mgr > *mgr, const char *iface_id) > > > > switch (iface->state) { > > case OIF_CLAIMED: > > + case OIF_WAITING_SB_COND: > > case OIF_INSTALL_FLOWS: > > /* Not yet fully installed interfaces: > > * pb->chassis still need to be deleted. > > @@ -424,6 +444,7 @@ if_status_mgr_delete_iface(struct if_status_mgr > *mgr, const char *iface_id, > > > > switch (iface->state) { > > case OIF_CLAIMED: > > + case OIF_WAITING_SB_COND: > > case OIF_INSTALL_FLOWS: > > /* Not yet fully installed interfaces: > > * pb->chassis still need to be deleted. > > @@ -500,6 +521,8 @@ if_status_mgr_update(struct if_status_mgr *mgr, > > const struct sbrec_chassis *chassis_rec, > > const struct ovsrec_interface_table *iface_table, > > const struct sbrec_port_binding_table *pb_table, > > + const struct hmap *local_datapaths, > > + const struct uuidset *waiting_sb_update, > > At this point, the argument name is too vague, these are datapaths but > it's hard to tell from the argument name. Maybe 'dps_waiting_for_sb'. > > Also, I think we should never call this function with a NULL > 'waiting_sb_update' argument. Maybe an "ovs_assert(dps_waiting_for_sb)" > just in the beginning of the function? > > > bool ovs_readonly, > > bool sb_readonly) > > { > > @@ -622,9 +645,7 @@ if_status_mgr_update(struct if_status_mgr *mgr, > > * in if_status_handle_claims or if_status_mgr_claim_iface > > */ > > if (iface->is_vif) { > > - ovs_iface_set_state(mgr, iface, OIF_INSTALL_FLOWS); > > - iface->install_seqno = mgr->iface_seqno + 1; > > - new_ifaces = true; > > + ovs_iface_set_state(mgr, iface, OIF_WAITING_SB_COND); > > } else { > > ovs_iface_set_state(mgr, iface, OIF_MARK_UP); > > } > > @@ -639,6 +660,33 @@ if_status_mgr_update(struct if_status_mgr *mgr, > > } > > } > > > > + if (!sb_readonly) { > > + HMAPX_FOR_EACH_SAFE (node, > > + > &mgr->ifaces_per_state[OIF_WAITING_SB_COND]) { > > + struct ovs_iface *iface = node->data; > > + if (local_datapaths) { > > + const struct sbrec_port_binding *pb = > > + sbrec_port_binding_table_get_for_uuid(pb_table, > > + > &iface->pb_uuid); > > + ovs_assert(pb); > > As mentioned in the review of v8, we should probably change this to a > graceful NULL check. > > > + struct local_datapath *ld = > > + get_local_datapath(local_datapaths, > > + pb->datapath->tunnel_key); > > + if (!ld) { > > + continue; > > + } > > + if (waiting_sb_update && > > As mentioned above, we'd never have a NULL uuidset here so we can skip > this part of the condition. > > > + !uuidset_find(waiting_sb_update, > > + &ld->datapath->header_.uuid)) { > > + ovs_iface_set_state(mgr, iface, OIF_INSTALL_FLOWS); > > + iface->install_seqno = mgr->iface_seqno + 1; > > + new_ifaces = true; > > + } > > + > > Unnecessary empty line. > > > + } > > + } > > + } > > + > > if (!sb_readonly) { > > HMAPX_FOR_EACH_SAFE (node, > &mgr->ifaces_per_state[OIF_UPDATE_PORT]) { > > struct ovs_iface *iface = node->data; > > diff --git a/controller/if-status.h b/controller/if-status.h > > index d15ca3008..8c057bf6b 100644 > > --- a/controller/if-status.h > > +++ b/controller/if-status.h > > @@ -18,6 +18,7 @@ > > > > #include "openvswitch/shash.h" > > #include "lib/vswitch-idl.h" > > +#include "lib/uuidset.h" > > > > #include "binding.h" > > #include "lport.h" > > @@ -43,6 +44,8 @@ void if_status_mgr_update(struct if_status_mgr *, > struct local_binding_data *, > > const struct sbrec_chassis *chassis, > > const struct ovsrec_interface_table > *iface_table, > > const struct sbrec_port_binding_table > *pb_table, > > + const struct hmap *local_datapaths, > > + const struct uuidset *waiting_sb_update, > > bool ovs_readonly, > > bool sb_readonly); > > void if_status_mgr_run(struct if_status_mgr *mgr, struct > local_binding_data *, > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index 35a5cd0b4..91c659718 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -170,6 +170,7 @@ static char *unixctl_path; > > struct controller_engine_ctx { > > struct lflow_cache *lflow_cache; > > struct if_status_mgr *if_mgr; > > + unsigned int *ovnsb_expected_cond_seqno; > > Nit: const unsigned int * > > > }; > > > > /* Pending packet to be injected into connected OVS. */ > > @@ -1145,10 +1146,51 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) > > * track that column which should be addressed in the future. */ > > } > > > > +struct ed_type_datapaths_updated { > > + struct uuidset waiting_sb_cond_update; > > +}; > > + > > +static void * > > +en_datapaths_updated_init(struct engine_node *node OVS_UNUSED, > > + struct engine_arg *arg OVS_UNUSED) > > +{ > > + struct ed_type_datapaths_updated *data = xzalloc(sizeof *data); > > + uuidset_init(&data->waiting_sb_cond_update); > > Nit: I'd do: > > struct ed_type_datapaths_updated *data = xmalloc(sizeof *data); > *data = (struct ed_type_datapaths_updated) { > .waiting_sb_cond_update = > UUIDSET_INITIALIZER(&data->waiting_sb_cond_update), > }; > > > + return data; > > +} > > + > > +static void > > +en_datapaths_updated_cleanup(void *data) > > +{ > > + struct ed_type_datapaths_updated *sb_data = data; > > + uuidset_destroy(&sb_data->waiting_sb_cond_update); > > +} > > + > > struct ed_type_ofctrl_is_connected { > > bool connected; > > }; > > I guess the en_datapaths_updated_run() function definition should've > been inserted above this struct definition. > > > > > +static enum engine_node_state > > +en_datapaths_updated_run(struct engine_node *node OVS_UNUSED, > > + void *data) > > +{ > > + struct controller_engine_ctx *ctrl_ctx = > engine_get_context()->client_ctx; > > + struct ovsdb_idl_txn *ovnsb_idl_txn = > engine_get_context()->ovnsb_idl_txn; > > + if (!ovnsb_idl_txn) { > > + return EN_UNCHANGED; > > + } > > + struct ovsdb_idl *ovnsb_idl = ovsdb_idl_txn_get_idl(ovnsb_idl_txn); > > + if (*ctrl_ctx->ovnsb_expected_cond_seqno == > > + ovsdb_idl_get_condition_seqno(ovnsb_idl)) { > > + struct ed_type_datapaths_updated *dp_data = data; > > + if (!uuidset_is_empty(&dp_data->waiting_sb_cond_update)) { > > + uuidset_clear(&dp_data->waiting_sb_cond_update); > > + return EN_UNCHANGED; > > + } > > + } > > + return EN_UNCHANGED; > > It's kind of weird that this always returns EN_UNCHANGED. > > > +} > > + > > static void * > > en_ofctrl_is_connected_init(struct engine_node *node OVS_UNUSED, > > struct engine_arg *arg OVS_UNUSED) > > @@ -1180,6 +1222,41 @@ en_ofctrl_is_connected_run(struct engine_node > *node OVS_UNUSED, void *data) > > return EN_UNCHANGED; > > } > > > > +struct ed_type_sb_cond_seqno { > > + unsigned int last_sb_cond_seqno; > > +}; > > + > > +static void * > > +en_sb_cond_seqno_init(struct engine_node *node OVS_UNUSED, > > + struct engine_arg *arg OVS_UNUSED) > > +{ > > + struct ed_type_sb_cond_seqno *data = xzalloc(sizeof *data); > > + return data; > > +} > > + > > +static void en_sb_cond_seqno_cleanup (void *data OVS_UNUSED) > > Please add the parenthesis just after the function name. Also, we > always add a new line between function return type and function name > when defining a function, i.e.: > > static void > en_sb_cond_seqno_cleanup(void *data OVS_UNUSED) > { > } > > > +{ > > +} > > + > > +static enum engine_node_state > > +en_sb_cond_seqno_run(struct engine_node *node OVS_UNUSED, void *data) > > +{ > > + struct ed_type_sb_cond_seqno *sb_seqno_data = data; > > + struct ovsdb_idl_txn *ovnsb_idl_txn = > engine_get_context()->ovnsb_idl_txn; > > + if (!ovnsb_idl_txn) { > > + return EN_UNCHANGED; > > + } > > + > > + unsigned int curr_seqno = > > + > ovsdb_idl_get_condition_seqno(ovsdb_idl_txn_get_idl(ovnsb_idl_txn)); > > + > > + if (sb_seqno_data->last_sb_cond_seqno != curr_seqno) { > > + sb_seqno_data->last_sb_cond_seqno = curr_seqno; > > + return EN_UPDATED; > > + } > > + return EN_UNCHANGED; > > +} > > + > > struct ed_type_if_status_mgr { > > const struct if_status_mgr *manager; > > const struct ovsrec_interface_table *iface_table; > > @@ -1512,6 +1589,7 @@ en_runtime_data_clear_tracked_data(void *data_) > > } > > > > static void * > > + > > Please remove this unrelated newline. > > > en_runtime_data_init(struct engine_node *node OVS_UNUSED, > > struct engine_arg *arg OVS_UNUSED) > > { > > @@ -6745,6 +6823,50 @@ evpn_arp_vtep_binding_handler(struct engine_node > *node, void *data OVS_UNUSED) > > return EN_UNHANDLED; > > } > > > > Please move the handler definition where we define the rest of the node > functions, i.e., just after en_datapaths_updated_run(). > > > +static enum engine_input_handler_result > > +new_datapaths_handler(struct engine_node *node, > > + void *data) > > void *data should be indented the same as the previous argument above. > > > +{ > > + struct ed_type_datapaths_updated *dp_data = data; > > + struct ed_type_runtime_data *rt_data = > > + engine_get_input_data("runtime_data", node); > > + > > + enum engine_input_handler_result status = EN_HANDLED_UNCHANGED; > > + struct tracked_datapath *tdp; > > + HMAP_FOR_EACH_SAFE (tdp, node, &rt_data->tracked_dp_bindings) { > > + if (tdp->tracked_type == TRACKED_RESOURCE_NEW) { > > + uuidset_insert(&dp_data->waiting_sb_cond_update, > > + &tdp->dp->header_.uuid); > > Shouldn't we make this conditional too, i.e., only if > ovnsb_expected_cond_seqno is not the right one? In the monitor-all case > there's no need to move these to waiting_sb_cond_update, right? > > We can't check the sequance number here. At this point it has not been updated for the new datapath. I added a boolean for if we are in monitor-all, let me know if there is something else that would be better. > > + status = EN_HANDLED_UPDATED; > > + } > > + } > > + return status; > > +} > > + > > +static enum engine_input_handler_result > > +datapaths_update_sb_cond_handler(struct engine_node *node OVS_UNUSED, > > + void *data) > > +{ > > + struct controller_engine_ctx *ctrl_ctx = > engine_get_context()->client_ctx; > > + struct ovsdb_idl_txn *ovnsb_idl_txn = > engine_get_context()->ovnsb_idl_txn; > > + if (!ovnsb_idl_txn) { > > + return EN_HANDLED_UNCHANGED; > > + } > > + struct ovsdb_idl *ovnsb_idl = ovsdb_idl_txn_get_idl(ovnsb_idl_txn); > > + if (ovnsb_idl_txn) { > > We checked !ovnsb_idl_txn just above, we can remove this one from here. > > > + if (*ctrl_ctx->ovnsb_expected_cond_seqno == > > + ovsdb_idl_get_condition_seqno(ovnsb_idl)) { > > + struct ed_type_datapaths_updated *dp_data = data; > > + > > + if (!uuidset_is_empty(&dp_data->waiting_sb_cond_update)) { > > + uuidset_clear(&dp_data->waiting_sb_cond_update); > > + return EN_HANDLED_UPDATED; > > + } > > + } > > + } > > + return EN_HANDLED_UNCHANGED; > > +} > > + > > /* Define engine node functions for nodes that represent SB tables. > > * > > * en_sb_<TABLE_NAME>_run() > > @@ -6867,6 +6989,8 @@ static ENGINE_NODE(neighbor_exchange_status); > > static ENGINE_NODE(evpn_vtep_binding, CLEAR_TRACKED_DATA); > > static ENGINE_NODE(evpn_fdb, CLEAR_TRACKED_DATA); > > static ENGINE_NODE(evpn_arp, CLEAR_TRACKED_DATA); > > +static ENGINE_NODE(datapaths_updated); > > +static ENGINE_NODE(sb_cond_seqno); > > > > static void > > inc_proc_ovn_controller_init( > > @@ -6889,6 +7013,8 @@ inc_proc_ovn_controller_init( > > engine_add_input(&en_template_vars, &en_sb_chassis_template_var, > > template_vars_sb_chassis_template_var_handler); > > > > + engine_add_input(&en_datapaths_updated, &en_sb_cond_seqno, > > + datapaths_update_sb_cond_handler); > > Let's group this one together with the handler for en_runtime data > below. We try to group all dependencies for each node. > > > engine_add_input(&en_lb_data, &en_sb_load_balancer, > > lb_data_sb_load_balancer_handler); > > engine_add_input(&en_lb_data, &en_template_vars, > > @@ -6975,6 +7101,9 @@ inc_proc_ovn_controller_init( > > engine_add_input(&en_pflow_output, &en_sb_sb_global, > > pflow_output_debug_handler); > > > > + engine_add_input(&en_datapaths_updated, &en_runtime_data, > > + new_datapaths_handler); > > The (I guess undocumented) convention for handler function names is > <NODE-NAME>_<INPUT-NODE-NAME>_handler() so in this case: > > datapaths_updated_runtime_data_handler() > > Also, nit, indentation: you need one more space before the handler name. > > > + > > engine_add_input(&en_northd_options, &en_sb_sb_global, > > en_northd_options_sb_sb_global_handler); > > > > @@ -7163,6 +7292,7 @@ inc_proc_ovn_controller_init( > > engine_add_input(&en_acl_id, &en_sb_acl_id, NULL); > > engine_add_input(&en_controller_output, &en_acl_id, > > controller_output_acl_id_handler); > > + engine_add_input(&en_controller_output, &en_datapaths_updated, > NULL); > > We shouldn't use a NULL handler for inputs of "en_controller_output" > (the final node of the DAG). Instead we should use a handler like the > ones we add for other nodes (e.g., > controller_output_route_exchange_handler()). > > > > > struct engine_arg engine_arg = { > > .sb_idl = sb_idl_loop->idl, > > @@ -7556,6 +7686,8 @@ main(int argc, char *argv[]) > > engine_get_internal_data(&en_evpn_fdb); > > struct ed_type_evpn_arp *earp_data = > > engine_get_internal_data(&en_evpn_arp); > > + struct ed_type_datapaths_updated *dp_updated_data = > > + engine_get_internal_data(&en_datapaths_updated); > > > > ofctrl_init(&lflow_output_data->group_table, > > &lflow_output_data->meter_table); > > @@ -7659,6 +7791,7 @@ main(int argc, char *argv[]) > > struct controller_engine_ctx ctrl_engine_ctx = { > > .lflow_cache = lflow_cache_create(), > > .if_mgr = if_status_mgr_create(), > > + .ovnsb_expected_cond_seqno = &ovnsb_expected_cond_seqno, > > }; > > struct if_status_mgr *if_mgr = ctrl_engine_ctx.if_mgr; > > > > @@ -7915,6 +8048,7 @@ main(int argc, char *argv[]) > > > > bool recompute_allowed = (ovnsb_idl_txn && > > !ofctrl_has_backlog()); > > + > > Please remove this unrelated newline. > > > engine_run(recompute_allowed); > > tracked_acl_ids = engine_get_data(&en_acl_id); > > > > @@ -8085,11 +8219,23 @@ main(int argc, char *argv[]) > > runtime_data ? &runtime_data->lbinding_data : > NULL; > > stopwatch_start(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME, > > time_msec()); > > + if (sb_monitor_all && dp_updated_data) { > > + uuidset_clear( > > + &dp_updated_data->waiting_sb_cond_update); > > + } > > See my comment about the monitor-all case above, if you check the cond > seqno in the handler you don't need this clear anymore. > > Also, it's very awkward to clear node-internal data outside the > incremental processing engine. > > > + > > + struct uuidset *waiting_sb_cond_update = > dp_updated_data ? > > const struct uuidset *? > > OTOH, dp_updated_data is always non-NULL, I'd just pass > &dp_updated_data->waiting_sb_cond_update to if_status_mgr_update(). > > > + &dp_updated_data->waiting_sb_cond_update > > + : NULL; > > if_status_mgr_update(if_mgr, binding_data, chassis, > > ovsrec_interface_table_get( > > ovs_idl_loop.idl), > > sbrec_port_binding_table_get( > > ovnsb_idl_loop.idl), > > + runtime_data ? > > + > &runtime_data->local_datapaths > > + : NULL, > > + waiting_sb_cond_update, > > !ovs_idl_txn, > > !ovnsb_idl_txn); > > stopwatch_stop(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME, > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > > index c98de9bc4..9dc7555ba 100644 > > --- a/tests/ovn-controller.at > > +++ b/tests/ovn-controller.at > > @@ -3944,3 +3944,69 @@ OVN_CLEANUP([hv1], [hv2 > > /already has encap ip.*cannot duplicate on/d]) > > AT_CLEANUP > > ]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([ovn-installed]) > > +ovn_start > > + > > +net_add n1 > > +sim_add hv1 > > + > > +as hv1 > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.1 > > +ovn-appctl vlog/set dbg > > +ovs-vsctl add-port br-int vif1 -- \ > > + set Interface vif1 external-ids:iface-id=lsp1 > > + > > +check ovn-nbctl ls-add ls1 > > +sleep_controller hv1 > > +check ovn-nbctl --wait=sb lsp-add ls1 lsp1 -- \ > > + lsp-set-addresses lsp1 "f0:00:00:00:00:01 > 10.0.0.1" > > + > > +sleep_sb > > +wake_up_controller hv1 > > + > > +# Wait for pflow for lsp1 > > +OVS_WAIT_UNTIL([ > > + ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface > name=vif1) > > + echo "vif1 port=$ofport" > > + test -n "$ofport" && test 1 -le $(as hv1 ovs-ofctl dump-flows > br-int | grep -c in_port=$ofport) > > +]) > > + > > +# If ovn-installed in ovs, all flows should be installed. > > +# In that case, there should be at least one flow with lsp1 address. > > +OVS_WAIT_UNTIL([ > > + ovn_installed=$(as hv1 ovs-vsctl get Interface vif1 > external_ids:ovn-installed) > > + echo $ovn_installed > > + flow_count=$(as hv1 ovs-ofctl dump-flows br-int | grep -Fc > "10.0.0.1") > > + # for the monitor-all=true case the flow gets installed because > ovn-controller is monitoring all > > + # tables in OVN_SOUTHBOUND. > > + if test -n "$ovn_installed"; then > > + as hv1 ovs-ofctl dump-flows br-int > output > > Leftover from debugging? > > > + test $flow_count -ge 1 > > + else > > + true > > + fi > > +]) > > + > > +wake_up_sb > > +# After the southbound db has woken up and can send the update to the > > +# ovn-controller not monitoring all tables in the southbound db it > > +# should be able to install the interface. > > +OVS_WAIT_UNTIL([ > > + ovn_installed=$(as hv1 ovs-vsctl get Interface vif1 > external_ids:ovn-installed) > > + flow_count=$(as hv1 ovs-ofctl dump-flows br-int | grep -Fc > "10.0.0.1") > > + echo "installed=$ovn_installed, count=$flow_count" > > + if test -n "$ovn_installed"; then > > + as hv1 ovs-ofctl dump-flows br-int > output > > Leftover from debugging? > > > + test $flow_count -ge 1 > > + else > > + false > > + fi > > +]) > > +wait_for_ports_up > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > +]) > > diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ > ovn-inc-proc-graph-dump.at > > index 178310978..036d44891 100644 > > --- a/tests/ovn-inc-proc-graph-dump.at > > +++ b/tests/ovn-inc-proc-graph-dump.at > > @@ -478,6 +478,10 @@ digraph "Incremental-Processing-Engine" { > > SB_acl_id [[style=filled, shape=box, fillcolor=white, > label="SB_acl_id"]]; > > acl_id [[style=filled, shape=box, fillcolor=white, > label="acl_id"]]; > > SB_acl_id -> acl_id [[label=""]]; > > + sb_cond_seqno [[style=filled, shape=box, fillcolor=white, > label="sb_cond_seqno"]]; > > + datapaths_updated [[style=filled, shape=box, fillcolor=white, > label="datapaths_updated"]]; > > + sb_cond_seqno -> datapaths_updated > [[label="datapaths_update_sb_cond_handler"]]; > > + runtime_data -> datapaths_updated > [[label="new_datapaths_handler"]]; > > controller_output [[style=filled, shape=box, fillcolor=white, > label="controller_output"]]; > > dns_cache -> controller_output [[label=""]]; > > lflow_output -> controller_output > [[label="controller_output_lflow_output_handler"]]; > > @@ -487,6 +491,7 @@ digraph "Incremental-Processing-Engine" { > > route_exchange -> controller_output > [[label="controller_output_route_exchange_handler"]]; > > garp_rarp -> controller_output > [[label="controller_output_garp_rarp_handler"]]; > > acl_id -> controller_output > [[label="controller_output_acl_id_handler"]]; > > + datapaths_updated -> controller_output [[label=""]]; > > } > > ]) > > AT_CLEANUP > > Regards, > Dumitru > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
