Until now any change to a logical router's ports fell back to a full northd recompute, because lr_changes_can_be_handled() rejected the LOGICAL_ROUTER "ports" column and any change to a router port row. Enable incremental processing for creation and deletion of regular (non-gateway) logical router ports (LRPs), mirroring the router logical switch port path added by the preceding commit.
Like a router switch port, an LRP is not self-contained: a full recompute assigns a tunnel key and SB Port_Binding in build_ports(), populates od->router_ips, and adds a connected route per LRP network. The incremental path now performs (and tears down) this work itself: lr_port_create()/lr_port_init() mirror the LRP branch of join_logical_ports() plus the build_ports() post-processing, and the connected routes are added/removed incrementally through the routes -> group_ecmp_route -> lflow chain. The LRP's own flows are regenerated in the lflow port-change handler (build_lswitch_and_lrouter_iterate_by_lrp() on op->lflow_ref and build_lbnat_lflows_iterate_by_lrp() on op->stateful_lflow_ref), while the per-datapath flows (e.g. lr_in_network_id) are regenerated by re-tracking the router datapath to the lflow engine. Dependencies that live outside the LRP's lflow_ref and that this path does not keep in sync trigger a fall back to a full recompute (lrp_needs_recompute()): distributed gateway ports (gateway_chassis / ha_chassis_group), gateway routers, LRP<->LRP peering, NAT, load balancers, dynamic routing, policies, IPv6 RA, prefix delegation, custom route tables, mcast relay, a pre-existing peer switch port, or an NB Static_MAC_Binding referencing the port. Updates to an existing LRP also fall back to recompute; only create and delete are incremental. The consumers of the tracked LRPs assert that invariant, so that enabling incremental LRP updates later has to revisit them. Static routes on the router are not on that list. Blocking every router that has one would be far too coarse: a CMS typically attaches a default route to a router before wiring up the rest of its ports, so every subsequent lrp-add would recompute. What actually couples an LRP to a static route is output port resolution. find_static_route_outport() resolves the route either through the name in 'output_port' or, lacking that, by scanning the ports of the route's own router for the first one whose networks contain the next hop, and routes_northd_change_handler() only adds and removes the connected routes of a tracked LRP - it never re-parses the static ones. So lrp_affects_static_routes() falls back to a recompute exactly when the LRP is named by some route's 'output_port' (resolved against the global set of router ports, hence the scan is not limited to this router) or when its networks cover the next hop of one of its own router's routes. A discard route resolves no output port at all and never blocks the incremental path. While enabling this, fix a latent bug in the group_ecmp route engine: unique_routes_remove() matched a route to delete by prefix hash only, so deleting one of several connected routes that share a prefix but differ in out_port (e.g. the automatic fe80::/64 link-local route present on every LRP) could remove the wrong entry. Match the out_port exactly on deletion. Also track the static routes of a newly created logical router in northd_handle_lr_changes(). Only the updated-router loop fed trk_lrs_routes, so routes_static_route_change_handler() never saw the routes of a router created together with them and dropped them silently. That was already reachable for a router created without any port; letting static routes through the LRP path above widens it to routers created with their ports, so fix it here. Add tests covering incremental create/delete of a router port (including the resulting connected-route flows), deletion of a router port on a router with several fe80::/64 link-local routes (which removes the wrong route without the fix above), incremental port create/delete on a router carrying static routes the port is unrelated to, a router created in one transaction with its ports and routes, and the recompute fallbacks for the distributed-gateway, dynamic-routing, pre-existing-peer, modification and static-route-resolution cases. Assisted-by: Claude Opus 4.8, Claude Opus 5, ClaudeCode Signed-off-by: Lucas Vargas Dias <[email protected]> --- northd/en-group-ecmp-route.c | 29 +- northd/en-lflow.c | 7 + northd/en-multicast.c | 11 + northd/en-northd.c | 66 +++- northd/en-sync-sb.c | 2 + northd/northd.c | 625 ++++++++++++++++++++++++++++++++++- northd/northd.h | 27 +- tests/ovn-northd.at | 238 ++++++++++++- 8 files changed, 955 insertions(+), 50 deletions(-) diff --git a/northd/en-group-ecmp-route.c b/northd/en-group-ecmp-route.c index aca197318..eb674be3e 100644 --- a/northd/en-group-ecmp-route.c +++ b/northd/en-group-ecmp-route.c @@ -200,11 +200,19 @@ route_sources_ecmp_compatible(enum route_source a, enum route_source b) (b == ROUTE_SOURCE_NAT || b == ROUTE_SOURCE_LB); } -/* Remove the unique_routes_node from the group, and return the parsed_route - * pointed by the removed node. */ +/* Remove a unique_routes_node from the group, and return the parsed_route + * pointed by the removed node. + * + * 'route->prefix'/plen/is_src_route/source/route_table_id are always matched. + * When 'exact' is true the output port is matched too: several routes can + * share the same prefix (e.g. the IPv6 link-local fe80::/64 connected route + * present on every router port) and differ only by their output port, so a + * deletion must target the specific route. When 'exact' is false any route + * to the prefix is returned, which is what ECMP-group formation needs when a + * second next hop for an existing prefix is added. */ static const struct parsed_route * -unique_routes_remove(struct group_ecmp_datapath *gn, - const struct parsed_route *route) +unique_routes_remove__(struct group_ecmp_datapath *gn, + const struct parsed_route *route, bool exact) { struct unique_routes_node *ur; HMAP_FOR_EACH_WITH_HASH (ur, hmap_node, route->hash, &gn->unique_routes) { @@ -213,7 +221,8 @@ unique_routes_remove(struct group_ecmp_datapath *gn, route->is_src_route == ur->route->is_src_route && route_sources_ecmp_compatible(route->source, ur->route->source) && - route->route_table_id == ur->route->route_table_id) { + route->route_table_id == ur->route->route_table_id && + (!exact || route->out_port == ur->route->out_port)) { hmap_remove(&gn->unique_routes, &ur->hmap_node); const struct parsed_route *existed_route = ur->route; free(ur); @@ -223,6 +232,13 @@ unique_routes_remove(struct group_ecmp_datapath *gn, return NULL; } +static const struct parsed_route * +unique_routes_remove(struct group_ecmp_datapath *gn, + const struct parsed_route *route) +{ + return unique_routes_remove__(gn, route, false); +} + static void ecmp_groups_add_route(struct ecmp_groups_node *group, const struct parsed_route *route) @@ -445,7 +461,8 @@ handle_deleted_route(struct group_ecmp_route_data *data, return false; } - const struct parsed_route *existing = unique_routes_remove(node, pr); + const struct parsed_route *existing = unique_routes_remove__(node, pr, + true); if (!existing) { /* The route must be part of an ecmp group. */ if (pr->source == ROUTE_SOURCE_CONNECTED) { diff --git a/northd/en-lflow.c b/northd/en-lflow.c index e8e6cdaa2..f35736857 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -169,6 +169,13 @@ lflow_northd_handler(struct engine_node *node, return EN_UNHANDLED; } + if (!lflow_handle_northd_lrp_changes(eng_ctx->ovnsb_idl_txn, + &northd_data->trk_data.trk_lrps, + &lflow_input, + lflow_data->lflow_table)) { + return EN_UNHANDLED; + } + if (!lflow_handle_northd_lb_changes( eng_ctx->ovnsb_idl_txn, &northd_data->trk_data.trk_lbs, &lflow_input, lflow_data->lflow_table)) { diff --git a/northd/en-multicast.c b/northd/en-multicast.c index 5148d8840..bfe3c4d92 100644 --- a/northd/en-multicast.c +++ b/northd/en-multicast.c @@ -160,6 +160,17 @@ multicast_igmp_northd_handler(struct engine_node *node, void *data OVS_UNUSED) return EN_UNHANDLED; } + /* A created/deleted logical router port may join or leave the router's + * multicast groups; recompute this (cheap) node. It does not force a + * recompute of the lflow node, which consumes multicast_igmp via its own + * incremental handler. */ + struct tracked_ovn_ports *trk_lrps = &northd_data->trk_data.trk_lrps; + if (hmapx_count(&trk_lrps->created) || + hmapx_count(&trk_lrps->updated) || + hmapx_count(&trk_lrps->deleted)) { + return EN_UNHANDLED; + } + /* This node uses the below data from the en_northd engine node. * - northd_data->lr_datapaths * - northd_data->ls_ports diff --git a/northd/en-northd.c b/northd/en-northd.c index 480dc61ca..8e86f5f48 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -199,15 +199,17 @@ northd_nb_logical_router_handler(struct engine_node *node, { struct northd_data *nd = data; struct northd_input input_data; + const struct engine_context *eng_ctx = engine_get_context(); northd_get_input_data(node, &input_data); - if (!northd_handle_lr_changes(&input_data, nd)) { + if (!northd_handle_lr_changes(eng_ctx->ovnsb_idl_txn, &input_data, nd)) { return EN_UNHANDLED; } if (northd_has_lr_nats_in_tracked_data(&nd->trk_data) || northd_has_lrouters_in_tracked_data(&nd->trk_data) || + northd_has_lrps_in_tracked_data(&nd->trk_data) || northd_has_lr_route_in_tracked_data(&nd->trk_data)) { return EN_HANDLED_UPDATED; } @@ -333,23 +335,61 @@ enum engine_input_handler_result routes_northd_change_handler(struct engine_node *node, void *data OVS_UNUSED) { + struct routes_data *routes_data = data; struct northd_data *northd_data = engine_get_input_data("northd", node); if (!northd_has_tracked_data(&northd_data->trk_data)) { return EN_UNHANDLED; } - /* This node uses the below data from the en_northd engine node. - * See (lr_stateful_get_input_data()) - * 1. northd_data->lr_datapaths - * 2. northd_data->lr_ports - * This data gets updated when a logical router or logical router port - * is created or deleted. - * Northd engine node presently falls back to full recompute when - * this happens and so does this node. - * Note: When we add I-P to the created/deleted logical routers or - * logical router ports, we need to revisit this handler. - * - */ + /* This node uses northd_data->lr_datapaths and northd_data->lr_ports. + * Creating or deleting a regular logical router port changes the set of + * directly-connected routes; handle that incrementally. Any other change + * to this data is either irrelevant to routes (e.g. a portless router + * create/delete has no connected routes) or already forced a full + * recompute by the northd node. */ + struct tracked_ovn_ports *trk_lrps = &northd_data->trk_data.trk_lrps; + if (hmapx_is_empty(&trk_lrps->created) && + hmapx_is_empty(&trk_lrps->updated) && + hmapx_is_empty(&trk_lrps->deleted)) { + return EN_HANDLED_UNCHANGED; + } + + /* LRP modifications are not incrementally processed by the northd node + * (they force a recompute), so trk_lrps->updated is always empty here. */ + ovs_assert(hmapx_is_empty(&trk_lrps->updated)); + + routes_data->tracked = true; + + struct hmapx_node *hmapx_node; + struct ovn_port *op; + + /* Deleted LRPs: drop their connected routes. An LRP with N networks has + * N connected routes that share the LRP's uuid as source hint, so loop + * until none remains. */ + HMAPX_FOR_EACH (hmapx_node, &trk_lrps->deleted) { + op = hmapx_node->data; + struct parsed_route *pr; + while ((pr = parsed_route_lookup_by_source( + ROUTE_SOURCE_CONNECTED, &op->nbrp->header_, + &routes_data->parsed_routes))) { + hmap_remove(&routes_data->parsed_routes, &pr->key_node); + hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route, pr); + } + } + + /* Created LRPs: add their connected routes. */ + HMAPX_FOR_EACH (hmapx_node, &trk_lrps->created) { + op = hmapx_node->data; + parsed_routes_add_connected( + op->od, op, &routes_data->parsed_routes, + &routes_data->trk_data.trk_crupdated_parsed_route); + } + + if (!hmapx_is_empty(&routes_data->trk_data.trk_crupdated_parsed_route) || + !hmapx_is_empty(&routes_data->trk_data.trk_deleted_parsed_route)) { + return EN_HANDLED_UPDATED; + } + return EN_HANDLED_UNCHANGED; } diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c index db9cf5cf3..b51ece452 100644 --- a/northd/en-sync-sb.c +++ b/northd/en-sync-sb.c @@ -413,6 +413,8 @@ sync_to_sb_pb_northd_handler(struct engine_node *node, void *data OVS_UNUSED) sync_pbs_for_northd_changed_ovn_ports(&nd->trk_data.trk_lsps, &lr_stateful_data->table); + sync_pbs_for_northd_changed_lrps(&nd->trk_data.trk_lrps, + &lr_stateful_data->table); return EN_HANDLED_UPDATED; } diff --git a/northd/northd.c b/northd/northd.c index 63012c625..fe57d0a44 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -4380,6 +4380,29 @@ sync_pbs_for_northd_changed_ovn_ports( } } +/* Set the SB Port_Binding options (peer, dynamic-routing, ...) of created + * logical router ports. Deleted LRPs already had their SB row removed by + * lr_handle_lrp_changes(). */ +void +sync_pbs_for_northd_changed_lrps( + struct tracked_ovn_ports *trk_lrps, + const struct lr_stateful_table *lr_stateful_table) +{ + struct hmapx_node *hmapx_node; + struct ovn_port *op; + + /* LRP modifications are not incrementally processed by the northd node + * (lr_handle_lrp_changes() falls back to a full recompute), so there is + * nothing to re-sync here. Assert the invariant: enabling incremental + * LRP updates must revisit this function. */ + ovs_assert(hmapx_is_empty(&trk_lrps->updated)); + + HMAPX_FOR_EACH (hmapx_node, &trk_lrps->created) { + op = hmapx_node->data; + sync_pb_for_lrp(op, lr_stateful_table); + } +} + void sync_pbs_for_lr_stateful_changes(const struct ovn_datapath *od, const struct lr_stateful_table *lr_stateful) @@ -4669,6 +4692,7 @@ destroy_northd_data_tracked_changes(struct northd_data *nd) { struct northd_tracked_data *trk_changes = &nd->trk_data; destroy_tracked_ovn_ports(&trk_changes->trk_lsps); + destroy_tracked_ovn_ports(&trk_changes->trk_lrps); destroy_tracked_lbs(&trk_changes->trk_lbs); hmapx_clear(&trk_changes->trk_nat_lrs); hmapx_clear(&trk_changes->trk_lrs_routes); @@ -4692,6 +4716,9 @@ init_northd_tracked_data(struct northd_data *nd) hmapx_init(&trk_data->trk_lsps.created); hmapx_init(&trk_data->trk_lsps.updated); hmapx_init(&trk_data->trk_lsps.deleted); + hmapx_init(&trk_data->trk_lrps.created); + hmapx_init(&trk_data->trk_lrps.updated); + hmapx_init(&trk_data->trk_lrps.deleted); hmapx_init(&trk_data->trk_lbs.crupdated); hmapx_init(&trk_data->trk_lbs.deleted); hmapx_init(&trk_data->trk_nat_lrs); @@ -4711,6 +4738,9 @@ destroy_northd_tracked_data(struct northd_data *nd) hmapx_destroy(&trk_data->trk_switches.deleted); hmapx_destroy(&trk_data->trk_lsps.updated); hmapx_destroy(&trk_data->trk_lsps.deleted); + hmapx_destroy(&trk_data->trk_lrps.created); + hmapx_destroy(&trk_data->trk_lrps.updated); + hmapx_destroy(&trk_data->trk_lrps.deleted); hmapx_destroy(&trk_data->trk_lbs.crupdated); hmapx_destroy(&trk_data->trk_lbs.deleted); hmapx_destroy(&trk_data->trk_nat_lrs); @@ -5144,6 +5174,280 @@ ls_port_reinit(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn, sbrec_encap_by_ip); } +/* Find the logical router port 'nbrp' among the ports of the logical router + * datapath 'od'. Mirror of ovn_port_find_in_datapath() for LRPs. */ +static struct ovn_port * +ovn_lrp_find_in_datapath(struct ovn_datapath *od, + const struct nbrec_logical_router_port *nbrp) +{ + struct ovn_port *op; + HMAP_FOR_EACH_WITH_HASH (op, dp_node, hash_string(nbrp->name, 0), + &od->ports) { + if (op->nbrp == nbrp && !strcmp(op->key, nbrp->name)) { + return op; + } + } + return NULL; +} + +/* Find a logical switch port of type "router" that peers with the logical + * router port 'nbrp' (i.e. its options:router-port names 'nbrp'). */ +static struct ovn_port * +lrp_find_peer_lsp(const struct hmap *ls_ports, + const struct nbrec_logical_router_port *nbrp) +{ + struct ovn_port *op; + HMAP_FOR_EACH (op, key_node, ls_ports) { + if (op->nbsp && lsp_is_router(op->nbsp)) { + const char *rp = smap_get(&op->nbsp->options, "router-port"); + if (rp && !strcmp(rp, nbrp->name)) { + return op; + } + } + } + return NULL; +} + +/* A static route resolves its output port either by name ('output_port') or, + * lacking that, by scanning the ports of its own router for the first one + * whose networks contain the next hop (see find_static_route_outport()). + * Creating or deleting an LRP can therefore change how an already existing + * static route resolves, and routes_northd_change_handler() only adds and + * removes the connected routes of a tracked LRP - it does not re-parse the + * static ones. + * + * Return true if the LRP named 'lrp_name' with networks 'networks', belonging + * to router 'od', takes part in the resolution of any static route. */ +static bool +lrp_affects_static_routes(const struct hmap *lr_datapaths, + const struct ovn_datapath *od, + const char *lrp_name, + const struct lport_addresses *networks) +{ + const struct ovn_datapath *lr_od; + HMAP_FOR_EACH (lr_od, key_node, lr_datapaths) { + for (size_t i = 0; i < lr_od->nbr->n_static_routes; i++) { + const struct nbrec_logical_router_static_route *sr = + lr_od->nbr->static_routes[i]; + + /* A discard route has no output port to resolve. */ + if (!strcmp(sr->nexthop, "discard")) { + continue; + } + + if (sr->output_port) { + /* find_route_outport() resolves 'output_port' against the + * global set of logical router ports, so a route of any + * router can end up naming this LRP. */ + if (!strcmp(sr->output_port, lrp_name)) { + return true; + } + continue; + } + + /* Without an explicit output port only the ports of the route's + * own router are searched. */ + if (lr_od != od) { + continue; + } + + if (sr->nexthop[0] && find_lport_address(networks, sr->nexthop)) { + return true; + } + } + } + + return false; +} + +/* Symmetric counterpart of router_lsp_needs_recompute(): a logical router port + * (LRP) is not self-contained either. A full recompute wires its peer + * relationship (to a "router" LSP) and generates flows owned by lflow_refs + * other than the port's own. Return true when the LRP pulls in a dependency + * that this incremental path does not keep in sync, so the caller falls back + * to a full recompute. */ +static bool +lrp_needs_recompute(struct ovn_datapath *od, + const struct nbrec_logical_router_port *nbrp, + const struct lport_addresses *networks, + const struct hmap *ls_ports, + const struct hmap *lr_datapaths, + const struct nbrec_static_mac_binding_table *nb_smb_table) +{ + /* A Static_MAC_Binding referencing this port is synced to the SB by + * build_static_mac_binding_table(), which only runs on a full northd + * recompute; creating/deleting the port incrementally would leave the SB + * Static_MAC_Binding stale. These are rare, so fall back to recompute. */ + const struct nbrec_static_mac_binding *nb_smb; + NBREC_STATIC_MAC_BINDING_TABLE_FOR_EACH (nb_smb, nb_smb_table) { + if (!strcmp(nb_smb->logical_port, nbrp->name)) { + return true; + } + } + + /* Distributed gateway ports / cr-ports (gateway_chassis, ha_chassis_group) + * pull in chassisredirect handling not supported here. */ + if (nbrp->n_gateway_chassis || nbrp->ha_chassis_group) { + return true; + } + + /* LRP-to-LRP peering, disabled ports, prefix delegation, redirect-type and + * per-port route tables are not supported. */ + if (nbrp->peer || !lrport_is_enabled(nbrp) || + smap_get_bool(&nbrp->options, "prefix_delegation", false) || + smap_get(&nbrp->options, "redirect-type") || + smap_get(&nbrp->options, "route_table")) { + return true; + } + + /* IPv6 RA flows are owned by the port's lflow_ref but also flip + * datapath-level state; keep it simple and fall back. */ + if (!smap_is_empty(&nbrp->ipv6_ra_configs)) { + return true; + } + + /* Gateway-router / distributed-gateway complexity on this router. */ + if (od->is_gw_router || smap_get(&od->nbr->options, "chassis") || + !vector_is_empty(&od->l3dgw_ports)) { + return true; + } + + /* NAT, route policies, LBs and dynamic routing on this router pull in + * stateful/routable/advertised-route/policy dependencies that live + * outside the port's lflow_ref and are not tracked here. */ + const struct nbrec_logical_router *nbr = od->nbr; + if (nbr->n_nat || nbr->n_policies || + nbr->n_load_balancer || nbr->n_load_balancer_group) { + return true; + } + if (od->dynamic_routing || + od->dynamic_routing_redistribute != DRRM_NONE) { + return true; + } + + /* Static routes are fine as long as this port is not involved in the + * resolution of any of them. */ + if (lrp_affects_static_routes(lr_datapaths, od, nbrp->name, networks)) { + return true; + } + + /* mcast relay flips od->mcast_info and the peer switch's flood_relay. */ + if (od->mcast_info.rtr.relay) { + return true; + } + + /* Wiring an already-present peer "router" LSP from the LRP side is not + * supported; fall back to recompute. The common ordering creates the LRP + * first (no peer yet), so the peer LSP is wired later by the incremental + * LSP path (ls_router_port_wire_peer()). */ + if (lrp_find_peer_lsp(ls_ports, nbrp)) { + return true; + } + + return false; +} + +/* Initialize a newly created ovn_port for a regular (non-gateway) logical + * router port, mirroring the LRP path of join_logical_ports_lrp() plus the + * tunnel-key and SB port-binding steps of build_ports(). Ownership of + * '*lrp_networks' is transferred to 'op'. 'op->nbrp' and 'op->key' must be + * set. Returns false (leaving cleanup to the caller) on failure. */ +static bool +lr_port_init(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn, + struct ovn_datapath *od, const struct sbrec_port_binding *sb, + struct lport_addresses *lrp_networks, + struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_chassis_by_hostname, + struct ovsdb_idl_index *sbrec_encap_by_ip) +{ + op->od = od; + op->lrp_networks = *lrp_networks; + op->prefix_delegation = smap_get_bool(&op->nbrp->options, + "prefix_delegation", false); + op->dynamic_routing_redistribute = + parse_dynamic_routing_redistribute(&op->nbrp->options, + od->dynamic_routing_redistribute, + op->nbrp->name); + + for (size_t j = 0; j < op->lrp_networks.n_ipv4_addrs; j++) { + sset_add(&op->od->router_ips, op->lrp_networks.ipv4_addrs[j].addr_s); + } + for (size_t j = 0; j < op->lrp_networks.n_ipv6_addrs; j++) { + /* Exclude the LLA. */ + if (!in6_is_lla(&op->lrp_networks.ipv6_addrs[j].addr)) { + sset_add(&op->od->router_ips, + op->lrp_networks.ipv6_addrs[j].addr_s); + } + } + + /* Assign explicitly requested tunnel ids first. */ + if (!ovn_port_assign_requested_tnl_id(op)) { + return false; + } + /* Keep a nonconflicting tunnel ID that is already assigned. */ + if (sb && !op->tunnel_key) { + ovn_port_add_tnlid(op, sb->tunnel_key); + } + /* Assign a new tunnel id if needed. */ + if (!ovn_port_allocate_key(op)) { + return false; + } + /* Create the SB port binding, if needed. */ + if (sb) { + op->sb = sb; + } else { + op->sb = sbrec_port_binding_insert(ovnsb_txn); + sbrec_port_binding_set_logical_port(op->sb, op->key); + } + /* A regular LRP takes the non-gateway "patch" branch of + * ovn_port_update_sbrec(), so the ha-chassis-group / mirror / queue-id + * arguments are unused and passing NULL is safe. The SB options are set + * later by sync_pb_for_lrp(). */ + ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name, + sbrec_chassis_by_hostname, NULL, NULL, + sbrec_encap_by_ip, op, NULL, NULL); + return true; +} + +/* Create an ovn_port for a regular logical router port and insert it into + * 'lr_ports' and 'od->ports'. Returns NULL (after cleaning up) on failure. */ +static struct ovn_port * +lr_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *lr_ports, + const char *key, + const struct nbrec_logical_router_port *nbrp, + struct ovn_datapath *od, struct lport_addresses *lrp_networks, + struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_chassis_by_hostname, + struct ovsdb_idl_index *sbrec_encap_by_ip) +{ + struct ovn_port *op = ovn_port_create(lr_ports, key, NULL, nbrp, NULL); + hmap_insert(&od->ports, &op->dp_node, hmap_node_hash(&op->key_node)); + if (!lr_port_init(op, ovnsb_txn, od, NULL, lrp_networks, + sbrec_chassis_by_name, sbrec_chassis_by_hostname, + sbrec_encap_by_ip)) { + ovn_port_destroy(lr_ports, op); + return NULL; + } + ipam_add_lrp_port_addresses(op); + return op; +} + +/* Remove the router-port IPs of 'op' from od->router_ips on deletion. */ +static void +lr_port_remove_router_ips(struct ovn_port *op) +{ + for (size_t j = 0; j < op->lrp_networks.n_ipv4_addrs; j++) { + sset_find_and_delete(&op->od->router_ips, + op->lrp_networks.ipv4_addrs[j].addr_s); + } + for (size_t j = 0; j < op->lrp_networks.n_ipv6_addrs; j++) { + if (!in6_is_lla(&op->lrp_networks.ipv6_addrs[j].addr)) { + sset_find_and_delete(&op->od->router_ips, + op->lrp_networks.ipv6_addrs[j].addr_s); + } + } +} + /* Returns true if the logical switch has changes which can be * incrementally handled. * Presently supports i-p for the below changes: @@ -5889,6 +6193,7 @@ lr_changes_can_be_handled(const struct nbrec_logical_router *lr) if (col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER || col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER_GROUP || col == NBREC_LOGICAL_ROUTER_COL_NAT + || col == NBREC_LOGICAL_ROUTER_COL_PORTS || col == NBREC_LOGICAL_ROUTER_COL_STATIC_ROUTES) { continue; } @@ -5896,14 +6201,12 @@ lr_changes_can_be_handled(const struct nbrec_logical_router *lr) } } + /* Note: changes to the referenced logical router port rows are classified + * per-port in lr_handle_lrp_changes() (created ports are handled; modified + * ports fall back to a full recompute). */ + /* Check if the referenced rows are changed. XXX: Need a better OVSDB IDL interface for this check. */ - for (size_t i = 0; i < lr->n_ports; i++) { - if (nbrec_logical_router_port_row_get_seqno(lr->ports[i], - OVSDB_IDL_CHANGE_MODIFY) > 0) { - return false; - } - } if (lr->copp && nbrec_copp_row_get_seqno(lr->copp, OVSDB_IDL_CHANGE_MODIFY) > 0) { return false; @@ -5961,6 +6264,135 @@ is_lr_static_routes_changed(const struct nbrec_logical_router *nbr) || is_lr_static_routes_seqno_changed(nbr); } +/* Handles logical router port changes of a changed (created, updated or + * deleted) logical router 'changed_lr' with datapath 'od'. Regular + * (non-gateway) LRPs are created and deleted incrementally; anything that + * pulls in dependencies not tracked here (see lrp_needs_recompute()) or a + * modification of an existing LRP falls back to a full recompute. + * + * Returns false if any port can't be incrementally handled. */ +static bool +lr_handle_lrp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, + const struct nbrec_logical_router *changed_lr, + const struct northd_input *ni, struct northd_data *nd, + struct ovn_datapath *od, + struct tracked_ovn_ports *trk_lrps) +{ + bool lr_deleted = nbrec_logical_router_is_deleted(changed_lr); + bool lr_ports_changed = lr_deleted; + if (!nbrec_logical_router_is_updated(changed_lr, + NBREC_LOGICAL_ROUTER_COL_PORTS)) { + for (size_t i = 0; i < changed_lr->n_ports; i++) { + if (nbrec_logical_router_port_row_get_seqno( + changed_lr->ports[i], OVSDB_IDL_CHANGE_MODIFY) > 0 || + !ovn_lrp_find_in_datapath(od, changed_lr->ports[i])) { + lr_ports_changed = true; + break; + } + } + } else { + lr_ports_changed = true; + } + + if (!lr_ports_changed) { + return true; + } + + struct ovn_port *op; + HMAP_FOR_EACH (op, dp_node, &od->ports) { + op->visited = false; + } + + /* Create newly added LRPs. Modifications fall back to recompute. */ + if (!lr_deleted) { + for (size_t j = 0; j < changed_lr->n_ports; j++) { + struct nbrec_logical_router_port *new_nbrp = changed_lr->ports[j]; + op = ovn_lrp_find_in_datapath(od, new_nbrp); + + if (!op) { + struct lport_addresses lrp_networks; + if (!extract_lrp_networks(new_nbrp, &lrp_networks)) { + goto fail; + } + if (!lrp_networks.n_ipv4_addrs && + !lrp_networks.n_ipv6_addrs) { + /* join_logical_ports_lrp() skips such ports. */ + destroy_lport_addresses(&lrp_networks); + goto fail; + } + if (lrp_needs_recompute(od, new_nbrp, &lrp_networks, + &nd->ls_ports, + &nd->lr_datapaths.datapaths, + ni->nbrec_static_mac_binding_table)) { + destroy_lport_addresses(&lrp_networks); + goto fail; + } + op = lr_port_create(ovnsb_idl_txn, &nd->lr_ports, + new_nbrp->name, new_nbrp, od, + &lrp_networks, + ni->sbrec_chassis_by_name, + ni->sbrec_chassis_by_hostname, + ni->sbrec_encap_by_ip); + if (!op) { + goto fail; + } + add_op_to_northd_tracked_ports(&trk_lrps->created, op); + /* Datapath-level router flows that iterate the router's ports + * (e.g. build_lrouter_network_id_flows()) live on + * od->datapath_lflows; retrack the router so the lflow engine + * regenerates them. */ + hmapx_add(&nd->trk_data.trk_routers.crupdated, od); + } else if (nbrec_logical_router_port_row_get_seqno( + new_nbrp, OVSDB_IDL_CHANGE_MODIFY) > 0) { + /* Re-initializing an LRP (re-wiring its peer, networks, ...) + * is not supported yet. */ + goto fail; + } + op->visited = true; + } + } + + /* Delete removed LRPs. */ + bool lrp_deleted = false; + HMAP_FOR_EACH_SAFE (op, dp_node, &od->ports) { + if (op->visited || !op->nbrp) { + continue; + } + /* cr-ports are synthetic and never appear in nbr->ports; a router + * with one is a distributed gateway router which is out of scope. */ + if (is_cr_port(op) || + lrp_needs_recompute(od, op->nbrp, &op->lrp_networks, + &nd->ls_ports, &nd->lr_datapaths.datapaths, + ni->nbrec_static_mac_binding_table)) { + goto fail; + } + lr_port_remove_router_ips(op); + add_op_to_northd_tracked_ports(&trk_lrps->deleted, op); + hmap_remove(&nd->lr_ports, &op->key_node); + hmap_remove(&od->ports, &op->dp_node); + sbrec_port_binding_delete(op->sb); + /* Regenerate the router's datapath-level flows (see the create + * path above). */ + hmapx_add(&nd->trk_data.trk_routers.crupdated, od); + lrp_deleted = true; + } + + /* Purge SB MAC_Bindings learned on the just-deleted LRPs. A full + * recompute does this in build_ports() via cleanup_mac_bindings(); the + * incremental path must do it too, otherwise MAC_Bindings referencing a + * deleted logical router port are leaked in the southbound DB. */ + if (lrp_deleted) { + cleanup_mac_bindings(ni->sbrec_mac_binding_table, + &nd->lr_datapaths.datapaths, &nd->lr_ports); + } + + return true; + +fail: + destroy_tracked_ovn_ports(trk_lrps); + return false; +} + /* Return true if changes are handled incrementally, false otherwise. * * Note: Changes to load balancer and load balancer groups associated with @@ -5968,7 +6400,8 @@ is_lr_static_routes_changed(const struct nbrec_logical_router *nbr) * handler - northd_handle_lb_data_changes(). * */ bool -northd_handle_lr_changes(const struct northd_input *ni, +northd_handle_lr_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, + const struct northd_input *ni, struct northd_data *nd) { const struct nbrec_logical_router *changed_lr; @@ -5984,9 +6417,9 @@ northd_handle_lr_changes(const struct northd_input *ni, const struct ovn_synced_logical_router *synced = node->data; const struct nbrec_logical_router *new_lr = synced->nb; - /* If the logical router is create with the below columns set, + /* If the logical router is created with the below columns set, * then we can't handle it in the incremental processor goto fail. */ - if (new_lr->copp || (new_lr->n_ports > 0)) { + if (new_lr->copp) { goto fail; } if (sparse_array_get(&nd->lr_datapaths.dps, synced->sdp->index)) { @@ -6005,6 +6438,20 @@ northd_handle_lr_changes(const struct northd_input *ni, od->nbr->name); hmapx_add(&nd->trk_data.trk_nat_lrs,od); hmapx_add(&nd->trk_data.trk_routers.crupdated, od); + + /* A new router can come with static routes already attached. Only + * the updated-router loop below tracks those, so track them here as + * well; otherwise routes_static_route_change_handler() never sees the + * routes of a newly created router and they are silently dropped. */ + if (new_lr->n_static_routes) { + hmapx_add(&nd->trk_data.trk_lrs_routes, od); + } + + /* Create any logical router ports the new router already has. */ + if (!lr_handle_lrp_changes(ovnsb_idl_txn, new_lr, ni, nd, od, + &nd->trk_data.trk_lrps)) { + goto fail; + } } HMAPX_FOR_EACH (node, &ni->synced_lrs->updated) { @@ -6012,11 +6459,27 @@ northd_handle_lr_changes(const struct northd_input *ni, changed_lr = synced->nb; /* Presently only able to handle load balancer, - * load balancer group changes and NAT changes. */ + * load balancer group changes, NAT changes and (regular) logical + * router port creation/deletion. */ if (!lr_changes_can_be_handled(changed_lr)) { goto fail; } + struct ovn_datapath *lrp_od = ovn_datapath_find_( + &nd->lr_datapaths.datapaths, + &changed_lr->header_.uuid); + if (!lrp_od) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "Internal error: a tracked updated LR " + "doesn't exist in lr_datapaths: "UUID_FMT, + UUID_ARGS(&changed_lr->header_.uuid)); + goto fail; + } + if (!lr_handle_lrp_changes(ovnsb_idl_txn, changed_lr, ni, nd, lrp_od, + &nd->trk_data.trk_lrps)) { + goto fail; + } + if (is_lr_nats_changed(changed_lr)) { struct ovn_datapath *od = ovn_datapath_find_( &nd->lr_datapaths.datapaths, @@ -6091,6 +6554,10 @@ northd_handle_lr_changes(const struct northd_input *ni, if (!hmapx_is_empty(&nd->trk_data.trk_lrs_routes)) { nd->trk_data.type |= NORTHD_TRACKED_LR_ROUTES; } + if (!hmapx_is_empty(&nd->trk_data.trk_lrps.created) || + !hmapx_is_empty(&nd->trk_data.trk_lrps.deleted)) { + nd->trk_data.type |= NORTHD_TRACKED_LR_PORTS; + } if (!hmapx_is_empty(&nd->trk_data.trk_routers.crupdated) || !hmapx_is_empty(&nd->trk_data.trk_routers.deleted)) { nd->trk_data.type |= NORTHD_TRACKED_ROUTERS; @@ -13249,29 +13716,41 @@ parsed_routes_add_static(const struct ovn_datapath *od, return pr; } -static void +/* Add the directly-connected routes for the logical router port 'op' to + * 'routes'. When 'trk_crupdated' is non-NULL each newly created parsed_route + * is added to it (used by the incremental routes handler). */ +void parsed_routes_add_connected(const struct ovn_datapath *od, const struct ovn_port *op, - struct hmap *routes) + struct hmap *routes, + struct hmapx *trk_crupdated) { for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { const struct ipv4_netaddr *addr = &op->lrp_networks.ipv4_addrs[i]; struct in6_addr prefix; in6_addr_set_mapped_ipv4(&prefix, addr->network); - parsed_route_add(od, NULL, &prefix, addr->plen, + struct parsed_route *pr = parsed_route_add( + od, NULL, &prefix, addr->plen, false, addr->addr_s, op, 0, false, false, false, NULL, ROUTE_SOURCE_CONNECTED, true, &op->nbrp->header_, NULL, routes); + if (trk_crupdated && pr) { + hmapx_add(trk_crupdated, pr); + } } for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { const struct ipv6_netaddr *addr = &op->lrp_networks.ipv6_addrs[i]; - parsed_route_add(od, NULL, &addr->network, addr->plen, false, + struct parsed_route *pr = parsed_route_add( + od, NULL, &addr->network, addr->plen, false, addr->addr_s, op, 0, false, false, false, NULL, ROUTE_SOURCE_CONNECTED, true, &op->nbrp->header_, NULL, routes); + if (trk_crupdated && pr) { + hmapx_add(trk_crupdated, pr); + } } } @@ -13289,7 +13768,7 @@ build_parsed_routes(const struct ovn_datapath *od, const struct hmap *lr_ports, const struct ovn_port *op; HMAP_FOR_EACH (op, dp_node, &od->ports) { - parsed_routes_add_connected(od, op, routes); + parsed_routes_add_connected(od, op, routes, NULL); } } @@ -15976,11 +16455,22 @@ static void build_route_flows_for_lrouter( struct ovn_datapath *od, struct lflow_table *lflows, const struct group_ecmp_route_data *route_data, - struct simap *route_tables, const struct sset *bfd_ports) + struct simap *route_tables, const struct sset *bfd_ports, + bool skip_data_route_flows) { ovs_assert(od->nbr); + /* The default route drop flows are owned by od->datapath_lflows. */ build_default_route_flows_for_lrouter(od, lflows, route_tables); + /* The per-route flows are owned by the group_ecmp_route node's + * lflow_ref, which is managed incrementally by + * lflow_group_ecmp_route_change_handler(). Skip them when regenerating a + * router's datapath flows incrementally to avoid double-adding them to + * that ref. */ + if (skip_data_route_flows) { + return; + } + const struct group_ecmp_datapath *datapath_node = group_ecmp_datapath_lookup(route_data, od); if (!datapath_node) { @@ -20734,6 +21224,14 @@ struct lswitch_flow_build_info { struct hmap *route_policies; struct simap *route_tables; const struct sbrec_acl_id_table *sbrec_acl_id_table; + + /* When true, build_lswitch_and_lrouter_iterate_by_lr() skips the logical + * router route flows. Those flows are owned by the group_ecmp_route + * node's lflow_ref, which is managed incrementally by + * lflow_group_ecmp_route_change_handler(); regenerating them here (e.g. + * when a router is retracked because its ports changed) would double-add + * them to that ref and corrupt its refcounts. */ + bool skip_route_flows; }; /* Helper function to combine all lflow generation which is iterated by @@ -20794,7 +21292,7 @@ build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, od->datapath_lflows); build_route_flows_for_lrouter(od, lsi->lflows, lsi->route_data, lsi->route_tables, - lsi->bfd_ports); + lsi->bfd_ports, lsi->skip_route_flows); build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, od->datapath_lflows); build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, @@ -21483,12 +21981,18 @@ lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsb_txn, } struct lswitch_flow_build_info lsi = { + .ls_ports = lflow_input->ls_ports, + .lr_ports = lflow_input->lr_ports, .lr_datapaths = lflow_input->lr_datapaths, .lr_stateful_table = lflow_input->lr_stateful_table, + .meter_groups = lflow_input->meter_groups, + .bfd_ports = lflow_input->bfd_ports, + .features = lflow_input->features, .lflows = lflows, .route_data = lflow_input->route_data, .route_tables = lflow_input->route_tables, .route_policies = lflow_input->route_policies, + .skip_route_flows = true, .match = DS_EMPTY_INITIALIZER, .actions = DS_EMPTY_INITIALIZER, }; @@ -21692,6 +22196,93 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, return handled; } +/* Regenerate the logical flows for created and deleted logical router ports + * tracked in 'trk_lrps'. Mirrors lflow_handle_northd_port_changes() but for + * LRPs: their flows live on op->lflow_ref (per-port routing/ARP/... flows via + * build_lswitch_and_lrouter_iterate_by_lrp()) and op->stateful_lflow_ref + * (LB/NAT flows via build_lbnat_lflows_iterate_by_lrp()). */ +bool +lflow_handle_northd_lrp_changes(struct ovsdb_idl_txn *ovnsb_txn, + struct tracked_ovn_ports *trk_lrps, + struct lflow_input *lflow_input, + struct lflow_table *lflows) +{ + struct hmapx_node *hmapx_node; + struct ovn_port *op; + + /* LRP modifications are not incrementally processed by the northd node + * (lr_handle_lrp_changes() falls back to a full recompute), so there are + * no updated LRPs to regenerate flows for. Assert the invariant: + * enabling incremental LRP updates must revisit this function. */ + ovs_assert(hmapx_is_empty(&trk_lrps->updated)); + + struct lswitch_flow_build_info lsi = { + .ls_ports = lflow_input->ls_ports, + .lr_ports = lflow_input->lr_ports, + .lr_datapaths = lflow_input->lr_datapaths, + .lr_stateful_table = lflow_input->lr_stateful_table, + .meter_groups = lflow_input->meter_groups, + .bfd_ports = lflow_input->bfd_ports, + .lflows = lflows, + .match = DS_EMPTY_INITIALIZER, + .actions = DS_EMPTY_INITIALIZER, + }; + + HMAPX_FOR_EACH (hmapx_node, &trk_lrps->deleted) { + op = hmapx_node->data; + ovs_assert(op->nbrp); + bool handled = lflow_ref_resync_flows( + op->lflow_ref, lflows, ovnsb_txn, lflow_input->dps, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + if (handled) { + handled = lflow_ref_resync_flows( + op->stateful_lflow_ref, lflows, ovnsb_txn, lflow_input->dps, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + } + if (!handled) { + goto out; + } + } + + HMAPX_FOR_EACH (hmapx_node, &trk_lrps->created) { + op = hmapx_node->data; + ovs_assert(op->nbrp); + build_lswitch_and_lrouter_iterate_by_lrp(op, &lsi); + bool handled = lflow_ref_sync_lflows( + op->lflow_ref, lflows, ovnsb_txn, lflow_input->dps, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + if (handled) { + build_lbnat_lflows_iterate_by_lrp( + op, lflow_input->lr_stateful_table, + lflow_input->meter_groups, lflow_input->bfd_ports, + &lsi.match, &lsi.actions, lflows); + handled = lflow_ref_sync_lflows( + op->stateful_lflow_ref, lflows, ovnsb_txn, lflow_input->dps, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + } + if (!handled) { + goto out; + } + } + + ds_destroy(&lsi.match); + ds_destroy(&lsi.actions); + return true; + +out: + ds_destroy(&lsi.match); + ds_destroy(&lsi.actions); + return false; +} + bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_lbs *trk_lbs, diff --git a/northd/northd.h b/northd/northd.h index 9a74a4abc..8aab69823 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -160,6 +160,7 @@ enum northd_tracked_data_type { NORTHD_TRACKED_SWITCHES = (1 << 5), NORTHD_TRACKED_ROUTERS = (1 << 6), NORTHD_TRACKED_LR_ROUTES = (1 << 7), + NORTHD_TRACKED_LR_PORTS = (1 << 8), }; /* Track what's changed in the northd engine node. @@ -171,6 +172,10 @@ struct northd_tracked_data { struct tracked_dps trk_switches; struct tracked_dps trk_routers; struct tracked_ovn_ports trk_lsps; + + /* Tracked created/updated/deleted logical router ports. + * hmapx node data is 'struct ovn_port *'. */ + struct tracked_ovn_ports trk_lrps; struct tracked_lbs trk_lbs; /* Tracked logical routers whose NATs have changed. @@ -916,6 +921,11 @@ struct parsed_route *parsed_routes_add_static( struct hmap *routes, struct simap *route_tables, struct hmap *bfd_active_connections); +void parsed_routes_add_connected(const struct ovn_datapath *od, + const struct ovn_port *op, + struct hmap *routes, + struct hmapx *trk_crupdated); + struct svc_monitors_map_data { const struct hmap *local_svc_monitors_map; const struct hmap *ic_learned_svc_monitors_map; @@ -941,7 +951,8 @@ void ovnsb_db_run(struct ovsdb_idl_txn *ovnsb_txn, bool northd_handle_ls_changes(struct ovsdb_idl_txn *, const struct northd_input *, struct northd_data *); -bool northd_handle_lr_changes(const struct northd_input *, +bool northd_handle_lr_changes(struct ovsdb_idl_txn *, + const struct northd_input *, struct northd_data *); bool northd_handle_pgs_acl_changes(const struct northd_input *ni, struct northd_data *nd); @@ -996,6 +1007,10 @@ bool lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_ovn_ports *, struct lflow_input *, struct lflow_table *lflows); +bool lflow_handle_northd_lrp_changes(struct ovsdb_idl_txn *ovnsb_txn, + struct tracked_ovn_ports *, + struct lflow_input *, + struct lflow_table *lflows); bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_lbs *, struct lflow_input *, @@ -1048,6 +1063,10 @@ void sync_pbs_for_northd_changed_ovn_ports( struct tracked_ovn_ports *, const struct lr_stateful_table *); +void sync_pbs_for_northd_changed_lrps( + struct tracked_ovn_ports *, + const struct lr_stateful_table *); + void sync_pbs_for_lr_stateful_changes( const struct ovn_datapath *od, const struct lr_stateful_table *lr_stateful); @@ -1069,6 +1088,12 @@ northd_has_lsps_in_tracked_data(struct northd_tracked_data *trk_nd_changes) return trk_nd_changes->type & NORTHD_TRACKED_PORTS; } +static inline bool +northd_has_lrps_in_tracked_data(struct northd_tracked_data *trk_nd_changes) +{ + return trk_nd_changes->type & NORTHD_TRACKED_LR_PORTS; +} + static inline bool northd_has_lr_nats_in_tracked_data(struct northd_tracked_data *trk_nd_changes) { diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index a57858a55..a2a84dbcd 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -12720,6 +12720,222 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE OVN_CLEANUP_NORTHD AT_CLEANUP +AT_SETUP([Logical router port incremental processing]) +AT_KEYWORDS([incremental processing]) +ovn_start + +check ovn-nbctl ls-add sw0 +check ovn-nbctl --wait=sb lr-add lr0 + +# Adding a regular router port to a bare router (no peer switch port yet) is +# incrementally processed. +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_engine_compute northd incremental +check_engine_compute lflow incremental + +# The router port's SB Port_Binding was created. +AT_CHECK([ovn-sbctl get port_binding lr0-sw0 type], [0], [dnl +patch +]) +# The directly-connected route flow is present on the router. +AT_CHECK([ovn-sbctl dump-flows lr0 | grep -c 'ip4.dst == 10.0.0.0/24'], [0], + [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Adding a second regular router port is also incremental. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24 +check_engine_compute northd incremental +check_engine_compute lflow incremental +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Deleting a router port is incrementally processed. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr0-sw1 +check_engine_compute northd incremental +check_engine_compute lflow incremental +AT_CHECK([ovn-sbctl find port_binding logical_port=lr0-sw1], [0], []) +AT_CHECK([ovn-sbctl dump-flows lr0 | grep -c 'ip4.dst == 20.0.0.0/24'], [1], + [0 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +OVN_CLEANUP_NORTHD +AT_CLEANUP + +AT_SETUP([Logical router port incremental processing - link-local routes]) +AT_KEYWORDS([incremental processing]) +ovn_start + +# Every router port gets an IPv6 link-local address, hence a connected route +# for the fe80::/64 prefix. Deleting one router port must remove that port's +# route only, not another port's route to the same prefix. +check ovn-nbctl lr-add lr0 +check ovn-nbctl lrp-add lr0 lr0-p1 00:00:00:00:ff:01 10.0.0.1/24 +check ovn-nbctl lrp-add lr0 lr0-p2 00:00:00:00:ff:02 20.0.0.1/24 +check ovn-nbctl --wait=sb lrp-add lr0 lr0-p3 00:00:00:00:ff:03 30.0.0.1/24 + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr0-p1 +check_engine_compute northd incremental +check_engine_compute lflow incremental + +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ + grep "ip6.dst == fe80::/64" | grep -o 'inport == "[[a-z0-9-]]*"' | sort], + [0], [dnl +inport == "lr0-p2" +inport == "lr0-p3" +]) + +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +OVN_CLEANUP_NORTHD +AT_CLEANUP + +AT_SETUP([Logical router port incremental processing - static routes]) +AT_KEYWORDS([incremental processing]) +ovn_start + +check ovn-nbctl lr-add lr0 +check ovn-nbctl --wait=sb lrp-add lr0 lr0-p1 00:00:00:00:ff:01 10.0.0.1/24 + +# A "discard" route has no output port to resolve, so adding a router port to +# a router that has one is still incrementally processed. +check ovn-nbctl --wait=sb lr-route-add lr0 8.8.8.8 discard +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr0 lr0-p2 00:00:00:00:ff:02 20.0.0.1/24 +check_engine_compute northd incremental +check_engine_compute lflow incremental +AT_CHECK([ovn-sbctl dump-flows lr0 | grep -c 'ip4.dst == 20.0.0.0/24'], [0], + [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# The same holds for a route whose next hop is reachable through another +# router port: the new port does not take part in its resolution. +check ovn-nbctl --wait=sb lr-route-add lr0 9.9.9.9 10.0.0.2 +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr0 lr0-p3 00:00:00:00:ff:03 30.0.0.1/24 +check_engine_compute northd incremental +check_engine_compute lflow incremental +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ + grep -c 'ip4.dst == 9.9.9.9/32'], [0], [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Deleting a router port no static route resolves through is incremental too. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr0-p3 +check_engine_compute northd incremental +check_engine_compute lflow incremental +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# A router created together with its static routes keeps them: the new router +# is tracked for the routes engine even though no port change happened. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-add lr1 -- lr-route-add lr1 8.8.8.8 discard +check_engine_compute northd incremental +AT_CHECK([ovn-sbctl dump-flows lr1 | grep lr_in_ip_routing | \ + grep -c 'ip4.dst == 8.8.8.8/32'], [0], [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Same when the router, its port and its routes are all created at once. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-add lr2 \ + -- lrp-add lr2 lr2-p1 00:00:00:00:ff:21 40.0.0.1/24 \ + -- lr-route-add lr2 8.8.8.8 discard +check_engine_compute northd incremental +AT_CHECK([ovn-sbctl dump-flows lr2 | grep lr_in_ip_routing | \ + grep -c 'ip4.dst == 8.8.8.8/32'], [0], [1 +]) +AT_CHECK([ovn-sbctl dump-flows lr2 | grep -c 'ip4.dst == 40.0.0.0/24'], [0], + [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +OVN_CLEANUP_NORTHD +AT_CLEANUP + +AT_SETUP([Logical router port incremental processing fallback cases]) +AT_KEYWORDS([incremental processing]) +ovn_start + +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 + +# Adding a distributed gateway port falls back to recompute (cr-port). +check ovn-nbctl --wait=sb lr-add lr0 +check ovn-nbctl ls-add public +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24 +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr0-public gw1 20 +check_engine_compute northd recompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Adding a router port to a dynamic-routing router falls back to recompute. +check ovn-nbctl --wait=sb lr-add lr1 \ + -- set Logical_Router lr1 options:dynamic-routing=true +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr1 lr1-sw0 00:00:00:00:ff:11 10.1.0.1/24 +check_engine_compute northd recompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Adding a router port whose peer "router" LSP already exists falls back to +# recompute (wiring the peer from the LRP side is not supported). +check ovn-nbctl --wait=sb lr-add lr2 +check ovn-nbctl ls-add sw2 +check ovn-nbctl lsp-add sw2 sw2-lr2 +check ovn-nbctl lsp-set-type sw2-lr2 router +check ovn-nbctl --wait=sb lsp-set-options sw2-lr2 router-port=lr2-sw2 +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr2 lr2-sw2 00:00:00:00:ff:21 10.2.0.1/24 +check_engine_compute northd recompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Modifying an existing router port falls back to recompute. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb set logical_router_port lr2-sw2 options:foo=bar +check_engine_compute northd recompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Adding a router port whose network covers the next hop of an existing static +# route falls back to recompute: the route starts resolving through the new +# port. +check ovn-nbctl --wait=sb lr-add lr3 +check ovn-nbctl lrp-add lr3 lr3-sw0 00:00:00:00:ff:31 10.3.0.1/24 +check ovn-nbctl --wait=sb lr-route-add lr3 9.9.9.9 10.4.0.2 +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr3 lr3-sw1 00:00:00:00:ff:32 10.4.0.1/24 +check_engine_compute northd recompute +AT_CHECK([ovn-sbctl dump-flows lr3 | grep lr_in_ip_routing | \ + grep -c 'ip4.dst == 9.9.9.9/32'], [0], [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# So does deleting the router port a static route resolves through. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr3-sw1 +check_engine_compute northd recompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# A static route naming the port in 'output_port' forces a recompute as well, +# even though the next hop belongs to another port's network. +check ovn-nbctl --wait=sb lr-add lr4 +check ovn-nbctl lrp-add lr4 lr4-sw0 00:00:00:00:ff:41 10.5.0.1/24 +check ovn-nbctl --wait=sb lr-route-add lr4 7.7.7.7 10.5.0.2 +route=$(ovn-nbctl --bare --columns=static_routes list logical_router lr4) +check ovn-nbctl --wait=sb set logical_router_static_route $route \ + output_port=lr4-sw1 +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr4 lr4-sw1 00:00:00:00:ff:42 10.6.0.1/24 +check_engine_compute northd recompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +OVN_CLEANUP_NORTHD +AT_CLEANUP + OVN_FOR_EACH_NORTHD_NO_HV([ AT_SETUP([SB Port binding incremental processing]) ovn_start @@ -12789,13 +13005,14 @@ check ovn-nbctl --wait=sb lsp-set-options e1 foo=bar check_recompute_counter 1 1 CHECK_NO_CHANGE_AFTER_RECOMPUTE -# Test lrp +# Test lrp. Adding a regular router port to a bare router with no peer switch +# port is incrementally processed. check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb lrp-add lr0 lrp 00:00:02:01:02:03 10.0.0.1/24 -check_recompute_counter 1 1 +check_recompute_counter 0 0 CHECK_NO_CHANGE_AFTER_RECOMPUTE -# Set some options on 'lrp'. northd should only recompute once. +# Set some options on 'lrp'. A router-port modification falls back to recompute. check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb lrp-set-options lrp route_table=rtb-1 check_recompute_counter 1 1 @@ -14202,18 +14419,13 @@ check_engine_stats lflow norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check ovn-nbctl --wait=sb lr-add lr0 -# Adding a logical router port should result in recompute +# Adding a regular logical router port to a bare router (no peer switch port +# yet) is incrementally processed. 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 -# for northd engine there will be both recompute and compute -# first it will be recompute to handle lr0-sw0 and then a compute -# for the SB port binding change. -check_engine_stats northd recompute compute -check_engine_stats lr_nat recompute nocompute -check_engine_stats lr_stateful recompute nocompute -check_engine_stats sync_to_sb_pb recompute nocompute -check_engine_stats sync_to_sb_lb recompute nocompute -check_engine_stats lflow recompute nocompute +check_engine_compute northd incremental +check_engine_compute lflow incremental +check_engine_stats sync_to_sb_pb norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check ovn-nbctl lsp-add sw0 sw0-lr0 -- 2.43.0 -- _'Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas'._ * **'Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
