On Thu, Jan 25, 2024, 3:29 PM Dumitru Ceara <dce...@redhat.com> wrote:
> On 1/25/24 17:46, Numan Siddique wrote: > > On Tue, Jan 16, 2024 at 6:15 AM Dumitru Ceara <dce...@redhat.com> wrote: > >> > >> On 1/11/24 16:28, num...@ovn.org wrote: > >>> From: Numan Siddique <num...@ovn.org> > >>> > >>> It also moves the logical router port IPv6 prefix delegation > >>> updates to "sync-from-sb" engine node. > >>> > >>> With these changes, northd engine node doesn't need to do much > >>> for SB Port binding changes other than updating 'op->sb'. And > >>> there is no need to fall back to recompute. Prior to this patch > >>> northd_handle_sb_port_binding_changes() was returning false for > >>> all SB port binding updates except for the VIF PBs. This patch > >>> now handles updates to all the SB port binding by setting 'op->sb'. > >>> > >>> Signed-off-by: Numan Siddique <num...@ovn.org> > >>> --- > >> > >> Hi Numan, > >> > >> With this patch applied the "633: ovn-northd.at:10298 SB Port binding > >> incremental processing -- parallelization=yes" test occasionally fails, > >> e.g.: > >> > >> https://github.com/dceara/ovn/actions/runs/7539663594/job/20524238508 > >> > >> I saw it locally too when running the test in a loop: > >> > >> It fails with: > >> ovn-nbctl lrp-set-gateway-chassis lrp hv1 > >> ./ovn-macros.at:449: "$@" > >> northd recompute count - 2 > >> ./ovn-northd.at:10238: test x$northd_recomp = x$1 > >> ./ovn-northd.at:10238: exit code was 1, expected 0 > >> > >> Here: > >> > https://github.com/dceara/ovn/blob/e1c76b6d347570618591fe6587f6af83445223ec/tests/ovn-northd.at#L10327 > >> > >> For some reason in some cases only 2 recomputes get triggered. > > > > Thanks Dumitru and Han for the reviews. > > > > @Dumitru Ceara I've addressed all the review comments below. > > > > I think the test case is failing because of the missing "--wait=sb" in > > the ovn-nbctl commands. > > > > I don't think it's only that. It still fails even with your updated > test changes, in the same way: > > as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > ./ovn-macros.at:449: "$@" > ovn-nbctl lrp-set-gateway-chassis lrp hv1 > ./ovn-macros.at:449: "$@" > northd recompute count - 2 > ./ovn-northd.at:10238: test x$northd_recomp = x$1 > ./ovn-northd.at:10238: exit code was 1, expected 0 > > > Will the below changes in ovn-northd.at look good to you ? If so, > > and if you can provide your ack I can apply this patch. > > Otherwise I'll include it in the v6. > > > > I would prefer if we don't apply it until we figure out this test, the > CI has been relatively stable the last few weeks, I'd rather not > destabilize it if possible. > Sure. Definitely. Its passing for me locally when run in a loop. Let me dig into it further. Thanks Numan > Regards, > Dumitru > > > ---- > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 72eb9e1173..492248b05d 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -10316,12 +10316,12 @@ check ovn-nbctl --wait=sb lr-add lr0 > > > > # Test normal VIF port > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-add ls0 p1 > > +check ovn-nbctl --wait=sb lsp-add ls0 p1 > > check_recompute_counter 0 0 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-set-addresses p1 "00:00:00:00:01:01 10.0.0.3" > > +check ovn-nbctl --wait=sb lsp-set-addresses p1 "00:00:00:00:01:01 > 10.0.0.3" > > check_recompute_counter 0 0 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -10329,12 +10329,13 @@ check as northd ovn-appctl -t ovn-northd > vlog/set info > > > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > check ovn-sbctl lsp-bind p1 hv1 > > +check ovn-nbctl --wait=sb sync > > check_recompute_counter 0 0 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Test lsp of type router > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-add ls0 rp -- lsp-set-type rp router > > +check ovn-nbctl --wait=sb lsp-add ls0 rp -- lsp-set-type rp router > > > > # northd engine recomputes twice. Both the times for handling NB > > logical switch port > > # changes and not because of SB port binding changes. This is > > because ovn-northd > > @@ -10344,37 +10345,37 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Set some options to 'rp'. northd should only recompute once. > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-set-options rp foo=bar > > +check ovn-nbctl --wait=sb lsp-set-options rp foo=bar > > check_recompute_counter 1 1 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Test lsp of type external > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-add ls0 e1 -- lsp-set-type e1 external > > +check ovn-nbctl --wait=sb lsp-add ls0 e1 -- lsp-set-type e1 external > > check_recompute_counter 2 2 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Set some options to 'e1'. northd should only recompute once. > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-set-options e1 foo=bar > > +check ovn-nbctl --wait=sb lsp-set-options e1 foo=bar > > check_recompute_counter 1 1 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Test lrp > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lrp-add lr0 lrp 00:00:02:01:02:03 10.0.0.1/24 > > +check ovn-nbctl --wait=sb lrp-add lr0 lrp 00:00:02:01:02:03 10.0.0.1/24 > > check_recompute_counter 1 1 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Set some options on 'lrp'. northd should only recompute once. > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lrp-set-options lrp route_table=rtb-1 > > +check ovn-nbctl --wait=sb lrp-set-options lrp route_table=rtb-1 > > check_recompute_counter 1 1 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Make lrp a gateway port > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lrp-set-gateway-chassis lrp hv1 > > +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lrp hv1 > > # There will be 3 recomputes of northd engine node > > # 1. missing handler for input NB_logical_router > > # 2. missing handler for input SB_ha_chassis_group > > @@ -10384,7 +10385,7 @@ check_recompute_counter 3 3 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl clear logical_router_port lrp gateway_chassis > > +check ovn-nbctl --wait=sb clear logical_router_port lrp gateway_chassis > > # There will be 2 recomputes of northd engine node > > # 1. missing handler for input NB_logical_router > > # 2. missing handler for input SB_ha_chassis_group > > @@ -10393,17 +10394,17 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > # Delete some of the ports. > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-del p1 > > +check ovn-nbctl --wait=sb lsp-del p1 > > check_recompute_counter 0 0 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lsp-del e1 > > +check ovn-nbctl --wait=sb lsp-del e1 > > check_recompute_counter 1 1 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > -check ovn-nbctl lrp-del lrp > > +check ovn-nbctl --wait=sb lrp-del lrp > > check_recompute_counter 1 1 > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > -------------------------------- > > > > Thanks > > Numan > > > > > > > >> > >>> northd/en-northd.c | 2 +- > >>> northd/en-sync-sb.c | 3 +- > >>> northd/northd.c | 296 ++++++++++++++++++++++++++------------------ > >>> northd/northd.h | 6 +- > >>> tests/ovn-northd.at | 158 +++++++++++++++++++++-- > >>> 5 files changed, 334 insertions(+), 131 deletions(-) > >>> > >>> diff --git a/northd/en-northd.c b/northd/en-northd.c > >>> index 28559ed211..677b2b1ab0 100644 > >>> --- a/northd/en-northd.c > >>> +++ b/northd/en-northd.c > >>> @@ -189,7 +189,7 @@ northd_sb_port_binding_handler(struct engine_node > *node, > >>> northd_get_input_data(node, &input_data); > >>> > >>> if (!northd_handle_sb_port_binding_changes( > >>> - input_data.sbrec_port_binding_table, &nd->ls_ports)) { > >>> + input_data.sbrec_port_binding_table, &nd->ls_ports, > &nd->lr_ports)) { > >>> return false; > >>> } > >>> > >>> diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c > >>> index 3aaab8d005..45be7ddbcb 100644 > >>> --- a/northd/en-sync-sb.c > >>> +++ b/northd/en-sync-sb.c > >>> @@ -288,7 +288,8 @@ en_sync_to_sb_pb_run(struct engine_node *node, > void *data OVS_UNUSED) > >>> const struct engine_context *eng_ctx = engine_get_context(); > >>> struct northd_data *northd_data = engine_get_input_data("northd", > node); > >>> > >>> - sync_pbs(eng_ctx->ovnsb_idl_txn, &northd_data->ls_ports); > >>> + sync_pbs(eng_ctx->ovnsb_idl_txn, &northd_data->ls_ports, > >>> + &northd_data->lr_ports); > >>> engine_set_node_state(node, EN_UPDATED); > >>> } > >>> > >>> diff --git a/northd/northd.c b/northd/northd.c > >>> index f32e3bf21a..23f2dae26b 100644 > >>> --- a/northd/northd.c > >>> +++ b/northd/northd.c > >>> @@ -3435,6 +3435,9 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn > *ovnsb_txn, > >>> { > >>> sbrec_port_binding_set_datapath(op->sb, op->od->sb); > >>> if (op->nbrp) { > >>> + /* Note: SB port binding options for router ports are set in > >>> + * sync_pbs(). */ > >>> + > >>> /* If the router is for l3 gateway, it resides on a chassis > >>> * and its port type is "l3gateway". */ > >>> const char *chassis_name = smap_get(&op->od->nbr->options, > "chassis"); > >>> @@ -3446,15 +3449,11 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn > *ovnsb_txn, > >>> sbrec_port_binding_set_type(op->sb, "patch"); > >>> } > >>> > >>> - struct smap new; > >>> - smap_init(&new); > >>> if (is_cr_port(op)) { > >>> ovs_assert(sbrec_chassis_by_name); > >>> ovs_assert(sbrec_chassis_by_hostname); > >>> ovs_assert(sbrec_ha_chassis_grp_by_name); > >>> ovs_assert(active_ha_chassis_grps); > >>> - const char *redirect_type = smap_get(&op->nbrp->options, > >>> - "redirect-type"); > >>> > >>> if (op->nbrp->ha_chassis_group) { > >>> if (op->nbrp->n_gateway_chassis) { > >>> @@ -3496,49 +3495,8 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn > *ovnsb_txn, > >>> /* Delete the legacy gateway_chassis from the pb. */ > >>> sbrec_port_binding_set_gateway_chassis(op->sb, NULL, > 0); > >>> } > >>> - smap_add(&new, "distributed-port", op->nbrp->name); > >>> - > >>> - bool always_redirect = > >>> - !op->od->has_distributed_nat && > >>> - > !l3dgw_port_has_associated_vtep_lports(op->l3dgw_port); > >>> - > >>> - if (redirect_type) { > >>> - smap_add(&new, "redirect-type", redirect_type); > >>> - /* XXX Why can't we enable always-redirect when > redirect-type > >>> - * is bridged? */ > >>> - if (!strcmp(redirect_type, "bridged")) { > >>> - always_redirect = false; > >>> - } > >>> - } > >>> - > >>> - if (always_redirect) { > >>> - smap_add(&new, "always-redirect", "true"); > >>> - } > >>> - } else { > >>> - if (op->peer) { > >>> - smap_add(&new, "peer", op->peer->key); > >>> - if (op->nbrp->ha_chassis_group || > >>> - op->nbrp->n_gateway_chassis) { > >>> - char *redirect_name = > >>> - ovn_chassis_redirect_name(op->nbrp->name); > >>> - smap_add(&new, "chassis-redirect-port", > redirect_name); > >>> - free(redirect_name); > >>> - } > >>> - } > >>> - if (chassis_name) { > >>> - smap_add(&new, "l3gateway-chassis", chassis_name); > >>> - } > >>> } > >>> > >>> - const char *ipv6_pd_list = smap_get(&op->sb->options, > >>> - "ipv6_ra_pd_list"); > >>> - if (ipv6_pd_list) { > >>> - smap_add(&new, "ipv6_ra_pd_list", ipv6_pd_list); > >>> - } > >>> - > >>> - sbrec_port_binding_set_options(op->sb, &new); > >>> - smap_destroy(&new); > >>> - > >>> sbrec_port_binding_set_parent_port(op->sb, NULL); > >>> sbrec_port_binding_set_tag(op->sb, NULL, 0); > >>> > >>> @@ -4768,12 +4726,14 @@ check_sb_lb_duplicates(const struct > sbrec_load_balancer_table *table) > >>> return duplicates; > >>> } > >>> > >>> -/* Syncs the SB port binding for the ovn_port 'op'. Caller should > make sure > >>> - * that the OVN SB IDL txn is not NULL. Presently it only syncs the > nat > >>> - * column of port binding corresponding to the 'op->nbsp' */ > >>> +/* Syncs the SB port binding for the ovn_port 'op' of a logical > switch port. > >>> + * Caller should make sure that the OVN SB IDL txn is not NULL. > Presently it > >>> + * only syncs the nat column of port binding corresponding to the > 'op->nbsp' */ > >>> static void > >>> -sync_pb_for_op(struct ovn_port *op) > >>> +sync_pb_for_lsp(struct ovn_port *op) > >>> { > >>> + ovs_assert(op->nbsp); > >>> + > >>> if (lsp_is_router(op->nbsp)) { > >>> const char *chassis = NULL; > >>> if (op->peer && op->peer->od && op->peer->od->nbr) { > >>> @@ -4895,18 +4855,86 @@ sync_pb_for_op(struct ovn_port *op) > >>> } > >>> } > >>> > >>> +/* Syncs the SB port binding for the ovn_port 'op' of a logical > router port. > >>> + * Caller should make sure that the OVN SB IDL txn is not NULL. > Presently it > >>> + * only sets the port binding options column for the router ports */ > >>> +static void > >>> +sync_pb_for_lrp(struct ovn_port *op) > >>> +{ > >>> + ovs_assert(op->nbrp); > >>> + > >>> + struct smap new; > >>> + smap_init(&new); > >>> + > >>> + const char *chassis_name = smap_get(&op->od->nbr->options, > "chassis"); > >>> + if (is_cr_port(op)) { > >>> + smap_add(&new, "distributed-port", op->nbrp->name); > >>> + > >>> + bool always_redirect = > >>> + !op->od->has_distributed_nat && > >>> + !l3dgw_port_has_associated_vtep_lports(op->l3dgw_port); > >>> + > >>> + const char *redirect_type = smap_get(&op->nbrp->options, > >>> + "redirect-type"); > >>> + if (redirect_type) { > >>> + smap_add(&new, "redirect-type", redirect_type); > >>> + /* XXX Why can't we enable always-redirect when > redirect-type > >>> + * is bridged? */ > >> > >> Nit: indentation > >> > >>> + if (!strcmp(redirect_type, "bridged")) { > >>> + always_redirect = false; > >>> + } > >>> + } > >>> + > >>> + if (always_redirect) { > >>> + smap_add(&new, "always-redirect", "true"); > >>> + } > >>> + } else { > >>> + if (op->peer) { > >>> + smap_add(&new, "peer", op->peer->key); > >>> + if (op->nbrp->ha_chassis_group || > >>> + op->nbrp->n_gateway_chassis) { > >>> + char *redirect_name = > >>> + ovn_chassis_redirect_name(op->nbrp->name); > >>> + smap_add(&new, "chassis-redirect-port", > redirect_name); > >>> + free(redirect_name); > >>> + } > >>> + } > >>> + if (chassis_name) { > >>> + smap_add(&new, "l3gateway-chassis", chassis_name); > >>> + } > >>> + } > >>> + > >>> + const char *ipv6_pd_list = smap_get(&op->sb->options, > "ipv6_ra_pd_list"); > >>> + if (ipv6_pd_list) { > >>> + smap_add(&new, "ipv6_ra_pd_list", ipv6_pd_list); > >>> + } > >>> + > >>> + sbrec_port_binding_set_options(op->sb, &new); > >>> + smap_destroy(&new); > >>> +} > >>> + > >>> +static void ovn_update_ipv6_options(struct hmap *lr_ports); > >>> +static void ovn_update_ipv6_opt_for_op(struct ovn_port *op); > >>> + > >>> /* Sync the SB Port bindings which needs to be updated. > >>> * Presently it syncs the nat column of port bindings corresponding to > >>> * the logical switch ports. */ > >>> void > >>> -sync_pbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap *ls_ports) > >>> +sync_pbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap *ls_ports, > >>> + struct hmap *lr_ports) > >>> { > >>> ovs_assert(ovnsb_idl_txn); > >>> > >>> struct ovn_port *op; > >>> HMAP_FOR_EACH (op, key_node, ls_ports) { > >>> - sync_pb_for_op(op); > >>> + sync_pb_for_lsp(op); > >>> + } > >>> + > >>> + HMAP_FOR_EACH (op, key_node, lr_ports) { > >>> + sync_pb_for_lrp(op); > >>> } > >>> + > >>> + ovn_update_ipv6_options(lr_ports); > >>> } > >>> > >>> /* Sync the SB Port bindings for the added and updated logical switch > ports > >>> @@ -4918,12 +4946,14 @@ sync_pbs_for_northd_changed_ovn_ports( struct > tracked_ovn_ports *trk_ovn_ports) > >>> struct ovn_port *op; > >>> HMAPX_FOR_EACH (hmapx_node, &trk_ovn_ports->created) { > >>> op = hmapx_node->data; > >>> - sync_pb_for_op(op); > >>> + ovs_assert(op->nbsp); > >> > >> This is already checked inside sync_pb_for_lsp(). Let's remove it from > >> here. Then what I mentioned in patch 01/16 still applies, we don't need > >> the local 'op' variable anymore. > >> > >>> + sync_pb_for_lsp(op); > >>> } > >>> > >>> HMAPX_FOR_EACH (hmapx_node, &trk_ovn_ports->updated) { > >>> op = hmapx_node->data; > >>> - sync_pb_for_op(op); > >>> + ovs_assert(op->nbsp); > >>> + sync_pb_for_lsp(op); > >> > >> Same here. > >> > >>> } > >>> > >>> return true; > >>> @@ -5671,20 +5701,31 @@ fail: > >>> bool > >>> northd_handle_sb_port_binding_changes( > >>> const struct sbrec_port_binding_table *sbrec_port_binding_table, > >>> - struct hmap *ls_ports) > >>> + struct hmap *ls_ports, struct hmap *lr_ports) > >>> { > >>> const struct sbrec_port_binding *pb; > >>> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > >>> SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, > sbrec_port_binding_table) { > >>> - /* Return false if the 'pb' belongs to a router port. We > don't handle > >>> - * I-P for router ports yet. */ > >>> - if (is_pb_router_type(pb)) { > >>> - return false; > >>> + bool is_router_port = is_pb_router_type(pb); > >>> + struct ovn_port *op = NULL; > >>> + > >>> + if (is_router_port) { > >>> + /* A router port binding 'pb' can belong to > >>> + * - a logical switch port of type router or > >>> + * - a logical router port. > >>> + * > >>> + * So, first search in lr_ports hmap. If not found, > search in > >>> + * ls_ports hmap. > >>> + * */ > >>> + op = ovn_port_find(lr_ports, pb->logical_port); > >>> } > >>> > >>> - struct ovn_port *op = ovn_port_find(ls_ports, > pb->logical_port); > >>> - if (op && !op->lsp_can_be_inc_processed) { > >>> - return false; > >>> + if (!op) { > >>> + op = ovn_port_find(ls_ports, pb->logical_port); > >>> + > >>> + if (op) { > >>> + is_router_port = false; > >>> + } > >>> } > >>> > >>> if (sbrec_port_binding_is_new(pb)) { > >>> @@ -5693,7 +5734,8 @@ northd_handle_sb_port_binding_changes( > >>> * pointer in northd data. Fallback to recompute > otherwise. */ > >>> if (!op) { > >>> VLOG_WARN_RL(&rl, "A port-binding for %s is created > but the " > >>> - "LSP is not found.", pb->logical_port); > >>> + "%s is not found.", pb->logical_port, > >>> + is_router_port ? "LRP" : "LSP"); > >>> return false; > >>> } > >>> op->sb = pb; > >>> @@ -5703,18 +5745,29 @@ northd_handle_sb_port_binding_changes( > >>> * case. Fallback to recompute otherwise, to avoid > dangling > >>> * sb idl pointers and other unexpected behavior. */ > >>> if (op && op->sb == pb) { > >>> - VLOG_WARN_RL(&rl, "A port-binding for %s is deleted > but " > >>> - "the LSP still exists.", > pb->logical_port); > >>> + VLOG_WARN_RL(&rl, "A port-binding for %s is deleted > but the " > >>> + "LSP/LRP still exists.", > pb->logical_port); > >>> return false; > >>> } > >>> } else { > >>> - /* The PB is updated, most likely because of > binding/unbinding > >>> - * to/from a chassis, and we can ignore the change > (updating NB > >>> - * "up" will be handled in the engine node > "sync_from_sb"). > >>> + /* The PB is updated. > >>> + * For an LSP PB it is most likely because of > >>> + * binding/unbinding to/from a chassis, and we can ignore > the > >>> + * change (updating NB "up" will be handled in the engine > node > >>> + * "sync_from_sb"). > >>> + * > >>> + * For an LRP PB, it is most likely because of > >>> + * - IPv6 prefix delagation updates from ovn-controller. > >>> + * This update is handled in "sync_from_sb" node. > >>> + * - ha chassis group and this can be ignored. > >>> + * > >>> + * All other changes can be ignored. > >>> + * > >>> * Fallback to recompute for anything unexpected. */ > >>> if (!op) { > >>> VLOG_WARN_RL(&rl, "A port-binding for %s is updated > but the " > >>> - "LSP is not found.", pb->logical_port); > >>> + "%s is not found.", pb->logical_port, > >>> + is_router_port ? "LRP" : "LSP"); > >>> return false; > >>> } > >>> if (op->sb != pb) { > >>> @@ -7855,67 +7908,70 @@ static void > >>> copy_ra_to_sb(struct ovn_port *op, const char *address_mode); > >>> > >>> static void > >>> -ovn_update_ipv6_options(struct hmap *lr_ports) > >>> +ovn_update_ipv6_opt_for_op(struct ovn_port *op) > >>> { > >>> - struct ovn_port *op; > >>> - HMAP_FOR_EACH (op, key_node, lr_ports) { > >>> - ovs_assert(op->nbrp); > >>> - > >>> - if (op->nbrp->peer || !op->peer) { > >>> - continue; > >>> - } > >>> + if (op->nbrp->peer || !op->peer) { > >>> + return; > >>> + } > >>> > >>> - if (!op->lrp_networks.n_ipv6_addrs) { > >>> - continue; > >>> - } > >>> + if (!op->lrp_networks.n_ipv6_addrs) { > >>> + return; > >>> + } > >>> > >>> - struct smap options; > >>> - smap_clone(&options, &op->sb->options); > >>> + struct smap options; > >>> + smap_clone(&options, &op->sb->options); > >>> > >>> - /* enable IPv6 prefix delegation */ > >>> - bool prefix_delegation = smap_get_bool(&op->nbrp->options, > >>> + /* enable IPv6 prefix delegation */ > >>> + bool prefix_delegation = smap_get_bool(&op->nbrp->options, > >>> "prefix_delegation", > false); > >>> - if (!lrport_is_enabled(op->nbrp)) { > >>> - prefix_delegation = false; > >>> - } > >>> - if (smap_get_bool(&options, "ipv6_prefix_delegation", > >>> - false) != prefix_delegation) { > >>> - smap_add(&options, "ipv6_prefix_delegation", > >>> - prefix_delegation ? "true" : "false"); > >>> - } > >>> + if (!lrport_is_enabled(op->nbrp)) { > >>> + prefix_delegation = false; > >>> + } > >>> + if (smap_get_bool(&options, "ipv6_prefix_delegation", > >>> + false) != prefix_delegation) { > >>> + smap_add(&options, "ipv6_prefix_delegation", > >>> + prefix_delegation ? "true" : "false"); > >>> + } > >>> > >>> - bool ipv6_prefix = smap_get_bool(&op->nbrp->options, > >>> + bool ipv6_prefix = smap_get_bool(&op->nbrp->options, > >>> "prefix", false); > >> > >> Nit: this fits on a single line now. > >> > >>> - if (!lrport_is_enabled(op->nbrp)) { > >>> - ipv6_prefix = false; > >>> - } > >>> - if (smap_get_bool(&options, "ipv6_prefix", false) != > ipv6_prefix) { > >>> - smap_add(&options, "ipv6_prefix", > >>> - ipv6_prefix ? "true" : "false"); > >>> - } > >>> - sbrec_port_binding_set_options(op->sb, &options); > >>> + if (!lrport_is_enabled(op->nbrp)) { > >>> + ipv6_prefix = false; > >>> + } > >>> + if (smap_get_bool(&options, "ipv6_prefix", false) != ipv6_prefix) > { > >>> + smap_add(&options, "ipv6_prefix", ipv6_prefix ? "true" : > "false"); > >>> + } > >>> + sbrec_port_binding_set_options(op->sb, &options); > >>> > >>> - smap_destroy(&options); > >>> + smap_destroy(&options); > >>> > >>> - const char *address_mode = smap_get( > >>> - &op->nbrp->ipv6_ra_configs, "address_mode"); > >>> + const char *address_mode = smap_get(&op->nbrp->ipv6_ra_configs, > >>> + "address_mode"); > >>> > >>> - if (!address_mode) { > >>> - continue; > >>> - } > >>> - if (strcmp(address_mode, "slaac") && > >>> - strcmp(address_mode, "dhcpv6_stateful") && > >>> - strcmp(address_mode, "dhcpv6_stateless")) { > >>> - static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(1, 5); > >>> - VLOG_WARN_RL(&rl, "Invalid address mode [%s] defined", > >>> - address_mode); > >>> - continue; > >>> - } > >>> + if (!address_mode) { > >>> + return; > >>> + } > >>> + if (strcmp(address_mode, "slaac") && > >>> + strcmp(address_mode, "dhcpv6_stateful") && > >>> + strcmp(address_mode, "dhcpv6_stateless")) { > >>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > >>> + VLOG_WARN_RL(&rl, "Invalid address mode [%s] defined", > >>> + address_mode); > >>> + return; > >>> + } > >>> > >>> - if (smap_get_bool(&op->nbrp->ipv6_ra_configs, "send_periodic", > >>> - false)) { > >>> - copy_ra_to_sb(op, address_mode); > >>> - } > >>> + if (smap_get_bool(&op->nbrp->ipv6_ra_configs, "send_periodic", > false)) { > >>> + copy_ra_to_sb(op, address_mode); > >>> + } > >>> +} > >>> + > >>> +static void > >>> +ovn_update_ipv6_options(struct hmap *lr_ports) > >>> +{ > >>> + struct ovn_port *op; > >>> + HMAP_FOR_EACH (op, key_node, lr_ports) { > >>> + ovs_assert(op->nbrp); > >> > >> I would move this assert inside ovn_update_ipv6_opt_for_op() just in > >> case we ever want to call it in multiple places in the future. > >> > >>> + ovn_update_ipv6_opt_for_op(op); > >>> } > >>> } > >>> > >>> @@ -18007,8 +18063,6 @@ ovnnb_db_run(struct northd_input *input_data, > >>> &data->lr_ports); > >>> stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec()); > >>> stopwatch_start(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec()); > >>> - ovn_update_ipv6_options(&data->lr_ports); > >>> - ovn_update_ipv6_prefix(&data->lr_ports); > >>> > >>> sync_mirrors(ovnsb_txn, input_data->nbrec_mirror_table, > >>> input_data->sbrec_mirror_table); > >>> @@ -18339,6 +18393,8 @@ ovnsb_db_run(struct ovsdb_idl_txn *ovnnb_txn, > >>> &ha_ref_chassis_map); > >>> } > >>> shash_destroy(&ha_ref_chassis_map); > >>> + > >>> + ovn_update_ipv6_prefix(lr_ports); > >>> } > >>> > >>> const char * > >>> diff --git a/northd/northd.h b/northd/northd.h > >>> index 23521065e8..233dca8084 100644 > >>> --- a/northd/northd.h > >>> +++ b/northd/northd.h > >>> @@ -364,7 +364,8 @@ bool lflow_handle_northd_port_changes(struct > ovsdb_idl_txn *ovnsb_txn, > >>> struct lflow_input *, > >>> struct hmap *lflows); > >>> bool northd_handle_sb_port_binding_changes( > >>> - const struct sbrec_port_binding_table *, struct hmap *ls_ports); > >>> + const struct sbrec_port_binding_table *, struct hmap *ls_ports, > >>> + struct hmap *lr_ports); > >>> > >>> struct tracked_lb_data; > >>> bool northd_handle_lb_data_changes(struct tracked_lb_data *, > >>> @@ -394,7 +395,8 @@ void sync_lbs(struct ovsdb_idl_txn *, const struct > sbrec_load_balancer_table *, > >>> struct chassis_features *chassis_features); > >>> bool check_sb_lb_duplicates(const struct sbrec_load_balancer_table *); > >>> > >>> -void sync_pbs(struct ovsdb_idl_txn *, struct hmap *ls_ports); > >>> +void sync_pbs(struct ovsdb_idl_txn *, struct hmap *ls_ports, > >>> + struct hmap *lr_ports); > >>> bool sync_pbs_for_northd_changed_ovn_ports( struct tracked_ovn_ports > *); > >>> > >>> static inline bool > >>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > >>> index 4a02dacf60..4edad24e53 100644 > >>> --- a/tests/ovn-northd.at > >>> +++ b/tests/ovn-northd.at > >>> @@ -10073,7 +10073,7 @@ check ovn-nbctl --wait=hv lsp-add ls0 lsp0-0 > -- lsp-set-addresses lsp0-0 "unknow > >>> ovs-vsctl add-port br-int lsp0-0 -- set interface lsp0-0 > external_ids:iface-id=lsp0-0 > >>> wait_for_ports_up > >>> check ovn-nbctl --wait=hv sync > >>> -check_recompute_counter 4 5 5 5 5 5 > >>> +check_recompute_counter 3 4 3 4 3 4 > >>> > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=hv lsp-add ls0 lsp0-1 -- lsp-set-addresses > lsp0-1 "aa:aa:aa:00:00:01 192.168.0.11" > >>> @@ -10235,6 +10235,126 @@ OVN_CLEANUP([hv1]) > >>> AT_CLEANUP > >>> ]) > >>> > >>> +OVN_FOR_EACH_NORTHD_NO_HV([ > >>> +AT_SETUP([SB Port binding incremental processing]) > >>> +ovn_start > >>> + > >>> +check_recompute_counter() { > >>> + northd_recomp=$(as northd ovn-appctl -t ovn-northd > inc-engine/show-stats northd recompute) > >>> + echo "northd recompute count - $northd_recomp" > >>> + AT_CHECK([test x$northd_recomp = x$1]) > >>> + > >>> + lflow_recomp=$(as northd ovn-appctl -t ovn-northd > inc-engine/show-stats lflow recompute) > >>> + echo "lflow recompute count - $lflow_recomp" > >>> + AT_CHECK([test x$lflow_recomp = x$2]) > >>> +} > >>> + > >>> +net_add n1 > >>> +sim_add hv1 > >>> +as hv1 > >>> +ovs-vsctl add-br br-phys > >>> +ovn_attach n1 br-phys 192.168.0.11 > >>> + > >>> +check ovn-nbctl --wait=sb ls-add ls0 > >>> +check ovn-nbctl --wait=sb lr-add lr0 > >>> + > >>> +# Test normal VIF port > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-add ls0 p1 > >>> +check_recompute_counter 0 0 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-set-addresses p1 "00:00:00:00:01:01 10.0.0.3" > >>> +check_recompute_counter 0 0 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +check as northd ovn-appctl -t ovn-northd vlog/set info > >>> + > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-sbctl lsp-bind p1 hv1 > >>> +check_recompute_counter 0 0 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Test lsp of type router > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-add ls0 rp -- lsp-set-type rp router > >>> + > >>> +# northd engine recomputes twice. Both the times for handling NB > logical switch port > >>> +# changes and not because of SB port binding changes. This is > because ovn-northd > >>> +# sets the "up" to true. > >>> +check_recompute_counter 2 2 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Set some options to 'rp'. northd should only recompute once. > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-set-options rp foo=bar > >>> +check_recompute_counter 1 1 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Test lsp of type external > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-add ls0 e1 -- lsp-set-type e1 external > >>> +check_recompute_counter 2 2 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Set some options to 'e1'. northd should only recompute once. > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-set-options e1 foo=bar > >>> +check_recompute_counter 1 1 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Test lrp > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lrp-add lr0 lrp 00:00:02:01:02:03 10.0.0.1/24 > >>> +check_recompute_counter 1 1 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Set some options on 'lrp'. northd should only recompute once. > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lrp-set-options lrp route_table=rtb-1 > >>> +check_recompute_counter 1 1 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Make lrp a gateway port > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lrp-set-gateway-chassis lrp hv1 > >>> +# There will be 3 recomputes of northd engine node > >>> +# 1. missing handler for input NB_logical_router > >>> +# 2. missing handler for input SB_ha_chassis_group > >>> +# 3. missing handler for input NB_logical_router when ovn-northd > >>> +# updates the hosting-chassis option in NB_logical_router_port. > >>> +check_recompute_counter 3 3 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl clear logical_router_port lrp gateway_chassis > >>> +# There will be 2 recomputes of northd engine node > >>> +# 1. missing handler for input NB_logical_router > >>> +# 2. missing handler for input SB_ha_chassis_group > >>> +check_recompute_counter 2 2 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +# Delete some of the ports. > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-del p1 > >>> +check_recompute_counter 0 0 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lsp-del e1 > >>> +check_recompute_counter 1 1 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> +check ovn-nbctl lrp-del lrp > >>> +check_recompute_counter 1 1 > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> + > >>> +OVN_CLEANUP([hv1]) > >>> +AT_CLEANUP > >>> +]) > >>> + > >>> OVN_FOR_EACH_NORTHD_NO_HV([ > >>> AT_SETUP([ACL/Meter incremental processing - no northd recompute]) > >>> ovn_start > >>> @@ -10269,6 +10389,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) > >>> AT_CLEANUP > >>> ]) > >>> > >>> + > >>> OVN_FOR_EACH_NORTHD_NO_HV([ > >>> AT_SETUP([check fip and lb flows]) > >>> AT_KEYWORDS([fip-lb-flows]) > >>> @@ -10471,7 +10592,7 @@ ovn-nbctl lsp-set-addresses sw0-lr0 > 00:00:00:00:ff:01 > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0 > >>> check_engine_stats lb_data norecompute compute > >>> -check_engine_stats northd recompute nocompute > >>> +check_engine_stats northd recompute compute > >>> check_engine_stats lflow recompute nocompute > >>> check_engine_stats sync_to_sb_lb recompute nocompute > >>> > >>> @@ -11114,12 +11235,14 @@ ovn_attach n1 br-phys 192.168.0.11 > >>> ovn-sbctl chassis-add gw1 geneve 127.0.0.1 > >>> > >>> check ovn-nbctl ls-add sw0 > >>> -check ovn-nbctl lsp-add sw0 sw0p1 -- lsp-set-addresses sw0p1 > "00:00:20:20:12:01 10.0.0.4" > >>> +check ovn-nbctl --wait=sb lsp-add sw0 sw0p1 -- lsp-set-addresses > sw0p1 "00:00:20:20:12:01 10.0.0.4" > >>> > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-add lr0 > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Adding a logical router port should result in recompute > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> @@ -11127,16 +11250,20 @@ check ovn-nbctl lrp-add lr0 lr0-sw0 > 00:00:00:00:ff:01 10.0.0.1/24 > >>> # for northd engine there will be both recompute and compute > >>> # first it will be recompute to handle lr0-sw0 and then a compute > >>> # for the SB port binding change. > >>> -check_engine_stats northd recompute nocompute > >>> +check_engine_stats northd recompute compute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> ovn-nbctl lsp-add sw0 sw0-lr0 > >>> ovn-nbctl lsp-set-type sw0-lr0 router > >>> ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01 > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0 > >>> -check_engine_stats northd recompute nocompute > >>> +check_engine_stats northd recompute compute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> ovn-nbctl ls-add public > >>> ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24 > >>> @@ -11158,7 +11285,9 @@ ovn-nbctl --wait=hv lrp-set-gateway-chassis > lr0-public hv1 20 > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl set logical_router_port lr0-sw0 options:foo=bar > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Do checks for NATs. > >>> # Add a NAT. This should not result in recompute of both northd and > lflow > >>> @@ -11167,6 +11296,7 @@ check as northd ovn-appctl -t ovn-northd > inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.110 > 10.0.0.4 > >>> check_engine_stats northd recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Update the NAT options column > >>> @@ -11174,6 +11304,7 @@ check as northd ovn-appctl -t ovn-northd > inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb set NAT . options:foo=bar > >>> check_engine_stats northd recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Update the NAT external_ip column > >>> @@ -11181,6 +11312,7 @@ check as northd ovn-appctl -t ovn-northd > inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb set NAT . external_ip=172.168.0.120 > >>> check_engine_stats northd recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Update the NAT logical_ip column > >>> @@ -11188,6 +11320,7 @@ check as northd ovn-appctl -t ovn-northd > inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb set NAT . logical_ip=10.0.0.10 > >>> check_engine_stats northd recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Update the NAT type > >>> @@ -11195,13 +11328,15 @@ check as northd ovn-appctl -t ovn-northd > inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb set NAT . type=snat > >>> check_engine_stats northd recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Create a dnat_and_snat NAT with external_mac and logical_port > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.110 > 10.0.0.4 sw0p1 30:54:00:00:00:03 > >>> -check_engine_stats northd recompute nocompute > >>> +check_engine_stats northd recompute compute > >>> check_engine_stats lflow recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> nat2_uuid=$(ovn-nbctl --bare --columns _uuid find nat > logical_ip=10.0.0.4) > >>> @@ -11210,6 +11345,7 @@ check as northd ovn-appctl -t ovn-northd > inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb set NAT $nat2_uuid > external_mac='"30:54:00:00:00:04"' > >>> check_engine_stats northd recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Create a load balancer and add the lb vip as NAT > >>> @@ -11223,31 +11359,35 @@ check ovn-nbctl lr-lb-add lr0 lb2 > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.140 > 10.0.0.20 > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.150 > 10.0.0.41 > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-nat-del lr0 dnat_and_snat 172.168.0.150 > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-nat-del lr0 dnat_and_snat 172.168.0.140 > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> # Delete the NAT > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb clear logical_router lr0 nat > >>> -check_engine_stats northd recompute nocompute > >>> +check_engine_stats northd recompute compute > >>> check_engine_stats lflow recompute nocompute > >>> check_engine_stats sync_to_sb_pb recompute nocompute > >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> @@ -11256,12 +11396,16 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-policy-add lr0 10 "ip4.src == 10.0.0.3" > reroute 172.168.0.101,172.168.0.102 > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>> check ovn-nbctl --wait=sb lr-policy-del lr0 10 "ip4.src == 10.0.0.3" > >>> check_engine_stats northd recompute nocompute > >>> +check_engine_stats sync_to_sb_pb recompute nocompute > >>> check_engine_stats lflow recompute nocompute > >>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>> > >>> OVN_CLEANUP([hv1]) > >>> AT_CLEANUP > >> > >> The rest looks good to me but I'd prefer we figure out the test failure > >> before acking the patch. > >> > >> Regards, > >> Dumitru > >> > >> > >> _______________________________________________ > >> dev mailing list > >> d...@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > > > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
