From patchwork Thu Jun 3 12:28:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1487210 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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=) 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 RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FwlYf1FzVz9sxS for ; Thu, 3 Jun 2021 22:28:38 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 88E4260A4C; Thu, 3 Jun 2021 12:28:36 +0000 (UTC) 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 0bR_y2JLP0Ls; Thu, 3 Jun 2021 12:28:34 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id 2DB1B607F3; Thu, 3 Jun 2021 12:28:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0011FC000D; Thu, 3 Jun 2021 12:28:32 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 89C15C0001 for ; Thu, 3 Jun 2021 12:28:31 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 6FC7B400E7 for ; Thu, 3 Jun 2021 12:28:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7BAqAjo5l1Fg for ; Thu, 3 Jun 2021 12:28:29 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by smtp2.osuosl.org (Postfix) with ESMTPS id B1027400E5 for ; Thu, 3 Jun 2021 12:28:28 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 2DD05240009; Thu, 3 Jun 2021 12:28:23 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 3 Jun 2021 08:28:20 -0400 Message-Id: <20210603122820.2066656-1-numans@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210603122721.2066524-1-numans@ovn.org> References: <20210603122721.2066524-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 1/5] ovn-controller: Split logical flow and physical flow processing. 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 Presently, the 'flow_output' engine node recomputes physical flows by calling physical_run() in the 'physical_flow_changes' handler in some scenarios. Because of this, an engine run can do a full recompute of physical flows but not full recompute of logical flows. Although this works now, it is problematic as the same desired flow table is used for both physical and logical flows. This patch now separates the handling of logical flows and physical flows and removes the 'physical_flow_changes' engine node. Two separate engine nodes are added - lflow_output and pflow_output with their own flow tables and these two nodes are now inputs to the main engine node - flow_output. This separation reflects the data dependency more clearly. CC: Han Zhou Signed-off-by: Numan Siddique --- TODO.rst | 6 + controller/ofctrl.c | 99 ++++-- controller/ofctrl.h | 6 +- controller/ovn-controller.c | 691 +++++++++++++++++------------------- controller/physical.c | 19 - controller/physical.h | 4 - 6 files changed, 406 insertions(+), 419 deletions(-) diff --git a/TODO.rst b/TODO.rst index c89fe203e..618ea4844 100644 --- a/TODO.rst +++ b/TODO.rst @@ -164,3 +164,9 @@ OVN To-do List to find a way of determining if routing has already been executed (on a different hypervisor) for the IP multicast packet being processed locally in the router pipeline. + +* ovn-controller Incremental processing + + * physical.c has a global simap -localvif_to_ofport which stores the + local OVS interfaces and the ofport numbers. Move this to the engine data + of the engine data node - ed_type_pflow_output. diff --git a/controller/ofctrl.c b/controller/ofctrl.c index c29c3d180..053631590 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -173,7 +173,7 @@ struct sb_flow_ref { struct uuid sb_uuid; }; -/* A installed flow, in static variable installed_flows. +/* An installed flow, in static variable installed_lflows/installed_pflows. * * Installed flows are updated in ofctrl_put for maintaining the flow * installation to OVS. They are updated according to desired flows: either by @@ -234,7 +234,7 @@ static struct desired_flow *desired_flow_lookup_conjunctive( static void desired_flow_destroy(struct desired_flow *); static struct installed_flow *installed_flow_lookup( - const struct ovn_flow *target); + const struct ovn_flow *target, struct hmap *installed_flows); static void installed_flow_destroy(struct installed_flow *); static struct installed_flow *installed_flow_dup(struct desired_flow *); static struct desired_flow *installed_flow_get_active(struct installed_flow *); @@ -302,9 +302,12 @@ static ovs_be32 xid, xid2; * zero, to avoid unbounded buffering. */ static struct rconn_packet_counter *tx_counter; -/* Flow table of "struct ovn_flow"s, that holds the flow table currently - * installed in the switch. */ -static struct hmap installed_flows; +/* Flow table of "struct ovn_flow"s, that holds the logical flow table + * currently installed in the switch. */ +static struct hmap installed_lflows; +/* Flow table of "struct ovn_flow"s, that holds the physical flow table + * currently installed in the switch. */ +static struct hmap installed_pflows; /* A reference to the group_table. */ static struct ovn_extend_table *groups; @@ -343,7 +346,8 @@ ofctrl_init(struct ovn_extend_table *group_table, swconn = rconn_create(inactivity_probe_interval, 0, DSCP_DEFAULT, 1 << OFP15_VERSION); tx_counter = rconn_packet_counter_create(); - hmap_init(&installed_flows); + hmap_init(&installed_lflows); + hmap_init(&installed_pflows); ovs_list_init(&flow_updates); ovn_init_symtab(&symtab); groups = group_table; @@ -1426,11 +1430,12 @@ desired_flow_lookup_conjunctive(struct ovn_desired_flow_table *flow_table, /* Finds and returns an installed_flow in installed_flows whose key is * identical to 'target''s key, or NULL if there is none. */ static struct installed_flow * -installed_flow_lookup(const struct ovn_flow *target) +installed_flow_lookup(const struct ovn_flow *target, + struct hmap *installed_flows) { struct installed_flow *i; HMAP_FOR_EACH_WITH_HASH (i, match_hmap_node, target->hash, - &installed_flows) { + installed_flows) { struct ovn_flow *f = &i->flow; if (f->table_id == target->table_id && f->priority == target->priority @@ -1542,8 +1547,14 @@ static void ovn_installed_flow_table_clear(void) { struct installed_flow *f, *next; - HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &installed_flows) { - hmap_remove(&installed_flows, &f->match_hmap_node); + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &installed_lflows) { + hmap_remove(&installed_lflows, &f->match_hmap_node); + unlink_all_refs_for_installed_flow(f); + installed_flow_destroy(f); + } + + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &installed_pflows) { + hmap_remove(&installed_pflows, &f->match_hmap_node); unlink_all_refs_for_installed_flow(f); installed_flow_destroy(f); } @@ -1553,7 +1564,8 @@ static void ovn_installed_flow_table_destroy(void) { ovn_installed_flow_table_clear(); - hmap_destroy(&installed_flows); + hmap_destroy(&installed_lflows); + hmap_destroy(&installed_pflows); } /* Flow table update. */ @@ -1829,6 +1841,7 @@ installed_flow_del(struct ovn_flow *i, static void update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, struct ofputil_bundle_ctrl_msg *bc, + struct hmap *installed_flows, struct ovs_list *msgs) { ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); @@ -1836,7 +1849,7 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, * longer desired, delete them; if any of them should have different * actions, update them. */ struct installed_flow *i, *next; - HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { + HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, installed_flows) { unlink_all_refs_for_installed_flow(i); struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow); if (!d) { @@ -1845,7 +1858,7 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, installed_flow_del(&i->flow, bc, msgs); ovn_flow_log(&i->flow, "removing installed"); - hmap_remove(&installed_flows, &i->match_hmap_node); + hmap_remove(installed_flows, &i->match_hmap_node); installed_flow_destroy(i); } else { if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, @@ -1863,14 +1876,14 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, * in the installed flow table. */ struct desired_flow *d; HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { - i = installed_flow_lookup(&d->flow); + i = installed_flow_lookup(&d->flow, installed_flows); if (!i) { ovn_flow_log(&d->flow, "adding installed"); installed_flow_add(&d->flow, bc, msgs); /* Copy 'd' from 'flow_table' to installed_flows. */ i = installed_flow_dup(d); - hmap_insert(&installed_flows, &i->match_hmap_node, i->flow.hash); + hmap_insert(installed_flows, &i->match_hmap_node, i->flow.hash); link_installed_to_desired(i, d); } else if (!d->installed_flow) { /* This is a desired_flow that conflicts with one installed @@ -1961,6 +1974,7 @@ merge_tracked_flows(struct ovn_desired_flow_table *flow_table) static void update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, struct ofputil_bundle_ctrl_msg *bc, + struct hmap *installed_flows, struct ovs_list *msgs) { merge_tracked_flows(flow_table); @@ -1979,7 +1993,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, installed_flow_del(&i->flow, bc, msgs); ovn_flow_log(&i->flow, "removing installed (tracked)"); - hmap_remove(&installed_flows, &i->match_hmap_node); + hmap_remove(installed_flows, &i->match_hmap_node); installed_flow_destroy(i); } else if (was_active) { /* There are other desired flow(s) referencing this @@ -1993,7 +2007,8 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, desired_flow_destroy(f); } else { /* The desired flow was added or modified. */ - struct installed_flow *i = installed_flow_lookup(&f->flow); + struct installed_flow *i = installed_flow_lookup(&f->flow, + installed_flows); if (!i) { /* Adding a new flow. */ installed_flow_add(&f->flow, bc, msgs); @@ -2001,7 +2016,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, /* Copy 'f' from 'flow_table' to installed_flows. */ struct installed_flow *new_node = installed_flow_dup(f); - hmap_insert(&installed_flows, &new_node->match_hmap_node, + hmap_insert(installed_flows, &new_node->match_hmap_node, new_node->flow.hash); link_installed_to_desired(new_node, f); } else if (installed_flow_get_active(i) == f) { @@ -2055,16 +2070,19 @@ ofctrl_can_put(void) * * This should be called after ofctrl_run() within the main loop. */ void -ofctrl_put(struct ovn_desired_flow_table *flow_table, +ofctrl_put(struct ovn_desired_flow_table *lflow_table, + struct ovn_desired_flow_table *pflow_table, struct shash *pending_ct_zones, const struct sbrec_meter_table *meter_table, uint64_t req_cfg, - bool flow_changed) + bool lflows_changed, + bool pflows_changed) { static bool skipped_last_time = false; static uint64_t old_req_cfg = 0; bool need_put = false; - if (flow_changed || skipped_last_time || need_reinstall_flows) { + if (lflows_changed || pflows_changed || skipped_last_time || + need_reinstall_flows) { need_put = true; old_req_cfg = req_cfg; } else if (req_cfg != old_req_cfg) { @@ -2093,7 +2111,6 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, return; } - skipped_last_time = false; need_reinstall_flows = false; /* OpenFlow messages to send to the switch to bring it up-to-date. */ @@ -2159,12 +2176,35 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, bundle_open = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc); ovs_list_push_back(&msgs, &bundle_open->list_node); - if (flow_table->change_tracked) { - update_installed_flows_by_track(flow_table, &bc, &msgs); - } else { - update_installed_flows_by_compare(flow_table, &bc, &msgs); + /* If skipped last time, then process the flow table + * (tracked) flows even if lflows_changed is not set. + * Same for pflows_changed. */ + if (lflows_changed || skipped_last_time) { + if (lflow_table->change_tracked) { + update_installed_flows_by_track(lflow_table, &bc, + &installed_lflows, + &msgs); + } else { + update_installed_flows_by_compare(lflow_table, &bc, + &installed_lflows, + &msgs); + } + } + + if (pflows_changed || skipped_last_time) { + if (pflow_table->change_tracked) { + update_installed_flows_by_track(pflow_table, &bc, + &installed_pflows, + &msgs); + } else { + update_installed_flows_by_compare(pflow_table, &bc, + &installed_pflows, + &msgs); + } } + skipped_last_time = false; + if (ovs_list_back(&msgs) == &bundle_open->list_node) { /* No flow updates. Removing the bundle open request. */ ovs_list_pop_back(&msgs); @@ -2287,8 +2327,11 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, cur_cfg = req_cfg; } - flow_table->change_tracked = true; - ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); + lflow_table->change_tracked = true; + ovs_assert(ovs_list_is_empty(&lflow_table->tracked_flows)); + + pflow_table->change_tracked = true; + ovs_assert(ovs_list_is_empty(&pflow_table->tracked_flows)); } /* Looks up the logical port with the name 'port_name' in 'br_int_'. If diff --git a/controller/ofctrl.h b/controller/ofctrl.h index 88769566a..ead8088c5 100644 --- a/controller/ofctrl.h +++ b/controller/ofctrl.h @@ -52,11 +52,13 @@ void ofctrl_init(struct ovn_extend_table *group_table, void ofctrl_run(const struct ovsrec_bridge *br_int, struct shash *pending_ct_zones); enum mf_field_id ofctrl_get_mf_field_id(void); -void ofctrl_put(struct ovn_desired_flow_table *, +void ofctrl_put(struct ovn_desired_flow_table *lflow_table, + struct ovn_desired_flow_table *pflow_table, struct shash *pending_ct_zones, const struct sbrec_meter_table *, uint64_t nb_cfg, - bool flow_changed); + bool lflow_changed, + bool pflow_changed); bool ofctrl_can_put(void); void ofctrl_wait(void); void ofctrl_destroy(void); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index d48ddc7a2..5ea3035fb 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -563,7 +563,7 @@ add_pending_ct_zone_entry(struct shash *pending_ct_zones, static void update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, struct simap *ct_zones, unsigned long *ct_zone_bitmap, - struct shash *pending_ct_zones, struct hmapx *updated_dps) + struct shash *pending_ct_zones) { struct simap_node *ct_zone, *ct_zone_next; int scan_start = 1; @@ -653,11 +653,6 @@ update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, bitmap_set1(ct_zone_bitmap, snat_req_node->data); simap_put(ct_zones, snat_req_node->name, snat_req_node->data); - struct shash_node *ld_node = shash_find(&all_lds, snat_req_node->name); - if (ld_node) { - struct local_datapath *dp = ld_node->data; - hmapx_add(updated_dps, (void *) dp->datapath); - } } /* xxx This is wasteful to assign a zone to each port--even if no @@ -686,12 +681,6 @@ update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, bitmap_set1(ct_zone_bitmap, zone); simap_put(ct_zones, user, zone); - - struct shash_node *ld_node = shash_find(&all_lds, user); - if (ld_node) { - struct local_datapath *dp = ld_node->data; - hmapx_add(updated_dps, (void *) dp->datapath); - } } simap_destroy(&req_snat_zones); @@ -983,9 +972,6 @@ struct ed_type_runtime_data { bool tracked; bool local_lports_changed; struct hmap tracked_dp_bindings; - - /* CT zone data. Contains datapaths that had updated CT zones */ - struct hmapx ct_updated_datapaths; }; /* struct ed_type_runtime_data has the below members for tracking the @@ -1077,8 +1063,6 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, /* Init the tracked data. */ hmap_init(&data->tracked_dp_bindings); - hmapx_init(&data->ct_updated_datapaths); - return data; } @@ -1101,7 +1085,6 @@ en_runtime_data_cleanup(void *data) } hmap_destroy(&rt_data->local_datapaths); local_binding_data_destroy(&rt_data->lbinding_data); - hmapx_destroy(&rt_data->ct_updated_datapaths); } static void @@ -1224,7 +1207,6 @@ en_runtime_data_run(struct engine_node *node, void *data) sset_init(&rt_data->egress_ifaces); smap_init(&rt_data->local_iface_ids); local_binding_data_init(&rt_data->lbinding_data); - hmapx_clear(&rt_data->ct_updated_datapaths); } struct binding_ctx_in b_ctx_in; @@ -1744,10 +1726,9 @@ en_ct_zones_run(struct engine_node *node, void *data) struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); - hmapx_clear(&rt_data->ct_updated_datapaths); update_ct_zones(&rt_data->local_lports, &rt_data->local_datapaths, &ct_zones_data->current, ct_zones_data->bitmap, - &ct_zones_data->pending, &rt_data->ct_updated_datapaths); + &ct_zones_data->pending); engine_set_node_state(node, EN_UPDATED); @@ -1790,107 +1771,13 @@ en_mff_ovn_geneve_run(struct engine_node *node, void *data) engine_set_node_state(node, EN_UNCHANGED); } -/* Engine node en_physical_flow_changes indicates whether - * there is a need to - * - recompute only physical flows or - * - we can incrementally process the physical flows. - * - * en_physical_flow_changes is an input to flow_output engine node. - * If the engine node 'en_physical_flow_changes' gets updated during - * engine run, it means the handler for this - - * flow_output_physical_flow_changes_handler() will either - * - recompute the physical flows by calling 'physical_run() or - * - incrementlly process some of the changes for physical flow - * calculation. Right now we handle OVS interfaces changes - * for physical flow computation. - * - * When ever a port binding happens, the follow up - * activity is the zone id allocation for that port binding. - * With this intermediate engine node, we avoid full recomputation. - * Instead we do physical flow computation (either full recomputation - * by calling physical_run() or handling the changes incrementally. - * - * Hence this is an intermediate engine node to indicate the - * flow_output engine to recomputes/compute the physical flows. - * - * TODO 1. Ideally this engine node should recompute/compute the physical - * flows instead of relegating it to the flow_output node. - * But this requires splitting the flow_output node to - * logical_flow_output and physical_flow_output. - * - * TODO 2. We can further optimise the en_ct_zone changes to - * compute the phsyical flows for changed zone ids. - * - * TODO 3: physical.c has a global simap -localvif_to_ofport which stores the - * local OVS interfaces and the ofport numbers. Ideally this should be - * part of the engine data. - */ -struct ed_type_pfc_data { - /* Both these variables are tracked and set in each engine run. */ - bool recompute_physical_flows; - bool ovs_ifaces_changed; -}; - -static void -en_physical_flow_changes_clear_tracked_data(void *data_) -{ - struct ed_type_pfc_data *data = data_; - data->recompute_physical_flows = false; - data->ovs_ifaces_changed = false; -} - -static void * -en_physical_flow_changes_init(struct engine_node *node OVS_UNUSED, - struct engine_arg *arg OVS_UNUSED) -{ - struct ed_type_pfc_data *data = xzalloc(sizeof *data); - return data; -} - -static void -en_physical_flow_changes_cleanup(void *data OVS_UNUSED) -{ -} - -/* Indicate to the flow_output engine that we need to recompute physical - * flows. */ -static void -en_physical_flow_changes_run(struct engine_node *node, void *data) -{ - struct ed_type_pfc_data *pfc_tdata = data; - pfc_tdata->recompute_physical_flows = true; - pfc_tdata->ovs_ifaces_changed = true; - engine_set_node_state(node, EN_UPDATED); -} - -/* ct_zone changes are not handled incrementally but a handler is required - * to avoid skipping the ovs_iface incremental change handler. - */ -static bool -physical_flow_changes_ct_zones_handler(struct engine_node *node OVS_UNUSED, - void *data OVS_UNUSED) -{ - return false; -} - -/* There are OVS interface changes. Indicate to the flow_output engine - * to handle these OVS interface changes for physical flow computations. */ -static bool -physical_flow_changes_ovs_iface_handler(struct engine_node *node, void *data) -{ - struct ed_type_pfc_data *pfc_tdata = data; - pfc_tdata->ovs_ifaces_changed = true; - engine_set_node_state(node, EN_UPDATED); - return true; -} - -struct flow_output_persistent_data { +struct lflow_output_persistent_data { uint32_t conj_id_ofs; struct lflow_cache *lflow_cache; }; -struct ed_type_flow_output { - /* desired flows */ +struct ed_type_lflow_output { + /* Logical flow table */ struct ovn_desired_flow_table flow_table; /* group ids for load balancing */ struct ovn_extend_table group_table; @@ -1901,81 +1788,15 @@ struct ed_type_flow_output { /* Data which is persistent and not cleared during * full recompute. */ - struct flow_output_persistent_data pd; + struct lflow_output_persistent_data pd; }; -static void init_physical_ctx(struct engine_node *node, - struct ed_type_runtime_data *rt_data, - struct physical_ctx *p_ctx) -{ - struct ovsdb_idl_index *sbrec_port_binding_by_name = - engine_ovsdb_node_get_index( - engine_get_input("SB_port_binding", node), - "name"); - - struct sbrec_multicast_group_table *multicast_group_table = - (struct sbrec_multicast_group_table *)EN_OVSDB_GET( - engine_get_input("SB_multicast_group", node)); - - struct sbrec_port_binding_table *port_binding_table = - (struct sbrec_port_binding_table *)EN_OVSDB_GET( - engine_get_input("SB_port_binding", node)); - - struct sbrec_chassis_table *chassis_table = - (struct sbrec_chassis_table *)EN_OVSDB_GET( - engine_get_input("SB_chassis", node)); - - struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = - engine_get_input_data("mff_ovn_geneve", node); - - struct ovsrec_open_vswitch_table *ovs_table = - (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( - engine_get_input("OVS_open_vswitch", node)); - struct ovsrec_bridge_table *bridge_table = - (struct ovsrec_bridge_table *)EN_OVSDB_GET( - engine_get_input("OVS_bridge", node)); - const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); - const char *chassis_id = get_ovs_chassis_id(ovs_table); - const struct sbrec_chassis *chassis = NULL; - struct ovsdb_idl_index *sbrec_chassis_by_name = - engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), - "name"); - if (chassis_id) { - chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); - } - - ovs_assert(br_int && chassis); - - struct ovsrec_interface_table *iface_table = - (struct ovsrec_interface_table *)EN_OVSDB_GET( - engine_get_input("OVS_interface", node)); - - struct ed_type_ct_zones *ct_zones_data = - engine_get_input_data("ct_zones", node); - struct simap *ct_zones = &ct_zones_data->current; - - p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; - p_ctx->port_binding_table = port_binding_table; - p_ctx->mc_group_table = multicast_group_table; - p_ctx->br_int = br_int; - p_ctx->chassis_table = chassis_table; - p_ctx->iface_table = iface_table; - p_ctx->chassis = chassis; - p_ctx->active_tunnels = &rt_data->active_tunnels; - p_ctx->local_datapaths = &rt_data->local_datapaths; - p_ctx->local_lports = &rt_data->local_lports; - p_ctx->ct_zones = ct_zones; - p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; - p_ctx->local_bindings = &rt_data->lbinding_data.bindings; - p_ctx->ct_updated_datapaths = &rt_data->ct_updated_datapaths; -} - -static void init_lflow_ctx(struct engine_node *node, - struct ed_type_runtime_data *rt_data, - struct ed_type_flow_output *fo, - struct lflow_ctx_in *l_ctx_in, - struct lflow_ctx_out *l_ctx_out) +static void +init_lflow_ctx(struct engine_node *node, + struct ed_type_runtime_data *rt_data, + struct ed_type_lflow_output *fo, + struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out) { struct ovsdb_idl_index *sbrec_port_binding_by_name = engine_ovsdb_node_get_index( @@ -2085,11 +1906,10 @@ static void init_lflow_ctx(struct engine_node *node, } static void * -en_flow_output_init(struct engine_node *node OVS_UNUSED, - struct engine_arg *arg OVS_UNUSED) +en_lflow_output_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) { - struct ed_type_flow_output *data = xzalloc(sizeof *data); - + struct ed_type_lflow_output *data = xzalloc(sizeof *data); ovn_desired_flow_table_init(&data->flow_table); ovn_extend_table_init(&data->group_table); ovn_extend_table_init(&data->meter_table); @@ -2099,9 +1919,9 @@ en_flow_output_init(struct engine_node *node OVS_UNUSED, } static void -en_flow_output_cleanup(void *data) +en_lflow_output_cleanup(void *data) { - struct ed_type_flow_output *flow_output_data = data; + struct ed_type_lflow_output *flow_output_data = data; ovn_desired_flow_table_destroy(&flow_output_data->flow_table); ovn_extend_table_destroy(&flow_output_data->group_table); ovn_extend_table_destroy(&flow_output_data->meter_table); @@ -2110,7 +1930,7 @@ en_flow_output_cleanup(void *data) } static void -en_flow_output_run(struct engine_node *node, void *data) +en_lflow_output_run(struct engine_node *node, void *data) { struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); @@ -2136,8 +1956,8 @@ en_flow_output_run(struct engine_node *node, void *data) ovs_assert(br_int && chassis); - struct ed_type_flow_output *fo = data; - struct ovn_desired_flow_table *flow_table = &fo->flow_table; + struct ed_type_lflow_output *fo = data; + struct ovn_desired_flow_table *lflow_table = &fo->flow_table; struct ovn_extend_table *group_table = &fo->group_table; struct ovn_extend_table *meter_table = &fo->meter_table; struct lflow_resource_ref *lfrr = &fo->lflow_resource_ref; @@ -2146,7 +1966,7 @@ en_flow_output_run(struct engine_node *node, void *data) if (first_run) { first_run = false; } else { - ovn_desired_flow_table_clear(flow_table); + ovn_desired_flow_table_clear(lflow_table); ovn_extend_table_clear(group_table, false /* desired */); ovn_extend_table_clear(meter_table, false /* desired */); lflow_resource_clear(lfrr); @@ -2168,7 +1988,7 @@ en_flow_output_run(struct engine_node *node, void *data) if (l_ctx_out.conj_id_overflow) { /* Conjunction ids overflow. There can be many holes in between. * Destroy lflow cache and call lflow_run() again. */ - ovn_desired_flow_table_clear(flow_table); + ovn_desired_flow_table_clear(lflow_table); ovn_extend_table_clear(group_table, false /* desired */); ovn_extend_table_clear(meter_table, false /* desired */); lflow_resource_clear(lfrr); @@ -2181,16 +2001,11 @@ en_flow_output_run(struct engine_node *node, void *data) } } - struct physical_ctx p_ctx; - init_physical_ctx(node, rt_data, &p_ctx); - - physical_run(&p_ctx, &fo->flow_table); - engine_set_node_state(node, EN_UPDATED); } static bool -flow_output_sb_logical_flow_handler(struct engine_node *node, void *data) +lflow_output_sb_logical_flow_handler(struct engine_node *node, void *data) { struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); @@ -2203,7 +2018,7 @@ flow_output_sb_logical_flow_handler(struct engine_node *node, void *data) const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); ovs_assert(br_int); - struct ed_type_flow_output *fo = data; + struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); @@ -2215,7 +2030,7 @@ flow_output_sb_logical_flow_handler(struct engine_node *node, void *data) } static bool -flow_output_sb_mac_binding_handler(struct engine_node *node, void *data) +lflow_output_sb_mac_binding_handler(struct engine_node *node, void *data) { struct ovsdb_idl_index *sbrec_port_binding_by_name = engine_ovsdb_node_get_index( @@ -2230,60 +2045,17 @@ flow_output_sb_mac_binding_handler(struct engine_node *node, void *data) engine_get_input_data("runtime_data", node); const struct hmap *local_datapaths = &rt_data->local_datapaths; - struct ed_type_flow_output *fo = data; - struct ovn_desired_flow_table *flow_table = &fo->flow_table; + struct ed_type_lflow_output *lfo = data; lflow_handle_changed_neighbors(sbrec_port_binding_by_name, - mac_binding_table, local_datapaths, flow_table); + mac_binding_table, local_datapaths, &lfo->flow_table); engine_set_node_state(node, EN_UPDATED); return true; } static bool -flow_output_sb_port_binding_handler(struct engine_node *node, - void *data) -{ - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - - struct ed_type_flow_output *fo = data; - struct ovn_desired_flow_table *flow_table = &fo->flow_table; - - struct physical_ctx p_ctx; - init_physical_ctx(node, rt_data, &p_ctx); - - /* We handle port-binding changes for physical flow processing - * only. flow_output runtime data handler takes care of processing - * logical flows for any port binding changes. - */ - physical_handle_port_binding_changes(&p_ctx, flow_table); - - engine_set_node_state(node, EN_UPDATED); - return true; -} - -static bool -flow_output_sb_multicast_group_handler(struct engine_node *node, void *data) -{ - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - - struct ed_type_flow_output *fo = data; - struct ovn_desired_flow_table *flow_table = &fo->flow_table; - - struct physical_ctx p_ctx; - init_physical_ctx(node, rt_data, &p_ctx); - - physical_handle_mc_group_changes(&p_ctx, flow_table); - - engine_set_node_state(node, EN_UPDATED); - return true; - -} - -static bool -_flow_output_resource_ref_handler(struct engine_node *node, void *data, +_lflow_output_resource_ref_handler(struct engine_node *node, void *data, enum ref_type ref_type) { struct ed_type_runtime_data *rt_data = @@ -2315,7 +2087,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, ovs_assert(br_int && chassis); - struct ed_type_flow_output *fo = data; + struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; @@ -2384,53 +2156,20 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, } static bool -flow_output_addr_sets_handler(struct engine_node *node, void *data) +lflow_output_addr_sets_handler(struct engine_node *node, void *data) { - return _flow_output_resource_ref_handler(node, data, REF_TYPE_ADDRSET); + return _lflow_output_resource_ref_handler(node, data, REF_TYPE_ADDRSET); } static bool -flow_output_port_groups_handler(struct engine_node *node, void *data) +lflow_output_port_groups_handler(struct engine_node *node, void *data) { - return _flow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP); -} - -static bool -flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) -{ - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - - struct ed_type_flow_output *fo = data; - struct physical_ctx p_ctx; - init_physical_ctx(node, rt_data, &p_ctx); - - engine_set_node_state(node, EN_UPDATED); - struct ed_type_pfc_data *pfc_data = - engine_get_input_data("physical_flow_changes", node); - - /* If there are OVS interface changes. Try to handle them incrementally. */ - if (pfc_data->ovs_ifaces_changed) { - if (!physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table)) { - return false; - } - } - - if (pfc_data->recompute_physical_flows) { - /* This indicates that we need to recompute the physical flows. */ - physical_clear_unassoc_flows_with_db(&fo->flow_table); - physical_clear_dp_flows(&p_ctx, &rt_data->ct_updated_datapaths, - &fo->flow_table); - physical_run(&p_ctx, &fo->flow_table); - return true; - } - - return true; + return _lflow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP); } static bool -flow_output_runtime_data_handler(struct engine_node *node, - void *data OVS_UNUSED) +lflow_output_runtime_data_handler(struct engine_node *node, + void *data OVS_UNUSED) { struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); @@ -2451,12 +2190,9 @@ flow_output_runtime_data_handler(struct engine_node *node, struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - struct ed_type_flow_output *fo = data; + struct ed_type_lflow_output *fo = data; init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); - struct physical_ctx p_ctx; - init_physical_ctx(node, rt_data, &p_ctx); - struct tracked_binding_datapath *tdp; HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { if (tdp->is_new) { @@ -2481,12 +2217,12 @@ flow_output_runtime_data_handler(struct engine_node *node, } static bool -flow_output_sb_load_balancer_handler(struct engine_node *node, void *data) +lflow_output_sb_load_balancer_handler(struct engine_node *node, void *data) { struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); - struct ed_type_flow_output *fo = data; + struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); @@ -2498,12 +2234,12 @@ flow_output_sb_load_balancer_handler(struct engine_node *node, void *data) } static bool -flow_output_sb_fdb_handler(struct engine_node *node, void *data) +lflow_output_sb_fdb_handler(struct engine_node *node, void *data) { struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); - struct ed_type_flow_output *fo = data; + struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); @@ -2514,6 +2250,205 @@ flow_output_sb_fdb_handler(struct engine_node *node, void *data) return handled; } +struct ed_type_pflow_output { + /* Desired physical flows. */ + struct ovn_desired_flow_table flow_table; +}; + +static void init_physical_ctx(struct engine_node *node, + struct ed_type_runtime_data *rt_data, + struct physical_ctx *p_ctx) +{ + struct ovsdb_idl_index *sbrec_port_binding_by_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_port_binding", node), + "name"); + + struct sbrec_multicast_group_table *multicast_group_table = + (struct sbrec_multicast_group_table *)EN_OVSDB_GET( + engine_get_input("SB_multicast_group", node)); + + struct sbrec_port_binding_table *port_binding_table = + (struct sbrec_port_binding_table *)EN_OVSDB_GET( + engine_get_input("SB_port_binding", node)); + + struct sbrec_chassis_table *chassis_table = + (struct sbrec_chassis_table *)EN_OVSDB_GET( + engine_get_input("SB_chassis", node)); + + struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = + engine_get_input_data("mff_ovn_geneve", node); + + struct ovsrec_open_vswitch_table *ovs_table = + (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( + engine_get_input("OVS_open_vswitch", node)); + struct ovsrec_bridge_table *bridge_table = + (struct ovsrec_bridge_table *)EN_OVSDB_GET( + engine_get_input("OVS_bridge", node)); + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); + const char *chassis_id = get_ovs_chassis_id(ovs_table); + const struct sbrec_chassis *chassis = NULL; + struct ovsdb_idl_index *sbrec_chassis_by_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_chassis", node), + "name"); + if (chassis_id) { + chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); + } + + ovs_assert(br_int && chassis); + + struct ovsrec_interface_table *iface_table = + (struct ovsrec_interface_table *)EN_OVSDB_GET( + engine_get_input("OVS_interface", node)); + + struct ed_type_ct_zones *ct_zones_data = + engine_get_input_data("ct_zones", node); + struct simap *ct_zones = &ct_zones_data->current; + + p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; + p_ctx->port_binding_table = port_binding_table; + p_ctx->mc_group_table = multicast_group_table; + p_ctx->br_int = br_int; + p_ctx->chassis_table = chassis_table; + p_ctx->iface_table = iface_table; + p_ctx->chassis = chassis; + p_ctx->active_tunnels = &rt_data->active_tunnels; + p_ctx->local_datapaths = &rt_data->local_datapaths; + p_ctx->local_lports = &rt_data->local_lports; + p_ctx->ct_zones = ct_zones; + p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; + p_ctx->local_bindings = &rt_data->lbinding_data.bindings; +} + +static void * +en_pflow_output_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_pflow_output *data = xzalloc(sizeof *data); + ovn_desired_flow_table_init(&data->flow_table); + return data; +} + +static void +en_pflow_output_cleanup(void *data OVS_UNUSED) +{ + struct ed_type_pflow_output *pfo = data; + ovn_desired_flow_table_destroy(&pfo->flow_table); +} + +static void +en_pflow_output_run(struct engine_node *node, void *data) +{ + struct ed_type_pflow_output *pfo = data; + struct ovn_desired_flow_table *pflow_table = &pfo->flow_table; + static bool first_run = true; + if (first_run) { + first_run = false; + } else { + ovn_desired_flow_table_clear(pflow_table); + } + + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + physical_run(&p_ctx, pflow_table); + + engine_set_node_state(node, EN_UPDATED); +} + +static bool +pflow_output_sb_port_binding_handler(struct engine_node *node, + void *data) +{ + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct ed_type_pflow_output *pfo = data; + + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + + /* We handle port-binding changes for physical flow processing + * only. flow_output runtime data handler takes care of processing + * logical flows for any port binding changes. + */ + physical_handle_port_binding_changes(&p_ctx, &pfo->flow_table); + + engine_set_node_state(node, EN_UPDATED); + return true; +} + +static bool +pflow_output_sb_multicast_group_handler(struct engine_node *node, void *data) +{ + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct ed_type_pflow_output *pfo = data; + + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + + physical_handle_mc_group_changes(&p_ctx, &pfo->flow_table); + + engine_set_node_state(node, EN_UPDATED); + return true; +} + +static bool +pflow_output_ovs_iface_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct ed_type_pflow_output *pfo = data; + + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + + engine_set_node_state(node, EN_UPDATED); + return physical_handle_ovs_iface_changes(&p_ctx, &pfo->flow_table); +} + +static void * +en_flow_output_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + return NULL; +} + +static void +en_flow_output_cleanup(void *data OVS_UNUSED) +{ + +} + +static void +en_flow_output_run(struct engine_node *node OVS_UNUSED, void *data OVS_UNUSED) +{ + engine_set_node_state(node, EN_UPDATED); +} + +static bool +flow_output_pflow_output_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + engine_set_node_state(node, EN_UPDATED); + return true; +} + +static bool +flow_output_lflow_output_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + engine_set_node_state(node, EN_UPDATED); + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -2706,8 +2641,8 @@ main(int argc, char *argv[]) ENGINE_NODE_WITH_CLEAR_TRACK_DATA(runtime_data, "runtime_data"); ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); - ENGINE_NODE_WITH_CLEAR_TRACK_DATA(physical_flow_changes, - "physical_flow_changes"); + ENGINE_NODE(pflow_output, "physical_flow_output"); + ENGINE_NODE(lflow_output, "logical_flow_output"); ENGINE_NODE(flow_output, "flow_output"); ENGINE_NODE(addr_sets, "addr_sets"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups"); @@ -2731,58 +2666,68 @@ main(int argc, char *argv[]) engine_add_input(&en_port_groups, &en_runtime_data, port_groups_runtime_data_handler); - /* Engine node physical_flow_changes indicates whether - * we can recompute only physical flows or we can - * incrementally process the physical flows. - * - * Note: The order of inputs is important, all OVS interface changes must + /* Note: The order of inputs is important, all OVS interface changes must * be handled before any ct_zone changes. */ - engine_add_input(&en_physical_flow_changes, &en_ovs_interface, - physical_flow_changes_ovs_iface_handler); - engine_add_input(&en_physical_flow_changes, &en_ct_zones, - physical_flow_changes_ct_zones_handler); - - engine_add_input(&en_flow_output, &en_addr_sets, - flow_output_addr_sets_handler); - engine_add_input(&en_flow_output, &en_port_groups, - flow_output_port_groups_handler); - engine_add_input(&en_flow_output, &en_runtime_data, - flow_output_runtime_data_handler); - engine_add_input(&en_flow_output, &en_mff_ovn_geneve, NULL); - engine_add_input(&en_flow_output, &en_physical_flow_changes, - flow_output_physical_flow_changes_handler); - - /* We need this input nodes for only data. Hence the noop handler. */ - engine_add_input(&en_flow_output, &en_ct_zones, engine_noop_handler); - engine_add_input(&en_flow_output, &en_ovs_interface, engine_noop_handler); - - engine_add_input(&en_flow_output, &en_ovs_open_vswitch, NULL); - engine_add_input(&en_flow_output, &en_ovs_bridge, NULL); - - engine_add_input(&en_flow_output, &en_sb_chassis, NULL); - engine_add_input(&en_flow_output, &en_sb_encap, NULL); - engine_add_input(&en_flow_output, &en_sb_multicast_group, - flow_output_sb_multicast_group_handler); - engine_add_input(&en_flow_output, &en_sb_port_binding, - flow_output_sb_port_binding_handler); - engine_add_input(&en_flow_output, &en_sb_mac_binding, - flow_output_sb_mac_binding_handler); - engine_add_input(&en_flow_output, &en_sb_logical_flow, - flow_output_sb_logical_flow_handler); + engine_add_input(&en_pflow_output, &en_ovs_interface, + pflow_output_ovs_iface_handler); + engine_add_input(&en_pflow_output, &en_ct_zones, NULL); + engine_add_input(&en_pflow_output, &en_sb_chassis, NULL); + engine_add_input(&en_pflow_output, &en_sb_port_binding, + pflow_output_sb_port_binding_handler); + engine_add_input(&en_pflow_output, &en_sb_multicast_group, + pflow_output_sb_multicast_group_handler); + + engine_add_input(&en_pflow_output, &en_runtime_data, + NULL); + engine_add_input(&en_pflow_output, &en_sb_encap, NULL); + engine_add_input(&en_pflow_output, &en_mff_ovn_geneve, NULL); + engine_add_input(&en_pflow_output, &en_ovs_open_vswitch, NULL); + engine_add_input(&en_pflow_output, &en_ovs_bridge, NULL); + + engine_add_input(&en_lflow_output, &en_addr_sets, + lflow_output_addr_sets_handler); + engine_add_input(&en_lflow_output, &en_port_groups, + lflow_output_port_groups_handler); + engine_add_input(&en_lflow_output, &en_runtime_data, + lflow_output_runtime_data_handler); + + /* We need these input nodes only for the data. Hence the noop handler. + * Changes to en_sb_multicast_group is handled by the pflow_output engine + * node. + * */ + engine_add_input(&en_lflow_output, &en_sb_multicast_group, + engine_noop_handler); + + engine_add_input(&en_lflow_output, &en_sb_chassis, NULL); + + /* Any changes to the port binding, need not be handled + * for lflow_outout engine. We still need sb_port_binding + * as input to access the port binding data in lflow.c and + * hence the noop handler. */ + engine_add_input(&en_lflow_output, &en_sb_port_binding, + engine_noop_handler); + + engine_add_input(&en_lflow_output, &en_ovs_open_vswitch, NULL); + engine_add_input(&en_lflow_output, &en_ovs_bridge, NULL); + + engine_add_input(&en_lflow_output, &en_sb_mac_binding, + lflow_output_sb_mac_binding_handler); + engine_add_input(&en_lflow_output, &en_sb_logical_flow, + lflow_output_sb_logical_flow_handler); /* Using a noop handler since we don't really need any data from datapath * groups or a full recompute. Update of a datapath group will put * logical flow into the tracked list, so the logical flow handler will * process all changes. */ - engine_add_input(&en_flow_output, &en_sb_logical_dp_group, + engine_add_input(&en_lflow_output, &en_sb_logical_dp_group, engine_noop_handler); - engine_add_input(&en_flow_output, &en_sb_dhcp_options, NULL); - engine_add_input(&en_flow_output, &en_sb_dhcpv6_options, NULL); - engine_add_input(&en_flow_output, &en_sb_dns, NULL); - engine_add_input(&en_flow_output, &en_sb_load_balancer, - flow_output_sb_load_balancer_handler); - engine_add_input(&en_flow_output, &en_sb_fdb, - flow_output_sb_fdb_handler); + engine_add_input(&en_lflow_output, &en_sb_dhcp_options, NULL); + engine_add_input(&en_lflow_output, &en_sb_dhcpv6_options, NULL); + engine_add_input(&en_lflow_output, &en_sb_dns, NULL); + engine_add_input(&en_lflow_output, &en_sb_load_balancer, + lflow_output_sb_load_balancer_handler); + engine_add_input(&en_lflow_output, &en_sb_fdb, + lflow_output_sb_fdb_handler); engine_add_input(&en_ct_zones, &en_ovs_open_vswitch, NULL); engine_add_input(&en_ct_zones, &en_ovs_bridge, NULL); @@ -2804,12 +2749,20 @@ main(int argc, char *argv[]) /* The OVS interface handler for runtime_data changes MUST be executed * after the sb_port_binding_handler as port_binding deletes must be * processed first. + * + * runtime_data needs to access the OVS Port data and hence a noop + * handler. */ engine_add_input(&en_runtime_data, &en_ovs_port, engine_noop_handler); engine_add_input(&en_runtime_data, &en_ovs_interface, runtime_data_ovs_interface_handler); + engine_add_input(&en_flow_output, &en_lflow_output, + flow_output_lflow_output_handler); + engine_add_input(&en_flow_output, &en_pflow_output, + flow_output_pflow_output_handler); + struct engine_arg engine_arg = { .sb_idl = ovnsb_idl_loop.idl, .ovs_idl = ovs_idl_loop.idl, @@ -2832,25 +2785,27 @@ main(int argc, char *argv[]) engine_ovsdb_node_add_index(&en_sb_datapath_binding, "key", sbrec_datapath_binding_by_key); - struct ed_type_flow_output *flow_output_data = - engine_get_internal_data(&en_flow_output); + struct ed_type_lflow_output *lflow_output_data = + engine_get_internal_data(&en_lflow_output); + struct ed_type_lflow_output *pflow_output_data = + engine_get_internal_data(&en_pflow_output); struct ed_type_ct_zones *ct_zones_data = engine_get_internal_data(&en_ct_zones); struct ed_type_runtime_data *runtime_data = engine_get_internal_data(&en_runtime_data); - ofctrl_init(&flow_output_data->group_table, - &flow_output_data->meter_table, + ofctrl_init(&lflow_output_data->group_table, + &lflow_output_data->meter_table, get_ofctrl_probe_interval(ovs_idl_loop.idl)); ofctrl_seqno_init(); unixctl_command_register("group-table-list", "", 0, 0, extend_table_list, - &flow_output_data->group_table); + &lflow_output_data->group_table); unixctl_command_register("meter-table-list", "", 0, 0, extend_table_list, - &flow_output_data->meter_table); + &lflow_output_data->meter_table); unixctl_command_register("ct-zone-list", "", 0, 0, ct_zone_list, @@ -2864,14 +2819,14 @@ main(int argc, char *argv[]) NULL); unixctl_command_register("lflow-cache/flush", "", 0, 0, lflow_cache_flush_cmd, - &flow_output_data->pd); + &lflow_output_data->pd); /* Keep deprecated 'flush-lflow-cache' command for now. */ unixctl_command_register("flush-lflow-cache", "[deprecated]", 0, 0, lflow_cache_flush_cmd, - &flow_output_data->pd); + &lflow_output_data->pd); unixctl_command_register("lflow-cache/show-stats", "", 0, 0, lflow_cache_show_stats_cmd, - &flow_output_data->pd); + &lflow_output_data->pd); bool reset_ovnsb_idl_min_index = false; unixctl_command_register("sb-cluster-state-reset", "", 0, 0, @@ -3117,13 +3072,17 @@ main(int argc, char *argv[]) runtime_data ? &runtime_data->lbinding_data : NULL; if_status_mgr_update(if_mgr, binding_data); - flow_output_data = engine_get_data(&en_flow_output); - if (flow_output_data && ct_zones_data) { - ofctrl_put(&flow_output_data->flow_table, + lflow_output_data = engine_get_data(&en_lflow_output); + pflow_output_data = engine_get_data(&en_pflow_output); + if (lflow_output_data && pflow_output_data && + ct_zones_data) { + ofctrl_put(&lflow_output_data->flow_table, + &pflow_output_data->flow_table, &ct_zones_data->pending, sbrec_meter_table_get(ovnsb_idl_loop.idl), ofctrl_seqno_get_req_cfg(), - engine_node_changed(&en_flow_output)); + engine_node_changed(&en_lflow_output), + engine_node_changed(&en_pflow_output)); } ofctrl_seqno_run(ofctrl_get_cur_cfg()); if_status_mgr_run(if_mgr, binding_data, !ovnsb_idl_txn, @@ -3491,7 +3450,7 @@ lflow_cache_flush_cmd(struct unixctl_conn *conn OVS_UNUSED, void *arg_) { VLOG_INFO("User triggered lflow cache flush."); - struct flow_output_persistent_data *fo_pd = arg_; + struct lflow_output_persistent_data *fo_pd = arg_; lflow_cache_flush(fo_pd->lflow_cache); fo_pd->conj_id_ofs = 1; engine_set_force_recompute(true); @@ -3503,7 +3462,7 @@ static void lflow_cache_show_stats_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *arg_) { - struct flow_output_persistent_data *fo_pd = arg_; + struct lflow_output_persistent_data *fo_pd = arg_; struct lflow_cache *lc = fo_pd->lflow_cache; struct ds ds = DS_EMPTY_INITIALIZER; diff --git a/controller/physical.c b/controller/physical.c index 018e09540..04259d44a 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1953,22 +1953,3 @@ physical_clear_unassoc_flows_with_db(struct ovn_desired_flow_table *flow_table) ofctrl_remove_flows(flow_table, hc_uuid); } } - -void -physical_clear_dp_flows(struct physical_ctx *p_ctx, - struct hmapx *ct_updated_datapaths, - struct ovn_desired_flow_table *flow_table) -{ - const struct sbrec_port_binding *binding; - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, p_ctx->port_binding_table) { - if (!hmapx_find(ct_updated_datapaths, binding->datapath)) { - continue; - } - const struct sbrec_port_binding *peer = - get_binding_peer(p_ctx->sbrec_port_binding_by_name, binding); - ofctrl_remove_flows(flow_table, &binding->header_.uuid); - if (peer) { - ofctrl_remove_flows(flow_table, &peer->header_.uuid); - } - } -} diff --git a/controller/physical.h b/controller/physical.h index 0bf13f268..feab41df4 100644 --- a/controller/physical.h +++ b/controller/physical.h @@ -56,16 +56,12 @@ struct physical_ctx { const struct simap *ct_zones; enum mf_field_id mff_ovn_geneve; struct shash *local_bindings; - struct hmapx *ct_updated_datapaths; }; void physical_register_ovs_idl(struct ovsdb_idl *); void physical_run(struct physical_ctx *, struct ovn_desired_flow_table *); void physical_clear_unassoc_flows_with_db(struct ovn_desired_flow_table *); -void physical_clear_dp_flows(struct physical_ctx *p_ctx, - struct hmapx *ct_updated_datapaths, - struct ovn_desired_flow_table *flow_table); void physical_handle_port_binding_changes(struct physical_ctx *, struct ovn_desired_flow_table *); void physical_handle_mc_group_changes(struct physical_ctx *, From patchwork Thu Jun 3 12:29:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1487211 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FwlZR2txLz9sxS for ; Thu, 3 Jun 2021 22:29:19 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id EB65A40517; Thu, 3 Jun 2021 12:29:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tfkIdO6JIZME; Thu, 3 Jun 2021 12:29:13 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTP id 6923B400CA; Thu, 3 Jun 2021 12:29:12 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3EB9CC000F; Thu, 3 Jun 2021 12:29:12 +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 E2095C0001 for ; Thu, 3 Jun 2021 12:29:10 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id C4978606A5 for ; Thu, 3 Jun 2021 12:29:10 +0000 (UTC) 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 7BITVvebXcX6 for ; Thu, 3 Jun 2021 12:29:10 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by smtp3.osuosl.org (Postfix) with ESMTPS id E6D9360679 for ; Thu, 3 Jun 2021 12:29:09 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 81D5EC0009; Thu, 3 Jun 2021 12:29:07 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 3 Jun 2021 08:29:01 -0400 Message-Id: <20210603122901.2066782-1-numans@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210603122721.2066524-1-numans@ovn.org> References: <20210603122721.2066524-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 2/5] controller: Handle sbrec_chassis changes in lflow and pflow output engines. 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 a sbrec_chassis change handler to both the lflow and plfow output engines. It returns false if a chassis is added or deleted so that a full recompute is triggered. Any updates to the sbrec_chassis table is ignored as there is no need to handle these changes. Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 5ea3035fb..b49786441 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2454,6 +2454,30 @@ struct ovn_controller_exit_args { bool *restart; }; +/* Handles sbrec_chassis changes. + * If a new chassis is added or removed return false, so that + * flows are recomputed. For any updates, there is no need for + * any flow computation. Encap changes will also result in + * sbrec_chassis changes, but we handle encap changes separately. + */ +static bool +pflow_lflow_output_sb_chassis_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + struct sbrec_chassis_table *chassis_table = + (struct sbrec_chassis_table *)EN_OVSDB_GET( + engine_get_input("SB_chassis", node)); + + const struct sbrec_chassis *ch; + SBREC_CHASSIS_TABLE_FOR_EACH_TRACKED (ch, chassis_table) { + if (sbrec_chassis_is_deleted(ch) || sbrec_chassis_is_new(ch)) { + return false; + } + } + + return true; +} + /* Returns false if the northd internal version stored in SB_Global * and ovn-controller internal version don't match. */ @@ -2672,7 +2696,9 @@ main(int argc, char *argv[]) engine_add_input(&en_pflow_output, &en_ovs_interface, pflow_output_ovs_iface_handler); engine_add_input(&en_pflow_output, &en_ct_zones, NULL); - engine_add_input(&en_pflow_output, &en_sb_chassis, NULL); + engine_add_input(&en_pflow_output, &en_sb_chassis, + pflow_lflow_output_sb_chassis_handler); + engine_add_input(&en_pflow_output, &en_sb_port_binding, pflow_output_sb_port_binding_handler); engine_add_input(&en_pflow_output, &en_sb_multicast_group, @@ -2699,7 +2725,8 @@ main(int argc, char *argv[]) engine_add_input(&en_lflow_output, &en_sb_multicast_group, engine_noop_handler); - engine_add_input(&en_lflow_output, &en_sb_chassis, NULL); + engine_add_input(&en_lflow_output, &en_sb_chassis, + pflow_lflow_output_sb_chassis_handler); /* Any changes to the port binding, need not be handled * for lflow_outout engine. We still need sb_port_binding From patchwork Thu Jun 3 12:29:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1487212 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FwlZX203yz9sxS for ; Thu, 3 Jun 2021 22:29:24 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 10C1B405AD; Thu, 3 Jun 2021 12:29:22 +0000 (UTC) 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 py7n1CdjLIjc; Thu, 3 Jun 2021 12:29:18 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTP id 14EFE401C1; Thu, 3 Jun 2021 12:29:17 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E2D38C000D; Thu, 3 Jun 2021 12:29:16 +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 1A6FCC0001 for ; Thu, 3 Jun 2021 12:29:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 7A012607F9 for ; Thu, 3 Jun 2021 12:29:15 +0000 (UTC) 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 IcFfna4z4nBZ for ; Thu, 3 Jun 2021 12:29:14 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by smtp3.osuosl.org (Postfix) with ESMTPS id EFDDA6081D for ; Thu, 3 Jun 2021 12:29:13 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 6E35D240008; Thu, 3 Jun 2021 12:29:10 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 3 Jun 2021 08:29:08 -0400 Message-Id: <20210603122908.2066838-1-numans@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210603122721.2066524-1-numans@ovn.org> References: <20210603122721.2066524-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 3/5] ovn-controller: Handle datapath changes incrementally for ct zone I-P 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 After the commit [1], we do a full recompute of ct zone I-P engine for any datapath binding change. This results in physical_flow() getting called. In a highly scaled environment this can result in costly CPU cycles. This patch addressed this by handling the datapath binding changes incrementally in the ct_zone engine node. ct zone recomputation is required only when a datapath binding is deleted or a logical router datapath binding changes the snat-ct-zone option value. This patch handles this. [1] - f9cab11d5fab("Allow explicit setting of the SNAT zone on a gateway router.") Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 55 ++++++++++++++++++++++++++++++++++++- controller/physical.c | 5 ++++ tests/ovn-performance.at | 26 ++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index b49786441..5b55a45b7 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1734,6 +1734,58 @@ en_ct_zones_run(struct engine_node *node, void *data) engine_set_node_state(node, EN_UPDATED); } +/* Handles datapath binding changes for the ct_zones engine. + * Returns false if the datapath is deleted or if the requested snat + * ct zone doesn't match with the ct_zones data. */ +static bool +ct_zones_datapath_binding_handler(struct engine_node *node, void *data) +{ + struct ed_type_ct_zones *ct_zones_data = data; + const struct sbrec_datapath_binding *dp; + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + struct sbrec_datapath_binding_table *dp_table = + (struct sbrec_datapath_binding_table *)EN_OVSDB_GET( + engine_get_input("SB_datapath_binding", node)); + + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { + if (!get_local_datapath(&rt_data->local_datapaths, + dp->tunnel_key)) { + continue; + } + + if (sbrec_datapath_binding_is_deleted(dp)) { + /* Fall back to full recompute of ct_zones engine. */ + return false; + } + + int req_snat_zone = datapath_snat_ct_zone(dp); + if (req_snat_zone == -1) { + /* datapath snat ct zone is not set. This condition will also hit + * when CMS clears the snat-ct-zone for the logical router. + * In this case there is no harm in using the previosly specified + * snat ct zone for this datapath. Also it is hard to know + * if this option was cleared or if this option is never set. */ + continue; + } + + /* Check if the requested snat zone has changed for the datapath + * or not. If so, then fall back to full recompute of + * ct_zone engine. */ + char *snat_dp_zone_key = alloc_nat_zone_key(&dp->header_.uuid, "snat"); + struct simap_node *simap_node = simap_find(&ct_zones_data->current, + snat_dp_zone_key); + free(snat_dp_zone_key); + if (!simap_node || simap_node->data != req_snat_zone) { + /* There is no entry yet or the requested snat zone has changed. + * Trigger full recompute of ct_zones engine. */ + return false; + } + } + + return true; +} + /* The data in the ct_zones node is always valid (i.e., no stale pointers). */ static bool en_ct_zones_is_valid(struct engine_node *node OVS_UNUSED) @@ -2758,7 +2810,8 @@ main(int argc, char *argv[]) engine_add_input(&en_ct_zones, &en_ovs_open_vswitch, NULL); engine_add_input(&en_ct_zones, &en_ovs_bridge, NULL); - engine_add_input(&en_ct_zones, &en_sb_datapath_binding, NULL); + engine_add_input(&en_ct_zones, &en_sb_datapath_binding, + ct_zones_datapath_binding_handler); engine_add_input(&en_ct_zones, &en_runtime_data, NULL); engine_add_input(&en_runtime_data, &en_ofctrl_is_connected, NULL); diff --git a/controller/physical.c b/controller/physical.c index 04259d44a..e70efc71d 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -15,6 +15,7 @@ #include #include "binding.h" +#include "coverage.h" #include "byte-order.h" #include "encaps.h" #include "flow.h" @@ -48,6 +49,8 @@ VLOG_DEFINE_THIS_MODULE(physical); +COVERAGE_DEFINE(physical_run); + /* Datapath zone IDs for connection tracking and NAT */ struct zone_ids { int ct; /* MFF_LOG_CT_ZONE. */ @@ -1528,6 +1531,8 @@ void physical_run(struct physical_ctx *p_ctx, struct ovn_desired_flow_table *flow_table) { + COVERAGE_INC(physical_run); + if (!hc_uuid) { hc_uuid = xmalloc(sizeof(struct uuid)); uuid_generate(hc_uuid); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index e510c6cef..61356e16d 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -462,6 +462,32 @@ OVN_CONTROLLER_EXPECT_HIT_COND( [ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public hv3 30 && ovn-nbctl --wait=hv sync] ) +# Test physical_run for logical router and other datapath binding changes. +OVN_CONTROLLER_EXPECT_HIT_COND( + [hv1 hv2 hv3 hv4 hv5], [physical_run], [>0 >0 >0 =0 =0], + [ovn-nbctl --wait=hv set logical_router lr1 options:snat-ct-zone=10 && ovn-nbctl --wait=hv sync] +) + +OVN_CONTROLLER_EXPECT_HIT_COND( + [hv1 hv2 hv3 hv4 hv5], [physical_run], [>0 >0 >0 =0 =0], + [ovn-nbctl --wait=hv set logical_router lr1 options:snat-ct-zone=11 && ovn-nbctl --wait=hv sync] +) + +OVN_CONTROLLER_EXPECT_NO_HIT( + [hv1 hv2 hv3 hv4 hv5], [physical_run], + [ovn-nbctl --wait=hv remove logical_router lr1 options snat-ct-zone && ovn-nbctl --wait=hv sync] +) + +OVN_CONTROLLER_EXPECT_NO_HIT( + [hv1 hv2 hv3 hv4 hv5], [physical_run], + [ovn-sbctl set datapath_binding lr1 external_ids:foo=bar && ovn-nbctl --wait=hv sync] +) + +OVN_CONTROLLER_EXPECT_NO_HIT( + [hv1 hv2 hv3 hv4 hv5], [physical_run], + [ovn-sbctl set datapath_binding ls1 external_ids:foo=bar && ovn-nbctl --wait=hv sync] +) + # After this, BFD should be enabled from hv1 and hv2 to hv3. # So there should be lflow_run hits in hv1, hv2, hv3 and hv4 OVN_CONTROLLER_EXPECT_HIT_COND( From patchwork Thu Jun 3 12:29:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1487214 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FwlZk4FCsz9sxS for ; Thu, 3 Jun 2021 22:29:34 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 5873E60824; Thu, 3 Jun 2021 12:29:32 +0000 (UTC) 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 XSNyLDgXEebC; Thu, 3 Jun 2021 12:29:28 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTP id B737760811; Thu, 3 Jun 2021 12:29:27 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9DCD0C000D; Thu, 3 Jun 2021 12:29:27 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 18D79C000F for ; Thu, 3 Jun 2021 12:29:26 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id BD216400E5 for ; Thu, 3 Jun 2021 12:29:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ITPx3VIvKHsR for ; Thu, 3 Jun 2021 12:29:20 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by smtp2.osuosl.org (Postfix) with ESMTPS id 3604D40529 for ; Thu, 3 Jun 2021 12:29:18 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id DBBE9C0014; Thu, 3 Jun 2021 12:29:16 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 3 Jun 2021 08:29:12 -0400 Message-Id: <20210603122912.2066891-1-numans@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210603122721.2066524-1-numans@ovn.org> References: <20210603122721.2066524-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 4/5] physical: Set the port binding uuid as cookie for flows where relevant. 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 Some of the OF flows added by physical.c in tables LOCAL_OUTPUT, CHECK_LOOPBACK, SAVE_INPORT can store the port binding uuid as flow cookie because these flows matches on port binding key in the MFF_LOG_OUTPORT field. Whereas right now it stores 0 as cookie. This patch stores the port binding uuid as cookie for these flows. When a port binding is deleted, the port binding handler for flow_output engine node flood removes the flows with the port binding uuid and such flows wont get removed. Right now this is not a problem because deleting a port binding also triggers in physical_run() being called and it recomputes all the physical flows. This patch is required for an upcoming patch which do not call physical_run() for port binding deletes. Signed-off-by: Numan Siddique --- controller/physical.c | 44 ++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/controller/physical.c b/controller/physical.c index e70efc71d..17ca5afbb 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -739,20 +739,23 @@ put_replace_router_port_mac_flows(struct ovsdb_idl_index } static void -put_local_common_flows(uint32_t dp_key, uint32_t port_key, - uint32_t parent_port_key, +put_local_common_flows(uint32_t dp_key, + const struct sbrec_port_binding *pb, + const struct sbrec_port_binding *parent_pb, const struct zone_ids *zone_ids, struct ofpbuf *ofpacts_p, struct ovn_desired_flow_table *flow_table) { struct match match; - /* Table 33, priority 100. + uint32_t port_key = pb->tunnel_key; + + /* Table 38, priority 100. * ======================= * * Implements output to local hypervisor. Each flow matches a * logical output port on the local hypervisor, and resubmits to - * table 34. + * table 39. */ match_init_catchall(&match); @@ -774,12 +777,13 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key, } } - /* Resubmit to table 34. */ + /* Resubmit to table 39. */ put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p); - ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0, - &match, ofpacts_p, hc_uuid); + ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, + pb->header_.uuid.parts[0], &match, ofpacts_p, + &pb->header_.uuid); - /* Table 34, Priority 100. + /* Table 39, Priority 100. * ======================= * * Drop packets whose logical inport and outport are the same @@ -791,8 +795,9 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key, 0, MLF_ALLOW_LOOPBACK); match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, port_key); match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, port_key); - ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 100, 0, - &match, ofpacts_p, hc_uuid); + ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 100, + pb->header_.uuid.parts[0], &match, ofpacts_p, + &pb->header_.uuid); /* Table 64, Priority 100. * ======================= @@ -806,7 +811,7 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key, * table 65 for logical-to-physical translation, then restore * the port number. * - * If 'parent_port_key' is set, then the 'port_key' represents a nested + * If 'parent_pb' is not NULL, then the 'pb' represents a nested * container. * * Note:We can set in_port to 0 too. But if recirculation happens @@ -815,7 +820,7 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key, * in_port is 0. * */ - bool nested_container = parent_port_key ? true: false; + bool nested_container = parent_pb ? true: false; match_init_catchall(&match); ofpbuf_clear(ofpacts_p); match_set_metadata(&match, htonll(dp_key)); @@ -829,8 +834,9 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key, put_load(ofp_to_u16(OFPP_NONE), MFF_IN_PORT, 0, 16, ofpacts_p); put_resubmit(OFTABLE_LOG_TO_PHY, ofpacts_p); put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(ofpacts_p)); - ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 100, 0, - &match, ofpacts_p, hc_uuid); + ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 100, + pb->header_.uuid.parts[0], &match, ofpacts_p, + &pb->header_.uuid); if (nested_container) { /* It's a nested container and when the packet from the nested @@ -852,7 +858,8 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key, match_init_catchall(&match); ofpbuf_clear(ofpacts_p); match_set_metadata(&match, htonll(dp_key)); - match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, parent_port_key); + match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, + parent_pb->tunnel_key); match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0, MLF_NESTED_CONTAINER, MLF_NESTED_CONTAINER); @@ -941,7 +948,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, } struct zone_ids binding_zones = get_zone_ids(binding, ct_zones); - put_local_common_flows(dp_key, port_key, 0, &binding_zones, + put_local_common_flows(dp_key, binding, NULL, &binding_zones, ofpacts_p, flow_table); match_init_catchall(&match); @@ -1141,10 +1148,9 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, */ struct zone_ids zone_ids = get_zone_ids(binding, ct_zones); - uint32_t parent_port_key = parent_port ? parent_port->tunnel_key : 0; - /* Pass the parent port tunnel key if the port is a nested + /* Pass the parent port binding if the port is a nested * container. */ - put_local_common_flows(dp_key, port_key, parent_port_key, &zone_ids, + put_local_common_flows(dp_key, binding, parent_port, &zone_ids, ofpacts_p, flow_table); /* Table 0, Priority 150 and 100. From patchwork Thu Jun 3 12:29:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1487215 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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=) 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 RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FwlZp55wWz9sXM for ; Thu, 3 Jun 2021 22:29:38 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id E06E360B5A; Thu, 3 Jun 2021 12:29:35 +0000 (UTC) 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 b2Mo3MgHO7Jz; Thu, 3 Jun 2021 12:29:32 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id 7CA1460B3A; Thu, 3 Jun 2021 12:29:30 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5AA0AC0025; Thu, 3 Jun 2021 12:29:30 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1D667C0026 for ; Thu, 3 Jun 2021 12:29:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 76EC840536 for ; Thu, 3 Jun 2021 12:29:25 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uUvV1CsbcIXy for ; Thu, 3 Jun 2021 12:29:24 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by smtp2.osuosl.org (Postfix) with ESMTPS id EAFCC40538 for ; Thu, 3 Jun 2021 12:29:23 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 98ECD60002; Thu, 3 Jun 2021 12:29:20 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 3 Jun 2021 08:29:17 -0400 Message-Id: <20210603122917.2066944-1-numans@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210603122721.2066524-1-numans@ovn.org> References: <20210603122721.2066524-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 5/5] controller I-P: ct zone runtime data handler. 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 an handler for runtime data changes to the ct_zones engine node. Before this patch, the handler was NULL. With this patch we do a full recompute of ct_zones engine if: 1. runtime_data's local_lports change. 2. If a new datapath is added to the local datapaths. For all other changes of runtime data, there is no need to recompute ct_zones engine node. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1962345 Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 39 ++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 5b55a45b7..4ab820ca9 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1271,8 +1271,10 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) return false; } + rt_data->local_lports_changed = b_ctx_out.local_lports_changed; if (b_ctx_out.local_lport_ids_changed || b_ctx_out.non_vif_ports_changed || + b_ctx_out.local_lports_changed || !hmap_is_empty(b_ctx_out.tracked_dp_bindings)) { engine_set_node_state(node, EN_UPDATED); } @@ -1786,6 +1788,40 @@ ct_zones_datapath_binding_handler(struct engine_node *node, void *data) return true; } +static bool +ct_zones_runtime_data_handler(struct engine_node *node, void *data OVS_UNUSED) +{ + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + /* There is no tracked data. Fall back to full recompute of ct_zones. */ + if (!rt_data->tracked) { + return false; + } + + /* If local_lports have changed then fall back to full recompute. */ + if (rt_data->local_lports_changed) { + return false; + } + + struct hmap *tracked_dp_bindings = &rt_data->tracked_dp_bindings; + struct tracked_binding_datapath *tdp; + HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { + if (tdp->is_new) { + /* A new datapath has been added. Fall back to full recompute. */ + return false; + } + + /* When an lport is claimed or released because of port binding, + * changes we don't have to compute the ct zone entries for these. + * That is because we generate the ct zone entries for each local + * OVS interface which has external_ids:iface-id set. For the local + * OVS interface changes, rt_data->local_ports_changed will be true. */ + } + + return true; +} + /* The data in the ct_zones node is always valid (i.e., no stale pointers). */ static bool en_ct_zones_is_valid(struct engine_node *node OVS_UNUSED) @@ -2812,7 +2848,8 @@ main(int argc, char *argv[]) engine_add_input(&en_ct_zones, &en_ovs_bridge, NULL); engine_add_input(&en_ct_zones, &en_sb_datapath_binding, ct_zones_datapath_binding_handler); - engine_add_input(&en_ct_zones, &en_runtime_data, NULL); + engine_add_input(&en_ct_zones, &en_runtime_data, + ct_zones_runtime_data_handler); engine_add_input(&en_runtime_data, &en_ofctrl_is_connected, NULL);