The commit in the fixes tag introduced options to map between the
system interface routes are learned on, and an LRP.

However, when an LR has LRPs with and without the option,
routes are attempted associated with LRPs without the option,
contrary to the documented behavior.

This was hidden in the original dynamic-routing - Gateway Router
test by populating all LRPs with the option, however this does
not represent a real world use case.

Ensure only LRPs with the option are considered when at least one
of LRPs in an LR has the option set.

Reported-at: https://launchpad.net/bugs/2123914
Fixes: d7d886eca553 ("controller: Support learning routes per iface.")
Signed-off-by: Frode Nordahl <fnord...@ubuntu.com>
---
 controller/route.c  | 14 ++++++++++++++
 tests/system-ovn.at |  7 ++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/controller/route.c b/controller/route.c
index 7afa2578d..56c1bd528 100644
--- a/controller/route.c
+++ b/controller/route.c
@@ -152,6 +152,7 @@ route_run(struct route_ctx_in *r_ctx_in,
 {
     struct advertise_datapath_entry *ad;
     const struct local_datapath *ld;
+    bool lr_has_port_name_filter;
     struct smap port_mapping = SMAP_INITIALIZER(&port_mapping);
 
     build_port_mapping(&port_mapping, r_ctx_in->dynamic_routing_port_mapping);
@@ -161,6 +162,7 @@ route_run(struct route_ctx_in *r_ctx_in,
             continue;
         }
         ad = NULL;
+        lr_has_port_name_filter = false;
 
         /* This is a LR datapath, find LRPs with route exchange options
          * that are bound locally. */
@@ -209,6 +211,8 @@ route_run(struct route_ctx_in *r_ctx_in,
                 /* No port-name set, so we learn routes from all ports. */
                 smap_add(&ad->bound_ports, local_peer->logical_port, "");
             } else {
+                lr_has_port_name_filter = true;
+
                 /* If a port_name is set the we filter for the name as set in
                  * the port-mapping or the interface name of the local
                  * binding. If the port is not in the port_mappings and not
@@ -224,6 +228,16 @@ route_run(struct route_ctx_in *r_ctx_in,
         }
 
         if (ad) {
+            if (lr_has_port_name_filter) {
+                struct smap_node *node;
+
+                SMAP_FOR_EACH_SAFE (node, &ad->bound_ports) {
+                    if (node->value && *node->value == '\0') {
+                        smap_remove_node(&ad->bound_ports, node);
+                    }
+                }
+            }
+
             tracked_datapath_add(ld->datapath, TRACKED_RESOURCE_NEW,
                                  r_ctx_out->tracked_re_datapaths);
             hmap_insert(r_ctx_out->announce_routes, &ad->node,
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 8e356df6f..b2319b180 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -16374,8 +16374,7 @@ check ovn-nbctl lr-add internet \
 check ovn-nbctl lrp-add internet internet-public \
         00:00:02:01:02:03 192.0.2.1/24 \
     -- set Logical_Router_Port internet-public \
-            options:dynamic-routing-redistribute="connected,static" \
-            options:dynamic-routing-port-name=wedontlearnstuffhere
+            options:dynamic-routing-redistribute="connected,static"
 check ovn-nbctl lsp-add public public-internet \
     -- set Logical_Switch_Port public-internet type=router \
             options:router-port=internet-public \
@@ -16548,7 +16547,9 @@ check ovn-nbctl set Logical_Router_Port internet-phys \
 check_row_count Learned_Route 0
 check ip route add 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf 
ovnvrf1337
 check ovn-nbctl --wait=hv sync
-check_row_count Learned_Route 1
+# With a Gateway Router all LRPs are locally bound, and without explicit
+# mapping/filtering they will all learn the route.
+check_row_count Learned_Route 2
 lp=$(fetch_column port_binding _uuid logical_port=internet-phys)
 check_row_count Learned_Route 1 logical_port=$lp ip_prefix=233.252.0.0/24 
nexthop=192.168.10.10
 
-- 
2.43.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to