On Thu, Jan 11, 2024 at 7:29 AM <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>
> ---
> 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? */
> + 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);
> + 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);
> }
>
> 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);
> - 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);
> + 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
> --
> 2.43.0
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Acked-by: Han Zhou <hz...@ovn.org>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
