Until now any change to a logical router's ports fell back to a full recompute, since lr_changes_can_be_handled() rejected the LOGICAL_ROUTER "ports" column and any LRP row change. Handle creation and deletion of regular (non-gateway) LRPs incrementally, mirroring the router LSP path of the preceding commit.
lr_port_create()/lr_port_init() mirror the LRP branch of join_logical_ports() plus the build_ports() post-processing (tunnel key, SB Port_Binding, od->router_ips), and connected routes are added/removed through the routes -> group_ecmp_route -> lflow chain. The port's flows are regenerated on op->lflow_ref and op->stateful_lflow_ref, and the per-datapath ones by re-tracking the router datapath. lrp_needs_recompute() falls back to a recompute for what this path does not keep in sync: the port being a distributed gateway port, gateway routers, LRP<->LRP peering, 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 on the port. lrp_affects_static_routes() also recomputes when the LRP is named in a static route's output_port or its networks cover the next hop of one of its router's routes. Updates of an existing LRP also recompute; consumers assert that only create/delete reach them. Also fix two latent bugs exposed by this: - unique_routes_remove() matched routes by prefix hash only, so deleting one of several connected routes sharing a prefix (e.g. fe80::/64) could remove the wrong one. Match out_port on deletion. - northd_handle_lr_changes() did not track static routes of a newly created router, so they were silently dropped. Add tests for incremental LRP create/delete on plain routers, routers with multiple fe80::/64 routes, unrelated static routes, NAT (including an is_router_ip flip), a distributed gateway port, a router created together with its ports and routes, and for each recompute fallback. 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 | 5 + northd/en-lr-stateful.c | 16 +- northd/en-multicast.c | 11 + northd/en-northd.c | 66 +++- northd/en-sync-sb.c | 2 + northd/northd.c | 628 ++++++++++++++++++++++++++++++++++- northd/northd.h | 27 +- tests/ovn-northd.at | 408 ++++++++++++++++++++++- 9 files changed, 1138 insertions(+), 54 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 a73d55486..a7291c847 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -157,6 +157,11 @@ lflow_northd_handler(struct engine_node *node, lflow_data->lflow_table, &lflow_data->trk_data.dirty_lflow_refs); + lflow_handle_northd_lrp_changes(&northd_data->trk_data.trk_lrps, + &lflow_input, + lflow_data->lflow_table, + &lflow_data->trk_data.dirty_lflow_refs); + lflow_handle_northd_lb_changes(&northd_data->trk_data.trk_lbs, &lflow_input, lflow_data->lflow_table, diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c index f9f6586e5..7f4abeccd 100644 --- a/northd/en-lr-stateful.c +++ b/northd/en-lr-stateful.c @@ -149,10 +149,18 @@ lr_stateful_northd_handler(struct engine_node *node, void *data OVS_UNUSED) * This data gets updated when a logical router is created or deleted. * * This node also accesses the router ports of the logical router - * (od->ports). When these logical router ports gets updated, - * en_northd engine recomputes and so does this node. - * Note: When we add I-P to handle router port changes, we need - * to revisit this handler. + * (od->ports), but only through build_lrouter_lb_reachable_ips() + * and od_has_lb_vip(), i.e. only for routers that have load + * balancers attached. en_northd handles logical router port + * creation/deletion incrementally only for routers without load + * balancers (see lrp_needs_recompute()); for any other router port + * change en_northd recomputes and so does this node. The port set + * also feeds the lr_nat record (ovn_nat.is_router_ip and + * ovn_nat.l3dgw_port), and en_northd tracks such routers in + * 'trk_nat_lrs' so that this node is updated through its en_lr_nat + * input instead. + * Note: When we relax either restriction, we need to revisit this + * handler. * * 2. northd_data->lb_datapaths_map * 3. northd_data->lb_group_datapaths_map 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 96859e4ae..f724f4fbf 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 e45a9d64a..285563ea9 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) @@ -4678,6 +4701,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); @@ -4701,6 +4725,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); @@ -4720,6 +4747,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); @@ -5157,6 +5187,300 @@ 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 complexity on this router. */ + if (od->is_gw_router || smap_get(&od->nbr->options, "chassis")) { + return true; + } + + /* A distributed gateway port (DGP) elsewhere on the router is fine. The + * set of DGPs does not change here, and the flows of the router that walk + * its ports alongside its DGPs either live on od->datapath_lflows (e.g. + * build_check_pkt_len_flows_for_lrouter() for a DGP with gateway_mtu), + * which lr_handle_lrp_changes() regenerates by re-tracking the router, or + * come from the lr_nat and lr_stateful records, which are refreshed + * through trk_nat_lrs for a router with NAT (see below). What the port's + * own flows gain from a DGP, the is_chassis_resident() checks of + * build_lrouter_ipv4_ip_input() and friends, only applies once the port + * has a peer switch port with a localnet port, and a pre-existing peer + * falls back below. */ + + /* Route policies, LBs and dynamic routing on this router pull in + * routable/advertised-route/policy dependencies that live outside the + * port's lflow_ref and are not tracked here. + * + * NAT is not on that list. What couples the NATs of a router to its port + * set is the lr_nat record: ovn_nat.is_router_ip is set when an SNAT + * external IP is also one of the router's port IPs (od->router_ips) and + * ovn_nat.l3dgw_port is resolved against the router's ports. Both are + * refreshed by tracking the router in 'trk_nat_lrs' (see + * lr_handle_lrp_changes()), which cascades to en_lr_stateful and from + * there regenerates the router's NAT flows and the per-port lbnat flows + * of every port of the router. */ + const struct nbrec_logical_router *nbr = od->nbr; + if (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: @@ -5904,6 +6228,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; } @@ -5911,14 +6236,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; @@ -5976,6 +6299,148 @@ 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; + } + + bool lrp_created = 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); + lrp_created = true; + } 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); + } + + /* The lr_nat record of a router is built from its port set (see + * lrp_needs_recompute()), so re-track the router for en_lr_nat whenever a + * port was created or deleted. en_lr_nat re-initializes the record from + * the new od->router_ips / lr_ports, en_lr_stateful picks that up and + * en_lflow then regenerates the router's NAT flows along with the lbnat + * flows of all of its ports. */ + if ((lrp_created || lrp_deleted) && od->nbr->n_nat) { + hmapx_add(&nd->trk_data.trk_nat_lrs, od); + } + + 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 @@ -5983,7 +6448,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; @@ -5999,9 +6465,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)) { @@ -6020,6 +6486,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) { @@ -6027,11 +6507,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, @@ -6107,6 +6603,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; @@ -13479,29 +13979,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); + } } } @@ -13519,7 +14031,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); } } @@ -16214,11 +16726,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) { @@ -21002,6 +21525,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 @@ -21062,7 +21593,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, @@ -21736,12 +22267,18 @@ lflow_handle_northd_lr_changes(struct tracked_dps *tracked_lrs, } 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, }; @@ -21873,6 +22410,63 @@ lflow_handle_northd_port_changes(struct tracked_ovn_ports *trk_lsps, hmapx_destroy(&ls_stateful_regen); } +/* 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()). */ +void +lflow_handle_northd_lrp_changes(struct tracked_ovn_ports *trk_lrps, + struct lflow_input *lflow_input, + struct lflow_table *lflows, + struct hmapx *dirty_lflow_refs) +{ + 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); + lflow_ref_unlink_lflows(op->lflow_ref, lflows); + hmapx_add(dirty_lflow_refs, op->lflow_ref); + lflow_ref_unlink_lflows(op->stateful_lflow_ref, lflows); + hmapx_add(dirty_lflow_refs, op->stateful_lflow_ref); + } + + 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); + hmapx_add(dirty_lflow_refs, op->lflow_ref); + 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); + hmapx_add(dirty_lflow_refs, op->stateful_lflow_ref); + } + + ds_destroy(&lsi.match); + ds_destroy(&lsi.actions); +} + void lflow_handle_northd_lb_changes(struct tracked_lbs *trk_lbs, struct lflow_input *lflow_input, diff --git a/northd/northd.h b/northd/northd.h index 3d1fd2d31..a1b8ae414 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. @@ -913,6 +918,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; @@ -938,7 +948,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); @@ -984,6 +995,10 @@ void build_route_data_flows_for_lrouter( const struct group_ecmp_datapath *route_node, const struct sset *bfd_ports); +void lflow_handle_northd_lrp_changes(struct tracked_ovn_ports *, + struct lflow_input *, + struct lflow_table *lflows, + struct hmapx *dirty_lflow_refs); void lflow_handle_northd_lr_changes(struct tracked_dps *, struct lflow_input *, struct lflow_table *lflows, @@ -1044,6 +1059,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); @@ -1065,6 +1084,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 a19912b09..b86d9f4aa 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -13309,6 +13309,392 @@ 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 - NAT]) +AT_KEYWORDS([incremental processing]) +ovn_start + +check ovn-nbctl ls-add sw0 +check ovn-nbctl lr-add lr0 +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 +check ovn-nbctl lsp-add sw0 sw0-lr0 \ + -- lsp-set-type sw0-lr0 router \ + -- lsp-set-addresses sw0-lr0 router \ + -- lsp-set-options sw0-lr0 router-port=lr0-sw0 +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 172.16.0.10 10.0.0.0/24 + +# Adding a router port to a router that already has NAT configured is +# incrementally processed: the router's lr_nat record is re-initialized from +# the new port set and its NAT flows are regenerated. +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 +AT_CHECK([ovn-sbctl dump-flows lr0 | grep -c 'ip4.dst == 20.0.0.0/24'], [0], + [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# The new port's network may contain the external IP of an SNAT entry, which +# flips ovn_nat.is_router_ip and changes the ARP request forwarder flows on +# the peer switch of the router's other ports. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw2 00:00:00:00:ff:03 172.16.0.10/24 +check_engine_compute northd incremental +check_engine_compute lflow incremental +AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_in_l2_lkup | \ + grep -c 'arp.tpa == 172.16.0.10'], [1], [0 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# ... and removing it flips it back. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr0-sw2 +check_engine_compute northd incremental +check_engine_compute lflow incremental +AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_in_l2_lkup | \ + grep -c 'arp.tpa == 172.16.0.10'], [0], [1 +]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Deleting a router port on a router with NAT is incremental as well. +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 +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +OVN_CLEANUP_NORTHD +AT_CLEANUP + +AT_SETUP([Logical router port incremental processing - distributed gateway]) +AT_KEYWORDS([incremental processing]) +ovn_start + +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 +check ovn-sbctl chassis-add gw2 geneve 127.0.0.2 + +# A router shaped like a Neutron one: a distributed gateway port towards an +# external network with a localnet port, a regular port towards a tenant +# network, SNAT, a default route, gateway_mtu and a CoPP. +check ovn-nbctl ls-add public +check ovn-nbctl lsp-add-localnet-port public ln-public public +check ovn-nbctl ls-add sw0 +check ovn-nbctl lr-add lr0 +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 \ + 172.168.0.100/24 2001:db8::100/64 +check ovn-nbctl lrp-set-gateway-chassis lr0-public gw1 20 +check ovn-nbctl lrp-set-gateway-chassis lr0-public gw2 10 +check ovn-nbctl set logical_router_port lr0-public options:gateway_mtu=1400 +check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 +check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0 +check ovn-nbctl lr-nat-add lr0 snat 172.168.0.100 10.0.0.0/24 +check ovn-nbctl lr-route-add lr0 0.0.0.0/0 172.168.0.1 +check ovn-nbctl meter-add meter0 drop 100 pktps 10 +check ovn-nbctl copp-add copp0 arp meter0 +check ovn-nbctl copp-add copp0 arp-resolve meter0 +check ovn-nbctl --wait=sb lr-copp-add copp0 lr0 + +# Adding a regular router port to it, not yet connected to any switch, is +# incrementally processed. +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 +AT_CHECK([ovn-sbctl dump-flows lr0 | grep -c 'ip4.dst == 20.0.0.0/24'], [0], + [1 +]) +# The gateway_mtu of the distributed gateway port applies to the traffic that +# leaves through every other router port, the new one included. +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_larger_pkts | \ + grep -c 'inport == "lr0-sw1"'], [0], [ignore]) +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Also when it has IPv6 networks. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw2 00:00:00:00:ff:03 \ + 30.0.0.1/24 2001:db8:30::1/64 +check_engine_compute northd incremental +check_engine_compute lflow incremental +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# A distributed dnat_and_snat entry makes the router's NAT flows depend on the +# distributed gateway port as well. +check ovn-nbctl lsp-add sw0 sw0-vm1 \ + -- lsp-set-addresses sw0-vm1 "00:00:00:00:00:11 10.0.0.11" +check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.110 \ + 10.0.0.11 sw0-vm1 00:00:00:00:01:11 +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw3 00:00:00:00:ff:04 40.0.0.1/24 +check_engine_compute northd incremental +check_engine_compute lflow incremental +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Deleting such a port is incrementally processed too. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr0-sw3 +check_engine_compute northd incremental +check_engine_compute lflow incremental +AT_CHECK([ovn-sbctl find port_binding logical_port=lr0-sw3], [0], []) +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-sw2 +check_engine_compute northd incremental +check_engine_compute lflow incremental +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# So is the same on a router with a distributed gateway port and no NAT. +check ovn-nbctl lr-add lr1 +check ovn-nbctl lrp-add lr1 lr1-public 00:00:20:20:12:14 172.168.0.101/24 +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-public gw1 20 +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 50.0.0.1/24 +check_engine_compute northd incremental +check_engine_compute lflow incremental +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr1-sw0 +check_engine_compute northd incremental +check_engine_compute lflow incremental +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# A regular port whose peer "router" LSP already exists falls back: on such a +# router its own flows depend on the peer switch through is_chassis_resident() +# checks. +check ovn-nbctl ls-add sw4 +check ovn-nbctl --wait=sb lsp-add-router-port sw4 sw4-lr0 lr0-sw4 +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw4 00:00:00:00:ff:05 60.0.0.1/24 +check_engine_compute northd recompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# ... and so does deleting a regular port that is connected to a switch. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lrp-del lr0-sw4 +check_engine_compute northd recompute +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 @@ -13378,13 +13764,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 @@ -14832,18 +15219,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
