Blob Blame History Raw
From 986137dc1d4dc6905a7c5ab5e279856260966e12 Mon Sep 17 00:00:00 2001
Message-Id: <986137dc1d4dc6905a7c5ab5e279856260966e12.1610458802.git.lorenzo.bianconi@redhat.com>
In-Reply-To: <f21c1b7a467a691847b5552d4570af706fcc5bb0.1610458802.git.lorenzo.bianconi@redhat.com>
References: <f21c1b7a467a691847b5552d4570af706fcc5bb0.1610458802.git.lorenzo.bianconi@redhat.com>
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Fri, 8 Jan 2021 17:36:24 +0100
Subject: [PATCH 16/16] ovn: integrate bfd for static routes.

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.

Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 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/ovn-northd.at |   8 +++
 tests/system-ovn.at | 136 ++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 203 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index 85f63503e..0b4b8f4d3 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 - 18 Dec 2020
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 363bb0895..fa2bd73c3 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -7952,7 +7952,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;
@@ -7993,6 +7994,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;
@@ -9579,7 +9599,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,
@@ -9591,7 +9611,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;
             }
@@ -11571,7 +11592,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,
@@ -11591,7 +11613,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);
@@ -11655,7 +11678,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;
@@ -11682,7 +11706,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);
@@ -11780,13 +11804,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);
@@ -12795,7 +12820,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 aea932f55..29019809c 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Northbound",
     "version": "5.31.0",
-    "cksum": "1511492848 28473",
+    "cksum": "2352750632 28701",
     "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 cdc5e0f3a..105d8697e 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/ovn-northd.at b/tests/ovn-northd.at
index eee004328..91eb9a3d1 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2373,6 +2373,14 @@ check_column 1000 bfd min_tx logical_port=r0-sw1
 check_column 1000 bfd min_rx logical_port=r0-sw1
 check_column 100 bfd detect_mult logical_port=r0-sw1
 
+check ovn-nbctl lr-route-add r0 100.0.0.0/8 192.168.10.2
+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_column down bfd status logical_port=r0-sw1
+
+check ovn-nbctl clear logical_router_static_route $route_uuid bfd
+check_column admin_down bfd status logical_port=r0-sw1
+
 ovn-nbctl destroy bfd $uuid
 check_row_count bfd 2
 
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 1e73001ab..06d606166 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -5531,3 +5531,139 @@ 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
+OVS_WAIT_UNTIL([ip netns exec server bfdd-control status | grep -qi state=Down])
+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
+
+# switch to gw router configuration
+check ovn-nbctl clear logical_router_static_route $route_uuid bfd
+wait_column "admin_down" nb:bfd status logical_port=rp-public
+OVS_WAIT_UNTIL([ip netns exec server bfdd-control status | grep -qi state=Down])
+check ovn-nbctl clear logical_router_port rp-public gateway_chassis
+check ovn-nbctl set logical_router R1 options:chassis=hv1
+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