On 1/13/25 1:11 PM, Lorenzo Bianconi wrote:
> 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 <[email protected]>
> ---
Hi Lorenzo,
> Changes in v2:
> - use OFTABLE_LOG_TO_PHY for table 65
> ---
> controller/physical.c | 14 ++++++++-----
> tests/ovn.at | 47 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/controller/physical.c b/controller/physical.c
> index c56c73c20..affa559d5 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -2413,12 +2413,16 @@ physical_handle_flows_for_lport(const struct
> sbrec_port_binding *pb,
>
> if (!removed) {
> 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")) {
> + const struct sbrec_port_binding *peer =
> + get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb);
> + if (peer) {
> + if (removed) {
> + ofctrl_remove_flows(flow_table, &peer->header_.uuid);
> }
> + physical_eval_port_binding(p_ctx, peer, flow_table);
I'm not sure if this works correctly. If "!removed", e.g., when the LSP
patch port is updated, we won't remove the flows. Won't that cause
stale flows?
I'm not sure of what the right way to fix this is. For example, if a
pb.type changes from "patch" to something else we won't be removing the
flows here. Because pb->type is already different.
CC-ing Ales, Numan in case they have ideas.
Regards,
Dumitru
> }
> }
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index de01a649f..3aad19a19 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -40752,3 +40752,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 | grep
> 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 | grep
> metadata=0x${ls_tnl_key} | grep -q load:0x2b->NXM_NX_REG14])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> +])
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
