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]> --- 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) _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
