Hi Zhenyu Gao

Is it possible for you to add a test case for this scenario. This test on
the master code( without your patch) should fail, and your patch should
make the test pass.

Thanks
Anil

On Wed, Aug 16, 2017 at 12:17 PM, Zhenyu Gao <sysugaozhe...@gmail.com>
wrote:

> The bfd_calculate_chassis function calculates gateway's peer
> datapaths to figure our which chassis should be add in bfd_chassis.
> But in most circumstance, a gateway's peer datapath has NO chassis.
> So it may disable BFD on some tunnel ports. Then a port on a remote
> ovs cannot send packet out because it believes all remote gateway are
> down.
>
> This patch will go though whole graph and visit all datapath's port
> which has connect with gateway.
>
> Signed-off-by: Zhenyu Gao <sysugaozhe...@gmail.com>
> ---
>  ovn/controller/bfd.c | 102 ++++++++++++++++++++++++++++++
> ++++++++++++---------
>  1 file changed, 84 insertions(+), 18 deletions(-)
>
> diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
> index d1448b1..dccfc98 100644
> --- a/ovn/controller/bfd.c
> +++ b/ovn/controller/bfd.c
> @@ -100,6 +100,88 @@ bfd_calculate_active_tunnels(const struct
> ovsrec_bridge *br_int,
>      }
>  }
>
> +struct local_datapath_node {
> +    struct ovs_list node;
> +    struct local_datapath *dp;
> +};
> +
> +static void
> +bfd_travel_gw_related_chassis(struct local_datapath *dp,
> +                              const struct hmap *local_datapaths,
> +                              struct ovsdb_idl_index_cursor *cursor,
> +                              struct sbrec_port_binding *lpval,
> +                              struct sset *bfd_chassis)
> +{
> +    struct ovs_list dp_list;
> +    const struct sbrec_port_binding *pb;
> +    struct sset visited_dp = SSET_INITIALIZER(&visited_dp);
> +    const char *dp_key;
> +    struct local_datapath_node *dp_binding;
> +
> +    if (!(dp_key = smap_get(&dp->datapath->external_ids,
> "logical-router")) &&
> +        !(dp_key = smap_get(&dp->datapath->external_ids,
> "logical-switch"))) {
> +        VLOG_INFO("datapath has no uuid, cannot travel graph");
> +        return;
> +    }
> +
> +    sset_add(&visited_dp, dp_key);
> +
> +    ovs_list_init(&dp_list);
> +    dp_binding = xmalloc(sizeof *dp_binding);
> +    dp_binding->dp = dp;
> +    ovs_list_push_back(&dp_list, &dp_binding->node);
> +
> +    /*
> +     * Go through whole graph to figure out all chassis which may deliver
> +     * packetsto gateway. */
> +    do {
> +        dp_binding = CONTAINER_OF(ovs_list_pop_front(&dp_list),
> +                                  struct local_datapath_node, node);
> +        dp = dp_binding->dp;
> +        free(dp_binding);
> +        for (size_t i = 0; i < dp->n_peer_dps; i++) {
> +            const struct sbrec_datapath_binding *pdp = dp->peer_dps[i];
> +            if (!pdp) {
> +                continue;
> +            }
> +
> +            if (!(dp_key = smap_get(&pdp->external_ids,
> "logical-router")) &&
> +                !(dp_key = smap_get(&pdp->external_ids,
> "logical-switch"))) {
> +                continue;
> +            }
> +
> +            if (sset_contains(&visited_dp, dp_key)) {
> +                continue;
> +            }
> +
> +            sset_add(&visited_dp, dp_key);
> +
> +            struct hmap_node *node = hmap_first_with_hash(local_
> datapaths,
> +
> pdp->tunnel_key);
> +            if (!node) {
> +                continue;
> +            }
> +
> +            dp_binding = xmalloc(sizeof *dp_binding);
> +            dp_binding->dp = CONTAINER_OF(node, struct local_datapath,
> +                                          hmap_node);
> +            ovs_list_push_back(&dp_list, &dp_binding->node);
> +
> +            sbrec_port_binding_index_set_datapath(lpval, pdp);
> +            SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, cursor, lpval) {
> +                if (pb->chassis) {
> +                    const char *chassis_name = pb->chassis->name;
> +                    if (chassis_name) {
> +                        sset_add(bfd_chassis, chassis_name);
> +                    }
> +                }
> +            }
> +        }
> +    } while (!ovs_list_is_empty(&dp_list));
> +
> +    sset_destroy(&visited_dp);
> +}
> +
>  static void
>  bfd_calculate_chassis(struct controller_ctx *ctx,
>                        const struct sbrec_chassis *our_chassis,
> @@ -155,24 +237,8 @@ bfd_calculate_chassis(struct controller_ctx *ctx,
>              }
>          }
>          if (our_chassis_is_gw_for_dp) {
> -            for (size_t i = 0; i < dp->n_peer_dps; i++) {
> -                const struct sbrec_datapath_binding *pdp =
> dp->peer_dps[i];
> -                if (!pdp) {
> -                    continue;
> -                }
> -
> -                sbrec_port_binding_index_set_datapath(lpval, pdp);
> -                SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, &cursor, lpval) {
> -                    if (pb->chassis) {
> -                        /* Gateway node has to enable bfd to all nodes
> hosting
> -                         * connected network ports */
> -                        const char *chassis_name = pb->chassis->name;
> -                        if (chassis_name) {
> -                            sset_add(bfd_chassis, chassis_name);
> -                        }
> -                    }
> -                }
> -            }
> +            bfd_travel_gw_related_chassis(dp, local_datapaths, &cursor,
> +                                          lpval, bfd_chassis);
>          }
>      }
>      sbrec_port_binding_index_destroy_row(lpval);
> --
> 1.8.3.1
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to