On Thu, Oct 26, 2023 at 11:14 AM <[email protected]> wrote:
>
> From: Numan Siddique <[email protected]>
>
> northd engine tracked data now also stores the logical switches
> and logical routers that got updated due to the changed load balancers.
>
> Eg 1. For this command 'ovn-nbctl ls-lb-add sw0 lb1 -- lr-lb-add lr0
> lb1', northd engine tracking data will store 'sw0' and 'lr0'.
>
> Eg 2. If load balancer lb1 is already associated with 'sw0' and 'lr0'
> then for this command 'ovn-nbctl set load_balancer <lb1>
> vips:10.0.0.10=20.0.0.20', northd engine tracking data will store
> 'sw0' and 'lr0'.
>
> An upcoming commit will make use of this tracked data.
>
> Signed-off-by: Numan Siddique <[email protected]>
> ---
> northd/northd.c | 34 +++++++++++++++++++++++++++++++++-
> northd/northd.h | 12 ++++++++++++
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index df22a9c658..9ce1b2cb5a 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -5146,6 +5146,8 @@ destroy_northd_data_tracked_changes(struct
northd_data *nd)
> struct northd_tracked_data *trk_changes = &nd->trk_northd_changes;
> destroy_tracked_ovn_ports(&trk_changes->trk_ovn_ports);
> destroy_tracked_lbs(&trk_changes->trk_lbs);
> + hmapx_clear(&trk_changes->ls_with_changed_lbs.crupdated);
> + hmapx_clear(&trk_changes->lr_with_changed_lbs.crupdated);
> nd->change_tracked = false;
> }
>
> @@ -5158,6 +5160,8 @@ init_northd_tracked_data(struct northd_data *nd)
> hmapx_init(&trk_changes->trk_ovn_ports.deleted);
> hmapx_init(&trk_changes->trk_lbs.crupdated);
> hmapx_init(&trk_changes->trk_lbs.deleted);
> + hmapx_init(&trk_changes->ls_with_changed_lbs.crupdated);
> + hmapx_init(&trk_changes->lr_with_changed_lbs.crupdated);
> }
>
> static void
> @@ -5169,6 +5173,8 @@ destroy_northd_tracked_data(struct northd_data *nd)
> hmapx_destroy(&trk_changes->trk_ovn_ports.deleted);
> hmapx_destroy(&trk_changes->trk_lbs.crupdated);
> hmapx_destroy(&trk_changes->trk_lbs.deleted);
> + hmapx_destroy(&trk_changes->ls_with_changed_lbs.crupdated);
> + hmapx_destroy(&trk_changes->lr_with_changed_lbs.crupdated);
> }
>
>
> @@ -5179,7 +5185,10 @@ northd_has_tracked_data(struct northd_tracked_data
*trk_nd_changes)
> || !hmapx_is_empty(&trk_nd_changes->trk_ovn_ports.updated)
> || !hmapx_is_empty(&trk_nd_changes->trk_ovn_ports.deleted)
> || !hmapx_is_empty(&trk_nd_changes->trk_lbs.crupdated)
> - || !hmapx_is_empty(&trk_nd_changes->trk_lbs.deleted));
> + || !hmapx_is_empty(&trk_nd_changes->trk_lbs.deleted)
> + ||
!hmapx_is_empty(&trk_nd_changes->ls_with_changed_lbs.crupdated)
> + ||
!hmapx_is_empty(&trk_nd_changes->lr_with_changed_lbs.crupdated)
> + );
> }
>
> bool
> @@ -5188,6 +5197,8 @@ northd_has_only_ports_in_tracked_data(
> {
> return (hmapx_is_empty(&trk_nd_changes->trk_lbs.crupdated)
> && hmapx_is_empty(&trk_nd_changes->trk_lbs.deleted)
> + &&
hmapx_is_empty(&trk_nd_changes->ls_with_changed_lbs.crupdated)
> + &&
hmapx_is_empty(&trk_nd_changes->lr_with_changed_lbs.crupdated)
> && (!hmapx_is_empty(&trk_nd_changes->trk_ovn_ports.created)
> || !hmapx_is_empty(&trk_nd_changes->trk_ovn_ports.updated)
> || !hmapx_is_empty(&trk_nd_changes->trk_ovn_ports.deleted)));
> @@ -5828,6 +5839,9 @@ northd_handle_lb_data_changes(struct
tracked_lb_data *trk_lb_data,
> lb_dps->nb_ls_map) {
> od = ls_datapaths->array[index];
> init_lb_for_datapath(od);
> +
> + /* Add the ls datapath to the northd tracked data. */
> + hmapx_add(&nd_changes->ls_with_changed_lbs.crupdated, od);
> }
>
> hmap_remove(lb_datapaths_map, &lb_dps->hmap_node);
> @@ -5909,6 +5923,9 @@ northd_handle_lb_data_changes(struct
tracked_lb_data *trk_lb_data,
>
> /* Re-evaluate 'od->has_lb_vip' */
> init_lb_for_datapath(od);
> +
> + /* Add the ls datapath to the northd tracked data. */
> + hmapx_add(&nd_changes->ls_with_changed_lbs.crupdated, od);
> }
>
> LIST_FOR_EACH (codlb, list_node, &trk_lb_data->crupdated_lr_lbs) {
> @@ -5954,6 +5971,9 @@ northd_handle_lb_data_changes(struct
tracked_lb_data *trk_lb_data,
>
> /* Re-evaluate 'od->has_lb_vip' */
> init_lb_for_datapath(od);
> +
> + /* Add the lr datapath to the northd tracked data. */
> + hmapx_add(&nd_changes->lr_with_changed_lbs.crupdated, od);
> }
>
> HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) {
> @@ -5968,6 +5988,9 @@ northd_handle_lb_data_changes(struct
tracked_lb_data *trk_lb_data,
> od = ls_datapaths->array[index];
> /* Re-evaluate 'od->has_lb_vip' */
> init_lb_for_datapath(od);
> +
> + /* Add the ls datapath to the northd tracked data. */
> + hmapx_add(&nd_changes->ls_with_changed_lbs.crupdated, od);
> }
>
> BITMAP_FOR_EACH_1 (index, ods_size(lr_datapaths),
> @@ -5991,6 +6014,9 @@ northd_handle_lb_data_changes(struct
tracked_lb_data *trk_lb_data,
> add_neigh_ips_to_lrouter(od, lb->neigh_mode,
> &clb->inserted_vips_v4,
> &clb->inserted_vips_v6);
> +
> + /* Add the lr datapath to the northd tracked data. */
> + hmapx_add(&nd_changes->lr_with_changed_lbs.crupdated, od);
> }
> }
>
> @@ -6018,6 +6044,9 @@ northd_handle_lb_data_changes(struct
tracked_lb_data *trk_lb_data,
>
> /* Add the lb_ips of lb_dps to the od. */
> build_lrouter_lb_ips(od->lb_ips, lb_dps->lb);
> +
> + /* Add the lr datapath to the northd tracked data. */
> + hmapx_add(&nd_changes->lr_with_changed_lbs.crupdated,
od);
> }
>
> for (size_t i = 0; i < lbgrp_dps->n_ls; i++) {
> @@ -6026,6 +6055,9 @@ northd_handle_lb_data_changes(struct
tracked_lb_data *trk_lb_data,
>
> /* Re-evaluate 'od->has_lb_vip' */
> init_lb_for_datapath(od);
> +
> + /* Add the ls datapath to the northd tracked data. */
> + hmapx_add(&nd_changes->ls_with_changed_lbs.crupdated,
od);
> }
>
> /* Add the lb to the northd tracked data. */
> diff --git a/northd/northd.h b/northd/northd.h
> index 2e4f125902..0478826f0a 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -107,12 +107,24 @@ struct tracked_lbs {
> struct hmapx deleted;
> };
>
> +/* Tracked logical switches whose load balancers have changed. */
> +struct tracked_lswitches_with_changed_lbs {
> + struct hmapx crupdated;
> +};
> +
> +/* Tracked logical routers whose load balancers have changed. */
> +struct tracked_lrouters_with_changed_lbs {
> + struct hmapx crupdated;
> +};
> +
> /* Track what's changed in the northd engine node.
> * Now only tracks ovn_ports (of vif type) - created, updated
> * and deleted. */
> struct northd_tracked_data {
> struct tracked_ovn_ports trk_ovn_ports;
> struct tracked_lbs trk_lbs;
> + struct tracked_lswitches_with_changed_lbs ls_with_changed_lbs;
> + struct tracked_lrouters_with_changed_lbs lr_with_changed_lbs;
> };
The data structure looks a little strange. struct
tracked_lswitches_with_changed_lbs and tracked_lrouters_with_changed_lbs
are the same and both have only a single member. IMHO it would be
unnecessary to define such structures. The vars ls_with_changed_lbs and
lr_with_changed_lbs can be defined as struct hmapx, and the var names are
sufficient to tell their different use. In addition, the name "crupdated"
is also a little misleading. It seems to indicate the tracked objects are
either created or updated but never deleted, but it is not the case. But
anyway, I think it is better not defining the struct at all.
Thanks,
Han
>
> struct northd_data {
> --
> 2.41.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
