[ovs-dev] [PATCH v2] ovn: Delete stale MAC_Binding records

[Ryan Moats]

"dev" <dev-bounces at openvswitch.org> wrote on 08/16/2016 02:27:30 PM:

> From: Chandra Sekhar Vejendla/San Jose/IBM at IBMUS
> To: dev at openvswitch.org
> Date: 08/16/2016 02:33 PM
> Subject: [ovs-dev] [PATCH v2] ovn: Delete stale MAC_Binding records
> Sent by: "dev" <dev-bounces at openvswitch.org>
>
> Entries in MAC_Binding table are not deleted when the logical_ports
> referred to in MAC_Bindings are deleted. The patch fixes this by
> deleting the MAC_Binding entry when the logical_port is not found.
>
> Signed-off-by: Chandra Sekhar Vejendla <csvejend at us.ibm.com>
> ---
>  ovn/controller/lflow.c | 25 +++++++++++++++++--------
>  tests/ovn.at           | 25 +++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index 341ca08..fbd36dc 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -510,13 +510,11 @@ put_load(const uint8_t *data, size_t len,
>  }
>
>  static void
> -consider_neighbor_flow(const struct lport_index *lports,
> +consider_neighbor_flow(const struct sbrec_port_binding *pb,
>                         const struct sbrec_mac_binding *b,
>                         struct ofpbuf *ofpacts_p,
>                         struct match *match_p)
>  {
> -    const struct sbrec_port_binding *pb
> -        = lport_lookup_by_name(lports, b->logical_port);
>      if (!pb) {
>          return;
>      }
> @@ -571,10 +569,19 @@ add_neighbor_flows(struct controller_ctx *ctx,
>      match_init_catchall(&match);
>      ofpbuf_init(&ofpacts, 0);
>
> -    const struct sbrec_mac_binding *b;
> +    const struct sbrec_port_binding *pb;
> +    const struct sbrec_mac_binding *b, *n;
>      if (full_neighbor_flow_processing) {
> -        SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) {
> -            consider_neighbor_flow(lports, b, &ofpacts, &match);
> +        SBREC_MAC_BINDING_FOR_EACH_SAFE (b, n, ctx->ovnsb_idl) {
> +            /* Remove mac binding record if the port binding for the
logical
> +             * port in mac binding record is not found. */
> +            pb = lport_lookup_by_name(lports, b->logical_port);
> +            if (!pb && ctx->ovnsb_idl_txn) {
> +                ofctrl_remove_flows(&b->header_.uuid);
> +                sbrec_mac_binding_delete(b);
> +                continue;
> +            }
> +            consider_neighbor_flow(pb, b, &ofpacts, &match);
>          }
>          full_neighbor_flow_processing = false;
>      } else {
> @@ -585,11 +592,13 @@ add_neighbor_flows(struct controller_ctx *ctx,
>                  if (!sbrec_mac_binding_is_new(b)) {
>                      ofctrl_remove_flows(&b->header_.uuid);
>                  }
> -                consider_neighbor_flow(lports, b, &ofpacts, &match);
> +                pb = lport_lookup_by_name(lports, b->logical_port);
> +                if (pb) {
> +                    consider_neighbor_flow(pb, b, &ofpacts, &match);
> +                }
>              }
>          }
>      }
> -
>      ofpbuf_uninit(&ofpacts);
>  }
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 216bb07..dbb205c 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -4965,3 +4965,28 @@ cat packets
>  OVN_CLEANUP([hv1])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- delete mac bindings])
> +AT_KEYWORDS([ovn])
> +ovn_start
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl -- add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +# Create logical switch ls0
> +ovn-nbctl ls-add ls0
> +# Create port lp0 in ls0
> +ovn-nbctl lsp-add ls0 lp0
> +ovn-nbctl lsp-set-addresses lp0 "f0:00:00:00:00:01 192.168.0.1"
> +dp_uuid=`ovn-sbctl find datapath | grep uuid | cut -f2 -d ":" | cut
> -f2 -d " "`
> +ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp0 mac="mac"
> +ovn-sbctl find MAC_Binding
> +#Delete port lp0
> +ovn-nbctl lsp-del lp0
> +ovn-sbctl find MAC_Binding
> +AT_CHECK([ovn-sbctl find MAC_Binding], [0], [])
> +
> +OVN_CLEANUP([hv1])
> +
> +AT_CLEANUP
> --

I think this should be abandoned in favor of
http://patchwork.ozlabs.org/patch/662152/ as trying to fix this
in ovn-controller rather than in ovn-northd will lead to race
conditions.