From patchwork Tue Jan 30 21:22:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1893122 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.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 4TPdTf545jz1yQ0 for ; Wed, 31 Jan 2024 08:24:22 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id D57A16140A; Tue, 30 Jan 2024 21:24:20 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org D57A16140A X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id R48H5dFsyTY2; Tue, 30 Jan 2024 21:24:18 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 96DD461419; Tue, 30 Jan 2024 21:24:17 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 96DD461419 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6BB0EC0077; Tue, 30 Jan 2024 21:24:17 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1123DC0037 for ; Tue, 30 Jan 2024 21:24:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 3A6B561436 for ; Tue, 30 Jan 2024 21:23:22 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 3A6B561436 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id QJ7XXI5CPy4B for ; Tue, 30 Jan 2024 21:23:20 +0000 (UTC) Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::224]) by smtp3.osuosl.org (Postfix) with ESMTPS id 79D3761405 for ; Tue, 30 Jan 2024 21:23:19 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 79D3761405 Received: by mail.gandi.net (Postfix) with ESMTPSA id A9391E0003; Tue, 30 Jan 2024 21:23:15 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Tue, 30 Jan 2024 16:22:58 -0500 Message-ID: <20240130212258.1483365-1-numans@ovn.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240130212028.1482153-1-numans@ovn.org> References: <20240130212028.1482153-1-numans@ovn.org> MIME-Version: 1.0 X-GND-Sasl: numans@ovn.org Subject: [ovs-dev] [PATCH ovn v6 09/13] northd: Add lr_stateful handler for lflow engine node. 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 Acked-by: Han Zhou Signed-off-by: Numan Siddique Acked-by: Dumitru Ceara --- northd/en-lflow.c | 27 +++ northd/en-lflow.h | 1 + northd/en-lr-stateful.c | 33 +++- northd/en-lr-stateful.h | 31 +++- northd/inc-proc-northd.c | 2 +- northd/northd.c | 369 ++++++++++++++++++++++----------------- northd/northd.h | 11 +- tests/ovn-northd.at | 48 ++--- 8 files changed, 333 insertions(+), 189 deletions(-) diff --git a/northd/en-lflow.c b/northd/en-lflow.c index c49d24d54b..41b1779539 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -163,6 +163,33 @@ lflow_port_group_handler(struct engine_node *node, void *data OVS_UNUSED) return true; } +bool +lflow_lr_stateful_handler(struct engine_node *node, void *data) +{ + struct ed_type_lr_stateful *lr_sful_data = + engine_get_input_data("lr_stateful", node); + + if (!lr_stateful_has_tracked_data(&lr_sful_data->trk_data) + || lr_sful_data->trk_data.vip_nats_changed) { + return false; + } + + const struct engine_context *eng_ctx = engine_get_context(); + struct lflow_data *lflow_data = data; + struct lflow_input lflow_input; + + lflow_get_input_data(node, &lflow_input); + if (!lflow_handle_lr_stateful_changes(eng_ctx->ovnsb_idl_txn, + &lr_sful_data->trk_data, + &lflow_input, + lflow_data->lflow_table)) { + return false; + } + + engine_set_node_state(node, EN_UPDATED); + return true; +} + void *en_lflow_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) { diff --git a/northd/en-lflow.h b/northd/en-lflow.h index f7325c56b1..1d813a2a29 100644 --- a/northd/en-lflow.h +++ b/northd/en-lflow.h @@ -20,5 +20,6 @@ void *en_lflow_init(struct engine_node *node, struct engine_arg *arg); void en_lflow_cleanup(void *data); bool lflow_northd_handler(struct engine_node *, void *data); bool lflow_port_group_handler(struct engine_node *, void *data); +bool lflow_lr_stateful_handler(struct engine_node *, void *data); #endif /* EN_LFLOW_H */ diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c index 3e2a6c254e..5f3d677131 100644 --- a/northd/en-lr-stateful.c +++ b/northd/en-lr-stateful.c @@ -40,6 +40,7 @@ #include "lib/ovn-sb-idl.h" #include "lib/ovn-util.h" #include "lib/stopwatch-names.h" +#include "lflow-mgr.h" #include "northd.h" VLOG_DEFINE_THIS_MODULE(en_lr_stateful); @@ -81,7 +82,7 @@ static void remove_lrouter_lb_reachable_ips(struct lr_stateful_record *, enum lb_neighbor_responder_mode, const struct sset *lb_ips_v4, const struct sset *lb_ips_v6); -static void lr_stateful_rebuild_vip_nats(struct lr_stateful_record *); +static bool lr_stateful_rebuild_vip_nats(struct lr_stateful_record *); /* 'lr_stateful' engine node manages the NB logical router LB data. */ @@ -109,6 +110,7 @@ en_lr_stateful_clear_tracked_data(void *data_) struct ed_type_lr_stateful *data = data_; hmapx_clear(&data->trk_data.crupdated); + data->trk_data.vip_nats_changed = false; } void @@ -196,6 +198,10 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) /* Add the lr_stateful_rec rec to the tracking data. */ hmapx_add(&data->trk_data.crupdated, lr_stateful_rec); + + if (!sset_is_empty(&lr_stateful_rec->vip_nats)) { + data->trk_data.vip_nats_changed = true; + } continue; } @@ -313,7 +319,9 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) HMAPX_FOR_EACH (hmapx_node, &data->trk_data.crupdated) { struct lr_stateful_record *lr_stateful_rec = hmapx_node->data; - lr_stateful_rebuild_vip_nats(lr_stateful_rec); + if (lr_stateful_rebuild_vip_nats(lr_stateful_rec)) { + data->trk_data.vip_nats_changed = true; + } const struct ovn_datapath *od = ovn_datapaths_find_by_index(input_data.lr_datapaths, lr_stateful_rec->lr_index); @@ -352,8 +360,13 @@ lr_stateful_lr_nat_handler(struct engine_node *node, void *data_) lr_stateful_record_create(&data->table, lrnat_rec, od, input_data.lb_datapaths_map, input_data.lbgrp_datapaths_map); + if (!sset_is_empty(&lr_stateful_rec->vip_nats)) { + data->trk_data.vip_nats_changed = true; + } } else { - lr_stateful_rebuild_vip_nats(lr_stateful_rec); + if (lr_stateful_rebuild_vip_nats(lr_stateful_rec)) { + data->trk_data.vip_nats_changed = true; + } } /* Add the lr_stateful_rec rec to the tracking data. */ @@ -455,6 +468,7 @@ lr_stateful_record_create(struct lr_stateful_table *table, lr_stateful_rec->nbr_uuid = od->nbr->header_.uuid; lr_stateful_rec->lrnat_rec = lrnat_rec; lr_stateful_rec->lr_index = od->index; + lr_stateful_rec->lflow_ref = lflow_ref_create(); /* Initialize the record. @@ -543,6 +557,7 @@ lr_stateful_record_destroy(struct lr_stateful_record *lr_stateful_rec) { ovn_lb_ip_set_destroy(lr_stateful_rec->lb_ips); sset_destroy(&lr_stateful_rec->vip_nats); + lflow_ref_destroy(lr_stateful_rec->lflow_ref); free(lr_stateful_rec); } @@ -657,10 +672,12 @@ remove_lrouter_lb_reachable_ips(struct lr_stateful_record *lr_stateful_rec, } } -static void +static bool lr_stateful_rebuild_vip_nats(struct lr_stateful_record *lr_stateful_rec) { - sset_clear(&lr_stateful_rec->vip_nats); + struct sset old_vip_nats = SSET_INITIALIZER(&old_vip_nats); + sset_swap(&lr_stateful_rec->vip_nats, &old_vip_nats); + const char *external_ip; SSET_FOR_EACH (external_ip, &lr_stateful_rec->lrnat_rec->external_ips) { bool is_vip_nat = false; @@ -676,4 +693,10 @@ lr_stateful_rebuild_vip_nats(struct lr_stateful_record *lr_stateful_rec) sset_add(&lr_stateful_rec->vip_nats, external_ip); } } + + bool vip_nats_changed = !sset_equals(&lr_stateful_rec->vip_nats, + &old_vip_nats); + sset_destroy(&old_vip_nats); + + return vip_nats_changed; } diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h index 7529a61f33..ecff2238d4 100644 --- a/northd/en-lr-stateful.h +++ b/northd/en-lr-stateful.h @@ -32,6 +32,7 @@ struct ovn_datapath; struct lr_nat_record; +struct lflow_ref; /* lr_stateful_table: This represents a table of logical routers with * stateful related data. @@ -63,6 +64,30 @@ struct lr_stateful_record { /* sset of vips which are also part of lr nats. */ struct sset vip_nats; + + /* 'lflow_ref' is used to reference logical flows generated for + * this lr_stateful_record. + * + * This data is initialized and destroyed by the en_lr_stateful node, + * but populated and used only by the en_lflow node. Ideally this data + * should be maintained as part of en_lflow's data. However, it would + * be less efficient and more complex: + * + * 1. It would require an extra search (using the index) to find the + * lflows. + * + * 2. Building the index needs to be thread-safe, using either a global + * lock which is obviously less efficient, or hash-based lock array which + * is more complex. + * + * Adding the lflow_ref here is more straightforward. The drawback is that + * we need to keep in mind that this data belongs to en_lflow node, so + * never access it from any other nodes. + * + * Note: lflow_ref is not thread safe. Only one thread should + * access lr_stateful_record->lflow_ref at any given time. + */ + struct lflow_ref *lflow_ref; }; struct lr_stateful_table { @@ -82,6 +107,10 @@ struct lr_stateful_table { struct lr_stateful_tracked_data { /* Created or updated logical router with LB and/or NAT data. */ struct hmapx crupdated; /* Stores 'struct lr_stateful_record'. */ + + /* Indicates if any router's NATs changed which were also LB vips + * or vice versa. */ + bool vip_nats_changed; }; struct ed_type_lr_stateful { @@ -112,7 +141,7 @@ const struct lr_stateful_record *lr_stateful_table_find_by_index( static inline bool lr_stateful_has_tracked_data(struct lr_stateful_tracked_data *trk_data) { - return !hmapx_is_empty(&trk_data->crupdated); + return !hmapx_is_empty(&trk_data->crupdated) || trk_data->vip_nats_changed; } static inline bool diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 0e17bfe2e6..dcce79510a 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -228,11 +228,11 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_lflow, &en_sb_logical_flow, NULL); engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); engine_add_input(&en_lflow, &en_sb_igmp_group, NULL); - engine_add_input(&en_lflow, &en_lr_stateful, NULL); engine_add_input(&en_lflow, &en_ls_stateful, NULL); engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL); engine_add_input(&en_lflow, &en_northd, lflow_northd_handler); engine_add_input(&en_lflow, &en_port_group, lflow_port_group_handler); + engine_add_input(&en_lflow, &en_lr_stateful, lflow_lr_stateful_handler); engine_add_input(&en_sync_to_sb_addr_set, &en_nb_address_set, sync_to_sb_addr_set_nb_address_set_handler); diff --git a/northd/northd.c b/northd/northd.c index 937ccdf354..e3448be29f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -12815,62 +12815,6 @@ build_ip_routing_flows_for_lrp( } } -/* Logical router ingress table IP_ROUTING : IP Routing. - * - * For the LSP 'op' of type router, if there are logical router ports other - * than the LSP's peer connected to the logical switch, then for routable - * addresses (such as NAT IPs, LB VIPs, etc.) on each of the connected router - * ports, add routes to the LSP's peer router. - */ -static void -build_ip_routing_flows_for_router_type_lsp( - struct ovn_port *op, const struct lr_stateful_table *lr_stateful_table, - const struct hmap *lr_ports, struct lflow_table *lflows, - struct lflow_ref *lflow_ref) -{ - ovs_assert(op->nbsp); - if (!lsp_is_router(op->nbsp)) { - return; - } - - struct ovn_port *peer = ovn_port_get_peer(lr_ports, op); - if (!peer || !peer->nbrp || !peer->lrp_networks.n_ipv4_addrs - || !op->od->n_router_ports) { - return; - } - - for (size_t i = 0; i < op->od->n_router_ports; i++) { - struct ovn_port *router_port = ovn_port_get_peer( - lr_ports, op->od->router_ports[i]); - if (!router_port || !router_port->nbrp || router_port == peer) { - continue; - } - - const struct lr_stateful_record *lr_stateful_rec = - lr_stateful_table_find_by_index(lr_stateful_table, - router_port->od->index); - - if (router_port->nbrp->ha_chassis_group || - router_port->nbrp->n_gateway_chassis) { - struct ovn_port_routable_addresses ra = - get_op_routable_addresses(router_port, lr_stateful_rec); - for (size_t j = 0; j < ra.n_addrs; j++) { - struct lport_addresses *laddrs = &ra.laddrs[j]; - for (size_t k = 0; k < laddrs->n_ipv4_addrs; k++) { - add_route(lflows, peer->od, peer, - peer->lrp_networks.ipv4_addrs[0].addr_s, - laddrs->ipv4_addrs[k].network_s, - laddrs->ipv4_addrs[k].plen, NULL, false, 0, - &peer->nbrp->header_, false, - ROUTE_PRIO_OFFSET_CONNECTED, - lflow_ref); - } - } - destroy_routable_addresses(&ra); - } - } -} - static void build_static_route_flows_for_lrouter( struct ovn_datapath *od, const struct chassis_features *features, @@ -13112,42 +13056,6 @@ build_arp_resolve_flows_for_lrouter( lflow_ref); } -static void -routable_addresses_to_lflows(struct lflow_table *lflows, - struct ovn_port *router_port, - struct ovn_port *peer, - const struct lr_stateful_record *lr_stateful_rec, - struct ds *match, struct ds *actions, - struct lflow_ref *lflow_ref) -{ - struct ovn_port_routable_addresses ra = - get_op_routable_addresses(router_port, lr_stateful_rec); - if (!ra.n_addrs) { - return; - } - - for (size_t i = 0; i < ra.n_addrs; i++) { - ds_clear(match); - ds_put_format(match, "outport == %s && "REG_NEXT_HOP_IPV4" == {", - peer->json_key); - bool first = true; - for (size_t j = 0; j < ra.laddrs[i].n_ipv4_addrs; j++) { - if (!first) { - ds_put_cstr(match, ", "); - } - ds_put_cstr(match, ra.laddrs[i].ipv4_addrs[j].addr_s); - first = false; - } - ds_put_cstr(match, "}"); - - ds_clear(actions); - ds_put_format(actions, "eth.dst = %s; next;", ra.laddrs[i].ea_s); - ovn_lflow_add(lflows, peer->od, S_ROUTER_IN_ARP_RESOLVE, 100, - ds_cstr(match), ds_cstr(actions), lflow_ref); - } - destroy_routable_addresses(&ra); -} - /* Local router ingress table ARP_RESOLVE: ARP Resolution. * * Any unicast packet that reaches this table is an IP packet whose @@ -13390,52 +13298,6 @@ build_arp_resolve_flows_for_lsp( } } -static void -build_arp_resolve_flows_for_lsp_routable_addresses( - struct ovn_port *op, struct lflow_table *lflows, - const struct hmap *lr_ports, - const struct lr_stateful_table *lr_stateful_table, - struct ds *match, struct ds *actions, - struct lflow_ref *lflow_ref) -{ - if (!lsp_is_router(op->nbsp)) { - return; - } - - struct ovn_port *peer = ovn_port_get_peer(lr_ports, op); - if (!peer || !peer->nbrp) { - return; - } - - if (peer->od->nbr && - smap_get_bool(&peer->od->nbr->options, - "dynamic_neigh_routers", false)) { - return; - } - - for (size_t i = 0; i < op->od->n_router_ports; i++) { - struct ovn_port *router_port = - ovn_port_get_peer(lr_ports, op->od->router_ports[i]); - if (!router_port || !router_port->nbrp) { - continue; - } - - /* Skip the router port under consideration. */ - if (router_port == peer) { - continue; - } - - if (smap_get(&peer->od->nbr->options, "chassis") || peer->cr_port) { - const struct lr_stateful_record *lr_stateful_rec; - lr_stateful_rec = lr_stateful_table_find_by_index( - lr_stateful_table, router_port->od->index); - routable_addresses_to_lflows(lflows, router_port, peer, - lr_stateful_rec, match, actions, - lflow_ref); - } - } -} - static void build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct lflow_table *lflows, @@ -15649,8 +15511,6 @@ build_lrouter_nat_defrag_and_lb( static void build_lsp_lflows_for_lbnats(struct ovn_port *lsp, const struct lr_stateful_record *lr_stateful_rec, - const struct lr_stateful_table *lr_stateful_table, - const struct hmap *lr_ports, struct lflow_table *lflows, struct ds *match, struct ds *actions, @@ -15661,20 +15521,108 @@ build_lsp_lflows_for_lbnats(struct ovn_port *lsp, build_lswitch_rport_arp_req_flows_for_lbnats( lsp->peer, lr_stateful_rec, lsp->od, lsp, lflows, &lsp->nbsp->header_, lflow_ref); - build_ip_routing_flows_for_router_type_lsp(lsp, lr_stateful_table, - lr_ports, lflows, - lflow_ref); - build_arp_resolve_flows_for_lsp_routable_addresses( - lsp, lflows, lr_ports, lr_stateful_table, match, actions, lflow_ref); build_lswitch_ip_unicast_lookup_for_nats(lsp, lr_stateful_rec, lflows, match, actions, lflow_ref); } +/* Logical router ingress table IP_ROUTING : IP Routing. + * + * Adds the LRP 'lrp's routable addresses (addresses which can be routed via + * the LRP's datapath) as routable flows into the other router datapaths + * which are connected to the LRP's peer's logical switch. + * + * i.e If logical switch sw0 is conencted to the routers R0, R1 and R2, + * and if LRP of R0 has routable addresses (IP1 and IP2), then it adds + * the routes to reach these IPs in the R1 and R2's datapaths. + * + * This function also adds the ARP resolve flows for these addresses + * (IP1 and IP2) in the ARP_RESOLVE table of R1 and R2. + * */ +static void +build_routable_flows_for_router_port( + struct ovn_port *lrp, const struct lr_stateful_record *lr_stateful_rec, + struct lflow_table *lflows, + struct ds *match, + struct ds *actions) +{ + ovs_assert(lrp->nbrp && uuid_equals(&lrp->od->nbr->header_.uuid, + &lr_stateful_rec->nbr_uuid)); + + struct ovn_port *lsp_peer = lrp->peer; + if (!lsp_peer || !lsp_peer->nbsp) { + return; + } + + struct ovn_datapath *peer_ls = lsp_peer->od; + ovs_assert(peer_ls->nbs); + + struct ovn_port_routable_addresses ra = + get_op_routable_addresses(lrp, lr_stateful_rec); + + struct ovn_port *router_port; + + for (size_t i = 0; i < peer_ls->n_router_ports; i++) { + router_port = peer_ls->router_ports[i]->peer; + + if (router_port == lrp) { + continue; + } + + if (lrp->nbrp->ha_chassis_group || + lrp->nbrp->n_gateway_chassis) { + for (size_t j = 0; j < ra.n_addrs; j++) { + struct lport_addresses *laddrs = &ra.laddrs[j]; + for (size_t k = 0; k < laddrs->n_ipv4_addrs; k++) { + add_route(lflows, router_port->od, router_port, + router_port->lrp_networks.ipv4_addrs[0].addr_s, + laddrs->ipv4_addrs[k].network_s, + laddrs->ipv4_addrs[k].plen, NULL, false, 0, + &router_port->nbrp->header_, false, + ROUTE_PRIO_OFFSET_CONNECTED, + lrp->stateful_lflow_ref); + } + } + } + + bool dynamic_neigh_router = + smap_get_bool(&router_port->od->nbr->options, + "dynamic_neigh_routers", false); + + if (!dynamic_neigh_router && + (router_port->od->is_gw_router || router_port->cr_port)) { + + for (size_t k = 0; k < ra.n_addrs; k++) { + ds_clear(match); + ds_put_format(match, "outport == %s && " + REG_NEXT_HOP_IPV4" == {", + router_port->json_key); + bool first = true; + for (size_t j = 0; j < ra.laddrs[k].n_ipv4_addrs; j++) { + if (!first) { + ds_put_cstr(match, ", "); + } + ds_put_cstr(match, ra.laddrs[k].ipv4_addrs[j].addr_s); + first = false; + } + ds_put_cstr(match, "}"); + + ds_clear(actions); + ds_put_format(actions, "eth.dst = %s; next;", + ra.laddrs[k].ea_s); + ovn_lflow_add(lflows, router_port->od, S_ROUTER_IN_ARP_RESOLVE, + 100, ds_cstr(match), ds_cstr(actions), + lrp->stateful_lflow_ref); + } + } + } + + destroy_routable_addresses(&ra); +} + static void build_lbnat_lflows_iterate_by_lsp( struct ovn_port *op, const struct lr_stateful_table *lr_stateful_table, - const struct hmap *lr_ports, struct ds *match, struct ds *actions, - struct lflow_table *lflows) + struct ds *match, struct ds *actions, struct lflow_table *lflows) { ovs_assert(op->nbsp); @@ -15688,8 +15636,8 @@ build_lbnat_lflows_iterate_by_lsp( ovs_assert(lr_stateful_rec); build_lsp_lflows_for_lbnats(op, lr_stateful_rec, - lr_stateful_table, lr_ports, lflows, - match, actions, op->stateful_lflow_ref); + lflows,match, actions, + op->stateful_lflow_ref); } static void @@ -15699,6 +15647,9 @@ build_lrp_lflows_for_lbnats(struct ovn_port *op, struct ds *match, struct ds *actions, struct lflow_table *lflows) { + ovs_assert(op->nbrp && uuid_equals(&op->od->nbr->header_.uuid, + &lr_stateful_rec->nbr_uuid)); + /* Drop IP traffic destined to router owned IPs except if the IP is * also a SNAT IP. Those are dropped later, in stage * "lr_in_arp_resolve", if unSNAT was unsuccessful. @@ -15731,6 +15682,9 @@ build_lrp_lflows_for_lbnats(struct ovn_port *op, match, actions, op->stateful_lflow_ref); } +/* Builds the load balancer and NAT related flows for the router port 'op'. + * It uses the op->stateful_lflow_ref for lflow referencing. + */ static void build_lbnat_lflows_iterate_by_lrp( struct ovn_port *op, const struct lr_stateful_table *lr_stateful_table, @@ -15746,6 +15700,9 @@ build_lbnat_lflows_iterate_by_lrp( build_lrp_lflows_for_lbnats(op, lr_stateful_rec, meter_groups, match, actions, lflows); + + build_routable_flows_for_router_port(op, lr_stateful_rec, lflows, match, + actions); } static void @@ -15765,12 +15722,14 @@ build_lr_stateful_flows(const struct lr_stateful_record *lr_stateful_rec, ovs_assert(uuid_equals(&od->nbr->header_.uuid, &lr_stateful_rec->nbr_uuid)); build_lrouter_nat_defrag_and_lb(lr_stateful_rec, od, lflows, ls_ports, - lr_ports, match, actions, - meter_groups, features, NULL); + lr_ports, match, actions, meter_groups, + features, lr_stateful_rec->lflow_ref); build_lr_gateway_redirect_flows_for_nats(od, lr_stateful_rec->lrnat_rec, - lflows, match, actions, NULL); - build_lrouter_arp_nd_for_datapath(od, lr_stateful_rec->lrnat_rec, lflows, - meter_groups, NULL); + lflows, match, actions, + lr_stateful_rec->lflow_ref); + build_lrouter_arp_nd_for_datapath(od, lr_stateful_rec->lrnat_rec, + lflows, meter_groups, + lr_stateful_rec->lflow_ref); } static void @@ -15956,6 +15915,7 @@ build_lflows_thread(void *arg) /* Note: lflow_ref is not thread safe. Ensure that * - op->lflow_ref * - lb_dps->lflow_ref + * - lr_stateful_rec->lflow_ref * are not accessed by multiple threads at the same time. */ while (!stop_parallel_processing()) { wait_for_work(control); @@ -16006,7 +15966,7 @@ build_lflows_thread(void *arg) &lsi->actions, lsi->lflows); build_lbnat_lflows_iterate_by_lsp( - op, lsi->lr_stateful_table, lsi->lr_ports, &lsi->match, + op, lsi->lr_stateful_table, &lsi->match, &lsi->actions, lsi->lflows); } } @@ -16258,7 +16218,7 @@ build_lswitch_and_lrouter_flows( &lsi.actions, lsi.lflows); build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_stateful_table, - lsi.lr_ports, &lsi.match, + &lsi.match, &lsi.actions, lsi.lflows); } HMAP_FOR_EACH (op, key_node, lr_ports) { @@ -16462,9 +16422,15 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, void lflow_reset_northd_refs(struct lflow_input *lflow_input) { + const struct lr_stateful_record *lr_stateful_rec; struct ovn_lb_datapaths *lb_dps; struct ovn_port *op; + LR_STATEFUL_TABLE_FOR_EACH (lr_stateful_rec, + lflow_input->lr_stateful_table) { + lflow_ref_clear(lr_stateful_rec->lflow_ref); + } + HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) { lflow_ref_clear(op->lflow_ref); lflow_ref_clear(op->stateful_lflow_ref); @@ -16534,8 +16500,7 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, lflow_ref_unlink_lflows(op->stateful_lflow_ref); build_lbnat_lflows_iterate_by_lsp(op, lflow_input->lr_stateful_table, - lflow_input->lr_ports, &match, - &actions, lflows); + &match, &actions, lflows); handled = lflow_ref_sync_lflows( op->stateful_lflow_ref, lflows, ovnsb_txn, lflow_input->ls_datapaths, @@ -16589,8 +16554,7 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, /* Now generate the stateful lflows for 'op' */ build_lbnat_lflows_iterate_by_lsp(op, lflow_input->lr_stateful_table, - lflow_input->lr_ports, &match, - &actions, lflows); + &match, &actions, lflows); handled = lflow_ref_sync_lflows( op->stateful_lflow_ref, lflows, ovnsb_txn, lflow_input->ls_datapaths, @@ -16701,6 +16665,91 @@ lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, return true; } +bool +lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn, + struct lr_stateful_tracked_data *trk_data, + struct lflow_input *lflow_input, + struct lflow_table *lflows) +{ + struct lr_stateful_record *lr_stateful_rec; + struct ds actions = DS_EMPTY_INITIALIZER; + struct ds match = DS_EMPTY_INITIALIZER; + struct hmapx_node *hmapx_node; + bool handled = true; + + HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) { + lr_stateful_rec = hmapx_node->data; + /* Unlink old lflows. */ + lflow_ref_unlink_lflows(lr_stateful_rec->lflow_ref); + + /* Generate new lflows. */ + build_lr_stateful_flows(lr_stateful_rec, lflow_input->lr_datapaths, + lflows, lflow_input->ls_ports, + lflow_input->lr_ports, &match, &actions, + lflow_input->meter_groups, + lflow_input->features); + + /* Sync the new flows to SB. */ + handled = lflow_ref_sync_lflows( + lr_stateful_rec->lflow_ref, lflows, ovnsb_txn, + lflow_input->ls_datapaths, lflow_input->lr_datapaths, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + if (!handled) { + goto exit; + } + + const struct ovn_datapath *od = + ovn_datapaths_find_by_index(lflow_input->lr_datapaths, + lr_stateful_rec->lr_index); + struct ovn_port *op; + HMAP_FOR_EACH (op, dp_node, &od->ports) { + lflow_ref_unlink_lflows(op->stateful_lflow_ref); + + build_lbnat_lflows_iterate_by_lrp(op, + lflow_input->lr_stateful_table, + lflow_input->meter_groups, + &match, &actions, + lflows); + + handled = lflow_ref_sync_lflows( + op->stateful_lflow_ref, lflows, ovnsb_txn, + lflow_input->ls_datapaths, lflow_input->lr_datapaths, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + if (!handled) { + goto exit; + } + + if (op->peer && op->peer->nbsp) { + lflow_ref_unlink_lflows(op->peer->stateful_lflow_ref); + + build_lbnat_lflows_iterate_by_lsp( + op->peer, lflow_input->lr_stateful_table, &match, &actions, + lflows); + + handled = lflow_ref_sync_lflows( + op->peer->stateful_lflow_ref, lflows, ovnsb_txn, + lflow_input->ls_datapaths, lflow_input->lr_datapaths, + lflow_input->ovn_internal_version_changed, + lflow_input->sbrec_logical_flow_table, + lflow_input->sbrec_logical_dp_group_table); + if (!handled) { + goto exit; + } + } + } + } + +exit: + ds_destroy(&match); + ds_destroy(&actions); + + return handled; +} + static bool mirror_needs_update(const struct nbrec_mirror *nb_mirror, const struct sbrec_mirror *sb_mirror) diff --git a/northd/northd.h b/northd/northd.h index f37d6dac75..224b5d72ba 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -647,8 +647,9 @@ struct ovn_port { * 'patch/router' to reference logical flows generated fo this ovn_port * from the 'lr_stateful' record of the peer port's datapath. * - * Note: lflow_ref is not thread safe. Only one thread should - * access ovn_ports->lflow_ref at any given time. + * Note: lflow_ref and stateful_lflow_ref are not thread safe. Only one + * thread should access ovn_ports->lflow_ref/stateful_lflow_ref at any + * given time. */ struct lflow_ref *lflow_ref; struct lflow_ref *stateful_lflow_ref; @@ -676,6 +677,8 @@ void northd_indices_create(struct northd_data *data, struct ovsdb_idl *ovnsb_idl); struct lflow_table; +struct lr_stateful_tracked_data; + void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, struct lflow_input *input_data, struct lflow_table *); @@ -689,6 +692,10 @@ bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_lbs *, struct lflow_input *, struct lflow_table *lflows); +bool lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn *, + struct lr_stateful_tracked_data *, + struct lflow_input *, + struct lflow_table *lflows); 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 816eded21a..8a20643989 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -12,6 +12,7 @@ m4_define([_DUMP_DB_TABLES], [ ovn-sbctl list meter >> $1 ovn-sbctl list meter_band >> $1 ovn-sbctl list port_group >> $1 + ovn-sbctl dump-flows > lflows_$1 ]) # CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10784,7 +10785,7 @@ check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10794,7 +10795,7 @@ check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"10.0.0.1 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10804,7 +10805,7 @@ check ovn-nbctl --wait=sb clear load_Balancer lb1 vips check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10814,7 +10815,7 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10824,7 +10825,7 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10832,6 +10833,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb lr-lb-del lr0 lb1 check_engine_stats lb_data norecompute compute check_engine_stats northd recompute nocompute +check_engine_stats lr_stateful recompute nocompute check_engine_stats lflow recompute nocompute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10946,7 +10948,7 @@ check ovn-nbctl add logical_router lr0 load_balancer_group $lbg1_uuid check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10956,7 +10958,7 @@ check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"10.0.0.1 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10966,7 +10968,7 @@ check ovn-nbctl --wait=sb clear load_Balancer lb1 vips check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10976,7 +10978,7 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -10986,7 +10988,7 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11067,7 +11069,7 @@ check ovn-nbctl --wait=sb set logical_router lr1 load_balancer_group=$lbg1_uuid check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11095,7 +11097,7 @@ check ovn-nbctl --wait=sb lr-lb-add lr1 lb2 check_engine_stats lb_data norecompute compute check_engine_stats northd norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_lb recompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11665,7 +11667,7 @@ check as northd ovn-appctl -t ovn-northd 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 lr_nat norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11674,7 +11676,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb set NAT . options:foo=bar check_engine_stats northd norecompute compute check_engine_stats lr_nat norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11684,7 +11686,7 @@ check ovn-nbctl --wait=sb set NAT . external_ip=172.168.0.120 check_engine_stats northd norecompute compute check_engine_stats lr_nat norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11694,7 +11696,7 @@ check ovn-nbctl --wait=sb set NAT . logical_ip=10.0.0.10 check_engine_stats northd norecompute compute check_engine_stats lr_nat norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11704,7 +11706,7 @@ check ovn-nbctl --wait=sb set NAT . type=snat check_engine_stats northd norecompute compute check_engine_stats lr_nat norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11714,7 +11716,7 @@ check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.4 sw check_engine_stats northd norecompute compute check_engine_stats lr_nat norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11725,7 +11727,7 @@ 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 lr_nat norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -11746,6 +11748,8 @@ check_engine_stats sync_to_sb_pb recompute nocompute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE +# lflow engine should recompute since the nat ip 172.168.0.150 +# is a lb vip. check as northd ovn-appctl -t ovn-northd 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 @@ -11755,6 +11759,8 @@ check_engine_stats sync_to_sb_pb recompute nocompute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE +# lflow engine should recompute since the deleted nat ip 172.168.0.150 +# is a lb vip. check as northd ovn-appctl -t ovn-northd 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 @@ -11764,6 +11770,8 @@ check_engine_stats sync_to_sb_pb recompute nocompute check_engine_stats lflow recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE +# lflow engine should recompute since the deleted nat ip 172.168.0.140 +# is a lb vip. check as northd ovn-appctl -t ovn-northd 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 @@ -11779,7 +11787,7 @@ check ovn-nbctl --wait=sb clear logical_router lr0 nat check_engine_stats northd norecompute compute check_engine_stats lr_nat norecompute compute check_engine_stats lr_stateful norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_to_sb_pb recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE