On Fri, Feb 07, 2025 at 03:58:03PM +0100, Dumitru Ceara wrote: > On 2/7/25 9:00 AM, [email protected] wrote: > > 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. > > > > With Martin's comments addressed feel free to add my ack too: > Acked-by: Dumitru Ceara <[email protected]>
Hi Martin, Hi Dumitru, thanks for the reviews. Will be fixed in v8. > > Thanks, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
