From 6fa5a8aa17ae6cde26e3aad4fa0687623338d209 Mon Sep 17 00:00:00 2001 From: Numan Siddique Date: Fri, 10 Jan 2025 11:27:05 -0500 Subject: [PATCH] northd: I-P for logical switch creation in en_northd engine node. This patch handles the logical switch creation incrementally in the northd engine node. The dependent engine nodes - ls_stateful, lflow and few others still fall back to full recompute, which will be handled in separate patches. Reported-at: https://issues.redhat.com/browse/FDP-754 Signed-off-by: Numan Siddique Signed-off-by: 0-day Robot --- lib/ovn-util.h | 23 ++++ northd/en-lflow.c | 4 + northd/en-ls-stateful.c | 4 + northd/en-northd.c | 19 +++ northd/en-northd.h | 1 + northd/inc-proc-northd.c | 8 +- northd/lb.c | 44 +++++- northd/lb.h | 36 +++-- northd/northd.c | 282 ++++++++++++++++++++++++++++++++++----- northd/northd.h | 21 +++ tests/ovn-northd.at | 70 +++++++++- 11 files changed, 467 insertions(+), 45 deletions(-) diff --git a/lib/ovn-util.h b/lib/ovn-util.h index 899bd9d12c..94eb1c185b 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -17,6 +17,7 @@ #define OVN_UTIL_H 1 #include "ovsdb-idl.h" +#include "lib/bitmap.h" #include "lib/packets.h" #include "lib/sset.h" #include "lib/svec.h" @@ -472,6 +473,28 @@ void sorted_array_apply_diff(const struct sorted_array *a1, bool add), const void *arg); +static inline unsigned long * +ovn_bitmap_realloc(unsigned long *bitmap, size_t n_bits_old, + size_t n_bits_new) +{ + ovs_assert(n_bits_new >= n_bits_old); + + if (bitmap_n_bytes(n_bits_old) == bitmap_n_bytes(n_bits_new)) { + return bitmap; + } + + bitmap = xrealloc(bitmap, bitmap_n_bytes(n_bits_new)); + + /* Set the unitialized bits to 0 as xrealloc doesn't initialize the + * added memory. */ + size_t i; + for (i = BITMAP_N_LONGS(n_bits_old); i < BITMAP_N_LONGS(n_bits_new); i++) { + bitmap[i] = 0; + } + + return bitmap; +} + /* Utilities around properly handling exit command. */ struct ovn_exit_args { struct unixctl_conn **conns; diff --git a/northd/en-lflow.c b/northd/en-lflow.c index fa1f0236d9..32ddfb70ff 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -127,6 +127,10 @@ lflow_northd_handler(struct engine_node *node, return false; } + if (northd_has_lswitchs_in_tracked_data(&northd_data->trk_data)) { + return false; + } + const struct engine_context *eng_ctx = engine_get_context(); struct lflow_data *lflow_data = data; diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c index 11e97aa9bb..1534bad9f6 100644 --- a/northd/en-ls-stateful.c +++ b/northd/en-ls-stateful.c @@ -138,6 +138,10 @@ ls_stateful_northd_handler(struct engine_node *node, void *data_) return false; } + if (northd_has_lswitchs_in_tracked_data(&northd_data->trk_data)) { + return false; + } + if (!northd_has_ls_lbs_in_tracked_data(&northd_data->trk_data) && !northd_has_ls_acls_in_tracked_data(&northd_data->trk_data)) { return true; diff --git a/northd/en-northd.c b/northd/en-northd.c index c7d1ebcb35..41911b214a 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -154,6 +154,25 @@ northd_nb_logical_switch_handler(struct engine_node *node, return true; } +bool +northd_sb_datapath_binding_handler(struct engine_node *node, + void *data) +{ + struct northd_data *nd = data; + + struct northd_input input_data; + + northd_get_input_data(node, &input_data); + + if (!northd_handle_sb_datapath_binding_changes( + input_data.sbrec_datapath_binding_table, &nd->ls_datapaths, + &nd->lr_datapaths)) { + return false; + } + + return true; +} + bool northd_sb_port_binding_handler(struct engine_node *node, void *data) diff --git a/northd/en-northd.h b/northd/en-northd.h index b676a0ad3f..6614d2d557 100644 --- a/northd/en-northd.h +++ b/northd/en-northd.h @@ -18,6 +18,7 @@ bool northd_global_config_handler(struct engine_node *, void *data OVS_UNUSED); bool northd_nb_logical_switch_handler(struct engine_node *, void *data); bool northd_nb_logical_router_handler(struct engine_node *, void *data); bool northd_sb_port_binding_handler(struct engine_node *, void *data); +bool northd_sb_datapath_binding_handler(struct engine_node *, void *data); bool northd_lb_data_handler(struct engine_node *, void *data); bool northd_sb_fdb_change_handler(struct engine_node *node, void *data); void *en_routes_init(struct engine_node *node OVS_UNUSED, diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 6e0aa04c46..4badef4bb5 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -193,10 +193,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_northd, &en_sb_chassis, NULL); engine_add_input(&en_northd, &en_sb_mirror, NULL); engine_add_input(&en_northd, &en_sb_meter, NULL); - engine_add_input(&en_northd, &en_sb_datapath_binding, NULL); engine_add_input(&en_northd, &en_sb_dns, NULL); engine_add_input(&en_northd, &en_sb_ha_chassis_group, NULL); - engine_add_input(&en_northd, &en_sb_ip_multicast, NULL); engine_add_input(&en_northd, &en_sb_service_monitor, NULL); engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL); engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL); @@ -215,6 +213,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_northd, &en_sb_mac_binding, engine_noop_handler); + engine_add_input(&en_northd, &en_sb_datapath_binding, + northd_sb_datapath_binding_handler); engine_add_input(&en_northd, &en_sb_port_binding, northd_sb_port_binding_handler); engine_add_input(&en_northd, &en_nb_logical_switch, @@ -223,6 +223,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, northd_nb_logical_router_handler); engine_add_input(&en_northd, &en_lb_data, northd_lb_data_handler); + /* No need for an explicit handler for the SB datapath and + * SB IP Multicast changes.*/ + engine_add_input(&en_northd, &en_sb_ip_multicast, engine_noop_handler); + engine_add_input(&en_lr_nat, &en_northd, lr_nat_northd_handler); engine_add_input(&en_lr_stateful, &en_northd, lr_stateful_northd_handler); diff --git a/northd/lb.c b/northd/lb.c index af0c92954c..3a3241d73a 100644 --- a/northd/lb.c +++ b/northd/lb.c @@ -40,6 +40,11 @@ static struct nbrec_load_balancer_health_check * ovn_lb_get_health_check(const struct nbrec_load_balancer *nbrec_lb, const char *vip_port_str, bool template); +static void ovn_lb_datapaths_realloc_ls_map(struct ovn_lb_datapaths *, + size_t n_ls_datapaths); +static void ovn_lb_datapaths_realloc_lr_map(struct ovn_lb_datapaths *, + size_t n_lr_datapaths); + struct ovn_lb_ip_set * ovn_lb_ip_set_create(void) { @@ -564,6 +569,8 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths, lb_dps->lb = lb; lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths); lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths); + lb_dps->ls_map_size = n_ls_datapaths; + lb_dps->lr_map_size = n_lr_datapaths; lb_dps->lflow_ref = lflow_ref_create(); return lb_dps; @@ -580,9 +587,12 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps) void ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *lb_dps, size_t n, - struct ovn_datapath **ods) + struct ovn_datapath **ods, + size_t n_lr_datapaths) { + ovn_lb_datapaths_realloc_lr_map(lb_dps, n_lr_datapaths); for (size_t i = 0; i < n; i++) { + ovs_assert(ods[i]->index < lb_dps->lr_map_size); if (!bitmap_is_set(lb_dps->nb_lr_map, ods[i]->index)) { bitmap_set1(lb_dps->nb_lr_map, ods[i]->index); lb_dps->n_nb_lr++; @@ -592,9 +602,12 @@ ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *lb_dps, size_t n, void ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *lb_dps, size_t n, - struct ovn_datapath **ods) + struct ovn_datapath **ods, + size_t n_ls_datapaths) { + ovn_lb_datapaths_realloc_ls_map(lb_dps, n_ls_datapaths); for (size_t i = 0; i < n; i++) { + ovs_assert(ods[i]->index < lb_dps->ls_map_size); if (!bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) { bitmap_set1(lb_dps->nb_ls_map, ods[i]->index); lb_dps->n_nb_ls++; @@ -626,6 +639,8 @@ ovn_lb_group_datapaths_create(const struct ovn_lb_group *lb_group, lb_group_dps->lb_group = lb_group; lb_group_dps->ls = xmalloc(max_ls_datapaths * sizeof *lb_group_dps->ls); lb_group_dps->lr = xmalloc(max_lr_datapaths * sizeof *lb_group_dps->lr); + lb_group_dps->max_lr = max_ls_datapaths; + lb_group_dps->max_lr = max_lr_datapaths; return lb_group_dps; } @@ -652,3 +667,28 @@ ovn_lb_group_datapaths_find(const struct hmap *lb_group_dps_map, } return NULL; } + +/* Static functions. */ +static void +ovn_lb_datapaths_realloc_ls_map(struct ovn_lb_datapaths *lb_dps, + size_t n_ls_datapaths) +{ + if (n_ls_datapaths > lb_dps->ls_map_size) { + lb_dps->nb_ls_map = ovn_bitmap_realloc(lb_dps->nb_ls_map, + lb_dps->ls_map_size, + n_ls_datapaths); + lb_dps->ls_map_size = n_ls_datapaths; + } +} + +static void +ovn_lb_datapaths_realloc_lr_map(struct ovn_lb_datapaths *lb_dps, + size_t n_lr_datapaths) +{ + if (n_lr_datapaths > lb_dps->lr_map_size) { + lb_dps->nb_lr_map = ovn_bitmap_realloc(lb_dps->nb_lr_map, + lb_dps->lr_map_size, + n_lr_datapaths); + lb_dps->lr_map_size = n_lr_datapaths; + } +} diff --git a/northd/lb.h b/northd/lb.h index aa6616af41..0e76870ccf 100644 --- a/northd/lb.h +++ b/northd/lb.h @@ -133,11 +133,13 @@ struct ovn_lb_datapaths { struct hmap_node hmap_node; const struct ovn_northd_lb *lb; - size_t n_nb_ls; unsigned long *nb_ls_map; + size_t ls_map_size; + size_t n_nb_ls; - size_t n_nb_lr; unsigned long *nb_lr_map; + size_t lr_map_size; + size_t n_nb_lr; /* Reference of lflows generated for this load balancer. * @@ -175,20 +177,24 @@ struct ovn_lb_datapaths *ovn_lb_datapaths_find(const struct hmap *, void ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *); void ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *, size_t n, - struct ovn_datapath **); + struct ovn_datapath **, + size_t n_lr_datapaths); void ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *, size_t n, - struct ovn_datapath **); - + struct ovn_datapath **, + size_t n_ls_datapaths); struct ovn_lb_group_datapaths { struct hmap_node hmap_node; const struct ovn_lb_group *lb_group; /* Datapaths to which 'lb_group' is applied. */ - size_t n_ls; struct ovn_datapath **ls; - size_t n_lr; + size_t max_ls; + size_t n_ls; + struct ovn_datapath **lr; + size_t max_lr; + size_t n_lr; }; struct ovn_lb_group_datapaths *ovn_lb_group_datapaths_create( @@ -201,16 +207,28 @@ struct ovn_lb_group_datapaths *ovn_lb_group_datapaths_find( static inline void ovn_lb_group_datapaths_add_ls(struct ovn_lb_group_datapaths *lbg_dps, size_t n, - struct ovn_datapath **ods) + struct ovn_datapath **ods, + size_t n_ls_datapaths) { + if (n_ls_datapaths > lbg_dps->max_ls) { + lbg_dps->ls = xrealloc(lbg_dps->ls, + n_ls_datapaths * sizeof *lbg_dps->ls); + lbg_dps->max_ls = n_ls_datapaths; + } memcpy(&lbg_dps->ls[lbg_dps->n_ls], ods, n * sizeof *ods); lbg_dps->n_ls += n; } static inline void ovn_lb_group_datapaths_add_lr(struct ovn_lb_group_datapaths *lbg_dps, - struct ovn_datapath *lr) + struct ovn_datapath *lr, + size_t n_lr_datapaths) { + if (n_lr_datapaths > lbg_dps->max_lr) { + lbg_dps->lr = xrealloc(lbg_dps->lr, + n_lr_datapaths * sizeof *lbg_dps->lr); + lbg_dps->max_lr = n_lr_datapaths; + } lbg_dps->lr[lbg_dps->n_lr++] = lr; } diff --git a/northd/northd.c b/northd/northd.c index 07cc332a03..201771dc71 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -595,6 +595,7 @@ init_ipam_info_for_datapath(struct ovn_datapath *od) char uuid_s[UUID_LEN + 1]; sprintf(uuid_s, UUID_FMT, UUID_ARGS(&od->key)); init_ipam_info(&od->ipam_info, &od->nbs->other_config, uuid_s); + od->ipam_info_initialized = true; } static void @@ -682,22 +683,25 @@ init_mcast_info_for_datapath(struct ovn_datapath *od) } else { init_mcast_info_for_router_datapath(od); } + od->mcast_info_initialized = true; } static void destroy_mcast_info_for_switch_datapath(struct ovn_datapath *od) { - struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw; + if (od->ipam_info_initialized) { + struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw; - free(mcast_sw_info->eth_src); - free(mcast_sw_info->ipv4_src); - free(mcast_sw_info->ipv6_src); + free(mcast_sw_info->eth_src); + free(mcast_sw_info->ipv4_src); + free(mcast_sw_info->ipv6_src); + } } static void destroy_mcast_info_for_datapath(struct ovn_datapath *od) { - if (!od->nbr && !od->nbs) { + if (!od->mcast_info_initialized || (!od->nbr && !od->nbs)) { return; } @@ -1001,6 +1005,19 @@ ovn_datapath_assign_requested_tnl_id( } } +static bool +ovn_datapath_assign_or_allocate_tnl_id(struct hmap *dp_tnlids, + struct ovn_datapath *od, uint32_t *hint) +{ + ovn_datapath_assign_requested_tnl_id(dp_tnlids, od); + if (!od->tunnel_key) { + od->tunnel_key = ovn_allocate_tnlid(dp_tnlids, "datapath", + OVN_MIN_DP_KEY_LOCAL, get_ovn_max_dp_key_local(vxlan_mode), hint); + } + + return od->tunnel_key != 0; +} + static void ods_build_array_index(struct ovn_datapaths *datapaths) { @@ -1009,8 +1026,11 @@ ods_build_array_index(struct ovn_datapaths *datapaths) * doesn't matter if they are different on every iteration. */ size_t index = 0; - datapaths->array = xrealloc(datapaths->array, - ods_size(datapaths) * sizeof *datapaths->array); + datapaths->n_array = ods_size(datapaths); + datapaths->n_allocated_array = datapaths->n_array + 10; + datapaths->array = xrealloc( + datapaths->array, + datapaths->n_allocated_array * sizeof *datapaths->array); struct ovn_datapath *od; HMAP_FOR_EACH (od, key_node, &datapaths->datapaths) { @@ -1020,6 +1040,23 @@ ods_build_array_index(struct ovn_datapaths *datapaths) } } +static void +ods_assign_array_index(struct ovn_datapaths *datapaths, + struct ovn_datapath *od) +{ + ovs_assert(ods_size(datapaths) == (datapaths->n_array + 1)); + od->index = datapaths->n_array; + + if (datapaths->n_array == datapaths->n_allocated_array) { + datapaths->n_allocated_array += 10; + datapaths->array = xrealloc( + datapaths->array, + datapaths->n_allocated_array * sizeof *datapaths->array); + } + datapaths->array[datapaths->n_array++] = od; + od->datapaths = datapaths; +} + /* Updates the southbound Datapath_Binding table so that it contains the * logical switches and routers specified by the northbound database. * @@ -3748,7 +3785,7 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, &od->nbs->load_balancer[i]->header_.uuid; lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); ovs_assert(lb_dps); - ovn_lb_datapaths_add_ls(lb_dps, 1, &od); + ovn_lb_datapaths_add_ls(lb_dps, 1, &od, ods_size(ls_datapaths)); } for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) { @@ -3758,7 +3795,8 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, ovn_lb_group_datapaths_find(lb_group_datapaths_map, lb_group_uuid); ovs_assert(lb_group_dps); - ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &od); + ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &od, + ods_size(ls_datapaths)); } } @@ -3773,7 +3811,8 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, ovn_lb_group_datapaths_find(lb_group_datapaths_map, lb_group_uuid); ovs_assert(lb_group_dps); - ovn_lb_group_datapaths_add_lr(lb_group_dps, od); + ovn_lb_group_datapaths_add_lr(lb_group_dps, od, + ods_size(lr_datapaths)); } for (size_t i = 0; i < od->nbr->n_load_balancer; i++) { @@ -3781,7 +3820,7 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, &od->nbr->load_balancer[i]->header_.uuid; lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); ovs_assert(lb_dps); - ovn_lb_datapaths_add_lr(lb_dps, 1, &od); + ovn_lb_datapaths_add_lr(lb_dps, 1, &od, ods_size(lr_datapaths)); } } @@ -3792,9 +3831,11 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); ovs_assert(lb_dps); ovn_lb_datapaths_add_ls(lb_dps, lb_group_dps->n_ls, - lb_group_dps->ls); + lb_group_dps->ls, + ods_size(ls_datapaths)); ovn_lb_datapaths_add_lr(lb_dps, lb_group_dps->n_lr, - lb_group_dps->lr); + lb_group_dps->lr, + ods_size(lr_datapaths)); } } } @@ -3840,6 +3881,7 @@ build_lb_svcs( static void build_lswitch_lbs_from_lrouter(struct ovn_datapaths *lr_datapaths, + struct ovn_datapaths *ls_datapaths, struct hmap *lb_dps_map, struct hmap *lb_group_dps_map) { @@ -3853,7 +3895,8 @@ build_lswitch_lbs_from_lrouter(struct ovn_datapaths *lr_datapaths, HMAP_FOR_EACH (lb_dps, hmap_node, lb_dps_map) { BITMAP_FOR_EACH_1 (index, ods_size(lr_datapaths), lb_dps->nb_lr_map) { struct ovn_datapath *od = lr_datapaths->array[index]; - ovn_lb_datapaths_add_ls(lb_dps, od->n_ls_peers, od->ls_peers); + ovn_lb_datapaths_add_ls(lb_dps, od->n_ls_peers, od->ls_peers, + ods_size(ls_datapaths)); } } @@ -3862,13 +3905,15 @@ build_lswitch_lbs_from_lrouter(struct ovn_datapaths *lr_datapaths, for (size_t i = 0; i < lb_group_dps->n_lr; i++) { struct ovn_datapath *od = lb_group_dps->lr[i]; ovn_lb_group_datapaths_add_ls(lb_group_dps, od->n_ls_peers, - od->ls_peers); + od->ls_peers, 0); for (size_t j = 0; j < lb_group_dps->lb_group->n_lbs; j++) { const struct uuid *lb_uuid = &lb_group_dps->lb_group->lbs[j]->nlb->header_.uuid; lb_dps = ovn_lb_datapaths_find(lb_dps_map, lb_uuid); ovs_assert(lb_dps); - ovn_lb_datapaths_add_ls(lb_dps, od->n_ls_peers, od->ls_peers); + ovn_lb_datapaths_add_ls(lb_dps, od->n_ls_peers, + od->ls_peers, + ods_size(ls_datapaths)); } } } @@ -3897,7 +3942,9 @@ build_lb_port_related_data( const struct sbrec_service_monitor_table *sbrec_service_monitor_table, const char *svc_monitor_mac, const struct eth_addr *svc_monitor_mac_ea, - struct ovn_datapaths *lr_datapaths, struct hmap *ls_ports, + struct ovn_datapaths *lr_datapaths, + struct ovn_datapaths *ls_datapaths, + struct hmap *ls_ports, struct hmap *lb_dps_map, struct hmap *lb_group_dps_map, struct sset *svc_monitor_lsps, struct hmap *svc_monitor_map) @@ -3905,7 +3952,8 @@ build_lb_port_related_data( build_lb_svcs(ovnsb_txn, sbrec_service_monitor_table, svc_monitor_mac, svc_monitor_mac_ea, ls_ports, lb_dps_map, svc_monitor_lsps, svc_monitor_map); - build_lswitch_lbs_from_lrouter(lr_datapaths, lb_dps_map, lb_group_dps_map); + build_lswitch_lbs_from_lrouter(lr_datapaths, ls_datapaths, + lb_dps_map, lb_group_dps_map); } /* Syncs the SB port binding for the ovn_port 'op' of a logical switch port. @@ -4344,6 +4392,12 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, sset_destroy(&active_ha_chassis_grps); } +static void +destroy_tracked_dps(struct tracked_dps *trk_dps) +{ + hmapx_clear(&trk_dps->crupdated); +} + static void destroy_tracked_ovn_ports(struct tracked_ovn_ports *trk_ovn_ports) { @@ -4380,6 +4434,7 @@ void destroy_northd_data_tracked_changes(struct northd_data *nd) { struct northd_tracked_data *trk_changes = &nd->trk_data; + destroy_tracked_dps(&trk_changes->trk_switches); destroy_tracked_ovn_ports(&trk_changes->trk_lsps); destroy_tracked_lbs(&trk_changes->trk_lbs); hmapx_clear(&trk_changes->trk_nat_lrs); @@ -4393,6 +4448,7 @@ init_northd_tracked_data(struct northd_data *nd) { struct northd_tracked_data *trk_data = &nd->trk_data; trk_data->type = NORTHD_TRACKED_NONE; + hmapx_init(&trk_data->trk_switches.crupdated); hmapx_init(&trk_data->trk_lsps.created); hmapx_init(&trk_data->trk_lsps.updated); hmapx_init(&trk_data->trk_lsps.deleted); @@ -4408,6 +4464,7 @@ destroy_northd_tracked_data(struct northd_data *nd) { struct northd_tracked_data *trk_data = &nd->trk_data; trk_data->type = NORTHD_TRACKED_NONE; + hmapx_destroy(&trk_data->trk_switches.crupdated); hmapx_destroy(&trk_data->trk_lsps.created); hmapx_destroy(&trk_data->trk_lsps.updated); hmapx_destroy(&trk_data->trk_lsps.deleted); @@ -4822,15 +4879,80 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct northd_input *ni, struct northd_data *nd) { - const struct nbrec_logical_switch *changed_ls; struct northd_tracked_data *trk_data = &nd->trk_data; + nd->trk_data.type = NORTHD_TRACKED_NONE; - NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (changed_ls, + /* Loop to handle deleted logical switches. */ + const struct nbrec_logical_switch *deleted_ls; + NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (deleted_ls, ni->nbrec_logical_switch_table) { - if (nbrec_logical_switch_is_new(changed_ls) || - nbrec_logical_switch_is_deleted(changed_ls)) { + if (nbrec_logical_switch_is_deleted(deleted_ls)) { + return false; + } + } + + /* Loop to handle only newly created logical switches and to + * create ovn_datapath objects. */ + const struct nbrec_logical_switch *new_ls; + uint32_t hint = 0; + NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED ( + new_ls, ni->nbrec_logical_switch_table) { + if (!nbrec_logical_switch_is_new(new_ls)) { + continue; + } + + /* If a logical switch is created with the below columns set, + * then we can't handle handle this yet. Return false. */ + if (new_ls->copp || new_ls->n_dns_records + || new_ls->n_forwarding_groups || new_ls->n_qos_rules) { + return false; + } + + struct ovn_datapath *od = ovn_datapath_create( + &nd->ls_datapaths.datapaths, &new_ls->header_.uuid, new_ls, + NULL, NULL); + + ods_assign_array_index(&nd->ls_datapaths, od); + + /* Assign tunnel id if requested or allocate a new one. */ + if (!ovn_datapath_assign_or_allocate_tnl_id(&nd->dp_tnlids, + od, &hint)) { + ovn_datapath_destroy(&nd->ls_datapaths.datapaths, od); + goto fail; + } + + init_ipam_info_for_datapath(od); + init_mcast_info_for_datapath(od); + + od->sb = sbrec_datapath_binding_insert_persist_uuid(ovnsb_idl_txn, + &od->key); + ovn_datapath_update_external_ids(od); + sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key); + + /* Create SB:IP_Multicast for the logical switch. */ + const struct sbrec_ip_multicast *ip_mcast = + sbrec_ip_multicast_insert(ovnsb_idl_txn); + store_mcast_info_for_switch_datapath(ip_mcast, od); + + if (!ls_handle_lsp_changes(ovnsb_idl_txn, new_ls, + ni, nd, od, &trk_data->trk_lsps)) { goto fail; } + + if (new_ls->n_acls) { + hmapx_add(&trk_data->ls_with_changed_acls, od); + } + hmapx_add(&trk_data->trk_switches.crupdated, od); + } + + /* Loop to handle updated logical switches. */ + const struct nbrec_logical_switch *changed_ls; + NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (changed_ls, + ni->nbrec_logical_switch_table) { + if (nbrec_logical_switch_is_new(changed_ls)) { + continue; + } + struct ovn_datapath *od = ovn_datapath_find_( &nd->ls_datapaths.datapaths, &changed_ls->header_.uuid); @@ -4857,6 +4979,10 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, } } + if (!hmapx_is_empty(&trk_data->trk_switches.crupdated)) { + trk_data->type |= NORTHD_TRACKED_SWITCHES; + } + if (!hmapx_is_empty(&trk_data->trk_lsps.created) || !hmapx_is_empty(&trk_data->trk_lsps.updated) || !hmapx_is_empty(&trk_data->trk_lsps.deleted)) { @@ -5002,6 +5128,93 @@ northd_handle_lr_changes(const struct northd_input *ni, return false; } +bool +northd_handle_sb_datapath_binding_changes( + const struct sbrec_datapath_binding_table *sbrec_dp_table, + const struct ovn_datapaths *ls_datapaths, + const struct ovn_datapaths *lr_datapaths) +{ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + const struct sbrec_datapath_binding *sbrec_dp; + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (sbrec_dp, sbrec_dp_table) { + const struct ovn_datapaths *datapaths = NULL; + bool external_ids_missing = false; + struct uuid key; + bool is_switch; + + const char *name = smap_get(&sbrec_dp->external_ids, "name"); + if (!name) { + external_ids_missing = true; + } + + if (smap_get_uuid(&sbrec_dp->external_ids, "logical-switch", &key)) { + datapaths = ls_datapaths; + is_switch = true; + } else if (smap_get_uuid(&sbrec_dp->external_ids, "logical-router", + &key)) { + datapaths = lr_datapaths; + is_switch = false; + } else { + external_ids_missing = true; + } + + if (external_ids_missing) { + VLOG_WARN_RL(&rl, "Datapath "UUID_FMT" is present in the" + "Southbound DB without valid external_ids keys", + UUID_ARGS(&sbrec_dp->header_.uuid)); + return false; + } + + if (!sbrec_datapath_binding_is_deleted(sbrec_dp) && + !uuid_equals(&sbrec_dp->header_.uuid, &key)) { + VLOG_WARN_RL(&rl, "Datapath "UUID_FMT" is present in the " + "Southbound DB with mismatching " + "external-ids:logical-switch/router "UUID_FMT, + UUID_ARGS(&sbrec_dp->header_.uuid), UUID_ARGS(&key)); + return false; + } + + struct ovn_datapath *od = + ovn_datapath_find_(&datapaths->datapaths, &key); + + if (sbrec_datapath_binding_is_new(sbrec_dp)) { + if (!od) { + VLOG_WARN_RL(&rl, "A datapath-binding for %s is created but " + "the %s is not found.", name, + is_switch ? "Logical switch" : "Logical router"); + return false; + } + od->sb = sbrec_dp; + } else if (sbrec_datapath_binding_is_deleted(sbrec_dp)) { + /* Most likely the sbrec_dp was deleted by northd and this is the + * notification of that transaction, and we can ignore in this + * case. Fallback to recompute otherwise, to avoid dangling + * sb idl pointers and other unexpected behavior. */ + if (od && od->sb == sbrec_dp) { + VLOG_WARN_RL(&rl, "A datapath-binding for %s is deleted but " + "the %s still exists.", name, + is_switch ? "Logical switch" : "Logical router"); + return false; + } + } else { + /* sbrec_dp is updated.*/ + if (!od) { + VLOG_WARN_RL(&rl, "A datapath-binding for %s is updated but " + "the %s is not found.", name, + is_switch ? "Logical switch" : "Logical router"); + return false; + } + if (od->sb != sbrec_dp) { + VLOG_WARN_RL(&rl, "A datapath-binding for %s is updated with " + "a new IDL row, which is unusual.", name); + return false; + } + } + } + + return true; +} + bool northd_handle_sb_port_binding_changes( const struct sbrec_port_binding_table *sbrec_port_binding_table, @@ -5214,7 +5427,7 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, UUIDSET_FOR_EACH (uuidnode, &codlb->assoc_lbs) { lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, &uuidnode->uuid); ovs_assert(lb_dps); - ovn_lb_datapaths_add_ls(lb_dps, 1, &od); + ovn_lb_datapaths_add_ls(lb_dps, 1, &od, ods_size(ls_datapaths)); /* Add the lb to the northd tracked data. */ hmapx_add(&nd_changes->trk_lbs.crupdated, lb_dps); @@ -5224,7 +5437,8 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, lbgrp_dps = ovn_lb_group_datapaths_find(lbgrp_datapaths_map, &uuidnode->uuid); ovs_assert(lbgrp_dps); - ovn_lb_group_datapaths_add_ls(lbgrp_dps, 1, &od); + ovn_lb_group_datapaths_add_ls(lbgrp_dps, 1, &od, + ods_size(ls_datapaths)); /* Associate all the lbs of the lbgrp to the datapath 'od' */ for (size_t j = 0; j < lbgrp_dps->lb_group->n_lbs; j++) { @@ -5232,7 +5446,8 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, = &lbgrp_dps->lb_group->lbs[j]->nlb->header_.uuid; lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); ovs_assert(lb_dps); - ovn_lb_datapaths_add_ls(lb_dps, 1, &od); + ovn_lb_datapaths_add_ls(lb_dps, 1, &od, + ods_size(ls_datapaths)); /* Add the lb to the northd tracked data. */ hmapx_add(&nd_changes->trk_lbs.crupdated, lb_dps); @@ -5251,7 +5466,7 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, UUIDSET_FOR_EACH (uuidnode, &codlb->assoc_lbs) { lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, &uuidnode->uuid); ovs_assert(lb_dps); - ovn_lb_datapaths_add_lr(lb_dps, 1, &od); + ovn_lb_datapaths_add_lr(lb_dps, 1, &od, ods_size(lr_datapaths)); /* Add the lb to the northd tracked data. */ hmapx_add(&nd_changes->trk_lbs.crupdated, lb_dps); @@ -5261,7 +5476,8 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, lbgrp_dps = ovn_lb_group_datapaths_find(lbgrp_datapaths_map, &uuidnode->uuid); ovs_assert(lbgrp_dps); - ovn_lb_group_datapaths_add_lr(lbgrp_dps, od); + ovn_lb_group_datapaths_add_lr(lbgrp_dps, od, + ods_size(lr_datapaths)); /* Associate all the lbs of the lbgrp to the datapath 'od' */ for (size_t j = 0; j < lbgrp_dps->lb_group->n_lbs; j++) { @@ -5269,7 +5485,8 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, = &lbgrp_dps->lb_group->lbs[j]->nlb->header_.uuid; lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); ovs_assert(lb_dps); - ovn_lb_datapaths_add_lr(lb_dps, 1, &od); + ovn_lb_datapaths_add_lr(lb_dps, 1, &od, + ods_size(lr_datapaths)); /* Add the lb to the northd tracked data. */ hmapx_add(&nd_changes->trk_lbs.crupdated, lb_dps); @@ -5310,12 +5527,14 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, ovs_assert(lb_dps); for (size_t i = 0; i < lbgrp_dps->n_lr; i++) { od = lbgrp_dps->lr[i]; - ovn_lb_datapaths_add_lr(lb_dps, 1, &od); + ovn_lb_datapaths_add_lr(lb_dps, 1, &od, + ods_size(lr_datapaths)); } for (size_t i = 0; i < lbgrp_dps->n_ls; i++) { od = lbgrp_dps->ls[i]; - ovn_lb_datapaths_add_ls(lb_dps, 1, &od); + ovn_lb_datapaths_add_ls(lb_dps, 1, &od, + ods_size(ls_datapaths)); /* Add the ls datapath to the northd tracked data. */ hmapx_add(&nd_changes->ls_with_changed_lbs, od); @@ -19204,7 +19423,8 @@ ovnnb_db_run(struct northd_input *input_data, input_data->sbrec_service_monitor_table, input_data->svc_monitor_mac, &input_data->svc_monitor_mac_ea, - &data->lr_datapaths, &data->ls_ports, + &data->lr_datapaths, + &data->ls_datapaths, &data->ls_ports, &data->lb_datapaths_map, &data->lb_group_datapaths_map, &data->svc_monitor_lsps, diff --git a/northd/northd.h b/northd/northd.h index 6d8383ca2b..ec5771d25a 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -79,6 +79,8 @@ struct ovn_datapaths { /* The array index of each element in 'datapaths'. */ struct ovn_datapath **array; + size_t n_array; + size_t n_allocated_array; }; static inline size_t @@ -99,6 +101,11 @@ enum redirected_routing_protcol_flag_type { REDIRECT_BFD = (1 << 1), }; +struct tracked_dps { + /* Tracked created or updated datapaths. */ + struct hmapx crupdated; +}; + struct tracked_ovn_ports { /* tracked created ports. * hmapx node data is 'struct ovn_port *' */ @@ -130,6 +137,7 @@ enum northd_tracked_data_type { NORTHD_TRACKED_LR_NATS = (1 << 2), NORTHD_TRACKED_LS_LBS = (1 << 3), NORTHD_TRACKED_LS_ACLS = (1 << 4), + NORTHD_TRACKED_SWITCHES = (1 << 5), }; /* Track what's changed in the northd engine node. @@ -138,6 +146,7 @@ enum northd_tracked_data_type { struct northd_tracked_data { /* Indicates the type of data tracked. One or all of NORTHD_TRACKED_*. */ enum northd_tracked_data_type type; + struct tracked_dps trk_switches; struct tracked_ovn_ports trk_lsps; struct tracked_lbs trk_lbs; @@ -348,9 +357,11 @@ struct ovn_datapath { /* IPAM data. */ struct ipam_info ipam_info; + bool ipam_info_initialized; /* Multicast data. */ struct mcast_info mcast_info; + bool mcast_info_initialized; /* Applies to only logical router datapath. * True if logical router is a gateway router. i.e options:chassis is set. @@ -784,6 +795,10 @@ bool lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *, struct ls_stateful_tracked_data *, struct lflow_input *, struct lflow_table *lflows); +bool northd_handle_sb_datapath_binding_changes( + const struct sbrec_datapath_binding_table *, + const struct ovn_datapaths *ls_datapaths, + const struct ovn_datapaths *lr_datapaths); bool northd_handle_sb_port_binding_changes( const struct sbrec_port_binding_table *, struct hmap *ls_ports, struct hmap *lr_ports); @@ -852,6 +867,12 @@ northd_has_ls_acls_in_tracked_data(struct northd_tracked_data *trk_nd_changes) return trk_nd_changes->type & NORTHD_TRACKED_LS_ACLS; } +static inline bool +northd_has_lswitchs_in_tracked_data(struct northd_tracked_data *trk_nd_changes) +{ + return trk_nd_changes->type & NORTHD_TRACKED_SWITCHES; +} + /* Returns 'true' if the IPv4 'addr' is on the same subnet with one of the * IPs configured on the router port. */ diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 507cc302fd..e1dbd8bab9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -12209,7 +12209,7 @@ check ovn-nbctl --wait=sb lsp-add sw0 sw0p1 -- lsp-set-addresses sw0p1 "00:00:20 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb lr-add lr0 -check_engine_stats northd recompute nocompute +check_engine_stats northd recompute compute check_engine_stats lr_nat recompute nocompute check_engine_stats lr_stateful recompute nocompute check_engine_stats sync_to_sb_pb recompute nocompute @@ -14270,6 +14270,74 @@ AT_CHECK([grep "lr_in_dnat" lr1flows | ovn_strip_lflows | grep "30.0.0.1"], [0], AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Logical switch incremental processing]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.11 + +ovn-sbctl chassis-add gw1 geneve 127.0.0.1 +check ovn-nbctl --wait=sb sync + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb ls-add sw0 +check_engine_stats northd norecompute compute +check_engine_stats ls_stateful recompute nocompute +check_engine_stats lflow recompute nocompute + +# For the below engine nodes, en_northd is input. So check +# their engine status. +check_engine_stats lr_stateful norecompute compute +check_engine_stats route_policies norecompute compute +check_engine_stats routes norecompute compute +check_engine_stats bfd_sync norecompute compute +check_engine_stats sync_to_sb_lb norecompute compute +check_engine_stats sync_to_sb_pb norecompute compute + +CHECK_NO_CHANGE_AFTER_RECOMPUTE((1)) + +# Update the logical switch. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb set logical_switch sw0 other_config:foo=bar + +check_engine_stats northd recompute nocompute +check_engine_stats ls_stateful recompute nocompute +check_engine_stats lflow recompute nocompute + +# For the below engine nodes, en_northd is input. So check +# their engine status. +check_engine_stats lr_stateful recompute nocompute +check_engine_stats route_policies recompute nocompute +check_engine_stats routes recompute nocompute +check_engine_stats bfd_sync recompute nocompute +check_engine_stats sync_to_sb_lb recompute nocompute +check_engine_stats sync_to_sb_pb recompute nocompute + +# Delete the logical switch +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb ls-del sw0 +check_engine_stats northd recompute compute +check_engine_stats ls_stateful recompute nocompute +check_engine_stats lflow recompute nocompute + +# For the below engine nodes, en_northd is input. So check +# their engine status. +check_engine_stats lr_stateful recompute nocompute +check_engine_stats route_policies recompute nocompute +check_engine_stats routes recompute nocompute +check_engine_stats bfd_sync recompute nocompute +check_engine_stats sync_to_sb_lb recompute nocompute +check_engine_stats sync_to_sb_pb recompute nocompute + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) + AT_SETUP([RBAC -- Recover builtin role and permissions]) ovn_start