When a router is created or destroyed add the ability to en-lr-nat engine node to compute the new state of the lr_nat_table instead of recalculating the lr_nat_table for the whole northbound datapabase.
I needed to change lr_nat_table_find_by_index_() function to lr_nat_table_find_by_uuid_() because the ovn_datapath indexes are no longer guaranteed sequential starting from zero there needed to be a way to determine if the entry lr_nat_table->array[index] is initialized or not. This can be determined by using the hmap of ovn_datapaths. 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: removed minor formatting issues. v5: - removed the array of entries from lr_nat_table leaving only the hmap. - changing lr_nat_table_find_by_index to lr_nat_table_find_by_uuid. - Acked by Lorenzo diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c index 0583e34a5..5a1f47697 100644 --- a/northd/en-lr-nat.c +++ b/northd/en-lr-nat.c @@ -45,8 +45,8 @@ static void lr_nat_table_destroy(struct lr_nat_table *); static void lr_nat_table_build(struct lr_nat_table *, const struct ovn_datapaths *lr_datapaths, const struct hmap *lr_ports); -static struct lr_nat_record *lr_nat_table_find_by_index_( - const struct lr_nat_table *, size_t od_index); +static struct lr_nat_record *lr_nat_table_find_by_uuid_( + const struct lr_nat_table *, struct uuid nb_uuid); static struct lr_nat_record *lr_nat_record_create(struct lr_nat_table *, const struct ovn_datapath *, @@ -68,10 +68,10 @@ static void snat_ip_add(struct lr_nat_record *, const char *ip, struct ovn_nat *); const struct lr_nat_record * -lr_nat_table_find_by_index(const struct lr_nat_table *table, - size_t od_index) +lr_nat_table_find_by_uuid(const struct lr_nat_table *table, + struct uuid nb_uuid) { - return lr_nat_table_find_by_index_(table, od_index); + return lr_nat_table_find_by_uuid_(table, nb_uuid); } /* 'lr_nat' engine node manages the NB logical router NAT data. @@ -83,6 +83,7 @@ en_lr_nat_init(struct engine_node *node OVS_UNUSED, struct ed_type_lr_nat_data *data = xzalloc(sizeof *data); lr_nat_table_init(&data->lr_nats); hmapx_init(&data->trk_data.crupdated); + hmapx_init(&data->trk_data.deleted); return data; } @@ -92,6 +93,7 @@ en_lr_nat_cleanup(void *data_) struct ed_type_lr_nat_data *data = (struct ed_type_lr_nat_data *) data_; lr_nat_table_destroy(&data->lr_nats); hmapx_destroy(&data->trk_data.crupdated); + hmapx_destroy(&data->trk_data.deleted); } void @@ -99,6 +101,11 @@ en_lr_nat_clear_tracked_data(void *data_) { struct ed_type_lr_nat_data *data = (struct ed_type_lr_nat_data *) data_; hmapx_clear(&data->trk_data.crupdated); + struct hmapx_node *hmapx_node; + HMAPX_FOR_EACH_SAFE (hmapx_node, &data->trk_data.deleted) { + lr_nat_record_destroy(hmapx_node->data); + hmapx_delete(&data->trk_data.deleted, hmapx_node); + } } enum engine_node_state @@ -125,11 +132,8 @@ lr_nat_northd_handler(struct engine_node *node, void *data_) return EN_UNHANDLED; } - if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) { - return EN_UNHANDLED; - } - - if (!northd_has_lr_nats_in_tracked_data(&northd_data->trk_data)) { + if (!northd_has_lr_nats_in_tracked_data(&northd_data->trk_data) && + hmapx_count(&northd_data->trk_data.trk_routers.deleted) == 0) { return EN_HANDLED_UNCHANGED; } @@ -137,17 +141,27 @@ lr_nat_northd_handler(struct engine_node *node, void *data_) struct lr_nat_record *lrnat_rec; const struct ovn_datapath *od; struct hmapx_node *hmapx_node; + struct lr_nat_table *table = &data->lr_nats; HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_nat_lrs) { od = hmapx_node->data; - lrnat_rec = lr_nat_table_find_by_index_(&data->lr_nats, od->index); - ovs_assert(lrnat_rec); - lr_nat_record_reinit(lrnat_rec, od, &northd_data->lr_ports); + lrnat_rec = lr_nat_table_find_by_uuid_(&data->lr_nats, od->key); + if (lrnat_rec) { + lr_nat_record_reinit(lrnat_rec, od, &northd_data->lr_ports); + } else { + lrnat_rec = lr_nat_record_create(table, od, + &northd_data->lr_ports); + } /* Add the lrnet rec to the tracking data. */ hmapx_add(&data->trk_data.crupdated, lrnat_rec); } - + HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_routers.deleted) { + od = hmapx_node->data; + lrnat_rec = lr_nat_table_find_by_uuid_(table, od->key); + hmap_remove(&table->entries, &lrnat_rec->key_node); + hmapx_add(&data->trk_data.deleted, lrnat_rec); + } if (lr_nat_has_tracked_data(&data->trk_data)) { return EN_HANDLED_UPDATED; } @@ -171,9 +185,6 @@ lr_nat_table_clear(struct lr_nat_table *table) HMAP_FOR_EACH_POP (lrnat_rec, key_node, &table->entries) { lr_nat_record_destroy(lrnat_rec); } - - free(table->array); - table->array = NULL; } static void @@ -181,8 +192,6 @@ lr_nat_table_build(struct lr_nat_table *table, const struct ovn_datapaths *lr_datapaths, const struct hmap *lr_ports) { - table->array = xrealloc(table->array, - ods_size(lr_datapaths) * sizeof *table->array); const struct ovn_datapath *od; HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { @@ -197,13 +206,19 @@ lr_nat_table_destroy(struct lr_nat_table *table) hmap_destroy(&table->entries); } -struct lr_nat_record * -lr_nat_table_find_by_index_(const struct lr_nat_table *table, - size_t od_index) +static struct lr_nat_record * +lr_nat_table_find_by_uuid_(const struct lr_nat_table *table, + struct uuid nb_uuid) { - ovs_assert(od_index <= hmap_count(&table->entries)); - - return table->array[od_index]; + struct lr_nat_record *record; + HMAP_FOR_EACH_WITH_HASH (record, key_node, + uuid_hash(&nb_uuid), + &table->entries){ + if (uuid_equals(&record->nbr_uuid, &nb_uuid)) { + return record; + } + } + return NULL; } static struct lr_nat_record * @@ -218,7 +233,6 @@ lr_nat_record_create(struct lr_nat_table *table, hmap_insert(&table->entries, &lrnat_rec->key_node, uuid_hash(&od->nbr->header_.uuid)); - table->array[od->index] = lrnat_rec; return lrnat_rec; } diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h index 495e24042..c84484104 100644 --- a/northd/en-lr-nat.h +++ b/northd/en-lr-nat.h @@ -97,17 +97,16 @@ struct lr_nat_record { struct lr_nat_tracked_data { /* Created or updated logical router with NAT data. */ struct hmapx crupdated; + /* Deleted logical router with NAT data. */ + struct hmapx deleted; }; struct lr_nat_table { struct hmap entries; /* Stores struct lr_nat_record. */ - - /* The array index of each element in 'entries'. */ - struct lr_nat_record **array; }; -const struct lr_nat_record * lr_nat_table_find_by_index( - const struct lr_nat_table *, size_t od_index); +const struct lr_nat_record * lr_nat_table_find_by_uuid( + const struct lr_nat_table *, struct uuid nb_uuid); #define LR_NAT_TABLE_FOR_EACH(LR_NAT_REC, TABLE) \ HMAP_FOR_EACH (LR_NAT_REC, key_node, &(TABLE)->entries) @@ -134,7 +133,8 @@ nat_entry_is_v6(const struct ovn_nat *nat_entry) static inline bool lr_nat_has_tracked_data(struct lr_nat_tracked_data *trk_data) { - return !hmapx_is_empty(&trk_data->crupdated); + return !hmapx_is_empty(&trk_data->crupdated) || + !hmapx_is_empty(&trk_data->deleted); } #endif /* EN_LR_NAT_H */ diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c index c80a49819..2338d4ba2 100644 --- a/northd/en-lr-stateful.c +++ b/northd/en-lr-stateful.c @@ -135,7 +135,8 @@ 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)) { + if (!northd_has_tracked_data(&northd_data->trk_data) || + northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) { return EN_UNHANDLED; } @@ -187,8 +188,8 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) struct lr_stateful_record *lr_stateful_rec = lr_stateful_table_find_(&data->table, od->nbr); if (!lr_stateful_rec) { - const struct lr_nat_record *lrnat_rec = lr_nat_table_find_by_index( - input_data.lr_nats, od->index); + const struct lr_nat_record *lrnat_rec = lr_nat_table_find_by_uuid( + input_data.lr_nats, od->key); ovs_assert(lrnat_rec); lr_stateful_rec = diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 2d62a2399..3c358c232 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -12521,7 +12521,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 norecompute compute -check_engine_stats lr_nat recompute nocompute +check_engine_stats lr_nat norecompute compute check_engine_stats lr_stateful recompute nocompute check_engine_stats sync_to_sb_pb recompute nocompute check_engine_stats sync_to_sb_lb norecompute compute @@ -12532,7 +12532,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb lr-del lr0 check_engine_stats northd norecompute compute -check_engine_stats lr_nat recompute nocompute +check_engine_stats lr_nat norecompute compute check_engine_stats lr_stateful recompute nocompute check_engine_stats sync_to_sb_pb recompute nocompute check_engine_stats sync_to_sb_lb norecompute compute -- 2.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
