On Tue, Dec 21, 2021 at 9:10 AM Lorenzo Bianconi
<[email protected]> wrote:
>
> If a patch port that connects a logical router to a logical switch is
> added in a different ovn-controller run with respect to the logical
> switch peer port, just the router port will be processed and
> ovn-controller will miss missing some open-flows for the ls patch port
> since the peer pointer was NULL when it has been processed.
> Fix the issue always processing both patch ports.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2025623
> Tested-by: Slawek Kaplonski <[email protected]>
> Acked-by: Mark Michelson <[email protected]>
> Fixes: 3ae8470edc64 ("I-P: Handle runtime data changes for pflow_output
> engine.")
> Signed-off-by: Lorenzo Bianconi <[email protected]>
Thanks Lorenzo for the fix and test case in v2.
I applied this patch to the main and backported to branch-21.12 and branch-21.09
Numan
> ---
> Changes since v1:
> - add unit-test
> ---
> controller/physical.c | 49 +++++++++++++++++++++++------------------
> tests/ovn-controller.at | 48 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 75 insertions(+), 22 deletions(-)
>
> diff --git a/controller/physical.c b/controller/physical.c
> index aa1d44bc6..836fc769a 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1564,6 +1564,24 @@ consider_mc_group(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> sset_destroy(&vtep_chassis);
> }
>
> +static void
> +physical_eval_port_binding(struct physical_ctx *p_ctx,
> + const struct sbrec_port_binding *pb,
> + struct ovn_desired_flow_table *flow_table)
> +{
> + struct ofpbuf ofpacts;
> + ofpbuf_init(&ofpacts, 0);
> + consider_port_binding(p_ctx->sbrec_port_binding_by_name,
> + p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
> + p_ctx->active_tunnels,
> + p_ctx->local_datapaths,
> + p_ctx->local_bindings,
> + p_ctx->patch_ofports,
> + p_ctx->chassis_tunnels,
> + pb, p_ctx->chassis, flow_table, &ofpacts);
> + ofpbuf_uninit(&ofpacts);
> +}
> +
> bool
> physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
> bool removed, struct physical_ctx *p_ctx,
> @@ -1585,33 +1603,20 @@ physical_handle_flows_for_lport(const struct
> sbrec_port_binding *pb,
> get_local_datapath(p_ctx->local_datapaths,
> pb->datapath->tunnel_key);
> if (ldp && ldp->localnet_port) {
> - struct ofpbuf ofpacts;
> ofctrl_remove_flows(flow_table,
> &ldp->localnet_port->header_.uuid);
> - ofpbuf_init(&ofpacts, 0);
> - consider_port_binding(p_ctx->sbrec_port_binding_by_name,
> - p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
> - p_ctx->active_tunnels,
> - p_ctx->local_datapaths,
> - p_ctx->local_bindings,
> - p_ctx->patch_ofports,
> - p_ctx->chassis_tunnels,
> - ldp->localnet_port, p_ctx->chassis,
> - flow_table, &ofpacts);
> - ofpbuf_uninit(&ofpacts);
> + physical_eval_port_binding(p_ctx, ldp->localnet_port,
> flow_table);
> }
> }
>
> if (!removed) {
> - struct ofpbuf ofpacts;
> - ofpbuf_init(&ofpacts, 0);
> - consider_port_binding(p_ctx->sbrec_port_binding_by_name,
> - p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
> - p_ctx->active_tunnels, p_ctx->local_datapaths,
> - p_ctx->local_bindings,
> - p_ctx->patch_ofports,
> - p_ctx->chassis_tunnels, pb,
> - p_ctx->chassis, flow_table, &ofpacts);
> - ofpbuf_uninit(&ofpacts);
> + 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);
> + }
> + }
> }
>
> return true;
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 7c683cbcc..ea632034b 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -806,3 +806,51 @@ OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-controller - ovn IP check path ports])
> +AT_KEYWORDS([ovn-ip-patch-ports])
> +
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +check ovn-nbctl lr-add lr0 \
> + -- lrp-add lr0 rp-ls0 00:00:01:01:02:02 192.168.0.254/24 \
> + -- ls-add ls0 \
> + -- lsp-add ls0 ls0-rp \
> + -- lsp-set-type ls0-rp router \
> + -- lsp-set-addresses ls0-rp router \
> + -- lsp-set-options ls0-rp router-port=rp-ls0 \
> + -- lsp-add ls0 lsp0 \
> + -- lsp-set-addresses lsp0 "00:11:00:00:00:01 192.168.0.1" \
> + -- ls-add ls1 \
> + -- lsp-add ls1 ls1-rp \
> + -- lsp-set-type ls1-rp router \
> + -- lsp-set-addresses ls1-rp router \
> + -- lsp-set-options ls1-rp router-port=rp-ls1 \
> + -- lsp-add ls1 lsp1 \
> + -- lsp-set-addresses lsp1 "00:00:00:00:00:01 192.168.1.1"
> +
> +as hv1
> +check ovs-vsctl \
> + -- add-port br-int vif0 \
> + -- set Interface vif0 external_ids:iface-id=lsp0 \
> + -- add-port br-int vif1 \
> + -- set Interface vif1 external_ids:iface-id=lsp1
> +
> +OVS_WAIT_UNTIL([grep -q 'Setting lport lsp0 up in Southbound'
> hv1/ovn-controller.log])
> +OVS_WAIT_UNTIL([grep -q 'Setting lport lsp1 up in Southbound'
> hv1/ovn-controller.log])
> +
> +meta=$(ovn-sbctl get datapath ls1 tunnel_key)
> +port=$(ovn-sbctl get port_binding ls1-rp tunnel_key)
> +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=38 | grep -q
> "reg15=0x${port},metadata=0x${meta}"])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> +])
> --
> 2.33.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
