[ovs-dev] [PATCH] ovn: Add stateful ACL support.
Justin Pettit
jpettit at nicira.com
Fri Oct 16 07:25:33 UTC 2015
> On Oct 15, 2015, at 5:21 PM, Ben Pfaff <blp at nicira.com> wrote:
>
> On Thu, Oct 15, 2015 at 10:32:51AM -0700, Justin Pettit wrote:
>> Add support for the "allow-related" ACL action. This is dependent on
>> the OVS conntrack functionality, which is not available on all platforms
>> or kernel versions.
>>
>> Here is a sample policy that will allow all tenants in logical switch
>> "ls0" to SSH to each other. Anyone can make an HTTP request to "lp0".
>> All other IP traffic is dropped:
>>
>> ovn-nbctl acl-add ls0 from-lport 100 ip allow-related
>> ovn-nbctl acl-add ls0 to-lport 100 tcp.dst==22 allow-related
>> ovn-nbctl acl-add ls0 to-lport 100 "outport == \"lp0\" \
>> && tcp.dst==80" allow-related
>> ovn-nbctl acl-add ls0 to-lport 1 ip drop
>>
>> Note: Kernel conntrack support is checked into the mainline Linux
>> kernel, but hasn't been backported to the main OVS repo yet.
>> ---
>> I've pushed this patch on a partial backport of conntrack here:
>>
>> https://github.com/justinpettit/ovs/tree/ovn-acl
>
> Thanks! This is going to be awesome.
>
> This lacks a Signed-off-by.
Whoops. Fixed.
> ovn-northd.xml needs an update to explain all the new flows and
> renumbered flow tables.
I totally missed that. Thanks.
> I get one "sparse" warning:
>
> ../ovn/lib/actions.c:151:13: warning: incorrect type in assignment (different base types)
> ../ovn/lib/actions.c:151:13: expected unsigned short [unsigned] [usertype] alg
> ../ovn/lib/actions.c:151:13: got restricted ovs_be16
D'oh. Fixed.
> In symtab_init() in ovn/controller/lflow.c, I think it would be a little
> better to define ct.trk as a subfield, instead of a predicate, since
> subfields are a little more general-purpose.
I couldn't get it to work by making it a predicate. I think it's related to the other ct_state fields depending on it, but let's discuss it tomorrow, because I'm probably missing something.
> Acked-by: Ben Pfaff <blp at nicira.com>
Thanks!
I went ahead an pushed it. I've appended an incremental.
--Justin
-=-=-=-=-=-=-=-=-=-=-=-
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 5a9562e..aebe5ce 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -206,7 +206,7 @@ emit_ct(struct action_context *ctx, bool recirc_next, bool c
ct->zone_src.n_bits = 16;
/* We do not support ALGs yet. */
- ct->alg = htons(0);
+ ct->alg = 0;
/* CT only works with IP, so set up a prerequisite. */
struct expr *expr;
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 3c5d362..f51852e 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -137,24 +137,63 @@
be dropped.
</p>
- <h2>Ingress table 1: <code>from-lport</code> ACLs</h2>
+ <h2>Ingress Table 1: <code>from-lport</code> Pre-ACLs</h2>
+
+ <p>
+ Ingress table 1 prepares flows for possible stateful ACL processing
+ in table 2. It contains a priority-0 flow that simply moves
+ traffic to table 2. 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 table 2.
+ </p>
+
+ <h2>Ingress table 2: <code>from-lport</code> ACLs</h2>
<p>
Logical flows in this table closely reproduce those in the
- <code>ACL</code> table in the <code>OVN_Northbound</code> database for
- the <code>from-lport</code> direction. <code>allow</code> and
- <code>allow-related</code> ACLs translate into logical flows with the
- <code>next;</code> action, others to <code>drop;</code>. The
- <code>priority</code> values from the <code>ACL</code> table are used
- directly.
+ <code>ACL</code> table in the <code>OVN_Northbound</code> database
+ for the <code>from-lport</code> direction. <code>allow</code>
+ ACLs translate into logical flows with the <code>next;</code>
+ action, <code>allow-related</code> ACLs translate into logical
+ flows with the <code>ct_next;</code> action, other ACLs translate
+ to <code>drop;</code>. The <code>priority</code> values from the
+ <code>ACL</code> table are used directly.
</p>
<p>
- Ingress table 1 also contains a priority 0 flow with action
- <code>next;</code>, so that ACLs allow packets by default.
+ Ingress table 2 also contains a priority 0 flow with action
+ <code>next;</code>, so that ACLs allow packets by default. If the
+ logical datapath has a statetful ACL, the following flows will
+ also be added:
</p>
- <h2>Ingress Table 2: Destination Lookup</h2>
+ <ul>
+ <li>
+ A priority-1 flow to commit IP traffic to the connection
+ tracker. This is needed for the default allow policy because,
+ while the initiater's direction may not have any stateful rules,
+ the server's may and then its return traffic would not be known
+ and marked as invalid.
+ </li>
+
+ <li>
+ A priority-65535 flow that allows any traffic that has been
+ committed to the connection tracker (i.e., established flows).
+ </li>
+
+ <li>
+ A priority-65535 flow that allows any traffic that is considered
+ related to a committed flow in the connection tracker (e.g., an
+ ICMP Port Unreachable from a non-listening UDP port).
+ </li>
+
+ <li>
+ A priority-65535 flow that drops all traffic marked by the
+ connection tracker as invalid.
+ </li>
+ </ul>
+
+ <h2>Ingress Table 3: Destination Lookup</h2>
<p>
This table implements switching behavior. It contains these logical
@@ -185,13 +224,20 @@
</li>
</ul>
- <h2>Egress Table 0: <code>to-lport</code> ACLs</h2>
+ <h2>Egress Table 0: <code>to-lport</code> Pre-ACLs</h2>
+
+ <p>
+ This is similar to ingress table 1 except for <code>to-lport</code>
+ traffic.
+ </p>
+
+ <h2>Egress Table 1: <code>to-lport</code> ACLs</h2>
<p>
- This is similar to ingress table 1 except for <code>to-lport</code> ACLs.
+ This is similar to ingress table 2 except for <code>to-lport</code> ACLs.
</p>
- <h2>Egress Table 1: Egress Port Security</h2>
+ <h2>Egress Table 2: Egress Port Security</h2>
<p>
This is similar to the ingress port security logic in ingress table 0,
More information about the dev
mailing list