[ovs-dev] [PATCH v2 ovn] controller: fix physical flow update for localport

Han Zhou hzhou at ovn.org
Mon May 17 21:00:12 UTC 2021


On Mon, May 17, 2021 at 8:37 AM Lorenzo Bianconi <
lorenzo.bianconi at redhat.com> wrote:
>
> Properly update logical/openflow flows for localport removing the
> interface from the ovs bridge. Openflows in table 65 are not recomputed
> removing a localport from an ovs-bridge and the ovs bridge ends-up with
> a stale configuration adding the interface back. Fix the issue taking
> care of localport special case in physical_handle_ovs_iface_changes
> routine.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
> Changes since v1:
> - fix test cases
> - add some some comment in physical_handle_ovs_iface_changes
> ---
>  controller/ovn-controller.c |  1 +
>  controller/physical.c       | 12 +++++++++++-
>  tests/ovn.at                | 20 ++++++++++++++++++++
>  3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index b08db728e..031b275c8 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1846,6 +1846,7 @@ en_physical_flow_changes_run(struct engine_node
*node, void *data)
>  {
>      struct ed_type_pfc_data *pfc_tdata = data;
>      pfc_tdata->recompute_physical_flows = true;
> +    pfc_tdata->ovs_ifaces_changed = true;
>      engine_set_node_state(node, EN_UPDATED);
>  }
>
> diff --git a/controller/physical.c b/controller/physical.c
> index 96c959d18..dbef96720 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1874,7 +1874,17 @@ physical_handle_ovs_iface_changes(struct
physical_ctx *p_ctx,
>          const struct sbrec_port_binding *lb_pb =
>              local_binding_get_primary_pb(p_ctx->local_bindings,
iface_id);
>          if (!lb_pb) {
> -            continue;
> +            /* For regular VIFs (e.g. lsp) the upcoming port-binding
update
> +             * will remove lfows related to the unclaimed ovs port.
> +             * Localport is a special case and it needs to be managed
here
> +             * since the port is not binded and otherwise the related
lfows
> +             * will not be cleared removing the ovs port.
> +             */
> +            lb_pb =
lport_lookup_by_name(p_ctx->sbrec_port_binding_by_name,
> +                                         iface_id);
> +            if (!lb_pb || strcmp(lb_pb->type, "localport")) {
> +                continue;
> +            }
>          }
>
>          int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
> diff --git a/tests/ovn.at b/tests/ovn.at
> index dcf3e0e09..363bd36fb 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -11814,6 +11814,26 @@ for i in 1 2; do
>      done
>  done
>
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int |awk '/table=65/{print
substr($8, 16, length($8))}' |sort -n], [0], [dnl
> +10
> +11
> +])
> +
> +# remove the localport from br-int and re-create it
> +as hv1
> +check ovs-vsctl del-port vif01
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int |awk '/table=65/{print
substr($8, 16, length($8))}' |sort -n], [0], [dnl
> +11
> +])
> +
> +as hv1
> +check ovs-vsctl add-port br-int vif01 \
> +    -- set Interface vif01 external-ids:iface-id=lp01
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int |awk '/table=65/{print
substr($8, 16, length($8))}' |sort -n], [0], [dnl
> +2
> +11
> +])
> +
>  OVN_CLEANUP([hv1],[hv2])
>
>  AT_CLEANUP
> --
> 2.31.1
>

Thanks Lorenzo! I applied this to master branch and backported to 21.03.
Please let me know if it needs to be backported to older branches.


More information about the dev mailing list