The incremental processing engine implementation is such that if an input node gets updated but the output node doesn't have a change handler for it then the output node is immediately recomputed. That is, no other input change handlers are executed.
In case of the en_physical_flow_changes node, if a ct-zone changes we were also skipping the OVS interface change handler. That is incorrect as there is an implicit requirement that flows for OVS interfaces that got deleted should be cleared before physical_run() is called. Instead, we now add an explicit change handler for ct-zone changes. This change handler never processes ct-zone updates incrementally but ensures that the OVS interface change handler is also called. Moreover, OVS interface changes (including deletes) are now processed before physical_run() is called in the flow_output physical_flow_changes_handler. This is a requirement because otherwise physical_run() will fail to add flows for new OVS interfaces that correspond to unchanged Port_Bindings. Reported-by: Daniel Alvarez <[email protected]> Reported-at: https://bugzilla.redhat.com/1908391 Fixes: a3005f0dc777 ("ovn-controller: I-P for ct zone and OVS interface changes in flow output stage.") Signed-off-by: Dumitru Ceara <[email protected]> --- controller/ovn-controller.c | 30 ++++++++++++++----- tests/ovn.at | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 8 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index b5f0c13..5708b7b 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1714,6 +1714,16 @@ en_physical_flow_changes_run(struct engine_node *node, void *data) engine_set_node_state(node, EN_UPDATED); } +/* ct_zone changes are not handled incrementally but a handler is required + * to avoid skipping the ovs_iface incremental change handler. + */ +static bool +physical_flow_changes_ct_zones_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return false; +} + /* There are OVS interface changes. Indicate to the flow_output engine * to handle these OVS interface changes for physical flow computations. */ static bool @@ -2256,6 +2266,13 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) struct ed_type_pfc_data *pfc_data = engine_get_input_data("physical_flow_changes", node); + /* If there are OVS interface changes. Try to handle them incrementally. */ + if (pfc_data->ovs_ifaces_changed) { + if (!physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table)) { + return false; + } + } + if (pfc_data->recompute_physical_flows) { /* This indicates that we need to recompute the physical flows. */ physical_clear_unassoc_flows_with_db(&fo->flow_table); @@ -2265,12 +2282,6 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) return true; } - if (pfc_data->ovs_ifaces_changed) { - /* There are OVS interface changes. Try to handle them - * incrementally. */ - return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table); - } - return true; } @@ -2549,11 +2560,14 @@ main(int argc, char *argv[]) /* Engine node physical_flow_changes indicates whether * we can recompute only physical flows or we can * incrementally process the physical flows. + * + * Note: The order of inputs is important, all OVS interface changes must + * be handled before any ct_zone changes. */ - engine_add_input(&en_physical_flow_changes, &en_ct_zones, - NULL); engine_add_input(&en_physical_flow_changes, &en_ovs_interface, physical_flow_changes_ovs_iface_handler); + engine_add_input(&en_physical_flow_changes, &en_ct_zones, + physical_flow_changes_ct_zones_handler); engine_add_input(&en_flow_output, &en_addr_sets, flow_output_addr_sets_handler); diff --git a/tests/ovn.at b/tests/ovn.at index 80bc099..66088a7 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22059,6 +22059,78 @@ OVS_WAIT_UNTIL([test x$(as hv1 ovn-appctl -t ovn-controller debug/status) = "xru OVN_CLEANUP([hv1]) AT_CLEANUP +AT_SETUP([ovn -- Multiple OVS port changes Incremental Processing]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.10 + +check ovn-nbctl ls-add sw +check ovn-nbctl lsp-add sw lsp1 +check ovn-nbctl lsp-add sw lsp2 + +as hv1 +ovs-vsctl \ + -- add-port br-int vif1 \ + -- set Interface vif1 external_ids:iface-id=lsp1 \ + ofport-request=1 +ovs-vsctl \ + -- add-port br-int vif2 \ + -- set Interface vif2 external_ids:iface-id=lsp2 \ + ofport-request=2 + +# Wait for ports to be bound. +wait_row_count Chassis 1 name=hv1 +ch=$(fetch_column Chassis _uuid name=hv1) +wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch +wait_row_count Port_Binding 1 logical_port=lsp2 chassis=$ch + +AS_BOX([check output flows for initial interfaces]) +as hv1 ovs-ofctl dump-flows br-int table=65 > offlows_table65.txt +AT_CAPTURE_FILE([offlows_table65.txt]) +AT_CHECK_UNQUOTED([grep -c "output:1" offlows_table65.txt], [0], [dnl +1 +]) +AT_CHECK_UNQUOTED([grep -c "output:2" offlows_table65.txt], [0], [dnl +1 +]) + +AS_BOX([delete and add OVS interfaces and force batch of updates]) +as hv1 ovn-appctl -t ovn-controller debug/pause + +as hv1 +ovs-vsctl \ + -- del-port vif1 \ + -- del-port vif2 + +as hv1 +ovs-vsctl \ + -- add-port br-int vif1 \ + -- set Interface vif1 external_ids:iface-id=lsp1 \ + ofport-request=3 \ + -- add-port br-int vif2 \ + -- set Interface vif2 external_ids:iface-id=lsp2 \ + ofport-request=4 + +as hv1 ovn-appctl -t ovn-controller debug/resume +check ovn-nbctl --wait=hv sync + +AS_BOX([check output flows for new interfaces]) +as hv1 ovs-ofctl dump-flows br-int table=65 > offlows_table65_2.txt +AT_CAPTURE_FILE([offlows_table65_2.txt]) +AT_CHECK_UNQUOTED([grep -c "output:3" offlows_table65_2.txt], [0], [dnl +1 +]) +AT_CHECK_UNQUOTED([grep -c "output:4" offlows_table65_2.txt], [0], [dnl +1 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + # Test dropping traffic destined to router owned IPs. AT_SETUP([ovn -- gateway router drop traffic for own IPs]) ovn_start -- 1.8.3.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
