[ovs-dev] [ovs-dev, RFC] ovn: support for service function chaining

Ben Pfaff blp at ovn.org
Fri Feb 3 22:10:53 UTC 2017


On Thu, Feb 02, 2017 at 03:22:56PM -0800, jmcdowall at paloaltonetworks.com wrote:
> From: John McDowall <jmcdowall at paloaltonetworks.com>
> 
> This patchset is the first round at having Service Function Chaining
> functionality through OVN. The implementation is done entirely
> on the northbound side of OVN. It is a bump on the wire implementation,
> so no attempt is being made in keeping state while packets visit each
> hop of the chain. ACLs are used as the classifiers, with the augmentation
> of action SFC, as well as option column.

Thanks for working on this!  Sorry it's taken so long to review.

cmp_port_pair_groups() treats two pair groups as equal if either one of
them has no keys, but this violates transitivity (see
https://en.wikipedia.org/wiki/Comparison_sort).  Example: if 0 is a pair
group with no keys and x and y are arbitrary pair groups, then this
function will conclude that x <= 0 and 0 <= y.  By transitivity, we
would also have x <= y, but that's a contradiction because x and y are
arbitrary.

I suggest that empty pair groups should sort as less than nonempty ones,
e.g.:

@@ -2772,8 +2772,10 @@ cmp_port_pair_groups(const void *ppg1_, const void *ppg2_)
     const struct nbrec_logical_port_pair_group *ppg1 = *ppg1p;
     const struct nbrec_logical_port_pair_group *ppg2 = *ppg2p;
 
-    if (ppg1->n_sortkey == 0 || ppg2->n_sortkey == 0) {
-        return 0;
+    if (ppg1->n_sortkey == 0) {
+        return ppg2->n_sortkey == 0 ? -1 : 0;
+    } else if (ppg2->n_sortkey == 0) {
+        return 1;
     }
 
     const int64_t key1 = ppg1->sortkey[0];

ovn-northd.c has some new uses of VLOG without rate-limiters.  These
should probably be rate-limited, like most VLOG calls in ovn-northd.

VLOG messages shouldn't include a new-line since VLOG takes care of that
itself.

The code has lots of lines longer than the coding style suggested limit
of 79 columns.

Comments should generally begin with a capital letter and end with a
period.

In build_chain_classifier_entry(), qsort() is a remarkably expensive way
to find the smallest element in an array.

In build_chain_classifier_entry(), it's strange to have "" in the middle
of the string here:
    ds_put_format(ds_action, "outport = %s;"" output;", first_ovn_port->json_key);

In build_chain(), please don't comment out code:
    //const uint16_t egress_inner_priority = 150;

In build_chain(), a lot of debug logging has escaped as VLOG_INFO calls.

In build_chain(), you can use xmemdup() instead of xmalloc() followed by
memcpy().

In build_chain(), I don't know why there's a line-splicing \ here:
                    VLOG_INFO("Warning: Currently lacking support for more than one port-pair %"PRIuSIZE"\n", \
                              lppg->n_port_pairs);
Also, we'd usually just use VLOG_WARN instead of writing "Warning:".

In build_chain(), please always put {} around a conditional statement,
e.g. here:
                if (!input_port_array[j] || !output_port_array[j]) obtainedAllNeededInfo = false;

Please wrap at 79 columns in ovn-architecture.7.xml as well.

I spent some time with the documentation.  I worked on getting rid of
passive voice because it's often unclear, e.g. "something happens"
doesn't tell the reader who does it, but "The CMS does something" does.
I dropped a lot of the items from the life cycle that didn't seem
really relevant to VNFs (it seemed like they were mostly cut-and-paste).

Some of the introductory documentation confuses me though.  The
following paragraph mentions about the OVN northbound database but it
seems to actually be about the southbound database.  I don't think this
is the right place to explain the logical flows that SFC puts into the
southbound database; that should be in ovn-northd.8.xml instead.  I
deleted the following paragraph but presumably it should be adapted for
ovn-nrothd.8.xml:

 <p>
   Service insertion is implemented by adding 4 new flow rules into the OVN northbound database for
   each VNF inserted. The rules are added into the last table in the ingress path (L2_LKUP).
   The service insertion rules have a higher priority than the standard forwarding rules.
   This means that they override the existing forwarding rules. There are four
   new rules added for each insertion. Two ingress and two egress, The first ingress
   rule sends all traffic destined for the application into the VNF ingress port and the
   second rule takes all traffic destined to the application from the VNF egress port
   to the application, the priorities are such that the second rule is always checked
   first. In the egress direction the rules are similar if the traffic is from the
   application it is sent to the VNF egress port and if if is from the application
   and is from the VNF ingress port it is delivered to the destination.
   <!-- Should this be a new table or is it a naturally part of the L2 lookup table ? -->
 </p>

Also this paragraph is confusing because the patch doesn't add any new
table named Service or Logical_Service or anything like that:

 <p>
   The steps in this example refer to the details of the OVN Northbound database schema.
   There is a new table in the OVN Northbound database to support service insertion
   called <code>Services</code>, which contains the required information for each new
   service inserted. The same service can be used for multiple applications, as
   there is typically an N:1 relationship between applications and VNFs. A
   single VNF may be part of several service insertions, but each one is logically
   separate.
 </p>

The ovn-nb documentation for port_chains says, "It is an error for
multiple logical switches to include the same logical port."  Should
this say "same port_chain" instead?

The ovn-nb documentation for Logical_Switch does not explain port chains
or port pairs.  I think that it is necessary to give an overview of what
they mean and how they should be defined; as is, I do not understand
them.  It might be reasonable to add a "Service Function Chaining"
<group> with some introductory material and then document these two
columns in there.

The new ovn-nbctl commands need documentation.

ovn-nbctl.c has a weird "#ifdef JED" block.

I'm appending my suggested documentation updates.  You can also find
these on the "sfc" branch at https://github.com/blp/ovs-reviews/tree/sfc

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

diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
index ee69fed..4aa52f3 100644
--- a/ovn/ovn-architecture.7.xml
+++ b/ovn/ovn-architecture.7.xml
@@ -386,7 +386,7 @@
       <dfn>Logical services</dfn> are logical references to virtual network functions
       (VNF). Adding a logical service requires adding steering rules to the OVN Northbound
       database. These are the only rules necessary to implement traffic steering for VNFs.
-      The section below "<code>Life Cycle of an inserted VNF</code>" provides more details.
+      See <code>Life Cycle of an inserted VNF</code>, below, for details.
     </li>
     <li>
       <p>
@@ -572,181 +572,79 @@
     </li>
   </ol>
 
- <h2>Life Cycle of an inserted Virtual Network Function (VNF)</h2>
+ <h2>Life Cycle of an Inserted Virtual Network Function (VNF)</h2>
 
  <p>
    OVN provides an abstraction to enable the insertion of an arbitrary virtual network
    function (VNF) into the path of traffic to and from an application. A VNF is different
-   from an application VM in that it acts on traffic between applications, and im most
-   cases does not terminiate a flow. Proxy functions are an exception as they terminate the
-   flow from the src and create a new flow to the dst. Examples of VNF's are security functions,
-   load balancing, and traffic enhancement services, this is not a complete list.
+   from an application VM in that it acts on traffic between applications, and in most
+   cases does not terminate a flow. Proxy functions are an exception as they terminate the
+   flow from the source and create a new flow to the destination. Examples of VNFs are security functions
+   (e.g. intrusion detection systems),
+   load balancers, and traffic enhancement services.
  </p>
  <p>
-   The requirements on the VNF to be inserted are minimal, it must
+   The requirements on the VNF to be inserted are minimal: it must
    act as a <code>bump in the wire (BITW)</code> and can have one or two virtual network
-   ports for traffic. If it has two network ports traffic is directed into one and out the other,
-   if it has only one port, then traffic is bi-directional. The requirement for the VNF to act as
-   a BITW removes the need for the VNF to participate in L3/L2 networking which provides
+   ports for traffic. If it has two network ports, it accepts traffic on one port and
+   transmits it out the other;
+   if it has only one port, that port serves both purposes. The requirement for the VNF to act as
+   a BITW removes the need for the VNF to participate in L2/L3 networking, which provides
    improved agility and reduces the coupling between OVN and the VNF.
  </p>
  <p>
-   The service insertion is implemented by adding 4 new flow rules into the ovn-nb database for
-   each VNF inserted. The rules are added into the last table in the ingress path (L2_LKUP).
-   The service insertion rules have a higher priority than the standard forwarding rules.
-   This means that they override the existing forwarding rules. There are four
-   new rules added for each insertion. Two ingress and two egress, The first ingress
-   rule sends all traffic destined for the application into the VNF ingress port and the
-   second rule takes all traffic destined to the application from the VNF egress port
-   to the application, the priorities are such that the second rule is always checked
-   first. In the egress direction the rules are similar if the traffic is from the
-   application it is sent to the VNF egress port and if if is from the application
-   and is from the VNF ingress port it is delivered to the destination.
-   <!-- Should this be a new table or is it a naturally part of the L2 lookup table ? -->
- </p>
- <p>
    The steps in this example refer to the details of the OVN Northbound database schema.
    There is a new table in the OVN Northbound database to support service insertion
-   called <code>Services</code> this contains the required information for each new
+   called <code>Services</code>, which contains the required information for each new
    service inserted. The same service can be used for multiple applications, as
-   there is typically a n:1 relationship between applications and VNFs. A
+   there is typically an N:1 relationship between applications and VNFs. A
    single VNF may be part of several service insertions, but each one is logically
    separate.
  </p>
  <p>
-   The steps in this example refer often to details of the OVN and OVN
-   Northbound database schemas.  Please see <code>ovn-sb</code>(5) and
-   <code>ovn-nb</code>(5), respectively, for the full story on these
-   databases. The ovn-nb database has specific schema enhancements for the service
-   insertion function. The ovn-sb database has no schema changes for the
-   service insertion function.
- </p>
- <p>
    The following steps are an overview to inserting a new VNF into the traffic path.
    The sections below go into each step in more detail.
  </p>
  <ol>
    <li>
-     The service insertion lifecycle begins when a CMS administrator creates a new
-     virtual network function <code>(VNF)</code> using the CMS user
+     The CMS administrator creates a new
+     virtual network function <code>(VNF)</code>, using the CMS user
      interface or API. The CMS administrator creates the logical ports (ingress and egress)
-     for the VNF. If the CMS is Openstack this will create a reusable port-pair defining the
-     interface to the VNF. There is also typically a seperate management port for the VNF,
+     for the VNF. If the CMS is OpenStack, this creates a reusable port-pair defining the
+     interface to the VNF. The administrator also typically creates a separate management port for the VNF,
      but that is not relevant to the service insertion workflow. A single VNF can
      participate with several applications, either as a security VM, protecting multiple
-     applications or as a load balancer VM, distributing load across multiple applications.
+     applications, or as a load balancer VM, distributing load across multiple applications.
    </li>
 
    <li>
-     The next step in the life cycle occurs when a CMS administrator creates a new application
-     with a VIF using the CMS user interface or API and adds it to a switch (one
-     implemented by OVN as a logical switch). Alternatively an already running application could
-     be used.
-
-     The CMS can now attach the port pair to the VIF by defining the logical port in the
-     service function classifier. This will direct traffic to the VIF to go through
-     the VNF, applying the rules discussed earlier.
+     The CMS administrator creates a new application with a VIF using the CMS
+     user interface or API (or chooses a running application) and adds it to an
+     OVN logical switch.
    </li>
 
    <li>
-     While within CMS the service insertion may be broken down into multiple reusable steps
-     (as is the case with Openstack). Within OVN the creating of a new service insertion
-     is treated as an atomic operation. This enables the easy atomic insertion and deletion of
-     service insertions. This is important as it is envisioned that the number of serivce
-     insertions can easily number in the hundreds, all with separate lifecycles. For each new
-     service insertion operation a new row is added to the OVN Northbound Database. The new row is
-     only added to the ovn-nb when the VNF, application and network are enabled by the CMS.
-
-     Once the serivce insertion is applied to the ovn-nb database, the ovn-nb controller will be
-     notified of a change and the rules will be pushed down to the specific OVS instances, using
-     the existing OVN mechanisms. It is important to note here that the logical abstraction of
-     OVN enables service insertion with minimal changes.
+     The CMS administrator attaches the VNF port pair to the VIF by defining
+     the logical port in the service function classifier.  To do so, the CMS
+     inserts a row into the <code>ACL</code> table in the OVN northbound
+     database.  The ACL's <code>type</code> is <code>sfc</code> and its
+     <code>options:sfc-port-chain</code> designates the VNF.  This directs
+     traffic to the VIF to go through the VNF, applying the rules discussed
+     earlier.
    </li>
 
    <li>
-     When the application VM shuts down the classification rule should be removed, however as
-     no traffic will be destinated to the application there would be no issues. If the VM is
-     being moved and the new application VM port is used to update the service then the change
-     would be detected and the rules pushed down.
-   </li>
-   <li>
-     A VNF can be removed at any time and traffic flows will revert to the pre-VNF traffic
-     paths if it is removed from the ovn-nb database. OVN must detect that the VNF has been
-     shut-off as it must remove all the rules that are attached to that VNF. Otherwise
-     traffic loss will occur, if a failure in a VNF occurs that is not detected.
-  </li>
-
-  <li>
-    On every hypervisor, <code>ovn-controller</code> receives the
-    <code>Logical_Service</code> table updates that <code>ovn-northd</code> made
-    in the previous step.  As long as the VM that owns the VIF is powered
-    off, <code>ovn-controller</code> cannot do much; it cannot, for example,
-    arrange to send packets to or receive packets from the VIF, because the
-    VIF does not actually exist anywhere. In addition the VNF cannot be inserted
-    into the traffic path as it will have no source to forward traffic too.
-    <!-- If there is no VM then traffic cannot be sent to it therefore having the
-    rules inserted is probably not an issue? -->
-  </li>
-
-  <li>
-    Some CMS systems, including OpenStack, fully start a VM only when its
-    networking is ready.  To support this, <code>ovn-northd</code> notices
-    the <code>chassis</code> column updated for the row in
-    <code>Binding</code> table and pushes this upward by updating the
-    <ref column="up" table="Logical_Port" db="OVN_NB"/> column in the OVN
-    Northbound database's <ref table="Logical_Port" db="OVN_NB"/> table to
-    indicate that the VIF is now up.  The CMS, if it uses this feature, can
-    then react by allowing the VM's execution to proceed.
-
-    Traffic now flows into and out of the VM that has a VNF inserted in its
-    datapath. The rules are applied to direct traffic to the VNF on the way
-    into the VM and on the way out of the VM.
-  </li>
-
-  <!-- Need a section on having multiple VM's using the same VNF
-    ( physcially or logically ). Different rule sets -->
-  <!-- Need a section on distributed cases - one section for each -->
-  <li>
-    On every hypervisor but the one where the VIF resides,
-    <code>ovn-controller</code> notices the completely populated row in the
-    <code>Binding</code> table.  This provides <code>ovn-controller</code>
-    the physical location of the logical port, so each instance updates the
-    OpenFlow tables of its switch (based on logical datapath flows in the OVN
-    DB <code>Logical_Flow</code> table) so that packets to and from the VIF
-    can be properly handled via tunnels.
-  </li>
-  <!-- Current implementation is open on delete, i.e. when the VNF is removed
-    the datapath behaviour reverts to the pre-existing paths. Does this make sense?
-    - could argue that close on delete should be an option ? -->
-
-  <li>
-    Eventually, a user removes the inserted service function from the ovn nb
-    database. The rules are then updated in the southbound database and pushed
-    down to the relevant ovs. No other SIF is impacted as the row in the ovn nb
-    is independant of all the other SIF.
-    <!-- This is really important in the case where many SIF are being added
-      and removed. Without both the independance of the enteries confusion would
-      exist. Also for debugging - possible to remove/add individual VNF's to
-      determine potentail problems. -->
-  </li>
-
-  <li>
-    The CMS plugin removes the SIF from the OVN Northbound database,
-    by deleting its row in the <code>Logical_Service</code> table.
-  </li>
-
-  <li>
-    <code>ovn-northd</code> receives the OVN Northbound update and in turn
-    updates the OVN Southbound database accordingly, by removing or updating
-    the rows from the OVN Southbound database <code>Logical_Service</code> table.
-  </li>
+     <p>
+       Eventually, the application VM shuts down and the CMS removes the
+       <code>sfc</code> ACL.  (However, it is harmless if it remains, since no
+       traffic will be sent to the application.)
+     </p>
 
-  <li>
-    On every hypervisor, <code>ovn-controller</code> receives the
-    <code>Logical_Service</code> table updates that <code>ovn-northd</code> made
-    in the previous step.  <code>ovn-controller</code> updates OpenFlow
-    tables to reflect the update. The datapath for the VM will now revert to
-    paths that existed before the VNF was inserted into the data path.
+     <p>
+       Alternatively, eventually the CMS administrator detaches the VIF from
+       the VNF.  The CMS deletes the <code>sfc</code> ACL row, and traffic
+       reverts to the pre-VNF traffic paths.
+     </p>
   </li>
 </ol>
 
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index e45c670..b952a46 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -994,9 +994,7 @@
 
         <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="options"/>.
+          The <ref column="options"/> column provides details.
         </li>
       </ul>
     </column>
@@ -1014,7 +1012,7 @@
     </column>
 
     <group title="Options">
-      <column name = "options">
+      <column name="options">
         This column provides key/value settings specific to the ACL
         <ref column="action"/>. The type-specific options are described
         individually below.


More information about the dev mailing list