On Thu, 2025-02-06 at 18:02 +0100, Dumitru Ceara wrote: > 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.
ack, I'll refine it. > > > + 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. maybe I misused it, my intention was to trigger en_routes_run when LB data changes. Thanks, Martin. > > > + 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
