125 lines
4.2 KiB
Diff
125 lines
4.2 KiB
Diff
From 1f82d15cb88333520c367365cf76a7b97bd18e16 Mon Sep 17 00:00:00 2001
|
|
From: Numan Siddique <nusiddiq@redhat.com>
|
|
Date: Sat, 14 Sep 2019 13:19:17 +0530
|
|
Subject: ovn: Exclude inport and outport symbol tables from conjunction
|
|
|
|
If there are multiple ACLs associated with a port group and they
|
|
match on a range of some field, then ovn-controller doesn't install
|
|
the flows properly and this results in broken ACL functionality.
|
|
|
|
For example, if there is a port group - pg1 with logical ports - [p1, p2]
|
|
and if there are below ACLs (only match condition is shown)
|
|
|
|
1 - outport == @pg1 && ip4 && tcp.dst >= 500 && tcp.dst <= 501
|
|
2 - outport == @pg1 && ip4 && tcp.dst >= 600 && tcp.dst <= 601
|
|
|
|
The first ACL will result in the below OF flows
|
|
|
|
1. conj_id=1,tcp
|
|
2. tcp,reg15=0x11: conjunction(1, 1/2)
|
|
3. tcp,reg15=0x12: conjunction(1, 1/2)
|
|
5. tcp,tp_dst=500: conjunction(1, 2/2)
|
|
6. tcp,tp_dst=501: conjunction(1, 2/2)
|
|
|
|
The second ACL will result in the below OF flows
|
|
7. conj_id=2,tcp
|
|
8. tcp,reg15=0x11: conjunction(2, 1/2)
|
|
9. tcp,reg15=0x12: conjunction(2, 1/2)
|
|
11. tcp,tp_dst=600: conjunction(2, 2/2)
|
|
12. tcp,tp_dst=601: conjunction(2, 3/2)
|
|
|
|
The OF flows (2) and (8) have the exact match but with different action.
|
|
This results in only one of the flows getting installed. The same goes
|
|
for the flows (3) and (9). And this completely breaks the ACL functionality
|
|
for such scenarios.
|
|
|
|
In order to fix this issue, this patch excludes the 'inport' and 'outport' symbols
|
|
from conjunction. With this patch we will have the below flows.
|
|
|
|
tcp,reg15=0x11,tp_dst=500
|
|
tcp,reg15=0x11,tp_dst=501
|
|
tcp,reg15=0x12,tp_dst=500
|
|
tcp,reg15=0x12,tp_dst=501
|
|
tcp,reg15=0x13,tp_dst=500
|
|
tcp,reg15=0x13,tp_dst=501
|
|
tcp,reg15=0x11,tp_dst=600
|
|
tcp,reg15=0x11,tp_dst=601
|
|
tcp,reg15=0x12,tp_dst=600
|
|
tcp,reg15=0x12,tp_dst=601
|
|
tcp,reg15=0x13,tp_dst=600
|
|
tcp,reg15=0x13,tp_dst=601
|
|
|
|
Acked-by: Mark Michelson <mmichels@redhat.com>
|
|
Acked-by: Daniel Alvarez <dalvarez@redhat.com>
|
|
Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
|
|
|
|
(cherry-picked from ovn commit 298701dbc99645700be41680a43d049cb061847a)
|
|
|
|
Signed-off-by: Ben Pfaff <blp@ovn.org>
|
|
---
|
|
ovn/lib/expr.c | 2 +-
|
|
tests/ovn.at | 26 ++++++++++++++++++++++++++
|
|
2 files changed, 27 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
|
|
index e4c650f7c..c0871e1e8 100644
|
|
--- a/ovn/lib/expr.c
|
|
+++ b/ovn/lib/expr.c
|
|
@@ -1499,7 +1499,7 @@ expr_symtab_add_string(struct shash *symtab, const char *name,
|
|
const struct mf_field *field = mf_from_id(id);
|
|
struct expr_symbol *symbol;
|
|
|
|
- symbol = add_symbol(symtab, name, 0, prereqs, EXPR_L_NOMINAL, false,
|
|
+ symbol = add_symbol(symtab, name, 0, prereqs, EXPR_L_NOMINAL, true,
|
|
field->writable);
|
|
symbol->field = field;
|
|
return symbol;
|
|
diff --git a/tests/ovn.at b/tests/ovn.at
|
|
index 2361524ff..54aa19bb2 100644
|
|
--- a/tests/ovn.at
|
|
+++ b/tests/ovn.at
|
|
@@ -573,6 +573,24 @@ ip,reg14=0x6
|
|
ipv6,reg14=0x5
|
|
ipv6,reg14=0x6
|
|
])
|
|
+AT_CHECK([expr_to_flow 'inport == {"eth0", "eth1", "eth2"} && ip4 && tcp && tcp.dst == {500, 501}'], [0], [dnl
|
|
+tcp,reg14=0x5,tp_dst=500
|
|
+tcp,reg14=0x5,tp_dst=501
|
|
+tcp,reg14=0x6,tp_dst=500
|
|
+tcp,reg14=0x6,tp_dst=501
|
|
+])
|
|
+AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 && tcp && tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl
|
|
+conj_id=1,tcp,reg15=0x5
|
|
+conj_id=2,tcp,reg15=0x6
|
|
+tcp,reg15=0x5,tp_dst=500: conjunction(1, 0/2)
|
|
+tcp,reg15=0x5,tp_dst=501: conjunction(1, 0/2)
|
|
+tcp,reg15=0x5,tp_src=400: conjunction(1, 1/2)
|
|
+tcp,reg15=0x5,tp_src=401: conjunction(1, 1/2)
|
|
+tcp,reg15=0x6,tp_dst=500: conjunction(2, 0/2)
|
|
+tcp,reg15=0x6,tp_dst=501: conjunction(2, 0/2)
|
|
+tcp,reg15=0x6,tp_src=400: conjunction(2, 1/2)
|
|
+tcp,reg15=0x6,tp_src=401: conjunction(2, 1/2)
|
|
+])
|
|
AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0], [dnl
|
|
(no flows)
|
|
])
|
|
@@ -677,6 +695,14 @@ reg15=0x11
|
|
reg15=0x12
|
|
reg15=0x13
|
|
])
|
|
+AT_CHECK([expr_to_flow 'outport == @pg1 && ip4.src == {10.0.0.4, 10.0.0.5}'], [0], [dnl
|
|
+ip,reg15=0x11,nw_src=10.0.0.4
|
|
+ip,reg15=0x11,nw_src=10.0.0.5
|
|
+ip,reg15=0x12,nw_src=10.0.0.4
|
|
+ip,reg15=0x12,nw_src=10.0.0.5
|
|
+ip,reg15=0x13,nw_src=10.0.0.4
|
|
+ip,reg15=0x13,nw_src=10.0.0.5
|
|
+])
|
|
AT_CHECK([expr_to_flow 'outport == {@pg_empty}'], [0], [dnl
|
|
(no flows)
|
|
])
|
|
--
|
|
2.14.1
|
|
|
|
|