On Fri, Aug 18, 2017 at 3:24 AM, Gao Zhenyu <sysugaozhe...@gmail.com> wrote:
> 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.
>
I know your feeling Gao, but, IMO it's better if we put the tests and code
together, it's has a couple of benefits:
1) Your expected behaviour can more easily be understood from the patch
itself.
2) Your behaviour changes are locked in, because if later patches break
them the test will fail.
Anil, I'll be away for the next two weeks, but could you give Gao a hand to
write the tests quickly?
Then we need to backport this to 2.8.0
>
> 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
