Skip to content

Commit

Permalink
northd: Always commit ct.est sampled traffic in the original direction.
Browse files Browse the repository at this point in the history 
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) allow-related [after-lb]

$ovn-nbctl list acl
_uuid               : e440336a-84d3-4a6d-95a9-edd1db1c3631
action              : allow-related
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 hit the second ACL, however ovn-controller will continue
sampling the existing connection with the observationPointID associated to
the removed ACL.
Fix the issue always committing ct.est sampled traffic in the original
direction in order to update the observationPointID stored in the connection
tracking table.

Fixes: d15b12d ("northd: Add ACL Sampling.")
Reported-at: https://issues.redhat.com/browse/FDP-848
Signed-off-by: Lorenzo Bianconi <[email protected]>
Acked-by: Ales Musil <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
(cherry picked from commit 3b32b7d)
  • Loading branch information
@LorenzoBianconi @dceara
LorenzoBianconi authored and dceara committed Dec 2, 2024
1 parent e51b449 commit 0054ab7
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 8 deletions.
2 changes: 1 addition & 1 deletion northd/northd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7165,7 +7165,7 @@ consider_acl(struct lflow_table *lflows, const struct ovn_datapath *od,
ds_truncate(actions, log_verdict_len);
ds_put_format(match, REGBIT_ACL_HINT_ALLOW " == 1 && (%s)",
acl->match);
if (acl->label) {
if (acl->label || acl->sample_est) {
ds_put_cstr(actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
}

Expand Down
21 changes: 14 additions & 7 deletions tests/ovn-northd.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12723,7 +12723,7 @@ ovn-nbctl --wait=sb \
--sample-new=@sample1 --sample-est=@sample2 acl-add ls from-lport 1 "1" allow-related
AT_CHECK([ovn-sbctl lflow-list | grep -e ls_in_acl_sample -e ls_in_acl_eval -e ls_out_acl_sample | ovn_strip_lflows | ovn_strip_collector_set | grep -e reg3 -e reg9 -e sample], [0], [dnl
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[7]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_sample ), priority=0 , match=(1), action=(next;)
table=??(ls_in_acl_sample ), priority=1100 , match=(ip && ct.new && reg3 == 4301), action=(sample(probability=65535,collector_set=??,obs_domain=42,obs_point=4301);sample(probability=65535,collector_set=??,obs_domain=42,obs_point=4301); next;)
table=??(ls_in_acl_sample ), priority=1200 , match=(ip && ct.trk && (ct.est || ct.rel) && !ct.rpl && ct_label.obs_point_id == 4302 && ct_label.obs_unused == 0), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302);sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302); next;)
Expand All @@ -12743,6 +12743,7 @@ AT_CHECK_UNQUOTED([ovn_trace --ct new ls "$flow" | TRACE_FILTER], [0], [dnl
dnl Trace estasblished connections.
flow="$base_flow && ct_label.obs_point_id == 4302"
AT_CHECK_UNQUOTED([ovn_trace --ct est ls "$flow" | TRACE_FILTER], [0], [dnl
ct_commit { ct_mark.blocked = 0; ct_mark.obs_stage = reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; ct_label.obs_point_id = reg9; };
reg9 = 4302;
sample(probability=65535,collector_set=100,obs_domain=43,obs_point=4302);
sample(probability=65535,collector_set=200,obs_domain=43,obs_point=4302);
Expand Down Expand Up @@ -12785,7 +12786,7 @@ ovn-nbctl --wait=sb \
--apply-after-lb --sample-new=@sample1 --sample-est=@sample2 acl-add ls from-lport 1 "1" allow-related
AT_CHECK([ovn-sbctl lflow-list | grep -e ls_in_acl_after_lb_sample -e ls_in_acl_after_lb_eval -e ls_out_acl_sample | ovn_strip_lflows | ovn_strip_collector_set | grep -e reg3 -e reg9 -e sample], [0], [dnl
table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(reg0[[7]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 1; next;)
table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 1; next;)
table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 1; next;)
table=??(ls_in_acl_after_lb_sample), priority=0 , match=(1), action=(next;)
table=??(ls_in_acl_after_lb_sample), priority=1100 , match=(ip && ct.new && reg3 == 4301), action=(sample(probability=65535,collector_set=??,obs_domain=42,obs_point=4301);sample(probability=65535,collector_set=??,obs_domain=42,obs_point=4301); next;)
table=??(ls_in_acl_after_lb_sample), priority=1200 , match=(ip && ct.trk && (ct.est || ct.rel) && !ct.rpl && ct_label.obs_point_id == 4302 && ct_label.obs_unused == 0), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302);sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302); next;)
Expand All @@ -12805,6 +12806,7 @@ AT_CHECK_UNQUOTED([ovn_trace --ct new ls "$flow" | TRACE_FILTER], [0], [dnl
dnl Trace estasblished connections.
flow="$base_flow && ct_label.obs_point_id == 4302"
AT_CHECK_UNQUOTED([ovn_trace --ct est ls "$flow" | TRACE_FILTER], [0], [dnl
ct_commit { ct_mark.blocked = 0; ct_mark.obs_stage = reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; ct_label.obs_point_id = reg9; };
reg9 = 4302;
sample(probability=65535,collector_set=100,obs_domain=43,obs_point=4302);
sample(probability=65535,collector_set=200,obs_domain=43,obs_point=4302);
Expand Down Expand Up @@ -12849,7 +12851,7 @@ AT_CHECK([ovn-sbctl lflow-list | grep -e ls_out_acl_sample -e ls_out_acl_eval -e
table=??(ls_in_acl_sample ), priority=0 , match=(1), action=(next;)
table=??(ls_in_acl_sample ), priority=1200 , match=(ip && ct.trk && (ct.est || ct.rel) && ct.rpl && ct_label.obs_point_id == 4302 && ct_label.obs_unused == 0), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302);sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302); next;)
table=??(ls_out_acl_eval ), priority=1001 , match=(reg0[[7]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 2; next;)
table=??(ls_out_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 2; next;)
table=??(ls_out_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 0; reg8[[8..15]] = 0; reg8[[19..20]] = 2; next;)
table=??(ls_out_acl_sample ), priority=0 , match=(1), action=(next;)
table=??(ls_out_acl_sample ), priority=1100 , match=(ip && (ct.new || !ct.trk) && reg3 == 4301), action=(sample(probability=65535,collector_set=??,obs_domain=42,obs_point=4301);sample(probability=65535,collector_set=??,obs_domain=42,obs_point=4301); next;)
table=??(ls_out_acl_sample ), priority=1200 , match=(ip && ct.trk && (ct.est || ct.rel) && !ct.rpl && ct_label.obs_point_id == 4302 && ct_label.obs_unused == 0), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302);sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302); next;)
Expand All @@ -12868,6 +12870,7 @@ AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new ls "$flow" | TRACE_FILTER], [0],
dnl Trace estasblished connections.
flow="$base_flow && ct_label.obs_point_id == 4302"
AT_CHECK_UNQUOTED([ovn_trace --ct est --ct est ls "$flow" | TRACE_FILTER], [0], [dnl
ct_commit { ct_mark.blocked = 0; ct_mark.obs_stage = reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; ct_label.obs_point_id = reg9; };
reg9 = 4302;
sample(probability=65535,collector_set=100,obs_domain=43,obs_point=4302);
sample(probability=65535,collector_set=200,obs_domain=43,obs_point=4302);
Expand Down Expand Up @@ -12941,7 +12944,7 @@ ovn-nbctl --wait=sb \
--sample-new=@sample1 --sample-est=@sample2 acl-add ls from-lport 1 "1" allow-related
AT_CHECK([ovn-sbctl lflow-list | grep -e ls_in_acl_sample -e ls_in_acl_eval -e ls_out_acl_sample | ovn_strip_lflows | ovn_strip_collector_set | grep -e reg3 -e reg9 -e sample], [0], [dnl
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[7]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_sample ), priority=0 , match=(1), action=(next;)
table=??(ls_in_acl_sample ), priority=1100 , match=(ip && ct.new && reg3 == 4301), action=(sample(probability=65535,collector_set=??,obs_domain=42,obs_point=4301); next;)
table=??(ls_in_acl_sample ), priority=1200 , match=(ip && ct.trk && (ct.est || ct.rel) && !ct.rpl && ct_label.obs_point_id == 4302 && ct_label.obs_unused == 0), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=4302); next;)
Expand All @@ -12962,6 +12965,7 @@ AT_CHECK_UNQUOTED([ovn_trace --ct new ls "$flow" | TRACE_FILTER], [0], [dnl
dnl Trace estasblished connections.
flow="$base_flow && ct_label.obs_point_id == 4302"
AT_CHECK_UNQUOTED([ovn_trace --ct est ls "$flow" | TRACE_FILTER], [0], [dnl
ct_commit { ct_mark.blocked = 0; ct_mark.obs_stage = reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; ct_label.obs_point_id = reg9; };
reg8[[0..7]] = 1;
reg8[[8..15]] = 1;
reg9 = 4302;
Expand All @@ -12978,7 +12982,7 @@ ovn-nbctl --wait=sb \
--sample-new=@sample1 --sample-est=@sample2 acl-add ls from-lport 1 "1" allow-related
AT_CHECK([ovn-sbctl lflow-list | grep -e ls_in_acl_sample -e ls_in_acl_eval -e ls_out_acl_sample | ovn_strip_lflows | ovn_strip_collector_set | grep -e reg3 -e reg9 -e sample], [0], [dnl
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[7]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 0; next;)
table=??(ls_in_acl_sample ), priority=0 , match=(1), action=(next;)
table=??(ls_in_acl_sample ), priority=1000 , match=(ip && ct.new && reg8[[0..7]] == 1 && reg8[[19..20]] == 0), action=(sample(probability=65535,collector_set=??,obs_domain=42,obs_point=reg3); next;)
table=??(ls_in_acl_sample ), priority=1000 , match=(ip && ct.trk && (ct.est || ct.rel) && ct_label.obs_unused == 0 && !ct.rpl && ct_mark.obs_collector_id == 1 && ct_mark.obs_stage == 0), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=ct_label.obs_point_id); next;)
Expand All @@ -12999,6 +13003,7 @@ AT_CHECK_UNQUOTED([ovn_trace --ct new ls "$flow" | TRACE_FILTER], [0], [dnl
dnl Trace estasblished connections.
flow="$base_flow && ct_label.obs_point_id == 4302 && ct_mark.obs_stage == 0 && ct_mark.obs_collector_id == 1"
AT_CHECK_UNQUOTED([ovn_trace --ct est ls "$flow" | TRACE_FILTER], [0], [dnl
ct_commit { ct_mark.blocked = 0; ct_mark.obs_stage = reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; ct_label.obs_point_id = reg9; };
reg8[[0..7]] = 1;
reg8[[8..15]] = 1;
reg9 = 4302;
Expand Down Expand Up @@ -13045,7 +13050,7 @@ ovn-nbctl --wait=sb \
--apply-after-lb --sample-new=@sample1 --sample-est=@sample2 acl-add ls from-lport 1 "1" allow-related
AT_CHECK([ovn-sbctl lflow-list | grep -e ls_in_acl_after_lb_sample -e ls_in_acl_after_lb_eval -e ls_out_acl_sample | ovn_strip_lflows | ovn_strip_collector_set | grep -e reg3 -e reg9 -e sample], [0], [dnl
table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(reg0[[7]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 1; next;)
table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 1; next;)
table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 1; next;)
table=??(ls_in_acl_after_lb_sample), priority=0 , match=(1), action=(next;)
table=??(ls_in_acl_after_lb_sample), priority=1000 , match=(ip && ct.new && reg8[[0..7]] == 1 && reg8[[19..20]] == 1), action=(sample(probability=65535,collector_set=??,obs_domain=42,obs_point=reg3); next;)
table=??(ls_in_acl_after_lb_sample), priority=1000 , match=(ip && ct.trk && (ct.est || ct.rel) && ct_label.obs_unused == 0 && !ct.rpl && ct_mark.obs_collector_id == 1 && ct_mark.obs_stage == 1), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=ct_label.obs_point_id); next;)
Expand All @@ -13066,6 +13071,7 @@ AT_CHECK_UNQUOTED([ovn_trace --ct new ls "$flow" | TRACE_FILTER], [0], [dnl
dnl Trace estasblished connections.
flow="$base_flow && ct_label.obs_point_id == 4302 && ct_mark.obs_stage == 1 && ct_mark.obs_collector_id == 1"
AT_CHECK_UNQUOTED([ovn_trace --ct est ls "$flow" | TRACE_FILTER], [0], [dnl
ct_commit { ct_mark.blocked = 0; ct_mark.obs_stage = reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; ct_label.obs_point_id = reg9; };
reg8[[0..7]] = 1;
reg8[[8..15]] = 1;
reg9 = 4302;
Expand Down Expand Up @@ -13114,7 +13120,7 @@ AT_CHECK([ovn-sbctl lflow-list | grep -e ls_out_acl_sample -e ls_out_acl_eval -e
table=??(ls_in_acl_sample ), priority=0 , match=(1), action=(next;)
table=??(ls_in_acl_sample ), priority=1000 , match=(ip && ct.trk && (ct.est || ct.rel) && ct_label.obs_unused == 0 && ct.rpl && ct_mark.obs_collector_id == 1), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=ct_label.obs_point_id); next;)
table=??(ls_out_acl_eval ), priority=1001 , match=(reg0[[7]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 2; next;)
table=??(ls_out_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 2; next;)
table=??(ls_out_acl_eval ), priority=1001 , match=(reg0[[8]] == 1 && (1)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 4301; reg9 = 4302; reg8[[0..7]] = 1; reg8[[8..15]] = 1; reg8[[19..20]] = 2; next;)
table=??(ls_out_acl_sample ), priority=0 , match=(1), action=(next;)
table=??(ls_out_acl_sample ), priority=1000 , match=(ip && (ct.new || !ct.trk) && reg8[[0..7]] == 1 && reg8[[19..20]] == 2), action=(sample(probability=65535,collector_set=??,obs_domain=42,obs_point=reg3); next;)
table=??(ls_out_acl_sample ), priority=1000 , match=(ip && ct.trk && (ct.est || ct.rel) && ct_label.obs_unused == 0 && !ct.rpl && ct_mark.obs_collector_id == 1 && ct_mark.obs_stage == 2), action=(sample(probability=65535,collector_set=??,obs_domain=43,obs_point=ct_label.obs_point_id); next;)
Expand All @@ -13134,6 +13140,7 @@ AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new ls "$flow" | TRACE_FILTER], [0],
dnl Trace estasblished connections.
flow="$base_flow && ct_label.obs_point_id == 4302 && ct_mark.obs_stage == 2 && ct_mark.obs_collector_id == 1"
AT_CHECK_UNQUOTED([ovn_trace --ct est --ct est ls "$flow" | TRACE_FILTER], [0], [dnl
ct_commit { ct_mark.blocked = 0; ct_mark.obs_stage = reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; ct_label.obs_point_id = reg9; };
reg8[[0..7]] = 1;
reg8[[8..15]] = 1;
reg9 = 4302;
Expand Down

0 comments on commit 0054ab7

Please sign in to comment.