Thanks for the suggestion. A testcase would be add in ovn testings. But I am not familiar with ovn test and busy on other stuff now. Since this issue affects many consumers who try to use HA gateways. I prefer to push this fix in ovs master first, then we have more time to make a testcase for it.
Thanks Zhenyu Gao 2017-08-18 1:21 GMT+08:00 Anil Venkata <anilvenk...@redhat.com>: > 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_dat >> apaths, >> + >> 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