From patchwork Fri Aug 18 08:58:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1822768 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RRwnj2DZCz1ygW for ; Fri, 18 Aug 2023 19:00:41 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 5A2F983FB7; Fri, 18 Aug 2023 09:00:39 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 5A2F983FB7 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IHY-ewg37s60; Fri, 18 Aug 2023 09:00:33 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id 0C11E841EF; Fri, 18 Aug 2023 09:00:30 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 0C11E841EF Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B4E35C0039; Fri, 18 Aug 2023 09:00:30 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 05A7BC0032 for ; Fri, 18 Aug 2023 09:00:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 89C2B83F93 for ; Fri, 18 Aug 2023 08:59:02 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 89C2B83F93 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CZXAVqP5a9YO for ; Fri, 18 Aug 2023 08:59:00 +0000 (UTC) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::221]) by smtp1.osuosl.org (Postfix) with ESMTPS id C7FC383FC6 for ; Fri, 18 Aug 2023 08:58:59 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org C7FC383FC6 Received: by mail.gandi.net (Postfix) with ESMTPSA id ADC3424000A; Fri, 18 Aug 2023 08:58:55 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Fri, 18 Aug 2023 14:28:51 +0530 Message-Id: <20230818085851.1031153-1-numans@ovn.org> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230818085606.1030792-1-numans@ovn.org> References: <20230818085606.1030792-1-numans@ovn.org> MIME-Version: 1.0 X-GND-Sasl: numans@ovn.org Subject: [ovs-dev] [PATCH ovn v6 16/16] northd: Handle NAT changes for a logical router incrementally. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique This patch adds the I-P support for any changes to router NATs in both the northd and lflow engine nodes. Signed-off-by: Numan Siddique --- northd/en-lflow.c | 10 + northd/en-northd.c | 30 ++- northd/en-northd.h | 4 +- northd/inc-proc-northd.c | 4 +- northd/northd.c | 463 ++++++++++++++++++++++++++++++++------- northd/northd.h | 23 +- tests/ovn-northd.at | 202 +++++++++++++++++ 7 files changed, 650 insertions(+), 86 deletions(-) diff --git a/northd/en-lflow.c b/northd/en-lflow.c index 56fe564c82..ad5ebc53fc 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -105,6 +105,10 @@ lflow_northd_handler(struct engine_node *node, return false; } + if (northd_data->trk_northd_changes.lb_nats_changed) { + return false; + } + const struct engine_context *eng_ctx = engine_get_context(); struct lflow_data *lflow_data = data; @@ -123,6 +127,12 @@ lflow_northd_handler(struct engine_node *node, return false; } + if (!lflow_handle_northd_od_changes(eng_ctx->ovnsb_idl_txn, + &northd_data->trk_northd_changes.trk_datapaths, + &lflow_input, lflow_data)) { + return false; + } + engine_set_node_state(node, EN_UPDATED); return true; diff --git a/northd/en-northd.c b/northd/en-northd.c index b3e84a6af6..5fa9ecbbc1 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -207,19 +207,41 @@ northd_sb_port_binding_handler(struct engine_node *node, } bool -northd_nb_logical_router_handler(struct engine_node *node, - void *data) +northd_nb_logical_router_handler_pre_lb(struct engine_node *node, + void *data) { struct northd_data *nd = data; struct northd_input input_data; northd_get_input_data(node, &input_data); - if (!northd_handle_lr_changes(&input_data, nd)) { + if (!northd_handle_lr_changes_pre_lb(&input_data, nd)) { return false; } - if (nd->change_tracked) { + if (northd_has_tracked_data(&nd->trk_northd_changes)) { + nd->change_tracked = true; + engine_set_node_state(node, EN_UPDATED); + } + + return true; +} + +bool +northd_nb_logical_router_handler_post_lb(struct engine_node *node, + void *data) +{ + struct northd_data *nd = data; + struct northd_input input_data; + + northd_get_input_data(node, &input_data); + + if (!northd_handle_lr_changes_post_lb(&input_data, nd)) { + return false; + } + + if (northd_has_tracked_data(&nd->trk_northd_changes)) { + nd->change_tracked = true; engine_set_node_state(node, EN_UPDATED); } diff --git a/northd/en-northd.h b/northd/en-northd.h index 84d8673e1b..02af2a758c 100644 --- a/northd/en-northd.h +++ b/northd/en-northd.h @@ -16,7 +16,9 @@ void en_northd_cleanup(void *data); void en_northd_clear_tracked_data(void *data); bool northd_nb_nb_global_handler(struct engine_node *, void *data OVS_UNUSED); bool northd_nb_logical_switch_handler(struct engine_node *, void *data); -bool northd_nb_logical_router_handler(struct engine_node *, void *data); +bool northd_nb_logical_router_handler_pre_lb(struct engine_node *, void *data); +bool northd_nb_logical_router_handler_post_lb(struct engine_node *, + void *data); bool northd_sb_port_binding_handler(struct engine_node *, void *data); bool northd_lb_data_handler_pre_od(struct engine_node *, void *data); bool northd_lb_data_handler_post_od(struct engine_node *, void *data); diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index e3bc2bda7b..12a5990521 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -189,8 +189,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_northd, &en_nb_logical_switch, northd_nb_logical_switch_handler); engine_add_input(&en_northd, &en_nb_logical_router, - northd_nb_logical_router_handler); + northd_nb_logical_router_handler_pre_lb); engine_add_input(&en_northd, &en_lb_data, northd_lb_data_handler_post_od); + engine_add_input(&en_northd, &en_nb_logical_router, + northd_nb_logical_router_handler_post_lb); engine_add_input(&en_mac_binding_aging, &en_nb_nb_global, NULL); engine_add_input(&en_mac_binding_aging, &en_sb_mac_binding, NULL); diff --git a/northd/northd.c b/northd/northd.c index 2ae41b5d8a..28e1f43a38 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -703,6 +703,18 @@ destroy_nat_entries(struct ovn_datapath *od) } } +static void +reinit_nat_entries(struct ovn_datapath *od) +{ + ovs_assert(od->nbr); + destroy_nat_entries(od); + od->n_nat_entries = 0; + od->has_distributed_nat = false; + free(od->nat_entries); + od->nat_entries = NULL; + init_nat_entries(od); +} + static void init_router_external_ips(struct ovn_datapath *od) { @@ -724,6 +736,14 @@ destroy_router_external_ips(struct ovn_datapath *od) sset_destroy(&od->external_ips); } +static void +reinit_router_external_ips(struct ovn_datapath *od) +{ + ovs_assert(od->nbr); + destroy_router_external_ips(od); + init_router_external_ips(od); +} + static bool lb_has_vip(const struct nbrec_load_balancer *lb) { @@ -792,6 +812,47 @@ destroy_lb_for_datapath(struct ovn_datapath *od) od->lb_ips = NULL; } +/* Initiailizes the router datapath's lb_nats sset which contains + * set of NAT ips which are also in the load balancer vips + * associated with the router datapath. */ +static void +init_router_lb_nats(struct ovn_datapath *od) +{ + ovs_assert(od->nbr); + + sset_init(&od->lb_nats); + const char *external_ip; + SSET_FOR_EACH (external_ip, &od->external_ips) { + bool is_vip_nat = false; + if (addr_is_ipv6(external_ip)) { + is_vip_nat = sset_contains(&od->lb_ips->ips_v6, external_ip); + } else { + is_vip_nat = sset_contains(&od->lb_ips->ips_v4, external_ip); + } + + if (is_vip_nat) { + sset_add(&od->lb_nats, external_ip); + } + } +} + +static void +destroy_router_lb_nats(struct ovn_datapath *od) +{ + if (od->nbr) { + sset_destroy(&od->lb_nats); + } +} + +static void +build_router_lb_nats(struct hmap *lr_datapaths) +{ + struct ovn_datapath *od; + HMAP_FOR_EACH (od, key_node, lr_datapaths) { + init_router_lb_nats(od); + } +} + /* A group of logical router datapaths which are connected - either * directly or indirectly. * Each logical router can belong to only one group. */ @@ -847,6 +908,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) free(od->ls_peers); destroy_nat_entries(od); destroy_router_external_ips(od); + destroy_router_lb_nats(od); destroy_lb_for_datapath(od); free(od->nat_entries); free(od->localnet_ports); @@ -1594,6 +1656,8 @@ destroy_routable_addresses(struct ovn_port_routable_addresses *ra) destroy_lport_addresses(&ra->laddrs[i]); } free(ra->laddrs); + ra->laddrs = NULL; + ra->n_addrs = 0; } static char **get_nat_addresses(const struct ovn_port *op, size_t *n, @@ -1638,6 +1702,14 @@ ovn_port_set_nb(struct ovn_port *op, op->lsp_can_be_inc_processed = lsp_can_be_inc_processed(nbsp); } op->nbrp = nbrp; + + if (nbrp) { + if (is_cr_port(op)) { + /* ovn-northd doesn't have to do anything when a chassis-redirect + * port gets updated as its an internal port created by northd. */ + op->lsp_can_be_inc_processed = true; + } + } init_mcast_port_info(&op->mcast_info, op->nbsp, op->nbrp); } @@ -5071,12 +5143,20 @@ destroy_tracked_lb_datapaths(struct tracked_lb_datapaths *trk_lbs) trk_lbs->n_nb_lr = 0; } +static void +destroy_tracked_datapaths(struct tracked_datapaths *trk_dps) +{ + hmapx_clear(&trk_dps->crupdated); +} + void destroy_northd_data_tracked_changes(struct northd_data *nd) { struct northd_tracked_data *trk_changes = &nd->trk_northd_changes; destroy_tracked_ovn_ports(&trk_changes->trk_ovn_ports); destroy_tracked_lb_datapaths(&trk_changes->trk_lbs); + destroy_tracked_datapaths(&trk_changes->trk_datapaths); + trk_changes->lb_nats_changed = false; nd->change_tracked = false; } @@ -5088,7 +5168,8 @@ bool northd_has_tracked_data(struct northd_tracked_data *trk_nd_changes) || !hmap_is_empty(&trk_nd_changes->trk_ovn_ports.updated) || !hmap_is_empty(&trk_nd_changes->trk_ovn_ports.deleted) || !hmapx_is_empty(&trk_nd_changes->trk_lbs.crupdated) - || !hmapx_is_empty(&trk_nd_changes->trk_lbs.deleted)); + || !hmapx_is_empty(&trk_nd_changes->trk_lbs.deleted) + || !hmapx_is_empty(&trk_nd_changes->trk_datapaths.crupdated)); } bool northd_has_only_ports_in_tracked_data( @@ -5098,6 +5179,7 @@ bool northd_has_only_ports_in_tracked_data( && !trk_nd_changes->trk_lbs.n_nb_lr && hmapx_is_empty(&trk_nd_changes->trk_lbs.crupdated) && hmapx_is_empty(&trk_nd_changes->trk_lbs.deleted) + && hmapx_is_empty(&trk_nd_changes->trk_datapaths.crupdated) && (!hmap_is_empty(&trk_nd_changes->trk_ovn_ports.created) || !hmap_is_empty(&trk_nd_changes->trk_ovn_ports.updated) || !hmap_is_empty(&trk_nd_changes->trk_ovn_ports.deleted))); @@ -5646,7 +5728,8 @@ check_unsupported_inc_proc_for_lr_changes( for (col = 0; col < NBREC_LOGICAL_ROUTER_N_COLUMNS; col++) { if (nbrec_logical_router_is_updated(lr, col)) { if (col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER || - col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER_GROUP) { + col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER_GROUP || + col == NBREC_LOGICAL_ROUTER_COL_NAT) { continue; } return true; @@ -5665,12 +5748,6 @@ check_unsupported_inc_proc_for_lr_changes( OVSDB_IDL_CHANGE_MODIFY) > 0) { return true; } - for (size_t i = 0; i < lr->n_nat; i++) { - if (nbrec_nat_row_get_seqno(lr->nat[i], - OVSDB_IDL_CHANGE_MODIFY) > 0) { - return true; - } - } for (size_t i = 0; i < lr->n_policies; i++) { if (nbrec_logical_router_policy_row_get_seqno(lr->policies[i], OVSDB_IDL_CHANGE_MODIFY) > 0) { @@ -5686,6 +5763,93 @@ check_unsupported_inc_proc_for_lr_changes( return false; } +static bool +handle_lr_nat_changes(const struct nbrec_logical_router *lr, + struct ovn_datapath *od, + bool *changed) +{ + /* If nothing has changed, return */ + bool nat_changed = false; + if (nbrec_logical_router_is_updated(lr, NBREC_LOGICAL_ROUTER_COL_NAT)) { + nat_changed = true; + for (size_t i = 0; i < lr->n_nat; i++) { + if ((nbrec_nat_row_get_seqno(lr->nat[i], + OVSDB_IDL_CHANGE_INSERT) > 0) || + (nbrec_nat_row_get_seqno(lr->nat[i], + OVSDB_IDL_CHANGE_DELETE) > 0)) { + /* Check if the modified NAT has add_route options set. + * If so, return false. */ + if (smap_get_bool(&lr->nat[i]->options, "add_route", false)) { + return false; + } + nat_changed = true; + break; + } + } + } else { + for (size_t i = 0; i < lr->n_nat; i++) { + if (nbrec_nat_row_get_seqno(lr->nat[i], + OVSDB_IDL_CHANGE_MODIFY) > 0) { + /* Check if the modified NAT has add_route options set. + * If so, return false. */ + if (smap_get_bool(&lr->nat[i]->options, "add_route", false)) { + return false; + } + nat_changed = true; + break; + } + } + } + + if (!nat_changed) { + return true; + } + + *changed = true; + reinit_nat_entries(od); + reinit_router_external_ips(od); + return true; +} + +static bool +lr_lb_nats_changed(struct ovn_datapath *od) +{ + /* Check if NATs changed. */ + bool nat_changed = + nbrec_logical_router_is_updated(od->nbr, NBREC_LOGICAL_ROUTER_COL_NAT); + + if (!nat_changed) { + for (size_t i = 0; i < od->nbr->n_nat; i++) { + if (nbrec_nat_row_get_seqno(od->nbr->nat[i], + OVSDB_IDL_CHANGE_MODIFY) > 0) { + nat_changed = true; + break; + } + } + } + + if (!nat_changed) { + return false; + } + + struct sset old_lb_nats = SSET_INITIALIZER(&old_lb_nats); + bool lb_nats_changed; + + sset_swap(&od->lb_nats, &old_lb_nats); + sset_destroy(&od->lb_nats); + init_router_lb_nats(od); + + lb_nats_changed = sset_count(&od->lb_nats) != sset_count(&old_lb_nats); + + if (!lb_nats_changed) { + lb_nats_changed = !sset_equals(&od->lb_nats, &old_lb_nats); + } + + sset_destroy(&old_lb_nats); + + return lb_nats_changed; +} + /* Return true if changes are handled incrementally, false otherwise. * When there are any changes, try to track what's exactly changed and set * northd_data->change_tracked accordingly: change tracked - true, otherwise, @@ -5696,8 +5860,8 @@ check_unsupported_inc_proc_for_lr_changes( * northd_handle_lb_data_changes_post_od). * */ bool -northd_handle_lr_changes(const struct northd_input *ni, - struct northd_data *nd) +northd_handle_lr_changes_pre_lb(const struct northd_input *ni, + struct northd_data *nd) { const struct nbrec_logical_router *changed_lr; @@ -5713,6 +5877,43 @@ northd_handle_lr_changes(const struct northd_input *ni, if (check_unsupported_inc_proc_for_lr_changes(changed_lr)) { goto fail; } + + struct ovn_datapath *od = ovn_datapath_find( + &nd->lr_datapaths.datapaths, &changed_lr->header_.uuid); + if (!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)); + return false; + } + + bool lr_nat_changed = false; + if (!handle_lr_nat_changes(changed_lr, od, + &lr_nat_changed)) { + goto fail; + } + + if (lr_nat_changed) { + nd->change_tracked = true; + hmapx_add(&nd->trk_northd_changes.trk_datapaths.crupdated, od); + + /* Also add the peer (logical switch port) of logical router ports + * to the northd tracked data. */ + struct ovn_port *op; + uint8_t lport_changes = + (EN_TRACKED_OP_LPORT_CHANGED | EN_TRACKED_OP_LB_CHANGED); + HMAP_FOR_EACH (op, dp_node, &od->ports) { + add_op_to_northd_tracked_ports( + &nd->trk_northd_changes.trk_ovn_ports.updated, op, + lport_changes); + if (op->peer) { + add_op_to_northd_tracked_ports( + &nd->trk_northd_changes.trk_ovn_ports.updated, + op->peer, lport_changes); + } + } + } } return true; @@ -5721,6 +5922,26 @@ fail: return false; } +bool +northd_handle_lr_changes_post_lb(const struct northd_input *ni, + struct northd_data *nd) +{ + const struct nbrec_logical_router *changed_lr; + + NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH_TRACKED (changed_lr, + ni->nbrec_logical_router_table) { + struct ovn_datapath *od = ovn_datapath_find( + &nd->lr_datapaths.datapaths, &changed_lr->header_.uuid); + ovs_assert(od); + + if (lr_lb_nats_changed(od)) { + nd->trk_northd_changes.lb_nats_changed = true; + } + } + + return true; +} + bool northd_handle_sb_port_binding_changes( const struct sbrec_port_binding_table *sbrec_port_binding_table, @@ -17118,38 +17339,42 @@ build_lswitch_and_lrouter_iterate_by_ls(struct ovn_datapath *od, */ static void build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, - struct lswitch_flow_build_info *lsi) + const struct shash *meter_groups, + const struct hmap *lr_ports, + const struct hmap *ls_ports, + const struct chassis_features *features, + const struct hmap *bfd_connections, + struct ds *match, + struct ds *actions, + struct lflow_data *lflows) { ovs_assert(od->nbr); - build_adm_ctrl_flows_for_lrouter(od, lsi->lflows); - build_neigh_learning_flows_for_lrouter(od, lsi->lflows, &lsi->match, - &lsi->actions, lsi->meter_groups); - build_ND_RA_flows_for_lrouter(od, lsi->lflows); - build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows); - build_static_route_flows_for_lrouter(od, lsi->features, - lsi->lflows, lsi->lr_ports, - lsi->bfd_connections); - build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, - &lsi->actions); - build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports); - build_arp_resolve_flows_for_lrouter(od, lsi->lflows); - build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, - &lsi->match, &lsi->actions, - lsi->meter_groups); - build_gateway_redirect_flows_for_lrouter(od, lsi->lflows, &lsi->match, - &lsi->actions); - build_arp_request_flows_for_lrouter(od, lsi->lflows, &lsi->match, - &lsi->actions, lsi->meter_groups); - build_misc_local_traffic_drop_flows_for_lrouter(od, lsi->lflows); - build_lrouter_arp_nd_for_datapath(od, lsi->lflows, lsi->meter_groups); - build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->ls_ports, - lsi->lr_ports, &lsi->match, - &lsi->actions, lsi->meter_groups, - lsi->features); - build_lrouter_nat_flows_lb_related(od, lsi->lflows, &lsi->match, - lsi->features); - build_lrouter_nat_flows_ct_lb_related(od, lsi->lflows); - build_lrouter_lb_affinity_default_flows(od, lsi->lflows); + build_adm_ctrl_flows_for_lrouter(od, lflows); + build_neigh_learning_flows_for_lrouter(od, lflows, match, + actions, meter_groups); + build_ND_RA_flows_for_lrouter(od, lflows); + build_ip_routing_pre_flows_for_lrouter(od, lflows); + build_static_route_flows_for_lrouter(od, features, + lflows, lr_ports, + bfd_connections); + build_mcast_lookup_flows_for_lrouter(od, lflows, match, + actions); + build_ingress_policy_flows_for_lrouter(od, lflows, lr_ports); + build_arp_resolve_flows_for_lrouter(od, lflows); + build_check_pkt_len_flows_for_lrouter(od, lflows, lr_ports, match, actions, + meter_groups); + build_gateway_redirect_flows_for_lrouter(od, lflows, match, actions); + build_arp_request_flows_for_lrouter(od, lflows, match, + actions, meter_groups); + build_misc_local_traffic_drop_flows_for_lrouter(od, lflows); + build_lrouter_arp_nd_for_datapath(od, lflows, meter_groups); + build_lrouter_nat_defrag_and_lb(od, lflows, ls_ports, + lr_ports, match, + actions, meter_groups, + features); + build_lrouter_nat_flows_lb_related(od, lflows, match, features); + build_lrouter_nat_flows_ct_lb_related(od, lflows); + build_lrouter_lb_affinity_default_flows(od, lflows); } /* Helper function to combine all lflow generation which is iterated by logical @@ -17205,37 +17430,38 @@ build_lb_related_iterate_by_lsp(struct ovn_port *op, */ static void build_lswitch_and_lrouter_iterate_by_lrp(struct ovn_port *op, - struct lswitch_flow_build_info *lsi) + const struct shash *meter_groups, + struct ds *match, + struct ds *actions, + struct lflow_data *lflows) { ovs_assert(op->nbrp); - build_adm_ctrl_flows_for_lrouter_port(op, lsi->lflows, &lsi->match, - &lsi->actions); - build_neigh_learning_flows_for_lrouter_port(op, lsi->lflows, &lsi->match, - &lsi->actions); - build_ip_routing_flows_for_lrp(op, lsi->lflows); - build_ND_RA_flows_for_lrouter_port(op, lsi->lflows, &lsi->match, - &lsi->actions, lsi->meter_groups); - build_arp_resolve_flows_for_lrp(op, lsi->lflows, &lsi->match, - &lsi->actions); - build_egress_delivery_flows_for_lrouter_port(op, lsi->lflows, &lsi->match, - &lsi->actions); - build_dhcpv6_reply_flows_for_lrouter_port(op, lsi->lflows, &lsi->match); - build_ipv6_input_flows_for_lrouter_port(op, lsi->lflows, - &lsi->match, &lsi->actions, - lsi->meter_groups); - build_lrouter_ipv4_ip_input(op, lsi->lflows, - &lsi->match, &lsi->actions, lsi->meter_groups); + build_adm_ctrl_flows_for_lrouter_port(op, lflows, match, + actions); + build_neigh_learning_flows_for_lrouter_port(op, lflows, match, + actions); + build_ip_routing_flows_for_lrp(op, lflows); + build_ND_RA_flows_for_lrouter_port(op, lflows, match, actions, + meter_groups); + build_arp_resolve_flows_for_lrp(op, lflows, match, actions); + build_egress_delivery_flows_for_lrouter_port(op, lflows, match, + actions); + build_dhcpv6_reply_flows_for_lrouter_port(op, lflows, match); + build_ipv6_input_flows_for_lrouter_port(op, lflows, match, actions, + meter_groups); + build_lrouter_ipv4_ip_input(op, lflows, match, actions, meter_groups); } static void build_lb_related_iterate_by_lrp(struct ovn_port *op, - struct lswitch_flow_build_info *lsi) + const struct shash *meter_groups, + struct ds *match, + struct ds *actions, + struct lflow_data *lflows) { ovs_assert(op->nbrp); - build_lrouter_ipv4_ip_input_lb_related(op, lsi->lflows, &lsi->match, - lsi->meter_groups); - build_lrouter_force_snat_flows_op(op, lsi->lflows, &lsi->match, - &lsi->actions); + build_lrouter_ipv4_ip_input_lb_related(op, lflows, match, meter_groups); + build_lrouter_force_snat_flows_op(op, lflows, match, actions); } static void * @@ -17280,7 +17506,10 @@ build_lflows_thread(void *arg) if (stop_parallel_processing()) { return NULL; } - build_lswitch_and_lrouter_iterate_by_lr(od, lsi); + build_lswitch_and_lrouter_iterate_by_lr( + od, lsi->meter_groups, lsi->lr_ports, lsi->ls_ports, + lsi->features, lsi->bfd_connections, &lsi->match, + &lsi->actions, lsi->lflows); } } for (bnum = control->id; @@ -17312,8 +17541,12 @@ build_lflows_thread(void *arg) if (stop_parallel_processing()) { return NULL; } - build_lswitch_and_lrouter_iterate_by_lrp(op, lsi); - build_lb_related_iterate_by_lrp(op, lsi); + build_lswitch_and_lrouter_iterate_by_lrp( + op, lsi->meter_groups, &lsi->match, &lsi->actions, + lsi->lflows); + build_lb_related_iterate_by_lrp( + op, lsi->meter_groups, &lsi->match, &lsi->actions, + lsi->lflows); } } for (bnum = control->id; @@ -17492,7 +17725,10 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths, build_lswitch_and_lrouter_iterate_by_ls(od, &lsi); } HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { - build_lswitch_and_lrouter_iterate_by_lr(od, &lsi); + build_lswitch_and_lrouter_iterate_by_lr( + od, lsi.meter_groups, lsi.lr_ports, lsi.ls_ports, + lsi.features, lsi.bfd_connections, &lsi.match, + &lsi.actions, lsi.lflows); } stopwatch_stop(LFLOWS_DATAPATHS_STOPWATCH_NAME, time_msec()); stopwatch_start(LFLOWS_PORTS_STOPWATCH_NAME, time_msec()); @@ -17506,8 +17742,12 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths, &lsi.actions, lsi.lflows); } HMAP_FOR_EACH (op, key_node, lr_ports) { - build_lswitch_and_lrouter_iterate_by_lrp(op, &lsi); - build_lb_related_iterate_by_lrp(op, &lsi); + build_lswitch_and_lrouter_iterate_by_lrp(op, lsi.meter_groups, + &lsi.match, + &lsi.actions, + lsi.lflows); + build_lb_related_iterate_by_lrp(op, lsi.meter_groups, &lsi.match, + &lsi.actions, lsi.lflows); } stopwatch_stop(LFLOWS_PORTS_STOPWATCH_NAME, time_msec()); stopwatch_start(LFLOWS_LBS_STOPWATCH_NAME, time_msec()); @@ -18193,20 +18433,20 @@ bool lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, struct ds match = DS_EMPTY_INITIALIZER; struct ds actions = DS_EMPTY_INITIALIZER; if (trk_op->changes & EN_TRACKED_OP_LPORT_CHANGED) { - /* We still don't support EN_TRACKED_OP_LPORT_CHANGED for - * router ports. */ - ovs_assert(op->nbsp); - /* unlink old lflows. */ unlink_lflows(op->od, OBJDEP_TYPE_LPORT, res_name, lflow_data, &op->lflow_dep_mgr); /* Generate new lflows. */ - build_lswitch_and_lrouter_iterate_by_lsp(op, lflow_input->ls_ports, - lflow_input->lr_ports, - lflow_input->meter_groups, - &match, &actions, - lflow_data); + if (op->nbsp) { + build_lswitch_and_lrouter_iterate_by_lsp( + op, lflow_input->ls_ports, lflow_input->lr_ports, + lflow_input->meter_groups, &match, &actions, lflow_data); + } else { + build_lswitch_and_lrouter_iterate_by_lrp( + op, lflow_input->meter_groups, &match, &actions, + lflow_data); + } } if (trk_op->changes & EN_TRACKED_OP_LB_CHANGED) { @@ -18457,6 +18697,70 @@ bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, return true; } +bool +lflow_handle_northd_od_changes(struct ovsdb_idl_txn *ovnsb_txn, + struct tracked_datapaths *trk_dps, + struct lflow_input *lflow_input, + struct lflow_data *lflow_data) +{ + struct ovn_datapath *od; + struct hmapx_node *hmapx_node; + HMAPX_FOR_EACH (hmapx_node, &trk_dps->crupdated) { + od = hmapx_node->data; + + /* We don't support handling logical switches yet. */ + ovs_assert(od->nbr); + + struct resource_to_objects_node *res_node = + objdep_mgr_find_objs(&od->lflow_dep_mgr, OBJDEP_TYPE_OD, + od->nbr->name); + + /* unlink old lflows of type OBJDEP_TYPE_OD. */ + unlink_objres_lflows(res_node, od, lflow_data, + &od->lflow_dep_mgr); + + res_node = objdep_mgr_find_objs( + &od->lflow_dep_mgr, OBJDEP_TYPE_LB, od->nbr->name); + + unlink_objres_lflows(res_node, od, lflow_data, &od->lflow_dep_mgr); + + res_node = objdep_mgr_find_objs( + &od->lflow_dep_mgr, OBJDEP_TYPE_CT_LB, od->nbr->name); + unlink_objres_lflows(res_node, od, lflow_data, &od->lflow_dep_mgr); + + struct ds match = DS_EMPTY_INITIALIZER; + struct ds actions = DS_EMPTY_INITIALIZER; + build_lswitch_and_lrouter_iterate_by_lr( + od, lflow_input->meter_groups, lflow_input->lr_ports, + lflow_input->ls_ports, lflow_input->features, + lflow_input->bfd_connections, &match, + &actions, lflow_data); + ds_destroy(&match); + ds_destroy(&actions); + + /* Sync the updated flows to SB. */ + res_node = objdep_mgr_find_objs(&od->lflow_dep_mgr, + OBJDEP_TYPE_OD, + od->nbr->name); + sync_lflows_from_objres(ovnsb_txn, res_node, lflow_input, + lflow_data, &od->lflow_dep_mgr); + + res_node = objdep_mgr_find_objs(&od->lflow_dep_mgr, + OBJDEP_TYPE_LB, + od->nbr->name); + sync_lflows_from_objres(ovnsb_txn, res_node, lflow_input, + lflow_data, &od->lflow_dep_mgr); + + res_node = objdep_mgr_find_objs(&od->lflow_dep_mgr, + OBJDEP_TYPE_CT_LB, + od->nbr->name); + sync_lflows_from_objres(ovnsb_txn, res_node, lflow_input, + lflow_data, &od->lflow_dep_mgr); + } + + return true; +} + /* Each port group in Port_Group table in OVN_Northbound has a corresponding * entry in Port_Group table in OVN_Southbound. In OVN_Northbound the entries * contains lport uuids, while in OVN_Southbound we store the lport names. @@ -19341,6 +19645,7 @@ northd_init(struct northd_data *data) hmap_init(&data->trk_northd_changes.trk_ovn_ports.deleted); hmapx_init(&data->trk_northd_changes.trk_lbs.crupdated); hmapx_init(&data->trk_northd_changes.trk_lbs.deleted); + hmapx_init(&data->trk_northd_changes.trk_datapaths.crupdated); } void @@ -19399,6 +19704,7 @@ northd_destroy(struct northd_data *data) hmap_destroy(&data->trk_northd_changes.trk_ovn_ports.deleted); hmapx_destroy(&data->trk_northd_changes.trk_lbs.crupdated); hmapx_destroy(&data->trk_northd_changes.trk_lbs.deleted); + hmapx_destroy(&data->trk_northd_changes.trk_datapaths.crupdated); } void @@ -19495,6 +19801,7 @@ ovnnb_db_run(struct northd_input *input_data, build_lb_datapaths(input_data->lbs, input_data->lb_groups, &data->ls_datapaths, &data->lr_datapaths, &data->lb_datapaths_map, &data->lb_group_datapaths_map); + build_router_lb_nats(&data->lr_datapaths.datapaths); build_ports(ovnsb_txn, input_data->sbrec_port_binding_table, input_data->sbrec_chassis_table, diff --git a/northd/northd.h b/northd/northd.h index fc2c083612..a499db00fc 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -113,6 +113,12 @@ struct tracked_ovn_ports { struct hmap deleted; }; +struct tracked_datapaths { + /* Tracked created or updated logical switches or + * routers. */ + struct hmapx crupdated; +}; + struct tracked_lb_datapaths { /* Tracked created or updated load balancers. * hmapx node data is 'struct ovn_lb_datapaths' */ @@ -137,6 +143,10 @@ struct tracked_lb_datapaths { struct northd_tracked_data { struct tracked_ovn_ports trk_ovn_ports; struct tracked_lb_datapaths trk_lbs; + struct tracked_datapaths trk_datapaths; + + /* Indicates if any router's NATs changed which were also LB vips. */ + bool lb_nats_changed; }; struct northd_data { @@ -334,6 +344,9 @@ struct ovn_datapath { /* Set of nat external ips on the router. */ struct sset external_ips; + /* Set of nat external ips which are lb vips too. */ + struct sset lb_nats; + /* SNAT IPs owned by the router (shash of 'struct ovn_snat_ip'). */ struct shash snat_ips; @@ -375,8 +388,10 @@ void ovnsb_db_run(struct ovsdb_idl_txn *ovnnb_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 *, - struct northd_data *); +bool northd_handle_lr_changes_pre_lb(const struct northd_input *, + struct northd_data *); +bool northd_handle_lr_changes_post_lb(const struct northd_input *, + struct northd_data *); void destroy_northd_data_tracked_changes(struct northd_data *); void northd_destroy(struct northd_data *data); void northd_init(struct northd_data *data); @@ -393,6 +408,10 @@ bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_lb_datapaths *, struct lflow_input *lflow_input, struct lflow_data *lflow_data); +bool lflow_handle_northd_od_changes(struct ovsdb_idl_txn *ovnsb_txn, + struct tracked_datapaths *, + struct lflow_input *lflow_input, + struct lflow_data *lflow_data); bool northd_handle_sb_port_binding_changes( const struct sbrec_port_binding_table *, struct hmap *ls_ports, struct hmap *lr_ports); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 37bfc3e0cf..ec28dea9c8 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -10395,3 +10395,205 @@ check test "$node_recompute_ct" -eq "1" AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Logical router incremental processing for NAT]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.11 + +check_engine_stats() { + node=$1 + recompute=$2 + compute=$3 + + echo "__file__:__line__: Checking engine stats for node $node : recompute - \ +$recompute : compute - $compute" + + node_stat=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats $node) + # node_stat will be of this format : + # - Node: lflow - recompute: 3 - compute: 0 - abort: 0 + node_recompute_ct=$(echo $node_stat | cut -d '-' -f2 | cut -d ':' -f2) + node_compute_ct=$(echo $node_stat | cut -d '-' -f3 | cut -d ':' -f2) + + if [[ "$recompute" == "norecompute" ]]; then + # node should not be recomputed + echo "Expecting $node recompute count - $node_recompute_ct to be 0" + check test "$node_recompute_ct" -eq "0" + else + echo "Expecting $node recompute count - $node_recompute_ct not to be 0" + check test "$node_recompute_ct" -ne "0" + fi + + if [[ "$compute" == "nocompute" ]]; then + # node should not be computed + echo "Expecting $node compute count - $node_compute_ct to be 0" + check test "$node_compute_ct" -eq "0" + else + echo "Expecting $node compute count - $node_compute_ct not to be 0" + check test "$node_compute_ct" -ne "0" + fi +} + +ovn-sbctl chassis-add gw1 geneve 127.0.0.1 + +check ovn-nbctl ls-add sw0 +check ovn-nbctl lsp-add sw0 sw0p1 -- lsp-set-addresses sw0p1 "00:00:20:20:12:01 10.0.0.4" + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-add lr0 +check_engine_stats northd recompute nocompute +check_engine_stats lflow recompute nocompute + +# Adding a logical router port should result in recompute +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl 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 lflow recompute nocompute + +ovn-nbctl lsp-add sw0 sw0-lr0 +ovn-nbctl lsp-set-type sw0-lr0 router +ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01 +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0 +check_engine_stats northd recompute nocompute +check_engine_stats lflow recompute nocompute + +ovn-nbctl ls-add public +ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24 +ovn-nbctl lsp-add public public-lr0 +ovn-nbctl lsp-set-type public-lr0 router +ovn-nbctl lsp-set-addresses public-lr0 router +ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public + +# localnet port +ovn-nbctl lsp-add public ln-public +ovn-nbctl lsp-set-type ln-public localnet +ovn-nbctl lsp-set-addresses ln-public unknown +ovn-nbctl lsp-set-options ln-public network_name=public + +# schedule the gw router port to a chassis. Change the name of the chassis +ovn-nbctl --wait=hv lrp-set-gateway-chassis lr0-public hv1 20 + +# Modify a logical router port and it should result in recompute. +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl set logical_router_port lr0-sw0 options:foo=bar +check_engine_stats northd recompute nocompute +check_engine_stats lflow recompute nocompute + +check as northd ovn-appctl -t NORTHD_TYPE vlog/set dbg + +# Do checks for NATs. +# Add a NAT. This should not result in recompute of both northd and lflow +# engine nodes. +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.4 +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Update the NAT options column +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb set NAT . options:foo=bar +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Update the NAT external_ip column +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb set NAT . external_ip=172.168.0.120 +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Update the NAT logical_ip column +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb set NAT . logical_ip=10.0.0.10 +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Update the NAT type +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb set NAT . type=snat +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Create a dnat_and_snat NAT with external_mac and logical_port +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.4 sw0p1 30:54:00:00:00:03 +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +nat2_uuid=$(ovn-nbctl --bare --columns _uuid find nat logical_ip=10.0.0.4) + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb set NAT $nat2_uuid external_mac='"30:54:00:00:00:04"' +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Create a load balancer and add the lb vip as NAT +check ovn-nbctl lb-add lb1 172.168.0.140 10.0.0.20 +check ovn-nbctl lb-add lb2 172.168.0.150:80 10.0.0.40:8080 +check ovn-nbctl lr-lb-add lr0 lb1 +check ovn-nbctl lr-lb-add lr0 lb2 + +# lflow engine should recompute since the nat ip 172.168.0.140 +# is a lb vip. +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.140 10.0.0.20 +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.150 10.0.0.41 +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-nat-del lr0 dnat_and_snat 172.168.0.150 +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-nat-del lr0 dnat_and_snat 172.168.0.140 +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Delete the NAT +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb clear logical_router lr0 nat +check_engine_stats northd norecompute compute +check_engine_stats lflow norecompute compute +check_engine_stats sync_to_sb_pb recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Create router Policy +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-policy-add lr0 10 "ip4.src == 10.0.0.3" reroute 172.168.0.101,172.168.0.102 +check_engine_stats northd recompute nocompute +check_engine_stats lflow recompute nocompute + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-policy-del lr0 10 "ip4.src == 10.0.0.3" +check_engine_stats northd recompute nocompute +check_engine_stats lflow recompute nocompute + +OVN_CLEANUP([hv1]) +AT_CLEANUP +])