[ovs-dev] [PATCH ovn] northd: Remove the usage of 'ct.inv' in logical flows.

numans at ovn.org numans at ovn.org
Wed Feb 24 13:26:38 UTC 2021


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'.

As per the 'ovs-fields' man page, this
ct state field can be used to identify problems such as:
 •  L3/L4 protocol handler is not loaded/unavailable.

 •  L3/L4 protocol handler determines that the packet is
    malformed.

 •  Packets are unexpected length for protocol.

This patch removes the usage of this field for the following
reasons:

 • Some of the smart NICs which support offloading datapath
   flows don't support this field.

 • 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.

 • Even if a ct.inv packet is delivered to a VIF, the
   networking stack of the VIF's kernel can handle such
   packets.

Signed-off-by: Numan Siddique <numans at ovn.org>
---
 northd/ovn-northd.c | 24 ++++++++++++------------
 tests/ovn-northd.at | 24 ++++++++++++------------
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index dcdb777a2..e30fb532c 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -5617,10 +5617,10 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
          *
          * This is enforced at a higher priority than ACLs can be defined. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
-                      "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
+                      "ct.est && ct.rpl && ct_label.blocked == 1",
                       "drop;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
+                      "ct.est && ct.rpl && ct_label.blocked == 1",
                       "drop;");
 
         /* Ingress and Egress ACL Table (Priority 65535).
@@ -5633,12 +5633,12 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
          *
          * This is enforced at a higher priority than ACLs can be defined. */
         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",
+                      "ct.est && !ct.rel && !ct.new && "
+                      "ct.rpl && ct_label.blocked == 0",
                       "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",
+                      "ct.est && !ct.rel && !ct.new && "
+                      "ct.rpl && ct_label.blocked == 0",
                       "next;");
 
         /* Ingress and Egress ACL Table (Priority 65535).
@@ -5653,12 +5653,12 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
          * related traffic such as an ICMP Port Unreachable through
          * that's generated from a non-listening UDP port.  */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
-                      "!ct.est && ct.rel && !ct.new && !ct.inv "
-                      "&& ct_label.blocked == 0",
+                      "!ct.est && ct.rel && !ct.new && "
+                      "ct_label.blocked == 0",
                       "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "!ct.est && ct.rel && !ct.new && !ct.inv "
-                      "&& ct_label.blocked == 0",
+                      "!ct.est && ct.rel && !ct.new && "
+                      "ct_label.blocked == 0",
                       "next;");
 
         /* Ingress and Egress ACL Table (Priority 65535).
@@ -5846,11 +5846,11 @@ build_lb(struct ovn_datapath *od, struct hmap *lflows)
          *
          * Send established traffic through conntrack for just NAT. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_LB, UINT16_MAX - 1,
-                      "ct.est && !ct.rel && !ct.new && !ct.inv && "
+                      "ct.est && !ct.rel && !ct.new && "
                       "ct_label.natted == 1",
                       REGBIT_CONNTRACK_NAT" = 1; next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_LB, UINT16_MAX - 1,
-                      "ct.est && !ct.rel && !ct.new && !ct.inv && "
+                      "ct.est && !ct.rel && !ct.new && "
                       "ct_label.natted == 1",
                       REGBIT_CONNTRACK_NAT" = 1; next;");
     }
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index ad0f9f562..dc49c2543 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1940,9 +1940,9 @@ AT_CHECK([ovn-sbctl lflow-list ls | grep -e ls_in_acl_hint -e ls_out_acl_hint -e
   table=4 (ls_out_acl_hint    ), priority=6    , match=(!ct.new && ct.est && !ct.rpl && ct_label.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;)
   table=4 (ls_out_acl_hint    ), priority=7    , match=(ct.new && !ct.est), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;)
   table=5 (ls_out_acl         ), priority=1    , match=(ip && (!ct.est || (ct.est && ct_label.blocked == 1))), action=(reg0[[1]] = 1; next;)
-  table=5 (ls_out_acl         ), priority=65535, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_label.blocked == 0), action=(next;)
-  table=5 (ls_out_acl         ), priority=65535, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0), action=(next;)
-  table=5 (ls_out_acl         ), priority=65535, match=(ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)), action=(drop;)
+  table=5 (ls_out_acl         ), priority=65535, match=(!ct.est && ct.rel && !ct.new && ct_label.blocked == 0), action=(next;)
+  table=5 (ls_out_acl         ), priority=65535, match=(ct.est && !ct.rel && !ct.new && ct.rpl && ct_label.blocked == 0), action=(next;)
+  table=5 (ls_out_acl         ), priority=65535, match=(ct.est && ct.rpl && ct_label.blocked == 1), action=(drop;)
   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=2    , match=(ct.est && ct_label.blocked == 1), action=(reg0[[9]] = 1; next;)
   table=8 (ls_in_acl_hint     ), priority=3    , match=(!ct.est), action=(reg0[[9]] = 1; next;)
@@ -1951,9 +1951,9 @@ AT_CHECK([ovn-sbctl lflow-list ls | grep -e ls_in_acl_hint -e ls_out_acl_hint -e
   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=7    , match=(ct.new && !ct.est), action=(reg0[[7]] = 1; reg0[[9]] = 1; 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=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=(!ct.est && ct.rel && !ct.new && ct_label.blocked == 0), action=(next;)
+  table=9 (ls_in_acl          ), priority=65535, match=(ct.est && !ct.rel && !ct.new && ct.rpl && ct_label.blocked == 0), action=(next;)
+  table=9 (ls_in_acl          ), priority=65535, match=(ct.est && ct.rpl && ct_label.blocked == 1), action=(drop;)
 ])
 
 AS_BOX([Check match ct_state with load balancer])
@@ -1972,9 +1972,9 @@ AT_CHECK([ovn-sbctl lflow-list ls | grep -e ls_in_acl_hint -e ls_out_acl_hint -e
   table=4 (ls_out_acl_hint    ), priority=6    , match=(!ct.new && ct.est && !ct.rpl && ct_label.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;)
   table=4 (ls_out_acl_hint    ), priority=7    , match=(ct.new && !ct.est), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;)
   table=5 (ls_out_acl         ), priority=1    , match=(ip && (!ct.est || (ct.est && ct_label.blocked == 1))), action=(reg0[[1]] = 1; next;)
-  table=5 (ls_out_acl         ), priority=65535, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_label.blocked == 0), action=(next;)
-  table=5 (ls_out_acl         ), priority=65535, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0), action=(next;)
-  table=5 (ls_out_acl         ), priority=65535, match=(ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)), action=(drop;)
+  table=5 (ls_out_acl         ), priority=65535, match=(!ct.est && ct.rel && !ct.new && ct_label.blocked == 0), action=(next;)
+  table=5 (ls_out_acl         ), priority=65535, match=(ct.est && !ct.rel && !ct.new && ct.rpl && ct_label.blocked == 0), action=(next;)
+  table=5 (ls_out_acl         ), priority=65535, match=(ct.est && ct.rpl && ct_label.blocked == 1), action=(drop;)
   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=2    , match=(ct.est && ct_label.blocked == 1), action=(reg0[[9]] = 1; next;)
   table=8 (ls_in_acl_hint     ), priority=3    , match=(!ct.est), action=(reg0[[9]] = 1; next;)
@@ -1983,9 +1983,9 @@ AT_CHECK([ovn-sbctl lflow-list ls | grep -e ls_in_acl_hint -e ls_out_acl_hint -e
   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=7    , match=(ct.new && !ct.est), action=(reg0[[7]] = 1; reg0[[9]] = 1; 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=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=(!ct.est && ct.rel && !ct.new && ct_label.blocked == 0), action=(next;)
+  table=9 (ls_in_acl          ), priority=65535, match=(ct.est && !ct.rel && !ct.new && ct.rpl && ct_label.blocked == 0), action=(next;)
+  table=9 (ls_in_acl          ), priority=65535, match=(ct.est && ct.rpl && ct_label.blocked == 1), action=(drop;)
 ])
 
 AT_CLEANUP
-- 
2.29.2



More information about the dev mailing list