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?
> + 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
