From patchwork Thu Nov 11 22:38:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1554103 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HqxTN1dFRz9s1l for ; Fri, 12 Nov 2021 09:38:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 9276D405B1; Thu, 11 Nov 2021 22:38:45 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eIBeTEbrJTYp; Thu, 11 Nov 2021 22:38:44 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id EC8B3405B0; Thu, 11 Nov 2021 22:38:43 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 25A31C0023; Thu, 11 Nov 2021 22:38:43 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8B882C0023 for ; Thu, 11 Nov 2021 22:38:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 6C2E74037A for ; Thu, 11 Nov 2021 22:38:42 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pyyASz0mnpx0 for ; Thu, 11 Nov 2021 22:38:41 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp2.osuosl.org (Postfix) with ESMTPS id 917174010B for ; Thu, 11 Nov 2021 22:38:41 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 51581FF805; Thu, 11 Nov 2021 22:38:36 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 11 Nov 2021 14:38:22 -0800 Message-Id: <20211111223825.1757021-2-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211111223825.1757021-1-hzhou@ovn.org> References: <20211111223825.1757021-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v3 1/4] lflow-cache.h: Fix comment about lflow-cache. 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" In commit fadbc04f38db it changed the lflow-cache behavior but forgot to update the commment. Fixes: fadbc04f38db ("ovn-controller: Fix incremental processing for logical port references.") Signed-off-by: Han Zhou Acked-by: Numan Siddique --- controller/lflow-cache.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h index 6166fa7c5..553e05f8e 100644 --- a/controller/lflow-cache.h +++ b/controller/lflow-cache.h @@ -32,8 +32,9 @@ struct lflow_cache; * - 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. + * (2) expr tree if the logical flow doesn't have port group/address set + * references but has other references (such as lport). + * (3) expr matches if the logical flow doesn't have any references. */ enum lflow_cache_type { LCACHE_T_CONJ_ID, /* Only conjunction id offset is cached. */ From patchwork Thu Nov 11 22:38:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1554104 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=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HqxTj0S0gz9s1l for ; Fri, 12 Nov 2021 09:39:05 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 4ABE2404A5; Thu, 11 Nov 2021 22:39:03 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DQiluRbwV3mT; Thu, 11 Nov 2021 22:39:01 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 282684047D; Thu, 11 Nov 2021 22:39:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id F064EC0012; Thu, 11 Nov 2021 22:38:59 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0F253C000E for ; Thu, 11 Nov 2021 22:38:58 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 4493881DCA for ; Thu, 11 Nov 2021 22:38:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wAXUozR-swY0 for ; Thu, 11 Nov 2021 22:38:48 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp1.osuosl.org (Postfix) with ESMTPS id 24BC880D24 for ; Thu, 11 Nov 2021 22:38:47 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 45A2CFF804; Thu, 11 Nov 2021 22:38:40 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 11 Nov 2021 14:38:23 -0800 Message-Id: <20211111223825.1757021-3-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211111223825.1757021-1-hzhou@ovn.org> References: <20211111223825.1757021-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v3 2/4] lflow-cache: Remove conjunction id cache. 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" Conjunction id cache is useful to keep conjunction ids consistent between iterations when match or expr cache is not available. However, the current implementation caches the conjunction id whenever n_conjs > 0. It assumes that when the lflow is reprocessed, the conjunction ids starting from the saved conj_id_ofs to the conj_id_ofs + n_conjs - 1 are always available. This would be true only if the n_conjs doesn't change. Unfortunately, the n_conjs can change when a lflow is reprocessed, e.g. when the members of an address-set/port-group increases from 1 to >1. This would result in conflict conjunction ids being used by different lflows. This patch adds a test case to cover this scenario, and the test passes when conjunction id cache is removed. A follow-up patch will take a different approach to keep conjunction ids consistent. Signed-off-by: Han Zhou --- controller/lflow-cache.c | 24 +-- controller/lflow-cache.h | 10 +- controller/lflow.c | 7 - controller/test-lflow-cache.c | 8 +- tests/ovn-lflow-cache.at | 272 +++++++--------------------------- tests/ovn.at | 222 ++++++++------------------- 6 files changed, 122 insertions(+), 421 deletions(-) diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index ece39dbf0..26228c960 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -30,10 +30,8 @@ VLOG_DEFINE_THIS_MODULE(lflow_cache); 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); @@ -46,7 +44,6 @@ COVERAGE_DEFINE(lflow_cache_made_room); COVERAGE_DEFINE(lflow_cache_trim); static 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", }; @@ -204,20 +201,6 @@ lflow_cache_get_stats(const struct lflow_cache *lc, struct ds *output) ROUND_UP(lc->mem_usage, 1024) / 1024); } -void -lflow_cache_add_conj_id(struct lflow_cache *lc, const struct uuid *lflow_uuid, - uint32_t conj_id_ofs) -{ - struct lflow_cache_value *lcv = - lflow_cache_add__(lc, lflow_uuid, LCACHE_T_CONJ_ID, 0); - - if (!lcv) { - return; - } - COVERAGE_INC(lflow_cache_add_conj_id); - lcv->conj_id_ofs = conj_id_ofs; -} - void lflow_cache_add_expr(struct lflow_cache *lc, const struct uuid *lflow_uuid, uint32_t conj_id_ofs, struct expr *expr, size_t expr_sz) @@ -294,9 +277,7 @@ lflow_cache_make_room__(struct lflow_cache *lc, enum lflow_cache_type type) { /* 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 + * type LCACHE_T_EXPR if there's no room to add an entry of type * LCACHE_T_MATCHES. */ for (size_t i = 0; i < type; i++) { @@ -372,9 +353,6 @@ lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce) case LCACHE_T_NONE: 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); diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h index 553e05f8e..d42bdeaa3 100644 --- a/controller/lflow-cache.h +++ b/controller/lflow-cache.h @@ -30,14 +30,11 @@ struct lflow_cache; * 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 doesn't have port group/address set + * (1) expr tree if the logical flow doesn't have port group/address set * references but has other references (such as lport). - * (3) expr matches if the logical flow doesn't have any references. + * (2) expr matches if the logical flow doesn't have any references. */ 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, @@ -63,9 +60,6 @@ void lflow_cache_enable(struct lflow_cache *, bool enabled, uint32_t capacity, bool lflow_cache_is_enabled(const struct lflow_cache *); void lflow_cache_get_stats(const struct lflow_cache *, struct ds *output); -void lflow_cache_add_conj_id(struct lflow_cache *, - const struct uuid *lflow_uuid, - uint32_t conj_id_ofs); void lflow_cache_add_expr(struct lflow_cache *, const struct uuid *lflow_uuid, uint32_t conj_id_ofs, struct expr *expr, size_t expr_sz); diff --git a/controller/lflow.c b/controller/lflow.c index 923d8f0a4..59f5d3a07 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -876,7 +876,6 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, /* 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); @@ -903,7 +902,6 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, /* 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); @@ -916,7 +914,6 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, /* 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); @@ -957,13 +954,9 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, &lflow->header_.uuid, 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->header_.uuid, conj_id_ofs); } } break; - case LCACHE_T_CONJ_ID: case LCACHE_T_EXPR: break; case LCACHE_T_MATCHES: diff --git a/controller/test-lflow-cache.c b/controller/test-lflow-cache.c index b46b1f743..95e619ee0 100644 --- a/controller/test-lflow-cache.c +++ b/controller/test-lflow-cache.c @@ -41,9 +41,7 @@ test_lflow_cache_add__(struct lflow_cache *lc, const char *op_type, 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_uuid, conj_id_ofs); - } else if (!strcmp(op_type, "expr")) { + if (!strcmp(op_type, "expr")) { lflow_cache_add_expr(lc, lflow_uuid, conj_id_ofs, expr_clone(e), TEST_LFLOW_CACHE_VALUE_SIZE); } else if (!strcmp(op_type, "matches")) { @@ -71,9 +69,6 @@ test_lflow_cache_lookup__(struct lflow_cache *lc, 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; @@ -251,7 +246,6 @@ test_lflow_cache_negative(struct ovs_cmdl_context *ctx OVS_UNUSED) 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, 0); lflow_cache_add_expr(lcs[i], NULL, 0, e, expr_size(e)); lflow_cache_add_matches(lcs[i], NULL, NULL, 0); diff --git a/tests/ovn-lflow-cache.at b/tests/ovn-lflow-cache.at index cb4604f51..97d24d0dc 100644 --- a/tests/ovn-lflow-cache.at +++ b/tests/ovn-lflow-cache.at @@ -6,27 +6,13 @@ AT_BANNER([OVN unit tests - lflow-cache]) AT_SETUP([unit test -- lflow-cache single add/lookup]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ - true 3 \ - add conj-id 1 \ + true 2 \ add expr 2 \ add matches 3 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 0 -ADD conj-id: - conj-id-ofs: 1 -LOOKUP: - conj_id_ofs: 1 - type: conj-id -Enabled: true -high-watermark : 1 -total : 1 -cache-conj-id : 1 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -36,9 +22,8 @@ LOOKUP: conj_id_ofs: 2 type: expr Enabled: true -high-watermark : 2 -total : 2 -cache-conj-id : 1 +high-watermark : 1 +total : 1 cache-expr : 1 cache-matches : 0 trim count : 0 @@ -48,9 +33,8 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true -high-watermark : 3 -total : 3 -cache-conj-id : 1 +high-watermark : 2 +total : 2 cache-expr : 1 cache-matches : 1 trim count : 0 @@ -60,30 +44,13 @@ AT_CLEANUP AT_SETUP([unit test -- lflow-cache single add/lookup/del]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ - true 3 \ - add-del conj-id 1 \ + true 2 \ add-del expr 2 \ add-del matches 3 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 0 -ADD conj-id: - conj-id-ofs: 1 -LOOKUP: - conj_id_ofs: 1 - type: conj-id -DELETE -LOOKUP: - not found -Enabled: true -high-watermark : 1 -total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -98,7 +65,6 @@ LOOKUP: Enabled: true high-watermark : 1 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -113,7 +79,6 @@ LOOKUP: Enabled: true high-watermark : 1 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -123,26 +88,13 @@ AT_CLEANUP AT_SETUP([unit test -- lflow-cache disabled single add/lookup/del]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ - false 3 \ - add conj-id 1 \ + false 2 \ add expr 2 \ add matches 3 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: false high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 0 -ADD conj-id: - conj-id-ofs: 1 -LOOKUP: - not found -Enabled: false -high-watermark : 0 -total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -153,7 +105,6 @@ LOOKUP: Enabled: false high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -164,7 +115,6 @@ LOOKUP: Enabled: false high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -174,16 +124,13 @@ AT_CLEANUP AT_SETUP([unit test -- lflow-cache disable/enable/flush]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ - true 12 \ - add conj-id 1 \ + true 9 \ add expr 2 \ add matches 3 \ disable \ - add conj-id 4 \ add expr 5 \ add matches 6 \ enable 1000 1024 \ - add conj-id 7 \ add expr 8 \ add matches 9 \ flush | grep -v 'Mem usage (KB)'], @@ -191,19 +138,6 @@ AT_CHECK( Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 0 -ADD conj-id: - conj-id-ofs: 1 -LOOKUP: - conj_id_ofs: 1 - type: conj-id -Enabled: true -high-watermark : 1 -total : 1 -cache-conj-id : 1 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -213,9 +147,8 @@ LOOKUP: conj_id_ofs: 2 type: expr Enabled: true -high-watermark : 2 -total : 2 -cache-conj-id : 1 +high-watermark : 1 +total : 1 cache-expr : 1 cache-matches : 0 trim count : 0 @@ -225,9 +158,8 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true -high-watermark : 3 -total : 3 -cache-conj-id : 1 +high-watermark : 2 +total : 2 cache-expr : 1 cache-matches : 1 trim count : 0 @@ -235,22 +167,10 @@ DISABLE Enabled: false high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 dnl At "disable" the cache was flushed. trim count : 1 -ADD conj-id: - conj-id-ofs: 4 -LOOKUP: - not found -Enabled: false -high-watermark : 0 -total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 1 ADD expr: conj-id-ofs: 5 LOOKUP: @@ -258,7 +178,6 @@ LOOKUP: Enabled: false high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 1 @@ -269,7 +188,6 @@ LOOKUP: Enabled: false high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 1 @@ -277,19 +195,6 @@ ENABLE Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 1 -ADD conj-id: - conj-id-ofs: 7 -LOOKUP: - conj_id_ofs: 7 - type: conj-id -Enabled: true -high-watermark : 1 -total : 1 -cache-conj-id : 1 cache-expr : 0 cache-matches : 0 trim count : 1 @@ -299,9 +204,8 @@ LOOKUP: conj_id_ofs: 8 type: expr Enabled: true -high-watermark : 2 -total : 2 -cache-conj-id : 1 +high-watermark : 1 +total : 1 cache-expr : 1 cache-matches : 0 trim count : 1 @@ -311,9 +215,8 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true -high-watermark : 3 -total : 3 -cache-conj-id : 1 +high-watermark : 2 +total : 2 cache-expr : 1 cache-matches : 1 trim count : 1 @@ -321,7 +224,6 @@ FLUSH Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 2 @@ -331,36 +233,20 @@ AT_CLEANUP AT_SETUP([unit test -- lflow-cache set limit]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ - true 12 \ - add conj-id 1 \ + true 9 \ add expr 2 \ add matches 3 \ enable 1 1024 \ - add conj-id 4 \ add expr 5 \ add matches 6 \ - add conj-id 7 \ + add expr 7 \ enable 1 1 \ - add conj-id 8 \ add expr 9 \ add matches 10 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 0 -ADD conj-id: - conj-id-ofs: 1 -LOOKUP: - conj_id_ofs: 1 - type: conj-id -Enabled: true -high-watermark : 1 -total : 1 -cache-conj-id : 1 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -370,9 +256,8 @@ LOOKUP: conj_id_ofs: 2 type: expr Enabled: true -high-watermark : 2 -total : 2 -cache-conj-id : 1 +high-watermark : 1 +total : 1 cache-expr : 1 cache-matches : 0 trim count : 0 @@ -382,9 +267,8 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true -high-watermark : 3 -total : 3 -cache-conj-id : 1 +high-watermark : 2 +total : 2 cache-expr : 1 cache-matches : 1 trim count : 0 @@ -395,19 +279,6 @@ dnl Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 1 -ADD conj-id: - conj-id-ofs: 4 -LOOKUP: - conj_id_ofs: 4 - type: conj-id -Enabled: true -high-watermark : 1 -total : 1 -cache-conj-id : 1 cache-expr : 0 cache-matches : 0 trim count : 1 @@ -416,14 +287,9 @@ ADD expr: 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 high-watermark : 1 total : 1 -cache-conj-id : 0 cache-expr : 1 cache-matches : 0 trim count : 1 @@ -439,22 +305,20 @@ dnl Enabled: true high-watermark : 1 total : 1 -cache-conj-id : 0 cache-expr : 0 cache-matches : 1 trim count : 1 -ADD conj-id: +ADD expr: 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 Cache is full and we're adding a expr entry so we shouldn't evict dnl anything else. dnl Enabled: true high-watermark : 1 total : 1 -cache-conj-id : 0 cache-expr : 0 cache-matches : 1 trim count : 1 @@ -466,19 +330,6 @@ dnl Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 -cache-expr : 0 -cache-matches : 0 -trim count : 2 -ADD conj-id: - conj-id-ofs: 8 -LOOKUP: - conj_id_ofs: 8 - type: conj-id -Enabled: true -high-watermark : 1 -total : 1 -cache-conj-id : 1 cache-expr : 0 cache-matches : 0 trim count : 2 @@ -491,9 +342,8 @@ 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 -high-watermark : 1 -total : 1 -cache-conj-id : 1 +high-watermark : 0 +total : 0 cache-expr : 0 cache-matches : 0 trim count : 2 @@ -506,9 +356,8 @@ 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 -high-watermark : 1 -total : 1 -cache-conj-id : 1 +high-watermark : 0 +total : 0 cache-expr : 0 cache-matches : 0 trim count : 2 @@ -520,11 +369,11 @@ AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ true 12 \ enable 1000 1024 trim-limit 100 trim-wmark-perc 100 \ - add conj-id 1 \ - add conj-id 2 \ - add conj-id 3 \ - add conj-id 4 \ - add conj-id 5 \ + add expr 1 \ + add expr 2 \ + add expr 3 \ + add expr 4 \ + add expr 5 \ del \ enable 1000 1024 trim-limit 0 trim-wmark-perc 100 \ del \ @@ -535,7 +384,6 @@ AT_CHECK( Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 @@ -543,68 +391,62 @@ ENABLE Enabled: true high-watermark : 0 total : 0 -cache-conj-id : 0 cache-expr : 0 cache-matches : 0 trim count : 0 -ADD conj-id: +ADD expr: conj-id-ofs: 1 LOOKUP: conj_id_ofs: 1 - type: conj-id + type: expr Enabled: true high-watermark : 1 total : 1 -cache-conj-id : 1 -cache-expr : 0 +cache-expr : 1 cache-matches : 0 trim count : 0 -ADD conj-id: +ADD expr: conj-id-ofs: 2 LOOKUP: conj_id_ofs: 2 - type: conj-id + type: expr Enabled: true high-watermark : 2 total : 2 -cache-conj-id : 2 -cache-expr : 0 +cache-expr : 2 cache-matches : 0 trim count : 0 -ADD conj-id: +ADD expr: conj-id-ofs: 3 LOOKUP: conj_id_ofs: 3 - type: conj-id + type: expr Enabled: true high-watermark : 3 total : 3 -cache-conj-id : 3 -cache-expr : 0 +cache-expr : 3 cache-matches : 0 trim count : 0 -ADD conj-id: +ADD expr: conj-id-ofs: 4 LOOKUP: conj_id_ofs: 4 - type: conj-id + type: expr Enabled: true high-watermark : 4 total : 4 -cache-conj-id : 4 -cache-expr : 0 +cache-expr : 4 cache-matches : 0 trim count : 0 -ADD conj-id: +ADD expr: conj-id-ofs: 5 LOOKUP: conj_id_ofs: 5 - type: conj-id + type: expr Enabled: true high-watermark : 5 total : 5 -cache-conj-id : 5 -cache-expr : 0 +cache-expr : 5 cache-matches : 0 trim count : 0 DELETE @@ -614,8 +456,7 @@ dnl Enabled: true high-watermark : 5 total : 4 -cache-conj-id : 4 -cache-expr : 0 +cache-expr : 4 cache-matches : 0 trim count : 0 ENABLE @@ -626,8 +467,7 @@ dnl Enabled: true high-watermark : 4 total : 4 -cache-conj-id : 4 -cache-expr : 0 +cache-expr : 4 cache-matches : 0 trim count : 1 DELETE @@ -638,16 +478,14 @@ dnl Enabled: true high-watermark : 3 total : 3 -cache-conj-id : 3 -cache-expr : 0 +cache-expr : 3 cache-matches : 0 trim count : 2 ENABLE Enabled: true high-watermark : 3 total : 3 -cache-conj-id : 3 -cache-expr : 0 +cache-expr : 3 cache-matches : 0 trim count : 2 DELETE @@ -658,8 +496,7 @@ dnl Enabled: true high-watermark : 3 total : 2 -cache-conj-id : 2 -cache-expr : 0 +cache-expr : 2 cache-matches : 0 trim count : 2 dnl @@ -670,8 +507,7 @@ DELETE Enabled: true high-watermark : 1 total : 1 -cache-conj-id : 1 -cache-expr : 0 +cache-expr : 1 cache-matches : 0 trim count : 3 ]) diff --git a/tests/ovn.at b/tests/ovn.at index 51e0dae0b..3a361b33f 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -15213,6 +15213,70 @@ OVN_CLEANUP([hv1]) AT_CLEANUP ]) +# Number of conjunctions can change for the same logical flow, which should not +# cause conflict conjunction IDs between logical flows. +OVN_FOR_EACH_NORTHD([ +AT_SETUP([n_conjs change]) +ovn_start + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" \ +-- set logical_switch_port ls1-lp1 options:requested-tnl-key=1 + +check ovn-nbctl lsp-add ls1 ls1-lp2 \ +-- lsp-set-addresses ls1-lp2 "f0:00:00:00:00:02" \ +-- set logical_switch_port ls1-lp1 options:requested-tnl-key=2 + +net_add n1 +sim_add hv1 + +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +check ovs-vsctl -- add-port br-int hv1-vif2 -- \ + set interface hv1-vif2 external-ids:iface-id=ls1-lp2 \ + options:tx_pcap=hv1/vif2-tx.pcap \ + options:rxq_pcap=hv1/vif2-rx.pcap \ + ofport-request=2 + +ovn-nbctl create address_set name=as1 addresses="10.0.0.1" +ovn-nbctl create address_set name=as2 addresses="10.0.0.11,10.0.0.12" +ovn-nbctl pg-add pg1 ls1-lp1 ls1-lp2 + +# The 1st ACL potentially can generate 2 conjunctions, but as1 has only 1 address, +# so it would generate 1 conjunction for now. +check ovn-nbctl acl-add pg1 to-lport 100 \ + '(outport == @pg1 && ip4.src == $as1) || (outport == @pg1 && ip4.dst == $as2)' allow + +# The 2nd ACL generates 1 conjunction (use another conjunction ID) +check ovn-nbctl acl-add pg1 to-lport 100 'outport == @pg1 && ip4.src == $as2' allow + +wait_for_ports_up +check ovn-nbctl --wait=hv sync +ovs-ofctl dump-flows br-int table=44 +AT_CHECK([test `ovs-ofctl dump-flows br-int table=44 | grep -c conj_id` = 2]) + +echo ------- +# Add another address in as1, so that the 1st ACL will now generate 2 conjunctions. +ovn-nbctl set address_set as1 addresses="10.0.0.1,10.0.0.2" +check ovn-nbctl --wait=hv sync + +ovs-ofctl dump-flows br-int table=44 +# There should be 3 conjunctions in total (2 from 1st ACL + 1 from 2nd ACL) +AT_CHECK([test `ovs-ofctl dump-flows br-int table=44 | grep -c conj_id` = 3]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) + # 3 hypervisors, one logical switch, 3 logical ports per hypervisor OVN_FOR_EACH_NORTHD([ AT_SETUP([L2 Drop and Allow ACL w/ Stateful ACL]) @@ -24530,156 +24594,6 @@ OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP ]) -OVN_FOR_EACH_NORTHD([ -AT_SETUP([lflow cache for conjunctions]) -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 - -check ovn-nbctl ls-add sw0 -check ovn-nbctl lsp-add sw0 sw0-p1 -check ovn-nbctl lsp-set-addresses sw0-p1 "10:14:00:00:00:03 10.0.0.3" -check ovn-nbctl lsp-set-port-security sw0-p1 "10:14:00:00:00:03 10.0.0.3" - -check ovn-nbctl lsp-add sw0 sw0-p2 -check ovn-nbctl lsp-set-addresses sw0-p2 "10:14:00:00:00:04 10.0.0.4" -check ovn-nbctl lsp-set-port-security sw0-p2 "10:14:00:00:00:04 10.0.0.4" - -check ovn-nbctl lsp-add sw0 sw0-p3 -check ovn-nbctl lsp-set-addresses sw0-p3 "10:14:00:00:00:05 10.0.0.5" -check ovn-nbctl lsp-set-port-security sw0-p3 "10:14:00:00:00:05 10.0.0.5" - -check ovn-nbctl lsp-add sw0 sw0-p4 -check ovn-nbctl lsp-set-addresses sw0-p4 "10:14:00:00:00:06 10.0.0.6" -check ovn-nbctl lsp-set-port-security sw0-p4 "10:14:00:00:00:06 10.0.0.6" - -as hv1 -ovs-vsctl -- add-port br-int hv1-vif1 -- \ - set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ - options:tx_pcap=hv1/vif1-tx.pcap \ - options:rxq_pcap=hv1/vif1-rx.pcap \ - ofport-request=1 -ovs-vsctl -- add-port br-int hv1-vif2 -- \ - set interface hv1-vif2 external-ids:iface-id=sw0-p2 \ - options:tx_pcap=hv1/vif2-tx.pcap \ - options:rxq_pcap=hv1/vif2-rx.pcap \ - ofport-request=2 -ovs-vsctl -- add-port br-int hv1-vif3 -- \ - set interface hv1-vif3 external-ids:iface-id=sw0-p3 \ - options:tx_pcap=hv1/vif3-tx.pcap \ - options:rxq_pcap=hv1/vif3-rx.pcap \ - ofport-request=3 -ovs-vsctl -- add-port br-int hv1-vif4 -- \ - set interface hv1-vif4 external-ids:iface-id=sw0-p4 \ - options:tx_pcap=hv1/vif4-tx.pcap \ - options:rxq_pcap=hv1/vif4-rx.pcap \ - ofport-request=4 - -wait_for_ports_up - -check ovn-nbctl pg-add pg0 sw0-p1 sw0-p2 -check ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82" allow -check ovn-nbctl --wait=hv sync - -# wait_conj_id_count COUNT ["ID COUNT [MATCH]"]... -# -# Waits until COUNT flows matching against conj_id appear in the -# table 44 on hv1's br-int bridge. Makes the flows available in -# "hv1flows", which will be logged on error. -# -# In addition, for each quoted "ID COUNT" or "ID COUNT MATCH", -# verifies that there are COUNT flows in table 45 that match -# aginst conj_id=ID and (if MATCH) is nonempty, match MATCH. -wait_conj_id_count() { - AT_CAPTURE_FILE([hv1flows]) - local retval - case $1 in - (0) retval=1 ;; - (*) retval=0 ;; - esac - - echo "waiting for $1 conj_id flows..." - OVS_WAIT_FOR_OUTPUT_UNQUOTED( - [ovs-ofctl dump-flows br-int > hv1flows - grep table=44 hv1flows | grep -c conj_id], - [$retval], [$1 -]) - - shift - for arg; do - set -- $arg; id=$1 count=$2 match=$3 - echo "checking that there are $count ${match:+$match }flows with conj_id=$id..." - AT_CHECK_UNQUOTED( - [grep table=44 hv1flows | grep "$match" | grep -c conj_id=$id], - [0], [$count -]) - done -} - -AS_BOX([Add sw0-p3 to the port group pg0. The conj_id should be 2.]) -check ovn-nbctl --wait=hv pg-set-ports pg0 sw0-p1 sw0-p2 sw0-p3 -wait_conj_id_count 1 "2 1" - -AS_BOX([Add sw0p4 to the port group pg0. The conj_id should be 2.]) -check ovn-nbctl --wait=hv pg-set-ports pg0 sw0-p1 sw0-p2 sw0-p3 sw0-p4 -wait_conj_id_count 1 "2 1" - -AS_BOX([Add another ACL with conjunction.]) -check ovn-nbctl --wait=hv acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && udp.dst >= 80 && udp.dst <= 82" allow -wait_conj_id_count 2 "2 1 tcp" "3 1 udp" - -AS_BOX([Delete tcp ACL.]) -check ovn-nbctl --wait=hv acl-del pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82" -wait_conj_id_count 1 "3 1 udp" - -AS_BOX([Add back the tcp ACL.]) -check ovn-nbctl --wait=hv acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82" allow -wait_conj_id_count 2 "3 1 udp" "4 1 tcp" -AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep udp | grep -c "conj_id=3")]) -AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep tcp | grep -c "conj_id=4")]) - -AS_BOX([Add another tcp ACL.]) -check ovn-nbctl --wait=hv acl-add pg0 to-lport 1002 "outport == @pg0 && inport == @pg0 && ip4 && tcp.dst >= 84 && tcp.dst <= 86" allow -wait_conj_id_count 3 "3 1 udp" "4 1 tcp" "5 1 tcp" - -AS_BOX([Clear ACLs.]) -check ovn-nbctl --wait=hv clear port_group pg0 acls -wait_conj_id_count 0 - -AS_BOX([Add TCP ACL.]) -check ovn-nbctl --wait=hv acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82" allow -check ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && udp.dst >= 80 && udp.dst <= 82" allow -wait_conj_id_count 2 "6 1 tcp" "7 1 udp" - -AS_BOX([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.]) -as hv1 ovs-vsctl set open . external_ids:ovn-enable-lflow-cache=false - -AS_BOX([Wait until ovn-enble-lflow-cache is processed by ovn-controller.]) -wait_row_count Chassis 1 name=hv1 other_config:ovn-enable-lflow-cache=false -wait_conj_id_count 2 "2 1" "3 1" - -AS_BOX([Remove port sw0-p4 from port group.]) -check ovn-nbctl --wait=hv pg-set-ports pg0 sw0-p1 sw0-p2 sw0-p3 -wait_conj_id_count 2 "4 1" "5 1" - -AS_BOX([Recompute.]) -as hv1 ovn-appctl -t ovn-controller recompute - -wait_conj_id_count 2 "2 1" "3 1" - -OVN_CLEANUP([hv1]) - -AT_CLEANUP -]) - OVN_FOR_EACH_NORTHD([ AT_SETUP([lflow cache operations]) ovn_start @@ -24710,31 +24624,26 @@ get_cache_count () { } 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) @@ -24742,19 +24651,16 @@ check ovn-nbctl acl-add ls1 from-lport 1 'inport == @pg1 && outport == @pg1 && i 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], []) AS_BOX([Check no 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], []) From patchwork Thu Nov 11 22:38:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1554105 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=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HqxTp0yNpz9s1l for ; Fri, 12 Nov 2021 09:39:10 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 885B28249E; Thu, 11 Nov 2021 22:39:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NWd1nvbNb2GQ; Thu, 11 Nov 2021 22:39:06 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id CF8B0823CF; Thu, 11 Nov 2021 22:39:04 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6BAFBC001E; Thu, 11 Nov 2021 22:39:04 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3C5E7C0012 for ; Thu, 11 Nov 2021 22:39:03 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 7A54680E25 for ; Thu, 11 Nov 2021 22:38:52 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jTdPZKm52amN for ; Thu, 11 Nov 2021 22:38:52 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp1.osuosl.org (Postfix) with ESMTPS id BA0FE80C30 for ; Thu, 11 Nov 2021 22:38:51 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id A796DFF807; Thu, 11 Nov 2021 22:38:46 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 11 Nov 2021 14:38:24 -0800 Message-Id: <20211111223825.1757021-4-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211111223825.1757021-1-hzhou@ovn.org> References: <20211111223825.1757021-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v3 3/4] test-utils: Add test_read_uint_hex_value helper. 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: Han Zhou --- tests/test-utils.c | 22 ++++++++++++++++++---- tests/test-utils.h | 2 ++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/test-utils.c b/tests/test-utils.c index 6a3b198ae..78e63acb7 100644 --- a/tests/test-utils.c +++ b/tests/test-utils.c @@ -19,9 +19,9 @@ #include "util.h" -bool -test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index, - const char *descr, unsigned int *result) +static bool +test_read_uint_value_base(struct ovs_cmdl_context *ctx, unsigned int index, + const char *descr, int base, unsigned int *result) { if (index >= ctx->argc) { fprintf(stderr, "Missing %s argument\n", descr); @@ -29,13 +29,27 @@ test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index, } const char *arg = ctx->argv[index]; - if (!str_to_uint(arg, 10, result)) { + if (!str_to_uint(arg, base, result)) { fprintf(stderr, "Invalid %s: %s\n", descr, arg); return false; } return true; } +bool +test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index, + const char *descr, unsigned int *result) +{ + return test_read_uint_value_base(ctx, index, descr, 10, result); +} + +bool +test_read_uint_hex_value(struct ovs_cmdl_context *ctx, unsigned int index, + const char *descr, unsigned int *result) +{ + return test_read_uint_value_base(ctx, index, descr, 16, result); +} + const char * test_read_value(struct ovs_cmdl_context *ctx, unsigned int index, const char *descr) diff --git a/tests/test-utils.h b/tests/test-utils.h index 721032f82..910358ea1 100644 --- a/tests/test-utils.h +++ b/tests/test-utils.h @@ -20,6 +20,8 @@ bool test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index, const char *descr, unsigned int *result); +bool test_read_uint_hex_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); From patchwork Thu Nov 11 22:38:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1554106 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HqxV50V6pz9s1l for ; Fri, 12 Nov 2021 09:39:24 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 3252F60BB2; Thu, 11 Nov 2021 22:39:21 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JkKUMaC8yxn4; Thu, 11 Nov 2021 22:39:14 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id AE4D660B91; Thu, 11 Nov 2021 22:39:13 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 69E7CC0012; Thu, 11 Nov 2021 22:39:13 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 12649C0012 for ; Thu, 11 Nov 2021 22:39:12 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 8A7034048E for ; Thu, 11 Nov 2021 22:38:58 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Zi1DCtsmZMSV for ; Thu, 11 Nov 2021 22:38:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp2.osuosl.org (Postfix) with ESMTPS id B649140482 for ; Thu, 11 Nov 2021 22:38:55 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id CD34BFF804; Thu, 11 Nov 2021 22:38:51 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Thu, 11 Nov 2021 14:38:25 -0800 Message-Id: <20211111223825.1757021-5-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211111223825.1757021-1-hzhou@ovn.org> References: <20211111223825.1757021-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v3 4/4] lflow: Consistent conjunction id generation. 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" When a logical flow translation uses conjunction ids, it is better to keep the conjuntion ids consistent between iterations, so that the generated OVS flows don't change unnecessarily and avoid reinstalling unchanged flows to OVS. The problem was partially solved when lflow-cache was enabled but there was a bug for a corner case so the lflow-cache based approach was reverted in a previous commit. This patch takes an approach that generates conjunction ids based on the logical flow's uuid, which ensures the same ids being used when processing the same lflow in most cases. There is a very little chance that different logial flows happen to have the same 32-bit prefix in their uuids, but the probability is really low. For the very unlikely situation, the conflicts are handled properly and correctness is ensured. A new module lflow-conj-ids is created to manage the id allocations and maintaining the usage of each ids and their owner lflows. Unlike the previously lflow-cache based approach, this approach works equally well when lflow-cache is not in use. Signed-off-by: Han Zhou --- controller/automake.mk | 2 + controller/lflow-cache.c | 6 +- controller/lflow-cache.h | 9 +- controller/lflow-conj-ids.c | 259 +++++++++++++++++++++++++++++++ controller/lflow-conj-ids.h | 41 +++++ controller/lflow.c | 147 ++++++++---------- controller/lflow.h | 4 +- controller/ovn-controller.c | 32 +--- controller/test-lflow-cache.c | 36 +++-- controller/test-lflow-conj-ids.c | 128 +++++++++++++++ tests/automake.mk | 3 + tests/ovn-lflow-cache.at | 123 ++++++++++----- tests/ovn-lflow-conj-ids.at | 112 +++++++++++++ tests/ovn.at | 49 ++++-- tests/testsuite.at | 1 + 15 files changed, 772 insertions(+), 180 deletions(-) create mode 100644 controller/lflow-conj-ids.c create mode 100644 controller/lflow-conj-ids.h create mode 100644 controller/test-lflow-conj-ids.c create mode 100644 tests/ovn-lflow-conj-ids.at diff --git a/controller/automake.mk b/controller/automake.mk index 9f9b49fe0..c2ab1bbe6 100644 --- a/controller/automake.mk +++ b/controller/automake.mk @@ -18,6 +18,8 @@ controller_ovn_controller_SOURCES = \ controller/lflow.h \ controller/lflow-cache.c \ controller/lflow-cache.h \ + controller/lflow-conj-ids.c \ + controller/lflow-conj-ids.h \ controller/lport.c \ controller/lport.h \ controller/ofctrl.c \ diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index 26228c960..6db85fc12 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -203,7 +203,7 @@ lflow_cache_get_stats(const struct lflow_cache *lc, struct ds *output) void lflow_cache_add_expr(struct lflow_cache *lc, const struct uuid *lflow_uuid, - uint32_t conj_id_ofs, struct expr *expr, size_t expr_sz) + struct expr *expr, size_t expr_sz) { struct lflow_cache_value *lcv = lflow_cache_add__(lc, lflow_uuid, LCACHE_T_EXPR, expr_sz); @@ -213,12 +213,12 @@ lflow_cache_add_expr(struct lflow_cache *lc, const struct uuid *lflow_uuid, return; } COVERAGE_INC(lflow_cache_add_expr); - lcv->conj_id_ofs = conj_id_ofs; lcv->expr = expr; } void lflow_cache_add_matches(struct lflow_cache *lc, const struct uuid *lflow_uuid, + uint32_t conj_id_ofs, uint32_t n_conjs, struct hmap *matches, size_t matches_sz) { struct lflow_cache_value *lcv = @@ -231,6 +231,8 @@ lflow_cache_add_matches(struct lflow_cache *lc, const struct uuid *lflow_uuid, } COVERAGE_INC(lflow_cache_add_matches); lcv->expr_matches = matches; + lcv->n_conjs = n_conjs; + lcv->conj_id_ofs = conj_id_ofs; } struct lflow_cache_value * diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h index d42bdeaa3..faad32bb6 100644 --- a/controller/lflow-cache.h +++ b/controller/lflow-cache.h @@ -43,6 +43,11 @@ enum lflow_cache_type { struct lflow_cache_value { enum lflow_cache_type type; + + /* n_conjs and conj_id_ofs are used only for LCACHE_T_MATCHES. + * They are saved together with the match, so that we can re-allocate the + * same ids when reusing the cached match for a given lflow. */ + uint32_t n_conjs; uint32_t conj_id_ofs; union { @@ -61,10 +66,10 @@ bool lflow_cache_is_enabled(const struct lflow_cache *); void lflow_cache_get_stats(const struct lflow_cache *, struct ds *output); void lflow_cache_add_expr(struct lflow_cache *, const struct uuid *lflow_uuid, - uint32_t conj_id_ofs, struct expr *expr, - size_t expr_sz); + struct expr *expr, size_t expr_sz); void lflow_cache_add_matches(struct lflow_cache *, const struct uuid *lflow_uuid, + uint32_t conj_id_ofs, uint32_t n_conjs, struct hmap *matches, size_t matches_sz); struct lflow_cache_value *lflow_cache_get(struct lflow_cache *, diff --git a/controller/lflow-conj-ids.c b/controller/lflow-conj-ids.c new file mode 100644 index 000000000..2678c4205 --- /dev/null +++ b/controller/lflow-conj-ids.c @@ -0,0 +1,259 @@ +/* + * Copyright (c) 2021, NVIDIA CORPORATION. All rights reserved. + * + * 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 "coverage.h" +#include "lflow-conj-ids.h" +#include "util.h" + +COVERAGE_DEFINE(lflow_conj_conflict); +COVERAGE_DEFINE(lflow_conj_alloc); +COVERAGE_DEFINE(lflow_conj_alloc_specified); +COVERAGE_DEFINE(lflow_conj_free); +COVERAGE_DEFINE(lflow_conj_free_unexpected); + +/* Node in struct conj_ids.conj_id_allocations. */ +struct conj_id_node { + struct hmap_node hmap_node; + uint32_t conj_id; +}; + +/* Node in struct conj_ids.lflow_conj_ids. */ +struct lflow_conj_node { + struct hmap_node hmap_node; + struct uuid lflow_uuid; + uint32_t start_conj_id; + uint32_t n_conjs; +}; + +/* Insert n_conjs conjuntion ids starting from start_conj_id into the conj_ids, + * assuming the ids are confirmed to be available. */ +static void +lflow_conj_ids_insert_(struct conj_ids *conj_ids, + const struct uuid *lflow_uuid, + uint32_t start_conj_id, uint32_t n_conjs) +{ + ovs_assert(n_conjs); + uint32_t conj_id = start_conj_id; + for (uint32_t i = 0; i < n_conjs; i++) { + ovs_assert(conj_id); + struct conj_id_node *node = xzalloc(sizeof *node); + node->conj_id = conj_id; + hmap_insert(&conj_ids->conj_id_allocations, &node->hmap_node, conj_id); + conj_id++; + } + + struct lflow_conj_node *lflow_conj = xzalloc(sizeof *lflow_conj); + lflow_conj->lflow_uuid = *lflow_uuid; + lflow_conj->start_conj_id = start_conj_id; + lflow_conj->n_conjs = n_conjs; + hmap_insert(&conj_ids->lflow_conj_ids, &lflow_conj->hmap_node, + uuid_hash(lflow_uuid)); +} + +static void +lflow_conj_ids_free_(struct conj_ids *conj_ids, const struct uuid *lflow_uuid, + bool expected) +{ + struct lflow_conj_node *lflow_conj; + bool found = false; + HMAP_FOR_EACH_WITH_HASH (lflow_conj, hmap_node, uuid_hash(lflow_uuid), + &conj_ids->lflow_conj_ids) { + if (uuid_equals(&lflow_conj->lflow_uuid, lflow_uuid)) { + found = true; + break; + } + } + if (!found) { + return; + } + ovs_assert(lflow_conj->n_conjs); + COVERAGE_INC(lflow_conj_free); + if (!expected) { + /* It is unexpected that an entry is found when calling from + * alloc/alloc_specified. Something may be wrong in the lflow module. + */ + COVERAGE_INC(lflow_conj_free_unexpected); + } + uint32_t conj_id = lflow_conj->start_conj_id; + for (uint32_t i = 0; i < lflow_conj->n_conjs; i++) { + ovs_assert(conj_id); + struct conj_id_node *conj_id_node; + HMAP_FOR_EACH_WITH_HASH (conj_id_node, hmap_node, conj_id, + &conj_ids->conj_id_allocations) { + if (conj_id_node->conj_id == conj_id) { + hmap_remove(&conj_ids->conj_id_allocations, + &conj_id_node->hmap_node); + free(conj_id_node); + break; + } + } + conj_id++; + } + + hmap_remove(&conj_ids->lflow_conj_ids, &lflow_conj->hmap_node); + free(lflow_conj); +} + +/* Allocate n_conjs continuous conjuction ids from the conj_ids for the given + * lflow_uuid. (0 is never included in an allocated range) + * + * The first conjunction id is returned. If no conjunction ids available, or if + * the input is invalid (n_conjs == 0), then 0 is returned. + * + * The algorithm tries to allocate the parts[0] of the input uuid as the first + * conjunction id. If it is unavailable, or any of the subsequent n_conjs - 1 + * ids are unavailable, iterate until the next available n_conjs ids are found. + * Given that n_conjs is very small (in most cases will be 1), the algorithm + * should be efficient enough and in most cases just return the lflow_uuid's + * part[0], which ensures conjunction ids are consistent for the same logical + * flow in most cases. + * + * The performance will degrade if most of the uint32_t are allocated because + * conflicts will happen a lot. In practice this is not expected to happen in + * reasonable scale. Otherwise, if the amount of logical flows is close to + * this (4G logical flows that need conjunction ids) there are other parts of + * the system expected to be suffered even before reaching to a scale much + * smaller than this. */ +uint32_t +lflow_conj_ids_alloc(struct conj_ids *conj_ids, const struct uuid *lflow_uuid, + uint32_t n_conjs) +{ + if (!n_conjs) { + return 0; + } + lflow_conj_ids_free_(conj_ids, lflow_uuid, false); + + COVERAGE_INC(lflow_conj_alloc); + + uint32_t start_conj_id = lflow_uuid->parts[0]; + uint32_t initial_id = start_conj_id; + bool initial = true; + while (true) { + if (start_conj_id == 0) { + start_conj_id++; + } + bool available = true; + uint32_t conj_id = start_conj_id; + for (uint32_t i = 0; i < n_conjs; i++) { + if (conj_id == 0) { + /* Overflow. Consider the current range as unavailable because + * we need a continuous range. Start over from 1 (0 is + * skipped). */ + available = false; + break; + } + if (!initial && conj_id == initial_id) { + /* It has checked all ids (extreme situation, not expected in + * real environment). */ + return 0; + } + initial = false; + struct conj_id_node *conj_id_node; + /* conj_id is both the key and the hash */ + HMAP_FOR_EACH_WITH_HASH (conj_id_node, hmap_node, conj_id, + &conj_ids->conj_id_allocations) { + if (conj_id_node->conj_id == conj_id) { + available = false; + COVERAGE_INC(lflow_conj_conflict); + break; + } + } + if (!available) { + break; + } + conj_id++; + } + if (available) { + break; + } + start_conj_id = conj_id + 1; + } + lflow_conj_ids_insert_(conj_ids, lflow_uuid, start_conj_id, n_conjs); + return start_conj_id; +} + +/* Similar to lflow_conj_ids_alloc, except that it takes an extra parameter + * start_conj_id, which specifies the desired conjunction ids to be allocated, + * and if they are unavailable, return false directly without trying to find + * the next available ones. It returns true if the specified range is + * allocated successfully. */ +bool +lflow_conj_ids_alloc_specified(struct conj_ids *conj_ids, + const struct uuid *lflow_uuid, + uint32_t start_conj_id, uint32_t n_conjs) +{ + if (!n_conjs) { + return false; + } + lflow_conj_ids_free_(conj_ids, lflow_uuid, false); + + uint32_t conj_id = start_conj_id; + for (uint32_t i = 0; i < n_conjs; i++) { + if (!conj_id) { + return false; + } + struct conj_id_node *conj_id_node; + HMAP_FOR_EACH_WITH_HASH (conj_id_node, hmap_node, conj_id, + &conj_ids->conj_id_allocations) { + if (conj_id_node->conj_id == conj_id) { + return false; + } + } + conj_id++; + } + lflow_conj_ids_insert_(conj_ids, lflow_uuid, start_conj_id, n_conjs); + COVERAGE_INC(lflow_conj_alloc_specified); + + return true; +} + +/* Frees the conjunction IDs used by lflow_uuid. */ +void +lflow_conj_ids_free(struct conj_ids *conj_ids, const struct uuid *lflow_uuid) +{ + lflow_conj_ids_free_(conj_ids, lflow_uuid, true); +} + +void +lflow_conj_ids_init(struct conj_ids *conj_ids) +{ + hmap_init(&conj_ids->conj_id_allocations); + hmap_init(&conj_ids->lflow_conj_ids); +} + +void +lflow_conj_ids_destroy(struct conj_ids *conj_ids) { + struct conj_id_node *conj_id_node, *next; + HMAP_FOR_EACH_SAFE (conj_id_node, next, hmap_node, + &conj_ids->conj_id_allocations) { + hmap_remove(&conj_ids->conj_id_allocations, &conj_id_node->hmap_node); + free(conj_id_node); + } + hmap_destroy(&conj_ids->conj_id_allocations); + + struct lflow_conj_node *lflow_conj, *l_c_next; + HMAP_FOR_EACH_SAFE (lflow_conj, l_c_next, hmap_node, + &conj_ids->lflow_conj_ids) { + hmap_remove(&conj_ids->lflow_conj_ids, &lflow_conj->hmap_node); + free(lflow_conj); + } + hmap_destroy(&conj_ids->lflow_conj_ids); +} + +void lflow_conj_ids_clear(struct conj_ids *conj_ids) { + lflow_conj_ids_destroy(conj_ids); + lflow_conj_ids_init(conj_ids); +} diff --git a/controller/lflow-conj-ids.h b/controller/lflow-conj-ids.h new file mode 100644 index 000000000..d333fa8d5 --- /dev/null +++ b/controller/lflow-conj-ids.h @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2021, NVIDIA CORPORATION. All rights reserved. + * + * 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_CONJ_IDS_H +#define LFLOW_CONJ_IDS_H 1 + +#include "openvswitch/hmap.h" +#include "uuid.h" + +struct conj_ids { + /* Allocated conjunction ids. Contains struct conj_id_node. */ + struct hmap conj_id_allocations; + /* A map from lflows to the conjunction ids used. Contains struct + * lflow_conj_node. */ + struct hmap lflow_conj_ids; +}; + +uint32_t lflow_conj_ids_alloc(struct conj_ids *, const struct uuid *lflow_uuid, + uint32_t n_conjs); +bool lflow_conj_ids_alloc_specified(struct conj_ids *, + const struct uuid *lflow_uuid, + uint32_t start_conj_id, uint32_t n_conjs); +void lflow_conj_ids_free(struct conj_ids *, const struct uuid *lflow_uuid); +void lflow_conj_ids_init(struct conj_ids *); +void lflow_conj_ids_destroy(struct conj_ids *); +void lflow_conj_ids_clear(struct conj_ids *); + +#endif /* controller/lflow-conj-ids.h */ diff --git a/controller/lflow.c b/controller/lflow.c index 59f5d3a07..d74924e3a 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -73,7 +73,7 @@ struct condition_aux { struct lflow_resource_ref *lfrr; }; -static bool +static void consider_logical_flow(const struct sbrec_logical_flow *lflow, struct hmap *dhcp_opts, struct hmap *dhcpv6_opts, struct hmap *nd_ra_opts, @@ -365,14 +365,9 @@ add_logical_flows(struct lflow_ctx_in *l_ctx_in, controller_event_opts_init(&controller_event_opts); SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow, l_ctx_in->logical_flow_table) { - if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, - &nd_ra_opts, &controller_event_opts, - l_ctx_in, l_ctx_out)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); - VLOG_ERR_RL(&rl, "Conjunction id overflow when processing lflow " - UUID_FMT, UUID_ARGS(&lflow->header_.uuid)); - l_ctx_out->conj_id_overflow = true; - } + consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, + &nd_ra_opts, &controller_event_opts, + l_ctx_in, l_ctx_out); } dhcp_opts_destroy(&dhcp_opts); @@ -434,18 +429,17 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in, HMAP_FOR_EACH (ofrn, hmap_node, &flood_remove_nodes) { /* Delete entries from lflow resource reference. */ lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->sb_uuid); + /* Delete conj_ids owned by the lflow. */ + lflow_conj_ids_free(l_ctx_out->conj_ids, &ofrn->sb_uuid); /* Reprocessing the lflow if the sb record is not deleted. */ lflow = sbrec_logical_flow_table_get_for_uuid( l_ctx_in->logical_flow_table, &ofrn->sb_uuid); if (lflow) { VLOG_DBG("re-add lflow "UUID_FMT, UUID_ARGS(&lflow->header_.uuid)); - if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, - &nd_ra_opts, &controller_event_opts, - l_ctx_in, l_ctx_out)) { - ret = false; - break; - } + consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, + &nd_ra_opts, &controller_event_opts, + l_ctx_in, l_ctx_out); } } HMAP_FOR_EACH_SAFE (ofrn, next, hmap_node, &flood_remove_nodes) { @@ -528,6 +522,7 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, /* Secondly, for each lflow that is actually removed, reprocessing it. */ HMAP_FOR_EACH (ofrn, hmap_node, &flood_remove_nodes) { lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->sb_uuid); + lflow_conj_ids_free(l_ctx_out->conj_ids, &ofrn->sb_uuid); const struct sbrec_logical_flow *lflow = sbrec_logical_flow_table_get_for_uuid(l_ctx_in->logical_flow_table, @@ -540,13 +535,9 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, continue; } - if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, - &nd_ra_opts, &controller_event_opts, - l_ctx_in, l_ctx_out)) { - ret = false; - l_ctx_out->conj_id_overflow = true; - break; - } + consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, + &nd_ra_opts, &controller_event_opts, + l_ctx_in, l_ctx_out); *changed = true; } HMAP_FOR_EACH_SAFE (ofrn, ofrn_next, hmap_node, &flood_remove_nodes) { @@ -568,17 +559,6 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, return ret; } -static bool -update_conj_id_ofs(uint32_t *conj_id_ofs, uint32_t n_conjs) -{ - if (*conj_id_ofs + n_conjs < *conj_id_ofs) { - /* overflow */ - return true; - } - *conj_id_ofs += n_conjs; - return false; -} - static void lflow_parse_ctrl_meter(const struct sbrec_logical_flow *lflow, struct ovn_extend_table *meter_table, @@ -762,7 +742,7 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, return expr_simplify(e); } -static bool +static void consider_logical_flow__(const struct sbrec_logical_flow *lflow, const struct sbrec_datapath_binding *dp, struct hmap *dhcp_opts, struct hmap *dhcpv6_opts, @@ -774,7 +754,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, if (!get_local_datapath(l_ctx_in->local_datapaths, dp->tunnel_key)) { VLOG_DBG("lflow "UUID_FMT" is not for local datapath, skip", UUID_ARGS(&lflow->header_.uuid)); - return true; + return; } const char *io_port = smap_get(&lflow->tags, "in_out_port"); @@ -787,14 +767,14 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, if (!pb) { VLOG_DBG("lflow "UUID_FMT" matches inport/outport %s that's not " "found, skip", UUID_ARGS(&lflow->header_.uuid), io_port); - return true; + return; } char buf[16]; get_unique_lport_key(dp->tunnel_key, pb->tunnel_key, buf, sizeof buf); if (!sset_contains(l_ctx_in->related_lport_ids, buf)) { VLOG_DBG("lflow "UUID_FMT" matches inport/outport %s that's not " "local, skip", UUID_ARGS(&lflow->header_.uuid), io_port); - return true; + return; } } @@ -837,7 +817,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, free(error); ovnacts_free(ovnacts.data, ovnacts.size); ofpbuf_uninit(&ovnacts); - return true; + return; } struct lookup_port_aux aux = { @@ -859,8 +839,6 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, struct lflow_cache_value *lcv = lflow_cache_get(l_ctx_out->lflow_cache, &lflow->header_.uuid); - 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; @@ -869,9 +847,19 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, size_t matches_size = 0; bool pg_addr_set_ref = false; - uint32_t n_conjs = 0; - bool conj_id_overflow = false; + if (lcv_type == LCACHE_T_MATCHES + && lcv->n_conjs + && !lflow_conj_ids_alloc_specified(l_ctx_out->conj_ids, + &lflow->header_.uuid, + lcv->conj_id_ofs, lcv->n_conjs)) { + /* This should happen very rarely. */ + VLOG_DBG("lflow "UUID_FMT" match cached with conjunctions, but the" + " cached ids are not available anymore. Drop the cache.", + UUID_ARGS(&lflow->header_.uuid)); + lflow_cache_delete(l_ctx_out->lflow_cache, &lflow->header_.uuid); + lcv_type = LCACHE_T_NONE; + } /* Get match expr, either from cache or from lflow match. */ switch (lcv_type) { @@ -912,17 +900,28 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, } /* Get matches, either from cache or from expr computed above. */ + uint32_t start_conj_id = 0; + uint32_t n_conjs = 0; switch (lcv_type) { case LCACHE_T_NONE: case LCACHE_T_EXPR: matches = xmalloc(sizeof *matches); n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, matches); - 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)); + UUID_ARGS(&lflow->header_.uuid)); goto done; } + if (n_conjs) { + start_conj_id = lflow_conj_ids_alloc(l_ctx_out->conj_ids, + &lflow->header_.uuid, + n_conjs); + if (!start_conj_id) { + VLOG_ERR("32-bit conjunction ids exhausted!"); + goto done; + } + matches_size = expr_matches_prepare(matches, start_conj_id - 1); + } break; case LCACHE_T_MATCHES: matches = lcv->expr_matches; @@ -935,23 +934,18 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, /* 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 && !lflow_ref_lookup(&l_ctx_out->lfrr->lflow_ref_table, &lflow->header_.uuid)) { lflow_cache_add_matches(l_ctx_out->lflow_cache, - &lflow->header_.uuid, matches, - matches_size); + &lflow->header_.uuid, start_conj_id, + n_conjs, matches, matches_size); matches = NULL; } else if (cached_expr) { lflow_cache_add_expr(l_ctx_out->lflow_cache, - &lflow->header_.uuid, conj_id_ofs, + &lflow->header_.uuid, cached_expr, expr_size(cached_expr)); cached_expr = NULL; } @@ -973,10 +967,9 @@ done: expr_destroy(cached_expr); expr_matches_destroy(matches); free(matches); - return !conj_id_overflow; } -static bool +static void consider_logical_flow(const struct sbrec_logical_flow *lflow, struct hmap *dhcp_opts, struct hmap *dhcpv6_opts, struct hmap *nd_ra_opts, @@ -986,30 +979,27 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, { const struct sbrec_logical_dp_group *dp_group = lflow->logical_dp_group; const struct sbrec_datapath_binding *dp = lflow->logical_datapath; - bool ret = true; if (!dp_group && !dp) { VLOG_DBG("lflow "UUID_FMT" has no datapath binding, skip", UUID_ARGS(&lflow->header_.uuid)); - return true; + return; } ovs_assert(!dp_group || !dp); - if (dp && !consider_logical_flow__(lflow, dp, - dhcp_opts, dhcpv6_opts, nd_ra_opts, - controller_event_opts, - l_ctx_in, l_ctx_out)) { - ret = false; + if (dp) { + consider_logical_flow__(lflow, dp, + dhcp_opts, dhcpv6_opts, nd_ra_opts, + controller_event_opts, + l_ctx_in, l_ctx_out); + return; } for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) { - if (!consider_logical_flow__(lflow, dp_group->datapaths[i], - dhcp_opts, dhcpv6_opts, nd_ra_opts, - controller_event_opts, - l_ctx_in, l_ctx_out)) { - ret = false; - } + consider_logical_flow__(lflow, dp_group->datapaths[i], + dhcp_opts, dhcpv6_opts, nd_ra_opts, + controller_event_opts, + l_ctx_in, l_ctx_out); } - return ret; } static void @@ -1933,13 +1923,9 @@ lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp, const struct sbrec_logical_flow *lflow; SBREC_LOGICAL_FLOW_FOR_EACH_EQUAL ( lflow, lf_row, l_ctx_in->sbrec_logical_flow_by_logical_datapath) { - if (!consider_logical_flow__(lflow, dp, &dhcp_opts, &dhcpv6_opts, - &nd_ra_opts, &controller_event_opts, - l_ctx_in, l_ctx_out)) { - handled = false; - l_ctx_out->conj_id_overflow = true; - goto lflow_processing_end; - } + consider_logical_flow__(lflow, dp, &dhcp_opts, &dhcpv6_opts, + &nd_ra_opts, &controller_event_opts, + l_ctx_in, l_ctx_out); } sbrec_logical_flow_index_destroy_row(lf_row); @@ -1963,16 +1949,11 @@ lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp, sbrec_logical_flow_index_set_logical_dp_group(lf_row, ldpg); SBREC_LOGICAL_FLOW_FOR_EACH_EQUAL ( lflow, lf_row, l_ctx_in->sbrec_logical_flow_by_logical_dp_group) { - if (!consider_logical_flow__(lflow, dp, &dhcp_opts, &dhcpv6_opts, - &nd_ra_opts, &controller_event_opts, - l_ctx_in, l_ctx_out)) { - handled = false; - l_ctx_out->conj_id_overflow = true; - goto lflow_processing_end; - } + consider_logical_flow__(lflow, dp, &dhcp_opts, &dhcpv6_opts, + &nd_ra_opts, &controller_event_opts, + l_ctx_in, l_ctx_out); } } -lflow_processing_end: sbrec_logical_flow_index_destroy_row(lf_row); dhcp_opts_destroy(&dhcp_opts); diff --git a/controller/lflow.h b/controller/lflow.h index 61ffbc0cc..b2f3385e5 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -35,6 +35,7 @@ #include #include "lflow-cache.h" +#include "lflow-conj-ids.h" #include "openvswitch/hmap.h" #include "openvswitch/uuid.h" #include "openvswitch/list.h" @@ -156,8 +157,7 @@ struct lflow_ctx_out { struct ovn_extend_table *meter_table; struct lflow_resource_ref *lfrr; struct lflow_cache *lflow_cache; - uint32_t *conj_id_ofs; - bool conj_id_overflow; + struct conj_ids *conj_ids; struct simap *hairpin_lb_ids; struct id_pool *hairpin_id_pool; }; diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index f223e0f09..661e51f0b 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -39,6 +39,7 @@ #include "openvswitch/hmap.h" #include "lflow.h" #include "lflow-cache.h" +#include "lflow-conj-ids.h" #include "lib/vswitch-idl.h" #include "local_data.h" #include "lport.h" @@ -2150,7 +2151,6 @@ non_vif_data_ovs_iface_handler(struct engine_node *node, void *data OVS_UNUSED) } struct lflow_output_persistent_data { - uint32_t conj_id_ofs; struct lflow_cache *lflow_cache; }; @@ -2168,6 +2168,8 @@ struct ed_type_lflow_output { struct ovn_extend_table meter_table; /* lflow resource cross reference */ struct lflow_resource_ref lflow_resource_ref; + /* conjunciton ID usage information of lflows */ + struct conj_ids conj_ids; /* Data which is persistent and not cleared during * full recompute. */ @@ -2293,9 +2295,8 @@ init_lflow_ctx(struct engine_node *node, l_ctx_out->group_table = &fo->group_table; 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; + l_ctx_out->conj_ids = &fo->conj_ids; l_ctx_out->lflow_cache = fo->pd.lflow_cache; - l_ctx_out->conj_id_overflow = false; l_ctx_out->hairpin_id_pool = fo->hd.pool; l_ctx_out->hairpin_lb_ids = &fo->hd.ids; } @@ -2308,8 +2309,8 @@ en_lflow_output_init(struct engine_node *node OVS_UNUSED, ovn_desired_flow_table_init(&data->flow_table); ovn_extend_table_init(&data->group_table); ovn_extend_table_init(&data->meter_table); - data->pd.conj_id_ofs = 1; lflow_resource_init(&data->lflow_resource_ref); + lflow_conj_ids_init(&data->conj_ids); simap_init(&data->hd.ids); data->hd.pool = id_pool_create(1, UINT32_MAX - 1); return data; @@ -2323,6 +2324,7 @@ en_lflow_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_conj_ids_destroy(&flow_output_data->conj_ids); lflow_cache_destroy(flow_output_data->pd.lflow_cache); simap_destroy(&flow_output_data->hd.ids); id_pool_destroy(flow_output_data->hd.pool); @@ -2371,37 +2373,18 @@ en_lflow_output_run(struct engine_node *node, void *data) ovn_extend_table_clear(group_table, false /* desired */); ovn_extend_table_clear(meter_table, false /* desired */); lflow_resource_clear(lfrr); + lflow_conj_ids_clear(&fo->conj_ids); } struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx; fo->pd.lflow_cache = ctrl_ctx->lflow_cache; - if (!lflow_cache_is_enabled(fo->pd.lflow_cache)) { - fo->pd.conj_id_ofs = 1; - } - struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; init_lflow_ctx(node, rt_data, non_vif_data, fo, &l_ctx_in, &l_ctx_out); lflow_run(&l_ctx_in, &l_ctx_out); - if (l_ctx_out.conj_id_overflow) { - /* Conjunction ids overflow. There can be many holes in between. - * Destroy lflow cache and call lflow_run() again. */ - ovn_desired_flow_table_clear(lflow_table); - ovn_extend_table_clear(group_table, false /* desired */); - ovn_extend_table_clear(meter_table, false /* desired */); - lflow_resource_clear(lfrr); - fo->pd.conj_id_ofs = 1; - 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) { - VLOG_WARN("Conjunction id overflow."); - } - } - engine_set_node_state(node, EN_UPDATED); } @@ -4158,7 +4141,6 @@ lflow_cache_flush_cmd(struct unixctl_conn *conn OVS_UNUSED, VLOG_INFO("User triggered lflow cache flush."); struct lflow_output_persistent_data *fo_pd = arg_; lflow_cache_flush(fo_pd->lflow_cache); - fo_pd->conj_id_ofs = 1; engine_set_force_recompute(true); poll_immediate_wake(); unixctl_command_reply(conn, NULL); diff --git a/controller/test-lflow-cache.c b/controller/test-lflow-cache.c index 95e619ee0..6f7a3c389 100644 --- a/controller/test-lflow-cache.c +++ b/controller/test-lflow-cache.c @@ -36,19 +36,22 @@ static void test_lflow_cache_add__(struct lflow_cache *lc, const char *op_type, const struct uuid *lflow_uuid, unsigned int conj_id_ofs, + unsigned int n_conjs, struct expr *e) { printf("ADD %s:\n", op_type); printf(" conj-id-ofs: %u\n", conj_id_ofs); + printf(" n_conjs: %u\n", n_conjs); if (!strcmp(op_type, "expr")) { - lflow_cache_add_expr(lc, lflow_uuid, conj_id_ofs, expr_clone(e), + lflow_cache_add_expr(lc, lflow_uuid, 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_uuid, matches, + lflow_cache_add_matches(lc, lflow_uuid, + conj_id_ofs, n_conjs, matches, TEST_LFLOW_CACHE_VALUE_SIZE); } else { OVS_NOT_REACHED(); @@ -68,6 +71,7 @@ test_lflow_cache_lookup__(struct lflow_cache *lc, } printf(" conj_id_ofs: %"PRIu32"\n", lcv->conj_id_ofs); + printf(" n_conjs: %"PRIu32"\n", lcv->n_conjs); switch (lcv->type) { case LCACHE_T_EXPR: printf(" type: expr\n"); @@ -139,6 +143,12 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx) goto done; } + unsigned int n_conjs; + if (!test_read_uint_value(ctx, shift++, "n_conjs", + &n_conjs)) { + goto done; + } + if (n_lflow_uuids == n_allocated_lflow_uuids) { lflow_uuids = x2nrealloc(lflow_uuids, &n_allocated_lflow_uuids, sizeof *lflow_uuids); @@ -146,7 +156,8 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx) struct uuid *lflow_uuid = &lflow_uuids[n_lflow_uuids++]; uuid_generate(lflow_uuid); - test_lflow_cache_add__(lc, op_type, lflow_uuid, conj_id_ofs, e); + test_lflow_cache_add__(lc, op_type, lflow_uuid, conj_id_ofs, + n_conjs, e); test_lflow_cache_lookup__(lc, lflow_uuid); } else if (!strcmp(op, "add-del")) { const char *op_type = test_read_value(ctx, shift++, "op_type"); @@ -160,9 +171,16 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx) goto done; } + unsigned int n_conjs; + if (!test_read_uint_value(ctx, shift++, "n_conjs", + &n_conjs)) { + goto done; + } + struct uuid lflow_uuid; uuid_generate(&lflow_uuid); - test_lflow_cache_add__(lc, op_type, &lflow_uuid, conj_id_ofs, e); + test_lflow_cache_add__(lc, op_type, &lflow_uuid, conj_id_ofs, + n_conjs, e); test_lflow_cache_lookup__(lc, &lflow_uuid); test_lflow_cache_delete__(lc, &lflow_uuid); test_lflow_cache_lookup__(lc, &lflow_uuid); @@ -246,10 +264,10 @@ test_lflow_cache_negative(struct ovs_cmdl_context *ctx OVS_UNUSED) ovs_assert(expr_to_matches(e, NULL, NULL, matches) == 0); ovs_assert(hmap_count(matches) == 1); - 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, + lflow_cache_add_expr(lcs[i], NULL, NULL, 0); + lflow_cache_add_expr(lcs[i], NULL, e, expr_size(e)); + lflow_cache_add_matches(lcs[i], NULL, 0, 0, NULL, 0); + lflow_cache_add_matches(lcs[i], NULL, 0, 0, matches, TEST_LFLOW_CACHE_VALUE_SIZE); lflow_cache_destroy(lcs[i]); } @@ -260,7 +278,7 @@ 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, + {"lflow_cache_operations", NULL, 4, INT_MAX, test_lflow_cache_operations, OVS_RO}, {"lflow_cache_negative", NULL, 0, 0, test_lflow_cache_negative, OVS_RO}, diff --git a/controller/test-lflow-conj-ids.c b/controller/test-lflow-conj-ids.c new file mode 100644 index 000000000..1273f9a4c --- /dev/null +++ b/controller/test-lflow-conj-ids.c @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2021, NVIDIA CORPORATION. All rights reserved. + * + * 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 "tests/ovstest.h" +#include "tests/test-utils.h" +#include "util.h" +#include "lib/uuid.h" + +#include "lflow-conj-ids.h" + +static bool +parse_lflow_uuid(struct ovs_cmdl_context *ctx, unsigned int shift, + struct uuid *uuid) +{ + const char *uuid_s = test_read_value(ctx, shift++, "lflow_uuid"); + if (!uuid_s) { + return false; + } + if (!uuid_from_string(uuid, uuid_s)) { + printf("Expected uuid, got %s.\n", uuid_s); + return false; + } + return true; +} + +static void +test_conj_ids_operations(struct ovs_cmdl_context *ctx) +{ + unsigned int shift = 1; + unsigned int n_ops; + struct conj_ids conj_ids; + lflow_conj_ids_init(&conj_ids); + + 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; + } + + if (!strcmp(op, "alloc")) { + struct uuid uuid; + if (!parse_lflow_uuid(ctx, shift++, &uuid)) { + goto done; + } + + unsigned int n_conjs; + if (!test_read_uint_value(ctx, shift++, "n_conjs", &n_conjs)) { + goto done; + } + + uint32_t start_conj_id = lflow_conj_ids_alloc(&conj_ids, &uuid, + n_conjs); + printf("alloc("UUID_FMT", %"PRIu32"): 0x%"PRIx32"\n", + UUID_ARGS(&uuid), n_conjs, start_conj_id); + } else if (!strcmp(op, "alloc-specified")) { + struct uuid uuid; + if (!parse_lflow_uuid(ctx, shift++, &uuid)) { + goto done; + } + + unsigned int start_conj_id; + if (!test_read_uint_hex_value(ctx, shift++, "start_conj_id", + &start_conj_id)) { + goto done; + } + + unsigned int n_conjs; + if (!test_read_uint_value(ctx, shift++, "n_conjs", &n_conjs)) { + goto done; + } + + bool ret = lflow_conj_ids_alloc_specified(&conj_ids, &uuid, + start_conj_id, n_conjs); + printf("alloc_specified("UUID_FMT", 0x%"PRIx32", %"PRIu32"): %s\n", + UUID_ARGS(&uuid), start_conj_id, n_conjs, + ret ? "true" : "false"); + } else if (!strcmp(op, "free")) { + struct uuid uuid; + if (!parse_lflow_uuid(ctx, shift++, &uuid)) { + goto done; + } + lflow_conj_ids_free(&conj_ids, &uuid); + printf("free("UUID_FMT")\n", UUID_ARGS(&uuid)); + } else { + printf("Unknown operation: %s\n", op); + goto done; + } + } +done: + lflow_conj_ids_destroy(&conj_ids); +} + +static void +test_lflow_conj_ids_main(int argc, char *argv[]) +{ + set_program_name(argv[0]); + static const struct ovs_cmdl_command commands[] = { + {"operations", NULL, 1, INT_MAX, + test_conj_ids_operations, 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-conj-ids", test_lflow_conj_ids_main); diff --git a/tests/automake.mk b/tests/automake.mk index 685d78c5b..a08dfa632 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -38,6 +38,7 @@ TESTSUITE_AT = \ tests/ovn-ipam.at \ tests/ovn-features.at \ tests/ovn-lflow-cache.at \ + tests/ovn-lflow-conj-ids.at \ tests/ovn-ipsec.at \ tests/ovn-vif-plug.at @@ -243,6 +244,7 @@ tests_ovstest_SOURCES = \ tests/test-utils.h \ tests/test-ovn.c \ controller/test-lflow-cache.c \ + controller/test-lflow-conj-ids.c \ controller/test-ofctrl-seqno.c \ controller/test-vif-plug.c \ lib/test-ovn-features.c \ @@ -255,6 +257,7 @@ tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \ controller/ha-chassis.$(OBJEXT) \ controller/if-status.$(OBJEXT) \ controller/lflow-cache.$(OBJEXT) \ + controller/lflow-conj-ids.$(OBJEXT) \ controller/local_data.$(OBJEXT) \ controller/lport.$(OBJEXT) \ controller/ofctrl-seqno.$(OBJEXT) \ diff --git a/tests/ovn-lflow-cache.at b/tests/ovn-lflow-cache.at index 97d24d0dc..593d3eaac 100644 --- a/tests/ovn-lflow-cache.at +++ b/tests/ovn-lflow-cache.at @@ -7,8 +7,8 @@ AT_SETUP([unit test -- lflow-cache single add/lookup]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ true 2 \ - add expr 2 \ - add matches 3 | grep -v 'Mem usage (KB)'], + add expr 2 1 \ + add matches 3 2 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true high-watermark : 0 @@ -18,8 +18,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 2 + n_conjs: 1 LOOKUP: - conj_id_ofs: 2 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 1 @@ -29,8 +31,10 @@ cache-matches : 0 trim count : 0 ADD matches: conj-id-ofs: 3 + n_conjs: 2 LOOKUP: - conj_id_ofs: 0 + conj_id_ofs: 3 + n_conjs: 2 type: matches Enabled: true high-watermark : 2 @@ -45,8 +49,8 @@ AT_SETUP([unit test -- lflow-cache single add/lookup/del]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ true 2 \ - add-del expr 2 \ - add-del matches 3 | grep -v 'Mem usage (KB)'], + add-del expr 2 1 \ + add-del matches 3 1 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true high-watermark : 0 @@ -56,8 +60,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 2 + n_conjs: 1 LOOKUP: - conj_id_ofs: 2 + conj_id_ofs: 0 + n_conjs: 0 type: expr DELETE LOOKUP: @@ -70,8 +76,10 @@ cache-matches : 0 trim count : 0 ADD matches: conj-id-ofs: 3 + n_conjs: 1 LOOKUP: - conj_id_ofs: 0 + conj_id_ofs: 3 + n_conjs: 1 type: matches DELETE LOOKUP: @@ -89,8 +97,8 @@ AT_SETUP([unit test -- lflow-cache disabled single add/lookup/del]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ false 2 \ - add expr 2 \ - add matches 3 | grep -v 'Mem usage (KB)'], + add expr 2 1 \ + add matches 3 1 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: false high-watermark : 0 @@ -100,6 +108,7 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 2 + n_conjs: 1 LOOKUP: not found Enabled: false @@ -110,6 +119,7 @@ cache-matches : 0 trim count : 0 ADD matches: conj-id-ofs: 3 + n_conjs: 1 LOOKUP: not found Enabled: false @@ -125,14 +135,14 @@ AT_SETUP([unit test -- lflow-cache disable/enable/flush]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ true 9 \ - add expr 2 \ - add matches 3 \ + add expr 2 1 \ + add matches 3 1 \ disable \ - add expr 5 \ - add matches 6 \ + add expr 5 1 \ + add matches 6 1 \ enable 1000 1024 \ - add expr 8 \ - add matches 9 \ + add expr 8 1 \ + add matches 9 1 \ flush | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true @@ -143,8 +153,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 2 + n_conjs: 1 LOOKUP: - conj_id_ofs: 2 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 1 @@ -154,8 +166,10 @@ cache-matches : 0 trim count : 0 ADD matches: conj-id-ofs: 3 + n_conjs: 1 LOOKUP: - conj_id_ofs: 0 + conj_id_ofs: 3 + n_conjs: 1 type: matches Enabled: true high-watermark : 2 @@ -173,6 +187,7 @@ dnl At "disable" the cache was flushed. trim count : 1 ADD expr: conj-id-ofs: 5 + n_conjs: 1 LOOKUP: not found Enabled: false @@ -183,6 +198,7 @@ cache-matches : 0 trim count : 1 ADD matches: conj-id-ofs: 6 + n_conjs: 1 LOOKUP: not found Enabled: false @@ -200,8 +216,10 @@ cache-matches : 0 trim count : 1 ADD expr: conj-id-ofs: 8 + n_conjs: 1 LOOKUP: - conj_id_ofs: 8 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 1 @@ -211,8 +229,10 @@ cache-matches : 0 trim count : 1 ADD matches: conj-id-ofs: 9 + n_conjs: 1 LOOKUP: - conj_id_ofs: 0 + conj_id_ofs: 9 + n_conjs: 1 type: matches Enabled: true high-watermark : 2 @@ -234,15 +254,15 @@ AT_SETUP([unit test -- lflow-cache set limit]) AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ true 9 \ - add expr 2 \ - add matches 3 \ + add expr 2 1 \ + add matches 3 1 \ enable 1 1024 \ - add expr 5 \ - add matches 6 \ - add expr 7 \ + add expr 5 1 \ + add matches 6 1 \ + add expr 7 1 \ enable 1 1 \ - add expr 9 \ - add matches 10 | grep -v 'Mem usage (KB)'], + add expr 9 1 \ + add matches 10 1 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true high-watermark : 0 @@ -252,8 +272,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 2 + n_conjs: 1 LOOKUP: - conj_id_ofs: 2 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 1 @@ -263,8 +285,10 @@ cache-matches : 0 trim count : 0 ADD matches: conj-id-ofs: 3 + n_conjs: 1 LOOKUP: - conj_id_ofs: 0 + conj_id_ofs: 3 + n_conjs: 1 type: matches Enabled: true high-watermark : 2 @@ -284,8 +308,10 @@ cache-matches : 0 trim count : 1 ADD expr: conj-id-ofs: 5 + n_conjs: 1 LOOKUP: - conj_id_ofs: 5 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 1 @@ -295,8 +321,10 @@ cache-matches : 0 trim count : 1 ADD matches: conj-id-ofs: 6 + n_conjs: 1 LOOKUP: - conj_id_ofs: 0 + conj_id_ofs: 6 + n_conjs: 1 type: matches dnl dnl Cache is full but we can evict the expr entry because we're adding @@ -310,6 +338,7 @@ cache-matches : 1 trim count : 1 ADD expr: conj-id-ofs: 7 + n_conjs: 1 LOOKUP: not found dnl @@ -335,6 +364,7 @@ cache-matches : 0 trim count : 2 ADD expr: conj-id-ofs: 9 + n_conjs: 1 LOOKUP: not found dnl @@ -349,6 +379,7 @@ cache-matches : 0 trim count : 2 ADD matches: conj-id-ofs: 10 + n_conjs: 1 LOOKUP: not found dnl @@ -369,11 +400,11 @@ AT_CHECK( [ovstest test-lflow-cache lflow_cache_operations \ true 12 \ enable 1000 1024 trim-limit 100 trim-wmark-perc 100 \ - add expr 1 \ - add expr 2 \ - add expr 3 \ - add expr 4 \ - add expr 5 \ + add expr 1 1 \ + add expr 2 1 \ + add expr 3 1 \ + add expr 4 1 \ + add expr 5 1 \ del \ enable 1000 1024 trim-limit 0 trim-wmark-perc 100 \ del \ @@ -396,8 +427,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 1 + n_conjs: 1 LOOKUP: - conj_id_ofs: 1 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 1 @@ -407,8 +440,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 2 + n_conjs: 1 LOOKUP: - conj_id_ofs: 2 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 2 @@ -418,8 +453,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 3 + n_conjs: 1 LOOKUP: - conj_id_ofs: 3 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 3 @@ -429,8 +466,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 4 + n_conjs: 1 LOOKUP: - conj_id_ofs: 4 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 4 @@ -440,8 +479,10 @@ cache-matches : 0 trim count : 0 ADD expr: conj-id-ofs: 5 + n_conjs: 1 LOOKUP: - conj_id_ofs: 5 + conj_id_ofs: 0 + n_conjs: 0 type: expr Enabled: true high-watermark : 5 diff --git a/tests/ovn-lflow-conj-ids.at b/tests/ovn-lflow-conj-ids.at new file mode 100644 index 000000000..818d67324 --- /dev/null +++ b/tests/ovn-lflow-conj-ids.at @@ -0,0 +1,112 @@ +# +# Unit tests for the controller/lflow-conj-ids.c module. +# +AT_BANNER([OVN unit tests - lflow-conj-ids]) + +AT_SETUP([unit test -- lflow-conj-ids basic-alloc]) + +AT_CHECK( + [ovstest test-lflow-conj-ids operations 3 \ + alloc aaaaaaaa-1111-1111-1111-111111111111 10 \ + alloc bbbbbbbb-1111-1111-1111-111111111111 10 \ + alloc cccccccc-1111-1111-1111-111111111111 10], + [0], [dnl +alloc(aaaaaaaa-1111-1111-1111-111111111111, 10): 0xaaaaaaaa +alloc(bbbbbbbb-1111-1111-1111-111111111111, 10): 0xbbbbbbbb +alloc(cccccccc-1111-1111-1111-111111111111, 10): 0xcccccccc +]) + +AT_CLEANUP + +AT_SETUP([unit test -- lflow-conj-ids alloc-with-conflict]) + +# Conflict of the same prefix +AT_CHECK( + [ovstest test-lflow-conj-ids operations 2 \ + alloc aaaaaaaa-1111-1111-1111-111111111111 1 \ + alloc aaaaaaaa-2222-1111-1111-111111111111 1], + [0], [dnl +alloc(aaaaaaaa-1111-1111-1111-111111111111, 1): 0xaaaaaaaa +alloc(aaaaaaaa-2222-1111-1111-111111111111, 1): 0xaaaaaaab +]) + +# Conflict of the different prefix but overlapping range, the second allocation +# should get the next available slot. +# Free the first range, then a new allocation should get the uuid prefix. +AT_CHECK( + [ovstest test-lflow-conj-ids operations 4 \ + alloc aaaaaaaa-1111-1111-1111-111111111111 16 \ + alloc aaaaaaab-1111-1111-1111-111111111111 1 \ + free aaaaaaaa-1111-1111-1111-111111111111 \ + alloc aaaaaaab-2222-1111-1111-111111111111 1], + [0], [dnl +alloc(aaaaaaaa-1111-1111-1111-111111111111, 16): 0xaaaaaaaa +alloc(aaaaaaab-1111-1111-1111-111111111111, 1): 0xaaaaaaba +free(aaaaaaaa-1111-1111-1111-111111111111) +alloc(aaaaaaab-2222-1111-1111-111111111111, 1): 0xaaaaaaab +]) + +# Conflict at the tail of the range. +AT_CHECK( + [ovstest test-lflow-conj-ids operations 2 \ + alloc aaaaaaaa-1111-1111-1111-111111111111 1 \ + alloc aaaaaaa0-1111-1111-1111-111111111111 11], + [0], [dnl +alloc(aaaaaaaa-1111-1111-1111-111111111111, 1): 0xaaaaaaaa +alloc(aaaaaaa0-1111-1111-1111-111111111111, 11): 0xaaaaaaab +]) + +# Realloc for the same lflow should get the same id, with the old allocations +# freeed (this is not supposed to be used but implemented for safety) +AT_CHECK( + [ovstest test-lflow-conj-ids operations 3 \ + alloc aaaaaaaa-1111-1111-1111-111111111111 16 \ + alloc aaaaaaaa-1111-1111-1111-111111111111 1 \ + alloc aaaaaaab-1111-1111-1111-111111111111 1], + [0], [dnl +alloc(aaaaaaaa-1111-1111-1111-111111111111, 16): 0xaaaaaaaa +alloc(aaaaaaaa-1111-1111-1111-111111111111, 1): 0xaaaaaaaa +alloc(aaaaaaab-1111-1111-1111-111111111111, 1): 0xaaaaaaab +]) + +AT_CLEANUP + +AT_SETUP([unit test -- lflow-conj-ids rewind]) + +AT_CHECK( + [ovstest test-lflow-conj-ids operations 3 \ + alloc ffffffff-1111-1111-1111-111111111111 2 \ + free ffffffff-1111-1111-1111-111111111111 \ + alloc 00000000-2222-1111-1111-111111111111 1], + [0], [dnl +alloc(ffffffff-1111-1111-1111-111111111111, 2): 0x1 +free(ffffffff-1111-1111-1111-111111111111) +alloc(00000000-2222-1111-1111-111111111111, 1): 0x1 +]) + +AT_CLEANUP + +AT_SETUP([unit test -- lflow-conj-ids alloc-specified]) + +AT_CHECK( + [ovstest test-lflow-conj-ids operations 4 \ + alloc 00000001-1111-1111-1111-111111111111 16 \ + alloc-specified 0000000a-1111-1111-1111-111111111111 0xa 1 \ + free 00000001-1111-1111-1111-111111111111 \ + alloc-specified 0000000a-1111-1111-1111-111111111111 0xa 1], + [0], [dnl +alloc(00000001-1111-1111-1111-111111111111, 16): 0x1 +alloc_specified(0000000a-1111-1111-1111-111111111111, 0xa, 1): false +free(00000001-1111-1111-1111-111111111111) +alloc_specified(0000000a-1111-1111-1111-111111111111, 0xa, 1): true +]) + +# alloc_specified for a range including 0 should always fail. +AT_CHECK( + [ovstest test-lflow-conj-ids operations 1 \ + alloc-specified fee1dead-1111-1111-1111-111111111111 0xffffffff 2], + [0], [dnl +alloc_specified(fee1dead-1111-1111-1111-111111111111, 0xffffffff, 2): false +]) + +AT_CLEANUP diff --git a/tests/ovn.at b/tests/ovn.at index 3a361b33f..c0ffa8254 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -14936,6 +14936,18 @@ grep conjunction.*conjunction | wc -l`]) OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \ grep conjunction.*conjunction.*conjunction | wc -l`]) +# Verify that conjunction IDs are consistent between recopmutes +old_conj_ids=`as hv1 ovs-ofctl dump-flows br-int | grep conj_id= | \ + awk -F 'conj_id=' '{ print $2 }' | awk -F ',' '{ print $1 }' | sort` +echo $old_conj_ids + +as hv1 ovn-appctl -t ovn-controller recompute +ovn-nbctl --wait=hv sync +new_conj_ids=`as hv1 ovs-ofctl dump-flows br-int | grep conj_id= | \ + awk -F 'conj_id=' '{ print $2 }' | awk -F ',' '{ print $1 }' | sort` +echo $new_conj_ids +AT_CHECK([test x"$old_conj_ids" = x"$new_conj_ids"]) + OVN_CLEANUP([hv1]) AT_CLEANUP ]) @@ -15076,9 +15088,10 @@ check ovn-nbctl --wait=hv sync # Check OVS flows, the less restrictive flows should have been installed. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ grep "priority=1003" | \ - sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl - table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) - table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) + sed 's/conjunction([[^)]]*)/conjunction()/g' | \ + sed 's/conj_id=[[0-9]]*,/conj_id=xxx,/g' | sort], [0], [dnl + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,45) @@ -15120,9 +15133,10 @@ check ovn-nbctl --wait=hv sync # Check OVS flows, the second less restrictive allow ACL should have been installed. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ grep "priority=1003" | \ - sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl - table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) - table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) + sed 's/conjunction([[^)]]*)/conjunction()/g' | \ + sed 's/conj_id=[[0-9]]*,/conj_id=xxx,/g' | sort], [0], [dnl + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,45) @@ -15137,9 +15151,10 @@ check ovn-nbctl --wait=hv sync # Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ grep "priority=1003" | \ - sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl - table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) - table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) + sed 's/conjunction([[^)]]*)/conjunction()/g' | \ + sed 's/conj_id=[[0-9]]*,/conj_id=xxx,/g' | sort], [0], [dnl + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() @@ -15176,9 +15191,10 @@ check ovn-nbctl --wait=hv sync # Check OVS flows, the less restrictive flows should have been installed. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ grep "priority=1003" | \ - sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl - table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) - table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) + sed 's/conjunction([[^)]]*)/conjunction()/g' | \ + sed 's/conj_id=[[0-9]]*,/conj_id=xxx,/g' | sort], [0], [dnl + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,45) @@ -15196,10 +15212,11 @@ check ovn-nbctl --wait=hv sync # New non-conjunctive flows should be added to match on 'udp'. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ grep "priority=1003" | \ - sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl - table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) - table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) - table=44, priority=1003,conj_id=4,ip,metadata=0x1 actions=resubmit(,45) + sed 's/conjunction([[^)]]*)/conjunction()/g' | \ + sed 's/conj_id=[[0-9]]*,/conj_id=xxx,/g' | sort], [0], [dnl + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) + table=44, priority=1003,conj_id=xxx,ip,metadata=0x1 actions=resubmit(,45) table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction(),conjunction() table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,45) diff --git a/tests/testsuite.at b/tests/testsuite.at index b716a1ad9..479e786bd 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -29,6 +29,7 @@ m4_include([tests/ovn-northd.at]) m4_include([tests/ovn-nbctl.at]) m4_include([tests/ovn-features.at]) m4_include([tests/ovn-lflow-cache.at]) +m4_include([tests/ovn-lflow-conj-ids.at]) m4_include([tests/ovn-ofctrl-seqno.at]) m4_include([tests/ovn-sbctl.at]) m4_include([tests/ovn-ic-nbctl.at])