On Sep 17, Jacob Tanenbaum via dev wrote:
> When a router is created or destroyed add the ability to en-lr-stateful
> engine node to compute the new state of the lr_stateful table instead of
> recalculating for the whole database.
> 
> Reported-by: Dumitru Ceara <[email protected]>
> Reported-at: https://issues.redhat.com/browse/FDP-757
> Signed-off-by: Jacob Tanenbaum <[email protected]>

Acked-by: Lorenzo Bianconi <[email protected]>

> 
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 50570b611..95c88ecea 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -146,6 +146,10 @@ lflow_northd_handler(struct engine_node *node,
>          return EN_UNHANDLED;
>      }
>  
> +    if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
> +        return EN_UNHANDLED;
> +    }
> +
>      const struct engine_context *eng_ctx = engine_get_context();
>      struct lflow_data *lflow_data = data;
>  
> diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
> index 2b6c0d0ff..cfb8fd1a1 100644
> --- a/northd/en-lr-stateful.c
> +++ b/northd/en-lr-stateful.c
> @@ -93,6 +93,7 @@ en_lr_stateful_init(struct engine_node *node OVS_UNUSED,
>      struct ed_type_lr_stateful *data = xzalloc(sizeof *data);
>      lr_stateful_table_init(&data->table);
>      hmapx_init(&data->trk_data.crupdated);
> +    hmapx_init(&data->trk_data.deleted);
>      return data;
>  }
>  
> @@ -102,6 +103,7 @@ en_lr_stateful_cleanup(void *data_)
>      struct ed_type_lr_stateful *data = data_;
>      lr_stateful_table_destroy(&data->table);
>      hmapx_destroy(&data->trk_data.crupdated);
> +    hmapx_destroy(&data->trk_data.deleted);
>  }
>  
>  void
> @@ -110,6 +112,11 @@ en_lr_stateful_clear_tracked_data(void *data_)
>      struct ed_type_lr_stateful *data = data_;
>  
>      hmapx_clear(&data->trk_data.crupdated);
> +    struct hmapx_node *hmapx_node;
> +    HMAPX_FOR_EACH_SAFE (hmapx_node, &data->trk_data.deleted) {
> +        lr_stateful_record_destroy(hmapx_node->data);
> +        hmapx_delete(&data->trk_data.deleted, hmapx_node);
> +    }
>      data->trk_data.vip_nats_changed = false;
>  }
>  
> @@ -132,11 +139,10 @@ en_lr_stateful_run(struct engine_node *node, void 
> *data_)
>  }
>  
>  enum engine_input_handler_result
> -lr_stateful_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
> +lr_stateful_northd_handler(struct engine_node *node, void *data_)
>  {
>      struct northd_data *northd_data = engine_get_input_data("northd", node);
> -    if (!northd_has_tracked_data(&northd_data->trk_data) ||
> -        northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
> +    if (!northd_has_tracked_data(&northd_data->trk_data)) {
>          return EN_UNHANDLED;
>      }
>  
> @@ -144,10 +150,6 @@ lr_stateful_northd_handler(struct engine_node *node, 
> void *data OVS_UNUSED)
>       * See (lr_stateful_get_input_data())
>       *   1. northd_data->lr_datapaths
>       *      This data gets updated when a logical router is created or 
> deleted.
> -     *      northd engine node presently falls back to full recompute when
> -     *      this happens and so does this node.
> -     *      Note: When we add I-P to the created/deleted logical routers, we
> -     *      need to revisit this handler.
>       *
>       *      This node also accesses the router ports of the logical router
>       *      (od->ports).  When these logical router ports gets updated,
> @@ -164,6 +166,19 @@ lr_stateful_northd_handler(struct engine_node *node, 
> void *data OVS_UNUSED)
>       * and (3) if any.
>       *
>       * */
> +    if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
> +        struct ed_type_lr_stateful *data = data_;
> +        struct lr_stateful_table  *table = &data->table;
> +
> +        if (hmap_count(&northd_data->lr_datapaths.datapaths) >
> +            hmap_count(&table->entries)) {
> +            table->array = xrealloc(table->array,
> +                                    ods_size(&northd_data->lr_datapaths) *
> +                                    sizeof *table->array);
> +            return EN_HANDLED_UPDATED;
> +        }
> +    }
> +
>      return EN_HANDLED_UNCHANGED;
>  }
>  
> @@ -348,6 +363,17 @@ lr_stateful_lr_nat_handler(struct engine_node *node, 
> void *data_)
>      struct ed_type_lr_stateful *data = data_;
>      struct hmapx_node *hmapx_node;
>  
> +    HMAPX_FOR_EACH (hmapx_node, &lr_nat_data->trk_data.deleted) {
> +        struct lr_nat_record *lr_nat_rec = hmapx_node->data;
> +        struct lr_stateful_record *lr_stateful_rec =
> +            lr_stateful_table_find_by_index_(&data->table,
> +                                             lr_nat_rec->lr_index);
> +        if (lr_stateful_rec) {
> +            hmap_remove(&data->table.entries, &lr_stateful_rec->key_node);
> +            hmapx_add(&data->trk_data.deleted, lr_stateful_rec);
> +        }
> +    }
> +
>      HMAPX_FOR_EACH (hmapx_node, &lr_nat_data->trk_data.crupdated) {
>          const struct lr_nat_record *lrnat_rec = hmapx_node->data;
>          const struct ovn_datapath *od =
> diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h
> index 75975c935..3c953d569 100644
> --- a/northd/en-lr-stateful.h
> +++ b/northd/en-lr-stateful.h
> @@ -107,6 +107,8 @@ struct lr_stateful_table {
>  struct lr_stateful_tracked_data {
>      /* Created or updated logical router with LB and/or NAT data. */
>      struct hmapx crupdated; /* Stores 'struct lr_stateful_record'. */
> +    /* Deleted logical router with LB and/or NAT data. */
> +    struct hmapx deleted;
>  
>      /* Indicates if any router's NATs changed which were also LB vips
>       * or vice versa. */
> @@ -145,7 +147,9 @@ const struct lr_stateful_record 
> *lr_stateful_table_find_by_index(
>  static inline bool
>  lr_stateful_has_tracked_data(struct lr_stateful_tracked_data *trk_data)
>  {
> -    return !hmapx_is_empty(&trk_data->crupdated) || 
> trk_data->vip_nats_changed;
> +    return !hmapx_is_empty(&trk_data->crupdated) ||
> +           !hmapx_is_empty(&trk_data->deleted) ||
> +           trk_data->vip_nats_changed;
>  }
>  
>  static inline bool
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 3c358c232..4c8a6aca2 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -12522,7 +12522,7 @@ check as northd ovn-appctl -t ovn-northd 
> inc-engine/clear-stats
>  check ovn-nbctl --wait=sb lr-add lr0
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_nat norecompute compute
> -check_engine_stats lr_stateful recompute nocompute
> +check_engine_stats lr_stateful norecompute compute
>  check_engine_stats sync_to_sb_pb recompute nocompute
>  check_engine_stats sync_to_sb_lb norecompute compute
>  check_engine_stats lflow recompute nocompute
> @@ -12533,7 +12533,7 @@ check ovn-nbctl --wait=sb lr-del lr0
>  
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_nat norecompute compute
> -check_engine_stats lr_stateful recompute nocompute
> +check_engine_stats lr_stateful norecompute compute
>  check_engine_stats sync_to_sb_pb recompute nocompute
>  check_engine_stats sync_to_sb_lb norecompute compute
>  check_engine_stats lflow recompute nocompute
> -- 
> 2.51.0
> 
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to