Hi Felix, I noticed small nits in the documentation, I'll list them below. On Thu, 2025-02-06 at 15:19 +0100, Felix Huettner via dev wrote: > Sometimes we want to use individual host routes instead of the > connected > routes of LRPs. > This allows the network fabric to know which addresses are actually > in > use and e.g. drop traffic to addresses that are not used anyway. > > Signed-off-by: Felix Huettner <[email protected]> > --- > v6->v7: > * switch the setting to a dynamic-routing-redistribute option > * support lrp<->lrp peer connection for host routes > * addressed review comments > v5->v6: > * addressed review comments > v4->v5: skipped > v3->v4: > * fix a memory leak > v2->v3: > * A lot of minor review comments. > > NEWS | 3 + > northd/en-advertised-route-sync.c | 249 > ++++++++++++++++++++++++++++-- > northd/en-advertised-route-sync.h | 11 ++ > northd/inc-proc-northd.c | 4 + > northd/northd.c | 37 ++--- > northd/northd.h | 25 ++- > ovn-nb.xml | 123 +++++++++++---- > tests/ovn-northd.at | 127 ++++++++++++++- > 8 files changed, 514 insertions(+), 65 deletions(-) > > diff --git a/NEWS b/NEWS > index 73a06843c..60d1e793a 100644 > --- a/NEWS > +++ b/NEWS > @@ -53,6 +53,9 @@ Post v24.09.0 > fabric. Routes entered into the "Learned_Route" table in the > southbound > database will be learned by the respective LR. They are > included in the > route table with a lower priority than static routes. > + * Add the option value "connected-as-host" to the > + "dynamic-routing-redistribute" LR and LRP option. If set then > connected > + routes are announced as individual host routes. > > OVN v24.09.0 - 13 Sep 2024 > -------------------------- > diff --git a/northd/en-advertised-route-sync.c b/northd/en- > advertised-route-sync.c > index 0fa91f5aa..9e8636806 100644 > --- a/northd/en-advertised-route-sync.c > +++ b/northd/en-advertised-route-sync.c > @@ -20,6 +20,7 @@ > #include "northd.h" > > #include "en-advertised-route-sync.h" > +#include "en-lr-stateful.h" > #include "lib/stopwatch-names.h" > #include "openvswitch/hmap.h" > #include "ovn-util.h" > @@ -28,34 +29,131 @@ static void > advertised_route_table_sync( > struct ovsdb_idl_txn *ovnsb_txn, > const struct sbrec_advertised_route_table > *sbrec_advertised_route_table, > - const struct hmap *parsed_routes); > + const struct lr_stateful_table *lr_stateful_table, > + const struct hmap *parsed_routes, > + struct advertised_route_sync_data *data); > + > +bool > +advertised_route_sync_lr_stateful_change_handler(struct engine_node > *node, > + void *data_) > +{ > + /* We only actually use lr_stateful data if we expose individual > host > + * routes. In this case we for now just recompute. > + * */ > + struct ed_type_lr_stateful *lr_stateful_data = > + engine_get_input_data("lr_stateful", node); > + struct advertised_route_sync_data *data = data_; > + > + struct hmapx_node *hmapx_node; > + const struct lr_stateful_record *lr_stateful_rec; > + HMAPX_FOR_EACH (hmapx_node, &lr_stateful_data- > >trk_data.crupdated) { > + lr_stateful_rec = hmapx_node->data; > + if (uuidset_contains(&data->nb_lr, > + &lr_stateful_rec->nbr_uuid)) { > + return false; > + } > + } > + > + return true; > +} > + > +bool > +advertised_route_sync_northd_change_handler(struct engine_node > *node, > + void *data_) > +{ > + struct advertised_route_sync_data *data = data_; > + struct northd_data *northd_data = > engine_get_input_data("northd", node); > + if (!northd_has_tracked_data(&northd_data->trk_data)) { > + return false; > + } > + > + /* We indirectly use northd_data->ls_ports if we announce host > routes. > + * For now we just recompute on any change to lsps that are > relevant to us. > + */ > + struct hmapx_node *hmapx_node; > + const struct ovn_port *op; > + HMAPX_FOR_EACH (hmapx_node, &northd_data- > >trk_data.trk_lsps.created) { > + op = hmapx_node->data; > + if (uuidset_contains(&data->nb_ls, > + &op->od->nbs->header_.uuid)) { > + return false; > + } > + } > + HMAPX_FOR_EACH (hmapx_node, &northd_data- > >trk_data.trk_lsps.updated) { > + op = hmapx_node->data; > + if (uuidset_contains(&data->nb_ls, > + &op->od->nbs->header_.uuid)) { > + return false; > + } > + } > + HMAPX_FOR_EACH (hmapx_node, &northd_data- > >trk_data.trk_lsps.deleted) { > + op = hmapx_node->data; > + if (uuidset_contains(&data->nb_ls, > + &op->od->nbs->header_.uuid)) { > + return false; > + } > + } > + > + return true; > +} > + > +static void > +routes_sync_init(struct advertised_route_sync_data *data) > +{ > + uuidset_init(&data->nb_lr); > + uuidset_init(&data->nb_ls); > +} > + > +static void > +routes_sync_clear(struct advertised_route_sync_data *data) > +{ > + uuidset_clear(&data->nb_lr); > + uuidset_clear(&data->nb_ls); > +} > + > +static void > +routes_sync_destroy(struct advertised_route_sync_data *data) > +{ > + uuidset_destroy(&data->nb_lr); > + uuidset_destroy(&data->nb_ls); > +} > > void > *en_advertised_route_sync_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > { > - return NULL; > + struct advertised_route_sync_data *data = xzalloc(sizeof *data); > + routes_sync_init(data); > + return data; > } > > void > en_advertised_route_sync_cleanup(void *data OVS_UNUSED) > { > + routes_sync_destroy(data); > } > > void > en_advertised_route_sync_run(struct engine_node *node, void *data > OVS_UNUSED) > { > + routes_sync_clear(data); > + > + struct advertised_route_sync_data *routes_sync_data = data; > struct routes_data *routes_data > = engine_get_input_data("routes", node); > const struct engine_context *eng_ctx = engine_get_context(); > const struct sbrec_advertised_route_table > *sbrec_advertised_route_table = > EN_OVSDB_GET(engine_get_input("SB_advertised_route", node)); > + struct ed_type_lr_stateful *lr_stateful_data = > + engine_get_input_data("lr_stateful", node); > > stopwatch_start(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME, > time_msec()); > > advertised_route_table_sync(eng_ctx->ovnsb_idl_txn, > sbrec_advertised_route_table, > - &routes_data->parsed_routes); > + &lr_stateful_data->table, > + &routes_data->parsed_routes, > + routes_sync_data); > > stopwatch_stop(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME, > time_msec()); > engine_set_node_state(node, EN_UPDATED); > @@ -68,19 +166,22 @@ struct ar_entry { > > const struct sbrec_port_binding *logical_port; > char *ip_prefix; > + const struct sbrec_port_binding *tracked_port; > }; > > /* Add a new entries to the to-be-advertised routes. > * Takes ownership of ip_prefix. */ > static struct ar_entry * > ar_add_entry(struct hmap *routes, const struct > sbrec_datapath_binding *sb_db, > - const struct sbrec_port_binding *logical_port, char > *ip_prefix) > + const struct sbrec_port_binding *logical_port, char > *ip_prefix, > + const struct sbrec_port_binding *tracked_port) > { > struct ar_entry *route_e = xzalloc(sizeof *route_e); > > route_e->sb_db = sb_db; > route_e->logical_port = logical_port; > route_e->ip_prefix = ip_prefix; > + route_e->tracked_port = tracked_port; > uint32_t hash = uuid_hash(&sb_db->header_.uuid); > hash = hash_string(logical_port->logical_port, hash); > hash = hash_string(ip_prefix, hash); > @@ -91,7 +192,8 @@ ar_add_entry(struct hmap *routes, const struct > sbrec_datapath_binding *sb_db, > > static struct ar_entry * > ar_find(struct hmap *route_map, const struct sbrec_datapath_binding > *sb_db, > - const struct sbrec_port_binding *logical_port, const char > *ip_prefix) > + const struct sbrec_port_binding *logical_port, const char > *ip_prefix, > + const struct sbrec_port_binding *tracked_port) > { > struct ar_entry *route_e; > uint32_t hash; > @@ -114,6 +216,10 @@ ar_find(struct hmap *route_map, const struct > sbrec_datapath_binding *sb_db, > continue; > } > > + if (tracked_port != route_e->tracked_port) { > + continue; > + } > + > return route_e; > } > > @@ -127,13 +233,118 @@ ar_entry_free(struct ar_entry *route_e) > free(route_e); > } > > +static void > +publish_lport_addresses(struct hmap *sync_routes, > + const struct sbrec_datapath_binding *sb_db, > + const struct ovn_port *logical_port, > + const struct lport_addresses *addresses, > + const struct ovn_port *tracking_port) > +{ > + for (size_t i = 0; i < addresses->n_ipv4_addrs; i++) { > + const struct ipv4_netaddr *addr = &addresses->ipv4_addrs[i]; > + char *addr_s = xasprintf("%s/32", addr->addr_s); > + ar_add_entry(sync_routes, sb_db, logical_port->sb, > + addr_s, tracking_port->sb); > + } > + for (size_t i = 0; i < addresses->n_ipv6_addrs; i++) { > + if (in6_is_lla(&addresses->ipv6_addrs[i].network)) { > + continue; > + } > + const struct ipv6_netaddr *addr = &addresses->ipv6_addrs[i]; > + char *addr_s = xasprintf("%s/128", addr->addr_s); > + ar_add_entry(sync_routes, sb_db, logical_port->sb, > + addr_s, tracking_port->sb); > + } > +} > + > +/* Collect all IP addresses connected via this LRP. */ > +static void > +publish_host_routes_lrp(struct hmap *sync_routes, > + const struct lr_stateful_table > *lr_stateful_table, > + const struct parsed_route *route, > + struct advertised_route_sync_data *data, > + const struct ovn_port *lrp) > +{ > + /* This is a LSP connected to an LRP */ > + const struct lport_addresses *addresses = &lrp->lrp_networks; > + publish_lport_addresses(sync_routes, route->od->sb, > + route->out_port, > + addresses, lrp); > + > + const struct lr_stateful_record *lr_stateful_rec; > + lr_stateful_rec = lr_stateful_table_find_by_index( > + lr_stateful_table, lrp->od->index); > + /* We also need to track this LR as we need to recompute when > + * any of its IPs change. */ > + uuidset_insert(&data->nb_lr, > + &lr_stateful_rec->nbr_uuid); > + struct ovn_port_routable_addresses addrs = get_op_addresses( > + lrp, lr_stateful_rec, false); > + for (size_t i = 0; i < addrs.n_addrs; i++) { > + publish_lport_addresses(sync_routes, route->od->sb, > + route->out_port, > + &addrs.laddrs[i], > + lrp); > + } > + destroy_routable_addresses(&addrs); > +} > + > +/* Collect all IP addresses connected to the out_port of a route. > + * This traverses all LSPs on the LS connected to the out_port. */ > +static void > +publish_host_routes(struct hmap *sync_routes, > + const struct lr_stateful_table > *lr_stateful_table, > + const struct parsed_route *route, > + struct advertised_route_sync_data *data) > +{ > + if (!route->out_port->peer) { > + return; > + } > + > + struct ovn_datapath *peer_od = route->out_port->peer->od; > + if (!peer_od->nbs && !peer_od->nbr) { > + return; > + } > + > + if (peer_od->nbr) { > + /* This is a LRP directly connected to another LRP. */ > + publish_host_routes_lrp(sync_routes, lr_stateful_table, > route, > + data, route->out_port->peer); > + return; > + } > + > + /* We need to track the LS we are publishing routes from, so > that we can > + * recompute when any port on there changes. */ > + uuidset_insert(&data->nb_ls, &peer_od->nbs->header_.uuid); > + > + struct ovn_port *port; > + HMAP_FOR_EACH (port, dp_node, &peer_od->ports) { > + if (port->peer) { > + /* This is a LSP connected to an LRP */ > + publish_host_routes_lrp(sync_routes, lr_stateful_table, > route, > + data, port->peer); > + } else { > + /* This is just a plain LSP */ > + for (size_t i = 0; i < port->n_lsp_addrs; i++) { > + publish_lport_addresses(sync_routes, route->od->sb, > + route->out_port, > + &port->lsp_addrs[i], > + port); > + } > + } > + } > +} > + > static void > advertised_route_table_sync( > struct ovsdb_idl_txn *ovnsb_txn, > const struct sbrec_advertised_route_table > *sbrec_advertised_route_table, > - const struct hmap *parsed_routes) > + const struct lr_stateful_table *lr_stateful_table, > + const struct hmap *parsed_routes, > + struct advertised_route_sync_data *data) > { > struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes); > + struct uuidset host_route_lrps = > UUIDSET_INITIALIZER(&host_route_lrps); > const struct parsed_route *route; > > struct ar_entry *route_e; > @@ -151,9 +362,21 @@ advertised_route_table_sync( > > enum dynamic_routing_redistribute_mode drr = > route->out_port->dynamic_routing_redistribute; > - if (route->source == ROUTE_SOURCE_CONNECTED && > - (drr & DRRM_CONNECTED) == 0) { > - continue; > + if (route->source == ROUTE_SOURCE_CONNECTED) { > + if ((drr & DRRM_CONNECTED) == 0) { > + continue; > + } > + /* If we advertise host routes, we only need to do so > once per > + * LRP. */ > + const struct uuid *lrp_uuid = > + &route->out_port->nbrp->header_.uuid; > + if (drr & DRRM_CONNECTED_AS_HOST && > + !uuidset_contains(&host_route_lrps, lrp_uuid)) { > + uuidset_insert(&host_route_lrps, lrp_uuid); > + publish_host_routes(&sync_routes, lr_stateful_table, > + route, data); > + continue; > + } > } > if (route->source == ROUTE_SOURCE_STATIC && (drr & > DRRM_STATIC) == 0) { > continue; > @@ -161,14 +384,15 @@ advertised_route_table_sync( > > char *ip_prefix = normalize_v46_prefix(&route->prefix, > route->plen); > route_e = ar_add_entry(&sync_routes, route->od->sb, > - route->out_port->sb, ip_prefix); > + route->out_port->sb, ip_prefix, > NULL); > } > + uuidset_destroy(&host_route_lrps); > > SBREC_ADVERTISED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route, > > sbrec_advertised_route_table) { > route_e = ar_find(&sync_routes, sb_route->datapath, > - sb_route->logical_port, > - sb_route->ip_prefix); > + sb_route->logical_port, sb_route- > >ip_prefix, > + sb_route->tracked_port); > if (route_e) { > hmap_remove(&sync_routes, &route_e->hmap_node); > ar_entry_free(route_e); > @@ -183,6 +407,7 @@ advertised_route_table_sync( > sbrec_advertised_route_set_datapath(sr, route_e->sb_db); > sbrec_advertised_route_set_logical_port(sr, route_e- > >logical_port); > sbrec_advertised_route_set_ip_prefix(sr, route_e- > >ip_prefix); > + sbrec_advertised_route_set_tracked_port(sr, route_e- > >tracked_port); > ar_entry_free(route_e); > } > > diff --git a/northd/en-advertised-route-sync.h b/northd/en- > advertised-route-sync.h > index 30e7cae1f..1f24fd329 100644 > --- a/northd/en-advertised-route-sync.h > +++ b/northd/en-advertised-route-sync.h > @@ -17,10 +17,21 @@ > #define EN_ADVERTISED_ROUTE_SYNC_H 1 > > #include "lib/inc-proc-eng.h" > +#include "lib/uuidset.h" > > struct advertised_route_sync_data { > + /* Contains the uuids of all NB Logical Routers where we used a > + * lr_stateful_record during computation. */ > + struct uuidset nb_lr; > + /* Contains the uuids of all NB Logical Switches where we rely on > port > + * changes for host routes. */ > + struct uuidset nb_ls; > }; > > +bool advertised_route_sync_lr_stateful_change_handler(struct > engine_node *, > + void *data); > +bool advertised_route_sync_northd_change_handler(struct engine_node > *, > + void *data); > void *en_advertised_route_sync_init(struct engine_node *, struct > engine_arg *); > void en_advertised_route_sync_cleanup(void *data); > void en_advertised_route_sync_run(struct engine_node *, void *data); > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index fa7ca6f2d..86b7b999e 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -284,6 +284,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop > *nb, > engine_add_input(&en_advertised_route_sync, &en_routes, NULL); > engine_add_input(&en_advertised_route_sync, > &en_sb_advertised_route, > NULL); > + engine_add_input(&en_advertised_route_sync, &en_lr_stateful, > + > advertised_route_sync_lr_stateful_change_handler); > + engine_add_input(&en_advertised_route_sync, &en_northd, > + advertised_route_sync_northd_change_handler); > > engine_add_input(&en_learned_route_sync, &en_routes, NULL); > engine_add_input(&en_learned_route_sync, &en_sb_learned_route, > NULL); > diff --git a/northd/northd.c b/northd/northd.c > index 51d8a6b56..3cd1ba9a6 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -828,6 +828,11 @@ parse_dynamic_routing_redistribute( > out |= DRRM_CONNECTED; > continue; > } > + if (!strcmp(token, "connected-as-host")) { > + /* Setting connected-as-host implies connected. */ > + out |= DRRM_CONNECTED | DRRM_CONNECTED_AS_HOST; > + continue; > + } > if (!strcmp(token, "static")) { > out |= DRRM_STATIC; > continue; > @@ -1119,19 +1124,6 @@ build_datapaths(struct ovsdb_idl_txn > *ovnsb_txn, > ods_build_array_index(lr_datapaths); > } > > -/* Structure representing logical router port > - * routable addresses. This includes DNAT and Load Balancer > - * addresses. This structure will only be filled in if the > - * router port is a gateway router port. Otherwise, all pointers > - * will be NULL and n_addrs will be 0. > - */ > -struct ovn_port_routable_addresses { > - /* The parsed routable addresses */ > - struct lport_addresses *laddrs; > - /* Number of items in the laddrs array */ > - size_t n_addrs; > -}; > - > static bool lsp_can_be_inc_processed(const struct > nbrec_logical_switch_port *); > > /* This function returns true if 'op' is a gateway router port. > @@ -1177,7 +1169,7 @@ is_transit_router_port(struct ovn_port *op) > smap_get_bool(&op->sb->chassis->other_config, "is- > remote", false); > } > > -static void > +void > destroy_routable_addresses(struct ovn_port_routable_addresses *ra) > { > for (size_t i = 0; i < ra->n_addrs; i++) { > @@ -1190,12 +1182,14 @@ static char **get_nat_addresses(const struct > ovn_port *op, size_t *n, > bool routable_only, bool > include_lb_ips, > const struct lr_stateful_record *); > > -static struct ovn_port_routable_addresses > -get_op_routable_addresses(struct ovn_port *op, > - const struct lr_stateful_record > *lr_stateful_rec) > +struct ovn_port_routable_addresses > +get_op_addresses(const struct ovn_port *op, > + const struct lr_stateful_record *lr_stateful_rec, > + bool routable_only) > { > size_t n; > - char **nats = get_nat_addresses(op, &n, true, true, > lr_stateful_rec); > + char **nats = get_nat_addresses(op, &n, routable_only, true, > + lr_stateful_rec); > > if (!nats) { > return (struct ovn_port_routable_addresses) { > @@ -1228,6 +1222,13 @@ get_op_routable_addresses(struct ovn_port *op, > }; > } > > +static struct ovn_port_routable_addresses > +get_op_routable_addresses(struct ovn_port *op, > + const struct lr_stateful_record > *lr_stateful_rec) > +{ > + return get_op_addresses(op, lr_stateful_rec, true); > +} > + > > static void > ovn_port_set_nb(struct ovn_port *op, > diff --git a/northd/northd.h b/northd/northd.h > index 1fa0d6365..a4bff1987 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -25,6 +25,7 @@ > #include "openvswitch/hmap.h" > #include "simap.h" > #include "ovs-thread.h" > +#include "en-lr-stateful.h" > > struct northd_input { > /* Northbound table references */ > @@ -302,12 +303,14 @@ struct mcast_port_info { > > enum dynamic_routing_redistribute_mode_bits { > DRRM_CONNECTED_BIT = 0, > - DRRM_STATIC_BIT = 1, > + DRRM_CONNECTED_AS_HOST_BIT = 1, > + DRRM_STATIC_BIT = 2, > }; > > enum dynamic_routing_redistribute_mode { > DRRM_NONE = 0, > DRRM_CONNECTED = (1 << DRRM_CONNECTED_BIT), > + DRRM_CONNECTED_AS_HOST = (1 << DRRM_CONNECTED_AS_HOST_BIT), > DRRM_STATIC = (1 << DRRM_STATIC_BIT), > }; > > @@ -954,4 +957,24 @@ void build_igmp_lflows(struct hmap *igmp_groups, > const struct hmap *ls_datapaths, > struct lflow_table *lflows, > struct lflow_ref *lflow_ref); > + > +/* Structure representing logical router port routable addresses. > This > + * includes DNAT and Load Balancer addresses. This structure will > only > + * be filled in if the router port is a gateway router port. > Otherwise, > + * all pointers will be NULL and n_addrs will be 0. > + */ > +struct ovn_port_routable_addresses { > + /* The parsed routable addresses */ > + struct lport_addresses *laddrs; > + /* Number of items in the laddrs array */ > + size_t n_addrs; > +}; > + > +struct ovn_port_routable_addresses get_op_addresses( > + const struct ovn_port *op, > + const struct lr_stateful_record *lr_stateful_rec, > + bool routable_only); > + > +void destroy_routable_addresses(struct ovn_port_routable_addresses > *ra); > + > #endif /* NORTHD_H */ > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 895d1b3a1..cc94937bc 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2978,31 +2978,63 @@ or > route types that should be advertised. See: > <ul> > <li><ref column="options" key="dynamic-routing-redistribute" > - table="Logical_Router"/></li> > + table="Logical_Router"/> on Logical_Router</li> > <li><ref column="options" key="dynamic-routing-redistribute" > - table="Logical_Router_Port"/></li> > + table="Logical_Router_Port"/> on > Logical_Router_Port</li> > </ul> > </column> > > <column name="options" key="dynamic-routing-redistribute" > type='{"type": "string"}'> > - Only relevant if <ref column="options" key="dynamic-routing" > - table="Logical_Router"/> is set to <code>true</code>. > + <p> > + Only relevant if <ref column="options" key="dynamic- > routing" > + table="Logical_Router"/> is set to <code>true</code>. > + </p> > > - This is a list of elements separated by <code>;</code>. > + <p> > + This is a list of elements separated by <code>;</code>. > + </p> > > - If <code>connected</code> is in the list then northd will > synchronize > - all "connected" routes to the southbound <ref table="Route" > - db="OVN_SB"/> table. "Connected" here means routes > implicitly created > - by networks associated with the LRPs. > + <p> > + If <code>connected</code> is in the list then northd will > synchronize > + all "connected" routes to the southbound <ref > table="Route" > + db="OVN_SB"/> table. "Connected" here means routes > implicitly created > + by networks associated with the LRPs. > + </p> > > - If <code>static</code> is in the list then northd will > synchronize all > - <ref table="Logical_Router_Static_Route"/> to the southbound > - <ref table="Route" db="OVN_SB"/> table. > + <p> > + If <code>connected-as-host</code> is in the list then > northd will > + enumerate all actively used individual IPs of a > "connected" route and > + announce these IPs as host routes instead of announcing > the subnet. > + Setting this implies the setting <code>connected</code>. > > - This value can be overwritten on a per LRP basis using > - <ref column="options" key="dynamic-routing-redistribute" > - table="Logical_Router_Port"/>. > + This setting can be used to: > + <ul> > + <li> > + allow the fabric outside of OVN to drop traffic > towards IP > + addresses that are not actually used. This traffic > would > + otherwise hit this LR and then be dropped. > + </li> > + > + <li> > + If this LR has multiple LRPs connected to the fabric > on different > + chassis: allows the fabric outside of OVN to steer > packets to the > + chassis which already hosts this backing ip address. > + </li> > + </ul> > + </p> > + > + <p> > + If <code>static</code> is in the list then northd will > synchronize > + all <ref table="Logical_Router_Static_Route"/> to the > southbound > + <ref table="Route" db="OVN_SB"/> table. > + </p> > + > + <p> > + This value can be overwritten on a per LRP basis using > + <ref column="options" key="dynamic-routing-redistribute" > + table="Logical_Router_Port"/> on the > Logical_Router_Port. > + </p> > </column> > </group> > > @@ -3774,24 +3806,57 @@ or > > <column name="options" key="dynamic-routing-redistribute" > type='{"type": "string"}'> > - Only relevant if <ref column="options" key="dynamic-routing" > - table="Logical_Router"/> on the respective Logical_Router is > set > - to <code>true</code>. > + <p> > + Only relevant if <ref column="options" key="dynamic- > routing" > + table="Logical_Router"/> on the respective Logical_Router > is set > + to <code>true</code>. > + </p> > > - This is a list of elements separated by <code>;</code>. > + <p> > + This is a list of elements separated by <code>;</code>. > + </p> > > - If <code>connected</code> is in the list then northd will > synchronize > - all "connected" routes to the southbound <ref table="Route" > - db="OVN_SB"/> table. "Connected" here means routes > implicitly created > - by networks associated with the LRPs. > + <p> > + If <code>connected</code> is in the list then northd will > synchronize > + all "connected" routes to the southbound <ref > table="Route" > + db="OVN_SB"/> table. "Connected" here means routes > implicitly created
Two nits that apply here and in the rest of the ovn-nb.xml file. nit1: you use db="OVN_SB", but the rest of the file seems to historically use db="OVN_Southbound". nit2: You reference table=Route. Did you mean table="Advertised_Route"? Thanks, Martin. > + by networks associated with the LRPs. > + </p> > > - If <code>static</code> is in the list then northd will > synchronize all > - <ref table="Logical_Router_Static_Route"/> to the southbound > - <ref table="Route" db="OVN_SB"/> table. > + <p> > + If <code>connected-as-host</code> is in the list then > northd will > + enumerate all actively used individual IPs of a > "connected" route and > + announce these IPs as host routes instead of announcing > the subnet. > + Setting this implies the setting <code>connected</code>. > + > + This setting can be used to: > + <ul> > + <li> > + allow the fabric outside of OVN to drop traffic > towards IP > + addresses that are not actually used. This traffic > would > + otherwise hit this LR and then be dropped. > + </li> > + > + <li> > + If this LR has multiple LRPs connected to the fabric > on different > + chassis: allows the fabric outside of OVN to steer > packets to the > + chassis which already hosts this backing ip address. > + </li> > + </ul> > + </p> > + > + <p> > + If <code>static</code> is in the list then northd will > synchronize > + all <ref table="Logical_Router_Static_Route"/> to the > southbound > + <ref table="Route" db="OVN_SB"/> table. > + </p> > + > + <p> > + If not set the value from <ref column="options" > + key="dynamic-routing-redistribute" > table="Logical_Router"/> on the > + Logical_Router will be used. > + </p> > > - If not set the value from <ref column="options" > - key="dynamic-routing-redistribute" table="Logical_Router"/> > will be > - used. > </column> > </group> > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index aacfc27df..c838633ff 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -14897,6 +14897,80 @@ AT_CHECK([grep -w "lr_in_ip_routing" > lr0flows | ovn_strip_lflows], [0], [dnl > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([dynamic-routing - host routes]) > +AT_KEYWORDS([dynamic-routing]) > +ovn_start > + > +# We start with announcing routes on a lr with 2 lrps. > +# LRP lr0-sw0 is connected to ls sw0. > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl set Logical_Router lr0 option:dynamic-routing=true \ > + option:dynamic-routing- > redistribute="connected;static" > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > +sw0=$(fetch_column port_binding _uuid logical_port=lr0-sw0) > +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 10.0.1.1/24 > +sw1=$(fetch_column port_binding _uuid logical_port=lr0-sw1) > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl lsp-add sw0 sw0-lr0 > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr0 > type=router options:router-port=lr0-sw0 > +check_row_count Advertised_Route 2 tracked_port='[[]]' > +datapath=$(fetch_column datapath_binding _uuid > external_ids:name=lr0) > + > +# Configuring the LRP lr0-sw0 to send host routes. > +# As sw0 is quite empty we will only see the addresses of lr0-sw0. > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 > options:dynamic-routing-redistribute="connected-as-host;static" > +check_row_count Advertised_Route 2 > +check_column 10.0.0.1/32 Advertised_Route ip_prefix > datapath=$datapath logical_port=$sw0 > +check_column $sw0 Advertised_Route tracked_port datapath=$datapath > logical_port=$sw0 > + > +# Adding a VIF to the LS sw0 will advertise it as well. > +check ovn-nbctl lsp-add sw0 sw0-vif0 > +check ovn-nbctl --wait=sb lsp-set-addresses sw0-vif0 > "00:aa:bb:cc:dd:ee 10.0.0.2" > +vif0=$(fetch_column port_binding _uuid logical_port=sw0-vif0) > +check_row_count Advertised_Route 3 > +check_row_count Advertised_Route 2 tracked_port!='[[]]' > +check_column $vif0 Advertised_Route tracked_port datapath=$datapath > logical_port=$sw0 ip_prefix=10.0.0.2/32 > + > +# Adding a LR lr1 to the LS sw0 will advertise the LRP of the new > router. > +check ovn-nbctl lr-add lr1 > +check ovn-nbctl lrp-add lr1 lr1-sw0 00:00:00:01:ff:01 10.0.0.10/24 > +check ovn-nbctl lsp-add sw0 sw0-lr1 > +lr1=$(fetch_column port_binding _uuid logical_port=lr1-sw0) > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr1 > type=router options:router-port=lr1-sw0 > +check_row_count Advertised_Route 4 > +check_row_count Advertised_Route 3 tracked_port!='[[]]' > +check_column $lr1 Advertised_Route tracked_port datapath=$datapath > logical_port=$sw0 ip_prefix=10.0.0.10/32 > + > +# Adding a NAT rule to lr1 will advertise it as well. > +check ovn-nbctl --wait=sb lr-nat-add lr1 dnat_and_snat 10.0.0.100 > 192.168.0.1 > +check_row_count Advertised_Route 5 > +check_row_count Advertised_Route 4 tracked_port!='[[]]' > +check_column $lr1 Advertised_Route tracked_port datapath=$datapath > logical_port=$sw0 ip_prefix=10.0.0.100/32 > + > +# Adding a static route to lr1 will be advertised just normally. > +check ovn-nbctl --wait=sb lr-route-add lr0 172.16.0.0/24 10.0.0.200 > +check_row_count Advertised_Route 6 > +check_row_count Advertised_Route 4 tracked_port!='[[]]' > +check_row_count Advertised_Route 1 datapath=$datapath > logical_port=$sw0 ip_prefix=172.16.0.0/24 > + > +# Adding lr2 directly connected using two LRPs will advertise it as > well. > +# lr2 contains a NAT rule that will be advertised. > +check ovn-nbctl lr-add lr2 > +check ovn-nbctl lrp-add lr0 lr0-lr2 00:00:00:02:ff:01 10.10.0.1/24 > peer=lr2-lr0 > +check ovn-nbctl set Logical_Router_Port lr0-lr2 options:dynamic- > routing-redistribute="connected-as-host;static" > +lr0lr2=$(fetch_column port_binding _uuid logical_port=lr0-lr2) > +check ovn-nbctl lrp-add lr2 lr2-lr0 00:00:00:02:ff:02 10.10.0.2/24 > peer=lr0-lr2 > +lr2lr0=$(fetch_column port_binding _uuid logical_port=lr2-lr0) > +check ovn-nbctl lrp-add lr2 lr2-sw1 00:00:00:02:ff:03 10.11.0.1/24 > +check ovn-nbctl --wait=sb lr-nat-add lr2 dnat_and_snat 10.10.0.20 > 10.20.0.123 > +check_row_count Advertised_Route 8 > +check_row_count Advertised_Route 6 tracked_port!='[[]]' > +check_row_count Advertised_Route 1 datapath=$datapath > logical_port=$lr0lr2 ip_prefix=10.10.0.20/32 tracked_port=$lr2lr0 > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([dynamic-routing incremental processing]) > AT_KEYWORDS([dynamic-routing]) > @@ -14920,13 +14994,16 @@ check_engine_stats lflow recompute > nocompute > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > -check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 > 10.0.0.1/24 > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl lsp-add sw0 sw0-lr0 > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr0 > type=router options:router-port=lr0-sw0 > sw0=$(fetch_column port_binding _uuid logical_port=lr0-sw0) > check_engine_stats northd recompute compute > -check_engine_stats routes recompute nocompute > -check_engine_stats advertised_route_sync recompute nocompute > -check_engine_stats learned_route_sync recompute nocompute > -check_engine_stats lflow recompute nocompute > +check_engine_stats routes recompute compute > +check_engine_stats advertised_route_sync recompute compute > +check_engine_stats learned_route_sync recompute compute > +check_engine_stats lflow recompute compute > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > @@ -15032,6 +15109,46 @@ check_engine_stats learned_route_sync > recompute nocompute > check_engine_stats lflow recompute nocompute > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 > options:dynamic-routing-redistribute="connected-as-host" > +check_engine_stats northd recompute nocompute > +check_engine_stats routes recompute nocompute > +check_engine_stats advertised_route_sync recompute nocompute > +check_engine_stats learned_route_sync recompute nocompute > +check_engine_stats lflow recompute nocompute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl lsp-add sw0 sw0-vif0 > +check ovn-nbctl --wait=sb lsp-set-addresses sw0-vif0 > "00:aa:bb:cc:dd:ee 10.0.0.2" > +check_engine_stats northd norecompute compute > +check_engine_stats routes norecompute compute > +check_engine_stats advertised_route_sync recompute nocompute > +check_engine_stats learned_route_sync norecompute compute > +check_engine_stats lflow norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl lr-add lr1 > +check ovn-nbctl lrp-add lr1 lr1-sw0 00:00:00:01:ff:01 10.0.0.10/24 > +check ovn-nbctl lsp-add sw0 sw0-lr1 > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr1 > type=router options:router-port=lr1-sw0 > +check_engine_stats northd recompute compute > +check_engine_stats routes recompute compute > +check_engine_stats advertised_route_sync recompute nocompute > +check_engine_stats learned_route_sync recompute compute > +check_engine_stats lflow recompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb lr-nat-add lr1 dnat_and_snat 10.0.0.100 > 192.168.0.1 > +check_engine_stats northd norecompute compute > +check_engine_stats routes norecompute compute > +check_engine_stats advertised_route_sync recompute nocompute > +check_engine_stats learned_route_sync norecompute compute > +check_engine_stats lflow norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > check ovn-nbctl --wait=sb lrp-del lr0-sw0 > check_engine_stats northd recompute compute _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
