| Message ID | 67010dca0a8dcc390e873349d662c9163e01050f.1746623091.git.felix.huettner@stackit.cloud |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | controller: Add garp_rarp engine. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Wed, May 07, 2025 at 04:21:02PM +0200, Felix Huettner via dev wrote: > When starting the ovn-controller without monitor_all we need a few > iterations of the engine node and updating monitoring conditions until > we have loaded all data. > Previously this was handled by the number of main loop iterations of > ovn-controller and an arbitrary timeout. > However this is was always best effort and there was no guarantee that > we actually had loaded all data. > > To solve this we now track the monitoring requests we sent out to > southbound and when that data becomes available. > > For monitor_all we just need to have the initial monitoring conditions > fullfilled and are afterwards sure that we have all required data. > > For non monitor_all we need at least one more iteration of the engine > node and updating sb monitoring conditions. We limit this to 20 rounds > for now, which should be sufficient and is at least the value of what > the previous implementation provided. > However if we notice that we do not update monitoring condition anymore > after a engine run we can also assume that we have finished loading all > data. > Recheck-request: github-robot > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > --- > v8->v9: addressed reviews > v7->v8: introduced > > controller/ovn-controller.c | 27 ++++++++++++++++++++++++++- > lib/ovn-util.c | 19 ++++++++++++++----- > tests/ovn-controller.at | 1 - > tests/ovn-performance.at | 3 --- > tests/ovn.at | 10 ---------- > 5 files changed, 40 insertions(+), 20 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 13a566d26..946e6146d 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -6029,6 +6029,7 @@ main(int argc, char *argv[]) > unsigned int ovs_cond_seqno = UINT_MAX; > unsigned int ovnsb_cond_seqno = UINT_MAX; > unsigned int ovnsb_expected_cond_seqno = UINT_MAX; > + bool has_computed_once = false; > > struct controller_engine_ctx ctrl_engine_ctx = { > .lflow_cache = lflow_cache_create(), > @@ -6103,6 +6104,20 @@ main(int argc, char *argv[]) > ovnsb_cond_seqno = new_ovnsb_cond_seqno; > } > > + /* Check if we have received all initial dumps of the southbound > + * based on the monitor condtions we set. > + * If we have sb_monitor_all that means we have all data that we would > + * ever need. > + * If we monitor cases individually we need at least one engine run. */ > + if (ovnsb_cond_seqno == ovnsb_expected_cond_seqno > + && ovnsb_expected_cond_seqno != UINT_MAX) { > + if (sb_monitor_all) { > + daemon_started_recently_ignore(); > + } else if (has_computed_once) { > + daemon_started_recently_countdown(); > + } > + } > + > struct engine_context eng_ctx = { > .ovs_idl_txn = ovs_idl_txn, > .ovnsb_idl_txn = ovnsb_idl_txn, > @@ -6260,8 +6275,9 @@ main(int argc, char *argv[]) > > stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, > time_msec()); > + > if (engine_has_updated()) { > - daemon_started_recently_countdown(); > + has_computed_once = true; > } > > ct_zones_data = engine_get_data(&en_ct_zones); > @@ -6380,6 +6396,15 @@ main(int argc, char *argv[]) > &runtime_data->local_datapaths, > sb_monitor_all); > } > + /* If there is no new expected seqno we have > + * finished loading all needed data from > + * southbound. We then need to run one more time since > + * we might behave differently. */ > + if (daemon_started_recently() > + && ovnsb_cond_seqno == ovnsb_expected_cond_seqno) { > + daemon_started_recently_ignore(); > + poll_immediate_wake(); > + } > if (ovs_idl_txn) { > update_qos(sbrec_port_binding_by_name, ovs_idl_txn, > ovsrec_port_by_qos, > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index af17082a9..153a69ff6 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -933,13 +933,24 @@ get_bridge(const struct ovsrec_bridge_table *bridge_table, const char *br_name) > return NULL; > } > > +/* This counts the amount of iterations of monitoring condition updates and > + * their respective update towards us. > + * This only needs to handle the case of non monitor_all since that will use > + * the ignore feature below. > + * In this case we need at least 2 iterations: > + * 1. the initial iteration where we pull the sb content based on the few > + * conditions we now have. > + * 2. after the first engine run we have enough information to update the > + * monitoring conditions to their final values. > + * However based on the structure of the datapaths we might need more. The > + * value below is just a hard limit of iterations. We detect if we are done > + * earlier and then skip further iterations. */ > #define DAEMON_STARTUP_DELAY_SEED 20 > -#define DAEMON_STARTUP_DELAY_MS 10000 > > static int64_t startup_ts; > static int startup_delay = DAEMON_STARTUP_DELAY_SEED; > > -/* Used by debug command only, for tests. */ > +/* Used if we do not need the startup delay (e.g. when using monitor_all). */ > static bool ignore_startup_delay = false; > > OVS_CONSTRUCTOR(startup_ts_initializer) { > @@ -980,9 +991,7 @@ daemon_started_recently(void) > if (startup_delay) { > return true; > } > - > - /* Ensure that at least an amount of time has passed. */ > - return time_wall_msec() - startup_ts <= DAEMON_STARTUP_DELAY_MS; > + return false; > } > > /* Builds a unique address set compatible name ([a-zA-Z_.][a-zA-Z_.0-9]*) > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index a878a440b..781ee242a 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -161,7 +161,6 @@ check_patches \ > 'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' > > # Delete the mapping and the ovn-bridge-mapping patch ports should go away. > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > AT_CHECK([ovs-vsctl remove Open_vSwitch . external-ids ovn-bridge-mappings]) > check_bridge_mappings > check_patches > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > index 44bd4f7e4..4ddb1bd81 100644 > --- a/tests/ovn-performance.at > +++ b/tests/ovn-performance.at > @@ -238,9 +238,6 @@ for i in `seq 1 5`; do > as hv$i > ovs-vsctl add-br br-phys > ovn_attach n1 br-phys 192.168.0.$i > - # Do not postpone patch ports related changes, as they may occur more or less anywere within the test, > - # hence causing unexpected recomputes. > - check ovn-appctl -t ovn-controller debug/ignore-startup-delay > if [[ $i -ge 3 ]] ; then > ovs-vsctl add-br br-ex > ovs-vsctl set open . external_ids:ovn-bridge-mappings="public:br-ex" > diff --git a/tests/ovn.at b/tests/ovn.at > index 596a7be2b..9af74774f 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -35301,7 +35301,6 @@ OVS_WAIT_UNTIL([ > test x430 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid} mtu_request) > ]) > > -as hv1 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > # Check that pointing requested-chassis somewhere else will unplug the port > check ovn-nbctl --wait=hv set Logical_Switch_Port lsp1 \ > options:requested-chassis=non-existent-chassis > @@ -35309,7 +35308,6 @@ OVS_WAIT_UNTIL([ > ! as hv1 ovs-vsctl get Interface ${iface1_uuid} _uuid > ]) > > -as hv2 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > # Check that removing an lport will unplug it > AT_CHECK([test x${iface2_uuid} = x$(as hv2 ovs-vsctl get Interface ${iface2_uuid} _uuid)], [0], []) > check ovn-nbctl --wait=hv lsp-del ${lsp2_uuid} > @@ -38073,9 +38071,6 @@ ovs-vsctl -- add-port br-phys-2 fake-phys-2-to-int-2 -- \ > > ovn_attach n1 br-phys 192.168.1.1 24 > > -# allow controller to immediately clean patch ports up > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > - > # check that patch port on br-int is cleaned up (and also its peer) > OVS_WAIT_UNTIL([ > test 0 = $(ovs-vsctl --columns _uuid --bare find Port name=fake-int-to-phys | wc -l) > @@ -38141,9 +38136,6 @@ check ovn-nbctl lsp-set-options ln_port network_name=phys-1 > wait_for_ports_up > check ovn-nbctl --wait=hv sync > > -# Allow controller to immediately clean patch ports up if any. > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > - > # check that patch port created on br-int and br-phys-1. > OVS_WAIT_UNTIL([ > test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=patch-br-int-to-ln_port | wc -l) > @@ -41407,8 +41399,6 @@ check ovn-nbctl lsp-add sw0 sw0p1 > > check ovn-nbctl --wait=hv sync > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > - > # Waits until the OVS database contains exactly the specified patch ports. > # Each argument should be of the form BRIDGE PORT PEER. > check_patch_ports () { > -- > 2.43.0 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Wed, May 7, 2025 at 4:21 PM Felix Huettner via dev < ovs-dev@openvswitch.org> wrote: > When starting the ovn-controller without monitor_all we need a few > iterations of the engine node and updating monitoring conditions until > we have loaded all data. > Previously this was handled by the number of main loop iterations of > ovn-controller and an arbitrary timeout. > However this is was always best effort and there was no guarantee that > we actually had loaded all data. > > To solve this we now track the monitoring requests we sent out to > southbound and when that data becomes available. > > For monitor_all we just need to have the initial monitoring conditions > fullfilled and are afterwards sure that we have all required data. > > For non monitor_all we need at least one more iteration of the engine > node and updating sb monitoring conditions. We limit this to 20 rounds > for now, which should be sufficient and is at least the value of what > the previous implementation provided. > However if we notice that we do not update monitoring condition anymore > after a engine run we can also assume that we have finished loading all > data. > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > --- > Hi Felix, sorry for the recent silence on this one. There is related discussion happening on https://patchwork.ozlabs.org/project/ovn/patch/20250318100817.117922-2-alekssmirnov@k2.cloud/ . It would be nice to get your input there too so we can move on with this. > v8->v9: addressed reviews > v7->v8: introduced > > controller/ovn-controller.c | 27 ++++++++++++++++++++++++++- > lib/ovn-util.c | 19 ++++++++++++++----- > tests/ovn-controller.at | 1 - > tests/ovn-performance.at | 3 --- > tests/ovn.at | 10 ---------- > 5 files changed, 40 insertions(+), 20 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 13a566d26..946e6146d 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -6029,6 +6029,7 @@ main(int argc, char *argv[]) > unsigned int ovs_cond_seqno = UINT_MAX; > unsigned int ovnsb_cond_seqno = UINT_MAX; > unsigned int ovnsb_expected_cond_seqno = UINT_MAX; > + bool has_computed_once = false; > > struct controller_engine_ctx ctrl_engine_ctx = { > .lflow_cache = lflow_cache_create(), > @@ -6103,6 +6104,20 @@ main(int argc, char *argv[]) > ovnsb_cond_seqno = new_ovnsb_cond_seqno; > } > > + /* Check if we have received all initial dumps of the southbound > + * based on the monitor condtions we set. > + * If we have sb_monitor_all that means we have all data that we > would > + * ever need. > + * If we monitor cases individually we need at least one engine > run. */ > + if (ovnsb_cond_seqno == ovnsb_expected_cond_seqno > + && ovnsb_expected_cond_seqno != UINT_MAX) { > + if (sb_monitor_all) { > + daemon_started_recently_ignore(); > + } else if (has_computed_once) { > + daemon_started_recently_countdown(); > + } > + } > + > struct engine_context eng_ctx = { > .ovs_idl_txn = ovs_idl_txn, > .ovnsb_idl_txn = ovnsb_idl_txn, > @@ -6260,8 +6275,9 @@ main(int argc, char *argv[]) > > stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, > time_msec()); > + > if (engine_has_updated()) { > - daemon_started_recently_countdown(); > + has_computed_once = true; > } > > ct_zones_data = engine_get_data(&en_ct_zones); > @@ -6380,6 +6396,15 @@ main(int argc, char *argv[]) > &runtime_data->local_datapaths, > sb_monitor_all); > } > + /* If there is no new expected seqno we have > + * finished loading all needed data from > + * southbound. We then need to run one more time > since > + * we might behave differently. */ > + if (daemon_started_recently() > + && ovnsb_cond_seqno == > ovnsb_expected_cond_seqno) { > + daemon_started_recently_ignore(); > + poll_immediate_wake(); > + } > if (ovs_idl_txn) { > update_qos(sbrec_port_binding_by_name, > ovs_idl_txn, > ovsrec_port_by_qos, > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index af17082a9..153a69ff6 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -933,13 +933,24 @@ get_bridge(const struct ovsrec_bridge_table > *bridge_table, const char *br_name) > return NULL; > } > > +/* This counts the amount of iterations of monitoring condition updates > and > + * their respective update towards us. > + * This only needs to handle the case of non monitor_all since that will > use > + * the ignore feature below. > + * In this case we need at least 2 iterations: > + * 1. the initial iteration where we pull the sb content based on the few > + * conditions we now have. > + * 2. after the first engine run we have enough information to update the > + * monitoring conditions to their final values. > + * However based on the structure of the datapaths we might need more. The > + * value below is just a hard limit of iterations. We detect if we are > done > + * earlier and then skip further iterations. */ > #define DAEMON_STARTUP_DELAY_SEED 20 > -#define DAEMON_STARTUP_DELAY_MS 10000 > > static int64_t startup_ts; > static int startup_delay = DAEMON_STARTUP_DELAY_SEED; > > -/* Used by debug command only, for tests. */ > +/* Used if we do not need the startup delay (e.g. when using > monitor_all). */ > static bool ignore_startup_delay = false; > > OVS_CONSTRUCTOR(startup_ts_initializer) { > @@ -980,9 +991,7 @@ daemon_started_recently(void) > if (startup_delay) { > return true; > } > - > - /* Ensure that at least an amount of time has passed. */ > - return time_wall_msec() - startup_ts <= DAEMON_STARTUP_DELAY_MS; > + return false; > } > > /* Builds a unique address set compatible name ([a-zA-Z_.][a-zA-Z_.0-9]*) > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index a878a440b..781ee242a 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -161,7 +161,6 @@ check_patches \ > 'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' > > # Delete the mapping and the ovn-bridge-mapping patch ports should go > away. > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > AT_CHECK([ovs-vsctl remove Open_vSwitch . external-ids > ovn-bridge-mappings]) > check_bridge_mappings > check_patches > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > index 44bd4f7e4..4ddb1bd81 100644 > --- a/tests/ovn-performance.at > +++ b/tests/ovn-performance.at > @@ -238,9 +238,6 @@ for i in `seq 1 5`; do > as hv$i > ovs-vsctl add-br br-phys > ovn_attach n1 br-phys 192.168.0.$i > - # Do not postpone patch ports related changes, as they may occur more > or less anywere within the test, > - # hence causing unexpected recomputes. > - check ovn-appctl -t ovn-controller debug/ignore-startup-delay > if [[ $i -ge 3 ]] ; then > ovs-vsctl add-br br-ex > ovs-vsctl set open . > external_ids:ovn-bridge-mappings="public:br-ex" > diff --git a/tests/ovn.at b/tests/ovn.at > index 596a7be2b..9af74774f 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -35301,7 +35301,6 @@ OVS_WAIT_UNTIL([ > test x430 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid} > mtu_request) > ]) > > -as hv1 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > # Check that pointing requested-chassis somewhere else will unplug the > port > check ovn-nbctl --wait=hv set Logical_Switch_Port lsp1 \ > options:requested-chassis=non-existent-chassis > @@ -35309,7 +35308,6 @@ OVS_WAIT_UNTIL([ > ! as hv1 ovs-vsctl get Interface ${iface1_uuid} _uuid > ]) > > -as hv2 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > # Check that removing an lport will unplug it > AT_CHECK([test x${iface2_uuid} = x$(as hv2 ovs-vsctl get Interface > ${iface2_uuid} _uuid)], [0], []) > check ovn-nbctl --wait=hv lsp-del ${lsp2_uuid} > @@ -38073,9 +38071,6 @@ ovs-vsctl -- add-port br-phys-2 > fake-phys-2-to-int-2 -- \ > > ovn_attach n1 br-phys 192.168.1.1 24 > > -# allow controller to immediately clean patch ports up > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > - > # check that patch port on br-int is cleaned up (and also its peer) > OVS_WAIT_UNTIL([ > test 0 = $(ovs-vsctl --columns _uuid --bare find Port > name=fake-int-to-phys | wc -l) > @@ -38141,9 +38136,6 @@ check ovn-nbctl lsp-set-options ln_port > network_name=phys-1 > wait_for_ports_up > check ovn-nbctl --wait=hv sync > > -# Allow controller to immediately clean patch ports up if any. > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > - > # check that patch port created on br-int and br-phys-1. > OVS_WAIT_UNTIL([ > test 1 = $(ovs-vsctl --columns _uuid --bare find Port > name=patch-br-int-to-ln_port | wc -l) > @@ -41407,8 +41399,6 @@ check ovn-nbctl lsp-add sw0 sw0p1 > > check ovn-nbctl --wait=hv sync > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > - > # Waits until the OVS database contains exactly the specified patch ports. > # Each argument should be of the form BRIDGE PORT PEER. > check_patch_ports () { > -- > 2.43.0 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales
On Tue, May 27, 2025 at 07:18:46AM +0200, Ales Musil wrote: > On Wed, May 7, 2025 at 4:21 PM Felix Huettner via dev < > ovs-dev@openvswitch.org> wrote: > > > When starting the ovn-controller without monitor_all we need a few > > iterations of the engine node and updating monitoring conditions until > > we have loaded all data. > > Previously this was handled by the number of main loop iterations of > > ovn-controller and an arbitrary timeout. > > However this is was always best effort and there was no guarantee that > > we actually had loaded all data. > > > > To solve this we now track the monitoring requests we sent out to > > southbound and when that data becomes available. > > > > For monitor_all we just need to have the initial monitoring conditions > > fullfilled and are afterwards sure that we have all required data. > > > > For non monitor_all we need at least one more iteration of the engine > > node and updating sb monitoring conditions. We limit this to 20 rounds > > for now, which should be sufficient and is at least the value of what > > the previous implementation provided. > > However if we notice that we do not update monitoring condition anymore > > after a engine run we can also assume that we have finished loading all > > data. > > > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > > --- > > > > Hi Felix, > > sorry for the recent silence on this one. There is related discussion > happening on > https://patchwork.ozlabs.org/project/ovn/patch/20250318100817.117922-2-alekssmirnov@k2.cloud/ > . > It would be nice to get your input there too so we can move on > with this. Hi Ales, no worries i was also busy on other topics. I think Alexander has a valid point there. I would wait for his further input and then send a new version that is also rebased against main. Thanks a lot, Felix > > > > v8->v9: addressed reviews > > v7->v8: introduced > > > > controller/ovn-controller.c | 27 ++++++++++++++++++++++++++- > > lib/ovn-util.c | 19 ++++++++++++++----- > > tests/ovn-controller.at | 1 - > > tests/ovn-performance.at | 3 --- > > tests/ovn.at | 10 ---------- > > 5 files changed, 40 insertions(+), 20 deletions(-) > > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index 13a566d26..946e6146d 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -6029,6 +6029,7 @@ main(int argc, char *argv[]) > > unsigned int ovs_cond_seqno = UINT_MAX; > > unsigned int ovnsb_cond_seqno = UINT_MAX; > > unsigned int ovnsb_expected_cond_seqno = UINT_MAX; > > + bool has_computed_once = false; > > > > struct controller_engine_ctx ctrl_engine_ctx = { > > .lflow_cache = lflow_cache_create(), > > @@ -6103,6 +6104,20 @@ main(int argc, char *argv[]) > > ovnsb_cond_seqno = new_ovnsb_cond_seqno; > > } > > > > + /* Check if we have received all initial dumps of the southbound > > + * based on the monitor condtions we set. > > + * If we have sb_monitor_all that means we have all data that we > > would > > + * ever need. > > + * If we monitor cases individually we need at least one engine > > run. */ > > + if (ovnsb_cond_seqno == ovnsb_expected_cond_seqno > > + && ovnsb_expected_cond_seqno != UINT_MAX) { > > + if (sb_monitor_all) { > > + daemon_started_recently_ignore(); > > + } else if (has_computed_once) { > > + daemon_started_recently_countdown(); > > + } > > + } > > + > > struct engine_context eng_ctx = { > > .ovs_idl_txn = ovs_idl_txn, > > .ovnsb_idl_txn = ovnsb_idl_txn, > > @@ -6260,8 +6275,9 @@ main(int argc, char *argv[]) > > > > stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, > > time_msec()); > > + > > if (engine_has_updated()) { > > - daemon_started_recently_countdown(); > > + has_computed_once = true; > > } > > > > ct_zones_data = engine_get_data(&en_ct_zones); > > @@ -6380,6 +6396,15 @@ main(int argc, char *argv[]) > > &runtime_data->local_datapaths, > > sb_monitor_all); > > } > > + /* If there is no new expected seqno we have > > + * finished loading all needed data from > > + * southbound. We then need to run one more time > > since > > + * we might behave differently. */ > > + if (daemon_started_recently() > > + && ovnsb_cond_seqno == > > ovnsb_expected_cond_seqno) { > > + daemon_started_recently_ignore(); > > + poll_immediate_wake(); > > + } > > if (ovs_idl_txn) { > > update_qos(sbrec_port_binding_by_name, > > ovs_idl_txn, > > ovsrec_port_by_qos, > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > index af17082a9..153a69ff6 100644 > > --- a/lib/ovn-util.c > > +++ b/lib/ovn-util.c > > @@ -933,13 +933,24 @@ get_bridge(const struct ovsrec_bridge_table > > *bridge_table, const char *br_name) > > return NULL; > > } > > > > +/* This counts the amount of iterations of monitoring condition updates > > and > > + * their respective update towards us. > > + * This only needs to handle the case of non monitor_all since that will > > use > > + * the ignore feature below. > > + * In this case we need at least 2 iterations: > > + * 1. the initial iteration where we pull the sb content based on the few > > + * conditions we now have. > > + * 2. after the first engine run we have enough information to update the > > + * monitoring conditions to their final values. > > + * However based on the structure of the datapaths we might need more. The > > + * value below is just a hard limit of iterations. We detect if we are > > done > > + * earlier and then skip further iterations. */ > > #define DAEMON_STARTUP_DELAY_SEED 20 > > -#define DAEMON_STARTUP_DELAY_MS 10000 > > > > static int64_t startup_ts; > > static int startup_delay = DAEMON_STARTUP_DELAY_SEED; > > > > -/* Used by debug command only, for tests. */ > > +/* Used if we do not need the startup delay (e.g. when using > > monitor_all). */ > > static bool ignore_startup_delay = false; > > > > OVS_CONSTRUCTOR(startup_ts_initializer) { > > @@ -980,9 +991,7 @@ daemon_started_recently(void) > > if (startup_delay) { > > return true; > > } > > - > > - /* Ensure that at least an amount of time has passed. */ > > - return time_wall_msec() - startup_ts <= DAEMON_STARTUP_DELAY_MS; > > + return false; > > } > > > > /* Builds a unique address set compatible name ([a-zA-Z_.][a-zA-Z_.0-9]*) > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > > index a878a440b..781ee242a 100644 > > --- a/tests/ovn-controller.at > > +++ b/tests/ovn-controller.at > > @@ -161,7 +161,6 @@ check_patches \ > > 'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' > > > > # Delete the mapping and the ovn-bridge-mapping patch ports should go > > away. > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > AT_CHECK([ovs-vsctl remove Open_vSwitch . external-ids > > ovn-bridge-mappings]) > > check_bridge_mappings > > check_patches > > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > > index 44bd4f7e4..4ddb1bd81 100644 > > --- a/tests/ovn-performance.at > > +++ b/tests/ovn-performance.at > > @@ -238,9 +238,6 @@ for i in `seq 1 5`; do > > as hv$i > > ovs-vsctl add-br br-phys > > ovn_attach n1 br-phys 192.168.0.$i > > - # Do not postpone patch ports related changes, as they may occur more > > or less anywere within the test, > > - # hence causing unexpected recomputes. > > - check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > if [[ $i -ge 3 ]] ; then > > ovs-vsctl add-br br-ex > > ovs-vsctl set open . > > external_ids:ovn-bridge-mappings="public:br-ex" > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 596a7be2b..9af74774f 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -35301,7 +35301,6 @@ OVS_WAIT_UNTIL([ > > test x430 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid} > > mtu_request) > > ]) > > > > -as hv1 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > # Check that pointing requested-chassis somewhere else will unplug the > > port > > check ovn-nbctl --wait=hv set Logical_Switch_Port lsp1 \ > > options:requested-chassis=non-existent-chassis > > @@ -35309,7 +35308,6 @@ OVS_WAIT_UNTIL([ > > ! as hv1 ovs-vsctl get Interface ${iface1_uuid} _uuid > > ]) > > > > -as hv2 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > # Check that removing an lport will unplug it > > AT_CHECK([test x${iface2_uuid} = x$(as hv2 ovs-vsctl get Interface > > ${iface2_uuid} _uuid)], [0], []) > > check ovn-nbctl --wait=hv lsp-del ${lsp2_uuid} > > @@ -38073,9 +38071,6 @@ ovs-vsctl -- add-port br-phys-2 > > fake-phys-2-to-int-2 -- \ > > > > ovn_attach n1 br-phys 192.168.1.1 24 > > > > -# allow controller to immediately clean patch ports up > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > - > > # check that patch port on br-int is cleaned up (and also its peer) > > OVS_WAIT_UNTIL([ > > test 0 = $(ovs-vsctl --columns _uuid --bare find Port > > name=fake-int-to-phys | wc -l) > > @@ -38141,9 +38136,6 @@ check ovn-nbctl lsp-set-options ln_port > > network_name=phys-1 > > wait_for_ports_up > > check ovn-nbctl --wait=hv sync > > > > -# Allow controller to immediately clean patch ports up if any. > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > - > > # check that patch port created on br-int and br-phys-1. > > OVS_WAIT_UNTIL([ > > test 1 = $(ovs-vsctl --columns _uuid --bare find Port > > name=patch-br-int-to-ln_port | wc -l) > > @@ -41407,8 +41399,6 @@ check ovn-nbctl lsp-add sw0 sw0p1 > > > > check ovn-nbctl --wait=hv sync > > > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > - > > # Waits until the OVS database contains exactly the specified patch ports. > > # Each argument should be of the form BRIDGE PORT PEER. > > check_patch_ports () { > > -- > > 2.43.0 > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > Thanks, > Ales
On Tue, May 27, 2025 at 4:56 PM Felix Huettner <felix.huettner@stackit.cloud> wrote: > On Tue, May 27, 2025 at 07:18:46AM +0200, Ales Musil wrote: > > On Wed, May 7, 2025 at 4:21 PM Felix Huettner via dev < > > ovs-dev@openvswitch.org> wrote: > > > > > When starting the ovn-controller without monitor_all we need a few > > > iterations of the engine node and updating monitoring conditions until > > > we have loaded all data. > > > Previously this was handled by the number of main loop iterations of > > > ovn-controller and an arbitrary timeout. > > > However this is was always best effort and there was no guarantee that > > > we actually had loaded all data. > > > > > > To solve this we now track the monitoring requests we sent out to > > > southbound and when that data becomes available. > > > > > > For monitor_all we just need to have the initial monitoring conditions > > > fullfilled and are afterwards sure that we have all required data. > > > > > > For non monitor_all we need at least one more iteration of the engine > > > node and updating sb monitoring conditions. We limit this to 20 rounds > > > for now, which should be sufficient and is at least the value of what > > > the previous implementation provided. > > > However if we notice that we do not update monitoring condition anymore > > > after a engine run we can also assume that we have finished loading all > > > data. > > > > > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > > > --- > > > > > > > Hi Felix, > > > > sorry for the recent silence on this one. There is related discussion > > happening on > > > https://patchwork.ozlabs.org/project/ovn/patch/20250318100817.117922-2-alekssmirnov@k2.cloud/ > > . > > It would be nice to get your input there too so we can move on > > with this. > > Hi Ales, > Hi Felix, > no worries i was also busy on other topics. > > I think Alexander has a valid point there. > I would wait for his further input and then send a new version that is > also rebased against main. > Based on this I'll mark the series as changes requested for now and we will continue on the next revision when the conversation is settled. > > Thanks a lot, > Felix > > > > > > > > v8->v9: addressed reviews > > > v7->v8: introduced > > > > > > controller/ovn-controller.c | 27 ++++++++++++++++++++++++++- > > > lib/ovn-util.c | 19 ++++++++++++++----- > > > tests/ovn-controller.at | 1 - > > > tests/ovn-performance.at | 3 --- > > > tests/ovn.at | 10 ---------- > > > 5 files changed, 40 insertions(+), 20 deletions(-) > > > > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > > index 13a566d26..946e6146d 100644 > > > --- a/controller/ovn-controller.c > > > +++ b/controller/ovn-controller.c > > > @@ -6029,6 +6029,7 @@ main(int argc, char *argv[]) > > > unsigned int ovs_cond_seqno = UINT_MAX; > > > unsigned int ovnsb_cond_seqno = UINT_MAX; > > > unsigned int ovnsb_expected_cond_seqno = UINT_MAX; > > > + bool has_computed_once = false; > > > > > > struct controller_engine_ctx ctrl_engine_ctx = { > > > .lflow_cache = lflow_cache_create(), > > > @@ -6103,6 +6104,20 @@ main(int argc, char *argv[]) > > > ovnsb_cond_seqno = new_ovnsb_cond_seqno; > > > } > > > > > > + /* Check if we have received all initial dumps of the > southbound > > > + * based on the monitor condtions we set. > > > + * If we have sb_monitor_all that means we have all data that > we > > > would > > > + * ever need. > > > + * If we monitor cases individually we need at least one > engine > > > run. */ > > > + if (ovnsb_cond_seqno == ovnsb_expected_cond_seqno > > > + && ovnsb_expected_cond_seqno != UINT_MAX) { > > > + if (sb_monitor_all) { > > > + daemon_started_recently_ignore(); > > > + } else if (has_computed_once) { > > > + daemon_started_recently_countdown(); > > > + } > > > + } > > > + > > > struct engine_context eng_ctx = { > > > .ovs_idl_txn = ovs_idl_txn, > > > .ovnsb_idl_txn = ovnsb_idl_txn, > > > @@ -6260,8 +6275,9 @@ main(int argc, char *argv[]) > > > > > > stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, > > > time_msec()); > > > + > > > if (engine_has_updated()) { > > > - daemon_started_recently_countdown(); > > > + has_computed_once = true; > > > } > > > > > > ct_zones_data = engine_get_data(&en_ct_zones); > > > @@ -6380,6 +6396,15 @@ main(int argc, char *argv[]) > > > &runtime_data->local_datapaths, > > > sb_monitor_all); > > > } > > > + /* If there is no new expected seqno we have > > > + * finished loading all needed data from > > > + * southbound. We then need to run one more > time > > > since > > > + * we might behave differently. */ > > > + if (daemon_started_recently() > > > + && ovnsb_cond_seqno == > > > ovnsb_expected_cond_seqno) { > > > + daemon_started_recently_ignore(); > > > + poll_immediate_wake(); > > > + } > > > if (ovs_idl_txn) { > > > update_qos(sbrec_port_binding_by_name, > > > ovs_idl_txn, > > > ovsrec_port_by_qos, > > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > > index af17082a9..153a69ff6 100644 > > > --- a/lib/ovn-util.c > > > +++ b/lib/ovn-util.c > > > @@ -933,13 +933,24 @@ get_bridge(const struct ovsrec_bridge_table > > > *bridge_table, const char *br_name) > > > return NULL; > > > } > > > > > > +/* This counts the amount of iterations of monitoring condition > updates > > > and > > > + * their respective update towards us. > > > + * This only needs to handle the case of non monitor_all since that > will > > > use > > > + * the ignore feature below. > > > + * In this case we need at least 2 iterations: > > > + * 1. the initial iteration where we pull the sb content based on the > few > > > + * conditions we now have. > > > + * 2. after the first engine run we have enough information to update > the > > > + * monitoring conditions to their final values. > > > + * However based on the structure of the datapaths we might need > more. The > > > + * value below is just a hard limit of iterations. We detect if we are > > > done > > > + * earlier and then skip further iterations. */ > > > #define DAEMON_STARTUP_DELAY_SEED 20 > > > -#define DAEMON_STARTUP_DELAY_MS 10000 > > > > > > static int64_t startup_ts; > > > static int startup_delay = DAEMON_STARTUP_DELAY_SEED; > > > > > > -/* Used by debug command only, for tests. */ > > > +/* Used if we do not need the startup delay (e.g. when using > > > monitor_all). */ > > > static bool ignore_startup_delay = false; > > > > > > OVS_CONSTRUCTOR(startup_ts_initializer) { > > > @@ -980,9 +991,7 @@ daemon_started_recently(void) > > > if (startup_delay) { > > > return true; > > > } > > > - > > > - /* Ensure that at least an amount of time has passed. */ > > > - return time_wall_msec() - startup_ts <= DAEMON_STARTUP_DELAY_MS; > > > + return false; > > > } > > > > > > /* Builds a unique address set compatible name > ([a-zA-Z_.][a-zA-Z_.0-9]*) > > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > > > index a878a440b..781ee242a 100644 > > > --- a/tests/ovn-controller.at > > > +++ b/tests/ovn-controller.at > > > @@ -161,7 +161,6 @@ check_patches \ > > > 'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' > > > > > > # Delete the mapping and the ovn-bridge-mapping patch ports should go > > > away. > > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > > AT_CHECK([ovs-vsctl remove Open_vSwitch . external-ids > > > ovn-bridge-mappings]) > > > check_bridge_mappings > > > check_patches > > > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > > > index 44bd4f7e4..4ddb1bd81 100644 > > > --- a/tests/ovn-performance.at > > > +++ b/tests/ovn-performance.at > > > @@ -238,9 +238,6 @@ for i in `seq 1 5`; do > > > as hv$i > > > ovs-vsctl add-br br-phys > > > ovn_attach n1 br-phys 192.168.0.$i > > > - # Do not postpone patch ports related changes, as they may occur > more > > > or less anywere within the test, > > > - # hence causing unexpected recomputes. > > > - check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > > if [[ $i -ge 3 ]] ; then > > > ovs-vsctl add-br br-ex > > > ovs-vsctl set open . > > > external_ids:ovn-bridge-mappings="public:br-ex" > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > index 596a7be2b..9af74774f 100644 > > > --- a/tests/ovn.at > > > +++ b/tests/ovn.at > > > @@ -35301,7 +35301,6 @@ OVS_WAIT_UNTIL([ > > > test x430 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid} > > > mtu_request) > > > ]) > > > > > > -as hv1 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > > # Check that pointing requested-chassis somewhere else will unplug the > > > port > > > check ovn-nbctl --wait=hv set Logical_Switch_Port lsp1 \ > > > options:requested-chassis=non-existent-chassis > > > @@ -35309,7 +35308,6 @@ OVS_WAIT_UNTIL([ > > > ! as hv1 ovs-vsctl get Interface ${iface1_uuid} _uuid > > > ]) > > > > > > -as hv2 check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > > # Check that removing an lport will unplug it > > > AT_CHECK([test x${iface2_uuid} = x$(as hv2 ovs-vsctl get Interface > > > ${iface2_uuid} _uuid)], [0], []) > > > check ovn-nbctl --wait=hv lsp-del ${lsp2_uuid} > > > @@ -38073,9 +38071,6 @@ ovs-vsctl -- add-port br-phys-2 > > > fake-phys-2-to-int-2 -- \ > > > > > > ovn_attach n1 br-phys 192.168.1.1 24 > > > > > > -# allow controller to immediately clean patch ports up > > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > > - > > > # check that patch port on br-int is cleaned up (and also its peer) > > > OVS_WAIT_UNTIL([ > > > test 0 = $(ovs-vsctl --columns _uuid --bare find Port > > > name=fake-int-to-phys | wc -l) > > > @@ -38141,9 +38136,6 @@ check ovn-nbctl lsp-set-options ln_port > > > network_name=phys-1 > > > wait_for_ports_up > > > check ovn-nbctl --wait=hv sync > > > > > > -# Allow controller to immediately clean patch ports up if any. > > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > > - > > > # check that patch port created on br-int and br-phys-1. > > > OVS_WAIT_UNTIL([ > > > test 1 = $(ovs-vsctl --columns _uuid --bare find Port > > > name=patch-br-int-to-ln_port | wc -l) > > > @@ -41407,8 +41399,6 @@ check ovn-nbctl lsp-add sw0 sw0p1 > > > > > > check ovn-nbctl --wait=hv sync > > > > > > -check ovn-appctl -t ovn-controller debug/ignore-startup-delay > > > - > > > # Waits until the OVS database contains exactly the specified patch > ports. > > > # Each argument should be of the form BRIDGE PORT PEER. > > > check_patch_ports () { > > > -- > > > 2.43.0 > > > > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > Thanks, > > Ales > > Thanks, Ales
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 13a566d26..946e6146d 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -6029,6 +6029,7 @@ main(int argc, char *argv[]) unsigned int ovs_cond_seqno = UINT_MAX; unsigned int ovnsb_cond_seqno = UINT_MAX; unsigned int ovnsb_expected_cond_seqno = UINT_MAX; + bool has_computed_once = false; struct controller_engine_ctx ctrl_engine_ctx = { .lflow_cache = lflow_cache_create(), @@ -6103,6 +6104,20 @@ main(int argc, char *argv[]) ovnsb_cond_seqno = new_ovnsb_cond_seqno; } + /* Check if we have received all initial dumps of the southbound + * based on the monitor condtions we set. + * If we have sb_monitor_all that means we have all data that we would + * ever need. + * If we monitor cases individually we need at least one engine run. */ + if (ovnsb_cond_seqno == ovnsb_expected_cond_seqno + && ovnsb_expected_cond_seqno != UINT_MAX) { + if (sb_monitor_all) { + daemon_started_recently_ignore(); + } else if (has_computed_once) { + daemon_started_recently_countdown(); + } + } + struct engine_context eng_ctx = { .ovs_idl_txn = ovs_idl_txn, .ovnsb_idl_txn = ovnsb_idl_txn, @@ -6260,8 +6275,9 @@ main(int argc, char *argv[]) stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, time_msec()); + if (engine_has_updated()) { - daemon_started_recently_countdown(); + has_computed_once = true; } ct_zones_data = engine_get_data(&en_ct_zones); @@ -6380,6 +6396,15 @@ main(int argc, char *argv[]) &runtime_data->local_datapaths, sb_monitor_all); } + /* If there is no new expected seqno we have + * finished loading all needed data from + * southbound. We then need to run one more time since + * we might behave differently. */ + if (daemon_started_recently() + && ovnsb_cond_seqno == ovnsb_expected_cond_seqno) { + daemon_started_recently_ignore(); + poll_immediate_wake(); + } if (ovs_idl_txn) { update_qos(sbrec_port_binding_by_name, ovs_idl_txn, ovsrec_port_by_qos, diff --git a/lib/ovn-util.c b/lib/ovn-util.c index af17082a9..153a69ff6 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -933,13 +933,24 @@ get_bridge(const struct ovsrec_bridge_table *bridge_table, const char *br_name) return NULL; } +/* This counts the amount of iterations of monitoring condition updates and + * their respective update towards us. + * This only needs to handle the case of non monitor_all since that will use + * the ignore feature below. + * In this case we need at least 2 iterations: + * 1. the initial iteration where we pull the sb content based on the few + * conditions we now have. + * 2. after the first engine run we have enough information to update the + * monitoring conditions to their final values. + * However based on the structure of the datapaths we might need more. The + * value below is just a hard limit of iterations. We detect if we are done + * earlier and then skip further iterations. */ #define DAEMON_STARTUP_DELAY_SEED 20 -#define DAEMON_STARTUP_DELAY_MS 10000 static int64_t startup_ts; static int startup_delay = DAEMON_STARTUP_DELAY_SEED; -/* Used by debug command only, for tests. */ +/* Used if we do not need the startup delay (e.g. when using monitor_all). */ static bool ignore_startup_delay = false; OVS_CONSTRUCTOR(startup_ts_initializer) { @@ -980,9 +991,7 @@ daemon_started_recently(void) if (startup_delay) { return true; } - - /* Ensure that at least an amount of time has passed. */ - return time_wall_msec() - startup_ts <= DAEMON_STARTUP_DELAY_MS; + return false; } /* Builds a unique address set compatible name ([a-zA-Z_.][a-zA-Z_.0-9]*) diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index a878a440b..781ee242a 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -161,7 +161,6 @@ check_patches \ 'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' # Delete the mapping and the ovn-bridge-mapping patch ports should go away. -check ovn-appctl -t ovn-controller debug/ignore-startup-delay AT_CHECK([ovs-vsctl remove Open_vSwitch . external-ids ovn-bridge-mappings]) check_bridge_mappings check_patches diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 44bd4f7e4..4ddb1bd81 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -238,9 +238,6 @@ for i in `seq 1 5`; do as hv$i ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.$i - # Do not postpone patch ports related changes, as they may occur more or less anywere within the test, - # hence causing unexpected recomputes. - check ovn-appctl -t ovn-controller debug/ignore-startup-delay if [[ $i -ge 3 ]] ; then ovs-vsctl add-br br-ex ovs-vsctl set open . external_ids:ovn-bridge-mappings="public:br-ex" diff --git a/tests/ovn.at b/tests/ovn.at index 596a7be2b..9af74774f 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -35301,7 +35301,6 @@ OVS_WAIT_UNTIL([ test x430 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid} mtu_request) ]) -as hv1 check ovn-appctl -t ovn-controller debug/ignore-startup-delay # Check that pointing requested-chassis somewhere else will unplug the port check ovn-nbctl --wait=hv set Logical_Switch_Port lsp1 \ options:requested-chassis=non-existent-chassis @@ -35309,7 +35308,6 @@ OVS_WAIT_UNTIL([ ! as hv1 ovs-vsctl get Interface ${iface1_uuid} _uuid ]) -as hv2 check ovn-appctl -t ovn-controller debug/ignore-startup-delay # Check that removing an lport will unplug it AT_CHECK([test x${iface2_uuid} = x$(as hv2 ovs-vsctl get Interface ${iface2_uuid} _uuid)], [0], []) check ovn-nbctl --wait=hv lsp-del ${lsp2_uuid} @@ -38073,9 +38071,6 @@ ovs-vsctl -- add-port br-phys-2 fake-phys-2-to-int-2 -- \ ovn_attach n1 br-phys 192.168.1.1 24 -# allow controller to immediately clean patch ports up -check ovn-appctl -t ovn-controller debug/ignore-startup-delay - # check that patch port on br-int is cleaned up (and also its peer) OVS_WAIT_UNTIL([ test 0 = $(ovs-vsctl --columns _uuid --bare find Port name=fake-int-to-phys | wc -l) @@ -38141,9 +38136,6 @@ check ovn-nbctl lsp-set-options ln_port network_name=phys-1 wait_for_ports_up check ovn-nbctl --wait=hv sync -# Allow controller to immediately clean patch ports up if any. -check ovn-appctl -t ovn-controller debug/ignore-startup-delay - # check that patch port created on br-int and br-phys-1. OVS_WAIT_UNTIL([ test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=patch-br-int-to-ln_port | wc -l) @@ -41407,8 +41399,6 @@ check ovn-nbctl lsp-add sw0 sw0p1 check ovn-nbctl --wait=hv sync -check ovn-appctl -t ovn-controller debug/ignore-startup-delay - # Waits until the OVS database contains exactly the specified patch ports. # Each argument should be of the form BRIDGE PORT PEER. check_patch_ports () {
When starting the ovn-controller without monitor_all we need a few iterations of the engine node and updating monitoring conditions until we have loaded all data. Previously this was handled by the number of main loop iterations of ovn-controller and an arbitrary timeout. However this is was always best effort and there was no guarantee that we actually had loaded all data. To solve this we now track the monitoring requests we sent out to southbound and when that data becomes available. For monitor_all we just need to have the initial monitoring conditions fullfilled and are afterwards sure that we have all required data. For non monitor_all we need at least one more iteration of the engine node and updating sb monitoring conditions. We limit this to 20 rounds for now, which should be sufficient and is at least the value of what the previous implementation provided. However if we notice that we do not update monitoring condition anymore after a engine run we can also assume that we have finished loading all data. Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> --- v8->v9: addressed reviews v7->v8: introduced controller/ovn-controller.c | 27 ++++++++++++++++++++++++++- lib/ovn-util.c | 19 ++++++++++++++----- tests/ovn-controller.at | 1 - tests/ovn-performance.at | 3 --- tests/ovn.at | 10 ---------- 5 files changed, 40 insertions(+), 20 deletions(-)