From patchwork Thu Feb 4 13:24:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436002 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=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=BZVxHvk2; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfS21SP2z9sWP for ; Fri, 5 Feb 2021 00:25:18 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 0D9E32000F; Thu, 4 Feb 2021 13:25:14 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id gNm3g3bvBSgh; Thu, 4 Feb 2021 13:25:11 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id DE79420130; Thu, 4 Feb 2021 13:25:10 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C2B75C07FD; Thu, 4 Feb 2021 13:25:10 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id BC72CC013A for ; Thu, 4 Feb 2021 13:25:09 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id AB5BC85F56 for ; Thu, 4 Feb 2021 13:25:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id zsosLwBiVSU9 for ; Thu, 4 Feb 2021 13:25:07 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 29B5E85EF1 for ; Thu, 4 Feb 2021 13:25:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445105; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1F/D2wT41MFaUgP0t7/Rt9SSNYjfGEE8QVSjdKaC+M4=; b=BZVxHvk2BxSPLThAz5fKZI9TPHHFfkfMxPMfKJHM+EY0iSjSVsgr7DUmOKHGATr+eqQkLp 0dsUVOI3cJoEEQDu2q74STxjFZE/8MV9p9y4IyOktryDv74FoSwsgYRTBtGQJ9f1rDFzrr B/C7Jm/fMZKr28m+fpBh79hMlfew4yo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-495-DT-PKSqlMLqmxwy49S7-Wg-1; Thu, 04 Feb 2021 08:25:04 -0500 X-MC-Unique: DT-PKSqlMLqmxwy49S7-Wg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1ED28C73A1 for ; Thu, 4 Feb 2021 13:25:03 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6EFFF1001901 for ; Thu, 4 Feb 2021 13:25:02 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:24:59 +0100 Message-Id: <20210204132457.9958.72645.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 01/10] lflow: Fix cache update when I-P engine aborts. 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" If the I-P engine aborts in the middle of a run, the 'flow_output' node change handlers or run callback might not be called. As a side effect this may cause that Logical_Flow IDL tracked changes are not processed during the iteration. As a consequence, if a Logical_Flow was removed from the Southound DB, then its associated cache entry (if any) will not be flushed. This has two side effects: 1. Stale entries are kept in the cache until a full recompute or cache flush happens. 2. If a new Logical_Flow is added to the Southbound and it happens to have a UUID that matches one of a stale cache entry then ovn-controller will install incorrect openflows. IDL tracked changes are cleared at every iteration of ovn-controller. Skipping the ovsdb_idl_track_clear() call if the I-P engine aborted is not a valid option for now because it might lead to some of the IDL changes to be processed twice. Instead, lflow_handle_cached_flows() is called now at every iteration of ovn-controller making sure deleted flows are removed from the cache. Also, rename the 'flush-lflow-cache' unixctl command to 'lflow-cache/flush' to better match the style of other command names. Fixes: 2662498bfd13 ("ovn-controller: Persist the conjunction ids allocated for conjuctive matches.") Signed-off-by: Dumitru Ceara --- controller/lflow.c | 30 +++++++++++++++++------------- controller/lflow.h | 4 +++- controller/ovn-controller.c | 20 ++++++++++++++++---- tests/ovn.at | 2 +- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 946c1e0..04ba0d1 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -1425,19 +1425,6 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out) { COVERAGE_INC(lflow_run); - /* when lflow_run is called, it's possible that some of the logical flows - * are deleted. We need to delete the lflow cache for these - * lflows (if present), otherwise, they will not be deleted at all. */ - if (l_ctx_out->lflow_cache_map) { - const struct sbrec_logical_flow *lflow; - SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, - l_ctx_in->logical_flow_table) { - if (sbrec_logical_flow_is_deleted(lflow)) { - lflow_cache_delete(l_ctx_out->lflow_cache_map, lflow); - } - } - } - add_logical_flows(l_ctx_in, l_ctx_out); add_neighbor_flows(l_ctx_in->sbrec_port_binding_by_name, l_ctx_in->mac_binding_table, l_ctx_in->local_datapaths, @@ -1446,6 +1433,23 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out) l_ctx_out->flow_table); } +/* Should be called at every ovn-controller iteration before IDL tracked + * changes are cleared to avoid maintaining cache entries for flows that + * don't exist anymore. + */ +void +lflow_handle_cached_flows(struct hmap *lflow_cache_map, + const struct sbrec_logical_flow_table *flow_table) +{ + const struct sbrec_logical_flow *lflow; + + SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, flow_table) { + if (sbrec_logical_flow_is_deleted(lflow)) { + lflow_cache_delete(lflow_cache_map, lflow); + } + } +} + void lflow_destroy(void) { diff --git a/controller/lflow.h b/controller/lflow.h index ba79cc3..cf4f0e8 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -156,7 +156,9 @@ struct lflow_ctx_out { }; void lflow_init(void); -void lflow_run(struct lflow_ctx_in *, struct lflow_ctx_out *); +void lflow_run(struct lflow_ctx_in *, struct lflow_ctx_out *); +void lflow_handle_cached_flows(struct hmap *lflow_cache, + const struct sbrec_logical_flow_table *); bool lflow_handle_changed_flows(struct lflow_ctx_in *, struct lflow_ctx_out *); bool lflow_handle_changed_ref(enum ref_type, const char *ref_name, struct lflow_ctx_in *, struct lflow_ctx_out *, diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index ef3e0e9..9014674 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -80,7 +80,7 @@ static unixctl_cb_func cluster_state_reset_cmd; static unixctl_cb_func debug_pause_execution; static unixctl_cb_func debug_resume_execution; static unixctl_cb_func debug_status_execution; -static unixctl_cb_func flush_lflow_cache; +static unixctl_cb_func lflow_cache_flush_cmd; static unixctl_cb_func debug_delay_nb_cfg_report; #define DEFAULT_BRIDGE_NAME "br-int" @@ -2666,7 +2666,8 @@ main(int argc, char *argv[]) unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd, NULL); - unixctl_command_register("flush-lflow-cache", "", 0, 0, flush_lflow_cache, + unixctl_command_register("lflow-cache/flush", "", 0, 0, + lflow_cache_flush_cmd, &flow_output_data->pd); bool reset_ovnsb_idl_min_index = false; @@ -2773,6 +2774,16 @@ main(int argc, char *argv[]) if (ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl) && northd_version_match) { + + /* Unconditionally remove all deleted lflows from the lflow + * cache. + */ + if (flow_output_data && flow_output_data->pd.lflow_cache_enabled) { + lflow_handle_cached_flows( + &flow_output_data->pd.lflow_cache_map, + sbrec_logical_flow_table_get(ovnsb_idl_loop.idl)); + } + /* Contains the transport zones that this Chassis belongs to */ struct sset transport_zones = SSET_INITIALIZER(&transport_zones); sset_from_delimited_string(&transport_zones, @@ -3253,8 +3264,9 @@ engine_recompute_cmd(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED, } static void -flush_lflow_cache(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *arg_) +lflow_cache_flush_cmd(struct unixctl_conn *conn OVS_UNUSED, + int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, + void *arg_) { VLOG_INFO("User triggered lflow cache flush."); struct flow_output_persistent_data *fo_pd = arg_; diff --git a/tests/ovn.at b/tests/ovn.at index e814c18..4a75acc 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22212,7 +22212,7 @@ check ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && udp.dst >= wait_conj_id_count 2 "6 1 tcp" "7 1 udp" AS_BOX([Flush lflow cache.]) -as hv1 ovn-appctl -t ovn-controller flush-lflow-cache +as hv1 ovn-appctl -t ovn-controller lflow-cache/flush wait_conj_id_count 2 "2 1" "3 1" AS_BOX([Disable lflow caching.]) From patchwork Thu Feb 4 13:25:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436003 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=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ao55rK7Y; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfSD2M8hz9sXV for ; Fri, 5 Feb 2021 00:25:32 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 7CF5D27384; Thu, 4 Feb 2021 13:25:30 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id nFD75yCXiDcj; Thu, 4 Feb 2021 13:25:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 7F0772044E; Thu, 4 Feb 2021 13:25:19 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 59879C1D9F; Thu, 4 Feb 2021 13:25:19 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 96053C1825 for ; Thu, 4 Feb 2021 13:25:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 7D70C87131 for ; Thu, 4 Feb 2021 13:25:17 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id u0m9LcJaH3VJ for ; Thu, 4 Feb 2021 13:25:17 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by hemlock.osuosl.org (Postfix) with ESMTPS id D52B087123 for ; Thu, 4 Feb 2021 13:25:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445115; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3AmbKCDj8u19LvA+nrW7nSZUlYivOCrRnIzVzomQ4vE=; b=Ao55rK7YBgIFGVu69s/DvdQJx6REGmKBzMjb5BycmrN1umD2OKCUAYnv0kVMvakNzzWeb+ wli+xXjxzwQH7GDz+f4P0KKwhHf+/O+I5y6ZITo6MNOGwZg+D1M5/O6ni7GMBydRV8u9+C FT34nVHXyz7MqIePVxAIJalMBCPRjN0= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-127-lynCqK44Msi0lZuExPCB3w-1; Thu, 04 Feb 2021 08:25:14 -0500 X-MC-Unique: lynCqK44Msi0lZuExPCB3w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2420EC73A0 for ; Thu, 4 Feb 2021 13:25:13 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7B9B35D9C9 for ; Thu, 4 Feb 2021 13:25:12 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:25:10 +0100 Message-Id: <20210204132508.9958.1650.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 02/10] lflow: Refactor convert_match_to_expr() to explicitly consume prereqs. 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" It was (and still is) the responsibility of the caller of convert_match_to_expr() to explicitly free any 'prereqs' expression that was passed as argument if the expression parsing of the 'lflow' match failed. However, convert_match_to_expr() now updates the value of '*prereqs' setting it to NULL in the successful case. This makes it easier for callers of the function because 'expr_destroy(prereqs)' can now be called unconditionally. Signed-off-by: Dumitru Ceara Acked-by: Numan Siddique --- Note: This patch doesn't change the callers of convert_match_to_expr() to take advantage of the new semantic but following patches do. --- controller/lflow.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 04ba0d1..495653f 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -758,11 +758,12 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, /* Converts the match and returns the simplified expr tree. * * The caller should evaluate the conditions and normalize the expr tree. + * If parsing is successful, '*prereqs' is also consumed. */ static struct expr * convert_match_to_expr(const struct sbrec_logical_flow *lflow, const struct sbrec_datapath_binding *dp, - struct expr *prereqs, + struct expr **prereqs, const struct shash *addr_sets, const struct shash *port_groups, struct lflow_resource_ref *lfrr, @@ -795,8 +796,9 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, sset_destroy(&port_groups_ref); if (!error) { - if (prereqs) { - e = expr_combine(EXPR_T_AND, e, prereqs); + if (*prereqs) { + e = expr_combine(EXPR_T_AND, e, *prereqs); + *prereqs = NULL; } e = expr_annotate(e, &symtab, &error); } @@ -854,7 +856,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, .n_tables = LOG_PIPELINE_LEN, .cur_ltable = lflow->table_id, }; - struct expr *prereqs; + struct expr *prereqs = NULL; char *error; error = ovnacts_parse_string(lflow->actions, &pp, &ovnacts, &prereqs); @@ -885,7 +887,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, struct expr *expr = NULL; if (!l_ctx_out->lflow_cache_map) { /* Caching is disabled. */ - expr = convert_match_to_expr(lflow, dp, prereqs, l_ctx_in->addr_sets, + expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, l_ctx_in->port_groups, l_ctx_out->lfrr, NULL); if (!expr) { @@ -949,7 +951,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, bool pg_addr_set_ref = false; if (!expr) { - expr = convert_match_to_expr(lflow, dp, prereqs, l_ctx_in->addr_sets, + expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, l_ctx_in->port_groups, l_ctx_out->lfrr, &pg_addr_set_ref); if (!expr) { From patchwork Thu Feb 4 13:25:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436006 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.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=gwW/GIyx; dkim-atps=neutral Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfT13Vwvz9sWP for ; Fri, 5 Feb 2021 00:26:12 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 29FEB86BC8; Thu, 4 Feb 2021 13:26:11 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hl5bJ2Z8rqS4; Thu, 4 Feb 2021 13:26:03 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id D7F4986BE5; Thu, 4 Feb 2021 13:26:03 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B1017C07FD; Thu, 4 Feb 2021 13:26:03 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id BF5D5C013A for ; Thu, 4 Feb 2021 13:26:02 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id A3BAE2741E for ; Thu, 4 Feb 2021 13:26:02 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fQv6Xve6oZ-e for ; Thu, 4 Feb 2021 13:25:41 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by silver.osuosl.org (Postfix) with ESMTPS id 048EB2750D for ; Thu, 4 Feb 2021 13:25:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445126; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=W9BVItBUoCBb0RrnOuoysBuW1uTnZpVCpYazEZ+cr80=; b=gwW/GIyxw9KTkECZtIJSwqFgKM3EFakjg/ShG3RmAaUTqK5IdzvKhWlMGyf4HCnztJ7YSY US1hYw8vZMmV+EnlJsrxQyUh6jgYHEV0vyR6K26am+6aPOKpr2iq4W1VWQ+qCTC/kLnq9b L7w+Et/h8GCwbLsSBk8GuT5MXpNy1xo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-153-M1W8GI9aMpa8jqdhZk_NyQ-1; Thu, 04 Feb 2021 08:25:24 -0500 X-MC-Unique: M1W8GI9aMpa8jqdhZk_NyQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4BEA4107ACC7 for ; Thu, 4 Feb 2021 13:25:23 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6441860C5F for ; Thu, 4 Feb 2021 13:25:22 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:25:19 +0100 Message-Id: <20210204132518.9958.18774.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 03/10] lflow-cache: Move the lflow cache to its own module. 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" This abstracts the implementation details of the ovn-controller logical flow cache and also has refactors how the cache is being used allowing further commits to expand its functionality. Signed-off-by: Dumitru Ceara Acked-by: Numan Siddique --- controller/automake.mk | 2 controller/lflow-cache.c | 230 +++++++++++++++++++++++++++++ controller/lflow-cache.h | 77 ++++++++++ controller/lflow.c | 336 ++++++++++++++----------------------------- controller/lflow.h | 8 - controller/ovn-controller.c | 57 +++---- lib/expr.c | 4 + 7 files changed, 447 insertions(+), 267 deletions(-) create mode 100644 controller/lflow-cache.c create mode 100644 controller/lflow-cache.h diff --git a/controller/automake.mk b/controller/automake.mk index 480578e..75119a6 100644 --- a/controller/automake.mk +++ b/controller/automake.mk @@ -14,6 +14,8 @@ controller_ovn_controller_SOURCES = \ controller/ip-mcast.h \ controller/lflow.c \ controller/lflow.h \ + controller/lflow-cache.c \ + controller/lflow-cache.h \ controller/lport.c \ controller/lport.h \ controller/ofctrl.c \ diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c new file mode 100644 index 0000000..ce129ac --- /dev/null +++ b/controller/lflow-cache.c @@ -0,0 +1,230 @@ +/* + * Copyright (c) 2015, 2016 Nicira, Inc. + * Copyright (c) 2021, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "lib/ovn-sb-idl.h" +#include "lflow-cache.h" +#include "ovn/expr.h" + +struct lflow_cache { + struct hmap entries[LCACHE_T_MAX]; + bool enabled; +}; + +struct lflow_cache_entry { + struct hmap_node node; + struct uuid lflow_uuid; /* key */ + + struct lflow_cache_value value; +}; + +static struct lflow_cache_value *lflow_cache_add__( + struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow, + enum lflow_cache_type type); +static void lflow_cache_delete__(struct lflow_cache *lc, + struct lflow_cache_entry *lce); + +struct lflow_cache * +lflow_cache_create(void) +{ + struct lflow_cache *lc = xmalloc(sizeof *lc); + + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + hmap_init(&lc->entries[i]); + } + + lc->enabled = true; + return lc; +} + +void +lflow_cache_flush(struct lflow_cache *lc) +{ + if (!lc) { + return; + } + + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + struct lflow_cache_entry *lce; + struct lflow_cache_entry *lce_next; + + HMAP_FOR_EACH_SAFE (lce, lce_next, node, &lc->entries[i]) { + lflow_cache_delete__(lc, lce); + } + } +} + +void +lflow_cache_destroy(struct lflow_cache *lc) +{ + if (!lc) { + return; + } + + lflow_cache_flush(lc); + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + hmap_destroy(&lc->entries[i]); + } + free(lc); +} + +void +lflow_cache_enable(struct lflow_cache *lc, bool enabled) +{ + if (!lc) { + return; + } + + if (lc->enabled && !enabled) { + lflow_cache_flush(lc); + } + lc->enabled = enabled; +} + +bool +lflow_cache_is_enabled(struct lflow_cache *lc) +{ + return lc && lc->enabled; +} + +void +lflow_cache_add_conj_id(struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow, + uint32_t conj_id_ofs) +{ + struct lflow_cache_value *lcv = + lflow_cache_add__(lc, lflow, LCACHE_T_CONJ_ID); + + if (!lcv) { + return; + } + lcv->conj_id_ofs = conj_id_ofs; +} + +void +lflow_cache_add_expr(struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow, + uint32_t conj_id_ofs, + struct expr *expr) +{ + struct lflow_cache_value *lcv = + lflow_cache_add__(lc, lflow, LCACHE_T_EXPR); + + if (!lcv) { + expr_destroy(expr); + return; + } + lcv->conj_id_ofs = conj_id_ofs; + lcv->expr = expr; +} + +void +lflow_cache_add_matches(struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow, + struct hmap *matches) +{ + struct lflow_cache_value *lcv = + lflow_cache_add__(lc, lflow, LCACHE_T_MATCHES); + + if (!lcv) { + expr_matches_destroy(matches); + free(matches); + return; + } + lcv->expr_matches = matches; +} + +struct lflow_cache_value * +lflow_cache_get(struct lflow_cache *lc, const struct sbrec_logical_flow *lflow) +{ + if (!lflow_cache_is_enabled(lc)) { + return NULL; + } + + size_t hash = uuid_hash(&lflow->header_.uuid); + + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + struct lflow_cache_entry *lce; + + HMAP_FOR_EACH_WITH_HASH (lce, node, hash, &lc->entries[i]) { + if (uuid_equals(&lce->lflow_uuid, &lflow->header_.uuid)) { + return &lce->value; + } + } + } + return NULL; +} + +void +lflow_cache_delete(struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow) +{ + if (!lc) { + return; + } + + struct lflow_cache_value *lcv = lflow_cache_get(lc, lflow); + if (lcv) { + lflow_cache_delete__(lc, CONTAINER_OF(lcv, struct lflow_cache_entry, + value)); + } +} + +static struct lflow_cache_value * +lflow_cache_add__(struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow, + enum lflow_cache_type type) +{ + if (!lflow_cache_is_enabled(lc) || !lflow) { + return NULL; + } + + struct lflow_cache_entry *lce = xzalloc(sizeof *lce); + + lce->lflow_uuid = lflow->header_.uuid; + lce->value.type = type; + hmap_insert(&lc->entries[type], &lce->node, + uuid_hash(&lflow->header_.uuid)); + return &lce->value; +} + +static void +lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce) +{ + if (!lce) { + return; + } + + hmap_remove(&lc->entries[lce->value.type], &lce->node); + switch (lce->value.type) { + case LCACHE_T_NONE: + OVS_NOT_REACHED(); + break; + case LCACHE_T_CONJ_ID: + break; + case LCACHE_T_EXPR: + expr_destroy(lce->value.expr); + break; + case LCACHE_T_MATCHES: + expr_matches_destroy(lce->value.expr_matches); + free(lce->value.expr_matches); + break; + } + free(lce); +} diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h new file mode 100644 index 0000000..74a5b81 --- /dev/null +++ b/controller/lflow-cache.h @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2015, 2016 Nicira, Inc. + * Copyright (c) 2021, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef LFLOW_CACHE_H +#define LFLOW_CACHE_H 1 + +#include "openvswitch/hmap.h" +#include "openvswitch/uuid.h" + +struct sbrec_logical_flow; +struct lflow_cache; + +/* Various lflow cache types which + * - store the conjunction id offset if the lflow matches + * results in conjunctive OpenvSwitch flows. + * + * - Caches + * (1) Conjunction ID offset if the logical flow has port group/address + * set references. + * (2) expr tree if the logical flow has is_chassis_resident() match. + * (3) expr matches if (1) and (2) are false. + */ +enum lflow_cache_type { + LCACHE_T_CONJ_ID, /* Only conjunction id offset is cached. */ + LCACHE_T_EXPR, /* Expr tree of the logical flow is cached. */ + LCACHE_T_MATCHES, /* Expression matches are cached. */ + LCACHE_T_MAX, + LCACHE_T_NONE = LCACHE_T_MAX, /* Not found in cache. */ +}; + +struct lflow_cache_value { + enum lflow_cache_type type; + uint32_t conj_id_ofs; + + union { + struct hmap *expr_matches; + struct expr *expr; + }; +}; + +struct lflow_cache *lflow_cache_create(void); +void lflow_cache_flush(struct lflow_cache *); +void lflow_cache_destroy(struct lflow_cache *); +void lflow_cache_enable(struct lflow_cache *, bool enabled); +bool lflow_cache_is_enabled(struct lflow_cache *); + +void lflow_cache_add_conj_id(struct lflow_cache *, + const struct sbrec_logical_flow *, + uint32_t conj_id_ofs); +void lflow_cache_add_expr(struct lflow_cache *, + const struct sbrec_logical_flow *, + uint32_t conj_id_ofs, + struct expr *expr); +void lflow_cache_add_matches(struct lflow_cache *, + const struct sbrec_logical_flow *, + struct hmap *matches); + +struct lflow_cache_value *lflow_cache_get(struct lflow_cache *, + const struct sbrec_logical_flow *); +void lflow_cache_delete(struct lflow_cache *, + const struct sbrec_logical_flow *); + +#endif /* controller/lflow-cache.h */ diff --git a/controller/lflow.c b/controller/lflow.c index 495653f..3e3605a 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -17,6 +17,7 @@ #include "lflow.h" #include "coverage.h" #include "ha-chassis.h" +#include "lflow-cache.h" #include "lport.h" #include "ofctrl.h" #include "openvswitch/dynamic-string.h" @@ -306,103 +307,6 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr, free(lfrn); } -enum lflow_cache_type { - LCACHE_T_NO_CACHE, - LCACHE_T_MATCHES, - LCACHE_T_EXPR, -}; - -/* Represents an lflow cache which - * - stores the conjunction id offset if the lflow matches - * results in conjunctive OpenvSwitch flows. - * - * - Caches - * (1) Nothing if the logical flow has port group/address set references. - * (2) expr tree if the logical flow has is_chassis_resident() match. - * (3) expr matches if (1) and (2) are false. - */ -struct lflow_cache { - struct hmap_node node; - struct uuid lflow_uuid; /* key */ - uint32_t conj_id_ofs; - - enum lflow_cache_type type; - union { - struct { - struct hmap *expr_matches; - size_t n_conjs; - }; - struct expr *expr; - }; -}; - -static struct lflow_cache * -lflow_cache_add(struct hmap *lflow_cache_map, - const struct sbrec_logical_flow *lflow) -{ - struct lflow_cache *lc = xmalloc(sizeof *lc); - lc->lflow_uuid = lflow->header_.uuid; - lc->conj_id_ofs = 0; - lc->type = LCACHE_T_NO_CACHE; - hmap_insert(lflow_cache_map, &lc->node, uuid_hash(&lc->lflow_uuid)); - return lc; -} - -static struct lflow_cache * -lflow_cache_get(struct hmap *lflow_cache_map, - const struct sbrec_logical_flow *lflow) -{ - struct lflow_cache *lc; - size_t hash = uuid_hash(&lflow->header_.uuid); - HMAP_FOR_EACH_WITH_HASH (lc, node, hash, lflow_cache_map) { - if (uuid_equals(&lc->lflow_uuid, &lflow->header_.uuid)) { - return lc; - } - } - - return NULL; -} - -static void -lflow_cache_delete(struct hmap *lflow_cache_map, - const struct sbrec_logical_flow *lflow) -{ - struct lflow_cache *lc = lflow_cache_get(lflow_cache_map, lflow); - if (lc) { - hmap_remove(lflow_cache_map, &lc->node); - if (lc->type == LCACHE_T_MATCHES) { - expr_matches_destroy(lc->expr_matches); - free(lc->expr_matches); - } else if (lc->type == LCACHE_T_EXPR) { - expr_destroy(lc->expr); - } - free(lc); - } -} - -void -lflow_cache_init(struct hmap *lflow_cache_map) -{ - hmap_init(lflow_cache_map); -} - -void -lflow_cache_destroy(struct hmap *lflow_cache_map) -{ - struct lflow_cache *lc; - HMAP_FOR_EACH_POP (lc, node, lflow_cache_map) { - if (lc->type == LCACHE_T_MATCHES) { - expr_matches_destroy(lc->expr_matches); - free(lc->expr_matches); - } else if (lc->type == LCACHE_T_EXPR) { - expr_destroy(lc->expr); - } - free(lc); - } - - hmap_destroy(lflow_cache_map); -} - /* Adds the logical flows from the Logical_Flow table to flow tables. */ static void add_logical_flows(struct lflow_ctx_in *l_ctx_in, @@ -496,8 +400,8 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in, ofrn->sb_uuid = lflow->header_.uuid; hmap_insert(&flood_remove_nodes, &ofrn->hmap_node, uuid_hash(&ofrn->sb_uuid)); - if (l_ctx_out->lflow_cache_map) { - lflow_cache_delete(l_ctx_out->lflow_cache_map, lflow); + if (lflow_cache_is_enabled(l_ctx_out->lflow_cache)) { + lflow_cache_delete(l_ctx_out->lflow_cache, lflow); } } } @@ -659,10 +563,10 @@ update_conj_id_ofs(uint32_t *conj_id_ofs, uint32_t n_conjs) { if (*conj_id_ofs + n_conjs < *conj_id_ofs) { /* overflow */ - return false; + return true; } *conj_id_ofs += n_conjs; - return true; + return false; } static void @@ -884,152 +788,126 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, .lfrr = l_ctx_out->lfrr, }; - struct expr *expr = NULL; - if (!l_ctx_out->lflow_cache_map) { - /* Caching is disabled. */ - expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, - l_ctx_in->port_groups, l_ctx_out->lfrr, - NULL); - if (!expr) { - expr_destroy(prereqs); - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - return true; - } + struct lflow_cache_value *lcv = + lflow_cache_get(l_ctx_out->lflow_cache, lflow); + uint32_t conj_id_ofs = + lcv ? lcv->conj_id_ofs : *l_ctx_out->conj_id_ofs; + enum lflow_cache_type lcv_type = + lcv ? lcv->type : LCACHE_T_NONE; - expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux, - NULL); - expr = expr_normalize(expr); - struct hmap matches = HMAP_INITIALIZER(&matches); - uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, - &matches); - expr_destroy(expr); - if (hmap_is_empty(&matches)) { - VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", - UUID_ARGS(&lflow->header_.uuid)); - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - expr_matches_destroy(&matches); - return true; - } - - expr_matches_prepare(&matches, *l_ctx_out->conj_id_ofs); - add_matches_to_flow_table(lflow, dp, &matches, ptable, output_ptable, - &ovnacts, ingress, l_ctx_in, l_ctx_out); - - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - expr_matches_destroy(&matches); - return update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs); - } - - /* Caching is enabled. */ - struct lflow_cache *lc = - lflow_cache_get(l_ctx_out->lflow_cache_map, lflow); - - if (lc && lc->type == LCACHE_T_MATCHES) { - /* 'matches' is cached. No need to do expr parsing and no need - * to call expr_matches_prepare() to update the conj ids. - * Add matches to flow table and return. */ - add_matches_to_flow_table(lflow, dp, lc->expr_matches, ptable, - output_ptable, &ovnacts, ingress, - l_ctx_in, l_ctx_out); - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - expr_destroy(prereqs); - return true; - } + struct expr *cached_expr = NULL, *expr = NULL; + struct hmap *matches = NULL; - if (!lc) { - /* Create the lflow_cache for the lflow. */ - lc = lflow_cache_add(l_ctx_out->lflow_cache_map, lflow); - } + bool is_cr_cond_present = false; + bool pg_addr_set_ref = false; + uint32_t n_conjs = 0; - if (lc && lc->type == LCACHE_T_EXPR) { - expr = lc->expr; - } + bool conj_id_overflow = false; - bool pg_addr_set_ref = false; - if (!expr) { + /* Get match expr, either from cache or from lflow match. */ + switch (lcv_type) { + case LCACHE_T_NONE: + case LCACHE_T_CONJ_ID: expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, l_ctx_in->port_groups, l_ctx_out->lfrr, &pg_addr_set_ref); if (!expr) { - expr_destroy(prereqs); - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - return true; + goto done; } - } else { - expr_destroy(prereqs); - } - - ovs_assert(lc && expr); - - /* Cache the 'expr' only if the lflow doesn't reference a port group and - * address set. */ - if (!pg_addr_set_ref) { - /* Note: If the expr doesn't have 'is_chassis_resident, then the - * type will be set to LCACHE_T_MATCHES and 'matches' will be - * cached instead. See below. */ - lc->type = LCACHE_T_EXPR; - lc->expr = expr; - } - - if (lc->type == LCACHE_T_EXPR) { - expr = expr_clone(lc->expr); + break; + case LCACHE_T_EXPR: + expr = expr_clone(lcv->expr); + break; + case LCACHE_T_MATCHES: + break; } - bool is_cr_cond_present = false; - expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux, - &is_cr_cond_present); - expr = expr_normalize(expr); - struct hmap *matches = xmalloc(sizeof *matches); - uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, - matches); - expr_destroy(expr); - if (hmap_is_empty(matches)) { - VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", - UUID_ARGS(&lflow->header_.uuid)); - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - expr_matches_destroy(matches); - free(matches); - return true; + /* If caching is enabled and this is a not cached expr that doesn't refer + * to address sets or port groups, save it to potentially cache it later. + */ + if (lcv_type == LCACHE_T_NONE + && lflow_cache_is_enabled(l_ctx_out->lflow_cache) + && !pg_addr_set_ref) { + cached_expr = expr_clone(expr); } - if (n_conjs && !lc->conj_id_ofs) { - lc->conj_id_ofs = *l_ctx_out->conj_id_ofs; - if (!update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs)) { - lc->conj_id_ofs = 0; - expr_matches_destroy(matches); - free(matches); - return false; + /* Normalize expression if needed. */ + switch (lcv_type) { + case LCACHE_T_NONE: + case LCACHE_T_CONJ_ID: + case LCACHE_T_EXPR: + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux, + &is_cr_cond_present); + expr = expr_normalize(expr); + break; + case LCACHE_T_MATCHES: + break; + } + + /* Get matches, either from cache or from expr computed above. */ + switch (lcv_type) { + case LCACHE_T_NONE: + case LCACHE_T_CONJ_ID: + case LCACHE_T_EXPR: + matches = xmalloc(sizeof *matches); + n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, matches); + expr_matches_prepare(matches, conj_id_ofs); + if (hmap_is_empty(matches)) { + VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", + UUID_ARGS(&lflow->header_.uuid)); + goto done; } + break; + case LCACHE_T_MATCHES: + matches = lcv->expr_matches; + break; } - expr_matches_prepare(matches, lc->conj_id_ofs); - - /* Encode OVN logical actions into OpenFlow. */ add_matches_to_flow_table(lflow, dp, matches, ptable, output_ptable, &ovnacts, ingress, l_ctx_in, l_ctx_out); + + /* Update cache if needed. */ + switch (lcv_type) { + case LCACHE_T_NONE: + /* Entry not already in cache, update conjunction id offset and + * add the entry to the cache. + */ + conj_id_overflow = update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs); + + /* Cache new entry if caching is enabled. */ + if (lflow_cache_is_enabled(l_ctx_out->lflow_cache)) { + if (cached_expr && !is_cr_cond_present) { + lflow_cache_add_matches(l_ctx_out->lflow_cache, lflow, + matches); + matches = NULL; + } else if (cached_expr) { + lflow_cache_add_expr(l_ctx_out->lflow_cache, lflow, + conj_id_ofs, cached_expr); + cached_expr = NULL; + } else { + lflow_cache_add_conj_id(l_ctx_out->lflow_cache, lflow, + conj_id_ofs); + } + } + break; + case LCACHE_T_CONJ_ID: + case LCACHE_T_EXPR: + break; + case LCACHE_T_MATCHES: + /* Cached matches were used, don't destroy them. */ + matches = NULL; + break; + } + +done: + expr_destroy(prereqs); ovnacts_free(ovnacts.data, ovnacts.size); ofpbuf_uninit(&ovnacts); - - if (!is_cr_cond_present && lc->type == LCACHE_T_EXPR) { - /* If 'is_chassis_resident' match is not present, then cache - * 'matches'. */ - expr_destroy(lc->expr); - lc->type = LCACHE_T_MATCHES; - lc->expr_matches = matches; - } - - if (lc->type != LCACHE_T_MATCHES) { - expr_matches_destroy(matches); - free(matches); - } - - return true; + expr_destroy(expr); + expr_destroy(cached_expr); + expr_matches_destroy(matches); + free(matches); + return !conj_id_overflow; } static bool @@ -1440,14 +1318,14 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out) * don't exist anymore. */ void -lflow_handle_cached_flows(struct hmap *lflow_cache_map, +lflow_handle_cached_flows(struct lflow_cache *lc, const struct sbrec_logical_flow_table *flow_table) { const struct sbrec_logical_flow *lflow; SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, flow_table) { if (sbrec_logical_flow_is_deleted(lflow)) { - lflow_cache_delete(lflow_cache_map, lflow); + lflow_cache_delete(lc, lflow); } } } diff --git a/controller/lflow.h b/controller/lflow.h index cf4f0e8..833b42f 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -34,6 +34,7 @@ */ #include +#include "lflow-cache.h" #include "openvswitch/hmap.h" #include "openvswitch/uuid.h" #include "openvswitch/list.h" @@ -150,14 +151,14 @@ struct lflow_ctx_out { struct ovn_extend_table *group_table; struct ovn_extend_table *meter_table; struct lflow_resource_ref *lfrr; - struct hmap *lflow_cache_map; + struct lflow_cache *lflow_cache; uint32_t *conj_id_ofs; bool conj_id_overflow; }; void lflow_init(void); void lflow_run(struct lflow_ctx_in *, struct lflow_ctx_out *); -void lflow_handle_cached_flows(struct hmap *lflow_cache, +void lflow_handle_cached_flows(struct lflow_cache *, const struct sbrec_logical_flow_table *); bool lflow_handle_changed_flows(struct lflow_ctx_in *, struct lflow_ctx_out *); bool lflow_handle_changed_ref(enum ref_type, const char *ref_name, @@ -171,9 +172,6 @@ void lflow_handle_changed_neighbors( bool lflow_handle_changed_lbs(struct lflow_ctx_in *, struct lflow_ctx_out *); void lflow_destroy(void); -void lflow_cache_init(struct hmap *); -void lflow_cache_destroy(struct hmap *); - bool lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *, struct lflow_ctx_in *, struct lflow_ctx_out *); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 9014674..7d247fd 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -36,6 +36,7 @@ #include "ip-mcast.h" #include "openvswitch/hmap.h" #include "lflow.h" +#include "lflow-cache.h" #include "lib/vswitch-idl.h" #include "lport.h" #include "memory.h" @@ -94,6 +95,10 @@ static unixctl_cb_func debug_delay_nb_cfg_report; static char *parse_options(int argc, char *argv[]); OVS_NO_RETURN static void usage(void); +struct controller_engine_ctx { + struct lflow_cache *lflow_cache; +}; + /* Pending packet to be injected into connected OVS. */ struct pending_pkt { /* Setting 'conn' indicates that a request is pending. */ @@ -530,7 +535,8 @@ get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl) static void update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, bool *monitor_all_p, bool *reset_ovnsb_idl_min_index, - bool *enable_lflow_cache, unsigned int *sb_cond_seqno) + struct controller_engine_ctx *ctx, + unsigned int *sb_cond_seqno) { const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl); if (!cfg) { @@ -571,9 +577,11 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, *reset_ovnsb_idl_min_index = false; } - if (enable_lflow_cache != NULL) { - *enable_lflow_cache = - smap_get_bool(&cfg->external_ids, "ovn-enable-lflow-cache", true); + if (ctx) { + lflow_cache_enable(ctx->lflow_cache, + smap_get_bool(&cfg->external_ids, + "ovn-enable-lflow-cache", + true)); } } @@ -952,10 +960,6 @@ enum ovs_engine_node { OVS_NODES #undef OVS_NODE -struct controller_engine_ctx { - bool enable_lflow_cache; -}; - struct ed_type_ofctrl_is_connected { bool connected; }; @@ -1714,8 +1718,7 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node, void *data) struct flow_output_persistent_data { uint32_t conj_id_ofs; - struct hmap lflow_cache_map; - bool lflow_cache_enabled; + struct lflow_cache *lflow_cache; }; struct ed_type_flow_output { @@ -1904,11 +1907,7 @@ static void init_lflow_ctx(struct engine_node *node, l_ctx_out->meter_table = &fo->meter_table; l_ctx_out->lfrr = &fo->lflow_resource_ref; l_ctx_out->conj_id_ofs = &fo->pd.conj_id_ofs; - if (fo->pd.lflow_cache_enabled) { - l_ctx_out->lflow_cache_map = &fo->pd.lflow_cache_map; - } else { - l_ctx_out->lflow_cache_map = NULL; - } + l_ctx_out->lflow_cache = fo->pd.lflow_cache; l_ctx_out->conj_id_overflow = false; } @@ -1923,8 +1922,6 @@ en_flow_output_init(struct engine_node *node OVS_UNUSED, ovn_extend_table_init(&data->meter_table); data->pd.conj_id_ofs = 1; lflow_resource_init(&data->lflow_resource_ref); - lflow_cache_init(&data->pd.lflow_cache_map); - data->pd.lflow_cache_enabled = true; return data; } @@ -1936,7 +1933,7 @@ en_flow_output_cleanup(void *data) ovn_extend_table_destroy(&flow_output_data->group_table); ovn_extend_table_destroy(&flow_output_data->meter_table); lflow_resource_destroy(&flow_output_data->lflow_resource_ref); - lflow_cache_destroy(&flow_output_data->pd.lflow_cache_map); + lflow_cache_destroy(flow_output_data->pd.lflow_cache); } static void @@ -1983,13 +1980,10 @@ en_flow_output_run(struct engine_node *node, void *data) } struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx; - if (fo->pd.lflow_cache_enabled && !ctrl_ctx->enable_lflow_cache) { - lflow_cache_destroy(&fo->pd.lflow_cache_map); - lflow_cache_init(&fo->pd.lflow_cache_map); - } - fo->pd.lflow_cache_enabled = ctrl_ctx->enable_lflow_cache; - if (!fo->pd.lflow_cache_enabled) { + fo->pd.lflow_cache = ctrl_ctx->lflow_cache; + + if (!lflow_cache_is_enabled(fo->pd.lflow_cache)) { fo->pd.conj_id_ofs = 1; } @@ -2006,8 +2000,7 @@ en_flow_output_run(struct engine_node *node, void *data) ovn_extend_table_clear(meter_table, false /* desired */); lflow_resource_clear(lfrr); fo->pd.conj_id_ofs = 1; - lflow_cache_destroy(&fo->pd.lflow_cache_map); - lflow_cache_init(&fo->pd.lflow_cache_map); + lflow_cache_flush(fo->pd.lflow_cache); l_ctx_out.conj_id_overflow = false; lflow_run(&l_ctx_in, &l_ctx_out); if (l_ctx_out.conj_id_overflow) { @@ -2692,7 +2685,7 @@ main(int argc, char *argv[]) unsigned int ovnsb_expected_cond_seqno = UINT_MAX; struct controller_engine_ctx ctrl_engine_ctx = { - .enable_lflow_cache = true + .lflow_cache = lflow_cache_create(), }; char *ovn_version = ovn_get_internal_version(); @@ -2736,8 +2729,7 @@ main(int argc, char *argv[]) update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, &sb_monitor_all, &reset_ovnsb_idl_min_index, - &ctrl_engine_ctx.enable_lflow_cache, - &ovnsb_expected_cond_seqno); + &ctrl_engine_ctx, &ovnsb_expected_cond_seqno); update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl)); ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl)); @@ -2778,9 +2770,9 @@ main(int argc, char *argv[]) /* Unconditionally remove all deleted lflows from the lflow * cache. */ - if (flow_output_data && flow_output_data->pd.lflow_cache_enabled) { + if (lflow_cache_is_enabled(ctrl_engine_ctx.lflow_cache)) { lflow_handle_cached_flows( - &flow_output_data->pd.lflow_cache_map, + ctrl_engine_ctx.lflow_cache, sbrec_logical_flow_table_get(ovnsb_idl_loop.idl)); } @@ -3270,8 +3262,7 @@ lflow_cache_flush_cmd(struct unixctl_conn *conn OVS_UNUSED, { VLOG_INFO("User triggered lflow cache flush."); struct flow_output_persistent_data *fo_pd = arg_; - lflow_cache_destroy(&fo_pd->lflow_cache_map); - lflow_cache_init(&fo_pd->lflow_cache_map); + lflow_cache_flush(fo_pd->lflow_cache); fo_pd->conj_id_ofs = 1; engine_set_force_recompute(true); poll_immediate_wake(); diff --git a/lib/expr.c b/lib/expr.c index 796e88a..356e6fe 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -3151,6 +3151,10 @@ expr_matches_destroy(struct hmap *matches) { struct expr_match *m; + if (!matches) { + return; + } + HMAP_FOR_EACH_POP (m, hmap_node, matches) { free(m->conjunctions); free(m); From patchwork Thu Feb 4 13:25:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436005 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=OD4soxSY; dkim-atps=neutral Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfSS1yqqz9sWP for ; Fri, 5 Feb 2021 00:25:43 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 617B58717B; Thu, 4 Feb 2021 13:25:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tppcaBLmrh33; Thu, 4 Feb 2021 13:25:39 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id A6ABB87120; Thu, 4 Feb 2021 13:25:39 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 81573C1825; Thu, 4 Feb 2021 13:25:39 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0AA22C013A for ; Thu, 4 Feb 2021 13:25:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id ECC1085F56 for ; Thu, 4 Feb 2021 13:25:37 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id SUQyTiJ9deZX for ; Thu, 4 Feb 2021 13:25:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by fraxinus.osuosl.org (Postfix) with ESMTPS id EA90685F52 for ; Thu, 4 Feb 2021 13:25:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445135; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eoD6P0e02g8YfHHA3YLVgIz7U79vvaC98AsIWiXmcpk=; b=OD4soxSYtU5c2OSkzB6yPOHE86jy8zw3u+Is6dxHWngYKetscd2Z93NtS38PNtlKe4o48n 33AC0U1710b1lkxArhTBegIKBuuUKwMUOmgxZQjQlSov7PWzf9ZsVNLlCuYWrwHyLGpdqB FxKU+FqkTOYtqzwcTi776XC6RczjUeU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-534-AyYz0zatO3-2VQx_SZcJNA-1; Thu, 04 Feb 2021 08:25:33 -0500 X-MC-Unique: AyYz0zatO3-2VQx_SZcJNA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F2476100CCD2 for ; Thu, 4 Feb 2021 13:25:32 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5B4624D for ; Thu, 4 Feb 2021 13:25:32 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:25:30 +0100 Message-Id: <20210204132528.9958.3799.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 04/10] lflow-cache: Add lflow-cache/show-stats command. 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" Signed-off-by: Dumitru Ceara --- controller/lflow-cache.c | 21 +++++++++++++++++++++ controller/lflow-cache.h | 7 +++++++ controller/ovn-controller.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index ce129ac..8e1c71e 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -21,6 +21,12 @@ #include "lflow-cache.h" #include "ovn/expr.h" +const char *lflow_cache_type_names[LCACHE_T_MAX] = { + [LCACHE_T_CONJ_ID] = "cache-conj-id", + [LCACHE_T_EXPR] = "cache-expr", + [LCACHE_T_MATCHES] = "cache-matches", +}; + struct lflow_cache { struct hmap entries[LCACHE_T_MAX]; bool enabled; @@ -103,6 +109,21 @@ lflow_cache_is_enabled(struct lflow_cache *lc) return lc && lc->enabled; } +struct lflow_cache_stats * +lflow_cache_get_stats(const struct lflow_cache *lc) +{ + if (!lc) { + return NULL; + } + + struct lflow_cache_stats *stats = xmalloc(sizeof *stats); + + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + stats->n_entries[i] = hmap_count(&lc->entries[i]); + } + return stats; +} + void lflow_cache_add_conj_id(struct lflow_cache *lc, const struct sbrec_logical_flow *lflow, diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h index 74a5b81..54873ee 100644 --- a/controller/lflow-cache.h +++ b/controller/lflow-cache.h @@ -42,6 +42,8 @@ enum lflow_cache_type { LCACHE_T_NONE = LCACHE_T_MAX, /* Not found in cache. */ }; +extern const char *lflow_cache_type_names[LCACHE_T_MAX]; + struct lflow_cache_value { enum lflow_cache_type type; uint32_t conj_id_ofs; @@ -52,11 +54,16 @@ struct lflow_cache_value { }; }; +struct lflow_cache_stats { + size_t n_entries[LCACHE_T_MAX]; +}; + struct lflow_cache *lflow_cache_create(void); void lflow_cache_flush(struct lflow_cache *); void lflow_cache_destroy(struct lflow_cache *); void lflow_cache_enable(struct lflow_cache *, bool enabled); bool lflow_cache_is_enabled(struct lflow_cache *); +struct lflow_cache_stats *lflow_cache_get_stats(const struct lflow_cache *); void lflow_cache_add_conj_id(struct lflow_cache *, const struct sbrec_logical_flow *, diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 7d247fd..6ee6119 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -82,6 +82,7 @@ static unixctl_cb_func debug_pause_execution; static unixctl_cb_func debug_resume_execution; static unixctl_cb_func debug_status_execution; static unixctl_cb_func lflow_cache_flush_cmd; +static unixctl_cb_func lflow_cache_show_stats_cmd; static unixctl_cb_func debug_delay_nb_cfg_report; #define DEFAULT_BRIDGE_NAME "br-int" @@ -2662,6 +2663,9 @@ main(int argc, char *argv[]) unixctl_command_register("lflow-cache/flush", "", 0, 0, lflow_cache_flush_cmd, &flow_output_data->pd); + unixctl_command_register("lflow-cache/show-stats", "", 0, 0, + lflow_cache_show_stats_cmd, + &flow_output_data->pd); bool reset_ovnsb_idl_min_index = false; unixctl_command_register("sb-cluster-state-reset", "", 0, 0, @@ -3270,6 +3274,31 @@ lflow_cache_flush_cmd(struct unixctl_conn *conn OVS_UNUSED, } 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_cache *lc = fo_pd->lflow_cache; + struct ds ds = DS_EMPTY_INITIALIZER; + + ds_put_format(&ds, "Enabled: %s\n", + lflow_cache_is_enabled(lc) ? "true" : "false"); + + struct lflow_cache_stats *stats = lflow_cache_get_stats(lc); + if (stats) { + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + ds_put_format(&ds, "%-16s: %"PRIuSIZE"\n", + lflow_cache_type_names[i], + stats->n_entries[i]); + } + } + unixctl_command_reply(conn, ds_cstr(&ds)); + + ds_destroy(&ds); + free(stats); +} + +static void cluster_state_reset_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *idl_reset_) { From patchwork Thu Feb 4 13:25:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436008 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=BlpOtSrE; dkim-atps=neutral Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfTV2ZSRz9sVq for ; Fri, 5 Feb 2021 00:26:38 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id E894386F69; Thu, 4 Feb 2021 13:26:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cwZFcoi+pb0D; Thu, 4 Feb 2021 13:26:35 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 5394285C6F; Thu, 4 Feb 2021 13:26:35 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2A6C8C1D9F; Thu, 4 Feb 2021 13:26:35 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id EE590C013A for ; Thu, 4 Feb 2021 13:26:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id C9A00273EB for ; Thu, 4 Feb 2021 13:26:33 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id x08y7n4BFGGm for ; Thu, 4 Feb 2021 13:26:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by silver.osuosl.org (Postfix) with ESMTPS id E4B97274ED for ; Thu, 4 Feb 2021 13:25:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445145; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UKPx9UBVx47SIQTZqTkdM21cOytBIUb+zzBUpXTWdv0=; b=BlpOtSrEIttN0W5nQF9F0LV2YaUeNgV1mb9txCOF1NtZxbZFnYCs79p27zt3bFH8Jp1xSc Os/h2cPWKMTkrTHUGvUvPdgtt80mzmRrUO80BP+46eUGJ05TDYDk0PV4jXUhug44H0hCMI JDvA1BdBJLqQsad6Ifltk1aawA9mt4Y= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-81-80Ef4eLlMZeMGV6Zfz4Zcw-1; Thu, 04 Feb 2021 08:25:43 -0500 X-MC-Unique: 80Ef4eLlMZeMGV6Zfz4Zcw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0122CC73A0 for ; Thu, 4 Feb 2021 13:25:43 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3ACE260D09 for ; Thu, 4 Feb 2021 13:25:42 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:25:40 +0100 Message-Id: <20210204132538.9958.40844.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 05/10] lflow-cache: Add unit tests. 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" Signed-off-by: Dumitru Ceara Acked-by: Numan Siddique --- controller/test-lflow-cache.c | 224 +++++++++++++++++++++++++++++++++++ controller/test-ofctrl-seqno.c | 18 --- tests/automake.mk | 8 + tests/ovn-lflow-cache.at | 257 ++++++++++++++++++++++++++++++++++++++++ tests/ovn.at | 68 +++++++++++ tests/test-utils.c | 49 ++++++++ tests/test-utils.h | 26 ++++ tests/testsuite.at | 1 8 files changed, 633 insertions(+), 18 deletions(-) create mode 100644 controller/test-lflow-cache.c create mode 100644 tests/ovn-lflow-cache.at create mode 100644 tests/test-utils.c create mode 100644 tests/test-utils.h diff --git a/controller/test-lflow-cache.c b/controller/test-lflow-cache.c new file mode 100644 index 0000000..d6e962e --- /dev/null +++ b/controller/test-lflow-cache.c @@ -0,0 +1,224 @@ +/* Copyright (c) 2021, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "ovn/expr.h" +#include "ovn-sb-idl.h" +#include "tests/ovstest.h" +#include "tests/test-utils.h" +#include "util.h" + +#include "lflow-cache.h" + +static void +test_lflow_cache_add__(struct lflow_cache *lc, const char *op_type, + const struct sbrec_logical_flow *lflow, + unsigned int conj_id_ofs, + struct expr *e) +{ + printf("ADD %s:\n", op_type); + printf(" conj-id-ofs: %u\n", conj_id_ofs); + + if (!strcmp(op_type, "conj-id")) { + lflow_cache_add_conj_id(lc, lflow, conj_id_ofs); + } else if (!strcmp(op_type, "expr")) { + lflow_cache_add_expr(lc, lflow, conj_id_ofs, expr_clone(e)); + } else if (!strcmp(op_type, "matches")) { + struct hmap *matches = xmalloc(sizeof *matches); + ovs_assert(expr_to_matches(e, NULL, NULL, matches) == 0); + ovs_assert(hmap_count(matches) == 1); + lflow_cache_add_matches(lc, lflow, matches); + } else { + OVS_NOT_REACHED(); + } +} + +static void +test_lflow_cache_lookup__(struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow) +{ + struct lflow_cache_value *lcv = lflow_cache_get(lc, lflow); + + printf("LOOKUP:\n"); + if (!lcv) { + printf(" not found\n"); + return; + } + + printf(" conj_id_ofs: %"PRIu32"\n", lcv->conj_id_ofs); + switch (lcv->type) { + case LCACHE_T_CONJ_ID: + printf(" type: conj-id\n"); + break; + case LCACHE_T_EXPR: + printf(" type: expr\n"); + break; + case LCACHE_T_MATCHES: + printf(" type: matches\n"); + break; + case LCACHE_T_NONE: + OVS_NOT_REACHED(); + break; + } +} + +static void +test_lflow_cache_delete__(struct lflow_cache *lc, + const struct sbrec_logical_flow *lflow) +{ + printf("DELETE\n"); + lflow_cache_delete(lc, lflow); +} + +static void +test_lflow_cache_stats__(struct lflow_cache *lc) +{ + struct lflow_cache_stats *lcs = lflow_cache_get_stats(lc); + + if (!lcs) { + return; + } + printf("Enabled: %s\n", lflow_cache_is_enabled(lc) ? "true" : "false"); + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + printf(" %s: %"PRIuSIZE"\n", lflow_cache_type_names[i], + lcs->n_entries[i]); + } + free(lcs); +} + +static void +test_lflow_cache_operations(struct ovs_cmdl_context *ctx) +{ + struct lflow_cache *lc = lflow_cache_create(); + struct expr *e = expr_create_boolean(true); + bool enabled = !strcmp(ctx->argv[1], "true"); + unsigned int shift = 2; + unsigned int n_ops; + + lflow_cache_enable(lc, enabled); + test_lflow_cache_stats__(lc); + + if (!test_read_uint_value(ctx, shift++, "n_ops", &n_ops)) { + goto done; + } + + for (unsigned int i = 0; i < n_ops; i++) { + const char *op = test_read_value(ctx, shift++, "op"); + + if (!op) { + goto done; + } + + struct sbrec_logical_flow lflow; + uuid_generate(&lflow.header_.uuid); + + if (!strcmp(op, "add")) { + const char *op_type = test_read_value(ctx, shift++, "op_type"); + if (!op_type) { + goto done; + } + + unsigned int conj_id_ofs; + if (!test_read_uint_value(ctx, shift++, "conj-id-ofs", + &conj_id_ofs)) { + goto done; + } + + test_lflow_cache_add__(lc, op_type, &lflow, conj_id_ofs, e); + test_lflow_cache_lookup__(lc, &lflow); + } else if (!strcmp(op, "add-del")) { + const char *op_type = test_read_value(ctx, shift++, "op_type"); + if (!op_type) { + goto done; + } + + unsigned int conj_id_ofs; + if (!test_read_uint_value(ctx, shift++, "conj-id-ofs", + &conj_id_ofs)) { + goto done; + } + + test_lflow_cache_add__(lc, op_type, &lflow, conj_id_ofs, e); + test_lflow_cache_lookup__(lc, &lflow); + test_lflow_cache_delete__(lc, &lflow); + test_lflow_cache_lookup__(lc, &lflow); + } else if (!strcmp(op, "enable")) { + printf("ENABLE\n"); + lflow_cache_enable(lc, true); + } else if (!strcmp(op, "disable")) { + printf("DISABLE\n"); + lflow_cache_enable(lc, false); + } else if (!strcmp(op, "flush")) { + printf("FLUSH\n"); + lflow_cache_flush(lc); + } else { + OVS_NOT_REACHED(); + } + test_lflow_cache_stats__(lc); + } +done: + lflow_cache_destroy(lc); + expr_destroy(e); +} + +static void +test_lflow_cache_negative(struct ovs_cmdl_context *ctx OVS_UNUSED) +{ + lflow_cache_flush(NULL); + lflow_cache_destroy(NULL); + lflow_cache_enable(NULL, true); + ovs_assert(!lflow_cache_is_enabled(NULL)); + ovs_assert(!lflow_cache_get_stats(NULL)); + + struct lflow_cache *lcs[] = { + NULL, + lflow_cache_create(), + }; + + for (size_t i = 0; i < ARRAY_SIZE(lcs); i++) { + struct expr *e = expr_create_boolean(true); + struct hmap *matches = xmalloc(sizeof *matches); + + ovs_assert(expr_to_matches(e, NULL, NULL, matches) == 0); + ovs_assert(hmap_count(matches) == 1); + + lflow_cache_add_conj_id(lcs[i], NULL, 0); + lflow_cache_add_expr(lcs[i], NULL, 0, NULL); + lflow_cache_add_expr(lcs[i], NULL, 0, e); + lflow_cache_add_matches(lcs[i], NULL, NULL); + lflow_cache_add_matches(lcs[i], NULL, matches); + lflow_cache_destroy(lcs[i]); + } +} + +static void +test_lflow_cache_main(int argc, char *argv[]) +{ + set_program_name(argv[0]); + static const struct ovs_cmdl_command commands[] = { + {"lflow_cache_operations", NULL, 3, INT_MAX, + test_lflow_cache_operations, OVS_RO}, + {"lflow_cache_negative", NULL, 0, 0, + test_lflow_cache_negative, OVS_RO}, + {NULL, NULL, 0, 0, NULL, OVS_RO}, + }; + struct ovs_cmdl_context ctx; + ctx.argc = argc - 1; + ctx.argv = argv + 1; + ovs_cmdl_run_command(&ctx, commands); +} + +OVSTEST_REGISTER("test-lflow-cache", test_lflow_cache_main); diff --git a/controller/test-ofctrl-seqno.c b/controller/test-ofctrl-seqno.c index fce88d4..b96da9d 100644 --- a/controller/test-ofctrl-seqno.c +++ b/controller/test-ofctrl-seqno.c @@ -16,6 +16,7 @@ #include #include "tests/ovstest.h" +#include "tests/test-utils.h" #include "sort.h" #include "util.h" @@ -27,23 +28,6 @@ test_init(void) ofctrl_seqno_init(); } -static bool -test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index, - const char *descr, unsigned int *result) -{ - if (index >= ctx->argc) { - fprintf(stderr, "Missing %s argument\n", descr); - return false; - } - - const char *arg = ctx->argv[index]; - if (!str_to_uint(arg, 10, result)) { - fprintf(stderr, "Invalid %s: %s\n", descr, arg); - return false; - } - return true; -} - static int test_seqno_compare(size_t a, size_t b, void *values_) { diff --git a/tests/automake.mk b/tests/automake.mk index 282d1b1..df6d0a2 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -33,7 +33,8 @@ TESTSUITE_AT = \ tests/ovn-macros.at \ tests/ovn-performance.at \ tests/ovn-ofctrl-seqno.at \ - tests/ovn-ipam.at + tests/ovn-ipam.at \ + tests/ovn-lflow-cache.at SYSTEM_KMOD_TESTSUITE_AT = \ tests/system-common-macros.at \ @@ -207,8 +208,13 @@ noinst_PROGRAMS += tests/ovstest tests_ovstest_SOURCES = \ tests/ovstest.c \ tests/ovstest.h \ + tests/test-utils.c \ + tests/test-utils.h \ tests/test-ovn.c \ + controller/test-lflow-cache.c \ controller/test-ofctrl-seqno.c \ + controller/lflow-cache.c \ + controller/lflow-cache.h \ controller/ofctrl-seqno.c \ controller/ofctrl-seqno.h \ northd/test-ipam.c \ diff --git a/tests/ovn-lflow-cache.at b/tests/ovn-lflow-cache.at new file mode 100644 index 0000000..f7e8959 --- /dev/null +++ b/tests/ovn-lflow-cache.at @@ -0,0 +1,257 @@ +# +# Unit tests for the controller/lflow-cache.c module. +# +AT_BANNER([OVN unit tests - lflow-cache]) + +AT_SETUP([ovn -- unit test -- lflow-cache single add/lookup]) +AT_CHECK( + [ovstest test-lflow-cache lflow_cache_operations \ + true 3 \ + add conj-id 1 \ + add expr 2 \ + add matches 3], + [0], [dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 1 +LOOKUP: + conj_id_ofs: 1 + type: conj-id +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 2 +LOOKUP: + conj_id_ofs: 2 + type: expr +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 0 +ADD matches: + conj-id-ofs: 3 +LOOKUP: + conj_id_ofs: 0 + type: matches +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 1 +]) +AT_CLEANUP + +AT_SETUP([ovn -- unit test -- lflow-cache single add/lookup/del]) +AT_CHECK( + [ovstest test-lflow-cache lflow_cache_operations \ + true 3 \ + add-del conj-id 1 \ + add-del expr 2 \ + add-del matches 3], + [0], [dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 1 +LOOKUP: + conj_id_ofs: 1 + type: conj-id +DELETE +LOOKUP: + not found +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 2 +LOOKUP: + conj_id_ofs: 2 + type: expr +DELETE +LOOKUP: + not found +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD matches: + conj-id-ofs: 3 +LOOKUP: + conj_id_ofs: 0 + type: matches +DELETE +LOOKUP: + not found +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +]) +AT_CLEANUP + +AT_SETUP([ovn -- unit test -- lflow-cache disabled single add/lookup/del]) +AT_CHECK( + [ovstest test-lflow-cache lflow_cache_operations \ + false 3 \ + add conj-id 1 \ + add expr 2 \ + add matches 3], + [0], [dnl +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 1 +LOOKUP: + not found +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 2 +LOOKUP: + not found +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD matches: + conj-id-ofs: 3 +LOOKUP: + not found +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +]) +AT_CLEANUP + +AT_SETUP([ovn -- unit test -- lflow-cache disable/enable/flush]) +AT_CHECK( + [ovstest test-lflow-cache lflow_cache_operations \ + true 12 \ + add conj-id 1 \ + add expr 2 \ + add matches 3 \ + disable \ + add conj-id 4 \ + add expr 5 \ + add matches 6 \ + enable \ + add conj-id 7 \ + add expr 8 \ + add matches 9 \ + flush], + [0], [dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 1 +LOOKUP: + conj_id_ofs: 1 + type: conj-id +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 2 +LOOKUP: + conj_id_ofs: 2 + type: expr +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 0 +ADD matches: + conj-id-ofs: 3 +LOOKUP: + conj_id_ofs: 0 + type: matches +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 1 +DISABLE +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 4 +LOOKUP: + not found +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 5 +LOOKUP: + not found +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD matches: + conj-id-ofs: 6 +LOOKUP: + not found +Enabled: false + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ENABLE +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 7 +LOOKUP: + conj_id_ofs: 7 + type: conj-id +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 8 +LOOKUP: + conj_id_ofs: 8 + type: expr +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 0 +ADD matches: + conj-id-ofs: 9 +LOOKUP: + conj_id_ofs: 0 + type: matches +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 1 +FLUSH +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +]) +AT_CLEANUP + +AT_SETUP([ovn -- unit test -- lflow-cache negative tests]) +AT_CHECK([ovstest test-lflow-cache lflow_cache_negative], [0], []) +AT_CLEANUP diff --git a/tests/ovn.at b/tests/ovn.at index 4a75acc..0e114cf 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22235,6 +22235,74 @@ OVN_CLEANUP([hv1]) AT_CLEANUP +AT_SETUP([ovn -- lflow cache operations]) +ovn_start +net_add n1 +sim_add hv1 + +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +as hv1 +ovs-vsctl -- add-port br-int hv1-vif1 \ + -- set interface hv1-vif1 external-ids:iface-id=lsp1 \ + -- add-port br-int hv1-vif2 \ + -- set interface hv1-vif2 external-ids:iface-id=lsp2 + +ovn-nbctl ls-add ls1 \ + -- lsp-add ls1 lsp1 \ + -- lsp-add ls1 lsp2 \ + -- pg-add pg1 lsp1 lsp2 \ + -- create Address_Set name=as1 addresses=\"10.0.0.1\",\"10.0.0.2\" +check ovn-nbctl --wait=hv sync +wait_for_ports_up lsp1 lsp2 + +get_cache_count () { + local cache_name=$1 + as hv1 ovn-appctl -t ovn-controller lflow-cache/show-stats | grep ${cache_name} | awk '{ print $3 }' +} + +AS_BOX([Check matches caching]) +conj_id_cnt=$(get_cache_count cache-conj-id) +expr_cnt=$(get_cache_count cache-expr) +matches_cnt=$(get_cache_count cache-matches) + +check ovn-nbctl acl-add ls1 from-lport 1 '1' drop +check ovn-nbctl --wait=hv sync + +AT_CHECK([test "$conj_id_cnt" = "$(get_cache_count cache-conj-id)"], [0], []) +AT_CHECK([test "$expr_cnt" = "$(get_cache_count cache-expr)"], [0], []) +AT_CHECK([test "$(($matches_cnt + 1))" = "$(get_cache_count cache-matches)"], [0], []) + +AS_BOX([Check expr caching for is_chassis_resident() matches]) +conj_id_cnt=$(get_cache_count cache-conj-id) +expr_cnt=$(get_cache_count cache-expr) +matches_cnt=$(get_cache_count cache-matches) + +check ovn-nbctl acl-add ls1 from-lport 1 'is_chassis_resident("lsp1")' drop +check ovn-nbctl --wait=hv sync + +AT_CHECK([test "$conj_id_cnt" = "$(get_cache_count cache-conj-id)"], [0], []) +AT_CHECK([test "$(($expr_cnt + 1))" = "$(get_cache_count cache-expr)"], [0], []) +AT_CHECK([test "$matches_cnt" = "$(get_cache_count cache-matches)"], [0], []) + +AS_BOX([Check conj-id caching for conjunctive port group/address set matches]) +conj_id_cnt=$(get_cache_count cache-conj-id) +expr_cnt=$(get_cache_count cache-expr) +matches_cnt=$(get_cache_count cache-matches) + +check ovn-nbctl acl-add ls1 from-lport 1 'inport == @pg1 && outport == @pg1 && is_chassis_resident("lsp1")' drop +check ovn-nbctl acl-add ls1 from-lport 1 'ip4.src == $as1 && ip4.dst == $as1 && is_chassis_resident("lsp1")' drop +check ovn-nbctl --wait=hv sync + +AT_CHECK([test "$(($conj_id_cnt + 2))" = "$(get_cache_count cache-conj-id)"], [0], []) +AT_CHECK([test "$expr_cnt" = "$(get_cache_count cache-expr)"], [0], []) +AT_CHECK([test "$matches_cnt" = "$(get_cache_count cache-matches)"], [0], []) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + AT_SETUP([ovn -- Delete Port_Binding and OVS port Incremental Processing]) ovn_start diff --git a/tests/test-utils.c b/tests/test-utils.c new file mode 100644 index 0000000..6a3b198 --- /dev/null +++ b/tests/test-utils.c @@ -0,0 +1,49 @@ +/* Copyright (c) 2021, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "test-utils.h" + +#include "util.h" + +bool +test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index, + const char *descr, unsigned int *result) +{ + if (index >= ctx->argc) { + fprintf(stderr, "Missing %s argument\n", descr); + return false; + } + + const char *arg = ctx->argv[index]; + if (!str_to_uint(arg, 10, result)) { + fprintf(stderr, "Invalid %s: %s\n", descr, arg); + return false; + } + return true; +} + +const char * +test_read_value(struct ovs_cmdl_context *ctx, unsigned int index, + const char *descr) +{ + if (index >= ctx->argc) { + fprintf(stderr, "Missing %s argument\n", descr); + return NULL; + } + + return ctx->argv[index]; +} diff --git a/tests/test-utils.h b/tests/test-utils.h new file mode 100644 index 0000000..721032f --- /dev/null +++ b/tests/test-utils.h @@ -0,0 +1,26 @@ +/* Copyright (c) 2021, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef TEST_UTILS_H +#define TEST_UTILS_H 1 + +#include "ovstest.h" + +bool test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index, + const char *descr, unsigned int *result); +const char *test_read_value(struct ovs_cmdl_context *ctx, unsigned int index, + const char *descr); + +#endif /* tests/test-utils.h */ diff --git a/tests/testsuite.at b/tests/testsuite.at index 6253d11..35908ad 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -27,6 +27,7 @@ m4_include([tests/ovn.at]) m4_include([tests/ovn-performance.at]) m4_include([tests/ovn-northd.at]) m4_include([tests/ovn-nbctl.at]) +m4_include([tests/ovn-lflow-cache.at]) m4_include([tests/ovn-ofctrl-seqno.at]) m4_include([tests/ovn-sbctl.at]) m4_include([tests/ovn-ic-nbctl.at]) From patchwork Thu Feb 4 13:25:51 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436007 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.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Bs+VJPCn; dkim-atps=neutral Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfTM3PKmz9sWP for ; Fri, 5 Feb 2021 00:26:31 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id E0AFC86C02; Thu, 4 Feb 2021 13:26:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CnDXRNglWtW4; Thu, 4 Feb 2021 13:26:24 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 2AB9886BC6; Thu, 4 Feb 2021 13:26:24 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1929AC1825; Thu, 4 Feb 2021 13:26:24 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 25C8DC1825 for ; Thu, 4 Feb 2021 13:26:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 08D7F27A60 for ; Thu, 4 Feb 2021 13:26:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id m1NydgLC9CFM for ; Thu, 4 Feb 2021 13:26:17 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by silver.osuosl.org (Postfix) with ESMTPS id 0C102228DF for ; Thu, 4 Feb 2021 13:25:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445157; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ABr8hq60A5VOQqHUPn9J8Bx0EVJdoh9khrBe7yWmbKs=; b=Bs+VJPCniIQaWVDNImEvquZRADwV4ZThXcj5w7+6rMc+jp6pZywga++EAEY+wwNWQvJO5s jgvJ2xI6bfUtXeg9L+fH7PL+sSuIKsDNShGN8jqcDnfbJzpkkQW6gLXh9rD7aq/TDReYLV jYrCoYbj9VUsOHndluG8xHXXijdXIVw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-561-2yz1Zzh7NPWnYIC9XBsmBw-1; Thu, 04 Feb 2021 08:25:55 -0500 X-MC-Unique: 2yz1Zzh7NPWnYIC9XBsmBw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 76922801961 for ; Thu, 4 Feb 2021 13:25:54 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id EFABF5D9C9 for ; Thu, 4 Feb 2021 13:25:53 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:25:51 +0100 Message-Id: <20210204132548.9958.61101.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 06/10] lflow: Do not cache non-conjunctive flows that use address sets/portgroups. 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" Caching the conjunction id offset for flows that refer to address sets and/or port groups but do not currently generate conjunctive matches, e.g., because the port group has only one locally bound port, is wrong. If ports are later added to the port group and/or addresses are added to the address set, this flow will be reconsidered but at this point will generate conjunctive matches. We cannot use the cached conjunction id offset because it might create collisions with other conjunction ids created for unrelated flows. Fixes: 1213bc827040 ("ovn-controller: Cache logical flow expr matches.") Signed-off-by: Dumitru Ceara --- controller/lflow.c | 2 +- tests/ovn.at | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/controller/lflow.c b/controller/lflow.c index 3e3605a..c493652 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -884,7 +884,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, lflow_cache_add_expr(l_ctx_out->lflow_cache, lflow, conj_id_ofs, cached_expr); cached_expr = NULL; - } else { + } else if (n_conjs) { lflow_cache_add_conj_id(l_ctx_out->lflow_cache, lflow, conj_id_ofs); } diff --git a/tests/ovn.at b/tests/ovn.at index 0e114cf..4bb92d6 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22300,6 +22300,18 @@ AT_CHECK([test "$(($conj_id_cnt + 2))" = "$(get_cache_count cache-conj-id)"], [0 AT_CHECK([test "$expr_cnt" = "$(get_cache_count cache-expr)"], [0], []) AT_CHECK([test "$matches_cnt" = "$(get_cache_count cache-matches)"], [0], []) +AS_BOX([Check no caching caching for non-conjunctive port group/address set matches]) +conj_id_cnt=$(get_cache_count cache-conj-id) +expr_cnt=$(get_cache_count cache-expr) +matches_cnt=$(get_cache_count cache-matches) + +check ovn-nbctl acl-add ls1 from-lport 1 'inport == @pg2 && outport == @pg2 && is_chassis_resident("lsp1")' drop +check ovn-nbctl --wait=hv sync + +AT_CHECK([test "$conj_id_cnt" = "$(get_cache_count cache-conj-id)"], [0], []) +AT_CHECK([test "$expr_cnt" = "$(get_cache_count cache-expr)"], [0], []) +AT_CHECK([test "$matches_cnt" = "$(get_cache_count cache-matches)"], [0], []) + OVN_CLEANUP([hv1]) AT_CLEANUP From patchwork Thu Feb 4 13:26:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436009 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=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KIkhr0DD; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfTl6f3xz9sVq for ; Fri, 5 Feb 2021 00:26:51 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 0F0832DE24; Thu, 4 Feb 2021 13:26:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NAGkmkEGtdOG; Thu, 4 Feb 2021 13:26:41 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 6560827447; Thu, 4 Feb 2021 13:26:18 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 20915C1825; Thu, 4 Feb 2021 13:26:18 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id E6335C013A for ; Thu, 4 Feb 2021 13:26:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id D67C186BF3 for ; Thu, 4 Feb 2021 13:26:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IYrwuN0dOXb1 for ; Thu, 4 Feb 2021 13:26:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by whitealder.osuosl.org (Postfix) with ESMTPS id 6C08D86BE0 for ; Thu, 4 Feb 2021 13:26:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445167; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6QZ/omQo/5XP2fI+G/7vG8Do2ogtSQzj5vuaNVSbygg=; b=KIkhr0DDdF3812azNB3BPTQgDSagMYKL99/T/BKnMKLX9LBWVgqjPTD4MMRZRo9VQ4L3e7 7ye8gZ/vD/4eiD17GvHgbVVGhcHRMZ+Ioegnt3BqFNTZbCaFT/fljKHbTPk2kKS7tiqiTI jcoLE7eGrYZcTt2Cwyj9tpagIEWb3bM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-542-toSb-z3tMAeZNRd_ntUvWQ-1; Thu, 04 Feb 2021 08:26:05 -0500 X-MC-Unique: toSb-z3tMAeZNRd_ntUvWQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B43B5C73A8 for ; Thu, 4 Feb 2021 13:26:04 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A6F019C79 for ; Thu, 4 Feb 2021 13:26:04 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:26:01 +0100 Message-Id: <20210204132559.9958.50315.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 07/10] lflow-cache: Add coverage counters. 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" These are available in ovn-controller logs or when explicitly requested by users with "ovn-appctl -t ovn-controller coverage/show". Signed-off-by: Dumitru Ceara --- controller/lflow-cache.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index 8e1c71e..1549034 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -17,10 +17,23 @@ #include +#include "coverage.h" #include "lib/ovn-sb-idl.h" #include "lflow-cache.h" #include "ovn/expr.h" +COVERAGE_DEFINE(lflow_cache_flush); +COVERAGE_DEFINE(lflow_cache_add_conj_id); +COVERAGE_DEFINE(lflow_cache_add_expr); +COVERAGE_DEFINE(lflow_cache_add_matches); +COVERAGE_DEFINE(lflow_cache_free_conj_id); +COVERAGE_DEFINE(lflow_cache_free_expr); +COVERAGE_DEFINE(lflow_cache_free_matches); +COVERAGE_DEFINE(lflow_cache_add); +COVERAGE_DEFINE(lflow_cache_hit); +COVERAGE_DEFINE(lflow_cache_miss); +COVERAGE_DEFINE(lflow_cache_delete); + const char *lflow_cache_type_names[LCACHE_T_MAX] = { [LCACHE_T_CONJ_ID] = "cache-conj-id", [LCACHE_T_EXPR] = "cache-expr", @@ -66,6 +79,7 @@ lflow_cache_flush(struct lflow_cache *lc) return; } + COVERAGE_INC(lflow_cache_flush); for (size_t i = 0; i < LCACHE_T_MAX; i++) { struct lflow_cache_entry *lce; struct lflow_cache_entry *lce_next; @@ -135,6 +149,7 @@ lflow_cache_add_conj_id(struct lflow_cache *lc, if (!lcv) { return; } + COVERAGE_INC(lflow_cache_add_conj_id); lcv->conj_id_ofs = conj_id_ofs; } @@ -151,6 +166,7 @@ lflow_cache_add_expr(struct lflow_cache *lc, expr_destroy(expr); return; } + COVERAGE_INC(lflow_cache_add_expr); lcv->conj_id_ofs = conj_id_ofs; lcv->expr = expr; } @@ -168,6 +184,7 @@ lflow_cache_add_matches(struct lflow_cache *lc, free(matches); return; } + COVERAGE_INC(lflow_cache_add_matches); lcv->expr_matches = matches; } @@ -185,10 +202,12 @@ lflow_cache_get(struct lflow_cache *lc, const struct sbrec_logical_flow *lflow) HMAP_FOR_EACH_WITH_HASH (lce, node, hash, &lc->entries[i]) { if (uuid_equals(&lce->lflow_uuid, &lflow->header_.uuid)) { + COVERAGE_INC(lflow_cache_hit); return &lce->value; } } } + COVERAGE_INC(lflow_cache_miss); return NULL; } @@ -202,6 +221,7 @@ lflow_cache_delete(struct lflow_cache *lc, struct lflow_cache_value *lcv = lflow_cache_get(lc, lflow); if (lcv) { + COVERAGE_INC(lflow_cache_delete); lflow_cache_delete__(lc, CONTAINER_OF(lcv, struct lflow_cache_entry, value)); } @@ -218,6 +238,7 @@ lflow_cache_add__(struct lflow_cache *lc, struct lflow_cache_entry *lce = xzalloc(sizeof *lce); + COVERAGE_INC(lflow_cache_add); lce->lflow_uuid = lflow->header_.uuid; lce->value.type = type; hmap_insert(&lc->entries[type], &lce->node, @@ -238,11 +259,14 @@ lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce) OVS_NOT_REACHED(); break; case LCACHE_T_CONJ_ID: + COVERAGE_INC(lflow_cache_free_conj_id); break; case LCACHE_T_EXPR: + COVERAGE_INC(lflow_cache_free_expr); expr_destroy(lce->value.expr); break; case LCACHE_T_MATCHES: + COVERAGE_INC(lflow_cache_free_matches); expr_matches_destroy(lce->value.expr_matches); free(lce->value.expr_matches); break; From patchwork Thu Feb 4 13:26:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436011 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Gm3cV/vJ; dkim-atps=neutral Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfTw0fYvz9sVq for ; Fri, 5 Feb 2021 00:27:00 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id A8355871EB; Thu, 4 Feb 2021 13:26:58 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id p-miokV6O3nU; Thu, 4 Feb 2021 13:26:57 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 6B48E871B8; Thu, 4 Feb 2021 13:26:57 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5C29DC1825; Thu, 4 Feb 2021 13:26:57 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id B542EC07FD for ; Thu, 4 Feb 2021 13:26:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 9D53220335 for ; Thu, 4 Feb 2021 13:26:55 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id y32QbY9V86jE for ; Thu, 4 Feb 2021 13:26:51 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by silver.osuosl.org (Postfix) with ESMTPS id 0A823276D4 for ; Thu, 4 Feb 2021 13:26:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445178; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pqrvfpy3AvJMEa61Xjn0ETmqy9HXwwJA0o9H19zCSp0=; b=Gm3cV/vJH+IFUWazWHGnf80xDi8EDJLRSWx7jcxObeHOmEQ3FURyZKym7DSSCgS4nyjJlc Zw8U45FVrd13+126ftHnOwGHQPPjvhvAZNfsmOLZcyK9UWDtpdGd8ZgrSv4TcxlNN2n9Cp epk3KRe26/d0LrLHqtATBJqz80WrBDA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-526-S8fjxP9IOx-QDhe4h6l3MA-1; Thu, 04 Feb 2021 08:26:16 -0500 X-MC-Unique: S8fjxP9IOx-QDhe4h6l3MA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E0C3D19611A1 for ; Thu, 4 Feb 2021 13:26:15 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 39D8A17F72 for ; Thu, 4 Feb 2021 13:26:15 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:26:13 +0100 Message-Id: <20210204132610.9958.16465.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 08/10] lflow-cache: Reclaim heap memory after cache flush. 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" If possible, automatically reclaim heap memory when the lflow cache is flushed. This can be an expensive operation but cache flushing is not a very common operation. This change is inspired by Ilya Maximets' OVS commit: f38f98a2c0dd ("ovsdb-server: Reclaim heap memory after compaction.") Additionally, when flushing the cache, also shrink the backing hmap. Signed-off-by: Dumitru Ceara Acked-by: Numan Siddique --- configure.ac | 1 + controller/lflow-cache.c | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/configure.ac b/configure.ac index b2d0843..ebb09a5 100644 --- a/configure.ac +++ b/configure.ac @@ -96,6 +96,7 @@ OVN_CHECK_DOT OVS_CHECK_IF_DL OVS_CHECK_STRTOK_R AC_CHECK_DECLS([sys_siglist], [], [], [[#include ]]) +AC_CHECK_DECLS([malloc_trim], [], [], [[#include ]]) AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec], [], [], [[#include ]]) AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include ]]) diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index 1549034..2453b10 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -17,6 +17,10 @@ #include +#if HAVE_DECL_MALLOC_TRIM +#include +#endif + #include "coverage.h" #include "lib/ovn-sb-idl.h" #include "lflow-cache.h" @@ -87,7 +91,12 @@ lflow_cache_flush(struct lflow_cache *lc) HMAP_FOR_EACH_SAFE (lce, lce_next, node, &lc->entries[i]) { lflow_cache_delete__(lc, lce); } + hmap_shrink(&lc->entries[i]); } + +#if HAVE_DECL_MALLOC_TRIM + malloc_trim(0); +#endif } void From patchwork Thu Feb 4 13:26:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436010 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.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EgnkyXu9; dkim-atps=neutral Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfTp2r6cz9sVq for ; Fri, 5 Feb 2021 00:26:54 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id D282486C12; Thu, 4 Feb 2021 13:26:52 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id VwaLyE2951gK; Thu, 4 Feb 2021 13:26:44 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 0893486C22; Thu, 4 Feb 2021 13:26:42 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C6B14C013A; Thu, 4 Feb 2021 13:26:41 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 09ABFC07FD for ; Thu, 4 Feb 2021 13:26:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id DDF7C86C19 for ; Thu, 4 Feb 2021 13:26:40 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0v4wJJrhlsoD for ; Thu, 4 Feb 2021 13:26:32 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by whitealder.osuosl.org (Postfix) with ESMTPS id 9F7C086C12 for ; Thu, 4 Feb 2021 13:26:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445190; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=67yyYrwUyEyXaINrZKygsCTOtPhGsafUaUb6kcroS+E=; b=EgnkyXu9K3CR0phwQD3cmPZJNXS49lMfIAcnmr3Ybe6Aca11xIRMCcgsj/Hb4jGMKth67M ZRzWp3hs4j+7sSRDQF+qaVfrvSzMl4+nZdQhFrgMrtPMZCbN77/9K0ngbQN5G7Msczry06 giVvHRXHq21SDUpMzPIk3nZfWMslZac= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-402-jQtIh2F8MCaMyRsvuPFxIw-1; Thu, 04 Feb 2021 08:26:28 -0500 X-MC-Unique: jQtIh2F8MCaMyRsvuPFxIw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 46672107ACE3 for ; Thu, 4 Feb 2021 13:26:27 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6B49C60C5F for ; Thu, 4 Feb 2021 13:26:26 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:26:24 +0100 Message-Id: <20210204132621.9958.27237.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 09/10] lflow-cache: Make maximum number of cache entries configurable. 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" Add a new OVS external-id, "ovn-limit-lflow-cache", through which users can specify the maximum size of the ovn-controller logical flow cache. To maintain backwards compatibility the default behavior is to not enforce any limit on the size of the cache. When the cache becomes full, the rule is to prefer more "important" cache entries over less "important" ones. That is, evict entries of type LCACHE_T_CONJ_ID if there's no room to add an entry of type LCACHE_T_EXPR. Similarly, evict entries of type LCACHE_T_CONJ_ID or LCACHE_T_EXPR if there's no room to add an entry of type LCACHE_T_MATCHES. Signed-off-by: Dumitru Ceara Acked-by: Numan Siddique --- NEWS | 3 + controller/chassis.c | 23 ++++++++- controller/lflow-cache.c | 48 +++++++++++++++++- controller/lflow-cache.h | 2 - controller/ovn-controller.8.xml | 16 ++++++ controller/ovn-controller.c | 8 +++ controller/test-lflow-cache.c | 12 +++-- tests/ovn-lflow-cache.at | 104 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 206 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 2044671..6e10557 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,9 @@ Post-v20.12.0 - Add a new option to Load_Balancer.options, "hairpin_snat_ip", to allow users to explicitly select which source IP should be used for load balancer hairpin traffic. + - ovn-controller: Add a configuration knob, through OVS external-id + "ovn-limit-lflow-cache", to allow enforcing a limit for the size of the + logical flow cache. OVN v20.12.0 - 18 Dec 2020 -------------------------- diff --git a/controller/chassis.c b/controller/chassis.c index b4d4b0e..c66837a 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -49,6 +49,7 @@ struct ovs_chassis_cfg { const char *monitor_all; const char *chassis_macs; const char *enable_lflow_cache; + const char *limit_lflow_cache; /* Set of encap types parsed from the 'ovn-encap-type' external-id. */ struct sset encap_type_set; @@ -135,6 +136,12 @@ get_enable_lflow_cache(const struct smap *ext_ids) } static const char * +get_limit_lflow_cache(const struct smap *ext_ids) +{ + return smap_get_def(ext_ids, "ovn-limit-lflow-cache", ""); +} + +static const char * get_encap_csum(const struct smap *ext_ids) { return smap_get_def(ext_ids, "ovn-encap-csum", "true"); @@ -256,6 +263,7 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table, ovs_cfg->monitor_all = get_monitor_all(&cfg->external_ids); ovs_cfg->chassis_macs = get_chassis_mac_mappings(&cfg->external_ids); ovs_cfg->enable_lflow_cache = get_enable_lflow_cache(&cfg->external_ids); + ovs_cfg->limit_lflow_cache = get_limit_lflow_cache(&cfg->external_ids); if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) { return false; @@ -283,13 +291,16 @@ chassis_build_other_config(struct smap *config, const char *bridge_mappings, const char *datapath_type, const char *cms_options, const char *monitor_all, const char *chassis_macs, const char *iface_types, - const char *enable_lflow_cache, bool is_interconn) + const char *enable_lflow_cache, + const char *limit_lflow_cache, + bool is_interconn) { smap_replace(config, "ovn-bridge-mappings", bridge_mappings); smap_replace(config, "datapath-type", datapath_type); smap_replace(config, "ovn-cms-options", cms_options); smap_replace(config, "ovn-monitor-all", monitor_all); smap_replace(config, "ovn-enable-lflow-cache", enable_lflow_cache); + smap_replace(config, "ovn-limit-lflow-cache", limit_lflow_cache); smap_replace(config, "iface-types", iface_types); smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs); smap_replace(config, "is-interconn", is_interconn ? "true" : "false"); @@ -305,6 +316,7 @@ chassis_other_config_changed(const char *bridge_mappings, const char *monitor_all, const char *chassis_macs, const char *enable_lflow_cache, + const char *limit_lflow_cache, const struct ds *iface_types, bool is_interconn, const struct sbrec_chassis *chassis_rec) @@ -344,6 +356,13 @@ chassis_other_config_changed(const char *bridge_mappings, return true; } + const char *chassis_limit_lflow_cache = + get_limit_lflow_cache(&chassis_rec->other_config); + + if (strcmp(limit_lflow_cache, chassis_limit_lflow_cache)) { + return true; + } + const char *chassis_mac_mappings = get_chassis_mac_mappings(&chassis_rec->other_config); if (strcmp(chassis_macs, chassis_mac_mappings)) { @@ -523,6 +542,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec, ovs_cfg->monitor_all, ovs_cfg->chassis_macs, ovs_cfg->enable_lflow_cache, + ovs_cfg->limit_lflow_cache, &ovs_cfg->iface_types, ovs_cfg->is_interconn, chassis_rec)) { @@ -536,6 +556,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec, ovs_cfg->chassis_macs, ds_cstr_ro(&ovs_cfg->iface_types), ovs_cfg->enable_lflow_cache, + ovs_cfg->limit_lflow_cache, ovs_cfg->is_interconn); sbrec_chassis_verify_other_config(chassis_rec); sbrec_chassis_set_other_config(chassis_rec, &other_config); diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index 2453b10..342b20a 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -37,6 +37,8 @@ COVERAGE_DEFINE(lflow_cache_add); COVERAGE_DEFINE(lflow_cache_hit); COVERAGE_DEFINE(lflow_cache_miss); COVERAGE_DEFINE(lflow_cache_delete); +COVERAGE_DEFINE(lflow_cache_full); +COVERAGE_DEFINE(lflow_cache_made_room); const char *lflow_cache_type_names[LCACHE_T_MAX] = { [LCACHE_T_CONJ_ID] = "cache-conj-id", @@ -46,6 +48,7 @@ const char *lflow_cache_type_names[LCACHE_T_MAX] = { struct lflow_cache { struct hmap entries[LCACHE_T_MAX]; + uint32_t capacity; bool enabled; }; @@ -56,6 +59,9 @@ struct lflow_cache_entry { struct lflow_cache_value value; }; +static size_t lflow_cache_n_entries__(const struct lflow_cache *lc); +static bool lflow_cache_make_room__(struct lflow_cache *lc, + enum lflow_cache_type type); static struct lflow_cache_value *lflow_cache_add__( struct lflow_cache *lc, const struct sbrec_logical_flow *lflow, @@ -114,16 +120,18 @@ lflow_cache_destroy(struct lflow_cache *lc) } void -lflow_cache_enable(struct lflow_cache *lc, bool enabled) +lflow_cache_enable(struct lflow_cache *lc, bool enabled, uint32_t capacity) { if (!lc) { return; } - if (lc->enabled && !enabled) { + if ((lc->enabled && !enabled) || capacity < lflow_cache_n_entries__(lc)) { lflow_cache_flush(lc); } + lc->enabled = enabled; + lc->capacity = capacity; } bool @@ -236,6 +244,33 @@ lflow_cache_delete(struct lflow_cache *lc, } } +static size_t +lflow_cache_n_entries__(const struct lflow_cache *lc) +{ + size_t n_entries = 0; + + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + n_entries += hmap_count(&lc->entries[i]); + } + return n_entries; +} + +static bool +lflow_cache_make_room__(struct lflow_cache *lc, enum lflow_cache_type type) +{ + for (size_t i = 0; i < type; i++) { + if (hmap_count(&lc->entries[i]) > 0) { + struct lflow_cache_entry *lce = + CONTAINER_OF(hmap_first(&lc->entries[i]), + struct lflow_cache_entry, node); + + lflow_cache_delete__(lc, lce); + return true; + } + } + return false; +} + static struct lflow_cache_value * lflow_cache_add__(struct lflow_cache *lc, const struct sbrec_logical_flow *lflow, @@ -245,6 +280,15 @@ lflow_cache_add__(struct lflow_cache *lc, return NULL; } + if (lflow_cache_n_entries__(lc) == lc->capacity) { + if (!lflow_cache_make_room__(lc, type)) { + COVERAGE_INC(lflow_cache_full); + return NULL; + } else { + COVERAGE_INC(lflow_cache_made_room); + } + } + struct lflow_cache_entry *lce = xzalloc(sizeof *lce); COVERAGE_INC(lflow_cache_add); diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h index 54873ee..23c64a0 100644 --- a/controller/lflow-cache.h +++ b/controller/lflow-cache.h @@ -61,7 +61,7 @@ struct lflow_cache_stats { struct lflow_cache *lflow_cache_create(void); void lflow_cache_flush(struct lflow_cache *); void lflow_cache_destroy(struct lflow_cache *); -void lflow_cache_enable(struct lflow_cache *, bool enabled); +void lflow_cache_enable(struct lflow_cache *, bool enabled, uint32_t capacity); bool lflow_cache_is_enabled(struct lflow_cache *); struct lflow_cache_stats *lflow_cache_get_stats(const struct lflow_cache *); diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml index 29833c7..e92db6d 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -249,6 +249,22 @@ processing the southbound and local Open vSwitch database changes. The default value is considered false if this option is not defined. + +
external_ids:ovn-enable-lflow-cache
+
+ The boolean flag indicates if ovn-controller should + enable/disable the logical flow in-memory cache it uses when + processing Southbound database logical flow changes. By default + caching is enabled. +
+ +
external_ids:ovn-limit-lflow-cache
+
+ When used, this configuration value determines the maximum number of + logical flow cache entries ovn-controller may create + when the logical flow cache is enabled. By default the size of the + cache is unlimited. +

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6ee6119..dd67d7f 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -96,6 +96,9 @@ static unixctl_cb_func debug_delay_nb_cfg_report; static char *parse_options(int argc, char *argv[]); OVS_NO_RETURN static void usage(void); +/* By default don't set an upper bound for the lflow cache. */ +#define DEFAULT_LFLOW_CACHE_MAX_ENTRIES UINT32_MAX + struct controller_engine_ctx { struct lflow_cache *lflow_cache; }; @@ -582,7 +585,10 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, lflow_cache_enable(ctx->lflow_cache, smap_get_bool(&cfg->external_ids, "ovn-enable-lflow-cache", - true)); + true), + smap_get_uint(&cfg->external_ids, + "ovn-limit-lflow-cache", + DEFAULT_LFLOW_CACHE_MAX_ENTRIES)); } } diff --git a/controller/test-lflow-cache.c b/controller/test-lflow-cache.c index d6e962e..45262b4 100644 --- a/controller/test-lflow-cache.c +++ b/controller/test-lflow-cache.c @@ -108,7 +108,7 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx) unsigned int shift = 2; unsigned int n_ops; - lflow_cache_enable(lc, enabled); + lflow_cache_enable(lc, enabled, UINT32_MAX); test_lflow_cache_stats__(lc); if (!test_read_uint_value(ctx, shift++, "n_ops", &n_ops)) { @@ -156,11 +156,15 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx) test_lflow_cache_delete__(lc, &lflow); test_lflow_cache_lookup__(lc, &lflow); } else if (!strcmp(op, "enable")) { + unsigned int limit; + if (!test_read_uint_value(ctx, shift++, "limit", &limit)) { + goto done; + } printf("ENABLE\n"); - lflow_cache_enable(lc, true); + lflow_cache_enable(lc, true, limit); } else if (!strcmp(op, "disable")) { printf("DISABLE\n"); - lflow_cache_enable(lc, false); + lflow_cache_enable(lc, false, UINT32_MAX); } else if (!strcmp(op, "flush")) { printf("FLUSH\n"); lflow_cache_flush(lc); @@ -179,7 +183,7 @@ test_lflow_cache_negative(struct ovs_cmdl_context *ctx OVS_UNUSED) { lflow_cache_flush(NULL); lflow_cache_destroy(NULL); - lflow_cache_enable(NULL, true); + lflow_cache_enable(NULL, true, UINT32_MAX); ovs_assert(!lflow_cache_is_enabled(NULL)); ovs_assert(!lflow_cache_get_stats(NULL)); diff --git a/tests/ovn-lflow-cache.at b/tests/ovn-lflow-cache.at index f7e8959..2cbc1f6 100644 --- a/tests/ovn-lflow-cache.at +++ b/tests/ovn-lflow-cache.at @@ -146,7 +146,7 @@ AT_CHECK( add conj-id 4 \ add expr 5 \ add matches 6 \ - enable \ + enable 1000 \ add conj-id 7 \ add expr 8 \ add matches 9 \ @@ -252,6 +252,108 @@ Enabled: true ]) AT_CLEANUP +AT_SETUP([ovn -- unit test -- lflow-cache set limit]) +AT_CHECK( + [ovstest test-lflow-cache lflow_cache_operations \ + true 8 \ + add conj-id 1 \ + add expr 2 \ + add matches 3 \ + enable 1 \ + add conj-id 4 \ + add expr 5 \ + add matches 6 \ + add conj-id 7], + [0], [dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 1 +LOOKUP: + conj_id_ofs: 1 + type: conj-id +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 2 +LOOKUP: + conj_id_ofs: 2 + type: expr +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 0 +ADD matches: + conj-id-ofs: 3 +LOOKUP: + conj_id_ofs: 0 + type: matches +Enabled: true + cache-conj-id: 1 + cache-expr: 1 + cache-matches: 1 +ENABLE +dnl +dnl Max capacity smaller than current usage, cache should be flushed. +dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 4 +LOOKUP: + conj_id_ofs: 4 + type: conj-id +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 5 +LOOKUP: + conj_id_ofs: 5 + type: expr +dnl +dnl Cache is full but we can evict the conj-id entry because we're adding +dnl an expr one. +dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 1 + cache-matches: 0 +ADD matches: + conj-id-ofs: 6 +LOOKUP: + conj_id_ofs: 0 + type: matches +dnl +dnl Cache is full but we can evict the expr entry because we're adding +dnl a matches one. +dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 1 +ADD conj-id: + conj-id-ofs: 7 +LOOKUP: + not found +dnl +dnl Cache is full and we're adding a conj-id entry so we shouldn't evict +dnl anything else. +dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 1 +]) +AT_CLEANUP + AT_SETUP([ovn -- unit test -- lflow-cache negative tests]) AT_CHECK([ovstest test-lflow-cache lflow_cache_negative], [0], []) AT_CLEANUP From patchwork Thu Feb 4 13:26:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1436013 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=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KdaDSzpD; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DWfWZ5n0Fz9sWP for ; Fri, 5 Feb 2021 00:28:26 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id F17142D47F; Thu, 4 Feb 2021 13:28:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bPD3BxITiplb; Thu, 4 Feb 2021 13:27:58 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id E8D5F2754C; Thu, 4 Feb 2021 13:26:52 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D823EC1834; Thu, 4 Feb 2021 13:26:52 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id CEEAAC013A for ; Thu, 4 Feb 2021 13:26:51 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id BA97886239 for ; Thu, 4 Feb 2021 13:26:51 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id O5a47Ic1BPuO for ; Thu, 4 Feb 2021 13:26:44 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 5022286283 for ; Thu, 4 Feb 2021 13:26:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612445200; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6RT6m7RUZS5wdQxVY60YsdHzZ+aJFA7rhvnZ1PUes38=; b=KdaDSzpDTG82cLAzJ6/bKxBU5/5fdHY9L0zr2NnYzBpBPqleycjQWc/b9zTyJuSKMKyctV cliHcTDow1q1wOsCgizNql7z4jHuODa35DUgS6anPnoMErYOyK3gV+SdcJgB+UMUq1aGtk /ucAtlqfB7BcKKgSuJEcWdYPmb2uh0s= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-508-UNAethaqN0-dNSAS7bFH1A-1; Thu, 04 Feb 2021 08:26:38 -0500 X-MC-Unique: UNAethaqN0-dNSAS7bFH1A-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DE7CBC73A1 for ; Thu, 4 Feb 2021 13:26:37 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-248.ams2.redhat.com [10.36.114.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id E4DD35D9C9 for ; Thu, 4 Feb 2021 13:26:36 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 4 Feb 2021 14:26:34 +0100 Message-Id: <20210204132632.9958.98982.stgit@dceara.remote.csb> In-Reply-To: <20210204132443.9958.96293.stgit@dceara.remote.csb> References: <20210204132443.9958.96293.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 10/10] lflow-cache: Make max cache memory usage configurable. 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" Add a new OVS external-id, "ovn-memlimit-lflow-cache-kb", through which users can specify the maximum amount of memory (in KB) ovn-controller can use for caching logical flows. To maintain backwards compatibility the default behavior is to not enforce any memory limit on the size of the cache. Add lflow cache reports to "ovn-appctl -t ovn-controller memory/show". The memory usage for cache entries of type LCACHE_T_EXPR is computed by doing another pass through the expression tree. While this adds a few extra cycles, entries of type LCACHE_T_EXPR shouldn't be very common because they are created only for flows with matches that include "is_chassis_resident()" but do not reference port groups and address sets. Signed-off-by: Dumitru Ceara --- NEWS | 8 +++-- controller/chassis.c | 21 +++++++++++++ controller/lflow-cache.c | 63 ++++++++++++++++++++++++++++++--------- controller/lflow-cache.h | 13 ++++++-- controller/lflow.c | 8 +++-- controller/ovn-controller.8.xml | 7 ++++ controller/ovn-controller.c | 8 ++++- controller/test-lflow-cache.c | 31 +++++++++++++------ include/ovn/expr.h | 3 +- lib/expr.c | 39 ++++++++++++++++++++++++ tests/ovn-lflow-cache.at | 54 +++++++++++++++++++++++++++++++-- 11 files changed, 213 insertions(+), 42 deletions(-) diff --git a/NEWS b/NEWS index 6e10557..ddb5977 100644 --- a/NEWS +++ b/NEWS @@ -16,9 +16,11 @@ Post-v20.12.0 - Add a new option to Load_Balancer.options, "hairpin_snat_ip", to allow users to explicitly select which source IP should be used for load balancer hairpin traffic. - - ovn-controller: Add a configuration knob, through OVS external-id - "ovn-limit-lflow-cache", to allow enforcing a limit for the size of the - logical flow cache. + - ovn-controller: Add configuration knobs, through OVS external-id + "ovn-limit-lflow-cache" and "ovn-memlimit-lflow-cache-kb", to allow + enforcing a limit for the size of the logical flow cache based on + maximum number of entries and/or memory usage. + - ovn-controller: Add lflow cache related memory reports. OVN v20.12.0 - 18 Dec 2020 -------------------------- diff --git a/controller/chassis.c b/controller/chassis.c index c66837a..a4a264c 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -50,6 +50,7 @@ struct ovs_chassis_cfg { const char *chassis_macs; const char *enable_lflow_cache; const char *limit_lflow_cache; + const char *memlimit_lflow_cache; /* Set of encap types parsed from the 'ovn-encap-type' external-id. */ struct sset encap_type_set; @@ -142,6 +143,12 @@ get_limit_lflow_cache(const struct smap *ext_ids) } static const char * +get_memlimit_lflow_cache(const struct smap *ext_ids) +{ + return smap_get_def(ext_ids, "ovn-memlimit-lflow-cache-kb", ""); +} + +static const char * get_encap_csum(const struct smap *ext_ids) { return smap_get_def(ext_ids, "ovn-encap-csum", "true"); @@ -264,6 +271,8 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table, ovs_cfg->chassis_macs = get_chassis_mac_mappings(&cfg->external_ids); ovs_cfg->enable_lflow_cache = get_enable_lflow_cache(&cfg->external_ids); ovs_cfg->limit_lflow_cache = get_limit_lflow_cache(&cfg->external_ids); + ovs_cfg->memlimit_lflow_cache = + get_memlimit_lflow_cache(&cfg->external_ids); if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) { return false; @@ -293,6 +302,7 @@ chassis_build_other_config(struct smap *config, const char *bridge_mappings, const char *iface_types, const char *enable_lflow_cache, const char *limit_lflow_cache, + const char *memlimit_lflow_cache, bool is_interconn) { smap_replace(config, "ovn-bridge-mappings", bridge_mappings); @@ -301,6 +311,7 @@ chassis_build_other_config(struct smap *config, const char *bridge_mappings, smap_replace(config, "ovn-monitor-all", monitor_all); smap_replace(config, "ovn-enable-lflow-cache", enable_lflow_cache); smap_replace(config, "ovn-limit-lflow-cache", limit_lflow_cache); + smap_replace(config, "ovn-memlimit-lflow-cache-kb", memlimit_lflow_cache); smap_replace(config, "iface-types", iface_types); smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs); smap_replace(config, "is-interconn", is_interconn ? "true" : "false"); @@ -317,6 +328,7 @@ chassis_other_config_changed(const char *bridge_mappings, const char *chassis_macs, const char *enable_lflow_cache, const char *limit_lflow_cache, + const char *memlimit_lflow_cache, const struct ds *iface_types, bool is_interconn, const struct sbrec_chassis *chassis_rec) @@ -363,6 +375,13 @@ chassis_other_config_changed(const char *bridge_mappings, return true; } + const char *chassis_memlimit_lflow_cache = + get_memlimit_lflow_cache(&chassis_rec->other_config); + + if (strcmp(memlimit_lflow_cache, chassis_memlimit_lflow_cache)) { + return true; + } + const char *chassis_mac_mappings = get_chassis_mac_mappings(&chassis_rec->other_config); if (strcmp(chassis_macs, chassis_mac_mappings)) { @@ -543,6 +562,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec, ovs_cfg->chassis_macs, ovs_cfg->enable_lflow_cache, ovs_cfg->limit_lflow_cache, + ovs_cfg->memlimit_lflow_cache, &ovs_cfg->iface_types, ovs_cfg->is_interconn, chassis_rec)) { @@ -557,6 +577,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec, ds_cstr_ro(&ovs_cfg->iface_types), ovs_cfg->enable_lflow_cache, ovs_cfg->limit_lflow_cache, + ovs_cfg->memlimit_lflow_cache, ovs_cfg->is_interconn); sbrec_chassis_verify_other_config(chassis_rec); sbrec_chassis_set_other_config(chassis_rec, &other_config); diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index 342b20a..60a8389 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -38,6 +38,7 @@ COVERAGE_DEFINE(lflow_cache_hit); COVERAGE_DEFINE(lflow_cache_miss); COVERAGE_DEFINE(lflow_cache_delete); COVERAGE_DEFINE(lflow_cache_full); +COVERAGE_DEFINE(lflow_cache_mem_full); COVERAGE_DEFINE(lflow_cache_made_room); const char *lflow_cache_type_names[LCACHE_T_MAX] = { @@ -49,12 +50,15 @@ const char *lflow_cache_type_names[LCACHE_T_MAX] = { struct lflow_cache { struct hmap entries[LCACHE_T_MAX]; uint32_t capacity; + uint64_t mem_usage; + uint64_t max_mem_usage; bool enabled; }; struct lflow_cache_entry { struct hmap_node node; struct uuid lflow_uuid; /* key */ + size_t size; struct lflow_cache_value value; }; @@ -65,7 +69,8 @@ static bool lflow_cache_make_room__(struct lflow_cache *lc, static struct lflow_cache_value *lflow_cache_add__( struct lflow_cache *lc, const struct sbrec_logical_flow *lflow, - enum lflow_cache_type type); + enum lflow_cache_type type, + uint64_t value_size); static void lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce); @@ -79,6 +84,7 @@ lflow_cache_create(void) } lc->enabled = true; + lc->mem_usage = 0; return lc; } @@ -120,18 +126,24 @@ lflow_cache_destroy(struct lflow_cache *lc) } void -lflow_cache_enable(struct lflow_cache *lc, bool enabled, uint32_t capacity) +lflow_cache_enable(struct lflow_cache *lc, bool enabled, uint32_t capacity, + uint64_t max_mem_usage_kb) { if (!lc) { return; } - if ((lc->enabled && !enabled) || capacity < lflow_cache_n_entries__(lc)) { + uint64_t max_mem_usage = max_mem_usage_kb * 1024; + + if ((lc->enabled && !enabled) + || capacity < lflow_cache_n_entries__(lc) + || max_mem_usage < lc->mem_usage) { lflow_cache_flush(lc); } lc->enabled = enabled; lc->capacity = capacity; + lc->max_mem_usage = max_mem_usage; } bool @@ -161,8 +173,7 @@ lflow_cache_add_conj_id(struct lflow_cache *lc, uint32_t conj_id_ofs) { struct lflow_cache_value *lcv = - lflow_cache_add__(lc, lflow, LCACHE_T_CONJ_ID); - + lflow_cache_add__(lc, lflow, LCACHE_T_CONJ_ID, 0); if (!lcv) { return; } @@ -173,12 +184,11 @@ lflow_cache_add_conj_id(struct lflow_cache *lc, void lflow_cache_add_expr(struct lflow_cache *lc, const struct sbrec_logical_flow *lflow, - uint32_t conj_id_ofs, - struct expr *expr) + uint32_t conj_id_ofs, struct expr *expr, + size_t expr_sz) { struct lflow_cache_value *lcv = - lflow_cache_add__(lc, lflow, LCACHE_T_EXPR); - + lflow_cache_add__(lc, lflow, LCACHE_T_EXPR, expr_sz); if (!lcv) { expr_destroy(expr); return; @@ -191,11 +201,10 @@ lflow_cache_add_expr(struct lflow_cache *lc, void lflow_cache_add_matches(struct lflow_cache *lc, const struct sbrec_logical_flow *lflow, - struct hmap *matches) + struct hmap *matches, size_t matches_sz) { struct lflow_cache_value *lcv = - lflow_cache_add__(lc, lflow, LCACHE_T_MATCHES); - + lflow_cache_add__(lc, lflow, LCACHE_T_MATCHES, matches_sz); if (!lcv) { expr_matches_destroy(matches); free(matches); @@ -271,15 +280,36 @@ lflow_cache_make_room__(struct lflow_cache *lc, enum lflow_cache_type type) return false; } +void +lflow_cache_get_memory_usage(const struct lflow_cache *lc, struct simap *usage) +{ + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + char *counter_name = xasprintf("lflow-cache-entries-%s", + lflow_cache_type_names[i]); + simap_increase(usage, counter_name, hmap_count(&lc->entries[i])); + free(counter_name); + } + simap_increase(usage, "lflow-cache-size-KB", + ROUND_UP(lc->mem_usage, 1024) / 1024); +} + static struct lflow_cache_value * lflow_cache_add__(struct lflow_cache *lc, const struct sbrec_logical_flow *lflow, - enum lflow_cache_type type) + enum lflow_cache_type type, + uint64_t value_size) { if (!lflow_cache_is_enabled(lc) || !lflow) { return NULL; } + struct lflow_cache_entry *lce; + size_t size = sizeof *lce + value_size; + if (size + lc->mem_usage > lc->max_mem_usage) { + COVERAGE_INC(lflow_cache_mem_full); + return NULL; + } + if (lflow_cache_n_entries__(lc) == lc->capacity) { if (!lflow_cache_make_room__(lc, type)) { COVERAGE_INC(lflow_cache_full); @@ -289,10 +319,12 @@ lflow_cache_add__(struct lflow_cache *lc, } } - struct lflow_cache_entry *lce = xzalloc(sizeof *lce); + lc->mem_usage += size; COVERAGE_INC(lflow_cache_add); + lce = xzalloc(sizeof *lce); lce->lflow_uuid = lflow->header_.uuid; + lce->size = size; lce->value.type = type; hmap_insert(&lc->entries[type], &lce->node, uuid_hash(&lflow->header_.uuid)); @@ -324,5 +356,8 @@ lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce) free(lce->value.expr_matches); break; } + + ovs_assert(lc->mem_usage >= lce->size); + lc->mem_usage -= lce->size; free(lce); } diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h index 23c64a0..411eb35 100644 --- a/controller/lflow-cache.h +++ b/controller/lflow-cache.h @@ -20,6 +20,7 @@ #include "openvswitch/hmap.h" #include "openvswitch/uuid.h" +#include "simap.h" struct sbrec_logical_flow; struct lflow_cache; @@ -61,7 +62,8 @@ struct lflow_cache_stats { struct lflow_cache *lflow_cache_create(void); void lflow_cache_flush(struct lflow_cache *); void lflow_cache_destroy(struct lflow_cache *); -void lflow_cache_enable(struct lflow_cache *, bool enabled, uint32_t capacity); +void lflow_cache_enable(struct lflow_cache *, bool enabled, uint32_t capacity, + uint64_t max_mem_usage_kb); bool lflow_cache_is_enabled(struct lflow_cache *); struct lflow_cache_stats *lflow_cache_get_stats(const struct lflow_cache *); @@ -70,15 +72,18 @@ void lflow_cache_add_conj_id(struct lflow_cache *, uint32_t conj_id_ofs); void lflow_cache_add_expr(struct lflow_cache *, const struct sbrec_logical_flow *, - uint32_t conj_id_ofs, - struct expr *expr); + uint32_t conj_id_ofs, struct expr *expr, + size_t expr_sz); void lflow_cache_add_matches(struct lflow_cache *, const struct sbrec_logical_flow *, - struct hmap *matches); + struct hmap *matches, size_t matches_sz); struct lflow_cache_value *lflow_cache_get(struct lflow_cache *, const struct sbrec_logical_flow *); void lflow_cache_delete(struct lflow_cache *, const struct sbrec_logical_flow *); +void lflow_cache_get_memory_usage(const struct lflow_cache *, + struct simap *usage); + #endif /* controller/lflow-cache.h */ diff --git a/controller/lflow.c b/controller/lflow.c index c493652..a8fa052 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -797,6 +797,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, struct expr *cached_expr = NULL, *expr = NULL; struct hmap *matches = NULL; + size_t matches_size = 0; bool is_cr_cond_present = false; bool pg_addr_set_ref = false; @@ -851,7 +852,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, case LCACHE_T_EXPR: matches = xmalloc(sizeof *matches); n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, matches); - expr_matches_prepare(matches, conj_id_ofs); + matches_size = expr_matches_prepare(matches, conj_id_ofs); if (hmap_is_empty(matches)) { VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", UUID_ARGS(&lflow->header_.uuid)); @@ -878,11 +879,12 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, if (lflow_cache_is_enabled(l_ctx_out->lflow_cache)) { if (cached_expr && !is_cr_cond_present) { lflow_cache_add_matches(l_ctx_out->lflow_cache, lflow, - matches); + matches, matches_size); matches = NULL; } else if (cached_expr) { lflow_cache_add_expr(l_ctx_out->lflow_cache, lflow, - conj_id_ofs, cached_expr); + conj_id_ofs, cached_expr, + expr_size(cached_expr)); cached_expr = NULL; } else if (n_conjs) { lflow_cache_add_conj_id(l_ctx_out->lflow_cache, lflow, diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml index e92db6d..75c6596 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -265,6 +265,13 @@ when the logical flow cache is enabled. By default the size of the cache is unlimited. +

external_ids:ovn-memlimit-lflow-cache-kb
+
+ When used, this configuration value determines the maximum size of + the logical flow cache (in KB) ovn-controller may create + when the logical flow cache is enabled. By default the size of the + cache is unlimited. +

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index dd67d7f..138d95a 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -98,6 +98,7 @@ OVS_NO_RETURN static void usage(void); /* By default don't set an upper bound for the lflow cache. */ #define DEFAULT_LFLOW_CACHE_MAX_ENTRIES UINT32_MAX +#define DEFAULT_LFLOW_CACHE_MAX_MEM_KB (UINT64_MAX / 1024) struct controller_engine_ctx { struct lflow_cache *lflow_cache; @@ -588,7 +589,10 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, true), smap_get_uint(&cfg->external_ids, "ovn-limit-lflow-cache", - DEFAULT_LFLOW_CACHE_MAX_ENTRIES)); + DEFAULT_LFLOW_CACHE_MAX_ENTRIES), + smap_get_ullong(&cfg->external_ids, + "ovn-memlimit-lflow-cache-kb", + DEFAULT_LFLOW_CACHE_MAX_MEM_KB)); } } @@ -2710,7 +2714,7 @@ main(int argc, char *argv[]) if (memory_should_report()) { struct simap usage = SIMAP_INITIALIZER(&usage); - /* Nothing special to report yet. */ + lflow_cache_get_memory_usage(ctrl_engine_ctx.lflow_cache, &usage); memory_report(&usage); simap_destroy(&usage); } diff --git a/controller/test-lflow-cache.c b/controller/test-lflow-cache.c index 45262b4..ef5427d 100644 --- a/controller/test-lflow-cache.c +++ b/controller/test-lflow-cache.c @@ -23,6 +23,9 @@ #include "lflow-cache.h" +/* Simulate 1KB large cache values. */ +#define TEST_LFLOW_CACHE_VALUE_SIZE 1024 + static void test_lflow_cache_add__(struct lflow_cache *lc, const char *op_type, const struct sbrec_logical_flow *lflow, @@ -35,12 +38,14 @@ test_lflow_cache_add__(struct lflow_cache *lc, const char *op_type, if (!strcmp(op_type, "conj-id")) { lflow_cache_add_conj_id(lc, lflow, conj_id_ofs); } else if (!strcmp(op_type, "expr")) { - lflow_cache_add_expr(lc, lflow, conj_id_ofs, expr_clone(e)); + lflow_cache_add_expr(lc, lflow, conj_id_ofs, expr_clone(e), + TEST_LFLOW_CACHE_VALUE_SIZE); } else if (!strcmp(op_type, "matches")) { struct hmap *matches = xmalloc(sizeof *matches); ovs_assert(expr_to_matches(e, NULL, NULL, matches) == 0); ovs_assert(hmap_count(matches) == 1); - lflow_cache_add_matches(lc, lflow, matches); + lflow_cache_add_matches(lc, lflow, matches, + TEST_LFLOW_CACHE_VALUE_SIZE); } else { OVS_NOT_REACHED(); } @@ -108,7 +113,7 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx) unsigned int shift = 2; unsigned int n_ops; - lflow_cache_enable(lc, enabled, UINT32_MAX); + lflow_cache_enable(lc, enabled, UINT32_MAX, UINT32_MAX); test_lflow_cache_stats__(lc); if (!test_read_uint_value(ctx, shift++, "n_ops", &n_ops)) { @@ -157,14 +162,19 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx) test_lflow_cache_lookup__(lc, &lflow); } else if (!strcmp(op, "enable")) { unsigned int limit; + unsigned int mem_limit_kb; if (!test_read_uint_value(ctx, shift++, "limit", &limit)) { goto done; } + if (!test_read_uint_value(ctx, shift++, "mem-limit", + &mem_limit_kb)) { + goto done; + } printf("ENABLE\n"); - lflow_cache_enable(lc, true, limit); + lflow_cache_enable(lc, true, limit, mem_limit_kb); } else if (!strcmp(op, "disable")) { printf("DISABLE\n"); - lflow_cache_enable(lc, false, UINT32_MAX); + lflow_cache_enable(lc, false, UINT32_MAX, UINT32_MAX); } else if (!strcmp(op, "flush")) { printf("FLUSH\n"); lflow_cache_flush(lc); @@ -183,7 +193,7 @@ test_lflow_cache_negative(struct ovs_cmdl_context *ctx OVS_UNUSED) { lflow_cache_flush(NULL); lflow_cache_destroy(NULL); - lflow_cache_enable(NULL, true, UINT32_MAX); + lflow_cache_enable(NULL, true, UINT32_MAX, UINT32_MAX); ovs_assert(!lflow_cache_is_enabled(NULL)); ovs_assert(!lflow_cache_get_stats(NULL)); @@ -200,10 +210,11 @@ test_lflow_cache_negative(struct ovs_cmdl_context *ctx OVS_UNUSED) ovs_assert(hmap_count(matches) == 1); lflow_cache_add_conj_id(lcs[i], NULL, 0); - lflow_cache_add_expr(lcs[i], NULL, 0, NULL); - lflow_cache_add_expr(lcs[i], NULL, 0, e); - lflow_cache_add_matches(lcs[i], NULL, NULL); - lflow_cache_add_matches(lcs[i], NULL, matches); + lflow_cache_add_expr(lcs[i], NULL, 0, NULL, 0); + lflow_cache_add_expr(lcs[i], NULL, 0, e, expr_size(e)); + lflow_cache_add_matches(lcs[i], NULL, NULL, 0); + lflow_cache_add_matches(lcs[i], NULL, matches, + TEST_LFLOW_CACHE_VALUE_SIZE); lflow_cache_destroy(lcs[i]); } } diff --git a/include/ovn/expr.h b/include/ovn/expr.h index c2c8218..0323700 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -413,6 +413,7 @@ expr_from_node(const struct ovs_list *node) void expr_format(const struct expr *, struct ds *); void expr_print(const struct expr *); +size_t expr_size(const struct expr *); struct expr *expr_parse(struct lexer *, const struct shash *symtab, const struct shash *addr_sets, const struct shash *port_groups, @@ -477,7 +478,7 @@ uint32_t expr_to_matches(const struct expr *, const void *aux, struct hmap *matches); void expr_matches_destroy(struct hmap *matches); -void expr_matches_prepare(struct hmap *matches, uint32_t conj_id_ofs); +size_t expr_matches_prepare(struct hmap *matches, uint32_t conj_id_ofs); void expr_matches_print(const struct hmap *matches, FILE *); /* Action parsing helper. */ diff --git a/lib/expr.c b/lib/expr.c index 356e6fe..f061a8f 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -471,6 +471,36 @@ expr_print(const struct expr *e) ds_destroy(&output); } +/* Expr Size. */ +size_t +expr_size(const struct expr *expr) { + size_t total_sz = sizeof *expr; + const struct expr *subexpr; + + switch (expr->type) { + case EXPR_T_CMP: + return total_sz + (expr->cmp.symbol->width + ? 0 + : strlen(expr->cmp.string)); + + case EXPR_T_AND: + case EXPR_T_OR: + LIST_FOR_EACH (subexpr, node, &expr->andor) { + total_sz += expr_size(subexpr); + } + return total_sz; + + case EXPR_T_BOOLEAN: + return total_sz; + + case EXPR_T_CONDITION: + return total_sz + strlen(expr->cond.string); + + default: + OVS_NOT_REACHED(); + } +} + /* Parsing. */ #define MAX_PAREN_DEPTH 100 @@ -3127,11 +3157,16 @@ expr_to_matches(const struct expr *expr, /* Prepares the expr matches in the hmap 'matches' by updating the * conj id offsets specified in 'conj_id_ofs'. + * + * Returns the total size (in bytes) of the matches data structure, including + * individual match entries. */ -void +size_t expr_matches_prepare(struct hmap *matches, uint32_t conj_id_ofs) { + size_t total_size = sizeof *matches; struct expr_match *m; + HMAP_FOR_EACH (m, hmap_node, matches) { if (m->match.wc.masks.conj_id) { m->match.flow.conj_id += conj_id_ofs; @@ -3141,7 +3176,9 @@ expr_matches_prepare(struct hmap *matches, uint32_t conj_id_ofs) struct cls_conjunction *src = &m->conjunctions[i]; src->id += conj_id_ofs; } + total_size += sizeof *m + m->allocated * sizeof *m->conjunctions; } + return total_size; } /* Destroys all of the 'struct expr_match'es in 'matches', as well as the diff --git a/tests/ovn-lflow-cache.at b/tests/ovn-lflow-cache.at index 2cbc1f6..e3b67e5 100644 --- a/tests/ovn-lflow-cache.at +++ b/tests/ovn-lflow-cache.at @@ -146,7 +146,7 @@ AT_CHECK( add conj-id 4 \ add expr 5 \ add matches 6 \ - enable 1000 \ + enable 1000 1024 \ add conj-id 7 \ add expr 8 \ add matches 9 \ @@ -255,15 +255,19 @@ AT_CLEANUP AT_SETUP([ovn -- unit test -- lflow-cache set limit]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ - true 8 \ + true 12 \ add conj-id 1 \ add expr 2 \ add matches 3 \ - enable 1 \ + enable 1 1024 \ add conj-id 4 \ add expr 5 \ add matches 6 \ - add conj-id 7], + add conj-id 7 \ + enable 1 1 \ + add conj-id 8 \ + add expr 9 \ + add matches 10], [0], [dnl Enabled: true cache-conj-id: 0 @@ -351,6 +355,48 @@ Enabled: true cache-conj-id: 0 cache-expr: 0 cache-matches: 1 +ENABLE +dnl +dnl Max memory usage smaller than current memory usage, cache should be +dnl flushed. +dnl +Enabled: true + cache-conj-id: 0 + cache-expr: 0 + cache-matches: 0 +ADD conj-id: + conj-id-ofs: 8 +LOOKUP: + conj_id_ofs: 8 + type: conj-id +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 +ADD expr: + conj-id-ofs: 9 +LOOKUP: + not found +dnl +dnl Cache is full and we're adding a cache entry that would go over the max +dnl memory limit so adding should fail. +dnl +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 +ADD matches: + conj-id-ofs: 10 +LOOKUP: + not found +dnl +dnl Cache is full and we're adding a cache entry that would go over the max +dnl memory limit so adding should fail. +dnl +Enabled: true + cache-conj-id: 1 + cache-expr: 0 + cache-matches: 0 ]) AT_CLEANUP