> On Tue, Jan 5, 2021 at 11:26 PM Lorenzo Bianconi > <[email protected]> wrote: > > > > Introduce the bfd reference in logical_router_static_router table > > in order to check if the next-hop is properly running using the BFD > > protocol. The CMS is supposed to populate bfd column with the proper > > reference otherwise the BFD status is set to admin_down. > > Add BFD tests in system-ovn.at. > > > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > Hi Lorenzo, > > I noticed a couple of issues. > > With a gateway router configured with a static route and BFD, the > status of the BFD never comes up. > The BFD goes to up state for a few seconds and then it's down all the time. > > I saw the similar situation with using a distributed router with a > gateway router port configured with 3 gateway chassis. > When I move the cr-xxx redirect-chassis port binding to another > gateway chassis, I see the same issue. It doesn't > happens 100% of the time though. > > The status in the BFD packet shows "up". I think there is some issue > in the pinctrl code which updates > the SB DB status.
Hi Numan, ack, thx for spotting this issue (gw-router use-case was not covered properly). I will fix it in v8 and I will add some unit-tests for it. Regards, Lorenzo > > Thanks > Numan > > > --- > > NEWS | 3 +- > > northd/ovn-northd.c | 45 ++++++++++++---- > > ovn-nb.ovsschema | 6 ++- > > ovn-nb.xml | 7 +++ > > tests/atlocal.in | 3 ++ > > tests/ovn-nbctl.at | 8 ++- > > tests/system-ovn.at | 126 ++++++++++++++++++++++++++++++++++++++++++++ > > 7 files changed, 185 insertions(+), 13 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index f7198d2a2..d6ac4e736 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -1,7 +1,8 @@ > > Post-v20.12.0 > > ------------------------- > > - Support ECMP multiple nexthops for reroute router policies. > > - - BFD protocol support according to RFC880 [0]. IPv6 is not suported > > yet. > > + - BFD protocol support according to RFC880 [0]. Introduce next-hop BFD > > + availability check for OVN static routes. IPv6 is not suported yet. > > [0] https://tools.ietf.org/html/rfc5880) > > > > OVN v20.12.0 - xx xxx xxxx > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index e72693670..f4103d4ec 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -7948,7 +7948,8 @@ route_hash(struct parsed_route *route) > > * Otherwise return NULL. */ > > static struct parsed_route * > > parsed_routes_add(struct ovs_list *routes, > > - const struct nbrec_logical_router_static_route *route) > > + const struct nbrec_logical_router_static_route *route, > > + struct hmap *bfd_connections) > > { > > /* Verify that the next hop is an IP address with an all-ones mask. */ > > struct in6_addr nexthop; > > @@ -7989,6 +7990,25 @@ parsed_routes_add(struct ovs_list *routes, > > return NULL; > > } > > > > + const struct nbrec_bfd *nb_bt = route->bfd; > > + if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) { > > + struct bfd_entry *bfd_e; > > + > > + bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, > > + nb_bt->dst_ip); > > + if (bfd_e) { > > + bfd_e->ref = true; > > + } > > + > > + if (!strcmp(nb_bt->status, "admin_down")) { > > + nbrec_bfd_set_status(nb_bt, "down"); > > + } > > + > > + if (!strcmp(nb_bt->status, "down")) { > > + return NULL; > > + } > > + } > > + > > struct parsed_route *pr = xzalloc(sizeof *pr); > > pr->prefix = prefix; > > pr->plen = plen; > > @@ -10546,7 +10566,7 @@ build_ip_routing_flows_for_lrouter_port( > > static void > > build_static_route_flows_for_lrouter( > > struct ovn_datapath *od, struct hmap *lflows, > > - struct hmap *ports) > > + struct hmap *ports, struct hmap *bfd_connections) > > { > > if (od->nbr) { > > ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 150, > > @@ -10558,7 +10578,8 @@ build_static_route_flows_for_lrouter( > > struct ecmp_groups_node *group; > > for (int i = 0; i < od->nbr->n_static_routes; i++) { > > struct parsed_route *route = > > - parsed_routes_add(&parsed_routes, > > od->nbr->static_routes[i]); > > + parsed_routes_add(&parsed_routes, > > od->nbr->static_routes[i], > > + bfd_connections); > > if (!route) { > > continue; > > } > > @@ -11578,7 +11599,8 @@ struct lswitch_flow_build_info { > > > > static void > > build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od, > > - struct lswitch_flow_build_info > > *lsi) > > + struct lswitch_flow_build_info > > *lsi, > > + struct hmap *bfd_connections) > > { > > /* Build Logical Switch Flows. */ > > build_lswitch_lflows_pre_acl_and_acl(od, lsi->port_groups, lsi->lflows, > > @@ -11593,7 +11615,8 @@ build_lswitch_and_lrouter_iterate_by_od(struct > > ovn_datapath *od, > > build_neigh_learning_flows_for_lrouter(od, lsi->lflows, &lsi->match, > > &lsi->actions); > > build_ND_RA_flows_for_lrouter(od, lsi->lflows); > > - build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->ports); > > + build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->ports, > > + bfd_connections); > > build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, > > &lsi->actions); > > build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->ports); > > @@ -11640,7 +11663,8 @@ build_lswitch_and_lrouter_flows(struct hmap > > *datapaths, struct hmap *ports, > > struct hmap *port_groups, struct hmap > > *lflows, > > struct hmap *mcgroups, > > struct hmap *igmp_groups, > > - struct shash *meter_groups, struct hmap > > *lbs) > > + struct shash *meter_groups, struct hmap > > *lbs, > > + struct hmap *bfd_connections) > > { > > struct ovn_datapath *od; > > struct ovn_port *op; > > @@ -11665,7 +11689,7 @@ build_lswitch_and_lrouter_flows(struct hmap > > *datapaths, struct hmap *ports, > > * will move here and will be reogranized by iterator type. > > */ > > HMAP_FOR_EACH (od, key_node, datapaths) { > > - build_lswitch_and_lrouter_iterate_by_od(od, &lsi); > > + build_lswitch_and_lrouter_iterate_by_od(od, &lsi, bfd_connections); > > } > > HMAP_FOR_EACH (op, key_node, ports) { > > build_lswitch_and_lrouter_iterate_by_op(op, &lsi); > > @@ -11757,13 +11781,14 @@ build_lflows(struct northd_context *ctx, struct > > hmap *datapaths, > > struct hmap *ports, struct hmap *port_groups, > > struct hmap *mcgroups, struct hmap *igmp_groups, > > struct shash *meter_groups, > > - struct hmap *lbs) > > + struct hmap *lbs, struct hmap *bfd_connections) > > { > > struct hmap lflows = HMAP_INITIALIZER(&lflows); > > > > build_lswitch_and_lrouter_flows(datapaths, ports, > > port_groups, &lflows, mcgroups, > > - igmp_groups, meter_groups, lbs); > > + igmp_groups, meter_groups, lbs, > > + bfd_connections); > > > > /* Collecting all unique datapath groups. */ > > struct hmap dp_groups = HMAP_INITIALIZER(&dp_groups); > > @@ -12772,7 +12797,7 @@ ovnnb_db_run(struct northd_context *ctx, > > build_meter_groups(ctx, &meter_groups); > > build_bfd_table(ctx, &bfd_connections, ports); > > build_lflows(ctx, datapaths, ports, &port_groups, &mcast_groups, > > - &igmp_groups, &meter_groups, &lbs); > > + &igmp_groups, &meter_groups, &lbs, &bfd_connections); > > ovn_update_ipv6_prefix(ports); > > > > sync_address_sets(ctx); > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > > index 3b8a94276..f6e89176d 100644 > > --- a/ovn-nb.ovsschema > > +++ b/ovn-nb.ovsschema > > @@ -1,7 +1,7 @@ > > { > > "name": "OVN_Northbound", > > "version": "5.31.0", > > - "cksum": "1925435483 28290", > > + "cksum": "4128092472 28518", > > "tables": { > > "NB_Global": { > > "columns": { > > @@ -374,6 +374,10 @@ > > "min": 0, "max": 1}}, > > "nexthop": {"type": "string"}, > > "output_port": {"type": {"key": "string", "min": 0, "max": > > 1}}, > > + "bfd": {"type": {"key": {"type": "uuid", "refTable": "BFD", > > + "refType": "weak"}, > > + "min": 0, > > + "max": 1}}, > > "options": { > > "type": {"key": "string", "value": "string", > > "min": 0, "max": "unlimited"}}, > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index ab4adb814..ac3561e84 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -2644,6 +2644,13 @@ > > </p> > > </column> > > > > + <column name="bfd"> > > + <p> > > + Reference to <ref table="BFD"/> row if the route has associated a > > + BFD session > > + </p> > > + </column> > > + > > <column name="external_ids" key="ic-learned-route"> > > <code>ovn-ic</code> populates this key if the route is learned from > > the > > global <ref db="OVN_IC_Southbound"/> database. In this case the > > value > > diff --git a/tests/atlocal.in b/tests/atlocal.in > > index d9a4c91d4..5ebc8e117 100644 > > --- a/tests/atlocal.in > > +++ b/tests/atlocal.in > > @@ -181,6 +181,9 @@ fi > > # Set HAVE_DIBBLER-SERVER > > find_command dibbler-server > > > > +# Set HAVE_BFDD_BEACON > > +find_command bfdd-beacon > > + > > # Turn off proxies. > > unset http_proxy > > unset https_proxy > > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > > index 01edfcbc1..2827b223c 100644 > > --- a/tests/ovn-nbctl.at > > +++ b/tests/ovn-nbctl.at > > @@ -1617,7 +1617,13 @@ IPv6 Routes > > 2001:db8::/64 2001:db8:0:f102::1 dst-ip lp0 > > 2001:db8:1::/64 2001:db8:0:f103::1 dst-ip > > ::/0 2001:db8:0:f101::1 dst-ip > > -])]) > > +]) > > + > > +AT_CHECK([ovn-nbctl lrp-add lr0 lr0-p0 00:00:01:01:02:03 192.168.10.1/24]) > > +bfd_uuid=$(ovn-nbctl create bfd logical_port=lr0-p0 dst_ip=100.0.0.50 > > status=down min_tx=250 min_rx=250 detect_mult=10) > > +AT_CHECK([ovn-nbctl lr-route-add lr0 100.0.0.0/24 192.168.0.1]) > > +route_uuid=$(fetch_column nb:logical_router_static_route _uuid > > ip_prefix="100.0.0.0/24") > > +AT_CHECK([ovn-nbctl set logical_router_static_route $route_uuid > > bfd=$bfd_uuid])]) > > > > dnl --------------------------------------------------------------------- > > > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index 1e73001ab..bb908123b 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -5531,3 +5531,129 @@ as > > OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d > > /.*terminating with signal 15.*/d"]) > > AT_CLEANUP > > + > > +AT_SETUP([ovn -- BFD]) > > +AT_SKIP_IF([test $HAVE_BFDD_BEACON = no]) > > +AT_SKIP_IF([test $HAVE_TCPDUMP = no]) > > +AT_KEYWORDS([ovn-bfd]) > > + > > +ovn_start > > +OVS_TRAFFIC_VSWITCHD_START() > > + > > +ADD_BR([br-int]) > > +ADD_BR([br-ext]) > > + > > +check ovs-ofctl add-flow br-ext action=normal > > +# Set external-ids in br-int needed for ovn-controller > > +check ovs-vsctl \ > > + -- set Open_vSwitch . external-ids:system-id=hv1 \ > > + -- set Open_vSwitch . > > external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ > > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ > > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ > > + -- set bridge br-int fail-mode=secure > > other-config:disable-in-band=true > > + > > +# Start ovn-controller > > +start_daemon ovn-controller > > + > > +check ovn-nbctl lr-add R1 > > + > > +check ovn-nbctl ls-add sw0 > > +check ovn-nbctl ls-add sw1 > > +check ovn-nbctl ls-add public > > + > > +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24 > > +check ovn-nbctl lrp-add R1 rp-sw1 00:00:03:01:02:03 192.168.2.1/24 > > +check ovn-nbctl lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24 \ > > + -- lrp-set-gateway-chassis rp-public hv1 > > + > > +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \ > > + type=router options:router-port=rp-sw0 \ > > + -- lsp-set-addresses sw0-rp router > > +check ovn-nbctl lsp-add sw1 sw1-rp -- set Logical_Switch_Port sw1-rp \ > > + type=router options:router-port=rp-sw1 \ > > + -- lsp-set-addresses sw1-rp router > > + > > +check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port > > public-rp \ > > + type=router options:router-port=rp-public \ > > + -- lsp-set-addresses public-rp router > > + > > +ADD_NAMESPACES(sw01) > > +ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \ > > + "192.168.1.1") > > +check ovn-nbctl lsp-add sw0 sw01 \ > > + -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2" > > + > > +ADD_NAMESPACES(sw11) > > +ADD_VETH(sw11, sw11, br-int, "192.168.2.2/24", "f0:00:00:02:02:03", \ > > + "192.168.2.1") > > +check ovn-nbctl lsp-add sw1 sw11 \ > > + -- lsp-set-addresses sw11 "f0:00:00:02:02:03 192.168.2.2" > > + > > +ADD_NAMESPACES(server) > > +NS_CHECK_EXEC([server], [ip link set dev lo up]) > > +ADD_VETH(s1, server, br-ext, "172.16.1.50/24", "f0:00:00:01:02:05", \ > > + "172.16.1.1") > > + > > +AT_CHECK([ovs-vsctl set Open_vSwitch . > > external-ids:ovn-bridge-mappings=phynet:br-ext]) > > +check ovn-nbctl lsp-add public public1 \ > > + -- lsp-set-addresses public1 unknown \ > > + -- lsp-set-type public1 localnet \ > > + -- lsp-set-options public1 network_name=phynet > > + > > +NS_CHECK_EXEC([server], [bfdd-beacon --listen=172.16.1.50], [0]) > > +NS_CHECK_EXEC([server], [bfdd-control allow 172.16.1.1], [0], [dnl > > +Allowing connections from 172.16.1.1 > > +]) > > + > > +uuid=$(ovn-nbctl create bfd logical_port=rp-public dst_ip=172.16.1.50 > > min_tx=250 min_rx=250 detect_mult=10) > > +check ovn-nbctl lr-route-add R1 100.0.0.0/8 172.16.1.50 > > +route_uuid=$(fetch_column nb:logical_router_static_route _uuid > > ip_prefix="100.0.0.0/8") > > +check ovn-nbctl set logical_router_static_route $route_uuid bfd=$uuid > > +check ovn-nbctl --wait=hv sync > > + > > +wait_column "up" nb:bfd status logical_port=rp-public > > +OVS_WAIT_UNTIL([ovn-sbctl dump-flows R1 | grep 'match=(ip4.dst == > > 100.0.0.0/8)' | grep -q 172.16.1.50]) > > + > > +# un-associate the bfd connection and the static route > > +check ovn-nbctl clear logical_router_static_route $route_uuid bfd > > +wait_column "admin_down" nb:bfd status logical_port=rp-public > > +NS_CHECK_EXEC([server], [tcpdump -nni s1 udp port 3784 -Q in > bfd.pcap &]) > > +sleep 5 > > +kill $(pidof tcpdump) > > +AT_CHECK([grep -qi bfd bfd.pcap],[1]) > > + > > +# restart the connection > > +check ovn-nbctl set logical_router_static_route $route_uuid bfd=$uuid > > +wait_column "up" nb:bfd status logical_port=rp-public > > + > > +# stop bfd endpoint > > +NS_CHECK_EXEC([server], [bfdd-control stop], [0], [dnl > > +stopping > > +]) > > + > > +wait_column "down" nb:bfd status logical_port=rp-public > > +OVS_WAIT_UNTIL([test "$(ovn-sbctl dump-flows R1 | grep 'match=(ip4.dst == > > 100.0.0.0/8)' | grep 172.16.1.50)" = ""]) > > + > > +# remove bfd entry > > +ovn-nbctl destroy bfd $uuid > > +check_row_count bfd 0 > > +NS_CHECK_EXEC([server], [tcpdump -nni s1 udp port 3784 -Q in > bfd.pcap &]) > > +sleep 5 > > +kill $(pidof tcpdump) > > +AT_CHECK([grep -qi bfd bfd.pcap],[1]) > > + > > +kill $(pidof ovn-controller) > > + > > +as ovn-sb > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > + > > +as ovn-nb > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > + > > +as northd > > +OVS_APP_EXIT_AND_WAIT([ovn-northd]) > > + > > +as > > +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d > > +/.*terminating with signal 15.*/d"]) > > +AT_CLEANUP > > -- > > 2.29.2 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
