[ovs-discuss] OVN SFC: Changes to include ACL based classifiers

Flavio Fernandes flavio at flaviof.com
Wed Nov 2 10:29:51 UTC 2016


> On Nov 1, 2016, at 1:30 PM, Flaviof <flavio at flaviof.com> wrote:
> 
> 
> 
> On Tue, Nov 1, 2016 at 1:05 PM, John McDowall <jmcdowall at paloaltonetworks.com>wrote:
> So we would have something like:
> 
>  
> 
> $ ovn-nbctl acl-add sw0 to-lport 1003 'outport == "sw0-port1" && ip'  sfc-action sfc-stage external_ids:lsp_chain_id=”chain-id”
> 
>  
> 
> The chain-id would be passed as metadata with the packet to the ls_in_chain stage where it would be processed according to the current state of its in/out ports in the chain.
> 
>  
> 
> Where sfc is the stage and the action – would the SFC ACL Table have any other action other than SFC? It seems a little redundant – not sure if there is a better way though.
> 
> 
> 
> 
> Right. If I understood correctly, the sfc-stage is optional and may be something we
> may add later on to ACLs. For now, having it all in a sigle stage will not invalidate
> that effort.
> 
> Using the example comand, my main 'focus' is actually in regards to what else goes as 
> external_ids. I can see that besides 'lsp_chain_id', we will need 'last_hop_port', and
> possibly 'bidirectional'. Sounds right?
>  
> I will send an email with a proposed schema+xml on this shortly.

It's a pretty simple change [1], as expected. :) Does that jive well with the changes
you had in mind? A small caveat here is in regards to additional attributes
the chain needs in order to create the end to end rules. That includes 'chain_uuid',
'last_hop_port' at a minimum. Other than using 'external_ids', I cannot see where else
to provide them. Any better ideas?

Side note: while inspecting that code I found a leak. Ben has merged the fix for that
already [leak].

Thanks,

-- flaviof


[leak]: https://patchwork.ozlabs.org/patch/690150/


[1]:

https://github.com/flavio-fernandes/ovs/commit/3edd8b23be33e5e206470c86e1fd2ac12864217b


commit 3edd8b23be33e5e206470c86e1fd2ac12864217b
Author: Flavio Fernandes <flavio at flaviof.com>
Date:   Wed Nov 2 05:17:03 2016 -0500

    ovn: initial acl changes to support sfc

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index df53d4c..f2caf3b 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -304,6 +304,12 @@
         connections.
       </li>
       <li>
+        <code>sfc</code> ACLs work as entry points for service function
+        chaining, also known as SFC classifiers. Further attributes such
+        as what chain to be used and the last_hop_port are provided via
+        external_ids of the ACL.
+      </li>
+      <li>
         Other ACLs translate to <code>drop;</code> for new or untracked
         connections and <code>ct_commit(ct_label=1/1);</code> for known
         connections.  Setting <code>ct_label</code> marks a connection
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 91affe4..fdb7679 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2522,6 +2522,15 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)

                 ds_destroy(&match);
             }
+        } else if (!strcmp(acl->action, "sfc")) {
+            struct ds match = DS_EMPTY_INITIALIZER;
+
+            // XXX FIXME (FF): Do something amazing here
+            ovn_lflow_add(lflows, od, stage,
+                          acl->priority + OVN_ACL_PRI_OFFSET,
+                          acl->match, "drop;");
+
+            ds_destroy(&match);
         } else if (!strcmp(acl->action, "drop")
                    || !strcmp(acl->action, "reject")) {
             struct ds match = DS_EMPTY_INITIALIZER;
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 865dd34..87994af 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Northbound",
-    "version": "5.4.0",
-    "cksum": "4176761817 11225",
+    "version": "5.4.1",
+    "cksum": "3029861762 11232",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -123,7 +123,7 @@
                                             "enum": ["set", ["from-lport", "to-lport"]]}}},
                 "match": {"type": "string"},
                 "action": {"type": {"key": {"type": "string",
-                                            "enum": ["set", ["allow", "allow-related", "drop", "reject"]]}}},
+                                            "enum": ["set", ["allow", "allow-related", "drop", "reject", "sfc"]]}}},
                 "log": {"type": "boolean"},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index e1b3136..bcac634 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -841,6 +841,13 @@
           ICMP unreachable message for other IP-based protocols.
           <code>Not implemented--currently treated as drop</code>
         </li>
+
+        <li>
+          <code>sfc</code>: Forward the packet into a logical port chain.
+          The chain to be used -- as well as any other attributes that determine
+          the behavior of the packet while in the chain -- are provided
+          via <ref column="external_ids"/>.
+        </li>
       </ul>
     </column>

@@ -858,8 +865,39 @@

     <group title="Common Columns">
       <column name="external_ids">
-        See <em>External IDs</em> at the beginning of this document.
-      </column>
+        <p>
+          See <em>External IDs</em> at the beginning of this document.
+        </p>
+
+        <p>
+          When action used is <code>sfc</code>, the following key-value pairs are
+          expected in order to further specify the action behavior:
+        </p>
+        <ul>
+          <li>
+            <p>
+              <code>port_chain</code> (mandatory): the value is the uuid of the port chain to be used.
+            </p>
+          </li>
+          <li>
+            <p>
+              <code>last_hop_port</code> (mandatory if <code>outport</code> is not part of match): the value
+              is the uuid of <ref table="Logical_Switch_Port"/> to be used once packet reaches the end of the
+              chain.
+            </p>
+          </li>
+          <li>
+            <p>
+              <code>bidirectional</code> (optional): if this key is set with value <code>true</code>, the
+              implementation will also add rules to make packets go through the chain in reverse direction. A
+              restriction on making bidirectional chains is that the inport parameter must be
+              present in the match, as it will be used as the <code>last_hop_port</code>. As expected, all
+              <code>src</code> fields in the match will be converted to <code>dst</code> in order to derive
+              the reverse ACL.
+            </p>
+          </li>
+        </ul>
+    </column>
     </group>
   </table>

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index df1c405..ac66024 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1279,9 +1279,10 @@ nbctl_acl_add(struct ctl_context *ctx)

     /* Validate action. */
     if (strcmp(action, "allow") && strcmp(action, "allow-related")
-        && strcmp(action, "drop") && strcmp(action, "reject")) {
+        && strcmp(action, "drop") && strcmp(action, "reject")
+        && strcmp(action, "sfc")) {
         ctl_fatal("%s: action must be one of \"allow\", \"allow-related\", "
-                  "\"drop\", and \"reject\"", action);
+                  "\"drop\", \"reject\" and \"sfc\"", action);
         return;
     }



> 
> -- flaviof
> 
>  
> 
> Regards
> 
>  
> 
> John
> 
>  
> 
>  
> 
>  
> 
> From: Flaviof <flavio at flaviof.com>
> Date: Tuesday, November 1, 2016 at 6:53 AM
> To: Russell Bryant <russell at ovn.org>
> Cc: discuss <discuss at openvswitch.org>, John McDowall <jmcdowall at paloaltonetworks.com>, Russell Bryant <russell at russellbryant.net>, Farhad Sunavala <Farhad.Sunavala at huawei.com>
> Subject: Re: [ovs-discuss] OVN SFC: Changes to include ACL based classifiers
> 
>  
> 
>  
> 
>  
> 
> On Tue, Nov 1, 2016 at 8:55 AM, Russell Bryant <russell at ovn.org> wrote:
> 
>  
> 
>  
> 
> On Tue, Nov 1, 2016 at 11:09 AM, Flaviof <flavio at flaviof.com> wrote:
> 
> [cc: John, Louis, Farhad, Russell] 
> 
>  
> 
> Hi folks,
> 
>  
> 
> Picking up from where we left off at the summit [1], I took
> 
> a stab at the nb schema changes to represent what I
> 
> understood Russell and others saying on how we could
> 
> use a secondary table of ACLs to serve as the SFC
> 
> classifiers: [2].
> 
>  
> 
> What I had in mind was proceeding with a proposal like this one where we change ACLs to have multiple stages.  This patch proposed two, but I think we later talked about extending it to have more (8 perhaps?).
> 
>  
> 
> http://openvswitch.org/pipermail/dev/2016-July/076674.html
> 
>  
> 
> Then if SFC was an ACL action, you could put it in any stage of ACLs you want, with other things before or after as desired.
> 
>  
> 
>  
> 
> I see. I like that! Let me better understand the code changes from that
> 
> email.
> 
>  
> 
> Thanks,
> 
>  
> 
> -- flaviof
> 
>  
> 
>  
> 
> Does it look right to you? If so, I will start making the
> 
> changes to incorporate that and obsolete the classifier based
> 
> code [3]. I'm not sure if I will be able to migrate to this new
> 
> table in time for the talk at OVSCon [4], but I will try.
> 
>  
> 
> Thanks,
> 
>  
> 
> -- flaviof
> 
>  
> 
> [1]: https://etherpad.openstack.org/p/r.f7cebb215b63ae657d91a28ab0da42bf
> 
>  
> 
> [2]: https://github.com/doonhammer/ovs/pull/3/commits/b10224a07de2970358eb5e105146ef1d5f5eca6d
> 
>  
> 
> [3]: https://github.com/doonhammer/ovs/pull/3/commits/2ebea7881c523dd356cd043a24531c268bddf6b4#diff-2c35162acf6ad144624954fdc4c3d9f4R2505
> 
>  
> 
> [4]: http://sched.co/8aZE
> 
>  
> 
>  
> 
> 
> _______________________________________________
> discuss mailing list
> discuss at openvswitch.org
> http://openvswitch.org/mailman/listinfo/discuss
> 
> 
> 
> 
>  
> 
> -- 
> 
> Russell Bryant
> 
>  
> 
> 
> _______________________________________________
> discuss mailing list
> discuss at openvswitch.org
> http://openvswitch.org/mailman/listinfo/discuss




More information about the discuss mailing list