From patchwork Tue Nov 28 02:37:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1869081 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 4SfRTK1jHpz1yRy for ; Tue, 28 Nov 2023 13:38:13 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id EBAA760F93; Tue, 28 Nov 2023 02:38:10 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org EBAA760F93 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 vG1wKXxKjRpJ; Tue, 28 Nov 2023 02:38:08 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 81BF96100A; Tue, 28 Nov 2023 02:38:07 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 81BF96100A Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4F4EAC0DD0; Tue, 28 Nov 2023 02:38:07 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 20BB8C0039 for ; Tue, 28 Nov 2023 02:38:06 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id C1F7641526 for ; Tue, 28 Nov 2023 02:37:42 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org C1F7641526 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Oxvx6saD8B-H for ; Tue, 28 Nov 2023 02:37:40 +0000 (UTC) Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::222]) by smtp4.osuosl.org (Postfix) with ESMTPS id D9D5741514 for ; Tue, 28 Nov 2023 02:37:39 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org D9D5741514 Received: by mail.gandi.net (Postfix) with ESMTPSA id 2C9B540002; Tue, 28 Nov 2023 02:37:36 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 27 Nov 2023 21:37:22 -0500 Message-ID: <20231128023722.569941-1-numans@ovn.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231128023410.569539-1-numans@ovn.org> References: <20231128023410.569539-1-numans@ovn.org> MIME-Version: 1.0 X-GND-Sasl: numans@ovn.org Subject: [ovs-dev] [PATCH ovn v3 12/16] 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 Signed-off-by: Numan Siddique --- northd/en-lflow.c | 29 +++ northd/en-lflow.h | 1 + northd/en-lr-stateful.c | 39 ++++- northd/en-lr-stateful.h | 26 +++ northd/inc-proc-northd.c | 3 +- northd/northd.c | 370 ++++++++++++++++++++++----------------- northd/northd.h | 10 ++ tests/ovn-northd.at | 48 ++--- 8 files changed, 336 insertions(+), 190 deletions(-) diff --git a/northd/en-lflow.c b/northd/en-lflow.c index 13284b5556..09748f570b 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -164,6 +164,35 @@ 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 a54749ad93..8e025f057e 100644 --- a/northd/en-lr-stateful.c +++ b/northd/en-lr-stateful.c @@ -39,6 +39,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_build_vip_nats(struct lr_stateful_record *); +static bool lr_stateful_build_vip_nats(struct lr_stateful_record *); /* 'lr_stateful' engine node manages the NB logical router LB data. */ @@ -110,6 +111,7 @@ en_lr_stateful_clear_tracked_data(void *data_) struct ed_type_lr_stateful *data = (struct ed_type_lr_stateful *) data_; hmapx_clear(&data->trk_data.crupdated); + data->trk_data.vip_nats_changed = false; } void @@ -190,6 +192,10 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) /* Add the lr_sful_rec rec to the tracking data. */ hmapx_add(&data->trk_data.crupdated, lr_sful_rec); + + if (!sset_is_empty(&lr_sful_rec->vip_nats)) { + data->trk_data.vip_nats_changed = true; + } continue; } @@ -298,7 +304,9 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_) * vip nats. */ HMAPX_FOR_EACH (hmapx_node, &data->trk_data.crupdated) { lr_sful_rec = hmapx_node->data; - lr_stateful_build_vip_nats(lr_sful_rec); + if (lr_stateful_build_vip_nats(lr_sful_rec)) { + data->trk_data.vip_nats_changed = true; + } lr_sful_rec->has_lb_vip = od_has_lb_vip(lr_sful_rec->od); } @@ -335,8 +343,13 @@ lr_stateful_lr_nat_handler(struct engine_node *node, void *data_) lrnat_rec, input_data.lb_datapaths_map, input_data.lbgrp_datapaths_map); + if (!sset_is_empty(&lr_sful_rec->vip_nats)) { + data->trk_data.vip_nats_changed = true; + } } else { - lr_stateful_build_vip_nats(lr_sful_rec); + if (lr_stateful_build_vip_nats(lr_sful_rec)) { + data->trk_data.vip_nats_changed = true; + } } /* Add the lr_sful_rec rec to the tracking data. */ @@ -435,6 +448,7 @@ lr_stateful_record_create(struct lr_stateful_table *table, lr_sful_rec->od = lrnat_rec->od; lr_stateful_record_init(lr_sful_rec, lb_datapaths_map, lbgrp_datapaths_map); + lr_sful_rec->lflow_ref = lflow_ref_alloc(lr_sful_rec->od->nbr->name); hmap_insert(&table->entries, &lr_sful_rec->key_node, uuid_hash(&lr_sful_rec->od->nbr->header_.uuid)); @@ -449,6 +463,7 @@ lr_stateful_record_destroy(struct lr_stateful_record *lr_sful_rec) ovn_lb_ip_set_destroy(lr_sful_rec->lb_ips); lr_sful_rec->lb_ips = NULL; sset_destroy(&lr_sful_rec->vip_nats); + lflow_ref_destroy(lr_sful_rec->lflow_ref); free(lr_sful_rec); } @@ -515,7 +530,7 @@ lr_stateful_record_init(struct lr_stateful_record *lr_sful_rec, sset_init(&lr_sful_rec->vip_nats); - if (!nbr->n_nat) { + if (nbr->n_nat) { lr_stateful_build_vip_nats(lr_sful_rec); } @@ -629,10 +644,12 @@ remove_lrouter_lb_reachable_ips(struct lr_stateful_record *lr_sful_rec, } } -static void +static bool lr_stateful_build_vip_nats(struct lr_stateful_record *lr_sful_rec) { - sset_clear(&lr_sful_rec->vip_nats); + struct sset old_vip_nats = SSET_INITIALIZER(&old_vip_nats); + sset_swap(&lr_sful_rec->vip_nats, &old_vip_nats); + const char *external_ip; SSET_FOR_EACH (external_ip, &lr_sful_rec->lrnat_rec->external_ips) { bool is_vip_nat = false; @@ -648,4 +665,14 @@ lr_stateful_build_vip_nats(struct lr_stateful_record *lr_sful_rec) sset_add(&lr_sful_rec->vip_nats, external_ip); } } + + bool vip_nats_changed = + sset_count(&lr_sful_rec->vip_nats) != sset_count(&old_vip_nats); + if (!vip_nats_changed) { + vip_nats_changed = !sset_equals(&lr_sful_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 0571e057f8..077de1bad1 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. @@ -56,6 +57,27 @@ 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_lb_nat_data record. + * + * This data is initialized and destroyed by the en_lr_lb_nat_data 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. + */ + struct lflow_ref *lflow_ref; }; struct lr_stateful_table { @@ -75,6 +97,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 { diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 0e17bfe2e6..8be413200e 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -228,11 +228,12 @@ 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_ls_stateful, 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 f56916b3da..235b9e100f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -1227,7 +1227,7 @@ ovn_port_create(struct hmap *ports, const char *key, op->lflow_ref = lflow_ref_alloc(key); op->stateful_lflow_ref = lflow_ref_alloc(key); - + op->routable_lflow_ref = lflow_ref_alloc(key); return op; } @@ -1268,6 +1268,7 @@ ovn_port_destroy_orphan(struct ovn_port *port) free(port->key); lflow_ref_destroy(port->lflow_ref); lflow_ref_destroy(port->stateful_lflow_ref); + lflow_ref_destroy(port->routable_lflow_ref); free(port); } @@ -12832,63 +12833,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_sful_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 (int 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_sful_rec = - lr_stateful_table_find_by_index(lr_sful_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_sful_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, @@ -13130,42 +13074,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_sful_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_sful_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 @@ -13408,52 +13316,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_sful_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_sful_rec; - lr_sful_rec = lr_stateful_table_find_by_index(lr_sful_table, - router_port->od->index); - routable_addresses_to_lflows(lflows, router_port, peer, - lr_sful_rec, match, actions, - lflow_ref); - } - } -} - static void build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct lflow_table *lflows, @@ -15670,8 +15532,6 @@ static void build_lsp_lflows_for_lbnats(struct ovn_port *lsp, struct ovn_port *lrp_peer, const struct lr_stateful_record *lr_sful_rec, - const struct lr_stateful_table *lr_sful_table, - const struct hmap *lr_ports, struct lflow_table *lflows, struct ds *match, struct ds *actions, @@ -15681,19 +15541,101 @@ build_lsp_lflows_for_lbnats(struct ovn_port *lsp, build_lswitch_rport_arp_req_flows_for_lbnats( lrp_peer, lr_sful_rec, lsp->od, lsp, lflows, &lsp->nbsp->header_, lflow_ref); - build_ip_routing_flows_for_router_type_lsp(lsp, lr_sful_table, - lr_ports, lflows, - lflow_ref); - build_arp_resolve_flows_for_lsp_routable_addresses( - lsp, lflows, lr_ports, lr_sful_table, match, actions, lflow_ref); build_lswitch_ip_unicast_lookup_for_nats(lsp, lr_sful_rec, lflows, match, actions, lflow_ref); } +/* Logical router ingress table IP_ROUTING : IP Routing. + * + * For the LRP 'lrp's peer's logical switch, if there are logical router + * ports ('peer_lrp's) other than the 'lrp', then for routable addresses + * (such as NAT IPs, LB VIPs, etc.) of the 'lrp' add routes to the + * peer_lrp's datapath. + */ +static void +build_routable_flows_for_router_port( + struct ovn_port *lrp, const struct lr_stateful_record *lr_sful_rec, + struct lflow_table *lflows, + struct ds *match, + struct ds *actions, + struct lflow_ref *lflow_ref) +{ + ovs_assert(lrp->nbrp && lrp->od == lr_sful_rec->od); + + 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_sful_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, + 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), + lflow_ref); + } + } + } + + destroy_routable_addresses(&ra); +} + static void build_lbnat_lflows_iterate_by_lsp(struct ovn_port *op, const struct lr_stateful_table *lr_sful_table, - const struct hmap *lr_ports, struct ds *match, struct ds *actions, struct lflow_table *lflows, @@ -15711,8 +15653,8 @@ build_lbnat_lflows_iterate_by_lsp(struct ovn_port *op, ovs_assert(lr_sful_rec); build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec, - lr_sful_table, lr_ports, lflows, - match, actions, lflow_ref); + lflows,match, actions, + lflow_ref); } static void @@ -15762,7 +15704,8 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port *op, struct ds *match, struct ds *actions, struct lflow_table *lflows, - struct lflow_ref *lflow_ref) + struct lflow_ref *lflow_ref, + struct lflow_ref *routable_lflow_ref) { ovs_assert(op->nbrp); @@ -15773,6 +15716,9 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port *op, build_lrp_lflows_for_lbnats(op, lr_sful_rec, meter_groups, match, actions, lflows, lflow_ref); + + build_routable_flows_for_router_port(op, lr_sful_rec, lflows, match, + actions, routable_lflow_ref); } static void @@ -15787,15 +15733,15 @@ build_lr_stateful_flows(const struct lr_stateful_record *lr_sful_rec, { build_lrouter_nat_defrag_and_lb(lr_sful_rec, lflows, ls_ports, lr_ports, match, actions, meter_groups, features, - NULL); + lr_sful_rec->lflow_ref); build_lr_gateway_redirect_flows_for_nats(lr_sful_rec->od, lr_sful_rec->lrnat_rec, lflows, match, actions, - NULL); + lr_sful_rec->lflow_ref); build_lrouter_arp_nd_for_datapath(lr_sful_rec->od, lr_sful_rec->lrnat_rec, lflows, meter_groups, - NULL); + lr_sful_rec->lflow_ref); } static void @@ -16026,7 +15972,6 @@ build_lflows_thread(void *arg) &lsi->actions, lsi->lflows); build_lbnat_lflows_iterate_by_lsp(op, lsi->lr_sful_table, - lsi->lr_ports, &lsi->match, &lsi->actions, lsi->lflows, @@ -16048,7 +15993,8 @@ build_lflows_thread(void *arg) &lsi->match, &lsi->actions, lsi->lflows, - op->stateful_lflow_ref); + op->stateful_lflow_ref, + op->routable_lflow_ref); } } for (bnum = control->id; @@ -16280,9 +16226,9 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths, &lsi.match, &lsi.actions, lsi.lflows); - build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table, lsi.lr_ports, - &lsi.match, &lsi.actions, - lsi.lflows, + build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table, + &lsi.match, + &lsi.actions, lsi.lflows, op->stateful_lflow_ref); } HMAP_FOR_EACH (op, key_node, lr_ports) { @@ -16292,7 +16238,8 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths, &lsi.match, &lsi.actions, lsi.lflows, - op->stateful_lflow_ref); + op->stateful_lflow_ref, + op->routable_lflow_ref); } stopwatch_stop(LFLOWS_PORTS_STOPWATCH_NAME, time_msec()); stopwatch_start(LFLOWS_LBS_STOPWATCH_NAME, time_msec()); @@ -16485,12 +16432,24 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, void reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input) { + const struct lr_stateful_record *lr_sful_rec; struct ovn_lb_datapaths *lb_dps; struct ovn_port *op; + LR_STATEFUL_TABLE_FOR_EACH (lr_sful_rec, lflow_input->lr_sful_table) { + lflow_ref_reset(lr_sful_rec->lflow_ref); + } + HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) { lflow_ref_reset(op->lflow_ref); lflow_ref_reset(op->stateful_lflow_ref); + lflow_ref_reset(op->routable_lflow_ref); + } + + HMAP_FOR_EACH (op, key_node, lflow_input->lr_ports) { + lflow_ref_reset(op->lflow_ref); + lflow_ref_reset(op->stateful_lflow_ref); + lflow_ref_reset(op->routable_lflow_ref); } HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) { @@ -16547,11 +16506,10 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, ovs_assert(lr_sful_rec); lflow_ref_clear_lflows(op->stateful_lflow_ref); build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec, - lflow_input->lr_sful_table, - lflow_input->lr_ports, lflows, &match, &actions, op->stateful_lflow_ref); - lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows, ovnsb_txn, + lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows, + ovnsb_txn, lflow_input->ls_datapaths, lflow_input->lr_datapaths, lflow_input->ovn_internal_version_changed, @@ -16705,6 +16663,92 @@ 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_sful_rec; + struct hmapx_node *hmapx_node; + + HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) { + lr_sful_rec = hmapx_node->data; + + lflow_ref_clear_lflows(lr_sful_rec->lflow_ref); + + /* Generate new lflows. */ + struct ds match = DS_EMPTY_INITIALIZER; + struct ds actions = DS_EMPTY_INITIALIZER; + + build_lr_stateful_flows(lr_sful_rec, lflows, lflow_input->ls_ports, + lflow_input->lr_ports, &match, &actions, + lflow_input->meter_groups, + lflow_input->features); + + /* Sync the new flows to SB. */ + lflow_ref_sync_lflows_to_sb(lr_sful_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); + + struct ovn_port *op; + HMAP_FOR_EACH (op, dp_node, &lr_sful_rec->od->ports) { + lflow_ref_clear_lflows(op->stateful_lflow_ref); + lflow_ref_clear_lflows_for_all_dps(op->routable_lflow_ref, + ods_size(lflow_input->ls_datapaths), + ods_size(lflow_input->lr_datapaths)); + + build_lbnat_lflows_iterate_by_lrp(op, lflow_input->lr_sful_table, + lflow_input->meter_groups, + &match, &actions, + lflows, + op->stateful_lflow_ref, + op->routable_lflow_ref); + + lflow_ref_sync_lflows_to_sb(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); + + lflow_ref_sync_lflows_to_sb(op->routable_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 (op->peer && op->peer->nbsp) { + lflow_ref_clear_lflows(op->peer->stateful_lflow_ref); + + build_lbnat_lflows_iterate_by_lsp(op->peer, + lflow_input->lr_sful_table, + &match, &actions, + lflows, + op->peer->stateful_lflow_ref); + + lflow_ref_sync_lflows_to_sb(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); + } + } + + ds_destroy(&match); + ds_destroy(&actions); + } + + return true; +} + 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 863bcce001..6adf06c26f 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -688,9 +688,13 @@ struct ovn_port { * 'stateful_lflow_ref' is used for logical switch ports of type * 'patch/router' to referenece logical flows generated fo this ovn_port * from the 'lr_stateful' record of the peer port's datapath. + * + * 'routable_lflow_ref' is used to reference logical flows generated for + * the routable ips of a logical router port. */ struct lflow_ref *lflow_ref; struct lflow_ref *stateful_lflow_ref; + struct lflow_ref *routable_lflow_ref; }; void ovnnb_db_run(struct northd_input *input_data, @@ -715,6 +719,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 *); @@ -728,6 +734,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 50d88fecd5..f517057534 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 @@ -10687,7 +10688,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 @@ -10697,7 +10698,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 @@ -10707,7 +10708,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 @@ -10717,7 +10718,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 @@ -10727,7 +10728,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 @@ -10735,6 +10736,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 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 @@ -10850,7 +10852,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 @@ -10860,7 +10862,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 @@ -10870,7 +10872,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 @@ -10880,7 +10882,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 @@ -10890,7 +10892,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 @@ -10971,7 +10973,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 @@ -10999,7 +11001,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 @@ -11177,7 +11179,7 @@ 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 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 @@ -11186,7 +11188,7 @@ 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 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 @@ -11196,7 +11198,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 @@ -11206,7 +11208,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 @@ -11216,7 +11218,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 @@ -11226,7 +11228,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 @@ -11237,7 +11239,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 @@ -11258,6 +11260,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 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 @@ -11267,6 +11271,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 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 @@ -11276,6 +11282,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 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 @@ -11291,7 +11299,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