On Thu, Nov 23, 2023 at 6:24 AM Dumitru Ceara <[email protected]> wrote:
> On 11/15/23 06:30, Han Zhou wrote: > > 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. > > > > I agree. Moreoever, I think I'd squash this patch together with the one > that actually uses the tracked data. > Done. Addressed in v3. Thanks Numan > Regards, > Dumitru > > > 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 > > _______________________________________________ > 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
