Reviewed change and it looks fine. Also tested the test case. On Sun, Aug 20, 2017 at 8:07 PM, Zhenyu Gao <[email protected]> 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 <[email protected]> > --- > > 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 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
