controller: Flush dynamic routes learned in uninterested LRP.

If logical router has more than one LRP as gateway router port
and dynamic routing configured, dynamic-routing-port-name could be
used to specify the LRP that will be used to dynamic routing.
However, if all LRPs learning routes, routes from LRP without
dynamic-routing-port-name must be flushed.
This happens when LRPs are scheduled in the same chassis.

Signed-off-by: Lucas Vargas Dias <lucas.vdias@magalu.cloud>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit b93685b)

Author: lucassdiass

controller/ovn-controller.c

@@ -5459,10 +5459,24 @@ en_route_exchange_run(struct engine_node *node, void *data)
         return EN_STALE;
     }
 
+    const struct ovsrec_open_vswitch_table *ovs_table =
+        EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
+    const char *chassis_id = get_ovs_chassis_id(ovs_table);
+    ovs_assert(chassis_id);
+
+    struct ovsdb_idl_index *sbrec_chassis_by_name =
+        engine_ovsdb_node_get_index(
+                engine_get_input("SB_chassis", node),
+                "name");
+    const struct sbrec_chassis *chassis
+        = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
+    ovs_assert(chassis);
+
     struct route_exchange_ctx_in r_ctx_in = {
         .ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn,
         .sbrec_learned_route_by_datapath = sbrec_learned_route_by_datapath,
         .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
+        .chassis = chassis,
         .announce_routes = &route_data->announce_routes,
     };
     struct route_exchange_ctx_out r_ctx_out = {
@@ -6755,6 +6769,8 @@ main(int argc, char *argv[])
     engine_add_input(&en_route, &en_sb_advertised_route,
                      route_sb_advertised_route_data_handler);
 
+    engine_add_input(&en_route_exchange, &en_ovs_open_vswitch, NULL);
+    engine_add_input(&en_route_exchange, &en_sb_chassis, NULL);
     engine_add_input(&en_route_exchange, &en_route, NULL);
     engine_add_input(&en_route_exchange, &en_sb_learned_route,
                      engine_noop_handler);

controller/route-exchange.c

@@ -21,6 +21,7 @@
 #include <net/if.h>
 #include <stdbool.h>
 
+#include "hmapx.h"
 #include "openvswitch/poll-loop.h"
 #include "openvswitch/vlog.h"
 #include "openvswitch/list.h"
@@ -86,7 +87,7 @@ maintained_route_table_add(uint32_t table_id)
     hmap_insert(&_maintained_route_tables, &mrt->node, hash);
 }
 
-static void
+static struct route_entry *
 route_add_entry(struct hmap *routes,
                 const struct sbrec_learned_route *sb_route,
                 bool stale)
@@ -102,6 +103,7 @@ route_add_entry(struct hmap *routes,
     hash = hash_string(sb_route->ip_prefix, hash);
 
     hmap_insert(routes, &route_e->hmap_node, hash);
+    return route_e;
 }
 
 static struct route_entry *
@@ -144,28 +146,80 @@ sb_sync_learned_routes(const struct vector *learned_routes,
                        struct ovsdb_idl_txn *ovnsb_idl_txn,
                        struct ovsdb_idl_index *sbrec_port_binding_by_name,
                        struct ovsdb_idl_index *sbrec_learned_route_by_datapath,
-                       bool *sb_changes_pending)
+                       bool *sb_changes_pending,
+                       const struct sbrec_chassis *chassis)
 {
     struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes);
     const struct sbrec_learned_route *sb_route;
-    struct route_entry *route_e;
+    struct hmapx lrp_with_dr_port_name =
+        HMAPX_INITIALIZER(&lrp_with_dr_port_name);
 
     struct sbrec_learned_route *filter =
         sbrec_learned_route_index_init_row(sbrec_learned_route_by_datapath);
     sbrec_learned_route_index_set_datapath(filter, datapath);
     SBREC_LEARNED_ROUTE_FOR_EACH_EQUAL (sb_route, filter,
                                         sbrec_learned_route_by_datapath) {
+        const struct sbrec_port_binding *cr_pb =
+            lport_get_cr_port(sbrec_port_binding_by_name,
+                              sb_route->logical_port, NULL);
+        struct route_entry *route_e = NULL;
+
+        /* Collect the set of unique logical ports we learned routes on. The
+         * (potentially expensive) dynamic-routing-port-name lookups are
+         * postponed until after the loop so that they are performed once per
+         * logical port instead of once per learned route. */
+        hmapx_add(&lrp_with_dr_port_name,
+                  CONST_CAST(void *, sb_route->logical_port));
+
+        if (sb_route->logical_port->chassis == chassis ||
+            (cr_pb && cr_pb->chassis == chassis)) {
+            route_e = route_add_entry(&sync_routes, sb_route, false);
+        }
+
         /* If the port is not local we don't care about it.
          * Some other ovn-controller will handle it.
          * We may not use smap_get since the value might be validly NULL. */
         if (!smap_get_node(bound_ports,
                            sb_route->logical_port->logical_port)) {
             continue;
         }
+        if (route_e) {
+            route_e->stale = true;
+            continue;
+        }
         route_add_entry(&sync_routes, sb_route, true);
     }
     sbrec_learned_route_index_destroy_row(filter);
 
+    /* Drop the logical ports that don't have a dynamic-routing-port-name set,
+     * either directly or via their distributed gateway port. */
+    struct hmapx_node *lrp_node;
+    HMAPX_FOR_EACH_SAFE (lrp_node, &lrp_with_dr_port_name) {
+        const struct sbrec_port_binding *lrp = lrp_node->data;
+        const struct sbrec_port_binding *cr_pb =
+            lport_get_cr_port(sbrec_port_binding_by_name, lrp, NULL);
+        const char *dynamic_routing_port_name =
+            smap_get(&lrp->options, "dynamic-routing-port-name");
+        if (!dynamic_routing_port_name && cr_pb) {
+            dynamic_routing_port_name =
+                smap_get(&cr_pb->options, "dynamic-routing-port-name");
+        }
+        if (!dynamic_routing_port_name) {
+            hmapx_delete(&lrp_with_dr_port_name, lrp_node);
+        }
+    }
+
+    if (!hmapx_is_empty(&lrp_with_dr_port_name)) {
+        struct route_entry *route_e;
+        HMAP_FOR_EACH (route_e, hmap_node, &sync_routes) {
+            if (!hmapx_contains(&lrp_with_dr_port_name,
+                                route_e->sb_route->logical_port)) {
+                route_e->stale = true;
+            }
+        }
+    }
+    hmapx_destroy(&lrp_with_dr_port_name);
+
     struct re_nl_received_route_node *learned_route;
     VECTOR_FOR_EACH_PTR (learned_routes, learned_route) {
         char *ip_prefix = normalize_v46_prefix(&learned_route->prefix,
@@ -187,8 +241,9 @@ sb_sync_learned_routes(const struct vector *learned_routes,
             if (!logical_port) {
                 continue;
             }
-            route_e = route_lookup(&sync_routes, datapath,
-                                   logical_port, ip_prefix, nexthop);
+            struct route_entry *route_e =
+                route_lookup(&sync_routes, datapath,
+                             logical_port, ip_prefix, nexthop);
             if (route_e) {
                 route_e->stale = false;
             } else {
@@ -209,6 +264,7 @@ sb_sync_learned_routes(const struct vector *learned_routes,
         free(nexthop);
     }
 
+    struct route_entry *route_e;
     HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) {
         if (route_e->stale) {
             sbrec_learned_route_delete(route_e->sb_route);
@@ -296,7 +352,8 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in,
                                &ad->bound_ports, r_ctx_in->ovnsb_idl_txn,
                                r_ctx_in->sbrec_port_binding_by_name,
                                r_ctx_in->sbrec_learned_route_by_datapath,
-                               &r_ctx_out->sb_changes_pending);
+                               &r_ctx_out->sb_changes_pending,
+                               r_ctx_in->chassis);
 
         route_table_add_watch_request(&r_ctx_out->route_table_watches,
                                       table_id);

controller/route-exchange.h

@@ -24,6 +24,7 @@ struct route_exchange_ctx_in {
     struct ovsdb_idl_txn *ovnsb_idl_txn;
     struct ovsdb_idl_index *sbrec_port_binding_by_name;
     struct ovsdb_idl_index *sbrec_learned_route_by_datapath;
+    const struct sbrec_chassis *chassis;
 
     /* Contains struct advertise_datapath_entry */
     const struct hmap *announce_routes;

tests/multinode.at

@@ -2989,6 +2989,148 @@ OVS_WAIT_UNTIL([m_as ovn-gw-2 ip netns exec frr-ns ping -W 1 -c 1 172.16.10.2])
 
 AT_CLEANUP
 
+AT_SETUP([ovn multinode dynamic-routing - BGP learned routes with router filter name and multiple DGPs])
+
+# This is the multinode counterpart of the system test
+# "dynamic-routing - BGP learned routes with router filter name and multiple
+# DGPs": a single logical router with dynamic routing enabled has two
+# distributed gateway ports scheduled on different chassis.  Both LRPs are used
+# for dynamic routing and each has its own dynamic-routing-port-name filter
+# resolving to the interface local to the chassis hosting the port.
+#
+# Each chassis learns a route for the same prefix from its own VRF, so both
+# gateway ports learn it.  Because every LRP has a dynamic-routing-port-name
+# that resolves to a local interface, neither route is flushed: the route
+# learned on ovn-gw-1 (lrp-local-bgp-port) and the route learned on ovn-gw-2
+# (lrp-dgp-dummy) both remain.
+#
+# Topology:
+#
+#   public                              ls-dummy
+#   (local-bgp-port, ovn-gw-1)          (ovn-gw-2)
+#        |                                   |
+#   lrp-local-bgp-port                  lrp-dgp-dummy
+#   (DGP @ ovn-gw-1,                    (DGP @ ovn-gw-2,
+#    port-name local-bgp-port)           port-name dummy-bgp-port)
+#        \                                  /
+#         +------------- lr-frr -----------+
+#          (dynamic-routing, vrf-id $vrf)
+
+# Check that ovn-fake-multinode setup is up and running.
+check_fake_multinode_setup
+
+CHECK_VRF()
+
+# Delete the multinode NB and OVS resources before starting the test.
+cleanup_multinode_resources
+
+CHECK_VRF()
+
+vrf=1000
+
+# Create the VRF manually on both gateway chassis (dynamic-routing-maintain-vrf
+# is false, so OVN does not create it) and learn/read routes from table $vrf.
+for gw in ovn-gw-1 ovn-gw-2; do
+    check m_as $gw ip link add vrf-bgp type vrf table $vrf
+    on_exit "m_as $gw ip link del vrf-bgp"
+    check m_as $gw ip link set vrf-bgp up
+done
+
+# Create the logical router with dynamic routing in VRF $vrf.
+check multinode_nbctl lr-add lr-frr
+check multinode_nbctl set Logical_Router lr-frr   \
+    options:dynamic-routing=true                  \
+    options:dynamic-routing-vrf-id=$vrf           \
+    options:dynamic-routing-redistribute=static
+
+# The DGP that is used for dynamic routing, scheduled on ovn-gw-1.
+check multinode_nbctl lrp-add lr-frr lrp-local-bgp-port 00:00:00:00:00:03 20.0.0.3/24 \
+    -- set Logical_Router_Port lrp-local-bgp-port \
+        options:dynamic-routing-maintain-vrf=false \
+    -- set Logical_Router_Port lrp-local-bgp-port \
+        options:routing-protocol-redirect=local-bgp-port
+check multinode_nbctl lrp-set-gateway-chassis lrp-local-bgp-port ovn-gw-1
+
+check multinode_nbctl ls-add public
+check multinode_nbctl lsp-add-router-port public public-lr-frr lrp-local-bgp-port
+check multinode_nbctl lsp-add public local-bgp-port \
+    -- lsp-set-addresses local-bgp-port unknown
+
+# The second DGP used for dynamic routing, scheduled on ovn-gw-2.  bgp-dummy is
+# not an OVN logical port, so the dynamic-routing-port-name filter is resolved
+# through the dynamic-routing-port-mapping configured on ovn-gw-2 below.
+check multinode_nbctl lrp-add lr-frr lrp-dgp-dummy 00:00:00:00:00:04 20.0.1.3/24 \
+    -- set Logical_Router_Port lrp-dgp-dummy \
+        options:dynamic-routing-maintain-vrf=false
+check multinode_nbctl lrp-set-gateway-chassis lrp-dgp-dummy ovn-gw-2
+
+check multinode_nbctl ls-add ls-dummy
+check multinode_nbctl lsp-add-router-port ls-dummy lsp-dummy lrp-dgp-dummy
+
+check multinode_nbctl --wait=hv sync
+
+# Bind local-bgp-port on ovn-gw-1 and move its interface into the VRF.  The
+# dynamic-routing-port-name filter will be resolved to this interface name.
+check m_as ovn-gw-1 ovs-vsctl add-port br-int local-bgp-port \
+    -- set Interface local-bgp-port type=internal            \
+    -- set Interface local-bgp-port external_ids:iface-id=local-bgp-port
+on_exit "m_as ovn-gw-1 ovs-vsctl del-port br-int local-bgp-port"
+check m_as ovn-gw-1 ip link set local-bgp-port master vrf-bgp
+check m_as ovn-gw-1 ip link set local-bgp-port address 00:00:00:00:00:03
+check m_as ovn-gw-1 ip addr add dev local-bgp-port 20.0.0.3/24
+check m_as ovn-gw-1 ip link set local-bgp-port up
+
+# ovn-gw-2 hosts lrp-dgp-dummy.  Add an interface to its VRF so a route can be
+# present in the VRF table and get learned on this gateway port, and map the
+# dynamic-routing-port-name (dummy-bgp-port) to this interface so the filter
+# resolves locally on ovn-gw-2.
+check m_as ovn-gw-2 ip link add bgp-dummy type dummy
+on_exit "m_as ovn-gw-2 ip link del bgp-dummy"
+check m_as ovn-gw-2 ip link set bgp-dummy master vrf-bgp
+check m_as ovn-gw-2 ip addr add dev bgp-dummy 20.0.1.3/24
+check m_as ovn-gw-2 ip link set bgp-dummy up
+
+check m_as ovn-gw-2 ovs-vsctl set open . \
+    external-ids:dynamic-routing-port-mapping="dummy-bgp-port=bgp-dummy"
+on_exit "m_as ovn-gw-2 ovs-vsctl remove open . external-ids dynamic-routing-port-mapping"
+
+m_wait_for_ports_up
+check multinode_nbctl --wait=hv sync
+
+# Both gateway ports are flagged for dynamic routing.
+m_check_row_count Port_Binding 1 logical_port=cr-lrp-local-bgp-port 'options:dynamic-routing=true'
+m_check_row_count Port_Binding 1 logical_port=cr-lrp-dgp-dummy 'options:dynamic-routing=true'
+
+# Simulate a route learned via a dynamic routing protocol in each chassis' VRF
+# for the same prefix.
+check m_as ovn-gw-1 ip route add 10.10.3.0/24 via 20.0.0.25 vrf vrf-bgp proto bgp
+check m_as ovn-gw-2 ip route add 10.10.3.0/24 via 20.0.1.25 vrf vrf-bgp proto bgp
+
+# Each chassis learns the route on its own gateway port, so there are two
+# Learned_Route rows for the prefix.
+m_wait_row_count Learned_Route 2 ip_prefix=10.10.3.0/24
+
+# Configure a dynamic-routing-port-name filter on both LRPs, each resolving to
+# the interface local to the chassis hosting the port: lrp-local-bgp-port ->
+# local-bgp-port (ovn-gw-1) and lrp-dgp-dummy -> dummy-bgp-port (ovn-gw-2, via
+# the port-mapping configured above).
+check multinode_nbctl set Logical_Router_Port lrp-local-bgp-port \
+    options:dynamic-routing-port-name=local-bgp-port
+check multinode_nbctl --wait=hv set Logical_Router_Port lrp-dgp-dummy \
+    options:dynamic-routing-port-name=dummy-bgp-port
+
+local_lp=$(m_fetch_column port_binding _uuid logical_port=lrp-local-bgp-port)
+dummy_lp=$(m_fetch_column port_binding _uuid logical_port=lrp-dgp-dummy)
+
+# Both filters resolve to a local interface, so no route is flushed: the route
+# learned on ovn-gw-1 (lrp-local-bgp-port) and the route learned on ovn-gw-2
+# (lrp-dgp-dummy) both remain.
+m_wait_row_count Learned_Route 2 ip_prefix=10.10.3.0/24
+m_wait_row_count Learned_Route 1 ip_prefix=10.10.3.0/24 logical_port=$local_lp
+m_wait_row_count Learned_Route 1 ip_prefix=10.10.3.0/24 logical_port=$dummy_lp
+
+AT_CLEANUP
+
 AT_SETUP([HA: Check for missing garp on leader when BFD goes back up])
 # Network topology
 #    ┌────────────────────────────────────────────────────────────────────────────────────────────────────────┐