| Message ID | 20170820143731.8945-1-sysugaozhenyu@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Russell Bryant |
| Headers | show |
Reviewed change and it looks fine. Also tested the test case. On Sun, Aug 20, 2017 at 8:07 PM, Zhenyu Gao <sysugaozhenyu@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 <sysugaozhenyu@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 > >
Thanks for the patch! I applied this to master and branch-2.8. On Wed, Aug 23, 2017 at 5:35 AM, Anil Venkata <anilvenkata@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 <sysugaozhenyu@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 <sysugaozhenyu@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 >> >
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
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 <sysugaozhenyu@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(-)