On 8/4/25 10:22 PM, Jacob Tanenbaum wrote:
> en_northd engine nodes provides the created or updated logical routers
> in its tracked data and en_lr_stateful node handles those changes.
>
> This is built on Mark Michelson's patch series located here
>
> https://patchwork.ozlabs.org/project/ovn/list/?series=465828
>
> this is required because the datapath refactor was required in order to
> accomplish incremental processing of the logical routers
>
> also built with the first two patches of this series by Lorenzo Bianconi
> located https://patchwork.ozlabs.org/project/ovn/list/?series=466579.
> He brought in some changes required to dynamically allocate the sizes of
> the datapaths array.
>
> Reported-by: Dumitru Ceara <dce...@redhat.com>
> Reported-at: https://issues.redhat.com/browse/FDP-757
> Signed-off-by: Jacob Tanenbaum <jtane...@redhat.com>
>
Hi Jacob,
Thanks for the patch! I know you're working to rebase it but in the
meantime I had a peek at this version of the series too.
> diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
> index 3b1dec0b1..872f29106 100644
> --- a/northd/en-lr-stateful.c
> +++ b/northd/en-lr-stateful.c
> @@ -132,14 +132,23 @@ 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)) {
> return EN_UNHANDLED;
> }
> if (northd_has_lr_new_in_tracked_data(&northd_data->trk_data)) {
> - return EN_UNHANDLED;
> + struct lr_stateful_input input_data =
> lr_stateful_get_input_data(node);
> + struct ed_type_lr_stateful *data = data_;
> +
> + struct lr_stateful_table *table = &data->table;
> + const struct ovn_datapaths *lr_datapaths = input_data.lr_datapaths;
> +
> + table->array = xrealloc(table->array,
> + ods_size(lr_datapaths) * sizeof *table->array);
> +
This just leaves the last part of the table->array unitialized. I'm not
sure I understand how this is supposed to work. I'd expect calls to
lr_stateful_record_create() for each of the new routers.
> + return EN_HANDLED_UPDATED;
> }
>
> /* This node uses the below data from the en_northd engine node.
> @@ -147,7 +156,7 @@ lr_stateful_northd_handler(struct engine_node *node, void
> *data OVS_UNUSED)
> * 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.
> + * a router is deleted and so does this node.
> * Note: When we add I-P to the created/deleted logical routers, we
> * need to revisit this handler.
> *
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index ff00bedf1..18ac13a92 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -12598,7 +12598,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
Regards,
Dumitru
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
