On 2/5/25 10:21 AM, Martin Kalcok wrote: > This change builds on top of the new "dynamic routing" OVN feature > that allows advertising routes to the fabric network. When LR option > "dynamic-routing" is set on the router, following two new LRP options > become available: > > * dynamic-routing-nat - When set to "true", ovn-controller will advertise > routes for external NAT IPs valid for the LRP. > * dynamic-routing-lb-vips - When set to "true", ovn-controller will advertise > host routes to LB VIPs via the LRP. > > Co-authored-by: Frode Nordahl <[email protected]> > Signed-off-by: Frode Nordahl <[email protected]> > Signed-off-by: Martin Kalcok <[email protected]> > --- > NEWS | 17 + > northd/en-advertised-route-sync.c | 198 +++++++++++ > northd/en-northd.c | 5 +- > northd/inc-proc-northd.c | 5 + > northd/northd.c | 134 ++++++- > northd/northd.h | 8 +- > ovn-nb.xml | 32 ++ > ovs | 2 +- > tests/ovn-northd.at | 556 ++++++++++++++++++++++++++++++ > tests/system-ovn.at | 379 ++++++++++++++++++++ > 10 files changed, 1332 insertions(+), 4 deletions(-)
Hi Martin, Not a full review but I'll drop this here for now (and continue reviewing). > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 86b7b999e..d74c2e3dc 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -267,6 +267,11 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_routes, &en_bfd, NULL); > engine_add_input(&en_routes, &en_northd, > routes_northd_change_handler); > + engine_add_input(&en_routes, &en_lr_nat, > + NULL); This means we trigger a recompute of en_routes every time a nat is changed which in turn triggers a recompute of the lflow node. I think we should add a handler that at least restricts it to recompute if changed nats/lbs are part of routers that have dynamic routing enabled. We can refine it further later and add real incremental processing (routes per datapath) but for now we shouldn't affect non-dynamic-routing deployment performance. > + engine_add_input(&en_routes, &en_lb_data, > + NULL); Do we need this dependency? I don't think we use en_lb_data in the en_routes node anywhere. > + engine_add_input(&en_routes, &en_lr_stateful, engine_noop_handler); > > engine_add_input(&en_bfd_sync, &en_bfd, NULL); > engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL); Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
