From: Numan Siddique <num...@ovn.org>
When the below ACL is added -
ovn-nbctl acl-add ls1 to-lport 3
'(ip4.src==10.0.0.1 || ip4.src==10.0.0.2) &&
(ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow
ovn-controller installs the below OF flows
table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
actions=conjunction(2,1/2)
table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
actions=conjunction(2,1/2)
table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2
actions=conjunction(2,2/2)
table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1
actions=conjunction(2,2/2)
table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
When a full recompute is triggered, ovn-controller deletes the last
OF flow with the match conj_id=2 and adds the below OF flow
table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
For subsequent recomputes, the conj_id keeps increasing by 1.
This disrupts the traffic which matches on conjuction action flows.
This patch fixes this issue.
Fixes: 1213bc8270("ovn-controller: Cache logical flow expr matches.")
Suggested-by: Dumitru Ceara <dce...@redhat.com>
Signed-off-by: Numan Siddique <num...@ovn.org>
---
controller/lflow.c | 46 ++++++++++++++++++++++++++++++----------------
tests/ovn.at | 28 ++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 16 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index c02585b1e..7bb3cdaeb 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -668,9 +668,8 @@ update_conj_id_ofs(uint32_t *conj_id_ofs, uint32_t n_conjs)
static void
add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
const struct sbrec_datapath_binding *dp,
- struct hmap *matches, size_t conj_id_ofs,
- uint8_t ptable, uint8_t output_ptable,
- struct ofpbuf *ovnacts,
+ struct hmap *matches, uint8_t ptable,
+ uint8_t output_ptable, struct ofpbuf *ovnacts,
bool ingress, struct lflow_ctx_in *l_ctx_in,
struct lflow_ctx_out *l_ctx_out)
{
@@ -708,9 +707,6 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
struct expr_match *m;
HMAP_FOR_EACH (m, hmap_node, matches) {
match_set_metadata(&m->match, htonll(dp->tunnel_key));
- if (m->match.wc.masks.conj_id) {
- m->match.flow.conj_id += conj_id_ofs;
- }
if (datapath_is_switch(dp)) {
unsigned int reg_index
= (ingress ? MFF_LOG_INPORT : MFF_LOG_OUTPORT) - MFF_REG0;
@@ -744,7 +740,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
struct ofpact_conjunction *dst;
dst = ofpact_put_CONJUNCTION(&conj);
- dst->id = src->id + conj_id_ofs;
+ dst->id = src->id;
dst->clause = src->clause;
dst->n_clauses = src->n_clauses;
}
@@ -815,6 +811,22 @@ convert_match_to_expr(const struct sbrec_logical_flow
*lflow,
return expr_simplify(e);
}
+static void
+prepare_expr_matches(struct hmap *matches, uint32_t conj_id_ofs)
+{
+ struct expr_match *m;
+ HMAP_FOR_EACH (m, hmap_node, matches) {
+ if (m->match.wc.masks.conj_id) {
+ m->match.flow.conj_id += conj_id_ofs;
+ }
+
+ for (int i = 0; i < m->n; i++) {
+ struct cls_conjunction *src = &m->conjunctions[i];
+ src->id += conj_id_ofs;
+ }
+ }
+}
+
static bool
consider_logical_flow__(const struct sbrec_logical_flow *lflow,
const struct sbrec_datapath_binding *dp,
@@ -915,9 +927,9 @@ consider_logical_flow__(const struct sbrec_logical_flow
*lflow,
return true;
}
- add_matches_to_flow_table(lflow, dp, &matches, *l_ctx_out->conj_id_ofs,
- ptable, output_ptable, &ovnacts, ingress,
- l_ctx_in, l_ctx_out);
+ prepare_expr_matches(&matches, *l_ctx_out->conj_id_ofs);
+ add_matches_to_flow_table(lflow, dp, &matches, ptable, output_ptable,
+ &ovnacts, ingress, l_ctx_in, l_ctx_out);
ovnacts_free(ovnacts.data, ovnacts.size);
ofpbuf_uninit(&ovnacts);
@@ -930,10 +942,11 @@ consider_logical_flow__(const struct sbrec_logical_flow
*lflow,
lflow_cache_get(l_ctx_out->lflow_cache_map, lflow);
if (lc && lc->type == LCACHE_T_MATCHES) {
- /* 'matches' is cached. No need to do expr parsing.
+ /* 'matches' is cached. No need to do expr parsing and no need
+ * to call prepare_expr_matches() to update the conj ids.
* Add matches to flow table and return. */
- add_matches_to_flow_table(lflow, dp, lc->expr_matches, lc->conj_id_ofs,
- ptable, output_ptable, &ovnacts, ingress,
+ add_matches_to_flow_table(lflow, dp, lc->expr_matches, ptable,
+ output_ptable, &ovnacts, ingress,
l_ctx_in, l_ctx_out);
ovnacts_free(ovnacts.data, ovnacts.size);
ofpbuf_uninit(&ovnacts);
@@ -1009,10 +1022,11 @@ consider_logical_flow__(const struct sbrec_logical_flow
*lflow,
}
}
+ prepare_expr_matches(matches, lc->conj_id_ofs);
+
/* Encode OVN logical actions into OpenFlow. */
- add_matches_to_flow_table(lflow, dp, matches, lc->conj_id_ofs,
- ptable, output_ptable, &ovnacts, ingress,
- l_ctx_in, l_ctx_out);
+ add_matches_to_flow_table(lflow, dp, matches, ptable, output_ptable,
+ &ovnacts, ingress, l_ctx_in, l_ctx_out);
ovnacts_free(ovnacts.data, ovnacts.size);
ofpbuf_uninit(&ovnacts);
diff --git a/tests/ovn.at b/tests/ovn.at
index 718b2eec5..1161cac50 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -13884,6 +13884,34 @@ reset_pcap_file hv1-vif2 hv1/vif2
rm -f 2.packets
> 2.expected
+# Trigger recompute and make sure that the traffic still works as expected.
+as hv1 ovn-appctl -t ovn-controller recompute
+
+# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed.
+for src in `seq 1 2`; do
+ for dst in `seq 3 4`; do
+ sip=`ip_to_hex 10 0 0 $src`
+ dip=`ip_to_hex 10 0 0 $dst`
+
+ test_ip 1 f00000000001 f00000000002 $sip $dip 2
+ done
+done
+
+# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped.
+dip=`ip_to_hex 10 0 0 5`
+for src in `seq 1 2`; do
+ sip=`ip_to_hex 10 0 0 $src`
+
+ test_ip 1 f00000000001 f00000000002 $sip $dip
+done
+
+cat 2.expected > expout
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
+AT_CHECK([cat 2.packets], [0], [expout])
+reset_pcap_file hv1-vif2 hv1/vif2
+rm -f 2.packets
+> 2.expected
+
# Add two less restrictive allow ACLs for src IP 10.0.0.1.
ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' allow
ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow
--
2.29.2
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
