From 73a10345a29c8d983566ea86c2a9ce1f91c7c963 Mon Sep 17 00:00:00 2001 From: Lorenzo Bianconi Date: Fri, 17 Jan 2025 18:29:46 +0100 Subject: [PATCH] controller: Update physical flows for peer port when the patch port is removed. Similar to commit 1431276926 ("controller: fix ovn patch port incremental processing"), update peer logical flows when the related patch port is removed. Reported-at: https://issues.redhat.com/browse/FDP-947 Signed-off-by: Lorenzo Bianconi Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 5 ++++ controller/physical.c | 20 +++++++++----- tests/ovn-controller.at | 53 +++++++++++++++++++++++++++++++++++++ tests/ovn.at | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 6 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 12cf52e579..854e54854c 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -4567,6 +4567,11 @@ pflow_output_sb_port_binding_handler(struct engine_node *node, */ const struct sbrec_port_binding *pb; SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) { + /* Trigger a full recompute if type column is updated. */ + if (sbrec_port_binding_is_updated(pb, SBREC_PORT_BINDING_COL_TYPE)) { + destroy_physical_ctx(&p_ctx); + return false; + } bool removed = sbrec_port_binding_is_deleted(pb); if (!physical_handle_flows_for_lport(pb, removed, &p_ctx, &pfo->flow_table)) { diff --git a/controller/physical.c b/controller/physical.c index 355d28aea4..69298e1e4c 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -2412,13 +2412,21 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb, physical_multichassis_reprocess(pb, p_ctx, flow_table); } - if (!removed) { + /* Always update pb and the configured peer for patch ports. */ + if (!removed || !strcmp(pb->type, "patch")) { physical_eval_port_binding(p_ctx, pb, flow_table); - if (!strcmp(pb->type, "patch")) { - const struct sbrec_port_binding *peer = - get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb); - if (peer) { - physical_eval_port_binding(p_ctx, peer, flow_table); + } + + if (!strcmp(pb->type, "patch")) { + if (removed) { + ofctrl_remove_flows(flow_table, &pb->header_.uuid); + } + const struct sbrec_port_binding *peer = + get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb); + if (peer) { + physical_eval_port_binding(p_ctx, peer, flow_table); + if (removed) { + ofctrl_remove_flows(flow_table, &peer->header_.uuid); } } } diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index df0158cd62..988de7bb3b 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -1,5 +1,53 @@ AT_BANNER([ovn-controller]) +m4_divert_push([PREPARE_TESTS]) +# Checks if the provided engine node recomputed or not +# and if it computed or not. +# 1st argument is the simulated hypervisor name. +# 2nd argument is the engine node. +# 3rd argument is the expected recompute state. +# Possible values are - "norecompute" and "recompute". +# 4th argument is the expected compute state. +# Possible values are - "nocompute" and "compute". +# +# Eg. 'check_controller_engine_stats hv1 logical_flow_output recompute nocompute' +# will verify that if the lflow engine node recompute stat is > 0 and +# compute stat is equal to 0. It fails otherwise. +check_controller_engine_stats() { + hv=$1 + node=$2 + recompute=$3 + compute=$4 + + echo "Checking engine stats for node $node : recompute - \ +$recompute : compute - $compute" + + node_stat=$(as $hv ovn-appctl -t ovn-controller inc-engine/show-stats $node) + # node_stat will be of this format : + # - Node: logical_flow_output - recompute: 3 - compute: 0 - cancel: 0 + node_recompute_ct=$(echo $node_stat | cut -d '-' -f2 | cut -d ':' -f2) + node_compute_ct=$(echo $node_stat | cut -d '-' -f3 | cut -d ':' -f2) + + if [[ "$recompute" == "norecompute" ]]; then + # node should not be recomputed + echo "Expecting $node recompute count - $node_recompute_ct to be 0" + check test "$node_recompute_ct" -eq "0" + else + echo "Expecting $node recompute count - $node_recompute_ct not to be 0" + check test "$node_recompute_ct" -ne "0" + fi + + if [[ "$compute" == "nocompute" ]]; then + # node should not be computed + echo "Expecting $node compute count - $node_compute_ct to be 0" + check test "$node_compute_ct" -eq "0" + else + echo "Expecting $node compute count - $node_compute_ct not to be 0" + check test "$node_compute_ct" -ne "0" + fi +} +m4_divert_pop([PREPARE_TESTS]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn-controller - ovn-bridge-mappings]) AT_KEYWORDS([ovn]) @@ -917,6 +965,11 @@ check ovn-nbctl lrp-add lr0 rp-ls1 00:00:01:01:02:03 192.168.1.254/24 OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int | grep table=OFTABLE_LOCAL_OUTPUT | grep -q "reg15=0x${port},metadata=0x${meta}"]) +# Check we have a full recompute if type column is updated +check as hv1 ovn-appctl -t ovn-controller inc-engine/clear-stats +check ovn-nbctl --wait=hv lsp-set-type ls0-rp localnet +check_controller_engine_stats hv1 physical_flow_output recompute nocompute + OVN_CLEANUP([hv1]) AT_CLEANUP ]) diff --git a/tests/ovn.at b/tests/ovn.at index 7a794f08d4..f0eca6ae41 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -40754,3 +40754,50 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([requested-tnl-key-recompute]) +AT_KEYWORDS([requested-tnl-key-recompute]) + +m4_define([OFTABLE_LOG_TO_PHY], [65]) + +ovn_start +net_add n1 + +check ovn-nbctl ls-add ls +check ovn-nbctl lsp-add ls lsp -- lsp-set-addresses lsp "00:00:10:01:02:01 10.0.0.1" + +check ovn-nbctl lr-add lr +check ovn-nbctl set logical_router lr options:mac_binding_age_threshold=3600 +check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:ff:01 10.0.0.254/24 +check ovn-nbctl lsp-add ls ls-lr +check ovn-nbctl lsp-set-type ls-lr router +check ovn-nbctl lsp-set-addresses ls-lr router +check ovn-nbctl lsp-set-options ls-lr router-port=lr-ls + +sim_add hv1 + +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys +ovs-vsctl add-port br-int vif -- set Interface vif external-ids:iface-id=lsp + +check ovn-nbctl --wait=hv sync +wait_for_ports_up + +check ovn-nbctl --wait=hv set logical_router_port lr-ls options:requested-tnl-key=42 +ls_tnl_key=$(fetch_column datapath_binding tunnel_key external_ids:name=ls) +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_LOG_TO_PHY,metadata=0x${ls_tnl_key} | grep -q load:0x2a->NXM_NX_REG14]) + +check ovn-nbctl lrp-del lr-ls +check ovn-nbctl \ + -- lrp-add lr lr-ls 00:00:00:00:10:00 192.168.10.1/24 \ + -- set logical_router_port lr-ls options:requested-tnl-key=43 +check ovn-nbctl --wait=hv sync + +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_LOG_TO_PHY,metadata=0x${ls_tnl_key} | grep -q load:0x2b->NXM_NX_REG14]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +])