@@ -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);
@@ -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);
@@ -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:
@@ -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);
@@ -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
])
@@ -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])
@@ -24462,156 +24526,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
@@ -24642,31 +24556,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)
@@ -24674,19 +24583,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], [])
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 <hzhou@ovn.org> --- 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(-)