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 <[email protected]> --- 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], []) -- 2.30.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
