Commit f2deb24c5c43 ("northd: Sync Advertised_Route to sb.") correctly
skipped over "link-local connected networks" not creating
Advertised_Route records for the link-local subnet. However, when
implementing the connected-as-host semantics it shouldn't have skipped
over the (non-LLA) IPs configured on routers/switches connected to said
interfaces.
It introduced an inconsistency between the case (a) when a router port is
configured with an explicit IP (and network) and the case (b) when the
router port is only configured with MAC address (just a link-local IPv6
address). In (a), if the router port is configured to advertise
"connected-as-host" ovn-northd creates Advertised_Route records for all
IPs known to be configured on the peer switch/router. In (b), however
it does not.
This forces CMS to always configure an explicit IP on the router port
even though that's not necessary for any of the routing decisions.
Fix it by advertising the connected-as-host routes for all connected
routes generated for a router port.
NOTE: we don't try to verify that advertised host routes are actually
reachable (there's a route prefix on the router that matches on them).
This was already the ovn-northd behavior.
Reported-at: https://issues.redhat.com/browse/FDP-1563
Reported-by: Jakub Libosvar <[email protected]>
Fixes: f2deb24c5c43 ("northd: Sync Advertised_Route to sb.")
Signed-off-by: Dumitru Ceara <[email protected]>
---
northd/en-advertised-route-sync.c | 72 +++++++++++++++++--------------
tests/ovn-northd.at | 72 +++++++++++++++++++++++++++++++
2 files changed, 112 insertions(+), 32 deletions(-)
diff --git a/northd/en-advertised-route-sync.c
b/northd/en-advertised-route-sync.c
index dfa9f9c447..35def9d227 100644
--- a/northd/en-advertised-route-sync.c
+++ b/northd/en-advertised-route-sync.c
@@ -649,36 +649,44 @@ should_advertise_route(const struct uuidset
*host_route_lrps,
}
}
-/* Returns true if the route has already been fully processed for
- * advertising (e.g., for connected routes when connected-as-host is set).
- * Returns false otherwise, in which case the a new 'ar_entry' needs to
- * be created for this route.
- */
+/* Returns true if the connected route was advertised as a set of host routes
+ * (/32 for IPv4 and /128 for IPv6), one for each individual IP known to be
+ * reachable in the connected route's subnet. Returns false otherwise. */
static bool
-process_prereqs_advertise_route(
+advertise_routes_as_host_prefix(
struct advertised_route_sync_data *data,
struct uuidset *host_route_lrps,
struct hmap *sync_routes,
const struct ovn_datapath *advertising_od,
const struct ovn_port *advertising_op,
- const struct ovn_port *tracked_op,
- enum route_source source)
+ enum route_source source
+)
{
+ if (source != ROUTE_SOURCE_CONNECTED) {
+ return false;
+ }
+
enum dynamic_routing_redistribute_mode drr =
advertising_op->dynamic_routing_redistribute;
+ if (!drr_mode_CONNECTED_AS_HOST_is_set(drr)) {
+ return false;
+ }
+
+ uuidset_insert(host_route_lrps, &advertising_op->nbrp->header_.uuid);
+ publish_host_routes(sync_routes, advertising_od, advertising_op, data);
+ return true;
+}
+/* Track datapaths (routers/switches) whose changes should trigger
+ * the set of advertised routes. That includes NAT and LB related
+ * advertised routes. */
+static void
+advertise_route_track_od(struct advertised_route_sync_data *data,
+ const struct ovn_datapath *advertising_od,
+ const struct ovn_port *tracked_op,
+ enum route_source source)
+{
switch (source) {
- case ROUTE_SOURCE_CONNECTED:
- if (drr_mode_CONNECTED_AS_HOST_is_set(drr)) {
- const struct uuid *lrp_uuid = &advertising_op->nbrp->header_.uuid;
- uuidset_insert(host_route_lrps, lrp_uuid);
- publish_host_routes(sync_routes, advertising_od, advertising_op,
- data);
- return true;
- }
- break;
- case ROUTE_SOURCE_STATIC:
- break;
case ROUTE_SOURCE_NAT:
/* If NAT route tracks port on a different DP than the one that
* advertises the route, we need to watch for changes on that DP as
@@ -702,12 +710,14 @@ process_prereqs_advertise_route(
&tracked_op->od->nbr->header_.uuid);
}
break;
+ case ROUTE_SOURCE_CONNECTED:
+ case ROUTE_SOURCE_STATIC:
+ break;
case ROUTE_SOURCE_LEARNED:
OVS_NOT_REACHED();
default:
OVS_NOT_REACHED();
}
- return false;
}
static void
@@ -728,23 +738,25 @@ advertised_route_table_sync(
continue;
}
- if (prefix_is_link_local(&route->prefix, route->plen)) {
- continue;
- }
-
if (!should_advertise_route(&host_route_lrps, route->od,
route->out_port, route->source)) {
continue;
}
- if (process_prereqs_advertise_route(data, &host_route_lrps,
+ if (advertise_routes_as_host_prefix(data, &host_route_lrps,
&sync_routes, route->od,
route->out_port,
- route->tracked_port,
route->source)) {
continue;
}
+ if (prefix_is_link_local(&route->prefix, route->plen)) {
+ continue;
+ }
+
+ advertise_route_track_od(data, route->od, route->tracked_port,
+ route->source);
+
ar_entry_add_nocopy(&sync_routes, route->od, route->out_port,
normalize_v46_prefix(&route->prefix, route->plen),
route->tracked_port,
@@ -759,12 +771,8 @@ advertised_route_table_sync(
continue;
}
- if (process_prereqs_advertise_route(data, &host_route_lrps,
- &sync_routes, route_e->od,
- route_e->op, route_e->tracked_port,
- route_e->source)) {
- continue;
- }
+ advertise_route_track_od(data, route_e->od, route_e->tracked_port,
+ route_e->source);
const struct sbrec_port_binding *tracked_pb =
route_e->tracked_port ? route_e->tracked_port->sb : NULL;
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 3d27f10cde..208bef20dc 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -15591,6 +15591,78 @@ AT_CHECK([fetch_column sb:Port_Binding options
logical_port=lr0-sw0 | grep -q 'd
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([dynamic-routing - host routes - unnumbered LRP interfaces])
+AT_KEYWORDS([dynamic-routing])
+ovn_start
+
+dnl Logical topology:
+dnl
+dnl internal-lr
+dnl |
+dnl |
+dnl internal-ls
+dnl |
+dnl +-------+----+
+dnl | |
+dnl | +--- ovs-bridge
+dnl | |
+dnl | +----+
+dnl | |
+dnl | public-ls
+dnl | |
+dnl | |
+dnl | | (explicit IP)
+dnl (no explicit IP) +-- public-lr (BGP enabled)
+dnl (just LLA) |
+dnl |
+dnl <fabric>
+dnl
+dnl When configured to advertise connected-routes "as-host" prefixes
+dnl router ports should also include connected router/switch IPs reachable
+dnl via unnumbered router ports (that only have LLA configured).
+
+check ovn-nbctl --wait=sb \
+ -- lr-add internal-lr \
+ -- lrp-add internal-lr internal-lrp 00:00:00:00:00:01 1.1.1.1/24 \
+ -- ls-add internal-ls \
+ -- lsp-add internal-ls internal-ls-lrp \
+ -- lsp-set-type internal-ls-lrp router \
+ -- lsp-set-options internal-ls-lrp router-port=internal-lrp \
+ -- lsp-set-addresses internal-ls-lrp router \
+ -- lsp-add internal-ls internal-ls-lla-lrp \
+ -- lsp-set-type internal-ls-lla-lrp router \
+ -- lsp-set-options internal-ls-lla-lrp router-port=lla-lrp \
+ -- lsp-set-addresses internal-ls-lla-lrp router \
+ -- lsp-add internal-ls internal-ls-localnet \
+ -- lsp-set-type internal-ls-localnet localnet \
+ -- ls-add public-ls \
+ -- lsp-add public-ls public-ls-lrp \
+ -- lsp-set-type public-ls-lrp router \
+ -- lsp-set-options public-ls-lrp router-port=public-lrp \
+ -- lsp-set-addresses public-ls-lrp router \
+ -- lsp-add public-ls public-ls-localnet \
+ -- lsp-set-type public-ls-localnet localnet \
+ -- lr-add public-lr \
+ -- lrp-add public-lr public-lrp 00:00:00:00:00:02 1.1.1.2/24 \
+ -- lrp-add public-lr lla-lrp 00:00:00:00:00:03 \
+ -- lrp-add public-lr fab-lrp 00:00:00:00:00:04 2.2.2.1/24 \
+ -- set Logical_Router public-lr option:dynamic-routing=true \
+ -- set Logical_Router_Port lla-lrp \
+ options:dynamic-routing-redistribute="connected-as-host"
+
+pub_lr_dp=$(fetch_column Datapath_Binding _uuid external_ids:name=public-lr)
+lla_lrp_pb=$(fetch_column Port_Binding _uuid logical_port=lla-lrp)
+int_lrp_pb=$(fetch_column Port_Binding _uuid logical_port=internal-lrp)
+check_column 1.1.1.1 Advertised_Route ip_prefix \
+ datapath=$pub_lr_dp \
+ logical_port=$lla_lrp_pb \
+ tracked_port=$int_lrp_pb
+check_row_count Advertised_Route 1
+
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD_NO_HV([
AT_SETUP([dynamic-routing incremental processing])
AT_KEYWORDS([dynamic-routing])
--
2.50.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
