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.
Acked-by: Lorenzo Bianconi <[email protected]> Reported-by: Dumitru Ceara <[email protected]> Reported-at: https://issues.redhat.com/browse/FDP-757 Signed-off-by: Jacob Tanenbaum <[email protected]> ---- v4: - added ack by Lorenzo. v5: - removed the array of entries for the lr_stateful_table leaving only the hmap - changing lr_stateful_table_find_by_index() to lr_stateful_table_find_by_uuid() - combined lr_stateful_table_find_() with lr_stateful_table_find_by_uuid_() so that there is only one way to search. diff --git a/northd/en-advertised-route-sync.c b/northd/en-advertised-route-sync.c index 35def9d22..a55689e42 100644 --- a/northd/en-advertised-route-sync.c +++ b/northd/en-advertised-route-sync.c @@ -338,8 +338,8 @@ build_nat_connected_routes( /* This is a directly connected LR peer. */ if (peer_od->nbr) { const struct lr_stateful_record *peer_lr_stateful = - lr_stateful_table_find_by_index(lr_stateful_table, - peer_od->index); + lr_stateful_table_find_by_uuid(lr_stateful_table, + peer_od->key); if (!peer_lr_stateful) { continue; } @@ -360,8 +360,8 @@ build_nat_connected_routes( } const struct lr_stateful_record *peer_lr_stateful = - lr_stateful_table_find_by_index(lr_stateful_table, - rp->peer->od->index); + lr_stateful_table_find_by_uuid(lr_stateful_table, + rp->peer->od->key); if (!peer_lr_stateful) { continue; } @@ -419,8 +419,8 @@ build_lb_connected_routes(const struct ovn_datapath *od, const struct lr_stateful_record *lr_stateful_rec; /* This is directly connected LR peer. */ if (peer_od->nbr) { - lr_stateful_rec = lr_stateful_table_find_by_index( - lr_stateful_table, peer_od->index); + lr_stateful_rec = lr_stateful_table_find_by_uuid( + lr_stateful_table, peer_od->key); build_lb_route_for_port(op, op->peer, lr_stateful_rec->lb_ips, routes); continue; @@ -435,8 +435,8 @@ build_lb_connected_routes(const struct ovn_datapath *od, * function.*/ continue; } - lr_stateful_rec = lr_stateful_table_find_by_index( - lr_stateful_table, rp->peer->od->index); + lr_stateful_rec = lr_stateful_table_find_by_uuid( + lr_stateful_table, rp->peer->od->key); build_lb_route_for_port(op, rp->peer, lr_stateful_rec->lb_ips, routes); 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 2338d4ba2..94db7276a 100644 --- a/northd/en-lr-stateful.c +++ b/northd/en-lr-stateful.c @@ -49,10 +49,8 @@ VLOG_DEFINE_THIS_MODULE(en_lr_stateful); static void lr_stateful_table_init(struct lr_stateful_table *); static void lr_stateful_table_clear(struct lr_stateful_table *); static void lr_stateful_table_destroy(struct lr_stateful_table *); -static struct lr_stateful_record *lr_stateful_table_find_( - const struct lr_stateful_table *, const struct nbrec_logical_router *); -static struct lr_stateful_record *lr_stateful_table_find_by_index_( - const struct lr_stateful_table *table, size_t od_index); +static struct lr_stateful_record *lr_stateful_table_find_by_uuid_( + const struct lr_stateful_table *table, struct uuid nb_uuid); static void lr_stateful_table_build(struct lr_stateful_table *, const struct lr_nat_table *, @@ -93,6 +91,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 +101,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 +110,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; } @@ -135,8 +140,7 @@ enum engine_input_handler_result lr_stateful_northd_handler(struct engine_node *node, void *data OVS_UNUSED) { 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 +148,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 +164,7 @@ lr_stateful_northd_handler(struct engine_node *node, void *data OVS_UNUSED) * and (3) if any. * * */ + return EN_HANDLED_UNCHANGED; } @@ -186,7 +187,7 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) ovs_assert(od); struct lr_stateful_record *lr_stateful_rec = - lr_stateful_table_find_(&data->table, od->nbr); + lr_stateful_table_find_by_uuid_(&data->table, od->key); if (!lr_stateful_rec) { const struct lr_nat_record *lrnat_rec = lr_nat_table_find_by_uuid( input_data.lr_nats, od->key); @@ -257,7 +258,7 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) ovn_datapaths_find_by_index(input_data.lr_datapaths, index); struct lr_stateful_record *lr_stateful_rec = - lr_stateful_table_find_(&data->table, od->nbr); + lr_stateful_table_find_by_uuid_(&data->table, od->key); ovs_assert(lr_stateful_rec); /* Update the od->lb_ips with the deleted and inserted @@ -300,7 +301,7 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) struct ovn_datapath *od; VECTOR_FOR_EACH (&lbgrp_dps->lr, od) { struct lr_stateful_record *lr_stateful_rec = - lr_stateful_table_find_(&data->table, od->nbr); + lr_stateful_table_find_by_uuid_(&data->table, od->key); ovs_assert(lr_stateful_rec); /* Add the lb_ips of lb_dps to the lr lb data. */ build_lrouter_lb_ips(lr_stateful_rec->lb_ips, lb_dps->lb); @@ -348,13 +349,24 @@ 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_uuid_(&data->table, + lr_nat_rec->nbr_uuid); + 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 = ovn_datapaths_find_by_index(input_data.lr_datapaths, lrnat_rec->lr_index); struct lr_stateful_record *lr_stateful_rec = - lr_stateful_table_find_(&data->table, od->nbr); + lr_stateful_table_find_by_uuid_(&data->table, od->key); if (!lr_stateful_rec) { lr_stateful_rec = lr_stateful_record_create(&data->table, lrnat_rec, od, @@ -381,10 +393,10 @@ lr_stateful_lr_nat_handler(struct engine_node *node, void *data_) } const struct lr_stateful_record * -lr_stateful_table_find_by_index(const struct lr_stateful_table *table, - size_t od_index) +lr_stateful_table_find_by_uuid(const struct lr_stateful_table *table, + struct uuid nbr_uuid) { - return lr_stateful_table_find_by_index_(table, od_index); + return lr_stateful_table_find_by_uuid_(table, nbr_uuid); } /* static functions. */ @@ -410,9 +422,6 @@ lr_stateful_table_clear(struct lr_stateful_table *table) HMAP_FOR_EACH_POP (lr_stateful_rec, key_node, &table->entries) { lr_stateful_record_destroy(lr_stateful_rec); } - - free(table->array); - table->array = NULL; } static void @@ -422,8 +431,6 @@ lr_stateful_table_build(struct lr_stateful_table *table, const struct hmap *lb_datapaths_map, const struct hmap *lbgrp_datapaths_map) { - table->array = xrealloc(table->array, - ods_size(lr_datapaths) * sizeof *table->array); const struct lr_nat_record *lrnat_rec; LR_NAT_TABLE_FOR_EACH (lrnat_rec, lr_nats) { const struct ovn_datapath *od = @@ -434,28 +441,19 @@ lr_stateful_table_build(struct lr_stateful_table *table, } static struct lr_stateful_record * -lr_stateful_table_find_(const struct lr_stateful_table *table, - const struct nbrec_logical_router *nbr) +lr_stateful_table_find_by_uuid_(const struct lr_stateful_table *table, + struct uuid nbr_uuid) { struct lr_stateful_record *lr_stateful_rec; - HMAP_FOR_EACH_WITH_HASH (lr_stateful_rec, key_node, - uuid_hash(&nbr->header_.uuid), &table->entries) { - if (uuid_equals(&lr_stateful_rec->nbr_uuid, &nbr->header_.uuid)) { + uuid_hash(&nbr_uuid), &table->entries) { + if (uuid_equals(&lr_stateful_rec->nbr_uuid, &nbr_uuid)) { return lr_stateful_rec; } } return NULL; } -static struct lr_stateful_record * -lr_stateful_table_find_by_index_(const struct lr_stateful_table *table, - size_t od_index) -{ - ovs_assert(od_index <= hmap_count(&table->entries)); - return table->array[od_index]; -} - static struct lr_stateful_record * lr_stateful_record_create(struct lr_stateful_table *table, const struct lr_nat_record *lrnat_rec, @@ -535,8 +533,6 @@ lr_stateful_record_create(struct lr_stateful_table *table, hmap_insert(&table->entries, &lr_stateful_rec->key_node, uuid_hash(&lr_stateful_rec->nbr_uuid)); - table->array[od->index] = lr_stateful_rec; - return lr_stateful_rec; } diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h index 75975c935..146f768c3 100644 --- a/northd/en-lr-stateful.h +++ b/northd/en-lr-stateful.h @@ -92,9 +92,6 @@ struct lr_stateful_record { struct lr_stateful_table { struct hmap entries; - - /* The array index of each element in 'entries'. */ - struct lr_stateful_record **array; }; #define LR_STATEFUL_TABLE_FOR_EACH(LR_LB_NAT_REC, TABLE) \ @@ -107,6 +104,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. */ @@ -139,13 +138,15 @@ lr_stateful_lr_nat_handler(struct engine_node *, void *data); enum engine_input_handler_result lr_stateful_lb_data_handler(struct engine_node *, void *data); -const struct lr_stateful_record *lr_stateful_table_find_by_index( - const struct lr_stateful_table *, size_t od_index); +const struct lr_stateful_record *lr_stateful_table_find_by_uuid( + const struct lr_stateful_table *, struct uuid nbr_uuid); 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/northd/northd.c b/northd/northd.c index eaecb424b..79a25a37b 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3568,8 +3568,8 @@ sync_pb_for_lsp(struct ovn_port *op, const struct lr_stateful_record *lr_stateful_rec = NULL; if (include_lb_vips) { - lr_stateful_rec = lr_stateful_table_find_by_index( - lr_stateful_table, op->peer->od->index); + lr_stateful_rec = lr_stateful_table_find_by_uuid( + lr_stateful_table, op->peer->od->key); } nats = get_nat_addresses(op->peer, &n_nats, false, include_lb_vips, lr_stateful_rec); @@ -3669,7 +3669,7 @@ sync_pb_for_lrp(struct ovn_port *op, const char *chassis_name = smap_get(&op->od->nbr->options, "chassis"); if (is_cr_port(op)) { const struct lr_stateful_record *lr_stateful_rec = - lr_stateful_table_find_by_index(lr_stateful_table, op->od->index); + lr_stateful_table_find_by_uuid(lr_stateful_table, op->od->key); ovs_assert(lr_stateful_rec); smap_add(&new, "distributed-port", op->primary_port->key); @@ -12422,7 +12422,7 @@ build_lrouter_nat_flows_for_lb( enum lrouter_nat_lb_flow_type type; const struct lr_stateful_record *lr_stateful_rec = - lr_stateful_table_find_by_index(lr_stateful_table, od->index); + lr_stateful_table_find_by_uuid(lr_stateful_table, od->key); ovs_assert(lr_stateful_rec); const struct lr_nat_record *lrnat_rec = lr_stateful_rec->lrnat_rec; @@ -17573,8 +17573,8 @@ build_lbnat_lflows_iterate_by_lsp( } const struct lr_stateful_record *lr_stateful_rec; - lr_stateful_rec = lr_stateful_table_find_by_index(lr_stateful_table, - op->peer->od->index); + lr_stateful_rec = lr_stateful_table_find_by_uuid(lr_stateful_table, + op->peer->od->key); ovs_assert(lr_stateful_rec); build_lsp_lflows_for_lbnats(op, lr_stateful_rec, @@ -17636,8 +17636,8 @@ build_lbnat_lflows_iterate_by_lrp( ovs_assert(op->nbrp); const struct lr_stateful_record *lr_stateful_rec; - lr_stateful_rec = lr_stateful_table_find_by_index(lr_stateful_table, - op->od->index); + lr_stateful_rec = lr_stateful_table_find_by_uuid(lr_stateful_table, + op->od->key); ovs_assert(lr_stateful_rec); build_lrp_lflows_for_lbnats(op, lr_stateful_rec, meter_groups, match, 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
