Considering the following configuration:

$ovn-nbctl acl-list sw01
from-lport   100 (inport == "sw01-port1" && udp.dst == 5201) allow-related 
[after-lb]
from-lport    10 (inport == "sw01-port1" && udp) drop [after-lb]

$ovn-nbctl list acl
_uuid               : e440336a-84d3-4a6d-95a9-edd1db1c3631
action              : drop
direction           : from-lport
external_ids        : {}
label               : 0
log                 : false
match               : "inport == \"sw01-port1\" && udp"
meter               : []
name                : []
options             : {apply-after-lb="true"}
priority            : 10
sample_est          : ac6a6efc-a2e0-4d68-b5f8-8cd91113e554
sample_new          : 5cdad2ab-4390-4772-ac40-74aa2980c06e
severity            : []
tier                : 0

_uuid               : 85ef08d7-aacc-41d7-b808-6ab011edd753
action              : allow-related
direction           : from-lport
external_ids        : {}
label               : 0
log                 : false
match               : "inport == \"sw01-port1\" && udp.dst == 5201"
meter               : []
name                : []
options             : {apply-after-lb="true"}
priority            : 100
sample_est          : 143ce7e2-fd13-4d5e-930c-133d5cf87d0d
sample_new          : 1d1a0a05-2a8a-4c72-ad35-77d7e2908183
severity            : []
tier                : 0

If the priority-100 acl is removed, the udp traffic with destination port
5201 will be dropped however ovn-controller will continue sampling the
existing connection with the observationPointID associated to the removed
acl. Fix the issue updating the ct_label.obs_point_id for the connection
marked with ct_mark.blocked.

Fixes: d15b12da6fe6 ("northd: Add ACL Sampling.")
Repoerted-at: https://issues.redhat.com/browse/FDP-819
Signed-off-by: Lorenzo Bianconi <[email protected]>

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Changes since v1:
- commit ct_label.obs_point_id set to 0 if the drop acl has no samples
  configured
---
 northd/northd.c     |  5 ++++-
 tests/ovn-northd.at | 20 ++++++++++----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 0aa0de637..a5bd9fd50 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -7205,7 +7205,10 @@ consider_acl(struct lflow_table *lflows, const struct 
ovn_datapath *od,
         /* For drop ACLs just sample all packets as "new" packets. */
         build_acl_sample_label_action(actions, acl, acl->sample_new, NULL,
                                       obs_stage);
-        ds_put_cstr(actions, "ct_commit { ct_mark.blocked = 1; }; next;");
+        uint32_t obs_pid = acl->sample_est ? acl->sample_est->metadata : 0;
+        ds_put_format(actions,
+                      "ct_commit { ct_mark.blocked = 1; "
+                      "ct_label.obs_point_id = %"PRIu32"; }; next;", obs_pid);
         ovn_lflow_add_with_hint(lflows, od, stage, priority,
                                 ds_cstr(match), ds_cstr(actions),
                                 &acl->header_, lflow_ref);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d6a8c4640..10db5398b 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2384,15 +2384,15 @@ AT_CAPTURE_FILE([sw1flows3])
 AT_CHECK([grep "ls_out_acl" sw0flows3 sw1flows3 | grep pg0 | 
ovn_strip_lflows], [0], [dnl
 sw0flows3:  table=??(ls_out_acl_eval    ), priority=2001 , match=(reg0[[7]] == 
1 && (outport == @pg0 && ip)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
 sw0flows3:  table=??(ls_out_acl_eval    ), priority=2001 , match=(reg0[[8]] == 
1 && (outport == @pg0 && ip)), action=(reg8[[16]] = 1; next;)
-sw0flows3:  table=??(ls_out_acl_eval    ), priority=2002 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; }; next;)
+sw0flows3:  table=??(ls_out_acl_eval    ), priority=2002 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; ct_label.obs_point_id = 0; }; next;)
 sw0flows3:  table=??(ls_out_acl_eval    ), priority=2002 , match=(reg0[[9]] == 
1 && (outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; next;)
-sw0flows3:  table=??(ls_out_acl_eval    ), priority=2003 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; }; next;)
+sw0flows3:  table=??(ls_out_acl_eval    ), priority=2003 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; ct_label.obs_point_id = 0; }; next;)
 sw0flows3:  table=??(ls_out_acl_eval    ), priority=2003 , match=(reg0[[9]] == 
1 && (outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; next;)
 sw1flows3:  table=??(ls_out_acl_eval    ), priority=2001 , match=(reg0[[7]] == 
1 && (outport == @pg0 && ip)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
 sw1flows3:  table=??(ls_out_acl_eval    ), priority=2001 , match=(reg0[[8]] == 
1 && (outport == @pg0 && ip)), action=(reg8[[16]] = 1; next;)
-sw1flows3:  table=??(ls_out_acl_eval    ), priority=2002 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; }; next;)
+sw1flows3:  table=??(ls_out_acl_eval    ), priority=2002 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; ct_label.obs_point_id = 0; }; next;)
 sw1flows3:  table=??(ls_out_acl_eval    ), priority=2002 , match=(reg0[[9]] == 
1 && (outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; next;)
-sw1flows3:  table=??(ls_out_acl_eval    ), priority=2003 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; }; next;)
+sw1flows3:  table=??(ls_out_acl_eval    ), priority=2003 , match=(reg0[[10]] 
== 1 && (outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; ct_commit { 
ct_mark.blocked = 1; ct_label.obs_point_id = 0; }; next;)
 sw1flows3:  table=??(ls_out_acl_eval    ), priority=2003 , match=(reg0[[9]] == 
1 && (outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; next;)
 ])
 
@@ -7711,13 +7711,13 @@ AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint" 
lsflows | ovn_strip_lflo
   table=??(ls_in_acl_eval     ), priority=0    , match=(1), action=(next;)
   table=??(ls_in_acl_eval     ), priority=1    , match=(ip && !ct.est), 
action=(reg0[[1]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=1    , match=(ip && ct.est && 
ct_mark.blocked == 1), action=(reg0[[1]] = 1; reg8[[16]] = 1; next;)
-  table=??(ls_in_acl_eval     ), priority=2001 , match=(reg0[[10]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; ct_commit { ct_mark.blocked = 1; }; next;)
+  table=??(ls_in_acl_eval     ), priority=2001 , match=(reg0[[10]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; ct_commit { ct_mark.blocked = 1; 
ct_label.obs_point_id = 0; }; next;)
   table=??(ls_in_acl_eval     ), priority=2001 , match=(reg0[[9]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=2002 , match=(reg0[[7]] == 1 && (ip4 
&& tcp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=2002 , match=(reg0[[8]] == 1 && (ip4 
&& tcp)), action=(reg8[[16]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=2003 , match=(reg0[[7]] == 1 && (ip4 
&& icmp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=2003 , match=(reg0[[8]] == 1 && (ip4 
&& icmp)), action=(reg8[[16]] = 1; next;)
-  table=??(ls_in_acl_eval     ), priority=2004 , match=(reg0[[10]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; ct_commit { 
ct_mark.blocked = 1; }; next;)
+  table=??(ls_in_acl_eval     ), priority=2004 , match=(reg0[[10]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; ct_commit { 
ct_mark.blocked = 1; ct_label.obs_point_id = 0; }; next;)
   table=??(ls_in_acl_eval     ), priority=2004 , match=(reg0[[9]] == 1 && (ip4 
&& ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=34000, match=(eth.dst == 
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=65532, match=(!ct.est && ct.rel && 
!ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] = 1; reg8[[16]] 
= 1; ct_commit_nat;)
@@ -7761,13 +7761,13 @@ AT_CAPTURE_FILE([lsflows])
 
 AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint" lsflows | 
ovn_strip_lflows], [0], [dnl
   table=??(ls_in_acl_after_lb_eval), priority=0    , match=(1), action=(next;)
-  table=??(ls_in_acl_after_lb_eval), priority=2001 , match=(reg0[[10]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; ct_commit { ct_mark.blocked = 1; }; next;)
+  table=??(ls_in_acl_after_lb_eval), priority=2001 , match=(reg0[[10]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; ct_commit { ct_mark.blocked = 1; 
ct_label.obs_point_id = 0; }; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2001 , match=(reg0[[9]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2002 , match=(reg0[[7]] == 1 && 
(ip4 && tcp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2002 , match=(reg0[[8]] == 1 && 
(ip4 && tcp)), action=(reg8[[16]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[7]] == 1 && 
(ip4 && icmp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[8]] == 1 && 
(ip4 && icmp)), action=(reg8[[16]] = 1; next;)
-  table=??(ls_in_acl_after_lb_eval), priority=2004 , match=(reg0[[10]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; ct_commit { 
ct_mark.blocked = 1; }; next;)
+  table=??(ls_in_acl_after_lb_eval), priority=2004 , match=(reg0[[10]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; ct_commit { 
ct_mark.blocked = 1; ct_label.obs_point_id = 0; }; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2004 , match=(reg0[[9]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=65532, match=(nd || nd_ra || 
nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=65532, match=(reg0[[17]] == 1), 
action=(reg8[[16]] = 1; next;)
@@ -7816,9 +7816,9 @@ AT_CAPTURE_FILE([lsflows])
 
 AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint" lsflows | 
ovn_strip_lflows], [0], [dnl
   table=??(ls_in_acl_after_lb_eval), priority=0    , match=(1), action=(next;)
-  table=??(ls_in_acl_after_lb_eval), priority=2001 , match=(reg0[[10]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; ct_commit { ct_mark.blocked = 1; }; next;)
+  table=??(ls_in_acl_after_lb_eval), priority=2001 , match=(reg0[[10]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; ct_commit { ct_mark.blocked = 1; 
ct_label.obs_point_id = 0; }; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2001 , match=(reg0[[9]] == 1 && 
(ip4)), action=(reg8[[17]] = 1; next;)
-  table=??(ls_in_acl_after_lb_eval), priority=2004 , match=(reg0[[10]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; ct_commit { 
ct_mark.blocked = 1; }; next;)
+  table=??(ls_in_acl_after_lb_eval), priority=2004 , match=(reg0[[10]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; ct_commit { 
ct_mark.blocked = 1; ct_label.obs_point_id = 0; }; next;)
   table=??(ls_in_acl_after_lb_eval), priority=2004 , match=(reg0[[9]] == 1 && 
(ip4 && ip4.dst == 10.0.0.2)), action=(reg8[[17]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=65532, match=(nd || nd_ra || 
nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
   table=??(ls_in_acl_after_lb_eval), priority=65532, match=(reg0[[17]] == 1), 
action=(reg8[[16]] = 1; next;)
-- 
2.46.2

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to