[ovs-dev] [PATCH ovn v2] northd: Support flow offloading for logical switches with no ACLs.

Dumitru Ceara dceara at redhat.com
Mon May 10 15:39:30 UTC 2021


On 5/7/21 4:42 PM, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
> 
> 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 <numans at ovn.org>
> ---

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()?

Regards,
Dumitru



More information about the dev mailing list