On Sep 22, Jacob Tanenbaum wrote: > 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. > > Reported-by: Dumitru Ceara <[email protected]> > Reported-at: https://issues.redhat.com/browse/FDP-757 > Signed-off-by: Jacob Tanenbaum <[email protected]>
Acked-by: Lorenzo Bianconi <[email protected]> > > ---- > > v4: removed minor formatting issues. > > diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c > index 0583e34a5..f2403ca17 100644 > --- a/northd/en-lr-nat.c > +++ b/northd/en-lr-nat.c > @@ -48,6 +48,9 @@ static void lr_nat_table_build(struct lr_nat_table *, > 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 > *, > const struct hmap > *lr_ports); > @@ -83,6 +86,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 +96,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 +104,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 +135,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 +144,36 @@ 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; > + > + if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) { > + if (hmap_count(&northd_data->lr_datapaths.datapaths) > > + hmap_count(&table->entries)) { > + table->array = xrealloc(table->array, > + ods_size(&northd_data->lr_datapaths) * > + sizeof *table->array); > + } > + } > > 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_index_(table, od->index); > + 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; > } > @@ -206,6 +232,19 @@ lr_nat_table_find_by_index_(const struct lr_nat_table > *table, > return table->array[od_index]; > } > > +static struct lr_nat_record * > +lr_nat_table_find_by_uuid_(const struct lr_nat_table *table, > + struct uuid nb_uuid) > +{ > + struct lr_nat_record *record; > + HMAP_FOR_EACH_WITH_HASH (record, key_node, > + uuid_hash(&nb_uuid), > + &table->entries){ > + return record; > + } > + return NULL; > +} > + > static struct lr_nat_record * > lr_nat_record_create(struct lr_nat_table *table, > const struct ovn_datapath *od, > diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h > index 495e24042..c78e30fbf 100644 > --- a/northd/en-lr-nat.h > +++ b/northd/en-lr-nat.h > @@ -97,6 +97,8 @@ 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 { > @@ -134,7 +136,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..2b6c0d0ff 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; > } > > 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
