[ovs-dev] [PATCH ovn 2/2] northd: Provide the option to not use ct.inv in lflows.

Ben Pfaff blp at ovn.org
Fri Mar 26 03:46:29 UTC 2021


On Mon, Mar 22, 2021 at 04:29:11PM +0530, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
> 
> Presently we add 65535 priority lflows in the stages -
> 'ls_in_acl' and 'ls_out_acl' to drop packets which
> match on 'ct.inv'.
> 
> This patch provides an option to not use this field in the
> logical flow matches for the following
> reasons:
> 
>  • Some of the smart NICs which support offloading datapath
>    flows may not support this field.  In such cases, almost
>    all the datapath flows cannot be offloaded if ACLs/load balancers
>    are configured on the logical switch datapath.
> 
>  • A recent commit in kernel ovs datapath sets the committed
>    connection tracking entry to be liberal for out-of-window
>    tcp packets (nf_ct_set_tcp_be_liberal()).  Such TCP
>    packets will not be marked as invalid.
> 
> There are still certain scenarios where a packet can be marked
> invalid.  Like
>  • If the tcp flags are not correct.
> 
>  • If there are checksum errors.
> 
> In such cases, the packet will be delivered to the destination
> port.  So CMS should evaluate their usecases before enabling
> this option.
> 
> Signed-off-by: Numan Siddique <numans at ovn.org>

Thanks for writing dldog code!  I have a few comments.

This looks correct:

    function can_use_ct_inv_match(options: Map<string,string>): bool {
        map_get_bool_def(options, "use_ct_inv_match", true)
    }

    relation UseCtInvMatch(use_ct_inv_match: bool)
    UseCtInvMatch(can_use_ct_inv_match(options)) :-
        nb::NB_Global(._uuid = uuid, .options = options).

I would make a few stylistic changes if I were writing it.  I would drop
the _uuid match because it is not used for anything.  I would use a
relation that just contains a bool, instead of a struct that contains a
bool, because then there are fewer names to invent.  I would usually
drop the function, since it is only used in one place and it is a
one-liner.  So I'd end up with this:

    relation UseCtInvMatch[bool]
    UseCtInvMatch[use_ct_inv_match] :-
        nb::NB_Global(.options = options),
        var use_ct_inv_match = map_get_bool_def(options, "use_ct_inv_match", true).

Or possibly I'd keep the map_get_bool_def(...) expression inside the
brackets, I haven't decided what's really the best way:

    relation UseCtInvMatch[bool]
    UseCtInvMatch[map_get_bool_def(options, "use_ct_inv_match", true)] :-
        nb::NB_Global(.options = options).

I'd also add a rule to handle the case where NB_Global doesn't exist.
This is really out of paranoia.  I don't know whether this can really
happen in practice, but it costs only a few lines and avoids a problem
if it does ever somehow happen:

    UseCtInvMatch[true] :-
        Unit(),
        not nb in nb::NB_Global().

I noticed is that this global feature flag is getting copied inside
every Switch.  That wastes a little memory and I do not know of a
benefit.  So I'd drop it from Switch and join with UseCtInvMatch in the
one place where it's needed.

Finally, there's are lots of redundant strings in ovn_northd.dl.  We can
make that better with a little parameterization, something like this:
    @@ -2290,10 +2290,15 @@ for (Reject(lsuuid, pipeline, stage, acl, fair_meter, extra_match_, extra_action
              .actions          = actions,
              .external_ids     = stage_hint(acl._uuid))
     }

     /* build_acls */
    +for (UseCtInvMatch[use_ct_inv_match]) {
    +    (var ct_inv_or, var and_not_ct_inv) = match (use_ct_inv_match) {
    +        true -> ("ct.inv || ", "&& !ct.inv "),
    +        false -> ("", ""),
    +    } in
         for (sw in &Switch(.ls = ls))
         var has_stateful = sw.has_stateful_acl or sw.has_lb_vip in
         {
             /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
              * default.  A related rule at priority 1 is added below if there
    @@ -2354,17 +2359,17 @@ var has_stateful = sw.has_stateful_acl or sw.has_lb_vip in
                  *
                  * This is enforced at a higher priority than ACLs can be defined. */
                 Flow(.logical_datapath = ls._uuid,
                      .stage            = switch_stage(IN, ACL),
                      .priority         = 65535,
    -             .__match          = "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
    +                 .__match          = ct_inv_or ++ "(ct.est && ct.rpl && ct_label.blocked == 1)",
                      .actions          = "drop;",
                      .external_ids     = map_empty());
                 Flow(.logical_datapath = ls._uuid,
                      .stage            = switch_stage(OUT, ACL),
                      .priority         = 65535,
    -             .__match          = "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
    +                 .__match          = ct_inv_or ++ "(ct.est && ct.rpl && ct_label.blocked == 1)",
                      .actions          = "drop;",
                      .external_ids     = map_empty());

                 /* Ingress and Egress ACL Table (Priority 65535).
                  *

When I put all that together, I get the following replacement overall
patch.  I haven't tested it, so maybe I made some mistakes, but it did
compile.  The change to ovn_northd.dl looks big but most of it is
reindentation; if you do a "git show -b" after applying it you'll see
the real changes.

Thanks,

Ben.

-8<--------------------------cut here-------------------------->8--

From: Numan Siddique <numans at ovn.org>
Date: Mon, 22 Mar 2021 16:29:11 +0530
Subject: [PATCH ovn] northd: Provide the option to not use ct.inv in lflows.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Presently we add 65535 priority lflows in the stages -
'ls_in_acl' and 'ls_out_acl' to drop packets which
match on 'ct.inv'.

This patch provides an option to not use this field in the
logical flow matches for the following
reasons:

 • Some of the smart NICs which support offloading datapath
   flows may not support this field.  In such cases, almost
   all the datapath flows cannot be offloaded if ACLs/load balancers
   are configured on the logical switch datapath.

 • A recent commit in kernel ovs datapath sets the committed
   connection tracking entry to be liberal for out-of-window
   tcp packets (nf_ct_set_tcp_be_liberal()).  Such TCP
   packets will not be marked as invalid.

There are still certain scenarios where a packet can be marked
invalid.  Like
 • If the tcp flags are not correct.

 • If there are checksum errors.

In such cases, the packet will be delivered to the destination
port.  So CMS should evaluate their usecases before enabling
this option.

Signed-off-by: Numan Siddique <numans at ovn.org>
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 NEWS                 |   5 +
 northd/lswitch.dl    |   8 ++
 northd/ovn-northd.c  |  41 +++---
 northd/ovn_northd.dl | 304 ++++++++++++++++++++++---------------------
 ovn-nb.xml           |  11 ++
 5 files changed, 204 insertions(+), 165 deletions(-)

diff --git a/NEWS b/NEWS
index 530c5d42fe85..3181649ba822 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,11 @@ Post-v21.03.0
     (This may take testing and tuning to be effective.)  This version of OVN
     requires DDLog 0.36.
   - Introduce ovn-controller incremetal processing engine statistics
+  - Add a new NB Global option - us_ct_inv_match with the default value of true.
+    If this is set to false, then the logical field - "ct.inv" will not be
+    used in the logical flow matches.  CMS can consider setting this to false,
+    if they want to use smart NICs which don't support offloading datapath flows
+    with this field used.
 
 OVN v21.03.0 - 12 Mar 2021
 -------------------------
diff --git a/northd/lswitch.dl b/northd/lswitch.dl
index 4bf8a5b907a9..db33bad52ffe 100644
--- a/northd/lswitch.dl
+++ b/northd/lswitch.dl
@@ -232,6 +232,7 @@ function ipv6_parse_prefix(s: string): Option<in6_addr> {
     LogicalSwitchLocalnetPorts(ls._uuid, localnet_ports),
     LogicalSwitchHasNonRouterPort(ls._uuid, has_non_router_port),
     mcast_cfg in &McastSwitchCfg(.datapath = ls._uuid),
+    UseCtInvMatch[use_ct_inv_match],
     var subnet =
         match (ls.other_config.get("subnet")) {
             None -> None,
@@ -744,3 +745,10 @@ function put_svc_monitor_mac(options: Map<string,string>,
 relation SvcMonitorMac(mac: eth_addr)
 SvcMonitorMac(get_svc_monitor_mac(options, uuid)) :-
     nb::NB_Global(._uuid = uuid, .options = options).
+
+relation UseCtInvMatch[bool]
+UseCtInvMatch[map_get_bool_def(options, "use_ct_inv_match", true)] :-
+    nb::NB_Global(.options = options).
+UseCtInvMatch[true] :-
+    Unit(),
+    not nb in nb::NB_Global().
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 57df62b9207a..f1332370eb54 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4068,6 +4068,10 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
  * logical datapath only by creating a datapath group. */
 static bool use_logical_dp_groups = false;
 
+/* If this option is 'true' northd will make use of ct.inv match fields.
+ * Otherwise, it will avoid using it.  The default is true. */
+static bool use_ct_inv_match = true;
+
 /* Adds a row with the specified contents to the Logical_Flow table. */
 static void
 ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
@@ -5647,12 +5651,13 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
          * for deletion (bit 0 of ct_label is set).
          *
          * This is enforced at a higher priority than ACLs can be defined. */
+        char *match = xasprintf("%s(ct.est && ct.rpl && ct_label.blocked == 1)",
+                                use_ct_inv_match ? "ct.inv || " : "");
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
-                      "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
-                      "drop;");
+                      match, "drop;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
-                      "drop;");
+                      match, "drop;");
+        free(match);
 
         /* Ingress and Egress ACL Table (Priority 65535).
          *
@@ -5663,14 +5668,15 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
          * direction to hit the currently defined policy from ACLs.
          *
          * This is enforced at a higher priority than ACLs can be defined. */
+        match = xasprintf("ct.est && !ct.rel && !ct.new%s && "
+                          "ct.rpl && ct_label.blocked == 0",
+                          use_ct_inv_match ? " && !ct.inv" : "");
+
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
-                      "ct.est && !ct.rel && !ct.new && !ct.inv "
-                      "&& ct.rpl && ct_label.blocked == 0",
-                      "next;");
+                      match, "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "ct.est && !ct.rel && !ct.new && !ct.inv "
-                      "&& ct.rpl && ct_label.blocked == 0",
-                      "next;");
+                      match, "next;");
+        free(match);
 
         /* Ingress and Egress ACL Table (Priority 65535).
          *
@@ -5683,14 +5689,14 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
          * a dynamically negotiated FTP data channel), but will allow
          * related traffic such as an ICMP Port Unreachable through
          * that's generated from a non-listening UDP port.  */
+        match = xasprintf("!ct.est && ct.rel && !ct.new%s && "
+                          "ct_label.blocked == 0",
+                          use_ct_inv_match ? " && !ct.inv" : "");
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
-                      "!ct.est && ct.rel && !ct.new && !ct.inv "
-                      "&& ct_label.blocked == 0",
-                      "next;");
+                      match, "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "!ct.est && ct.rel && !ct.new && !ct.inv "
-                      "&& ct_label.blocked == 0",
-                      "next;");
+                      match, "next;");
+        free(match);
 
         /* Ingress and Egress ACL Table (Priority 65535).
          *
@@ -12974,6 +12980,9 @@ ovnnb_db_run(struct northd_context *ctx,
 
     use_logical_dp_groups = smap_get_bool(&nb->options,
                                           "use_logical_dp_groups", false);
+    use_ct_inv_match = smap_get_bool(&nb->options,
+                                     "use_ct_inv_match", true);
+
     /* deprecated, use --event instead */
     controller_event_en = smap_get_bool(&nb->options,
                                         "controller_event", false);
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 4262b83b9760..4f966e50fb7b 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -2292,173 +2292,179 @@ for (Reject(lsuuid, pipeline, stage, acl, fair_meter, extra_match_, extra_action
 }
 
 /* build_acls */
-for (sw in &Switch(.ls = ls))
-var has_stateful = sw.has_stateful_acl or sw.has_lb_vip in
-{
-    /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
-     * default.  A related rule at priority 1 is added below if there
-     * are any stateful ACLs in this datapath. */
-    Flow(.logical_datapath = ls._uuid,
-         .stage            = switch_stage(IN, ACL),
-         .priority         = 0,
-         .__match          = "1",
-         .actions          = "next;",
-         .external_ids     = map_empty());
-    Flow(.logical_datapath = ls._uuid,
-         .stage            = switch_stage(OUT, ACL),
-         .priority         = 0,
-         .__match          = "1",
-         .actions          = "next;",
-         .external_ids     = map_empty());
-
-    if (has_stateful) {
-        /* Ingress and Egress ACL Table (Priority 1).
-         *
-         * By default, traffic is allowed.  This is partially handled by
-         * the Priority 0 ACL flows added earlier, but we also need to
-         * commit IP flows.  This is because, while the initiater's
-         * direction may not have any stateful rules, the server's may
-         * and then its return traffic would not have an associated
-         * conntrack entry and would return "+invalid".
-         *
-         * We use "ct_commit" for a connection that is not already known
-         * by the connection tracker.  Once a connection is committed,
-         * subsequent packets will hit the flow at priority 0 that just
-         * uses "next;"
-         *
-         * We also check for established connections that have ct_label.blocked
-         * set on them.  That's a connection that was disallowed, but is
-         * now allowed by policy again since it hit this default-allow flow.
-         * We need to set ct_label.blocked=0 to let the connection continue,
-         * which will be done by ct_commit() in the "stateful" stage.
-         * Subsequent packets will hit the flow at priority 0 that just
-         * uses "next;". */
-        Flow(.logical_datapath = ls._uuid,
-             .stage            = switch_stage(IN, ACL),
-             .priority         = 1,
-             .__match          = "ip && (!ct.est || (ct.est && ct_label.blocked == 1))",
-             .actions          = "${rEGBIT_CONNTRACK_COMMIT()} = 1; next;",
-             .external_ids     = map_empty());
-        Flow(.logical_datapath = ls._uuid,
-             .stage            = switch_stage(OUT, ACL),
-             .priority         = 1,
-             .__match          = "ip && (!ct.est || (ct.est && ct_label.blocked == 1))",
-             .actions          = "${rEGBIT_CONNTRACK_COMMIT()} = 1; next;",
-             .external_ids     = map_empty());
-
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Always drop traffic that's in an invalid state.  Also drop
-         * reply direction packets for connections that have been marked
-         * for deletion (bit 0 of ct_label is set).
-         *
-         * This is enforced at a higher priority than ACLs can be defined. */
-        Flow(.logical_datapath = ls._uuid,
-             .stage            = switch_stage(IN, ACL),
-             .priority         = 65535,
-             .__match          = "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
-             .actions          = "drop;",
-             .external_ids     = map_empty());
-        Flow(.logical_datapath = ls._uuid,
-             .stage            = switch_stage(OUT, ACL),
-             .priority         = 65535,
-             .__match          = "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
-             .actions          = "drop;",
-             .external_ids     = map_empty());
-
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Allow reply traffic that is part of an established
-         * conntrack entry that has not been marked for deletion
-         * (bit 0 of ct_label).  We only match traffic in the
-         * reply direction because we want traffic in the request
-         * direction to hit the currently defined policy from ACLs.
-         *
-         * This is enforced at a higher priority than ACLs can be defined. */
+for (UseCtInvMatch[use_ct_inv_match]) {
+    (var ct_inv_or, var and_not_ct_inv) = match (use_ct_inv_match) {
+        true -> ("ct.inv || ", "&& !ct.inv "),
+        false -> ("", ""),
+    } in
+    for (sw in &Switch(.ls = ls))
+    var has_stateful = sw.has_stateful_acl or sw.has_lb_vip in
+    {
+        /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
+         * default.  A related rule at priority 1 is added below if there
+         * are any stateful ACLs in this datapath. */
         Flow(.logical_datapath = ls._uuid,
              .stage            = switch_stage(IN, ACL),
-             .priority         = 65535,
-             .__match          = "ct.est && !ct.rel && !ct.new && !ct.inv "
-                                 "&& ct.rpl && ct_label.blocked == 0",
+             .priority         = 0,
+             .__match          = "1",
              .actions          = "next;",
              .external_ids     = map_empty());
         Flow(.logical_datapath = ls._uuid,
              .stage            = switch_stage(OUT, ACL),
-             .priority         = 65535,
-             .__match          = "ct.est && !ct.rel && !ct.new && !ct.inv "
-                                 "&& ct.rpl && ct_label.blocked == 0",
+             .priority         = 0,
+             .__match          = "1",
              .actions          = "next;",
              .external_ids     = map_empty());
 
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Allow traffic that is related to an existing conntrack entry that
-         * has not been marked for deletion (bit 0 of ct_label).
-         *
-         * This is enforced at a higher priority than ACLs can be defined.
-         *
-         * NOTE: This does not support related data sessions (eg,
-         * a dynamically negotiated FTP data channel), but will allow
-         * related traffic such as an ICMP Port Unreachable through
-         * that's generated from a non-listening UDP port.  */
-        Flow(.logical_datapath = ls._uuid,
-             .stage            = switch_stage(IN, ACL),
-             .priority         = 65535,
-             .__match          = "!ct.est && ct.rel && !ct.new && !ct.inv "
-                                 "&& ct_label.blocked == 0",
-             .actions          = "next;",
-             .external_ids     = map_empty());
-        Flow(.logical_datapath = ls._uuid,
-             .stage            = switch_stage(OUT, ACL),
-             .priority         = 65535,
-             .__match          = "!ct.est && ct.rel && !ct.new && !ct.inv "
-                                 "&& ct_label.blocked == 0",
-             .actions          = "next;",
-             .external_ids     = map_empty());
+        if (has_stateful) {
+            /* Ingress and Egress ACL Table (Priority 1).
+             *
+             * By default, traffic is allowed.  This is partially handled by
+             * the Priority 0 ACL flows added earlier, but we also need to
+             * commit IP flows.  This is because, while the initiater's
+             * direction may not have any stateful rules, the server's may
+             * and then its return traffic would not have an associated
+             * conntrack entry and would return "+invalid".
+             *
+             * We use "ct_commit" for a connection that is not already known
+             * by the connection tracker.  Once a connection is committed,
+             * subsequent packets will hit the flow at priority 0 that just
+             * uses "next;"
+             *
+             * We also check for established connections that have ct_label.blocked
+             * set on them.  That's a connection that was disallowed, but is
+             * now allowed by policy again since it hit this default-allow flow.
+             * We need to set ct_label.blocked=0 to let the connection continue,
+             * which will be done by ct_commit() in the "stateful" stage.
+             * Subsequent packets will hit the flow at priority 0 that just
+             * uses "next;". */
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(IN, ACL),
+                 .priority         = 1,
+                 .__match          = "ip && (!ct.est || (ct.est && ct_label.blocked == 1))",
+                 .actions          = "${rEGBIT_CONNTRACK_COMMIT()} = 1; next;",
+                 .external_ids     = map_empty());
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(OUT, ACL),
+                 .priority         = 1,
+                 .__match          = "ip && (!ct.est || (ct.est && ct_label.blocked == 1))",
+                 .actions          = "${rEGBIT_CONNTRACK_COMMIT()} = 1; next;",
+                 .external_ids     = map_empty());
 
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Not to do conntrack on ND packets. */
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Always drop traffic that's in an invalid state.  Also drop
+             * reply direction packets for connections that have been marked
+             * for deletion (bit 0 of ct_label is set).
+             *
+             * This is enforced at a higher priority than ACLs can be defined. */
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(IN, ACL),
+                 .priority         = 65535,
+                 .__match          = ct_inv_or ++ "(ct.est && ct.rpl && ct_label.blocked == 1)",
+                 .actions          = "drop;",
+                 .external_ids     = map_empty());
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(OUT, ACL),
+                 .priority         = 65535,
+                 .__match          = ct_inv_or ++ "(ct.est && ct.rpl && ct_label.blocked == 1)",
+                 .actions          = "drop;",
+                 .external_ids     = map_empty());
+
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Allow reply traffic that is part of an established
+             * conntrack entry that has not been marked for deletion
+             * (bit 0 of ct_label).  We only match traffic in the
+             * reply direction because we want traffic in the request
+             * direction to hit the currently defined policy from ACLs.
+             *
+             * This is enforced at a higher priority than ACLs can be defined. */
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(IN, ACL),
+                 .priority         = 65535,
+                 .__match          = "ct.est && !ct.rel && !ct.new " ++ and_not_ct_inv ++
+                                     "&& ct.rpl && ct_label.blocked == 0",
+                 .actions          = "next;",
+                 .external_ids     = map_empty());
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(OUT, ACL),
+                 .priority         = 65535,
+                 .__match          = "ct.est && !ct.rel && !ct.new " ++ and_not_ct_inv ++
+                                     "&& ct.rpl && ct_label.blocked == 0",
+                 .actions          = "next;",
+                 .external_ids     = map_empty());
+
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Allow traffic that is related to an existing conntrack entry that
+             * has not been marked for deletion (bit 0 of ct_label).
+             *
+             * This is enforced at a higher priority than ACLs can be defined.
+             *
+             * NOTE: This does not support related data sessions (eg,
+             * a dynamically negotiated FTP data channel), but will allow
+             * related traffic such as an ICMP Port Unreachable through
+             * that's generated from a non-listening UDP port.  */
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(IN, ACL),
+                 .priority         = 65535,
+                 .__match          = "!ct.est && ct.rel && !ct.new " ++ and_not_ct_inv ++
+                                     "&& ct_label.blocked == 0",
+                 .actions          = "next;",
+                 .external_ids     = map_empty());
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(OUT, ACL),
+                 .priority         = 65535,
+                 .__match          = "!ct.est && ct.rel && !ct.new " ++ and_not_ct_inv ++
+                                     "&& ct_label.blocked == 0",
+                 .actions          = "next;",
+                 .external_ids     = map_empty());
+
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Not to do conntrack on ND packets. */
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(IN, ACL),
+                 .priority         = 65535,
+                 .__match          = "nd || nd_ra || nd_rs || mldv1 || mldv2",
+                 .actions          = "next;",
+                 .external_ids     = map_empty());
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(OUT, ACL),
+                 .priority         = 65535,
+                 .__match          = "nd || nd_ra || nd_rs || mldv1 || mldv2",
+                 .actions          = "next;",
+                 .external_ids     = map_empty())
+        };
+
+        /* Add a 34000 priority flow to advance the DNS reply from ovn-controller,
+         * if the CMS has configured DNS records for the datapath.
+         */
+        if (sw.has_dns_records) {
+            Flow(.logical_datapath = ls._uuid,
+                 .stage            = switch_stage(OUT, ACL),
+                 .priority         = 34000,
+                 .__match          = "udp.src == 53",
+                 .actions          = if has_stateful "ct_commit; next;" else "next;",
+                 .external_ids     = map_empty())
+        };
+
+        /* Add a 34000 priority flow to advance the service monitor reply
+         * packets to skip applying ingress ACLs. */
         Flow(.logical_datapath = ls._uuid,
              .stage            = switch_stage(IN, ACL),
-             .priority         = 65535,
-             .__match          = "nd || nd_ra || nd_rs || mldv1 || mldv2",
+             .priority         = 34000,
+             .__match          = "eth.dst == $svc_monitor_mac",
              .actions          = "next;",
              .external_ids     = map_empty());
-        Flow(.logical_datapath = ls._uuid,
-             .stage            = switch_stage(OUT, ACL),
-             .priority         = 65535,
-             .__match          = "nd || nd_ra || nd_rs || mldv1 || mldv2",
-             .actions          = "next;",
-             .external_ids     = map_empty())
-    };
-
-    /* Add a 34000 priority flow to advance the DNS reply from ovn-controller,
-     * if the CMS has configured DNS records for the datapath.
-     */
-    if (sw.has_dns_records) {
         Flow(.logical_datapath = ls._uuid,
              .stage            = switch_stage(OUT, ACL),
              .priority         = 34000,
-             .__match          = "udp.src == 53",
-             .actions          = if has_stateful "ct_commit; next;" else "next;",
+             .__match          = "eth.src == $svc_monitor_mac",
+             .actions          = "next;",
              .external_ids     = map_empty())
-    };
-
-    /* Add a 34000 priority flow to advance the service monitor reply
-     * packets to skip applying ingress ACLs. */
-    Flow(.logical_datapath = ls._uuid,
-         .stage            = switch_stage(IN, ACL),
-         .priority         = 34000,
-         .__match          = "eth.dst == $svc_monitor_mac",
-         .actions          = "next;",
-         .external_ids     = map_empty());
-    Flow(.logical_datapath = ls._uuid,
-         .stage            = switch_stage(OUT, ACL),
-         .priority         = 34000,
-         .__match          = "eth.src == $svc_monitor_mac",
-         .actions          = "next;",
-         .external_ids     = map_empty())
+    }
 }
 
 /* This stage builds hints for the IN/OUT_ACL stage. Based on various
diff --git a/ovn-nb.xml b/ovn-nb.xml
index b0a4adffe537..c3ff3b586ad1 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -227,6 +227,17 @@
         </p>
       </column>
 
+      <column name="options" key="use_ct_inv_match">
+        <p>
+          If set to false, <code>ovn-northd</code> will not use the
+          <code>ct.inv</code> field in any of the logical flow matches.
+          The default value is true.  CMS can consider setting this to
+          false, in case the NIC doesn't support offloading OVS datapath
+          flows having matches <code>ct_state(..+inv..)</code> or
+          <code>ct_state(..-inv..)</code>.
+        </p>
+      </column>
+
       <group title="Options for configuring interconnection route advertisement">
         <p>
           These options control how routes are advertised between OVN
-- 
2.29.2



More information about the dev mailing list