On 5/10/21 5:39 PM, Dumitru Ceara wrote: > On 5/7/21 4:42 PM, [email protected] wrote: >> From: Numan Siddique <[email protected]> >> >> Some smart NICs can't offload datapath flows matching on conntrack >> fields. If a deployment desires to make use of such smart NICs >> then it cannot configure ACLs on the logical switches. If suppose >> a logical switch S1 has no ACLs configured and a logical switch S2 >> has ACLs configured, then the CMS would expect the datapath flows >> belonging to S1 logical ports are offloaded since it has no ACLs. >> But this is not working as expected (even if S1 and S2 are >> not connected via a logical router). >> >> ovn-northd generates the below logical flows in ls_in_acl_hint >> and ls_in_acl stages for S1 >> >> table=8 (ls_in_acl_hint ), priority=0 , match=(1), action=(next;) >> table=9 (ls_in_acl ), priority=0 , match=(1), action=(next;) >> >> And the below for S2 >> >> table=8 (ls_in_acl_hint ), priority=7 , match=(ct.new && !ct.est), >> action=(reg0[7] = 1; reg0[9] = 1; next;) >> table=8 (ls_in_acl_hint ), priority=6 , match=(!ct.new && ct.est && >> !ct.rpl && ct_label.blocked == 1), action=(reg0[7] = 1; reg0[9] = 1; next;) >> table=8 (ls_in_acl_hint ), priority=5 , match=(!ct.trk), >> action=(reg0[8] = 1; reg0[9] = 1; next;) >> table=8 (ls_in_acl_hint ), priority=4 , match=(!ct.new && ct.est && >> !ct.rpl && ct_label.blocked == 0), action=(reg0[8] = 1; reg0[10] = 1; next;) >> table=8 (ls_in_acl_hint ), priority=3 , match=(!ct.est), >> action=(reg0[9] = 1; next;) >> table=8 (ls_in_acl_hint ), priority=2 , match=(ct.est && >> ct_label.blocked == 1), action=(reg0[9] = 1; next;) >> table=8 (ls_in_acl_hint ), priority=1 , match=(ct.est && >> ct_label.blocked == 0), action=(reg0[10] = 1; next;) >> table=8 (ls_in_acl_hint ), priority=0 , match=(1), action=(next;) >> table=9 (ls_in_acl ), priority=65535, match=(!ct.est && ct.rel && >> !ct.new && !ct.inv && ct_label.blocked == 0), action=(next;) >> table=9 (ls_in_acl ), priority=65535, match=(ct.est && !ct.rel && >> !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0), action=(next;) >> table=9 (ls_in_acl ), priority=65535, match=(ct.inv || (ct.est && >> ct.rpl && ct_label.blocked == 1)), action=(drop;) >> table=9 (ls_in_acl ), priority=65535, match=(nd || nd_ra || nd_rs >> || mldv1 || mldv2), action=(next;) >> table=9 (ls_in_acl ), priority=34000, match=(eth.dst == >> $svc_monitor_mac), action=(next;) >> table=9 (ls_in_acl ), priority=1 , match=(ip && (!ct.est || >> (ct.est && ct_label.blocked == 1))), action=(reg0[1] = 1; next;) >> table=9 (ls_in_acl ), priority=0 , match=(1), action=(next;) >> >> Because there are higher priority flows in 'ls_in_acl_hint' and >> 'ls_in_acl' with the match on conntrack fields, >> ovs-vswitchd will generate a datapath flow with the match on ct_state fields >> as - >> 'ct_state(-new-est-rel-rpl-inv-trk)' for the packet from S1, even though >> the S1 pipeline doesn't have logical flows which match on conntrack >> fields. [1] has more information. >> >> Modifying the below flows if a logical switch has no ACLs solves this >> problem. >> >> table=8 (ls_in_acl_hint ), priority=65535 , match=(1), action=(next;) >> table=9 (ls_in_acl ), priority=65535 , match=(1), action=(next;) >> >> With the above flows with higher priority, ovs-vswitchd will not >> consider other flows in the same table during translation. >> >> This patch addresses this issue by using higher prioriy flows (for both >> ls_in_acl* and ls_out_acl* stages). >> >> [1] - https://bugzilla.redhat.com/show_bug.cgi?id=1955191#c8 >> >> Signed-off-by: Numan Siddique <[email protected]> >> --- > > Hi Numan, > > A couple of tests are failing after rebase: > > 789: ovn -- ct.inv usage -- ovn-northd -- dp-groups=yes FAILED > (ovn-northd.at:3147) > 790: ovn -- ct.inv usage -- ovn-northd FAILED > (ovn-northd.at:3147) > >> v1 -> v2 >> ---- >> * Rebased to resolve conflicts. >> * Addressed review comment from Dumitru. Combined ls_has_stateful_acl() >> and ls_has_acl() into one single function - od_ls_update_acls_flags(). > > Nit: There's no other function in ovn_northd that's prefixed with > od_ls_.*(). Maybe it makes sense to rename this to ls_get_acl_flags() > to be inline with ls_has_lb_vip()? >
I assume the test failure is just due to the rebase and can be easily fixed. The function rename can be done at apply time then: Acked-by: Dumitru Ceara <[email protected]> Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
