Thanks for the patch! I applied this to master and branch-2.8.
On Wed, Aug 23, 2017 at 5:35 AM, Anil Venkata <anilvenk...@redhat.com> wrote: > Reviewed change and it looks fine. Also tested the test case. > > On Sun, Aug 20, 2017 at 8:07 PM, Zhenyu Gao <sysugaozhe...@gmail.com> wrote: >> >> The bfd_calculate_chassis function calculates gateway's peer datapaths >> to figure out which tunnel's BFD should be enabled to from the current >> chassis. >> Existing algorithm only calculats peer datapaths at one hop, but multiple >> logical switches and E/W routers could be in the path, making several hops >> which were not considered on the calculation. >> It may disable BFD on some gw's tunnel ports. Then a port on a remote ovs >> cannot send packet out because it believes all remote gateways are down. >> >> This patch will go through whole graph and visit all datapath's port >> which has connection with gateways. >> >> Signed-off-by: Zhenyu Gao <sysugaozhe...@gmail.com> >> --- >> >> v1->v2: Address Miguel's comments and add ovn test >> >> ovn/controller/bfd.c | 102 +++++++++++++++++++++----- >> tests/ovn.at | 203 >> ++++++++++++++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 286 insertions(+), 19 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); >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 7d81678..5060b5d 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -6924,7 +6924,7 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) >> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> AT_CLEANUP >> >> -AT_SETUP([ovn -- packet test with HA distributed router gateway port]) >> +AT_SETUP([ovn -- 4 HV, 1 LS, 1 LR, packet test with HA distributed router >> gateway port]) >> AT_SKIP_IF([test $HAVE_PYTHON = no]) >> ovn_start >> >> @@ -7107,6 +7107,207 @@ test_ip_packet gw2 gw1 >> OVN_CLEANUP([hv1],[gw1],[gw2],[ext1]) >> AT_CLEANUP >> >> +AT_SETUP([ovn -- 4 HV, 3 LS, 2 LR, packet test with HA distributed router >> gateway port]) >> +AT_SKIP_IF([test $HAVE_PYTHON = no]) >> +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 >> +ovs-vsctl -- add-port br-int hv1-vif1 -- \ >> + set interface hv1-vif1 external-ids:iface-id=foo1 \ >> + options:tx_pcap=hv1/vif1-tx.pcap \ >> + options:rxq_pcap=hv1/vif1-rx.pcap \ >> + ofport-request=1 >> + >> +sim_add gw1 >> +as gw1 >> +ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.2 >> + >> +sim_add gw2 >> +as gw2 >> +ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.4 >> + >> +sim_add ext1 >> +as ext1 >> +ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.3 >> +ovs-vsctl -- add-port br-int ext1-vif1 -- \ >> + set interface ext1-vif1 external-ids:iface-id=outside1 \ >> + options:tx_pcap=ext1/vif1-tx.pcap \ >> + options:rxq_pcap=ext1/vif1-rx.pcap \ >> + ofport-request=1 >> + >> +# Pre-populate the hypervisors' ARP tables so that we don't lose any >> +# packets for ARP resolution (native tunneling doesn't queue packets >> +# for ARP resolution). >> +ovn_populate_arp >> + >> +ovn-nbctl create Logical_Router name=R0 >> +ovn-nbctl create Logical_Router name=R1 >> + >> +ovn-nbctl ls-add foo >> +ovn-nbctl ls-add join >> +ovn-nbctl ls-add alice >> +ovn-nbctl ls-add outside >> + >> +#Connect foo to R0 >> +ovn-nbctl lrp-add R0 R0-foo 00:00:01:01:02:03 192.168.1.1/24 >> +ovn-nbctl lsp-add foo foo-R0 -- set Logical_Switch_Port foo-R0 \ >> + type=router options:router-port=R0-foo \ >> + -- lsp-set-addresses foo-R0 router >> + >> +#Connect R0 to join >> +ovn-nbctl lrp-add R0 R0-join 00:00:0d:01:02:03 100.60.1.1/24 >> +ovn-nbctl lsp-add join join-R0 -- set Logical_Switch_Port join-R0 \ >> + type=router options:router-port=R0-join \ >> + -- lsp-set-addresses join-R0 router >> + >> +#Connect join to R1 >> +ovn-nbctl lrp-add R1 R1-join 00:00:0e:01:02:03 100.60.1.2/24 >> +ovn-nbctl lsp-add join join-R1 -- set Logical_Switch_Port join-R1 \ >> + type=router options:router-port=R1-join \ >> + -- lsp-set-addresses join-R1 router >> + >> +#add route rules >> +ovn-nbctl lr-route-add R0 0.0.0.0/0 100.60.1.2 >> +ovn-nbctl lr-route-add R1 192.168.0.0/16 100.60.1.1 >> + >> +# Connect alice to R1 as distributed router gateway port on gw1 >> +ovn-nbctl lrp-add R1 alice 00:00:02:01:02:03 172.16.1.1/24 >> + >> +ovn-nbctl \ >> + --id=@gc0 create Gateway_Chassis name=alice_gw1 \ >> + chassis_name=gw1 \ >> + priority=20 -- \ >> + --id=@gc1 create Gateway_Chassis name=alice_gw2 \ >> + chassis_name=gw2 \ >> + priority=10 -- \ >> + set Logical_Router_Port alice 'gateway_chassis=[@gc0,@gc1]' >> + >> +ovn-nbctl lsp-add alice rp-alice -- set Logical_Switch_Port rp-alice \ >> + type=router options:router-port=alice \ >> + -- lsp-set-addresses rp-alice router >> + >> +# Create logical port foo1 in foo >> +ovn-nbctl lsp-add foo foo1 \ >> +-- lsp-set-addresses foo1 "f0:00:00:01:02:03 192.168.1.2" >> + >> +# Create logical port outside1 in outside >> +ovn-nbctl lsp-add outside outside1 \ >> +-- lsp-set-addresses outside1 "f0:00:00:01:02:04 172.16.1.3" >> + >> +# Create localnet port in alice >> +ovn-nbctl lsp-add alice ln-alice >> +ovn-nbctl lsp-set-addresses ln-alice unknown >> +ovn-nbctl lsp-set-type ln-alice localnet >> +ovn-nbctl lsp-set-options ln-alice network_name=phys >> + >> +# Create localnet port in outside >> +ovn-nbctl lsp-add outside ln-outside >> +ovn-nbctl lsp-set-addresses ln-outside unknown >> +ovn-nbctl lsp-set-type ln-outside localnet >> +ovn-nbctl lsp-set-options ln-outside network_name=phys >> + >> +# Create bridge-mappings on gw1, gw2 and ext1, hv1 doesn't need >> +# mapping to the external network, is the one generating packets >> +as gw1 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys >> +as gw2 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys >> +as ext1 ovs-vsctl set open . >> external-ids:ovn-bridge-mappings=phys:br-phys >> + >> +AT_CHECK([ovn-nbctl --timeout=3 --wait=sb sync], [0], [ignore]) >> + >> +# Allow some time for ovn-northd and ovn-controller to catch up. >> +# XXX This should be more systematic. >> +sleep 2 >> + >> +ip_to_hex() { >> + printf "%02x%02x%02x%02x" "$@" >> +} >> + >> +reset_pcap_file() { >> + local iface=$1 >> + local pcap_file=$2 >> + ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ >> +options:rxq_pcap=dummy-rx.pcap >> + rm -f ${pcap_file}*.pcap >> + ovs-vsctl -- set Interface $iface >> options:tx_pcap=${pcap_file}-tx.pcap \ >> +options:rxq_pcap=${pcap_file}-rx.pcap >> +} >> + >> +test_ip_packet() >> +{ >> + local active_gw=$1 >> + local backup_gw=$2 >> + >> + # Send ip packet between foo1 and outside1 >> + src_mac="f00000010203" # foo1 mac >> + dst_mac="000001010203" # foo-R0 mac (internal router leg) >> + src_ip=`ip_to_hex 192 168 1 2` >> + dst_ip=`ip_to_hex 172 16 1 3` >> + >> packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 >> + >> + # ARP request packet to expect at outside1 >> + >> #arp_request=ffffffffffff${src_mac}08060001080006040001${src_mac}${src_ip}000000000000${dst_ip} >> + >> + as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet >> + >> + # Send ARP reply from outside1 back to the router >> + # XXX: note, we could avoid this if we plug this port into a netns >> + # and setup the IP address into the port, so the kernel would simply >> reply >> + src_mac="000002010203" >> + reply_mac="f00000010204" >> + dst_ip=`ip_to_hex 172 16 1 3` >> + src_ip=`ip_to_hex 172 16 1 1` >> + >> arp_reply=${src_mac}${reply_mac}08060001080006040002${reply_mac}${dst_ip}${src_mac}${src_ip} >> + >> + as ext1 ovs-appctl netdev-dummy/receive ext1-vif1 $arp_reply >> + >> + # Packet to Expect at ext1 chassis, outside1 port >> + src_mac="000002010203" >> + dst_mac="f00000010204" >> + src_ip=`ip_to_hex 192 168 1 2` >> + dst_ip=`ip_to_hex 172 16 1 3` >> + >> expected=${dst_mac}${src_mac}08004500001c000000003e110200${src_ip}${dst_ip}0035111100080000 >> + echo $expected > ext1-vif1.expected >> + >> + as $active_gw reset_pcap_file br-phys_n1 $active_gw/br-phys_n1 >> + as $backup_gw reset_pcap_file br-phys_n1 $backup_gw/br-phys_n1 >> + as ext1 reset_pcap_file ext1-vif1 ext1/vif1 >> + >> + # Resend packet from foo1 to outside1 >> + as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet >> + >> + sleep 1 >> + >> + OVN_CHECK_PACKETS([ext1/vif1-tx.pcap], [ext1-vif1.expected]) >> + $PYTHON "$top_srcdir/utilities/ovs-pcap.in" >> $active_gw/br-phys_n1-tx.pcap > packets >> + AT_CHECK([grep $expected packets | sort], [0], [expout]) >> + $PYTHON "$top_srcdir/utilities/ovs-pcap.in" >> $backup_gw/br-phys_n1-tx.pcap > packets >> + AT_CHECK([grep $expected packets | sort], [0], []) >> +} >> + >> +test_ip_packet gw1 gw2 >> + >> +ovn-nbctl --timeout=3 --wait=hv \ >> + --id=@gc0 create Gateway_Chassis name=alice_gw1 \ >> + chassis_name=gw1 \ >> + priority=10 -- \ >> + --id=@gc1 create Gateway_Chassis name=alice_gw2 \ >> + chassis_name=gw2 \ >> + priority=20 -- \ >> + set Logical_Router_Port alice 'gateway_chassis=[@gc0,@gc1]' >> + >> +test_ip_packet gw2 gw1 >> + >> +OVN_CLEANUP([hv1],[gw1],[gw2],[ext1]) >> +AT_CLEANUP >> + >> AT_SETUP([ovn -- 1 LR with distributed router gateway port]) >> AT_SKIP_IF([test $HAVE_PYTHON = no]) >> ovn_start >> -- >> 1.8.3.1 >> > -- Russell Bryant _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev