diff mbox

[ovs-dev,3/7] ovn-northd: Introduce pre-stateful table.

Message ID 1467188231-10194-4-git-send-email-guru@ovn.org
State Accepted
Headers show

Commit Message

Gurucharan Shetty June 29, 2016, 8:17 a.m. UTC 
Currently, the only use of stateful services in conntrack is
OVN ACLs. In table pre-ACL, we send the packet to conntrack
to track it (to get its status) and to defrag via the ct_next
action.

As we introduce more stateful services, the ACL feature will
have to share the conntrack module with others. As
preparation for more stateful features like loadbalancing,
this commit introduces a new pre-stateful table that is
responsible to send packets through conntrack via
ct_next action. If pre-ACL table needs to send a packet
through conntrack, it just sets the 'reg0[0]' as 1.
Pre-stateful table in-turn will send the packet to conntrack
if 'reg0[0]' is 1.

Signed-off-by: Gurucharan Shetty <guru@ovn.org>
---
 ovn/northd/ovn-northd.8.xml | 36 ++++++++++++++++++++++++++---------
 ovn/northd/ovn-northd.c     | 46 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 63 insertions(+), 19 deletions(-)

Comments

Zong Kai LI June 30, 2016, 8:33 a.m. UTC | #1 
>
> @@ -1377,13 +1381,34 @@ build_pre_acls(struct ovn_datapath *od, struct
> hmap *lflows,
>           *
>           * Regardless of whether the ACL is "from-lport" or "to-lport",
>           * we need rules in both the ingress and egress table, because
> -         * the return traffic needs to be followed. */
> -        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip",
> "ct_next;");
> -        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> "ct_next;");
> +         * the return traffic needs to be followed.
> +         *
> +         * 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table
> send
> +         * it to conntrack for tracking and defragmentation. */
> +        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip",
> +                      REGBIT_CONNTRACK_DEFRAG" = 1; next;");
> +        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> +                      REGBIT_CONNTRACK_DEFRAG" = 1; next;");
>      }
>  }
>
>  static void
> +build_pre_stateful(struct ovn_datapath *od, struct hmap *lflows)
> +{
> +    /* Ingress and Egress pre-stateful Table (Priority 0): Packets are
> +     * allowed by default. */
> +    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 0, "1", "next;");
> +    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 0, "1", "next;");
> +
> +    /* If REGBIT_CONNTRACK_DEFRAG is set as 1, then the packets should be
> +     * sent to conntrack for tracking and defragmentation. */
> +    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 100,
> +                  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
> +    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 100,
> +                  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
> +}
>

I hope you can also modify the default next lflow with 0 priority for
PRE_ACL, instead of using "next;" as action, try to directly resubmit to
table ACL.
Since in PRE_STATEFUL table, for non-stateful stuff, there is just another
"next;", this is not fun.

Thanks,
Zong Kai, LI
Gurucharan Shetty June 30, 2016, 2:56 p.m. UTC | #2 
On 30 June 2016 at 01:33, Zong Kai LI <zealokii@gmail.com> wrote:

> >
> > @@ -1377,13 +1381,34 @@ build_pre_acls(struct ovn_datapath *od, struct
> > hmap *lflows,
> >           *
> >           * Regardless of whether the ACL is "from-lport" or "to-lport",
> >           * we need rules in both the ingress and egress table, because
> > -         * the return traffic needs to be followed. */
> > -        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip",
> > "ct_next;");
> > -        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> > "ct_next;");
> > +         * the return traffic needs to be followed.
> > +         *
> > +         * 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful
> table
> > send
> > +         * it to conntrack for tracking and defragmentation. */
> > +        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip",
> > +                      REGBIT_CONNTRACK_DEFRAG" = 1; next;");
> > +        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> > +                      REGBIT_CONNTRACK_DEFRAG" = 1; next;");
> >      }
> >  }
> >
> >  static void
> > +build_pre_stateful(struct ovn_datapath *od, struct hmap *lflows)
> > +{
> > +    /* Ingress and Egress pre-stateful Table (Priority 0): Packets are
> > +     * allowed by default. */
> > +    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 0, "1",
> "next;");
> > +    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 0, "1",
> "next;");
> > +
> > +    /* If REGBIT_CONNTRACK_DEFRAG is set as 1, then the packets should
> be
> > +     * sent to conntrack for tracking and defragmentation. */
> > +    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 100,
> > +                  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
> > +    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 100,
> > +                  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
> > +}
> >
>
> I hope you can also modify the default next lflow with 0 priority for
> PRE_ACL, instead of using "next;" as action, try to directly resubmit to
> table ACL.
> Since in PRE_STATEFUL table, for non-stateful stuff, there is just another
> "next;", this is not fun.
>

A future commit in this series introduces a "pre-lb" table between
"pre-acl" and "pre-stateful". If I jump directly from "pre-acl" to "acl" as
you suggest, I will miss any load balancing rules added in "pre-lb" table.



> Thanks,
> Zong Kai, LI
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
Ben Pfaff July 3, 2016, 4:39 a.m. UTC | #3 
On Wed, Jun 29, 2016 at 01:17:07AM -0700, Gurucharan Shetty wrote:
> Currently, the only use of stateful services in conntrack is
> OVN ACLs. In table pre-ACL, we send the packet to conntrack
> to track it (to get its status) and to defrag via the ct_next
> action.
> 
> As we introduce more stateful services, the ACL feature will
> have to share the conntrack module with others. As
> preparation for more stateful features like loadbalancing,
> this commit introduces a new pre-stateful table that is
> responsible to send packets through conntrack via
> ct_next action. If pre-ACL table needs to send a packet
> through conntrack, it just sets the 'reg0[0]' as 1.
> Pre-stateful table in-turn will send the packet to conntrack
> if 'reg0[0]' is 1.
> 
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>

Acked-by: Ben Pfaff <blp@ovn.org>
diff mbox

Patch

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 4d712a1..a63e1cb 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -246,12 +246,24 @@ 
       This table prepares flows for possible stateful ACL processing in
       ingress table <code>ACLs</code>.  It contains a priority-0 flow that
       simply moves traffic to the next table.  If stateful ACLs are used in the
-      logical datapath, a priority-100 flow is added that sends IP packets to
-      the connection tracker before advancing to ingress table
-      <code>ACLs</code>.
+      logical datapath, a priority-100 flow is added that sets a hint
+      (with <code>reg0[0] = 1; next;</code>) for table
+      <code>Pre-stateful</code> to send IP packets to the connection tracker
+      before eventually advancing to ingress table <code>ACLs</code>.
     </p>
 
-    <h3>Ingress table 4: <code>from-lport</code> ACLs</h3>
+    <h3>Ingress Table 4: Pre-stateful</h3>
+
+    <p>
+      This table prepares flows for all possible stateful processing
+      in next tables.  It contains a priority-0 flow that simply moves
+      traffic to the next table.  A priority-100 flow sends the packets to
+      connection tracker based on a hint provided by the previous tables
+      (with a match for <code>reg0[0] == 1</code>) by using the
+      <code>ct_next;</code> action.
+    </p>
+
+    <h3>Ingress table 5: <code>from-lport</code> ACLs</h3>
 
     <p>
       Logical flows in this table closely reproduce those in the
@@ -299,7 +311,7 @@ 
       </li>
     </ul>
 
-    <h3>Ingress Table 5: ARP responder</h3>
+    <h3>Ingress Table 6: ARP responder</h3>
 
     <p>
       This table implements ARP responder for known IPs.  It contains these
@@ -344,7 +356,7 @@  output;
       </li>
     </ul>
 
-    <h3>Ingress Table 6: Destination Lookup</h3>
+    <h3>Ingress Table 7: Destination Lookup</h3>
 
     <p>
       This table implements switching behavior.  It contains these logical
@@ -382,14 +394,20 @@  output;
      <code>to-lport</code> traffic.
     </p>
 
-    <h3>Egress Table 1: <code>to-lport</code> ACLs</h3>
+    <h3>Egress Table 1: Pre-stateful</h3>
+
+    <p>
+      This is similar to ingress table <code>Pre-stateful</code>.
+    </p>
+
+    <h3>Egress Table 2: <code>to-lport</code> ACLs</h3>
 
     <p>
       This is similar to ingress table <code>ACLs</code> except for
       <code>to-lport</code> ACLs.
     </p>
 
-    <h3>Egress Table 2: Egress Port Security - IP</h3>
+    <h3>Egress Table 3: Egress Port Security - IP</h3>
 
     <p>
       This is similar to the port security logic in table
@@ -399,7 +417,7 @@  output;
       <code>ip4.src</code> and <code>ip6.src</code>
     </p>
 
-    <h3>Egress Table 3: Egress Port Security - L2</h3>
+    <h3>Egress Table 4: Egress Port Security - L2</h3>
 
     <p>
       This is similar to the ingress port security logic in ingress table
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 97ddf80..952a118 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -92,15 +92,17 @@  enum ovn_stage {
     PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_IP,    1, "ls_in_port_sec_ip")     \
     PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_ND,    2, "ls_in_port_sec_nd")     \
     PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,        3, "ls_in_pre_acl")      \
-    PIPELINE_STAGE(SWITCH, IN,  ACL,            4, "ls_in_acl")          \
-    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        5, "ls_in_arp_rsp")      \
-    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        6, "ls_in_l2_lkup")      \
+    PIPELINE_STAGE(SWITCH, IN,  PRE_STATEFUL,   4, "ls_in_pre_stateful")    \
+    PIPELINE_STAGE(SWITCH, IN,  ACL,            5, "ls_in_acl")          \
+    PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,        6, "ls_in_arp_rsp")      \
+    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        7, "ls_in_l2_lkup")      \
                                                                       \
     /* Logical switch egress stages. */                               \
-    PIPELINE_STAGE(SWITCH, OUT, PRE_ACL,     0, "ls_out_pre_acl")     \
-    PIPELINE_STAGE(SWITCH, OUT, ACL,         1, "ls_out_acl")         \
-    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_IP, 2, "ls_out_port_sec_ip")    \
-    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_L2, 3, "ls_out_port_sec_l2")    \
+    PIPELINE_STAGE(SWITCH, OUT, PRE_ACL,      0, "ls_out_pre_acl")     \
+    PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 1, "ls_out_pre_stateful")  \
+    PIPELINE_STAGE(SWITCH, OUT, ACL,          2, "ls_out_acl")         \
+    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_IP,  3, "ls_out_port_sec_ip")    \
+    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_L2,  4, "ls_out_port_sec_l2")    \
                                                                       \
     /* Logical router ingress stages. */                              \
     PIPELINE_STAGE(ROUTER, IN,  ADMISSION,   0, "lr_in_admission")    \
@@ -128,6 +130,8 @@  enum ovn_stage {
  * priority to determine the ACL's logical flow priority. */
 #define OVN_ACL_PRI_OFFSET 1000
 
+#define REGBIT_CONNTRACK_DEFRAG "reg0[0]"
+
 /* Returns an "enum ovn_stage" built from the arguments. */
 static enum ovn_stage
 ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline pipeline,
@@ -1377,13 +1381,34 @@  build_pre_acls(struct ovn_datapath *od, struct hmap *lflows,
          *
          * Regardless of whether the ACL is "from-lport" or "to-lport",
          * we need rules in both the ingress and egress table, because
-         * the return traffic needs to be followed. */
-        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip", "ct_next;");
-        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip", "ct_next;");
+         * the return traffic needs to be followed.
+         *
+         * 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table send
+         * it to conntrack for tracking and defragmentation. */
+        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip",
+                      REGBIT_CONNTRACK_DEFRAG" = 1; next;");
+        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
+                      REGBIT_CONNTRACK_DEFRAG" = 1; next;");
     }
 }
 
 static void
+build_pre_stateful(struct ovn_datapath *od, struct hmap *lflows)
+{
+    /* Ingress and Egress pre-stateful Table (Priority 0): Packets are
+     * allowed by default. */
+    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 0, "1", "next;");
+    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 0, "1", "next;");
+
+    /* If REGBIT_CONNTRACK_DEFRAG is set as 1, then the packets should be
+     * sent to conntrack for tracking and defragmentation. */
+    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 100,
+                  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
+    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 100,
+                  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
+}
+
+static void
 build_acls(struct ovn_datapath *od, struct hmap *lflows)
 {
     bool has_stateful = has_stateful_acl(od);
@@ -1505,6 +1530,7 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
         }
 
         build_pre_acls(od, lflows, ports);
+        build_pre_stateful(od, lflows);
         build_acls(od, lflows);
     }