On Thu, Nov 7, 2024 at 4:03 AM Ales Musil <[email protected]> wrote: > > On Tue, Oct 8, 2024 at 6:45 PM Lorenzo Bianconi <[email protected]> > wrote: > > > 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]> > > > > nit: Double Signed-off-by. > > --- > > 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 > > > > > Looks good to me, thanks. > > Acked-by: Ales Musil <[email protected]>
Thanks. Applied to main and backported to branch-24.09. Numan > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > [email protected] > <https://red.ht/sig> > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
