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