On Thu, Oct 9, 2025 at 2:32 PM Xavier Simonart via dev < [email protected]> wrote:
> Hi Jacob > > Thanks for the patch > > Acked-by: Xavier Simonart <[email protected]> > > Thanks > Xavier > > On Wed, Oct 8, 2025 at 5:45 PM Jacob Tanenbaum via dev < > [email protected]> wrote: > > > Adding sync_to_sb_pb_lr_stateful_handler() allows port_bindings to be > > computed in the incremental processor instead of always fully > > recalculating when the lr_stateful engine is updated. > > > > The process is taking the updated lr_stateful_record, getting the > > logical router it references and syncing the routers ports and peers to > > those router ports. > > > > Signed-off-by: Jacob Tanenbaum <[email protected]> > > > > diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c > > index 198881e20..ff4de8d92 100644 > > --- a/northd/en-sync-sb.c > > +++ b/northd/en-sync-sb.c > > @@ -422,6 +422,28 @@ sync_to_sb_pb_northd_handler(struct engine_node > > *node, void *data OVS_UNUSED) > > return EN_HANDLED_UPDATED; > > } > > > > +enum engine_input_handler_result > > +sync_to_sb_pb_lr_stateful_handler(struct engine_node *node, > > + void *data OVS_UNUSED) > > +{ > > + struct ed_type_lr_stateful *lr_stateful_data = > > + engine_get_input_data("lr_stateful", node); > > + struct northd_data *northd_data = engine_get_input_data("northd", > > node); > > + > > + const struct ovn_datapaths *lr_datapaths = > &northd_data->lr_datapaths; > > + struct hmapx_node *hmapx_node; > > + HMAPX_FOR_EACH (hmapx_node, &lr_stateful_data->trk_data.crupdated) { > > + const struct lr_stateful_record *lr_stateful_rec = > > hmapx_node->data; > > + const struct ovn_datapath *od = > > + ovn_datapaths_find_by_index(lr_datapaths, > > + lr_stateful_rec->lr_index); > > + sync_pbs_for_lr_stateful_changes(od, > > + &lr_stateful_data->table); > > + } > > + > > + return EN_HANDLED_UPDATED; > > +} > > + > > /* static functions. */ > > static void > > sync_addr_set(struct ovsdb_idl_txn *ovnsb_txn, const char *name, > > diff --git a/northd/en-sync-sb.h b/northd/en-sync-sb.h > > index 21061a5b4..67768fe0c 100644 > > --- a/northd/en-sync-sb.h > > +++ b/northd/en-sync-sb.h > > @@ -31,5 +31,7 @@ enum engine_node_state en_sync_to_sb_pb_run(struct > > engine_node *, void *data); > > void en_sync_to_sb_pb_cleanup(void *data); > > enum engine_input_handler_result > > sync_to_sb_pb_northd_handler(struct engine_node *, void *data > OVS_UNUSED); > > +enum engine_input_handler_result > > +sync_to_sb_pb_lr_stateful_handler(struct engine_node *, void *data > > OVS_UNUSED); > > > > #endif /* end of EN_SYNC_SB_H */ > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > > index ff9515be7..fdea550d7 100644 > > --- a/northd/inc-proc-northd.c > > +++ b/northd/inc-proc-northd.c > > @@ -431,7 +431,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > > > engine_add_input(&en_sync_to_sb_pb, &en_northd, > > sync_to_sb_pb_northd_handler); > > - engine_add_input(&en_sync_to_sb_pb, &en_lr_stateful, NULL); > > + engine_add_input(&en_sync_to_sb_pb, &en_lr_stateful, > > + sync_to_sb_pb_lr_stateful_handler); > > > > /* en_sync_to_sb engine node syncs the SB database tables from > > * the NB database tables. > > diff --git a/northd/northd.c b/northd/northd.c > > index e6062979e..495f00cdc 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -3941,6 +3941,24 @@ sync_pbs_for_northd_changed_ovn_ports( > > } > > } > > > > +void > > +sync_pbs_for_lr_stateful_changes(const struct ovn_datapath *od, > > + const struct lr_stateful_table > > *lr_stateful) > > +{ > > + struct ovn_port *op; > > + HMAP_FOR_EACH (op, dp_node, &od->ports) { > > + sync_pb_for_lrp(op, lr_stateful); > > + > > + if (op->peer && op->peer->nbsp != NULL) { > nit: No need to compare to !NULL. > > + sync_pb_for_lsp(op->peer, lr_stateful); > > + } > > + > > + if (op->cr_port) { > > + sync_pb_for_lrp(op->cr_port, lr_stateful); > > + } > > + } > > +} > > + > > static bool > > ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key) > > { > > diff --git a/northd/northd.h b/northd/northd.h > > index a9c971aed..14639936d 100644 > > --- a/northd/northd.h > > +++ b/northd/northd.h > > @@ -992,6 +992,10 @@ void sync_pbs_for_northd_changed_ovn_ports( > > struct tracked_ovn_ports *, > > const struct lr_stateful_table *); > > > > +void > > +sync_pbs_for_lr_stateful_changes(const struct ovn_datapath *od, > > + const struct lr_stateful_table > > *lr_stateful); > nit: Declaration return type should be on the same line as the name. > > + > > static inline bool > > northd_has_tracked_data(struct northd_tracked_data *trk_nd_changes) { > > return trk_nd_changes->type != NORTHD_TRACKED_NONE; > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index b1aee0008..07fb57bd6 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -12609,7 +12609,7 @@ 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 norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > check_engine_stats lflow norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -12619,7 +12619,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 norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > check_engine_stats lflow norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -12687,7 +12687,7 @@ check ovn-nbctl --wait=sb lr-nat-add lr0 > > dnat_and_snat 172.168.0.110 10.0.0.4 > > check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -12697,7 +12697,7 @@ check ovn-nbctl --wait=sb set NAT . > options:foo=bar > > check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -12708,7 +12708,7 @@ check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -12719,7 +12719,7 @@ check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -12730,7 +12730,7 @@ check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -12741,7 +12741,7 @@ check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -12753,7 +12753,7 @@ check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > @@ -12770,7 +12770,7 @@ check ovn-nbctl --wait=sb lr-nat-add lr0 > > dnat_and_snat 172.168.0.140 10.0.0.20 > > check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > check_engine_stats lflow recompute nocompute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -12782,7 +12782,7 @@ check ovn-nbctl --wait=sb lr-nat-add lr0 > > dnat_and_snat 172.168.0.150 10.0.0.41 > > check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > check_engine_stats lflow recompute nocompute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -12794,7 +12794,7 @@ check ovn-nbctl --wait=sb lr-nat-del lr0 > > dnat_and_snat 172.168.0.150 > > check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > check_engine_stats lflow recompute nocompute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -12806,7 +12806,7 @@ check ovn-nbctl --wait=sb lr-nat-del lr0 > > dnat_and_snat 172.168.0.140 > > check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > check_engine_stats lflow recompute nocompute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -12818,7 +12818,7 @@ check_engine_stats northd norecompute compute > > check_engine_stats lr_nat norecompute compute > > check_engine_stats lr_stateful norecompute compute > > check_engine_stats lflow norecompute compute > > -check_engine_stats sync_to_sb_pb recompute nocompute > > +check_engine_stats sync_to_sb_pb norecompute compute > > check_engine_stats sync_to_sb_lb norecompute compute > > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > -- > > 2.51.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 Thank you Jacob and Xavier, I have addressed both nits and merged this into main. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
