On 10/26/23 20:14, [email protected] wrote:
> From: Numan Siddique <[email protected]>
> 
> It also moves the logical router port IPv6 prefix delegation
> updates to "sync-from-sb" engine node.
> 
> Signed-off-by: Numan Siddique <[email protected]>
> ---

I think I agree in general with the patch; I'd like to see the new
revision however because the check that Han also commented on [0] in
northd_handle_sb_port_binding_changes() makes me wonder too if we're not
missing cases (or if we should just relax the LSP check as well).

[0] https://mail.openvswitch.org/pipermail/ovs-dev/2023-November/409451.html

Aside from that I left a few minor comments below.

Thanks,
Dumitru

>  northd/en-northd.c  |   2 +-
>  northd/en-sync-sb.c |   3 +-
>  northd/northd.c     | 283 ++++++++++++++++++++++++++------------------
>  northd/northd.h     |   6 +-
>  tests/ovn-northd.at |  31 ++++-
>  5 files changed, 198 insertions(+), 127 deletions(-)
> 
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 96c2ce9f69..13e731cad9 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 2540fcfb97..a14c609acd 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 9ce1b2cb5a..c9c7045755 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -3419,6 +3419,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");
> @@ -3430,15 +3433,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) {
> @@ -3480,49 +3479,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);
>  
> @@ -4752,12 +4710,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) {
> @@ -4879,18 +4839,87 @@ 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");

Nit: this now fits on a single line.

> +    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
> @@ -4902,12 +4931,22 @@ 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);
> +        if (op->nbsp) {
> +            sync_pb_for_lsp(op);
> +        } else {
> +            sync_pb_for_lrp(op);
> +            ovn_update_ipv6_opt_for_op(op);
> +        }
>      }
>  
>      HMAPX_FOR_EACH (hmapx_node, &trk_ovn_ports->updated) {
>          op = hmapx_node->data;
> -        sync_pb_for_op(op);
> +        if (op->nbsp) {
> +            sync_pb_for_lsp(op);
> +        } else {
> +            sync_pb_for_lrp(op);
> +            ovn_update_ipv6_opt_for_op(op);
> +        }
>      }
>  
>      return true;
> @@ -5703,20 +5742,21 @@ 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;
>  
> -        struct ovn_port *op = ovn_port_find(ls_ports, pb->logical_port);
> -        if (op && !op->lsp_can_be_inc_processed) {
> -            return false;
> +        if (is_router_port) {
> +            op = ovn_port_find(lr_ports, pb->logical_port);
> +        } else {
> +            op = ovn_port_find(ls_ports, pb->logical_port);
> +            if (op && !op->lsp_can_be_inc_processed) {
> +                return false;
> +            }
>          }
>  
>          if (sbrec_port_binding_is_new(pb)) {
> @@ -5725,7 +5765,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;
> @@ -5736,7 +5777,7 @@ northd_handle_sb_port_binding_changes(
>               * sb idl pointers and other unexpected behavior. */
>              if (op) {
>                  VLOG_WARN_RL(&rl, "A port-binding for %s is deleted but the "
> -                            "LSP still exists.", pb->logical_port);
> +                            "LSP/LRP still exists.", pb->logical_port);
>                  return false;
>              }
>          } else {
> @@ -5746,7 +5787,8 @@ northd_handle_sb_port_binding_changes(
>               * 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) {
> @@ -7895,67 +7937,72 @@ 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,
> -                                           "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");
> -        }
> +    /* enable IPv6 prefix delegation */
> +    bool prefix_delegation = smap_get_bool(&op->nbrp->options,
> +                                        "prefix_delegation", false);

Nit: please indent and align under the first function argument.

> +    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");

Nit: indent

> +    }
> +    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");

Nit: This can now be written as:

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);

Nit: indent

> +        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)) {

Nit: It all fits on one line now.

> +        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);
>      }
>  }
>  
> @@ -18027,8 +18074,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);
> @@ -18359,6 +18404,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 0478826f0a..3945e84bb8 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -368,7 +368,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 *,
> @@ -398,7 +399,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 *);
>  
>  bool northd_has_tracked_data(struct northd_tracked_data *);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 699f6cfdce..55a244c8c4 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -11018,7 +11018,9 @@ check ovn-nbctl lsp-add sw0 sw0p1 -- 
> lsp-set-addresses sw0p1 "00:00:20:20:12:01
>  check as northd ovn-appctl -t NORTHD_TYPE 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 NORTHD_TYPE inc-engine/clear-stats
> @@ -11026,8 +11028,10 @@ 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
> @@ -11035,7 +11039,9 @@ ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
>  check as northd ovn-appctl -t NORTHD_TYPE 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 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
> @@ -11057,9 +11063,9 @@ ovn-nbctl --wait=hv lrp-set-gateway-chassis 
> lr0-public hv1 20
>  check as northd ovn-appctl -t NORTHD_TYPE 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 as northd ovn-appctl -t NORTHD_TYPE vlog/set dbg
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  
>  # Do checks for NATs.
>  # Add a NAT. This should not result in recompute of both northd and lflow
> @@ -11068,6 +11074,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 
> 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
> @@ -11075,6 +11082,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 
> 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
> @@ -11082,6 +11090,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 
> 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
> @@ -11089,6 +11098,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 
> 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
> @@ -11096,13 +11106,15 @@ check as northd ovn-appctl -t NORTHD_TYPE 
> 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 NORTHD_TYPE 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)
> @@ -11111,6 +11123,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 
> 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
> @@ -11124,31 +11137,35 @@ check ovn-nbctl lr-lb-add lr0 lb2
>  check as northd ovn-appctl -t NORTHD_TYPE 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 NORTHD_TYPE 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 NORTHD_TYPE 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 NORTHD_TYPE 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 NORTHD_TYPE 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
> @@ -11157,12 +11174,16 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE 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 NORTHD_TYPE 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

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to