On Sun, Jun 28, 2026 at 12:12 PM chanyeol yoon <[email protected]> wrote:
> From: Chanyeol Yoon <[email protected]> > > en-advertised-mac-binding-sync was recomputed on every northd and > lr_nat change. Add a northd change handler so that the node is > recomputed only when something that can affect the advertised set > actually changed: a full northd recompute (no tracked data), a created > or deleted Logical Switch, or a change to a Logical Router's NATs (the > distributed floating IPs advertised on its peer provider switch). > > The lr_nat input is then only read as data in run() and uses > engine_noop_handler, since the relevant NAT changes are picked up > through the northd tracked data. > > Add a test that exercises the distributed NAT path through incremental > processing: adding and deleting a floating IP must update the SB > Advertised_MAC_Binding without a full recompute, which would not happen > if the router's tracked NAT change were not propagated to the node. > > Suggested-by: Ales Musil <[email protected]> > Signed-off-by: Chanyeol Yoon <[email protected]> > --- > Hi Chanyeol, sorry for the dealy, I have misunderstood, this wasn't needed before. I have squashed the 1/3 and 2/3 as there is no point in keeping them separate. northd/en-advertised-route-sync.c | 31 ++++++++++++++++ > northd/en-advertised-route-sync.h | 3 ++ > northd/inc-proc-northd.c | 11 +++--- > tests/ovn-inc-proc-graph-dump.at | 4 +- > tests/ovn-northd.at | 61 +++++++++++++++++++++++++++++++ > 5 files changed, 103 insertions(+), 7 deletions(-) > > diff --git a/northd/en-advertised-route-sync.c > b/northd/en-advertised-route-sync.c > index 3c9d0bcb7..631368c29 100644 > --- a/northd/en-advertised-route-sync.c > +++ b/northd/en-advertised-route-sync.c > @@ -1105,3 +1105,34 @@ northd_output_advertised_mac_binding_sync_handler( > { > return EN_HANDLED_UPDATED; > } > + > +enum engine_input_handler_result > +advertised_mac_binding_sync_northd_change_handler(struct engine_node > *node, > + void *data OVS_UNUSED) > +{ > + struct northd_data *northd_data = engine_get_input_data("northd", > node); > + struct northd_tracked_data *trk_data = &northd_data->trk_data; > + > + /* Without tracked data northd did a full recompute (for example a > + * Logical_Switch 'other_config' change that toggles the EVPN > settings), > + * so the advertised set must be re-evaluated. */ > + if (!northd_has_tracked_data(trk_data)) { > + return EN_UNHANDLED; > + } > + > + /* A created or deleted Logical_Switch may have EVPN redistribution > + * enabled and contribute advertised MAC bindings. */ > + if (!hmapx_is_empty(&trk_data->trk_switches.crupdated) || > + !hmapx_is_empty(&trk_data->trk_switches.deleted)) { > + return EN_UNHANDLED; > + } > + > + /* The distributed dnat_and_snat NAT entries (floating IPs) of a > Logical > + * Router are advertised on its peer provider Logical Switch, so a > change > + * to a router's NATs must re-evaluate the advertised MAC bindings. */ > + if (!hmapx_is_empty(&trk_data->trk_nat_lrs)) { > + return EN_UNHANDLED; > + } > + > + return EN_HANDLED_UNCHANGED; > +} > diff --git a/northd/en-advertised-route-sync.h > b/northd/en-advertised-route-sync.h > index 71cd417de..3bb48fa0a 100644 > --- a/northd/en-advertised-route-sync.h > +++ b/northd/en-advertised-route-sync.h > @@ -52,4 +52,7 @@ en_advertised_mac_binding_sync_run(struct engine_node *, > void *data); > enum engine_input_handler_result > northd_output_advertised_mac_binding_sync_handler(struct engine_node *, > void *data); > +enum engine_input_handler_result > +advertised_mac_binding_sync_northd_change_handler(struct engine_node > *node, > + void *data); > #endif /* EN_ADVERTISED_ROUTE_SYNC_H */ > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index be968672f..ec9deff4c 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -364,11 +364,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > NULL); > engine_add_input(&en_advertised_mac_binding_sync, > &en_sb_advertised_mac_binding, NULL); > - /* The advertised MAC bindings are derived from northd data (port > lookups) > - * and the lr_nat NAT entries, both read in run(), so recompute on any > - * change to either input. */ > - engine_add_input(&en_advertised_mac_binding_sync, &en_northd, NULL); > - engine_add_input(&en_advertised_mac_binding_sync, &en_lr_nat, NULL); > + engine_add_input(&en_advertised_mac_binding_sync, &en_northd, > + advertised_mac_binding_sync_northd_change_handler); > + /* The lr_nat data is only read in run(); the recompute is triggered > via > + * the northd tracked NAT data (see the handler above). */ > + engine_add_input(&en_advertised_mac_binding_sync, &en_lr_nat, > + engine_noop_handler); > > engine_add_input(&en_learned_route_sync, &en_sb_learned_route, > learned_route_sync_sb_learned_route_change_handler); > diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ > ovn-inc-proc-graph-dump.at > index b0924ae73..eee6a1fb7 100644 > --- a/tests/ovn-inc-proc-graph-dump.at > +++ b/tests/ovn-inc-proc-graph-dump.at > @@ -228,8 +228,8 @@ digraph "Incremental-Processing-Engine" { > advertised_mac_binding_sync [[style=filled, shape=box, > fillcolor=white, label="advertised_mac_binding_sync"]]; > SB_port_binding -> advertised_mac_binding_sync [[label=""]]; > SB_advertised_mac_binding -> advertised_mac_binding_sync > [[label=""]]; > - northd -> advertised_mac_binding_sync [[label=""]]; > - lr_nat -> advertised_mac_binding_sync [[label=""]]; > + northd -> advertised_mac_binding_sync > [[label="advertised_mac_binding_sync_northd_change_handler"]]; > + lr_nat -> advertised_mac_binding_sync > [[label="engine_noop_handler"]]; > northd_output [[style=filled, shape=box, fillcolor=white, > label="northd_output"]]; > acl_id -> northd_output [[label="northd_output_acl_id_handler"]]; > sync_from_sb -> northd_output [[label=""]]; > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index b2ba12d20..192c1a0d4 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -8,6 +8,7 @@ m4_define([_DUMP_DB_TABLES], [ > echo ======= >> $1 > ovn-sbctl list logical_flow >> $1 > ovn-sbctl list port_binding >> $1 > + ovn-sbctl list advertised_mac_binding >> $1 > ovn-sbctl list address_set >> $1 > ovn-sbctl list meter >> $1 > ovn-sbctl list meter_band >> $1 > @@ -20448,6 +20449,66 @@ OVN_CLEANUP_NORTHD > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([LR EVPN Advertised_MAC_Binding sync - incremental NAT changes]) > +ovn_start > + > +check ovn-sbctl chassis-add ch geneve 127.0.0.1 > + > +dnl Same topology as the distributed NAT test: a tenant LS, a LR with a > +dnl distributed gateway port, and an EVPN-enabled provider LS (with the > 'nat' > +dnl token) so that distributed floating IPs are advertised. > +check ovn-nbctl ls-add ls-tenant > \ > + -- lsp-add ls-tenant vm1 > \ > + -- lsp-set-addresses vm1 "00:00:00:00:02:00 10.0.0.10" > +check ovn-nbctl ls-add ls-evpn > \ > + -- set logical_switch ls-evpn other_config:dynamic-routing-vni=10 > \ > + -- set logical_switch ls-evpn > other_config:dynamic-routing-redistribute=ip,nat > +check ovn-nbctl lsp-add ls-evpn ln-evpn > \ > + -- lsp-set-type ln-evpn localnet > \ > + -- lsp-set-addresses ln-evpn unknown > \ > + -- lsp-set-options ln-evpn network_name=phys > +check ovn-nbctl lr-add lr > \ > + -- lrp-add lr lrp-int 00:00:00:00:01:01 10.0.0.1/24 > \ > + -- lrp-add lr lrp-ext 00:00:00:00:01:02 172.16.0.1/24 > \ > + -- lrp-set-gateway-chassis lrp-ext ch > \ > + -- lsp-add ls-tenant ls-tenant-lrp > \ > + -- lsp-set-type ls-tenant-lrp router > \ > + -- lsp-set-addresses ls-tenant-lrp 00:00:00:00:01:01 > \ > + -- lsp-set-options ls-tenant-lrp router-port=lrp-int > \ > + -- lsp-add ls-evpn ls-evpn-lrp > \ > + -- lsp-set-type ls-evpn-lrp router > \ > + -- lsp-set-addresses ls-evpn-lrp 00:00:00:00:01:02 > \ > + -- lsp-set-options ls-evpn-lrp router-port=lrp-ext > +check ovn-nbctl --wait=sb sync > + > +ls_evpn_uuid=$(fetch_column Datapath_Binding _uuid > external_ids:name=ls-evpn) > +check_row_count Advertised_MAC_Binding 1 ip=172.16.0.1 > datapath=$ls_evpn_uuid > + > +AS_BOX([Adding a distributed floating IP is processed incrementally]) > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb lr-nat-add lr dnat_and_snat 172.16.0.100 > 10.0.0.10 vm1 00:11:22:33:44:55 > +check_row_count Advertised_MAC_Binding 1 ip=172.16.0.100 > datapath=$ls_evpn_uuid mac='00\:11\:22\:33\:44\:55' > +check_row_count Advertised_MAC_Binding 2 > +dnl The NAT lives on the router while the advertisement is attached to the > +dnl peer provider switch. The router's tracked NAT change must drive a > +dnl recompute of the advertised MAC bindings, otherwise the floating IP > would > +dnl never reach the SB. > +check_engine_compute advertised_mac_binding_sync recompute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +AS_BOX([Deleting a distributed floating IP is processed incrementally]) > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb lr-nat-del lr dnat_and_snat 172.16.0.100 > +check_row_count Advertised_MAC_Binding 0 ip=172.16.0.100 > +check_row_count Advertised_MAC_Binding 1 > +check_engine_compute advertised_mac_binding_sync recompute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +OVN_CLEANUP_NORTHD > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([IGMP northd crash]) > ovn_start > -- > 2.54.0 (Apple Git-157) > > Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
